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

[Michael Niedermayer]

On Mon, May 24, 2010 at 11:39:13AM +0530, Jai Menon wrote:
> On Mon, May 24, 2010 at 5:55 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > 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
> 
> okay, so something like attached?
> 
> -- 
> Jai Menon

>  ffplay.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 03b737d84bf45dbc1a3065ae971060614aee6e71  ffplay-reget-buffer.diff
> diff --git a/ffplay.c b/ffplay.c
> index a48891e..a5e3287 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -1607,6 +1607,21 @@ static void input_release_buffer(AVCodecContext *codec, AVFrame *pic)
>      avfilter_unref_pic(pic->opaque);
>  }
>  
> +static int input_reget_buffer(AVCodecContext *codec, AVFrame *pic)
> +{
> +    AVFilterPicRef *ref = pic->opaque;
> +
> +    if(pic->data[0] == NULL ||
> +       (codec->width != ref->w) || (codec->height != ref->h) ||
> +       (codec->pix_fmt != ref->pic->format)) {
> +        pic->buffer_hints |= FF_BUFFER_HINTS_READABLE;
> +        return codec->get_buffer(codec, pic);
> +    }

i think mismatching w/h/pixfmt should print an error

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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/c9564a3b/attachment.pgp>