[FFmpeg-devel] [PATCH] avformat/mpegts: correctly skip TP_extra_header in m2ts

[FFmpeg-devel] [PATCH] avformat/mpegts: correctly skip TP_extra_header in m2ts

[llyyr]

instead of just resyncing and skipping a bunch of TS packets, leading to
a loss of frames.

Before this, a stray byte with the value of 0x47 in TP_extra_header
would throw off the detection of where TS packets start.

A typical file that could cause issues would look like this:

    00000300: 238f 4780 4750 1110 0000 01e0 0000 84c0
                   ^^   ^^
The first four bytes here are TP_extra_header and the actual TS packet
starts at offset 0x304

FFmpeg would try to read a packet at 0x300 but since nothing skips the
4 byte TP_extra_header, find that the first byte is not 0x47 and
immediately go into mpegts_resync, and incorrectly detect the stray 0x47
in the TP_extra_header at 0x302 as the new sync byte.

Fix this by correctly skipping the first 4 bytes if the source packet
is 192 bytes.

Signed-off-by: llyyr <llyyr.public@gmail.com>
---
 libavformat/mpegts.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index c66a1ea6ede0..0d80ad80f1aa 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2944,6 +2944,12 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size,
     AVIOContext *pb = s->pb;
     int len;
 
+    // 192 bytes source packet that start with a 4 bytes TP_extra_header
+    // followed by 188 bytes of TS packet. The sync byte is at offset 4, so skip
+    // the first 4 bytes otherwise we'll end up syncing to the wrong packet.
+    if (raw_packet_size == TS_DVHS_PACKET_SIZE)
+        avio_skip(pb, 4);
+
     for (;;) {
         len = ffio_read_indirect(pb, buf, TS_PACKET_SIZE, data);
         if (len != TS_PACKET_SIZE)
@@ -2966,7 +2972,11 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size,
 static void finished_reading_packet(AVFormatContext *s, int raw_packet_size)
 {
     AVIOContext *pb = s->pb;
-    int skip = raw_packet_size - TS_PACKET_SIZE;
+    int skip;
+    if (raw_packet_size == TS_DVHS_PACKET_SIZE)
+        skip = raw_packet_size - TS_DVHS_PACKET_SIZE;
+    else
+        skip = raw_packet_size - TS_PACKET_SIZE;
     if (skip > 0)
         avio_skip(pb, skip);
 }

base-commit: e9197db4f7227a341b781c227a0a8f3a99033b5e
-- 
2.45.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/mpegts: correctly skip TP_extra_header in m2ts

[Hendrik Leppkes]

On Mon, May 27, 2024 at 3:47 PM llyyr <llyyr.public@gmail.com> wrote:
>
> instead of just resyncing and skipping a bunch of TS packets, leading to
> a loss of frames.
>
> Before this, a stray byte with the value of 0x47 in TP_extra_header
> would throw off the detection of where TS packets start.
>
> A typical file that could cause issues would look like this:
>
>     00000300: 238f 4780 4750 1110 0000 01e0 0000 84c0
>                    ^^   ^^
> The first four bytes here are TP_extra_header and the actual TS packet
> starts at offset 0x304
>
> FFmpeg would try to read a packet at 0x300 but since nothing skips the
> 4 byte TP_extra_header, find that the first byte is not 0x47 and
> immediately go into mpegts_resync, and incorrectly detect the stray 0x47
> in the TP_extra_header at 0x302 as the new sync byte.
>
> Fix this by correctly skipping the first 4 bytes if the source packet
> is 192 bytes.
>
> Signed-off-by: llyyr <llyyr.public@gmail.com>
> ---
>  libavformat/mpegts.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index c66a1ea6ede0..0d80ad80f1aa 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -2944,6 +2944,12 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size,
>      AVIOContext *pb = s->pb;
>      int len;
>
> +    // 192 bytes source packet that start with a 4 bytes TP_extra_header
> +    // followed by 188 bytes of TS packet. The sync byte is at offset 4, so skip
> +    // the first 4 bytes otherwise we'll end up syncing to the wrong packet.
> +    if (raw_packet_size == TS_DVHS_PACKET_SIZE)
> +        avio_skip(pb, 4);
> +

I think this doesn't work with mpegts_resync, since it always resyncs
for the packet start to be on the 0x47 marker, not 4 bytes before it.
So if sync is lost, it would never recover, if I read this right.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/mpegts: correctly skip TP_extra_header in m2ts

[Hendrik Leppkes]

On Fri, Jun 7, 2024 at 9:47 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> On Mon, May 27, 2024 at 3:47 PM llyyr <llyyr.public@gmail.com> wrote:
> >
> > instead of just resyncing and skipping a bunch of TS packets, leading to
> > a loss of frames.
> >
> > Before this, a stray byte with the value of 0x47 in TP_extra_header
> > would throw off the detection of where TS packets start.
> >
> > A typical file that could cause issues would look like this:
> >
> >     00000300: 238f 4780 4750 1110 0000 01e0 0000 84c0
> >                    ^^   ^^
> > The first four bytes here are TP_extra_header and the actual TS packet
> > starts at offset 0x304
> >
> > FFmpeg would try to read a packet at 0x300 but since nothing skips the
> > 4 byte TP_extra_header, find that the first byte is not 0x47 and
> > immediately go into mpegts_resync, and incorrectly detect the stray 0x47
> > in the TP_extra_header at 0x302 as the new sync byte.
> >
> > Fix this by correctly skipping the first 4 bytes if the source packet
> > is 192 bytes.
> >
> > Signed-off-by: llyyr <llyyr.public@gmail.com>
> > ---
> >  libavformat/mpegts.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index c66a1ea6ede0..0d80ad80f1aa 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -2944,6 +2944,12 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size,
> >      AVIOContext *pb = s->pb;
> >      int len;
> >
> > +    // 192 bytes source packet that start with a 4 bytes TP_extra_header
> > +    // followed by 188 bytes of TS packet. The sync byte is at offset 4, so skip
> > +    // the first 4 bytes otherwise we'll end up syncing to the wrong packet.
> > +    if (raw_packet_size == TS_DVHS_PACKET_SIZE)
> > +        avio_skip(pb, 4);
> > +
>
> I think this doesn't work with mpegts_resync, since it always resyncs
> for the packet start to be on the 0x47 marker, not 4 bytes before it.
> So if sync is lost, it would never recover, if I read this right.
>

Since we're dealing with a special case for 192 byte packets here
anyway, maybe a special case in mpegts_resync that just checks if raw
size = 192 && [4] = 0x47 would handle this with less potential
fallout?

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/mpegts: correctly skip TP_extra_header in m2ts

[llyyr]

On 6/7/24 1:29 PM, Hendrik Leppkes wrote:
> On Fri, Jun 7, 2024 at 9:47 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>
>> On Mon, May 27, 2024 at 3:47 PM llyyr <llyyr.public@gmail.com> wrote:
>>>
>>> instead of just resyncing and skipping a bunch of TS packets, leading to
>>> a loss of frames.
>>>
>>> Before this, a stray byte with the value of 0x47 in TP_extra_header
>>> would throw off the detection of where TS packets start.
>>>
>>> A typical file that could cause issues would look like this:
>>>
>>>      00000300: 238f 4780 4750 1110 0000 01e0 0000 84c0
>>>                     ^^   ^^
>>> The first four bytes here are TP_extra_header and the actual TS packet
>>> starts at offset 0x304
>>>
>>> FFmpeg would try to read a packet at 0x300 but since nothing skips the
>>> 4 byte TP_extra_header, find that the first byte is not 0x47 and
>>> immediately go into mpegts_resync, and incorrectly detect the stray 0x47
>>> in the TP_extra_header at 0x302 as the new sync byte.
>>>
>>> Fix this by correctly skipping the first 4 bytes if the source packet
>>> is 192 bytes.
>>>
>>> Signed-off-by: llyyr <llyyr.public@gmail.com>
>>> ---
>>>   libavformat/mpegts.c | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>>> index c66a1ea6ede0..0d80ad80f1aa 100644
>>> --- a/libavformat/mpegts.c
>>> +++ b/libavformat/mpegts.c
>>> @@ -2944,6 +2944,12 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size,
>>>       AVIOContext *pb = s->pb;
>>>       int len;
>>>
>>> +    // 192 bytes source packet that start with a 4 bytes TP_extra_header
>>> +    // followed by 188 bytes of TS packet. The sync byte is at offset 4, so skip
>>> +    // the first 4 bytes otherwise we'll end up syncing to the wrong packet.
>>> +    if (raw_packet_size == TS_DVHS_PACKET_SIZE)
>>> +        avio_skip(pb, 4);
>>> +
>>
>> I think this doesn't work with mpegts_resync, since it always resyncs
>> for the packet start to be on the 0x47 marker, not 4 bytes before it.
>> So if sync is lost, it would never recover, if I read this right.
>>
> 
> Since we're dealing with a special case for 192 byte packets here
> anyway, maybe a special case in mpegts_resync that just checks if raw
> size = 192 && [4] = 0x47 would handle this with less potential
> fallout?

I've looked at this again and I don't see a way to gracefully handle it 
in mpegts_resync. It's much cleaner to just skip the header that's not 
relevant to us so we line up with the first packet being the sync byte.

FWIW this can be reproduced with

`ffmpeg -i <file> -c copy out.h264`

Sample file: https://0x0.st/XVJi.m2ts
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".