[FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format

[wm4]

On Wed, 21 Oct 2015 17:15:14 +0200
Julian Scheel <julian at jusst.de> wrote:

> Am 21.10.2015 um 16:18 schrieb wm4:
> > On Wed, 21 Oct 2015 16:07:08 +0200
> > Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> >  
> >> On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel <julian at jusst.de> wrote:  
> >>> Wait for the first decoded frame to be returned by mmal before setting
> >>> pix_fmt. This is important for avformat probing to work properly as it is one
> >>> of the criterions to decide whether to decode a frame or not for probing.
> >>>
> >>> Signed-off-by: Julian Scheel <julian at jusst.de>
> >>> ---
> >>>   libavcodec/mmaldec.c | 10 +++++-----
> >>>   1 file changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> >>> index 7db90d2..429990a 100644
> >>> --- a/libavcodec/mmaldec.c
> >>> +++ b/libavcodec/mmaldec.c
> >>> @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext *avctx)
> >>>           return AVERROR(ENOSYS);
> >>>       }
> >>>
> >>> -    if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0)
> >>> -        return ret;
> >>> -
> >>> -    avctx->pix_fmt = ret;
> >>> -
> >>>       if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder)))
> >>>           goto fail;
> >>>
> >>> @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, AVFrame *frame, int *got_fra
> >>>
> >>>               av_log(avctx, AV_LOG_INFO, "Changing output format.\n");
> >>>
> >>> +            if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0)
> >>> +                return ret;
> >>> +
> >>> +            avctx->pix_fmt = ret;
> >>> +
> >>>               if ((status = mmal_port_disable(decoder->output[0])))
> >>>                   goto done;
> >>>  
> >>
> >> pix_fmt is already used by the decoder before this point to decide if
> >> mmal surfaces or memory buffers are to be used, changing it afterwards
> >> will not have the same effect as doing it in init.  
> >
> > Oh, you're right. It's used in ffmal_update_format(), so this patch can
> > only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if
> > the entire decoding init is delayed and not done in AVCodec.init.  
> 
> I think it would be sufficient to query the pix_fmt via ff_get_format in 
> ffmal_update_format instead of using avctx->pix_fmt. The other paramters 
> should be changeable without disabling the port. But this should be 
> tested probably :)
> Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the 
> decoder in any case I have opened an issue for clarification: 
> https://github.com/raspberrypi/firmware/issues/489

ffmal_update_format() is also called in the init function. Isn't this what
you're trying to avoid?