Message ID | 20201016131649.4361-5-jeebjp@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | ffmpeg: late A/V encoder init, AVFrame metadata usage | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
Quoting Jan Ekström (2020-10-16 15:16:47) > Additionally, reap the first rewards by being able to set the > color related encoding values based on the passed AVFrame. > > The only tests that seem to have changed their results with this > change seem to be the MXF tests. There, the muxer writes the > limited/full range flag to the output container if the encoder > is not set to "unspecified". > --- > fftools/ffmpeg.c | 42 +++++++++++++++++++++++++++---------- > tests/ref/lavf/mxf_d10 | 2 +- > tests/ref/lavf/mxf_dv25 | 2 +- > tests/ref/lavf/mxf_dvcpro50 | 2 +- > tests/ref/lavf/mxf_opatom | 2 +- > 5 files changed, 35 insertions(+), 15 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 08db67a6ab..b2e210c814 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -941,9 +941,11 @@ early_exit: > return float_pts; > } > > -static int init_output_stream(OutputStream *ost, char *error, int error_len); > +static int init_output_stream(OutputStream *ost, AVFrame *frame, > + char *error, int error_len); > > -static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) > +static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame, > + unsigned int fatal) > { > int ret = AVERROR_BUG; > char error[1024] = {0}; > @@ -951,7 +953,7 @@ static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) > if (ost->initialized) > return 0; > > - ret = init_output_stream(ost, error, sizeof(error)); > + ret = init_output_stream(ost, frame, error, sizeof(error)); > if (ret < 0) { > av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n", > ost->file_index, ost->index, error); > @@ -1125,7 +1127,7 @@ static void do_video_out(OutputFile *of, > InputStream *ist = NULL; > AVFilterContext *filter = ost->filter->filter; > > - init_output_stream_wrapper(ost, 1); > + init_output_stream_wrapper(ost, next_picture, 1); > sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture); > > if (ost->source_index >= 0) > @@ -1507,7 +1509,7 @@ static int reap_filters(int flush) > * the encoder earlier than receiving the first AVFrame. > */ > if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO) > - init_output_stream_wrapper(ost, 1); > + init_output_stream_wrapper(ost, NULL, 1); > > if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) { > return AVERROR(ENOMEM); > @@ -1930,7 +1932,7 @@ static void flush_encoders(void) > finish_output_stream(ost); > } > > - init_output_stream_wrapper(ost, 1); > + init_output_stream_wrapper(ost, NULL, 1); > } > > if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != AVMEDIA_TYPE_AUDIO) > @@ -3302,7 +3304,7 @@ static void init_encoder_time_base(OutputStream *ost, AVRational default_time_ba > enc_ctx->time_base = default_time_base; > } > > -static int init_output_stream_encode(OutputStream *ost) > +static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > { > InputStream *ist = get_input_stream(ost); > AVCodecContext *enc_ctx = ost->enc_ctx; > @@ -3399,6 +3401,23 @@ static int init_output_stream_encode(OutputStream *ost) > enc_ctx->bits_per_raw_sample = FFMIN(dec_ctx->bits_per_raw_sample, > av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth); > > + if (frame) { > + if (!av_dict_get(ost->encoder_opts, "color_range", NULL, 0)) It doesn't seem to me the checks are needed, since encoder_opts are applied AFTER this block, so they'd override whatever you set here anyway. Beyond that, looks very good.
On Tue, Oct 27, 2020 at 1:03 PM Anton Khirnov <anton@khirnov.net> wrote: > > Quoting Jan Ekström (2020-10-16 15:16:47) > > Additionally, reap the first rewards by being able to set the > > color related encoding values based on the passed AVFrame. > > > > The only tests that seem to have changed their results with this > > change seem to be the MXF tests. There, the muxer writes the > > limited/full range flag to the output container if the encoder > > is not set to "unspecified". > > --- > > fftools/ffmpeg.c | 42 +++++++++++++++++++++++++++---------- > > tests/ref/lavf/mxf_d10 | 2 +- > > tests/ref/lavf/mxf_dv25 | 2 +- > > tests/ref/lavf/mxf_dvcpro50 | 2 +- > > tests/ref/lavf/mxf_opatom | 2 +- > > 5 files changed, 35 insertions(+), 15 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 08db67a6ab..b2e210c814 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -941,9 +941,11 @@ early_exit: > > return float_pts; > > } > > > > -static int init_output_stream(OutputStream *ost, char *error, int error_len); > > +static int init_output_stream(OutputStream *ost, AVFrame *frame, > > + char *error, int error_len); > > > > -static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) > > +static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame, > > + unsigned int fatal) > > { > > int ret = AVERROR_BUG; > > char error[1024] = {0}; > > @@ -951,7 +953,7 @@ static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) > > if (ost->initialized) > > return 0; > > > > - ret = init_output_stream(ost, error, sizeof(error)); > > + ret = init_output_stream(ost, frame, error, sizeof(error)); > > if (ret < 0) { > > av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n", > > ost->file_index, ost->index, error); > > @@ -1125,7 +1127,7 @@ static void do_video_out(OutputFile *of, > > InputStream *ist = NULL; > > AVFilterContext *filter = ost->filter->filter; > > > > - init_output_stream_wrapper(ost, 1); > > + init_output_stream_wrapper(ost, next_picture, 1); > > sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture); > > > > if (ost->source_index >= 0) > > @@ -1507,7 +1509,7 @@ static int reap_filters(int flush) > > * the encoder earlier than receiving the first AVFrame. > > */ > > if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO) > > - init_output_stream_wrapper(ost, 1); > > + init_output_stream_wrapper(ost, NULL, 1); > > > > if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) { > > return AVERROR(ENOMEM); > > @@ -1930,7 +1932,7 @@ static void flush_encoders(void) > > finish_output_stream(ost); > > } > > > > - init_output_stream_wrapper(ost, 1); > > + init_output_stream_wrapper(ost, NULL, 1); > > } > > > > if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != AVMEDIA_TYPE_AUDIO) > > @@ -3302,7 +3304,7 @@ static void init_encoder_time_base(OutputStream *ost, AVRational default_time_ba > > enc_ctx->time_base = default_time_base; > > } > > > > -static int init_output_stream_encode(OutputStream *ost) > > +static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > > { > > InputStream *ist = get_input_stream(ost); > > AVCodecContext *enc_ctx = ost->enc_ctx; > > @@ -3399,6 +3401,23 @@ static int init_output_stream_encode(OutputStream *ost) > > enc_ctx->bits_per_raw_sample = FFMIN(dec_ctx->bits_per_raw_sample, > > av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth); > > > > + if (frame) { > > + if (!av_dict_get(ost->encoder_opts, "color_range", NULL, 0)) > > It doesn't seem to me the checks are needed, since encoder_opts are > applied AFTER this block, so they'd override whatever you set here > anyway. Ah, I kind of missed that being focused on the task at hand. I was so happy that the options were in a dict so they could be checked for (since, you know, unknown is still something the user could set). Oh the happiness :D . So I will then just simplify the code and remove the checks in that case. > > Beyond that, looks very good. Thanks. I think late encoder init is what API users should in general be doing unless their in/out is very constrained and they already know what we currently have as the lavfi black box will output. Jan
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 08db67a6ab..b2e210c814 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -941,9 +941,11 @@ early_exit: return float_pts; } -static int init_output_stream(OutputStream *ost, char *error, int error_len); +static int init_output_stream(OutputStream *ost, AVFrame *frame, + char *error, int error_len); -static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) +static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame, + unsigned int fatal) { int ret = AVERROR_BUG; char error[1024] = {0}; @@ -951,7 +953,7 @@ static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal) if (ost->initialized) return 0; - ret = init_output_stream(ost, error, sizeof(error)); + ret = init_output_stream(ost, frame, error, sizeof(error)); if (ret < 0) { av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n", ost->file_index, ost->index, error); @@ -1125,7 +1127,7 @@ static void do_video_out(OutputFile *of, InputStream *ist = NULL; AVFilterContext *filter = ost->filter->filter; - init_output_stream_wrapper(ost, 1); + init_output_stream_wrapper(ost, next_picture, 1); sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture); if (ost->source_index >= 0) @@ -1507,7 +1509,7 @@ static int reap_filters(int flush) * the encoder earlier than receiving the first AVFrame. */ if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO) - init_output_stream_wrapper(ost, 1); + init_output_stream_wrapper(ost, NULL, 1); if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) { return AVERROR(ENOMEM); @@ -1930,7 +1932,7 @@ static void flush_encoders(void) finish_output_stream(ost); } - init_output_stream_wrapper(ost, 1); + init_output_stream_wrapper(ost, NULL, 1); } if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != AVMEDIA_TYPE_AUDIO) @@ -3302,7 +3304,7 @@ static void init_encoder_time_base(OutputStream *ost, AVRational default_time_ba enc_ctx->time_base = default_time_base; } -static int init_output_stream_encode(OutputStream *ost) +static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) { InputStream *ist = get_input_stream(ost); AVCodecContext *enc_ctx = ost->enc_ctx; @@ -3399,6 +3401,23 @@ static int init_output_stream_encode(OutputStream *ost) enc_ctx->bits_per_raw_sample = FFMIN(dec_ctx->bits_per_raw_sample, av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth); + if (frame) { + if (!av_dict_get(ost->encoder_opts, "color_range", NULL, 0)) + enc_ctx->color_range = frame->color_range; + + if (!av_dict_get(ost->encoder_opts, "color_primaries", NULL, 0)) + enc_ctx->color_primaries = frame->color_primaries; + + if (!av_dict_get(ost->encoder_opts, "color_trc", NULL, 0)) + enc_ctx->color_trc = frame->color_trc; + + if (!av_dict_get(ost->encoder_opts, "colorspace", NULL, 0)) + enc_ctx->colorspace = frame->colorspace; + + if (!av_dict_get(ost->encoder_opts, "chroma_sample_location", NULL, 0)) + enc_ctx->chroma_sample_location = frame->chroma_location; + } + enc_ctx->framerate = ost->frame_rate; ost->st->avg_frame_rate = ost->frame_rate; @@ -3456,7 +3475,8 @@ static int init_output_stream_encode(OutputStream *ost) return 0; } -static int init_output_stream(OutputStream *ost, char *error, int error_len) +static int init_output_stream(OutputStream *ost, AVFrame *frame, + char *error, int error_len) { int ret = 0; @@ -3465,7 +3485,7 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len) AVCodecContext *dec = NULL; InputStream *ist; - ret = init_output_stream_encode(ost); + ret = init_output_stream_encode(ost, frame); if (ret < 0) return ret; @@ -3717,7 +3737,7 @@ static int transcode_init(void) output_streams[i]->enc_ctx->codec_type == AVMEDIA_TYPE_AUDIO)) continue; - ret = init_output_stream_wrapper(output_streams[i], 0); + ret = init_output_stream_wrapper(output_streams[i], NULL, 0); if (ret < 0) goto dump_format; } @@ -4650,7 +4670,7 @@ static int transcode_step(void) * early encoder initialization. */ if (av_buffersink_get_type(ost->filter->filter) == AVMEDIA_TYPE_AUDIO) - init_output_stream_wrapper(ost, 1); + init_output_stream_wrapper(ost, NULL, 1); if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0) return ret; diff --git a/tests/ref/lavf/mxf_d10 b/tests/ref/lavf/mxf_d10 index aea469bb58..85e337d157 100644 --- a/tests/ref/lavf/mxf_d10 +++ b/tests/ref/lavf/mxf_d10 @@ -1,3 +1,3 @@ -e597f73ef9c9819710d2f815813eb91f *tests/data/lavf/lavf.mxf_d10 +36fc2a640368f6d33987d2b2d39df966 *tests/data/lavf/lavf.mxf_d10 5332013 tests/data/lavf/lavf.mxf_d10 tests/data/lavf/lavf.mxf_d10 CRC=0x6c74d488 diff --git a/tests/ref/lavf/mxf_dv25 b/tests/ref/lavf/mxf_dv25 index db6b76c6f8..d4559df862 100644 --- a/tests/ref/lavf/mxf_dv25 +++ b/tests/ref/lavf/mxf_dv25 @@ -1,3 +1,3 @@ -0fc964fa22bc8b3a389b81b9a2efccb3 *tests/data/lavf/lavf.mxf_dv25 +57623b3b968c0bb0d8a0bbaeef6fe657 *tests/data/lavf/lavf.mxf_dv25 3834413 tests/data/lavf/lavf.mxf_dv25 tests/data/lavf/lavf.mxf_dv25 CRC=0xbdaf7f52 diff --git a/tests/ref/lavf/mxf_dvcpro50 b/tests/ref/lavf/mxf_dvcpro50 index 09999914bf..8bcf67d17f 100644 --- a/tests/ref/lavf/mxf_dvcpro50 +++ b/tests/ref/lavf/mxf_dvcpro50 @@ -1,3 +1,3 @@ -aa81ea83af44a69e73849e327cc4bd12 *tests/data/lavf/lavf.mxf_dvcpro50 +6e82b4cc962199e2593e30851ff7ccfb *tests/data/lavf/lavf.mxf_dvcpro50 7431213 tests/data/lavf/lavf.mxf_dvcpro50 tests/data/lavf/lavf.mxf_dvcpro50 CRC=0xe3bbe4b4 diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom index 05794a4e5e..1aa843a22a 100644 --- a/tests/ref/lavf/mxf_opatom +++ b/tests/ref/lavf/mxf_opatom @@ -1,3 +1,3 @@ -06a1816aa91c733e1ef7e45d82e4f1d3 *tests/data/lavf/lavf.mxf_opatom +d5f56215c2b16dee204f03bfa653dd1b *tests/data/lavf/lavf.mxf_opatom 4717625 tests/data/lavf/lavf.mxf_opatom tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a