[FFmpeg-devel] [PATCH v3 4/6] ffmpeg: pass decoded or filtered AVFrame to output stream initialization

[Jan Ekström]

On Tue, Oct 27, 2020 at 1:03 PM Anton Khirnov <anton at 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