[FFmpeg-devel] avformat/mpegts: add skip_unknown_pmt option

Submitted by Aman Gupta on May 18, 2018, 12:04 a.m.

Details

Message ID 20180518000444.19218-1-ffmpeg@tmm1.net
State Accepted
Commit e24d768a76b14a1cbea0ec622c89573f7d06fdee
Headers show

Commit Message

Aman Gupta May 18, 2018, 12:04 a.m.
From: Aman Gupta <aman@tmm1.net>

Some filtered mpegts streams may erroneously include PMTs for programs
that are not advertised in the PAT. This confuses ffmpeg and most
players because multiple audio/video streams are created and it is
unclear which ones actually contain data.

See for example https://tmm1.s3.amazonaws.com/unknown-pmts.ts

Before:

    Input #0, mpegts, from 'unknown-pmts.ts':
      Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
      Program 4
        Stream #0:2[0x41]: Video: mpeg2video (Main) ([2][0][0][0] / 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9], Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
        Stream #0:3[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, 5.1(side), fltp, 384 kb/s
        Stream #0:4[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, stereo, fltp, 128 kb/s
      No Program
        Stream #0:0[0x31]: Video: mpeg2video ([2][0][0][0] / 0x0002), none(tv), 90k tbr, 90k tbn, 90k tbc
        Stream #0:1[0x34](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 channels, fltp
        Stream #0:5[0x51]: Video: mpeg2video ([2][0][0][0] / 0x0002), none, 90k tbr, 90k tbn
        Stream #0:6[0x54](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 channels

With skip_unknown_pmt=1:

    Input #0, mpegts, from 'unknown-pmts.ts':
      Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
      Program 4
        Stream #0:0[0x41]: Video: mpeg2video (Main) ([2][0][0][0] / 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9], Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
        Stream #0:1[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, 5.1(side), fltp, 384 kb/s
        Stream #0:2[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, stereo, fltp, 128 kb/s

Signed-off-by: Aman Gupta <aman@tmm1.net>
---
 doc/demuxers.texi    | 3 +++
 libavformat/mpegts.c | 5 +++++
 2 files changed, 8 insertions(+)

Comments

Aman Gupta May 18, 2018, 12:21 a.m.
On Thu, May 17, 2018 at 5:04 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:

> From: Aman Gupta <aman@tmm1.net>
>
> Some filtered mpegts streams may erroneously include PMTs for programs
> that are not advertised in the PAT. This confuses ffmpeg and most
> players because multiple audio/video streams are created and it is
> unclear which ones actually contain data.
>
> See for example https://tmm1.s3.amazonaws.com/unknown-pmts.ts
>
> Before:
>
>     Input #0, mpegts, from 'unknown-pmts.ts':
>       Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
>       Program 4
>         Stream #0:2[0x41]: Video: mpeg2video (Main) ([2][0][0][0] /
> 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9],
> Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
>         Stream #0:3[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> 5.1(side), fltp, 384 kb/s
>         Stream #0:4[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> stereo, fltp, 128 kb/s
>       No Program
>         Stream #0:0[0x31]: Video: mpeg2video ([2][0][0][0] / 0x0002),
> none(tv), 90k tbr, 90k tbn, 90k tbc
>         Stream #0:1[0x34](eng): Audio: ac3 (AC-3 / 0x332D4341), 0
> channels, fltp
>         Stream #0:5[0x51]: Video: mpeg2video ([2][0][0][0] / 0x0002),
> none, 90k tbr, 90k tbn
>         Stream #0:6[0x54](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 channels
>
> With skip_unknown_pmt=1:
>
>     Input #0, mpegts, from 'unknown-pmts.ts':
>       Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
>       Program 4
>         Stream #0:0[0x41]: Video: mpeg2video (Main) ([2][0][0][0] /
> 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9],
> Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
>         Stream #0:1[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> 5.1(side), fltp, 384 kb/s
>         Stream #0:2[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> stereo, fltp, 128 kb/s
>
> Signed-off-by: Aman Gupta <aman@tmm1.net>
> ---
>  doc/demuxers.texi    | 3 +++
>  libavformat/mpegts.c | 5 +++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index e7c2abce57..1d2ee5bf37 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -538,6 +538,9 @@ This demuxer accepts the following options:
>  Set size limit for looking up a new synchronization. Default value is
>  65536.
>
> +@item skip_unknown_pmt
> +Skip PMTs for programs not defined in the PAT. Default value is 0.
>

Maybe it's worth turning this option on by default?

We could also add debug/verbose logging when these PMTs are skipped,
but since it would happen every time the PMT is received it could become
very noisy.

Aman


> +
>  @item fix_teletext_pts
>  Override teletext packet PTS and DTS values with the timestamps calculated
>  from the PCR of the first program which the teletext stream is part of
> and is
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index fc9bb3940e..18df1afd9d 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -143,6 +143,7 @@ struct MpegTSContext {
>
>      int skip_changes;
>      int skip_clear;
> +    int skip_unknown_pmt;
>
>      int scan_all_pmts;
>
> @@ -172,6 +173,8 @@ static const AVOption options[] = {
>       {.i64 = 0}, 0, 0, AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_EXPORT |
> AV_OPT_FLAG_READONLY },
>      {"scan_all_pmts", "scan and combine all PMTs",
> offsetof(MpegTSContext, scan_all_pmts), AV_OPT_TYPE_BOOL,
>       {.i64 = -1}, -1, 1, AV_OPT_FLAG_DECODING_PARAM },
> +    {"skip_unknown_pmt", "skip PMTs for programs not advertised in the
> PAT", offsetof(MpegTSContext, skip_unknown_pmt), AV_OPT_TYPE_BOOL,
> +     {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
>      {"skip_changes", "skip changing / adding streams / programs",
> offsetof(MpegTSContext, skip_changes), AV_OPT_TYPE_BOOL,
>       {.i64 = 0}, 0, 1, 0 },
>      {"skip_clear", "skip clearing programs", offsetof(MpegTSContext,
> skip_clear), AV_OPT_TYPE_BOOL,
> @@ -2028,6 +2031,8 @@ static void pmt_cb(MpegTSFilter *filter, const
> uint8_t *section, int section_len
>      if (!ts->scan_all_pmts && ts->skip_changes)
>          return;
>
> +    if (ts->skip_unknown_pmt && !get_program(ts, h->id))
> +        return;
>      if (!ts->skip_clear)
>          clear_program(ts, h->id);
>
> --
> 2.14.2
>
>
wm4 May 18, 2018, 9:50 a.m.
On Thu, 17 May 2018 17:21:42 -0700
Aman Gupta <aman@tmm1.net> wrote:

> On Thu, May 17, 2018 at 5:04 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> > From: Aman Gupta <aman@tmm1.net>
> >
> > Some filtered mpegts streams may erroneously include PMTs for programs
> > that are not advertised in the PAT. This confuses ffmpeg and most
> > players because multiple audio/video streams are created and it is
> > unclear which ones actually contain data.
> >
> > See for example https://tmm1.s3.amazonaws.com/unknown-pmts.ts
> >
> > Before:
> >
> >     Input #0, mpegts, from 'unknown-pmts.ts':
> >       Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
> >       Program 4
> >         Stream #0:2[0x41]: Video: mpeg2video (Main) ([2][0][0][0] /
> > 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9],
> > Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
> >         Stream #0:3[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> > 5.1(side), fltp, 384 kb/s
> >         Stream #0:4[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> > stereo, fltp, 128 kb/s
> >       No Program
> >         Stream #0:0[0x31]: Video: mpeg2video ([2][0][0][0] / 0x0002),
> > none(tv), 90k tbr, 90k tbn, 90k tbc
> >         Stream #0:1[0x34](eng): Audio: ac3 (AC-3 / 0x332D4341), 0
> > channels, fltp
> >         Stream #0:5[0x51]: Video: mpeg2video ([2][0][0][0] / 0x0002),
> > none, 90k tbr, 90k tbn
> >         Stream #0:6[0x54](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 channels
> >
> > With skip_unknown_pmt=1:
> >
> >     Input #0, mpegts, from 'unknown-pmts.ts':
> >       Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s
> >       Program 4
> >         Stream #0:0[0x41]: Video: mpeg2video (Main) ([2][0][0][0] /
> > 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9],
> > Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc
> >         Stream #0:1[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> > 5.1(side), fltp, 384 kb/s
> >         Stream #0:2[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz,
> > stereo, fltp, 128 kb/s
> >
> > Signed-off-by: Aman Gupta <aman@tmm1.net>
> > ---
> >  doc/demuxers.texi    | 3 +++
> >  libavformat/mpegts.c | 5 +++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > index e7c2abce57..1d2ee5bf37 100644
> > --- a/doc/demuxers.texi
> > +++ b/doc/demuxers.texi
> > @@ -538,6 +538,9 @@ This demuxer accepts the following options:
> >  Set size limit for looking up a new synchronization. Default value is
> >  65536.
> >
> > +@item skip_unknown_pmt
> > +Skip PMTs for programs not defined in the PAT. Default value is 0.
> >  
> 
> Maybe it's worth turning this option on by default?

Sounds reasonable, but I don't know too much about TS.

> We could also add debug/verbose logging when these PMTs are skipped,
> but since it would happen every time the PMT is received it could become
> very noisy.

Just make it a higher log level.
Devin Heitmueller May 18, 2018, 1:12 p.m.
Hello Aman,

On Thu, May 17, 2018 at 8:04 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> From: Aman Gupta <aman@tmm1.net>
>
> Some filtered mpegts streams may erroneously include PMTs for programs
> that are not advertised in the PAT. This confuses ffmpeg and most
> players because multiple audio/video streams are created and it is
> unclear which ones actually contain data.

I guess my big question would be, why is the ffmpeg TS demux
interpreting some PIDs as containing PMT if they are not referenced by
the PAT?  I have to assume there is some heuristic in there which
looks at arbitrary PID data and attempts to treat it as a PMT table
(presumably in an attempt to play TS streams which are missing PAT).
If that's the case, then I think *that's* what the patch should
disable - assume the PAT/PMT are present and well-formed and don't
play games trying to find a PMT assuming the PAT is absent/malformed.

I can appreciate a player doing it's best to play some screwed up
stream, but that sort of logic should not be the default.  There are
all sorts of strange proprietary data sent by broadcasters over PIDs
and I wouldn't want those to get mis-detected as PMT when the spec
says PIDs not explicitly in the PAT/PMT should be completely ignored.

Devin
Aman Gupta May 18, 2018, 5:15 p.m.
On Fri, May 18, 2018 at 6:12 AM, Devin Heitmueller <
dheitmueller@kernellabs.com> wrote:

> Hello Aman,
>
> On Thu, May 17, 2018 at 8:04 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> > From: Aman Gupta <aman@tmm1.net>
> >
> > Some filtered mpegts streams may erroneously include PMTs for programs
> > that are not advertised in the PAT. This confuses ffmpeg and most
> > players because multiple audio/video streams are created and it is
> > unclear which ones actually contain data.
>
> I guess my big question would be, why is the ffmpeg TS demux
> interpreting some PIDs as containing PMT if they are not referenced by
> the PAT?  I have to assume there is some heuristic in there which
> looks at arbitrary PID data and attempts to treat it as a PMT table
> (presumably in an attempt to play TS streams which are missing PAT).
>

There is no such heuristic, and ffmpeg is not interpreting random PIDs as
containing PMTs.

The issue is that the PMT PID advertised in the PAT contains multiple PMTs
for different programs. This is because the broadcaster decided to re-use
the same PID for multiple program PMTs.


> If that's the case, then I think *that's* what the patch should
> disable - assume the PAT/PMT are present and well-formed and don't
> play games trying to find a PMT assuming the PAT is absent/malformed.
>
> I can appreciate a player doing it's best to play some screwed up
> stream, but that sort of logic should not be the default.  There are
> all sorts of strange proprietary data sent by broadcasters over PIDs
> and I wouldn't want those to get mis-detected as PMT when the spec
> says PIDs not explicitly in the PAT/PMT should be completely ignored.
>

Again, we don't look at random PIDs and try to make sense of the data on
them.

I think this option makes sense by default, because it follows how the spec
says the stream should be interpreted. Look at the PAT for programs, then
search the pids listed for PMTs that match those programs.

To be clear, this issue is specific to _filtered_ mpegts streams. In this
case, my tuner hardware is trying to filter the mpegts stream from the
broadcaster to include only the program I care about. It does this by
rewriting the PAT to contain only the program I requested, but since it
just passes through the pid reference in the PAT, all the PMTs  (including
those for other programs) come through. Most of the time this works fine,
because the pmt pid only contains PMTs for a single program.

Hope that makes sense,
Aman


>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com
>
Devin Heitmueller May 18, 2018, 5:28 p.m.
> The issue is that the PMT PID advertised in the PAT contains multiple PMTs
> for different programs. This is because the broadcaster decided to re-use
> the same PID for multiple program PMTs.

Ugh, ok.  I understand the case you're talking about now.  Thanks for
clarifying.

> To be clear, this issue is specific to _filtered_ mpegts streams. In this
> case, my tuner hardware is trying to filter the mpegts stream from the
> broadcaster to include only the program I care about.

This is actually what I assumed, but I thought you were referring to
the broken behavior found in some of the LinuxTV command line tools
which allow you to save the TS but they don't include the PAT in the
list of filtered PIDs.

Now that I understand what you're trying to do, the approach you've
proposed makes sense.  I withdrawal any objections.

Devin
Aman Gupta May 18, 2018, 5:43 p.m.
On Fri, May 18, 2018 at 10:28 AM, Devin Heitmueller <
dheitmueller@kernellabs.com> wrote:

> > The issue is that the PMT PID advertised in the PAT contains multiple
> PMTs
> > for different programs. This is because the broadcaster decided to re-use
> > the same PID for multiple program PMTs.
>
> Ugh, ok.  I understand the case you're talking about now.  Thanks for
> clarifying.
>
> > To be clear, this issue is specific to _filtered_ mpegts streams. In this
> > case, my tuner hardware is trying to filter the mpegts stream from the
> > broadcaster to include only the program I care about.
>
> This is actually what I assumed, but I thought you were referring to
> the broken behavior found in some of the LinuxTV command line tools
> which allow you to save the TS but they don't include the PAT in the
> list of filtered PIDs.
>
> Now that I understand what you're trying to do, the approach you've
> proposed makes sense.  I withdrawal any objections.
>

Thanks.

I'll try to expand the commit message to make it more clear before
committing.

FYI, the way ffmpeg deals with missing PAT/PMT is by automatically creating
streams for any PIDs that appear containing PES streams.

In this case, those extra pids are not in the file, so the streams were
only getting created because of the erroneous PMTs being seen.

Aman


>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com
>
Aman Gupta May 18, 2018, 7:07 p.m.
On Fri, May 18, 2018 at 10:43 AM, Aman Gupta <aman@tmm1.net> wrote:

>
>
> On Fri, May 18, 2018 at 10:28 AM, Devin Heitmueller <
> dheitmueller@kernellabs.com> wrote:
>
>> > The issue is that the PMT PID advertised in the PAT contains multiple
>> PMTs
>> > for different programs. This is because the broadcaster decided to
>> re-use
>> > the same PID for multiple program PMTs.
>>
>> Ugh, ok.  I understand the case you're talking about now.  Thanks for
>> clarifying.
>>
>> > To be clear, this issue is specific to _filtered_ mpegts streams. In
>> this
>> > case, my tuner hardware is trying to filter the mpegts stream from the
>> > broadcaster to include only the program I care about.
>>
>> This is actually what I assumed, but I thought you were referring to
>> the broken behavior found in some of the LinuxTV command line tools
>> which allow you to save the TS but they don't include the PAT in the
>> list of filtered PIDs.
>>
>> Now that I understand what you're trying to do, the approach you've
>> proposed makes sense.  I withdrawal any objections.
>>
>
> Thanks.
>
> I'll try to expand the commit message to make it more clear before
> committing.
>

Updated commit message and applied as-is (off by default).

Aman


>
> FYI, the way ffmpeg deals with missing PAT/PMT is by automatically
> creating streams for any PIDs that appear containing PES streams.
>
> In this case, those extra pids are not in the file, so the streams were
> only getting created because of the erroneous PMTs being seen.
>
> Aman
>
>
>>
>> Devin
>>
>> --
>> Devin J. Heitmueller - Kernel Labs
>> http://www.kernellabs.com
>>
>
>

Patch hide | download patch | download mbox

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index e7c2abce57..1d2ee5bf37 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -538,6 +538,9 @@  This demuxer accepts the following options:
 Set size limit for looking up a new synchronization. Default value is
 65536.
 
+@item skip_unknown_pmt
+Skip PMTs for programs not defined in the PAT. Default value is 0.
+
 @item fix_teletext_pts
 Override teletext packet PTS and DTS values with the timestamps calculated
 from the PCR of the first program which the teletext stream is part of and is
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index fc9bb3940e..18df1afd9d 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -143,6 +143,7 @@  struct MpegTSContext {
 
     int skip_changes;
     int skip_clear;
+    int skip_unknown_pmt;
 
     int scan_all_pmts;
 
@@ -172,6 +173,8 @@  static const AVOption options[] = {
      {.i64 = 0}, 0, 0, AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY },
     {"scan_all_pmts", "scan and combine all PMTs", offsetof(MpegTSContext, scan_all_pmts), AV_OPT_TYPE_BOOL,
      {.i64 = -1}, -1, 1, AV_OPT_FLAG_DECODING_PARAM },
+    {"skip_unknown_pmt", "skip PMTs for programs not advertised in the PAT", offsetof(MpegTSContext, skip_unknown_pmt), AV_OPT_TYPE_BOOL,
+     {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
     {"skip_changes", "skip changing / adding streams / programs", offsetof(MpegTSContext, skip_changes), AV_OPT_TYPE_BOOL,
      {.i64 = 0}, 0, 1, 0 },
     {"skip_clear", "skip clearing programs", offsetof(MpegTSContext, skip_clear), AV_OPT_TYPE_BOOL,
@@ -2028,6 +2031,8 @@  static void pmt_cb(MpegTSFilter *filter, const uint8_t *section, int section_len
     if (!ts->scan_all_pmts && ts->skip_changes)
         return;
 
+    if (ts->skip_unknown_pmt && !get_program(ts, h->id))
+        return;
     if (!ts->skip_clear)
         clear_program(ts, h->id);