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

[Michel Bardiaux]

Michael Niedermayer wrote:
> 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 ...

But I can make an educated guess...

> 
> 
>> Do you really want one?
> 
> yes

OK, phase 1 will be changing the API filename_number_test as above. No 
change to find_image_range and get_frame_filename, right? And you also 
mentionned they should actually be av_ prefixed, can that go in the same 
patch?

After that, change to support only of numbered sequences in output and 
cataloged sequences (file and stdin) for input?

> 
> [...]


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/