[FFmpeg-devel] [PATCH] Implement recusive avfilter_get_video_buffer()
Michael Niedermayer
michaelni
Thu Oct 8 00:05:24 CEST 2009
On Wed, Oct 07, 2009 at 09:24:45PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2009-10-06 02:18:22 +0200, Stefano Sabatini encoded:
> > On date Monday 2009-10-05 16:08:13 +0200, Michael Niedermayer encoded:
> > > On Sun, Oct 04, 2009 at 06:09:15PM +0200, Stefano Sabatini wrote:
> [...]
> > > > Index: libavfilter/avfilter.h
> > > > ===================================================================
> > > > --- libavfilter/avfilter.h (revision 20167)
> > > > +++ libavfilter/avfilter.h (working copy)
> > >
> > > > @@ -22,8 +22,8 @@
> > > > #ifndef AVFILTER_AVFILTER_H
> > > > #define AVFILTER_AVFILTER_H
> > > >
> > > > -#define LIBAVFILTER_VERSION_MAJOR 0
> > > > -#define LIBAVFILTER_VERSION_MINOR 5
> > > > +#define LIBAVFILTER_VERSION_MAJOR 1
> > > > +#define LIBAVFILTER_VERSION_MINOR 0
> > > > #define LIBAVFILTER_VERSION_MICRO 0
> > > >
> > > > #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> > >
> > > iam not sure this is needed given libavfilter is not useable in svn yet
> > > but i dont mind that hunk ...
>
> I prefer to bump, also some distros are already shipping libavfilter
> so I think this may avoid some your-code-doesnt-compile-here issue.
>
> Also consider it as a marketing move :-).
i dont really mind but i would like to emphasise that distros that patch
ffmpeg with stuff that isnt officially part of ffmpeg have to deal with
the resulting ABI issues themselfs
i dont want to throw them stones in the way but i also dont want to
spend time identifying problems and fixing them that may arrise due to this
>
> > > > @@ -291,7 +291,7 @@ struct AVFilterPad
> > > > *
> > > > * Input video pads only.
> > > > */
> > > > - AVFilterPicRef *(*get_video_buffer)(AVFilterLink *link, int perms);
> > > > + AVFilterPicRef *(*get_video_buffer)(AVFilterLink *link, int perms, int w, int h);
> > > >
> > > > /**
> > > > * Callback called after the slices of a frame are completely sent. If
> > > > @@ -359,7 +359,7 @@ int avfilter_default_config_output_link(AVFilterLi
> > > > int avfilter_default_config_input_link (AVFilterLink *link);
> > > > /** default handler for get_video_buffer() for video inputs */
> > > > AVFilterPicRef *avfilter_default_get_video_buffer(AVFilterLink *link,
> > > > - int perms);
> > > > + int perms, int w, int h);
> > > > /**
> > > > * A helper for query_formats() which sets all links to the same list of
> > > > * formats. If there are no links hooked to this filter, the list of formats is
> > > > @@ -497,10 +497,13 @@ int avfilter_config_links(AVFilterContext *filter)
> > > > * @param link the output link to the filter from which the picture will
> > > > * be requested
> > > > * @param perms the required access permissions
> > > > + * @param w the minimum width of the buffer to allocate
> > > > + * @param h the minimum height of the buffer to allocate
> > > > * @return A reference to the picture. This must be unreferenced with
> > > > * avfilter_unref_pic when you are finished with it.
> > > > */
> > > > -AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms);
> > > > +AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms,
> > > > + int w, int h);
> > > >
> > > > /**
> > > > * Requests an input frame from the filter at the other end of the link.
> > > > Index: libavfilter/defaults.c
> > > > ===================================================================
> > > > --- libavfilter/defaults.c (revision 20167)
> > > > +++ libavfilter/defaults.c (working copy)
> > > > @@ -32,7 +32,7 @@ void avfilter_default_free_video_buffer(AVFilterPi
> > > > /* 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)
> > > > +AVFilterPicRef *avfilter_default_get_video_buffer(AVFilterLink *link, int perms, int w, int h)
> > > > {
> > > > AVFilterPic *pic = av_mallocz(sizeof(AVFilterPic));
> > > > AVFilterPicRef *ref = av_mallocz(sizeof(AVFilterPicRef));
> > >
> > > above 4 hunks are ok
> > >
> > >
> > > > @@ -40,8 +40,8 @@ void avfilter_default_free_video_buffer(AVFilterPi
> > > > char *buf;
> > > >
> > > > ref->pic = pic;
> > > > - ref->w = link->w;
> > > > - ref->h = link->h;
> > > > + ref->w = w;
> > > > + ref->h = h;
> > > >
> > > > /* make sure the buffer gets read permission or it's useless for output */
> > > > ref->perms = perms | AV_PERM_READ;
> > >
> > > iam not sure about this one but it seems AVFilterPic is missing a w/h
>
> This should be fixed now.
>
> > I posted a patch just for that.
> >
> > > > @@ -72,7 +72,7 @@ void avfilter_default_start_frame(AVFilterLink *li
> > > > out = link->dst->outputs[0];
> > > >
> > > > if(out) {
> > > > - out->outpic = avfilter_get_video_buffer(out, AV_PERM_WRITE);
> > > > + out->outpic = avfilter_get_video_buffer(out, AV_PERM_WRITE, link->w, link->h);
> > > > out->outpic->pts = picref->pts;
> > > > avfilter_start_frame(out, avfilter_ref_pic(out->outpic, ~0));
> > > > }
> > >
> > > probably ok
> > >
> > > > Index: libavfilter/avfilter.c
> > > > ===================================================================
> > > > --- libavfilter/avfilter.c (revision 20167)
> > > > +++ libavfilter/avfilter.c (working copy)
> > > > @@ -160,15 +160,18 @@ int avfilter_config_links(AVFilterContext *filter)
> > > > return 0;
> > > > }
> > > >
> > > > -AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms)
> > > > +AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms, int w, int h)
> > > > {
> > > > AVFilterPicRef *ret = NULL;
> > > >
> > > > if(link_dpad(link).get_video_buffer)
> > > > - ret = link_dpad(link).get_video_buffer(link, perms);
> > > > + ret = link_dpad(link).get_video_buffer(link, perms, w, h);
> > > >
> > > > + if(!ret && link->dst->output_count)
> > > > + ret = avfilter_get_video_buffer(link->dst->outputs[0], perms, w, h);
> > > > +
> > > > if(!ret)
> > > > - ret = avfilter_default_get_video_buffer(link, perms);
> > > > + ret = avfilter_default_get_video_buffer(link, perms, w, h);
> > > >
> > > > return ret;
> > > > }
> > > > @@ -218,7 +221,7 @@ void avfilter_start_frame(AVFilterLink *link, AVFi
> > > > link_dpad(link).min_perms, link_dpad(link).rej_perms);
> > > > */
> > > >
> > > > - link->cur_pic = avfilter_default_get_video_buffer(link, dst->min_perms);
> > > > + link->cur_pic = avfilter_default_get_video_buffer(link, dst->min_perms, link->w, link->h);
> > > > link->srcpic = picref;
> > > > link->cur_pic->pts = link->srcpic->pts;
> > > > link->cur_pic->pixel_aspect = link->srcpic->pixel_aspect;
> > >
> > > these look ok
> > >
> > > i think the other patches where not for review yet?
> >
> > I don't know if you are interested to review the soc patch, I need to
> > apply it to the soc anyway, at least to be able to run regression
> > tests.
>
> Updated patch attached.
>
> For what regards the soc patch, is OK to apply that as well once this
> is applied?
[...]
> avfilter.c | 13 ++++++++-----
> avfilter.h | 15 +++++++++------
> defaults.c | 8 ++++----
> 3 files changed, 21 insertions(+), 15 deletions(-)
> ad7c75f534241787cf08f3b8ba6f735ae2168ebe lavfi-recursive-get-video-buffer.patch
Probably ok but this should be tested better too much than too little
if you have any regression tests please check that they dont change due to
this ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20091008/30143bc4/attachment.pgp>
More information about the ffmpeg-devel
mailing list