[FFmpeg-devel] [RFC] libavfilter-soc: direct rendering
Michael Niedermayer
michaelni
Thu Jun 18 12:31:28 CEST 2009
On Thu, Jun 18, 2009 at 01:00:19AM +0200, Stefano Sabatini wrote:
> On date Thursday 2009-06-11 01:51:00 +0200, Michael Niedermayer encoded:
> > On Thu, Jun 11, 2009 at 01:20:35AM +0200, Stefano Sabatini wrote:
> [...]
> > > Attached patches implement this new system, try them 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"
> > >
> > > (no I didn't tested them extensively).
> >
> > more testing that mixes pad, crop and scale is definitly recommanded ...
> > also testing with non trivial chains that have loops is a good idea
> > but i guess you know that too ...
> >
> > that brings up the point of the very very important regression tests
> > we need some, it would make testing things like this easier
>
> Yes, I agree this is something we need at this point. I'll try to
> think at the test to implement, meaningwhile ideas are welcome.
>
> > > Attached patches, *does not still implement direct rendering*, direct
> > > rendering should be made possible using this new get_video_buffer() API.
> >
> > it does implement direct rendering between filters just not with the decoder
> > and vo
> > one issue might be that the decoder requests frames in a order
> > different from what it outputs
> >
> >
> > >
> > > Let me consider the specific example of ffplay.c, which currently
> > > does:
> > >
> > > while (!(ret = get_video_frame(priv->is, priv->frame, &pts, &pkt)))
> > > av_free_packet(&pkt);
> > > if (ret < 0)
> > > return -1;
> > >
> > > /* FIXME: until I figure out how to hook everything up to the codec
> > > * right, we're just copying the entire frame. */
> > > picref = avfilter_get_video_buffer(link, AV_PERM_WRITE, 0, 0, link->w, link->h);
> > > av_picture_copy((AVPicture *)&picref->data, (AVPicture *)priv->frame,
> > > picref->pic->format, picref->w, picref->h);
> > > av_free_packet(&pkt);
> > >
> > > We want to avoid the initial memcpy, furthermore if the last frame is
> > > an output device it may directly return a buffer from the display
> > > buffer area, thus allowing for direct rendering.
> > >
> > > Ideally we should be able to pass to get_video_frame() /
> > > avcodec_decode_video2() the buffer when we want the decoded buffer to
> > > be written.
> > >
> > > Hints regarding the correct approach to follow are much appreciated, I
> >
> > > suppose I need to use the AVCodecContext::{get|release}_buffer, right?
> >
> > yes i think so
> >
> >
> > [...]
> > > Index: libavfilter-soc/ffmpeg/libavfilter/avfilter.h
> > > ===================================================================
> > > --- libavfilter-soc.orig/ffmpeg/libavfilter/avfilter.h 2009-06-11 00:14:27.000000000 +0200
> > > +++ libavfilter-soc/ffmpeg/libavfilter/avfilter.h 2009-06-11 00:28:49.000000000 +0200
> > > @@ -88,6 +88,8 @@
> > > int linesize[4]; ///< number of bytes per line
> > > int w; ///< image width
> > > int h; ///< image height
> > > + int padleft, padtop;
> > > + int exp_w, exp_h;
> > >
> > > int64_t pts; ///< presentation timestamp in units of 1/AV_TIME_BASE
> > >
> >
> > can you generate patches so they show the function / struct name that is
> > changed?
> > that -p flag ...
>
> uhm using quilt which is purely diff/patch based there is no way I
> think, well maybe it's time to me to swith to git...
you can just apply the patch to a clean tree and then generate one with
svn -x -p di
should be automatable easily with a 2-3 line script
>
> > > @@ -291,7 +293,7 @@
> > > *
> > > * Input video pads only.
> > > */
> > > - AVFilterPicRef *(*get_video_buffer)(AVFilterLink *link, int perms);
> > > + AVFilterPicRef *(*get_video_buffer)(AVFilterLink *link, int perms, int padleft, int padtop, int exp_w, int exp_h);
> > >
> > > /**
> > > * Callback called after the slices of a frame are completely sent. If
> > > @@ -359,7 +361,9 @@
> > > 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 padleft, int padtop,
> > > + int exp_w, int exp_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
> > > @@ -500,7 +504,7 @@
> > > * @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 padleft, int padtop, int exp_w, int exp_h);
> >
> > the new arguments need to be documented
>
> In attachment a try, writing the docs for them I realized that maybe
> it would be better to call the params shiftright / shiftbottom.
>
> > [...]
> >
> > > +const char *var_names[]= {
> >
> > static
>
> Fixed.
>
> > > + "PI",
> > > + "E",
> > > + "in_w", ///< input width
> > > + "exp_w", ///< expanded width
> >
> > is out_w a poor choice for some reason ?
>
> Thinking at it out_[wh] are better names, so changed vf_pad
> accordingly.
>
> Attached patches are not ready for review, consider it a
> backup/proof-of-concept, still many points need to be addressed, in
> particular:
>
> * vflip filter is broken with the patch applied, the pad filter needs
> to be modified to work with negative logic too if possible, in the
> worst possible case maybe the vflip shouldn't be made slice-friendly
> as currently is.
>
> * More docs to the new added fields are needed
>
> * I suspect the new get_video_buffer API won't work with non-planar
> formats, also maybe a pixel format parameter has to be added to the
> params list.
>
> * this patchset still doesn't implement direct rendering, both
> vsrc_movie/vsrc_buffer/ffplay need to be changed accordingly, but
> this may be a separate step.
>
> * a lavfi regression test is required.
>
> Feel free to have a look at any of these.
ive too a long back log of stuff to look at the moment so i guess its better
if you explicitly ask me to look at something if you need help or have
a question or want a review ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20090618/7dc24513/attachment.pgp>
More information about the ffmpeg-devel
mailing list