Re: [FFmpeg-devel] [PATCH] avformat/demux: Fix segfault due to avcodec_open2 failure (v2)

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

On Tue, Jun 10, 2025 at 9:29 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Hi Pavel
>
> On Tue, Jun 10, 2025 at 08:42:08AM -0600, Pavel Koshevoy wrote:
> > On Tue, Jun 10, 2025, 07:39 Michael Niedermayer <michael@niedermayer.cc>
> > wrote:
> >
> > > On Mon, Jun 09, 2025 at 09:45:28PM -0600, Pavel Koshevoy wrote:
> > > > Fixes 'ffprobe 1_poc.mp4' segfault introduced with
> > > > commit 0021484d05f9b0f032fa319399de6e24eea0c04f
> > > >
> > > > codec_close should not assume that the codec_id did not change.
> > > > ---
> > > >  libavformat/demux.c | 8 +++++++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavformat/demux.c b/libavformat/demux.c
> > > > index ecd4f40da9..3749ab67a3 100644
> > > > --- a/libavformat/demux.c
> > > > +++ b/libavformat/demux.c
> > > > @@ -1292,9 +1292,15 @@ static int codec_close(FFStream *sti)
> > > >  {
> > > >      AVCodecContext *avctx_new = NULL;
> > > >      AVCodecParameters *par_tmp = NULL;
> > > > +    const AVCodec *new_codec = NULL;
> > > >      int ret;
> > > >
> > > > -    avctx_new = avcodec_alloc_context3(sti->avctx->codec);
> > > > +    new_codec =
> > > > +      (sti->avctx->codec_id != sti->pub.codecpar->codec_id) ?
> > > > +      avcodec_find_decoder(sti->pub.codecpar->codec_id) :
> > > > +      sti->avctx->codec;
> > > > +
> > > > +    avctx_new = avcodec_alloc_context3(new_codec);
> > > >      if (!avctx_new) {
> > > >          ret = AVERROR(ENOMEM);
> > > >          goto fail;
> > >
> > > This is not about request_probe
> > > but about the mpegts demuxer randomly changeing codec id midstream
> > >
> >
> >
> > I have several real (not crafted like 1_poc.mp4 is) .ts files where codec
> > changes from mpeg2video to hevc, from mpeg2audio to eac3 -- while
> remaining
> > on the same PIDs.  I also have .ts files where codec switches between
> > mpeg2video and h264.  VLC was able to play such files, but my ffmpeg
> based
> > player (apprenticevideo) could not even see that the codecs changed prior
> > to 0021484d05f9b0f032fa319399de6e24eea0c04f.
>
> do these work ?
> (work here means the result is a complete file with all frames from the
> input
>  and is playable and seekable)
> ./ffmpeg -i input.ts -codec copy output.ts
> ./ffmpeg -i input.ts -codec copy output.mp4
> ./ffmpeg -i input.ts -vcodec libx264 -acodec libopus output.mkv
>

not really relevant because if they don't work -- it's a defect in the
command line tool, not the demuxer.
I need the libavformat API to work with mpeg-ts data I have, I don't
actually care if ffprobe/ffplay/ffmpeg can handle these files ... they
couldn't handle them before and they still can't, but I'm not an fftools
maintainer so I don't think I'll be taking on a 3rd job to fix fftools as
well.



>
> > Reverting isn't really an
> > option for me, not unless there is a better solution presented.
>
> is adding an exploitable security issue an option for you ?
>

I'm not a security expert, but IIRC the patch I've posted here fixes the
segfault you are referring to?



>
> If people want to keep this, it should be behind a flag and
> disabled by default.
>
> Its not enough to fix our code that crashes, other applications
> similarly wont expect such id and type changes mid stream
>

If mpeg-ts allows the codecs to change at any time -- it's a bug in the
application if it doesn't support that.



>
>
> >
> > As I am primarily a public ffmpeg API user -- I am well out of my depth
> > when it comes to making non-trivial changes to ffmpegs internals.
>
> Thats ok, but you applied this change to ffmpeg internals, and here
> you say "I am well out of my depth when it comes to making non-trivial
> changes to ffmpegs internals."
>
> Did someone review this ?
>

No, it was ignored for weeks.



>
> commit 0021484d05f9b0f032fa319399de6e24eea0c04f
> Author:     Pavel Koshevoy <pkoshevoy@gmail.com>
> AuthorDate: Sun May 18 08:57:31 2025 -0600
> Commit:     Pavel Koshevoy <pkoshevoy@gmail.com>
> CommitDate: Sun May 18 08:57:31 2025 -0600
>
>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
> _______________________________________________
> 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".
>
_______________________________________________
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".