> From: Devin Heitmueller <devin.heitmuel...@ltnglobal.com> > Sent: Tuesday, June 9, 2020 23:47 > To: FFmpeg development discussions and patches <ffmpeg- > de...@ffmpeg.org> > Cc: Fu, Linjie <linjie...@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg: flush and recreate > encoder instance if resolution changes > > On Tue, Jun 9, 2020 at 4:53 AM Linjie Fu <linjie...@intel.com> wrote: > > > > Signed-off-by: Linjie Fu <linjie...@intel.com> > > --- > > Should be squashed with: > > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1434 > > > > fftools/ffmpeg.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 5859781..8cdd532 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -130,6 +130,7 @@ static void do_video_stats(OutputStream *ost, int > frame_size); > > static BenchmarkTimeStamps get_benchmark_time_stamps(void); > > static int64_t getmaxrss(void); > > static int ifilter_has_all_input_formats(FilterGraph *fg); > > +static void flush_encoders(void); > > > > static int run_as_daemon = 0; > > static int nb_frames_dup = 0; > > @@ -1058,11 +1059,21 @@ static void do_video_out(OutputFile *of, > > > > if (next_picture && (enc->width != next_picture->width || > > enc->height != next_picture->height)) { > > + flush_encoders(); > > + avcodec_flush_buffers(enc); > > if (!(enc->codec->capabilities & > AV_CODEC_CAP_VARIABLE_DIMENSIONS)) { > > av_log(NULL, AV_LOG_ERROR, "Variable dimension encoding " > > "is not supported by %s.\n", enc->codec->name); > > goto error; > > } > > + > > + enc->width = next_picture->width; > > + enc->height = next_picture->height; > > Perhaps from a workflow standpoint it makes more sense to move the > code which changes the encoder parameters to after where you close the > existing encoder (i.e. between the close and init calls). I can't > think of a specific case where this might break a downstream encoder, That's the reason I simply set the width/height ahead. > but it seems like a good practice to only have the parameters applied > to the new encoder instance. Indeed, fixed locally.
> > + > > + if (enc->codec->close(enc) < 0) > > + goto error; > > + if (enc->codec->init(enc) < 0) > > + goto error; > > } > > > > if (ost->source_index >= 0) > > In general do we really think this is a safe thing to do? Does > something also need to be propagated to the output as well? I know > that this would break use cases like the decklink output where the > frame resolution suddenly changes in the middle of the stream without > calling the output's write_header() routine. Yes, noticed the constraints in sequence header and container. Since resolution changing is allowed in single bitstream, one global header is not enough anymore as Nicolas has pointed out in [1]. Hence as to the container level, for the formats with AVFMT_GLOBALHEADER flag, we should place sps/pps information in key frame instead of in extradata. (e.g. disable AV_CODEC_FLAG_GLOBAL_HEADER) - if (oc->oformat->flags & AVFMT_GLOBALHEADER) + if (oc->oformat->flags & AVFMT_GLOBALHEADER && ost->autoscale) ost->enc_ctx->flags |= AV_CODEC_FLAG_GLOBAL_HEADER; Verified this works, at least for containers like mp4. - Linjie [1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1591606685-4450-1-git-send-email-linjie...@intel.com/#55293 _______________________________________________ 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".