[FFmpeg-devel] [PATCH 3/3] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters

[wm4]

On Thu, 29 Oct 2015 08:34:42 +0100
Clément Bœsch <u at pkh.me> wrote:

> On Wed, Oct 28, 2015 at 06:34:06PM +0100, Hendrik Leppkes wrote:
> > On Sat, Oct 17, 2015 at 10:34 PM, Matthieu Bouron
> > <matthieu.bouron at gmail.com> wrote:  
> > > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> > >
> > > Avoid decoding twice images such as jpeg and png, once in the
> > > avformat_find_stream_info and once when the actual decode is made.
> > >
> > > The decoder must honor the skip_frame option in order to skip
> > > decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and
> > > png decoders.
> > > ---
> > >  libavformat/utils.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index 689473e..67dfffc 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, const char **errmsg_ptr)
> > >  static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket *avpkt,
> > >                              AVDictionary **options)
> > >  {
> > > +    int i;
> > >      const AVCodec *codec;
> > >      int got_picture = 1, ret = 0;
> > >      AVFrame *frame = av_frame_alloc();
> > >      AVSubtitle subtitle;
> > >      AVPacket pkt = *avpkt;
> > > +    int skip_frame;
> > > +    static const enum AVCodecID no_decode_codecs[] = {
> > > +        AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG,
> > > +    };  
> > 
> > Hardcoded lists of codecs in random places are never a good thing to have.
> > If this is a feature we agree to have, its codecs should just get an
> > internal capability that tells this code if it can parse all params
> > without decoding.
> >   
> 
> This list is supposed to be temporary (yes I know) until all other
> decoders that currently support AVDISCARD_ALL set the information field
> required for probing.

I do not think this is the right direction. We should go away from
mutating AVCodecContext fields, towards returning all per-frame info in
AVFrame. We should also go away from decoding in libavformat.

Before this becomes final, maybe you could check whether this could be
done in a codec parser instead? Some parsers do set the pixfmt, e.g.
h264. Yes, this suggestion is a bit late. I don't actually mind this
hack being applied in this form; at least it makes it explicit that
it's a hack for the image formats, and can be easily fixed at a later
point.