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

[Vitor Sessak]

Hi

Michael Niedermayer wrote:
> 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).

Agreed with everything. New patch attached.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: allfilters.h
Type: text/x-chdr
Size: 884 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/e803b56f/attachment.h>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avfilter.c
Type: text/x-csrc
Size: 11267 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/e803b56f/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avfilter.h
Type: text/x-chdr
Size: 22090 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/e803b56f/attachment-0001.h>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: defaults.c
Type: text/x-csrc
Size: 4437 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080210/e803b56f/attachment-0001.c>