On Tue, Oct 27, 2020 at 1:03 PM Anton Khirnov <an...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
