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

[Vitor Sessak]

Hi, and thanks for the review

Michael Niedermayer wrote:
> On Wed, Apr 02, 2008 at 07:02:27PM +0200, Vitor Sessak wrote:
>> Hi
>>
>> Michael Niedermayer wrote:
>>> On Tue, Apr 01, 2008 at 11:43:55PM +0200, Michael Niedermayer wrote:
>>>> On Tue, Apr 01, 2008 at 10:49:39PM +0200, Vitor Sessak wrote:
>>>>> Hi
>>>>>
>>>>> Michael Niedermayer wrote:
>>>>>> On Sun, Mar 30, 2008 at 01:50:51PM +0200, Vitor Sessak wrote:
>>>>> (...)
>>>>>
>>>>>>> static int query_formats(AVFilterGraph *graph)
>>>>>>> {
>>>>>> Now i dont think this belongs in the parser code ...
>>>>>> There should be IMHO
>>>>>> A. code for a filter graph (a single flat filter graph that is a filter 
>>>>>> graph
>>>>>>    is not a filter) If we ever see the need for having filter graphs be
>>                                                            ^^^^^^^^^^^^^
>>>>>>    filters this can be added later as a filter easily. Not weirdly
>>         ^^^^^^^
>>>>>>    intermingled with everything else.
>>>>>> B. the parser
>>>>> This function is _not_ passed as a callback to AVFilter.query_formats 
>>>>> (but it was some svn revs ago). It is called by the code 
>>>>> (avfilter_graph_config_formats()) that tries to set a agreed upon filter 
>>>>> format for all filters.
>>>> This does NOT belong in the parser! Absolutely nothing related to 
>>>> colorspace
>>>> negotation belongs in the parser. No matter in what form.
>>> A Parser takes a string and builds a graph of filters. At that point the
>>> parser is done with its work and you have a bunch of filters with links
>>> between them.
>>> Next and indepednant on how the graph was constructed some colorspace
>>> format resolving code would be run over the graph, this might insert
>>> scale filters.
>>> And as last step all the remaining init/config is done this again should
>>> be independant of how the previous steps have been done.
>>> Is there some problem/disadvantage with my suggestion? If so please 
>>> elaborate.
>> No, I agree that it hasn't much to do with the parser. What I thought was 
>> that the 100 lines of the common graph code didn't deserve its own file. 
>> When I replied to your previous message, I thought you didn't notice that 
>> filter graphs as a filter is gone, so I wanted to make clear it is.
>>
>> But anyway, I'm not against of having this code in its own file, as 
>> attached.
> 
> thanks, i think its much better to have this in a seperate file.

(... review skipped ...)

All good points. New version attached.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avfiltergraph.c
Type: text/x-csrc
Size: 4752 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080403/7ad394f9/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avfiltergraph.h
Type: text/x-chdr
Size: 1625 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080403/7ad394f9/attachment.h>