[Ffmpeg-devel] [PATCH] Wildcard and catalog image sequences
Wed Aug 30 15:37:33 CEST 2006
Michael Niedermayer wrote:
>>>>> +int filename_catalog_test(const char *filename)
>>>>> + if(!filename)
>>>>> + return (-1);
>>>>> + else if(filename=='@')
>>>>> + return 0;
>>>>> + else
>>>>> + return (-1);
>>>> return filename && filename=='@';
>>> 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:-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
one-liner can be considered a reason to *change* the API as you suggest
>> 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.
>>> 2. in C 0 is false not 0 is true, using 0 as true is broken as logical
>>> 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... Do you really want one?
T +32  2 790 29 41
F +32  2 790 29 02
E mailto:mbardiaux at mediaxim.be
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
More information about the ffmpeg-devel