On 5/9/2018 6:35 PM, Aman Gupta wrote: > From: Aman Gupta <a...@tmm1.net> > > Fixes PMT parsing in some mpegts streams which contain > multiple tables within the PMT pid. Previously, the parser > assumed only one table was present in each packet, and discarded > the rest of the section data after attempting to parse the first > table. > > A similar issue was documented in the BeyondTV software[1], which > helped me diagnose the same bug in the ffmpeg mpegts demuxer. I also > tried DVBInspector, libdvbpsi's dvbinfo, and tstools' tsinfo to > help debug. The former two properly read PMTs with multiple tables, > whereas the last has the same bug as ffmpeg. > > I've created a minimal sample[2] which contains the combined PMT. > Here's what ffmpeg probe shows before and after this patch: > > Before: > > Input #0, mpegts, from 'combined-pmt-tids.ts': > Duration: 00:00:01.08, start: 4932.966167, bitrate: 741 kb/s > Program 1 > No Program > Stream #0:0[0xf9d]: Audio: ac3, 48000 Hz, mono, fltp, 96 kb/s > Stream #0:1[0xf9b]: Audio: mp3, 0 channels, fltp > Stream #0:2[0xf9c]: Unknown: none > > After: > > Input #0, mpegts, from 'combined-pmt-tids.ts': > Duration: 00:00:01.11, start: 4932.966167, bitrate: 718 kb/s > Program 1 > Stream #0:0[0xf9b]: Video: mpeg2video ([2][0][0][0] / 0x0002), > none(tv, top first), 29.97 fps, 29.97 tbr, 90k tbn, 90k tbc > Stream #0:1[0xf9c](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > 5.1(side), fltp, 384 kb/s > Stream #0:2[0xf9d](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > mono, fltp, 96 kb/s > > With the patch, the PMT is parsed correctly so the streams are > created in the correct order, are associated with "Program 1", > and their codecs are set correctly. > > [1] http://forums.snapstream.com/vb/showpost.php?p=343816&postcount=201 > [2] https://s3.amazonaws.com/tmm1/combined-pmt-tids.ts > > Signed-off-by: Aman Gupta <a...@tmm1.net> > --- > libavformat/mpegts.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > index 0d1dda1c36..3c9f9421cb 100644 > --- a/libavformat/mpegts.c > +++ b/libavformat/mpegts.c > @@ -391,7 +391,8 @@ static void write_section_data(MpegTSContext *ts, > MpegTSFilter *tss1, > const uint8_t *buf, int buf_size, int > is_start) > { > MpegTSSectionFilter *tss = &tss1->u.section_filter; > - int len; > + uint8_t *cur_section_buf = NULL; > + int len, offset; > > if (is_start) { > memcpy(tss->section_buf, buf, buf_size); > @@ -408,23 +409,26 @@ static void write_section_data(MpegTSContext *ts, > MpegTSFilter *tss1, > tss->section_index += len; > } > > + offset = 0; > + cur_section_buf = tss->section_buf; > + while (cur_section_buf - tss->section_buf < MAX_SECTION_SIZE && > cur_section_buf[0] != 0xff) { > /* compute section length if possible */ > - if (tss->section_h_size == -1 && tss->section_index >= 3) { > - len = (AV_RB16(tss->section_buf + 1) & 0xfff) + 3; > + if (tss->section_h_size == -1 && tss->section_index - offset >= 3) { > + len = (AV_RB16(cur_section_buf + 1) & 0xfff) + 3; > if (len > MAX_SECTION_SIZE) > return; > tss->section_h_size = len; > } > > if (tss->section_h_size != -1 && > - tss->section_index >= tss->section_h_size) { > + tss->section_index >= offset + tss->section_h_size) { > int crc_valid = 1; > tss->end_of_section_reached = 1; > > if (tss->check_crc) { > - crc_valid = !av_crc(av_crc_get_table(AV_CRC_32_IEEE), -1, > tss->section_buf, tss->section_h_size); > + crc_valid = !av_crc(av_crc_get_table(AV_CRC_32_IEEE), -1, > cur_section_buf, tss->section_h_size); > if (tss->section_h_size >= 4) > - tss->crc = AV_RB32(tss->section_buf + tss->section_h_size - > 4); > + tss->crc = AV_RB32(cur_section_buf + tss->section_h_size - > 4); > > if (crc_valid) { > ts->crc_validity[ tss1->pid ] = 100; > @@ -434,10 +438,18 @@ static void write_section_data(MpegTSContext *ts, > MpegTSFilter *tss1, > crc_valid = 2; > } > if (crc_valid) { > - tss->section_cb(tss1, tss->section_buf, tss->section_h_size); > + tss->section_cb(tss1, cur_section_buf, tss->section_h_size); > if (crc_valid != 1) > tss->last_ver = -1; > } > + > + cur_section_buf += tss->section_h_size; > + offset += tss->section_h_size; > + tss->section_h_size = -1; > + } else { > + tss->end_of_section_reached = 0; > + break; > + } > } > }
Valgrind seems to complain about this change (Conditional jump or move depends on uninitialised value). http://fate.ffmpeg.org/report.cgi?slot=x86_64-archlinux-gcc-valgrind&time=20180513001958 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
