[FFmpeg-devel] [PATCH] libavfilter-soc: implement pad filter

[Vitor Sessak]

Stefano Sabatini wrote:
> On date Tuesday 2009-05-19 02:09:53 +0200, Stefano Sabatini encoded:
>> On date Monday 2009-05-18 23:49:50 +0200, Stefano Sabatini encoded:
>>> On date Sunday 2009-05-17 13:14:22 +0200, Stefano Sabatini encoded:
>>>> On date Monday 2009-05-11 19:17:51 +0200, Vitor Sessak encoded:
>>> [...]
>>>> I'm going to try this approach:
>>>> int request_frame(AVFilterLink *link, int width, int height, int left, int top);
>>>> picref *avfilter_get_video_buffer(link, int exp_w, int exp_h, int left, int top, AV_PERM_WRITE);
>>>>
>>>> the additional parameters given to avfilter_get_video_buffer() (at
>>>> least left and top) seem to be necessary to correctly free the buffer.
>>>>
>>>> An alternative approach would be to extend avfilter_config_links() to
>>>> take the additional parameters w, h, left, top and store them in the
>>>> link, but looks somehow more complicated.
>>> And implementation attached.
>>>
>>> Note how I also extended avfilter_poll_frame(): this seems to be
>>> required for the way it is used in vf_fps.c.
>> Please don't still review it, I'm still nailing down some bugs.
> 
> Should be fixed now, try for example with:
> 
> ffplay in.avi -vfilters "pad = exp_w=in_w+30 : exp_h=in_h + 30 : color = red,
>                          pad = exp_w=in_w+30 : exp_h=in_h + 30 : color = blue,
>                          pad = exp_w=in_w+30 : exp_h=in_h + 30 : color = yellow"
> 
> This should provide a good base for further development, in particular
> it shouldn't be too hard to add support for RGB colorspaces.

Nice! Thanks for digging into this!

Some comments:

>  /**
>   * Polls a frame from the filter chain.
> @@ -515,7 +521,7 @@
>   * @return the number of immediately available frames, a negative
>   * number in case of error
>   */
> -int avfilter_poll_frame(AVFilterLink *link);
> +int avfilter_poll_frame(AVFilterLink *link, int exp_w, int exp_h, int left, int top);

What is the rationale for adding this parameters also to poll_frame()?

> Index: libavfilter-soc/ffmpeg/ffmpeg.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/ffmpeg.c	2009-05-18 23:25:48.000000000 +0200
> +++ libavfilter-soc/ffmpeg/ffmpeg.c	2009-05-18 23:26:03.000000000 +0200
> @@ -355,7 +355,7 @@
>  {
>      AVFilterPicRef *pic;
>  
> -    if(avfilter_request_frame(ctx->inputs[0]))
> +    if(avfilter_request_frame(ctx->inputs[0], ctx->inputs[0]->w, ctx->inputs[0]->h, 0, 0))

I suppose you didn't test the changes to ffmpeg.c, unless you forgot to 
attach the patch for vsrc_buffer.c. I imagine that here handling 
avfilter_request_frame() without memcpy'ing the whole frame (as is done 
in ffplay.c) would be non trivial.

-Vitor