[FFmpeg-devel] [PATCH 2/4] lavf/utils: avoid decoding a frame to get the codec parameters

[Matthieu Bouron]

On Mon, Nov 02, 2015 at 07:56:50AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron <matthieu.bouron at gmail.com>
> wrote:
> 
> > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> >
> > Avoid decoding a frame to get the codec parameters while the codec
> > supports FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM. This is particulary useful
> > to avoid decoding twice images (once in avformat_find_stream_info and
> > once when the actual decode is made).
> > ---
> >  libavformat/utils.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 5c4d452..ba62f2b 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -2695,6 +2695,8 @@ static int try_decode_frame(AVFormatContext *s,
> > AVStream *st, AVPacket *avpkt,
> >      AVFrame *frame = av_frame_alloc();
> >      AVSubtitle subtitle;
> >      AVPacket pkt = *avpkt;
> > +    int do_skip_frame = 0;
> > +    enum AVDiscard skip_frame;
> >
> >      if (!frame)
> >          return AVERROR(ENOMEM);
> > @@ -2733,6 +2735,12 @@ static int try_decode_frame(AVFormatContext *s,
> > AVStream *st, AVPacket *avpkt,
> >          goto fail;
> >      }
> >
> > +    if (st->codec->codec->caps_internal &
> > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > +        do_skip_frame = 1;
> > +        skip_frame = st->codec->skip_frame;
> > +        st->codec->skip_frame = AVDISCARD_ALL;
> > +    }
> > +
> >      while ((pkt.size > 0 || (!pkt.data && got_picture)) &&
> >             ret >= 0 &&
> >             (!has_codec_parameters(st, NULL) ||
> > !has_decode_delay_been_guessed(st) ||
> > @@ -2768,6 +2776,10 @@ static int try_decode_frame(AVFormatContext *s,
> > AVStream *st, AVPacket *avpkt,
> >          ret = -1;
> >
> >  fail:
> > +    if (do_skip_frame) {
> > +        st->codec->skip_frame = skip_frame;
> > +    }
> > +
> >      av_frame_free(&frame);
> >      return ret;
> >  }
> > --
> > 2.6.2
> 
> 
> I think we need an assert in the try_decode loop to ensure that it indeed
> did fill all the params. This is to prevent the case where someone adds a
> new "thing" to the list of things required for find_stream_info to succeed,
> and forgets to update the codecs.

While the codec_has_parameters function fits that role, it does not check
all codec parameters as they can be codec dependant.

I'm not sure if we can create a generic rule here for the same reason as
above, as the parameters set can be codec specific and maybe stream specific.

Having some fate test to cover this area seems to be another option but would
require to inspect all the relevant parameters of AVCodecContext depending
on the codec and a lot of different streams.

What do you think of this approach ?

Matthieu

[...]