On Sat, May 16, 2020 at 07:36:45PM +0200, Marton Balint wrote: > > > On Sat, 16 May 2020, lance.lmw...@gmail.com wrote: > > > On Sat, May 16, 2020 at 11:44:01AM +0200, Marton Balint wrote: > > > > > > > > > On Sat, 16 May 2020, lance.lmw...@gmail.com wrote: > > > > > > > On Fri, May 15, 2020 at 06:52:55PM +0200, Marton Balint wrote: > > > > > > > > > On Fri, 15 May 2020, lance.lmw...@gmail.com wrote: > > > > > > > > On Fri, May 15, 2020 at 08:02:44PM +0800, myp...@gmail.com > > > wrote: > > > > > > > On Fri, May 15, 2020 at 6:23 PM <lance.lmw...@gmail.com> wrote: > > > > > > > > > > > > > > > > From: Limin Wang <lance.lmw...@gmail.com> > > > > > > > > > > > > > > > > reanalyze() may misdetect the new packet size as 204, but it's > > > > > > > > 188 still actualy, > > > > > > > > it'll cause many cc errors report and cannot be recovered. So > > > > > > > > it's better to check > > > > > > > > the next sync byte before skip the extra unused bytes. > > > > > > > > > > > > > > > > Also, it is best to change SIZE_STAT_THRESHOLD from 10 to 100? > > > > > > > > If the input stream has > > > > > > > > a master/slave switch serveral times, the raw size can be > > > > > > > > easily detected by mistake. > > > > > > > > > > > > > > > > Signed-off-by: Limin Wang <lance.lmw...@gmail.com> > > > > > > > > --- > > > > > > > > libavformat/mpegts.c | 13 +++++++------ > > > > > > > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > > > > > > > > > > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > > > > > > > > index 0833d62..049555c 100644 > > > > > > > > --- a/libavformat/mpegts.c > > > > > > > > +++ b/libavformat/mpegts.c > > > > > > > > @@ -2932,11 +2932,12 @@ static int read_packet(AVFormatContext > > > > > > > > *s, uint8_t *buf, int raw_packet_size, > > > > > > > > return 0; > > > > > > > > } > > > > > > > > > > > > > > > > -static void finished_reading_packet(AVFormatContext *s, int > > > > > > > > raw_packet_size) > > > > > > > > +static void finished_reading_packet(AVFormatContext *s, const > > > > > > > > uint8_t *data, int raw_packet_size) > > > > > > > > { > > > > > > > > AVIOContext *pb = s->pb; > > > > > > > > int skip = raw_packet_size - TS_PACKET_SIZE; > > > > > > > > - if (skip > 0) > > > > > > > > + /* Check the next sync byte to avoid incorrectt detected > > > > > > > > raw packet size */ > > > > > > > > + if (skip > 0 && data[TS_PACKET_SIZE] != 0x47) > > > > > > > > avio_skip(pb, skip); > > > > > > > > } > > > > > > > > > > > > > > > > @@ -2985,7 +2986,7 @@ static int handle_packets(MpegTSContext > > > > > > > > *ts, int64_t nb_packets) > > > > > > > > if (ret != 0) > > > > > > > > break; > > > > > > > > ret = handle_packet(ts, data, avio_tell(s->pb)); > > > > > > > > - finished_reading_packet(s, ts->raw_packet_size); > > > > > > > > + finished_reading_packet(s, data, ts->raw_packet_size); > > > > > > > > if (ret != 0) > > > > > > > > break; > > > > > > > > } > > > > > > > > @@ -3137,7 +3138,7 @@ static int > > > > > > > > mpegts_read_header(AVFormatContext *s) > > > > > > > > pid = AV_RB16(data + 1) & 0x1fff; > > > > > > > > if ((pcr_pid == -1 || pcr_pid == pid) && > > > > > > > > parse_pcr(&pcr_h, &pcr_l, data) == 0) { > > > > > > > > - finished_reading_packet(s, > > > > > > > > ts->raw_packet_size); > > > > > > > > + finished_reading_packet(s, data, > > > > > > > > ts->raw_packet_size); > > > > > > > > pcr_pid = pid; > > > > > > > > packet_count[nb_pcrs] = nb_packets; > > > > > > > > pcrs[nb_pcrs] = pcr_h * 300 + pcr_l; > > > > > > > > @@ -3154,7 +3155,7 @@ static int > > > > > > > > mpegts_read_header(AVFormatContext *s) > > > > > > > > } > > > > > > > > } > > > > > > > > } else { > > > > > > > > - finished_reading_packet(s, > > > > > > > > ts->raw_packet_size); > > > > > > > > + finished_reading_packet(s, data, > > > > > > > > ts->raw_packet_size); > > > > > > > > } > > > > > > > > nb_packets++; > > > > > > > > } > > > > > > > > @@ -3194,7 +3195,7 @@ static int > > > > > > > > mpegts_raw_read_packet(AVFormatContext *s, AVPacket *pkt) > > > > > > > > } > > > > > > > > if (data != pkt->data) > > > > > > > > memcpy(pkt->data, data, ts->raw_packet_size); > > > > > > > > - finished_reading_packet(s, ts->raw_packet_size); > > > > > > > > + finished_reading_packet(s, data, ts->raw_packet_size); > > > > > > > > if (ts->mpeg2ts_compute_pcr) { > > > > > > > > /* compute exact PCR for each packet */ > > > > > > > > if (parse_pcr(&pcr_h, &pcr_l, pkt->data) == 0) { > > > > > > > > -- > > > > > > > > 1.8.3.1 > > > > > > > > > > > > > > > Do you have a case to reproduce the cc errors report? > > > > > > > Yes, it's real case which can be reproduced in lab. > > > > > > > Can you share the sample? > > > > > No, tested with master and slave udp ts stream which switch > > > between the master and slave stream > > > > more than five times, it'll cause tons of cc error report and can't be > > > > recovered. > > > > > > I suggest you capture the input, so this issue can be properly reproduced > > > and share it. It may even make sense to create a fate test from it. > > > > Have tried, the issue can't be reproduced by capture offset file. I guess > > it's > > caused by live stream seek isn't same as off line file. > > If that is the case then you can override the seekability of files by using > -seekable 0 option. Alternatively you might find some other means to replay > the recorded ts, anyhow this should be reproduced, because I have a feeling > we don't have the whole picture of what is going on. I have spend more time to investigate it, I have try to capture a pcap file to reproduce the issue, with tcprelay, it can be reproduced every time, however if export to ts, I failed to reproduce it.
> > > > > > > > > Anyway, your patch does not seem right, because it seems to revert back > > > the > > > packet size as soon as the first byte to be skipped is 0x47. That will > > > surely cause issues for normal streams having a packet size > 188. > > > > I haven't sample to test such case, I think 0x47 is sync byte, if the next > > 188 > > byte is 0x47, it's more possible it's sync byte as sync byte checking is > > always > > make sure the next 188/192/204 payload is 0x47. > > No, you can only involve any resync logic if you came across a sync byte > which is not 0x47. You cannot make assumptions about packet size by checking > bytes of packet data as long as you are in sync, because that would mean > that packet data can affect sync decisions even for perfectly synced > streams. Thanks for the advices, I spend more time to investigate and try to fix it in the resync logic, with more debug info, I notice the old reanalyze() has one drawback, the pos isn't one of 188/192/204, so it'll cause the stat will reset all stats ahead of one of stats[i] which cause normal 188 failed to detected anymore. I think it's better to return if pos isn't one of 188/192/204, it'll fix my issue by testing, please help to review whether it's reasonable or not. my debug log change: diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index 0833d62..9a13146 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -2855,6 +2855,8 @@ static void reanalyze(MpegTSContext *ts) { } ts->size_stat_count ++; + av_log(ts->stream, AV_LOG_INFO, "reanalyze: stat: %d, stat[0]: %d, stat[1]: %d, stat[2]: %d, pos: %lld\n", + ts->size_stat_count, ts->size_stat[0], ts->size_stat[1], ts->size_stat[2], pos); if (ts->size_stat_count > SIZE_STAT_THRESHOLD) { int newsize = 0; if (ts->size_stat[0] > SIZE_STAT_THRESHOLD) { The bug trigger log: [mpegts @ 0x3ef0180] reanalyze: stat: 0, stat[0]: 0, stat[1]: 1, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] PES packet size mismatch [mpegts @ 0x3ef0180] Packet corrupt (stream = 0, dts = 1320433). [mpegts @ 0x3ef0180] DTS 8590011941 < 8591255025 out of orderN/A speed=1.11x [mpegts @ 0x3ef0180] reanalyze: stat: 1, stat[0]: 0, stat[1]: 2, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 2, stat[0]: 0, stat[1]: 3, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 3, stat[0]: 0, stat[1]: 4, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] PES packet size mismatch [mpegts @ 0x3ef0180] Packet corrupt (stream = 0, dts = 1229349). [mpegts @ 0x3ef0180] DTS 8590128272 < 8591163941 out of orderN/A speed=1.07x [mpegts @ 0x3ef0180] reanalyze: stat: 4, stat[0]: 0, stat[1]: 5, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 5, stat[0]: 0, stat[1]: 6, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 6, stat[0]: 0, stat[1]: 7, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] PES packet size mismatch [mpegts @ 0x3ef0180] Packet corrupt (stream = 0, dts = 1464480). [mpegts @ 0x3ef0180] DTS 8590009421 < 8591399072 out of orderN/A speed=1.06x [mpegts @ 0x3ef0180] reanalyze: stat: 7, stat[0]: 0, stat[1]: 8, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 8, stat[0]: 0, stat[1]: 9, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] reanalyze: stat: 9, stat[0]: 0, stat[1]: 10, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] PES packet size mismatch [mpegts @ 0x3ef0180] Packet corrupt (stream = 0, dts = 1828029). [mpegts @ 0x3ef0180] DTS 8590030653 < 8591762621 out of orderN/A speed=1.02x [mpegts @ 0x3ef0180] reanalyze: stat: 10, stat[0]: 0, stat[1]: 11, stat[2]: 0, raw_packet_size: 188, pos: 192 [mpegts @ 0x3ef0180] changing packet size to 192 [mpegts @ 0x3ef0180] reanalyze: stat: 0, stat[0]: 1, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] PES packet size mismatch [mpegts @ 0x3ef0180] Packet corrupt (stream = 0, dts = 1442461). [mpegts @ 0x3ef0180] reanalyze: stat: 1, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 2, stat[0]: 3, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 3, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 4, stat[0]: 5, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 the log which explain why it failed to detect back to 188: [mpegts @ 0x3ef0180] reanalyze: stat: 1, stat[0]: 1, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 2, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 3, stat[0]: 3, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 4, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 5, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 564 [mpegts @ 0x3ef0180] reanalyze: stat: 6, stat[0]: 5, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 7, stat[0]: 6, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 8, stat[0]: 6, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 376 [mpegts @ 0x3ef0180] reanalyze: stat: 9, stat[0]: 7, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 10, stat[0]: 7, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 474 [mpegts @ 0x3ef0180] reanalyze: stat: 0, stat[0]: 0, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 752 [mpegts @ 0x3ef0180] reanalyze: stat: 1, stat[0]: 0, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 940 [mpegts @ 0x3ef0180] reanalyze: stat: 2, stat[0]: 1, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 3, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 4, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 737 [mpegts @ 0x3ef0180] reanalyze: stat: 5, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 203 [mpegts @ 0x3ef0180] reanalyze: stat: 6, stat[0]: 3, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 7, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 8, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 376 [mpegts @ 0x3ef0180] reanalyze: stat: 9, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 564 [mpegts @ 0x3ef0180] reanalyze: stat: 10, stat[0]: 5, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 0, stat[0]: 1, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 1, stat[0]: 1, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 376 [mpegts @ 0x3ef0180] reanalyze: stat: 2, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 3, stat[0]: 2, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 564 [mpegts @ 0x3ef0180] reanalyze: stat: 4, stat[0]: 3, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 5, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 6, stat[0]: 4, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 376 [mpegts @ 0x3ef0180] reanalyze: stat: 7, stat[0]: 5, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 [mpegts @ 0x3ef0180] reanalyze: stat: 8, stat[0]: 5, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 564 [mpegts @ 0x3ef0180] reanalyze: stat: 9, stat[0]: 6, stat[1]: 0, stat[2]: 0, raw_packet_size: 192, pos: 188 .... > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". -- Thanks, Limin Wang _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".