[FFmpeg-devel] [PATCH] ffprobe: Stash and use width and height before opening the codec

Stefano Sabatini stefasab at gmail.com
Thu Mar 7 02:37:34 CET 2013


On date Friday 2013-03-01 10:41:34 -0500, Derek Buitenhuis encoded:
> Some codecs, such as VP6, will only have their correct width and
> height set if a few frames have been decoded. This is accomplished
> when we call avformat_find_stream_info(). However, we call
> avcodec_open2() after this, which can possibly reset the width
> and height in the decoder's context to an erroneous value.
> 
> Stash and propagate the width and height after calling
> avformat_find_stream_info(), but before calling avcodec_open2().
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
>  ffprobe.c |   46 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/ffprobe.c b/ffprobe.c
> index f70c24c..f4074d4 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -1592,7 +1592,8 @@ static void read_packets(WriterContext *w, AVFormatContext *fmt_ctx)
>      }
>  }
>  
> -static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_idx)
> +static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx,
> +                        int stream_idx, int orig_width, int orig_height)
>  {
>      AVStream *stream = fmt_ctx->streams[stream_idx];
>      AVCodecContext *dec_ctx;
> @@ -1641,8 +1642,8 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
>  
>          switch (dec_ctx->codec_type) {
>          case AVMEDIA_TYPE_VIDEO:
> -            print_int("width",        dec_ctx->width);
> -            print_int("height",       dec_ctx->height);
> +            print_int("width",        orig_width);
> +            print_int("height",       orig_height);
>              print_int("has_b_frames", dec_ctx->has_b_frames);
>              sar = av_guess_sample_aspect_ratio(fmt_ctx, stream, NULL);
>              if (sar.den) {
> @@ -1742,13 +1743,19 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
>      fflush(stdout);
>  }
>  
> -static void show_streams(WriterContext *w, AVFormatContext *fmt_ctx)
> +typedef struct Dimension {
> +    int width;
> +    int height;
> +} Dimension;
> +
> +static void show_streams(WriterContext *w, AVFormatContext *fmt_ctx,
> +                         Dimension *orig_dims)
>  {
>      int i;
>      writer_print_section_header(w, SECTION_ID_STREAMS);
>      for (i = 0; i < fmt_ctx->nb_streams; i++)
>          if (selected_streams[i])
> -            show_stream(w, fmt_ctx, i);
> +            show_stream(w, fmt_ctx, i, orig_dims[i].width, orig_dims[i].height);
>      writer_print_section_footer(w);
>  }
>  
> @@ -1791,7 +1798,8 @@ static void show_error(WriterContext *w, int err)
>      writer_print_section_footer(w);
>  }
>  
> -static int open_input_file(AVFormatContext **fmt_ctx_ptr, const char *filename)
> +static int open_input_file(AVFormatContext **fmt_ctx_ptr, const char *filename,
> +                           Dimension **orig_dims)
>  {
>      int err, i;
>      AVFormatContext *fmt_ctx = NULL;
> @@ -1807,8 +1815,6 @@ static int open_input_file(AVFormatContext **fmt_ctx_ptr, const char *filename)
>          return AVERROR_OPTION_NOT_FOUND;
>      }
>  
> -
> -    /* fill the streams in the format context */
>      if ((err = avformat_find_stream_info(fmt_ctx, NULL)) < 0) {
>          print_error(filename, err);
>          return err;
> @@ -1816,11 +1822,27 @@ static int open_input_file(AVFormatContext **fmt_ctx_ptr, const char *filename)
>  
>      av_dump_format(fmt_ctx, 0, filename, 0);
>  
> +    /*
> +     * Allocate our dimensions buffer.
> +     * We need to stash the dimensions before we call avcodec_open2(),
> +     * because some codecs require a few frames to be decoded before
> +     * width and height are set properly, and avcodec_open2() can
> +     * reset these values.
> +     */
> +    if (!(*orig_dims = av_mallocz(sizeof(**orig_dims) * fmt_ctx->nb_streams))) {
> +        av_log(NULL, AV_LOG_ERROR,
> +               "Could not allocate temporary dimensions buffer.\n");
> +        return AVERROR(ENOMEM);
> +    }
> +
>      /* bind a decoder to each input stream */
>      for (i = 0; i < fmt_ctx->nb_streams; i++) {
>          AVStream *stream = fmt_ctx->streams[i];
>          AVCodec *codec;
>  
> +        (*orig_dims)[i].width  = stream->codec->width;
> +        (*orig_dims)[i].height = stream->codec->height;
> +
>          if (stream->codec->codec_id == AV_CODEC_ID_PROBE) {
>              av_log(NULL, AV_LOG_ERROR,
>                     "Failed to probe codec for input stream %d\n",
> @@ -1855,13 +1877,14 @@ static void close_input_file(AVFormatContext **ctx_ptr)
>  static int probe_file(WriterContext *wctx, const char *filename)
>  {
>      AVFormatContext *fmt_ctx;
> +    Dimension *orig_dims;
>      int ret, i;
>      int section_id;
>  
>      do_read_frames = do_show_frames || do_count_frames;
>      do_read_packets = do_show_packets || do_count_packets;
>  
> -    ret = open_input_file(&fmt_ctx, filename);
> +    ret = open_input_file(&fmt_ctx, filename, &orig_dims);
>      if (ret >= 0) {
>          nb_streams_frames  = av_calloc(fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
>          nb_streams_packets = av_calloc(fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
> @@ -1896,7 +1919,7 @@ static int probe_file(WriterContext *wctx, const char *filename)
>                  writer_print_section_footer(wctx);
>          }
>          if (do_show_streams)
> -            show_streams(wctx, fmt_ctx);
> +            show_streams(wctx, fmt_ctx, orig_dims);
>          if (do_show_format)
>              show_format(wctx, fmt_ctx);
>  
> @@ -1906,6 +1929,9 @@ static int probe_file(WriterContext *wctx, const char *filename)
>          av_freep(&nb_streams_packets);
>          av_freep(&selected_streams);
>      }
> +
> +    av_freep(&orig_dims);
> +
>      return ret;
>  }

Approach seems acceptable (and fixes ticket #1386), but I wonder if
there is some way to avoid this at the library level.

w/h resets happens in libavcodec/utils.c:879, in avcodec_open2():

    //We only call avcodec_set_dimensions() for non h264 codecs so as not to overwrite previously setup dimensions
    if (!( avctx->coded_width && avctx->coded_height && avctx->width && avctx->height && avctx->codec_id == AV_CODEC_ID_H264)){

    if (avctx->coded_width && avctx->coded_height)
        avcodec_set_dimensions(avctx, avctx->coded_width, avctx->coded_height);
    else if (avctx->width && avctx->height)
        avcodec_set_dimensions(avctx, avctx->width, avctx->height);
    }

and I really don't know why it is done this way.
-- 
FFmpeg = Faithless and Fundamental Mere Pitiless Enlightened Guru


More information about the ffmpeg-devel mailing list