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

[Jens Ziller]

Am Sonntag, den 26.06.2016, 18:08 +0200 schrieb 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
> 
Yes, AVCodecContext is similise to MMAL_ES_FORMAT_T. But copy the data
from MMAL_ES_FORMAT_T to AVCodecContext and the user app copy the data
to a new structure MMAL_ES_FORMAT_T is not smart. I think give a
pointer from the existing MMAL_ES_FORMAT_T to user app is nicer. I
don't have find a better solution to give a pointer to user app.
It's not a re-definining. It's an addition. The structure
MMAL_ES_FORMAT_T is only used with AV_PIX_FMT_MMAL format. MMAL is
RaspberryPi specific.

Regards Jens.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel