[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