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

[Vitor Sessak]

Hi

Michael Niedermayer wrote:
> On Tue, Apr 01, 2008 at 10:49:39PM +0200, Vitor Sessak wrote:
> [...]
>>>
>>> [...]
>>>>     char tmp[20];
>>>>
>>>>     snprintf(tmp, 20, "%d", index);
>>>                     ^^
>>> sizeof
>> Agreed. But are you ok with the instance name/instance lookup logic here?
> 
> Well as you mention it now, i realize what you are actually doing there.
> No i do not like this design at all.
> What i dont understand is why dont you just use pointers to AVFilterContext
> instead of these numbers in char* identifers? I mean without looking
> at the code common sense tells me whereever you store the numbers you could
> store pointers to AVFilterContext as well.

Well, it made more sense before the new parser was written. Changed that.

> 
> 
>>>
>>>>     if(!(filterdef = avfilter_get_by_name(name)) ||
>>>>        !(filt = avfilter_open(filterdef, tmp))) {
>>>>         av_log(&log_ctx, AV_LOG_ERROR,
>>>>                "error creating filter '%s'\n", name);
>>> The purpose of the context is so the user can associate it with some
>>> specific instance of something. using a global context defeats this.
>> Yes. Any suggestion?
> 
> Its not very important ...
> The solution is to put that AVClass in some context, something related to
> the parser ...

I don't see any candidates...

Another change is that I changed the notation of labels to '[in]' to 
allow implementing parenthesis later.

-Vitor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: graphparser.c
Type: text/x-csrc
Size: 8584 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080406/17e64eb3/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graphparser.h
Type: text/x-chdr
Size: 1550 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080406/17e64eb3/attachment.h>