[FFmpeg-devel] [PATCH] [2/??] [3/3] Filter graphs - Parser for a graph description

[Vitor Sessak]

Michael Niedermayer wrote:
> On Wed, Mar 19, 2008 at 07:48:04PM +0100, Vitor Sessak wrote:
>> Hi Michael, Vmrsss
>>
>> Michael Niedermayer wrote:
>>> On Wed, Mar 19, 2008 at 03:01:41PM +0000, vmrsss wrote:
>>>> Hi Michael,
>>>>
>>>> On 19 Mar 2008, at 14:00, Michael Niedermayer wrote:
>>> [...]
>>>>> [...]
>>>>>> As for the extra operator, you are right. However, I claim that in
>>>>>> order to allow reusable libraries of graphs in your syntax you will
>>>>>> have to provide a renaming operator, eg:
>>>>>>
>>>>>>  	{tmp1/in} (in)overlay(out) {tmp2/out}  = (tmp1) overlay (tmp2)
>>>>> You are inventing some syntax and then complain about it, there is  
>>>>> nothing
>>>>> in the code nor in any mails from vitor suggesting anything like  
>>>>> above.
>>>> Hmm, I might not have made myself clear: sure you haven't suggested  
>>>> anything like the above, I am arguing (and made a specific example in  
>>>> support) that you **have** to if you want to reuse filters.
>>> Look, as there is nothing specified about "reuseable libraries" any
>>> system/syntax could be used, this clearly would allow to use "your syntax"
>>> as well thus this is a proper proof that it cannot be worse than "your
>>> syntax".
>> One problem about libraries of filter chains is that ideally they should 
>> be more flexible than just some graph hunk. Citing an example of what 
>> I'd like to be possible:
>>
>> AVFilter avfilter_vf_rotate =
>> {
>>      .name = "rotate";
>>      .init = init;
>>
>>      {some magic to make it a pseudofilter}
>> }
>>
>> static void init(char *args, AVFilter *ctx) {
>>      int ang= atoi(args);
>>      switch (ang) {
>>      case 90:
>>          ctx->actual_filter = "transpose,vflip";
>>          break;
>>      case 180:
>>          ctx->actual_filter = "hflip,vflip";
>>          break;
>>      case 270:
>>          ctx->actual_filter = "vflip,transpose";
>>          break;
>>      else
>>          snprintf(ctx->actual_filter, "rotate_slow=%d", ang);
>>          break;
>>      }
>> }
>>
>> But for doing this it would have to be a real filter, not something that 
>> pasted some code while parsing the chain.
> 
> no, implement 1 filter doing generic rotations/flip/transpose with special
> cases for what can be done faster.
> And then just replace "transpose" / "vflip" / "hflip" by the appropriate
> generic filter string
> 
> no code duplication, simple and fast

Well, it's only simpler if you consider the pseudo-filter framework as 
part of the complexity of the rotate filter. It's usually simpler to 
read/maintain pieces code that do just one operation.

Also, I cited that more as an example. One could think in more cases 
like using movie_src, set_pts and overlay to make a add_logo filter, 
etc. Anyway, if this complexity is ever added, the pseudo-filter code 
now in avfiltergraph.c would be only used for those pseudo-filters, and 
not always as is in current code.

-Vitor