[FFmpeg-devel] [PATCH v3] Improved the performance of 1 decode + N filter graphs and adaptive bitrate.

[Guo, Yejun]

> -----Original Message-----
> From: Wang, Shaofei
> Sent: Thursday, January 17, 2019 3:36 PM
> To: Guo, Yejun <yejun.guo at intel.com>; FFmpeg development discussions
> and patches <ffmpeg-devel at ffmpeg.org>
> Cc: michael at niedermayer.cc; atomnuker at gmail.com; cus at passwd.hu;
> ceffmpeg at gmail.com
> Subject: RE: [FFmpeg-devel] [PATCH v3] Improved the performance of 1
> decode + N filter graphs and adaptive bitrate.
> 
> > From: Guo, Yejun
> > Sent: Thursday, January 17, 2019 9:25 AM
> > To: FFmpeg development discussions and patches
> > <ffmpeg-devel at ffmpeg.org>
> > Cc: michael at niedermayer.cc; atomnuker at gmail.com; cus at passwd.hu;
> Wang,
> > Shaofei <shaofei.wang at intel.com>; ceffmpeg at gmail.com
> > Subject: RE: [FFmpeg-devel] [PATCH v3] Improved the performance of 1
> > decode + N filter graphs and adaptive bitrate.
> >
> >
> > > +static int pipeline_reap_filters(int flush, InputFilter * ifilter) {
> > > +    AVFrame *filtered_frame = NULL;
> > > +    int i;
> > > +
> > > +    for (i = 0; i < nb_output_streams; i++) {
> > > +        if (ifilter == output_streams[i]->filter->graph->inputs[0]) break;
> > > +    }
> > > +    OutputStream *ost = output_streams[i];
> > > +    OutputFile    *of = output_files[ost->file_index];
> > > +    AVFilterContext *filter;
> > > +    AVCodecContext *enc = ost->enc_ctx;
> > > +    int ret = 0;
> > > +
> > > +    if (!ost->filter || !ost->filter->graph->graph)
> > > +        return 0;
> > > +    filter = ost->filter->filter;
> > > +
> > > +    if (!ost->initialized) {
> > > +        char error[1024] = "";
> > > +        ret = init_output_stream(ost, 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);
> > > +            exit_program(1);
> >
> > imo, it's not good to exit the program.
> Any reason? These lines are similar as them in reap_filters(). Line 1445.

I'm just wondering, in general, how the program could exit in the middle. And the function does have a return value for error code. 

> 
> > > +        }
> > > +    }
> > > +
> > > +    if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc()))
> > > +        return AVERROR(ENOMEM);
> > > +    filtered_frame = ost->filtered_frame;
> > > +
> > > +    while (1) {
> > > +        double float_pts = AV_NOPTS_VALUE; // this is identical to
> > > filtered_frame.pts but with higher precision
> > > +        ret = av_buffersink_get_frame_flags(filter, filtered_frame,
> > > +
> > AV_BUFFERSINK_FLAG_NO_REQUEST);
> > > +        if (ret < 0) {
> > > +            if (ret != AVERROR(EAGAIN) && ret != AVERROR_EOF) {
> > > +                av_log(NULL, AV_LOG_WARNING,
> > > +                       "Error in av_buffersink_get_frame_flags():
> > > + %s\n",
> > > av_err2str(ret));
> > > +            } else if (flush && ret == AVERROR_EOF) {
> > > +                if (av_buffersink_get_type(filter) ==
> > AVMEDIA_TYPE_VIDEO)
> > > +                    do_video_out(of, ost, NULL, AV_NOPTS_VALUE);
> > > +            }
> > > +            break;
> > > +        }
> > > +        if (ost->finished) {
> > > +            av_frame_unref(filtered_frame);
> > > +            continue;
> > > +        }
> > > +        if (filtered_frame->pts != AV_NOPTS_VALUE) {
> > > +            int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ?
> > > + 0 : of-
> > > >start_time;
> > > +            AVRational filter_tb = av_buffersink_get_time_base(filter);
> > > +            AVRational tb = enc->time_base;
> > > +            int extra_bits = av_clip(29 - av_log2(tb.den), 0, 16);
> > > +
> > > +            tb.den <<= extra_bits;
> > > +            float_pts =
> > > +                av_rescale_q(filtered_frame->pts, filter_tb, tb) -
> > > +                av_rescale_q(start_time, AV_TIME_BASE_Q, tb);
> > > +            float_pts /= 1 << extra_bits;
> > > +            // avoid exact midoints to reduce the chance of
> > > + rounding differences,
> > > this can be removed in case the fps code is changed to work with
> > > integers
> > > +            float_pts += FFSIGN(float_pts) * 1.0 / (1<<17);
> > > +
> > > +            filtered_frame->pts =
> > > +                av_rescale_q(filtered_frame->pts, filter_tb,
> > enc->time_base) -
> > > +                av_rescale_q(start_time, AV_TIME_BASE_Q,
> > enc->time_base);
> > > +        }
> > > +
> > > +        switch (av_buffersink_get_type(filter)) {
> > > +        case AVMEDIA_TYPE_VIDEO:
> > > +            if (!ost->frame_aspect_ratio.num)
> > > +                enc->sample_aspect_ratio =
> > > + filtered_frame->sample_aspect_ratio;
> > > +
> > > +            if (debug_ts) {
> > > +                av_log(NULL, AV_LOG_INFO, "filter -> pts:%s
> > > + pts_time:%s exact:%f
> > > time_base:%d/%d\n",
> > > +                        av_ts2str(filtered_frame->pts),
> > > + av_ts2timestr(filtered_frame-
> > > >pts, &enc->time_base),
> > > +                        float_pts,
> > > +                        enc->time_base.num, enc->time_base.den);
> > > +            }
> > > +
> > > +            do_video_out(of, ost, filtered_frame, float_pts);
> > > +            break;
> > > +        case AVMEDIA_TYPE_AUDIO:
> > > +            if (!(enc->codec->capabilities &
> > > + AV_CODEC_CAP_PARAM_CHANGE)
> > > &&
> > > +                enc->channels != filtered_frame->channels) {
> > > +                av_log(NULL, AV_LOG_ERROR,
> > > +                       "Audio filter graph output is not normalized
> > > + and encoder does
> > > not support parameter changes\n");
> > > +                break;
> > > +            }
> > > +            do_audio_out(of, ost, filtered_frame);
> > > +            break;
> > > +        default:
> > > +            // TODO support subtitle filters
> > > +            av_assert0(0);
> >
> > maybe better to return AVERROR_PATCHWELCOME?
> Is it expected to terminate the program here? It also the similar as previous
> code in reap_filters()

imho, I think the code can be improved to add more description here, instead of the simple assert(0).


anyway, just wait for maintainer's comment.

>