[FFmpeg-devel] [RFC] ffplay dr1 + reget buffer issue

[Michael Niedermayer]

On Mon, May 24, 2010 at 12:51:39AM +0530, Jai Menon wrote:
> On Sun, May 23, 2010 at 11:22 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, May 23, 2010 at 09:05:04PM +0530, Jai Menon wrote:
> >> Hi,
> >>
> >> Here's yet another ffplay issue i thought i'd post. Currently, the
> >> ffplay input filter doesn't define a reget_buffer callback for codecs.
> >> So codecs fallback to avcodec_default_reget_buffer which always tries
> >> to obtain a new buffer and copy the old picture onto it. This used to
> >> work fine for codecs which output rgb earlier because internal buffers
> >> are always "reused". With current ffplay however, buffer_type is
> >> FF_BUFFER_TYPE_USER which means that with every reget_buffer call, we
> >> unnecessarily make get/release buffer calls. Also, since
> >> av_picture_copy doesn't account for the single data pointer used for
> >> rgb pixfmts, all codecs using reget_buffer and outputting rgb crash
> >> with ffplay. I'm not sure which "solution" is preferable here (or if
> >> there is something else which can be done), so attached are two
> >> patches for the different issues. both of them fix the issue
> >> independently. comments welcome.
> >
> > i think we want both paches, comments below
> >
> >>
> >> --
> >> Jai Menon
> >
> >> ?ffplay.c | ? 12 ++++++++++++
> >> ?1 file changed, 12 insertions(+)
> >> c1749823e51b918e66af05b6635754f3fb86b0d3 ?ffplay-reget-buffer.diff
> >> diff --git a/ffplay.c b/ffplay.c
> >> index a48891e..d2f0e67 100644
> >> --- a/ffplay.c
> >> +++ b/ffplay.c
> >> @@ -1607,6 +1607,17 @@ static void input_release_buffer(AVCodecContext *codec, AVFrame *pic)
> >> ? ? ?avfilter_unref_pic(pic->opaque);
> >> ?}
> >>
> >> +static int input_reget_buffer(AVCodecContext *codec, AVFrame *pic)
> >> +{
> >> + ? ?if(pic->data[0] == NULL) {
> >> + ? ? ? ?pic->buffer_hints |= FF_BUFFER_HINTS_READABLE;
> >> + ? ? ? ?return codec->get_buffer(codec, pic);
> >> + ? ?}
> >> +
> >> + ? ?pic->reordered_opaque = codec->reordered_opaque;
> >> + ? ?return 0;
> >> +}
> >
> > iam not sure if we have to, but i think we should check that
> > width/height/pixfmt matches
> 
> Would a check whether linesizes with current image properties (using
> ff_fill_linesize on a throwaway pic) and the input AVFrame match, be
> enough? I'm not sure if anything more than that required though (or
> perhaps something the code in its current state supports).

pic->opaque is a AVFilterPicRef that has a w/h for checking


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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20100524/c5e6af80/attachment.pgp>