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.



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.

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".

Reply via email to