[Ffmpeg-devel] [PATCH] Wildcard and catalog image sequences

[Michael Niedermayer]

Hi

On Wed, Aug 30, 2006 at 03:37:33PM +0200, Michel Bardiaux wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> [snip]
> >>>>>+int filename_catalog_test(const char *filename)
> >>>>>+{
> >>>>>+    if(!filename)
> >>>>>+        return (-1);
> >>>>>+    else if(filename[0]=='@')
> >>>>>+        return 0;
> >>>>>+    else
> >>>>>+        return (-1);
> >>>versus
> >>>
> >>>>return filename && filename[0]=='@';
> >>>i cant see how the later is more obfuscated, additionally
> >>I dont mean all one-liners are obfuscated, only that they quickly become 
> >>so. Esp. since the correct one is
> >>
> >>return (filename && filename[0]=='@')?0:-1; // justified below.
> >>
> >>Still reasonable, but the one about wildcards is even bigger. If one has 
> >>to code for multiple levels of conditions, nested ?: become quite 
> >>unreadable. So there is a limit to one-liners, and where it is is a 
> >>matter of taste, isnt it?
> >
> >sure but my suggestion wouldnt need the ?: and without that the one liner
> >really seems to be the better solution
> 
> But your suggestion doesnt conform to the API! (And IIRC *your* API, 
> arent you the prime author of img2.c?) Of course, the awkwardness of the 

the offending stuff is in utils.c and used by img.c too ...


> one-liner can be considered a reason to *change* the API as you suggest 
> below.

yes


> 
> >
> >
> >>You dont dispute that the object code size is likely to be the same?
> >
> >it will likely be similar yes
> >
> >
> >>>1. is the NULL check really needed at all?
> >>I dont know: there was one in the original filename_number_test() so I 
> >>put one too. Keeping maximum symmetry between all 3 kinds of sequences 
> >>seemed a good strategy for implementation.
> >
> >yes, and factoring that check out and putting it before the decission which
> >of the 3 functions is called also seems like its a good idea
> 
> But it cant be factored out of filename_number_test because that is 
> called for output sequences too. And actually *every* call to one of the 
> probes would have to have a test for !filename before. The end result 
> would probably be *more* lines of code.

ok then leave the NULL check in there


> 
> >
> >
> >>>2. in C 0 is false not 0 is true, using 0 as true is broken as logical 
> >>>and/or
> >>>  will no longer have their intuitive meaning, do you disagree here?
> >>Not at all. I knew you want patches to be as little intrusive as 
> >>possible, so I tried not to change anything to numbered sequences, which 
> >>are known to work. Hence I had to conform to the API defined by 
> >>get_frame_filename and filename_number_test, which is on the lines of 
> >>UNIX-style syscalls. Maybe this has to be changed, but shouldnt that be 
> >>a different patch?
> >
> >of course it should be a seperate patch :)
> 
> I wonder what your reaction would have been to a patch just to change 
> *that* (filename_number_test returning 0/1 instead of -1/0) coming out 
> of the blue... 

i dont know ...


> Do you really want one?

yes

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is