| Message ID | 20180518000444.19218-1-ffmpeg@tmm1.net |
|---|---|
| State | Accepted |
| Commit | e24d768a76b14a1cbea0ec622c89573f7d06fdee |
| Headers | show |
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 > >
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.
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
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 >
> 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
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 >
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 >> > >
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);