[FFmpeg-devel] [PATCH] [1/??] [2/3] Basic infrastructure

[Michael Niedermayer]

On Sun, Feb 10, 2008 at 05:59:43PM +0100, Vitor Sessak wrote:
> Hi
>
> Michael Niedermayer wrote:
>> On Sun, Feb 10, 2008 at 11:03:48AM +0100, Vitor Sessak wrote:
>>> Hi and thanks for the review
>> [...]
>>>>>         switch(link->init_state) {
>>>>>         case AVLINK_INIT:
>>>>>             continue;
>>>>>         case AVLINK_STARTINIT:
>>>>>             av_log(filter, AV_LOG_ERROR, "circular filter chain 
>>>>> detected\n");
>>>>>             return -1;
>>>>>         case AVLINK_UNINIT:
>>>>>             link->init_state = AVLINK_STARTINIT;
>>>>>
>>>>>             if(avfilter_config_links(link->src))
>>>>>                 return -1;
>>>>>
>>>>>             if(!(config_link = link_spad(link).config_props))
>>>>>                 config_link  = avfilter_default_config_output_link;
>>>>>             if(config_link(link))
>>>>>                 return -1;
>>>>>
>>>>>             if((config_link = link_dpad(link).config_props))
>>>>>             if(config_link(link))
>>>>>                 return -1;
>>>>>
>>>>>             link->init_state = AVLINK_INIT;
>>>>>         }
>>>>>     }
>>>> what does the above mean for filter graphs like:
>>>>
>>>> ->mix--->duplicate--->
>>>>    ^         |
>>>>    |         v
>>>>     \------delay
>>>>
>>>> aka an infinite impulse response video filter :)
>>>> it looks like it would return -1 for that ...
>>> What should be done in these cases? Something like av_log("Circular video 
>>> chain, expect trouble\n")?
>> Well, id say avfilter should work with circular chains as well. Of course
>> not with all, one can easily build ones which would deadlock ...
>> Actually most random circular chains would deadlock, but above would not
>> as long as the duplicate filter is carefully implemented. That is if
>> a request from the delay filter would be satifies with whatever last
>> frame the duplicate filter has and never be passed on to the mix filter.
>
> Ok. So I think a warning is more appropriated.
>
>> [...]
>>> All the other points I don't mention, agreed and changed. New code in 
>>> http://svn.mplayerhq.hu/soc/libavfilter/avfilter.c?content-type=text%2Fplain&view=co 
>>> (Diego, nits changed in the soc tree too).
>> This is just one file the patch contained more, thus i cant review it and
>> you should send patches anyway instead of links to non constant files.
>
> Ok. Attached.
[...]

> int avfilter_poll_frame(AVFilterLink *link);

this should be where avfilter_request_frame, avfilter_start_frame, ... are
and also have a doxy


> 
> /**
>  * A filter pad used for either input or output.
>  */
> struct AVFilterPad
> {
>     /**
>      * Pad name.  The name is unique among inputs and among outputs, but an
>      * input may have the same name as an output.  This may be NULL if this
>      * pad has no need to ever be referenced by name.
>      */
>     const char *name;
> 

>     /**
>      * AVFilterPad type.  Only video supported now, hopefully someone will
>      * add audio in the future.
>      */
>     int type;
> #define AV_PAD_VIDEO 0      ///< video pad

CodecType could be used here


[...]
> /* TODO: set the buffer's priv member to a context structure for the whole
>  * filter chain.  This will allow for a buffer pool instead of the constant
>  * alloc & free cycle currently implemented. */
> AVFilterPicRef *avfilter_default_get_video_buffer(AVFilterLink *link, int perms)
> {
>     AVFilterPic *pic = av_mallocz(sizeof(AVFilterPic));
>     AVFilterPicRef *ref = av_mallocz(sizeof(AVFilterPicRef));
> 
>     ref->pic   = pic;
>     ref->w     = link->w;
>     ref->h     = link->h;
> 
>     /* make sure the buffer gets read permission or it's useless for output */
>     ref->perms = perms | AV_PERM_READ;
> 
>     pic->refcount = 1;
>     pic->format   = link->format;
>     pic->free     = avfilter_default_free_video_buffer;
>     avpicture_alloc((AVPicture *)pic, pic->format, ref->w, ref->h);

This will set linesize == w*pixel_size while many filters will need linesize
to be a multiple of 16 (due to SSE not likring misaligned accesses).


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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- 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/20080210/0e704c71/attachment.pgp>