Re: [FFmpeg-devel] [PATCH] avformat/mpegts: Add AVFMT_FLAG_ALLOW_CODEC_CHANGES

[Pavel Koshevoy <pkoshevoy-at-gmail.com@ffmpeg.org>]

On Sun, Jun 29, 2025 at 7:01 PM Michael Niedermayer <michael@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 <
> michael@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-request@ffmpeg.org with subject "unsubscribe".