[FFmpeg-devel] [PATCH] libavcodec/mmaldec.c: add interlaced_frame and format struct to AVFrame for deinterlacing

[Hendrik Leppkes]

On Sun, Jun 26, 2016 at 5:12 PM, Jens Ziller <zillevdr at gmx.de> wrote:
> Am Samstag, den 25.06.2016, 12:52 +0200 schrieb Michael Niedermayer:
>> On Fri, Jun 24, 2016 at 06:27:38PM +0200, Jens Ziller wrote:
>> >
>> > Hello,
>> >
>> > deinterlacing need frame->interlaced_frame and format struct. This
>> > patch added this to AVFrame.
>> >
>> > Regards Jens.
>> >
>> >  mmaldec.c |   16 ++++++++++++++++
>> >  1 file changed, 16 insertions(+)
>> > 6351a54c36d98d1f6ffdaeea96af8c0db1305358  0001-for-deinterlacing-
>> > needed.patch
>> > From 8a8961a4fab0da2bd98ef6cbfaf55462a00d3450 Mon Sep 17 00:00:00
>> > 2001
>> > From: Jens Ziller <zillevdr at gmx.de>
>> > Date: Fri, 24 Jun 2016 18:18:12 +0200
>> >
>> > Subject: [PATCH] for deinterlacing needed
>> This commit message is not ok
>>
>> The message should describe
>> 1. what is changed
>> 2. why it is changed
>> 3. how it is changed
>>
>> When in doubt always write a longer commit message than a short one
>>
>> [...]
>> >
>> >      if (avctx->pix_fmt == AV_PIX_FMT_MMAL) {
>> >          if (!ctx->pool_out)
>> > +        // in data[2] give the format struct for configure
>> > deinterlacer and renderer
>> > +        frame->data[2] = ctx->decoder->output[0]->format;
>> This is not how AV_PIX_FMT_MMAL is documented:
>>
>>     /**
>>      * HW acceleration though MMAL, data[3] contains a pointer to the
>>      * MMAL_BUFFER_HEADER_T structure.
>>      */
>>     AV_PIX_FMT_MMAL,
>>
>> also where is the code that uses data[2] ?
>>
>> [...]
>>
>
> Attached is the new Version. Hints and comments are welcome.
>
> Regards Jens.
>

MMAL_ES_FORMAT_T looks like it contains information about the encoded
stream, and pretty trivial information at that - things that are in
AVCodecContext anyway.
Maybe whoever needs such a struct should just re-assemble it instead
of re-definining the pixel format definition here?

- Hendrik