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

[Michael Niedermayer]

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.


> 
> > 
> > 
> >>     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 ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080402/95c594e4/attachment.pgp>