On Sun, Jun 29, 2025 at 7:01 PM Michael Niedermayer <mich...@niedermayer.cc> wrote:
> Hi > > On Sat, Jun 28, 2025 at 08:49:08PM -0600, Pavel Koshevoy wrote: > > On Sat, Jun 28, 2025 at 5:03 PM Michael Niedermayer < > mich...@niedermayer.cc> > > wrote: > > > > > Hi > > > > > > On Sun, Jun 22, 2025 at 12:10:30PM -0600, Pavel Koshevoy wrote: > > > > Make midstream AVStream.codecpar updates optional and disabled > > > > by default, so that avformat API clients can enable this feature > > > > explicitly when they add support for midstream codec changes. > > > > --- > > > > doc/APIchanges | 3 +++ > > > > doc/formats.texi | 5 +++++ > > > > libavformat/avformat.h | 2 ++ > > > > libavformat/mpegts.c | 4 +++- > > > > libavformat/options_table.h | 1 + > > > > libavformat/version.h | 2 +- > > > > tests/fate/demux.mak | 2 +- > > > > 7 files changed, 16 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > > > index 91710bb27d..43172fbcdd 100644 > > > > --- a/doc/APIchanges > > > > +++ b/doc/APIchanges > > > > @@ -2,6 +2,9 @@ The last version increases of all libraries were on > > > 2025-03-28 > > > > > > > > API changes, most recent first: > > > > > > > > +2025-06-17 - xxxxxxxxxx - lavf 62.2.100 - avformat.h > > > > + Add AVFMT_FLAG_ALLOW_CODEC_CHANGES flag. > > > > + > > > > 2025-05-21 - xxxxxxxxxx - lavu 60.3.100 - avassert.h > > > > Add av_unreachable() and av_assume() macros. > > > > > > > > diff --git a/doc/formats.texi b/doc/formats.texi > > > > index 876a9e92b3..5a0d070247 100644 > > > > --- a/doc/formats.texi > > > > +++ b/doc/formats.texi > > > > @@ -39,6 +39,11 @@ Set format flags. Some are implemented for a > limited > > > number of formats. > > > > > > > > Possible values for input files: > > > > @table @samp > > > > +@item allow_codec_changes > > > > +Allow AVStream.codecpar to change midstream if input changes > > > > +(for example when MPEG-TS ES stream_type changes). > > > > +This is disabled by default, because most clients of avformat API > > > > +do not support random midstream codec changes. > > > > @item discardcorrupt > > > > Discard corrupted packets. > > > > @item fastseek > > > > > > should this document the relation to resolution & pixfmt changes ? > > > > > > > How about this: > > > > Allow AVStream.codecpar to change midstream if input changes > > (for example when MPEG-TS ES stream_type changes). > > This is disabled by default, because most clients of avformat API > > do not support random midstream codec changes. > > Changes may include media type, codec id, pixel format, color specs, > > channel layout, sample rate, and may be accompanied by timeline > anomalies. > > The chain suppports pixel format and resolution changes from demuxers > to decoder and ffmpeg/ffplay completely unrelated to this flag. > > IDK what this means ... I do know that ffplay can't play slate-to-network-short.ts correctly when stream changes from SDR mpeg2 video to HDR10 hevc video, with or without this flag. > > > > > > > > > > > > > > > > > > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > > > > index b6c63e2237..2e5232c96d 100644 > > > > --- a/libavformat/avformat.h > > > > +++ b/libavformat/avformat.h > > > > @@ -1436,6 +1436,8 @@ typedef struct AVFormatContext { > > > > #define AVFMT_FLAG_FAST_SEEK 0x80000 ///< Enable fast, but > inaccurate > > > seeks for some formats > > > > #define AVFMT_FLAG_AUTO_BSF 0x200000 ///< Add bitstream filters as > > > requested by the muxer > > > > > > > > +#define AVFMT_FLAG_ALLOW_CODEC_CHANGES 0x400000 ///< Allow > > > AVStream.codecpar to be updated midstream if input changes (e.g. > MPEG-TS ES > > > stream_type changes) > > > > + > > > > /** > > > > * Maximum number of bytes read from input in order to determine > > > stream > > > > * properties. Used when reading the global header and in > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > > > > index deb69a0548..ed4ff580e5 100644 > > > > --- a/libavformat/mpegts.c > > > > +++ b/libavformat/mpegts.c > > > > @@ -2510,7 +2510,9 @@ static void pmt_cb(MpegTSFilter *filter, const > > > uint8_t *section, int section_len > > > > if (!st) > > > > goto out; > > > > > > > > - if (pes && pes->stream_type != stream_type) > > > > + if (pes && (!pes->stream_type || > > > > + (pes->stream_type != stream_type && > > > > + !!(ts->stream->flags & > > > AVFMT_FLAG_ALLOW_CODEC_CHANGES)))) > > > > mpegts_set_stream_info(st, pes, stream_type, > prog_reg_desc); > > > > > > > > add_pid_to_program(prg, pid); > > > > diff --git a/libavformat/options_table.h > b/libavformat/options_table.h > > > > index e2e690fd2a..811dd342cc 100644 > > > > --- a/libavformat/options_table.h > > > > +++ b/libavformat/options_table.h > > > > @@ -50,6 +50,7 @@ static const AVOption avformat_options[] = { > > > > {"sortdts", "try to interleave outputted packets by dts", 0, > > > AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_SORT_DTS }, INT_MIN, INT_MAX, D, > > > .unit = "fflags"}, > > > > {"fastseek", "fast but inaccurate seeks", 0, AV_OPT_TYPE_CONST, > {.i64 = > > > AVFMT_FLAG_FAST_SEEK }, INT_MIN, INT_MAX, D, .unit = "fflags"}, > > > > {"nobuffer", "reduce the latency introduced by optional buffering", > 0, > > > AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_NOBUFFER }, 0, INT_MAX, D, .unit > = > > > "fflags"}, > > > > +{"allow_codec_changes", "allow AVStream.codecpar to change at > runtime > > > if input changes", 0, AV_OPT_TYPE_CONST, { .i64 = > > > AVFMT_FLAG_ALLOW_CODEC_CHANGES }, 0, 0, D, .unit = "fflags" }, > > > > > > If the option is user settable then ffmpeg/ffplay should possibly > print a > > > warning, that they do not support this if the user forces the flag > > > > > > > Well, it's not as if they don't support it at all ... they consume the > > option and set the flag, and the demuxer does what it's supposed to. > > In case of ffplay, with -fflags +allow_codec_changes it's actually able > to > > play 1_poc.mp4 and render a partially decoded image. > > > > > Anyway, what would this warning look like? > > I'm having a hard time coming up with a warning message that isn't > > misleading or confusing. > > Would you like to suggest one? Would something like this work: > > > > if (!!(ic->flags & AVFMT_FLAG_ALLOW_CODEC_CHANGES)) > > av_log(NULL, AV_LOG_WARNING, "downstream support for > > allow_codec_changes is not implemented"); > > I was thinking more along the lines of the > "--disable-safe-bitstream-reader" > help text: > disable buffer boundary checking in bitreaders > (This disables some security checks and can > cause undefined behavior, > crashes and arbitrary code execution, it may > be faster, but > should only be used with trusted input) > > Bascially, what the goal here is, is to explain to the user that with this > flag > they are potentially giving the input file arbitrary code execution on > their > machiene > No, that's not what this flag does, and I will not document it as such. The arbitrary code execution is not a feature controlled by this flag... if such thing exists -- it was not added by me. This flag only controls the re-probing of the input stream when PMT ES stream_type changes, that is all. The ffprobe 1_poc.mp4 segfault was due to a previously unknown defect in demux.c codec_close probing helper function, for which I provided a fix. > > I still think andreas comment that this should be set by the application > not the user should be considered. > Fine, I can make this feature more hidden. That would also obviate any need to document it in formats.texi, so I'll remove that as well. > Can you explain in what use case this is usefull ? > It is useful for playback of real-world mpeg-ts captures like the one I've provided: slate-to-network-short.ts ... but not with ffplay, obviously. > > (you cannot saftely use this for any untrusted input if the tool does not > fully support this) > By this reasoning I wouldn't trust using ffmpeg with any un-trusted input, regardless of this flag. A fix for the codec_close bug in demux.c has been committed, and I am not aware of any additional issues that may be exposed when this is enabled. I will submit a new patch removing the allow_codec_changes flag from option_table.h and formats.texi Pavel _______________________________________________ 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".