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

[Jai Menon]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffplay-reget-buffer.diff
Type: text/x-patch
Size: 1123 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100524/8782f36c/attachment.bin>