[FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Nicolas Gaullier]

This issue happens in the following case:
- unaligned PES and NAL encoding
- the first NAL of the access unit begins at the very end of a ts packet, sometimes only the 0x00 of the trailing byte
(unfortunately, this is still conformant to the standards!)
- the video frame is so small (ex: typically still picture) it fits in a ts packet and a new PES is immediately started

Two sample files can be found here:
a) https://0x0.st/HDwD.ts
b) https://0x0.st/HDwd.ts

For sample a, the first NAL (AUD) is splited this way:
0x00 / 0x00 0x00 0x01 0x09
And for sample b:
0x00 0x00 0x00 / 0x01 0x09

ffmpeg -i input.ts -f null /dev/null
=> Application provided invalid, non monotonically increasing dts...


Nicolas Gaullier (1):
  avformat/mpegts: fix first NAL start code splited in two different
    packets

 libavformat/mpegts.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

-- 
2.30.2

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

[FFmpeg-devel] [PATCH] avformat/mpegts: fix first NAL start code splited in two different packets

[Nicolas Gaullier]

When PES are not aligned and a tiny nal-encoded frame is contained in a single TS packet,
the first NAL startcode may be split by the PES boundary, making the packet unworkable.
This patch shift the PES boundaries to avoid this.

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 libavformat/mpegts.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index bef00c88e7..4b4cf82fb9 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -1149,6 +1149,7 @@ static int mpegts_push_data(MpegTSFilter *filter,
     PESContext *pes   = filter->u.pes_filter.opaque;
     MpegTSContext *ts = pes->ts;
     const uint8_t *p;
+    int pes_align_shift = 0;
     int ret, len;
 
     if (!ts->pkt)
@@ -1156,6 +1157,37 @@ static int mpegts_push_data(MpegTSFilter *filter,
 
     if (is_start) {
         if (pes->state == MPEGTS_PAYLOAD && pes->data_index > 0) {
+            if ((pes->st->codecpar->codec_id == AV_CODEC_ID_H264
+             || pes->st->codecpar->codec_id == AV_CODEC_ID_HEVC)
+                && pes->data_index < TS_PACKET_SIZE - 4 - PES_HEADER_SIZE
+                && pes->data_index >= 4
+                && buf_size >= 4 ) {
+                    /* check/avoid spliting the start code + first byte of the first NAL unit in two different packets.
+                     * this could happen with a tiny unaligned PES that fits in a single ts packet. */
+                    uint8_t *last_p_end = pes->buffer->data + pes->data_index - 4;
+                    p = buf + PES_HEADER_SIZE + buf[PES_HEADER_SIZE - 1];
+                    if (last_p_end[3] == 0x00 && AV_RB24(p) == 0x000001)
+                        pes_align_shift = 4;
+                    else if (AV_RB16(last_p_end + 2)== 0x0000 && AV_RB16(p) == 0x0001)
+                        pes_align_shift = 3;
+                    else if (AV_RB24(last_p_end + 1)== 0x000000 && *p == 0x01)
+                        pes_align_shift = 2;
+                    else if (AV_RB32(last_p_end) == 0x00000001)
+                        pes_align_shift = 1;
+                    if (pes_align_shift)
+                    {
+                        last_p_end += 4;
+                        if (pes_align_shift > 3)
+                            *last_p_end++ = 0x00;
+                        if (pes_align_shift > 2)
+                            *last_p_end++ = 0x00;
+                        if (pes_align_shift > 1)
+                            *last_p_end++ = 0x01;
+                        *last_p_end = *(p + pes_align_shift - 1);
+                        pes->data_index += pes_align_shift;
+                        buf_size -= pes_align_shift;
+                    }
+                }
             ret = new_pes_packet(pes, ts->pkt);
             if (ret < 0)
                 return ret;
@@ -1301,6 +1333,7 @@ skip:
                 /* we got the full header. We parse it and get the payload */
                 pes->state = MPEGTS_PAYLOAD;
                 pes->data_index = 0;
+                p += pes_align_shift;
                 if (pes->stream_type == 0x12 && buf_size > 0) {
                     int sl_header_bytes = read_sl_header(pes, &pes->sl, p,
                                                          buf_size);
@@ -1409,9 +1442,13 @@ skip:
                 pes->data_index += buf_size;
                 /* emit complete packets with known packet size
                  * decreases demuxer delay for infrequent packets like subtitles from
-                 * a couple of seconds to milliseconds for properly muxed files. */
+                 * a couple of seconds to milliseconds for properly muxed files.
+                 * disabled for video/NALs because at this point it could split/break the first NAL start code.
+                 */
                 if (!ts->stop_parse && pes->PES_packet_length &&
-                    pes->pes_header_size + pes->data_index == pes->PES_packet_length + PES_START_SIZE) {
+                    pes->pes_header_size + pes->data_index == pes->PES_packet_length + PES_START_SIZE &&
+                    pes->st->codecpar->codec_id != AV_CODEC_ID_H264 &&
+                    pes->st->codecpar->codec_id != AV_CODEC_ID_HEVC) {
                     ts->stop_parse = 1;
                     ret = new_pes_packet(pes, ts->pkt);
                     pes->state = MPEGTS_SKIP;
-- 
2.30.2

_______________________________________________
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 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Nicolas Gaullier]

>Envoyé : vendredi 2 février 2024 16:24
>À : ffmpeg-devel@ffmpeg.org
>Objet : [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets
>
>This issue happens in the following case:
>- unaligned PES and NAL encoding
>- the first NAL of the access unit begins at the very end of a ts packet, sometimes only the 0x00 of the trailing byte (unfortunately, this is still conformant to the standards!)
>- the video frame is so small (ex: typically still picture) it fits in a ts packet and a new PES is immediately started
>
>Two sample files can be found here:
>a) https://0x0.st/HDwD.ts
>b) https://0x0.st/HDwd.ts
>
>For sample a, the first NAL (AUD) is splited this way:
>0x00 / 0x00 0x00 0x01 0x09
>And for sample b:
>0x00 0x00 0x00 / 0x01 0x09
>
>ffmpeg -i input.ts -f null /dev/null
>=> Application provided invalid, non monotonically increasing dts...
>
>
>Nicolas Gaullier (1):
>  avformat/mpegts: fix first NAL start code splited in two different
>    packets
>
> libavformat/mpegts.c | 41 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 39 insertions(+), 2 deletions(-)

Ping ?
If you think it would be better to have a related trac ticket, just tell me, I can do this of course.
Nicolas
_______________________________________________
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 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Kieran Kunhya]

On Tue, 6 Feb 2024 at 10:58, Nicolas Gaullier <nicolas.gaullier@cji.paris>
wrote:

> >Envoyé : vendredi 2 février 2024 16:24
> >À : ffmpeg-devel@ffmpeg.org
> >Objet : [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in
> two different packets
> >
> >This issue happens in the following case:
> >- unaligned PES and NAL encoding
> >- the first NAL of the access unit begins at the very end of a ts packet,
> sometimes only the 0x00 of the trailing byte (unfortunately, this is still
> conformant to the standards!)
> >- the video frame is so small (ex: typically still picture) it fits in a
> ts packet and a new PES is immediately started
> >
> >Two sample files can be found here:
> >a) https://0x0.st/HDwD.ts
> >b) https://0x0.st/HDwd.ts
> >
> >For sample a, the first NAL (AUD) is splited this way:
> >0x00 / 0x00 0x00 0x01 0x09
> >And for sample b:
> >0x00 0x00 0x00 / 0x01 0x09
> >
> >ffmpeg -i input.ts -f null /dev/null
> >=> Application provided invalid, non monotonically increasing dts...
> >
> >
> >Nicolas Gaullier (1):
> >  avformat/mpegts: fix first NAL start code splited in two different
> >    packets
> >
> > libavformat/mpegts.c | 41 +++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 39 insertions(+), 2 deletions(-)
>
> Ping ?
> If you think it would be better to have a related trac ticket, just tell
> me, I can do this of course.
> Nicolas
>

I think it's ok. I wish it were less ugly but I guess it can't be.

Kieran
_______________________________________________
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 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Nicolas Gaullier]

>> If you think it would be better to have a related trac ticket, just 
>> tell me, I can do this of course.
>> Nicolas
>>
>
>I think it's ok. I wish it were less ugly but I guess it can't be.
>
>Kieran

Yes, of course! I am really really sorry.
Personally, I have not found a better solution as an mpegts fix, but if anyone has a better code, of course my version can be dropped.
(I have not looked for a possible fix in the upper ffmpeg demux/parser layers, but it would be certainly even more ugly, if possible at all).

Again, sorry. There are some implementers who take the standards very crude to "optimize" for every little byte unreasonably.
Fact is, here, the standard should not have allowed this!
Nicolas
_______________________________________________
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 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Kieran Kunhya]

On Tue, 6 Feb 2024 at 11:39, Nicolas Gaullier <nicolas.gaullier@cji.paris>
wrote:

> >> If you think it would be better to have a related trac ticket, just
> >> tell me, I can do this of course.
> >> Nicolas
> >>
> >
> >I think it's ok. I wish it were less ugly but I guess it can't be.
> >
> >Kieran
>
> Yes, of course! I am really really sorry.
> Personally, I have not found a better solution as an mpegts fix, but if
> anyone has a better code, of course my version can be dropped.
> (I have not looked for a possible fix in the upper ffmpeg demux/parser
> layers, but it would be certainly even more ugly, if possible at all).
>
> Again, sorry. There are some implementers who take the standards very
> crude to "optimize" for every little byte unreasonably.
> Fact is, here, the standard should not have allowed this!
> Nicolas
>

Yes, I know exactly who is creating these files.

Kieran
_______________________________________________
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 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Marton Balint]

On Tue, 6 Feb 2024, Nicolas Gaullier wrote:

>>> If you think it would be better to have a related trac ticket, just
>>> tell me, I can do this of course.
>>> Nicolas
>>>
>>
>> I think it's ok. I wish it were less ugly but I guess it can't be.
>>
>> Kieran
>
> Yes, of course! I am really really sorry.
> Personally, I have not found a better solution as an mpegts fix, but if anyone has a better code, of course my version can be dropped.
> (I have not looked for a possible fix in the upper ffmpeg demux/parser layers, but it would be certainly even more ugly, if possible at all).

I don't quite see why this can't be the job of the parser. After all, that 
is what is codec specific, and that is what usually splits unaligned 
packets... Could you please elaborate on that?

Thanks,
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets

[Nicolas Gaullier]

>> Personally, I have not found a better solution as an mpegts fix, but if anyone has a better code, of course my version can be dropped.
>> (I have not looked for a possible fix in the upper ffmpeg demux/parser layers, but it would be certainly even more ugly, if possible at all).
>
>I don't quite see why this can't be the job of the parser. After all, that is what is codec specific, and that is what usually splits unaligned packets... Could you please elaborate on that?
>
>Thanks,
>Marton

So here it is, I digged in the demux/parsers layers and come to a new patch as you may have noticed:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10843

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