diff mbox series

[FFmpeg-devel,1/2] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets

Message ID 20240220163302.1943993-2-nicolas.gaullier@cji.paris
State New
Headers show
Series fix an mpegts scenario with unaligned pes | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Nicolas Gaullier Feb. 20, 2024, 4:33 p.m. UTC
Fix fetch_timestamp when the frame start is in a previous packet.

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 libavcodec/parser.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer Feb. 21, 2024, 4:32 a.m. UTC | #1
On Tue, Feb 20, 2024 at 05:33:01PM +0100, Nicolas Gaullier wrote:
> Fix fetch_timestamp when the frame start is in a previous packet.
> 
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  libavcodec/parser.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

This change looses pts

--- /tmp/old	2024-02-21 05:21:13.201646780 +0100
+++ /tmp/new	2024-02-21 05:20:52.205417887 +0100
@@ -1,174 +1,172 @@
 [mpegts  read_frame_internal stream=2, pts=310332330, dts=310332330, size=49072, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310348800, dts=310348800, size=16516, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310356000, dts=310356000, size=16876, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310366800, dts=310366800, size=51652, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310363200, dts=NOPTS, size=15588, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310370400, dts=NOPTS, size=21476, duration=0, flags=0
-[mpegts  read_frame_internal stream=2, pts=310402800, dts=NOPTS, size=276868, duration=3600, flags=1
-[mpegts  read_frame_internal stream=2, pts=310388400, dts=NOPTS, size=85884, duration=3600, flags=0
-[mpegts  read_frame_internal stream=2, pts=310381200, dts=NOPTS, size=45761, duration=3600, flags=0
+[mpegts  read_frame_internal stream=2, pts=NOPTS, dts=NOPTS, size=16516, duration=0, flags=0
+[mpegts  read_frame_internal stream=2, pts=310348800, dts=310348800, size=16876, duration=0, flags=0
+[mpegts  read_frame_internal stream=2, pts=310356000, dts=310356000, size=51652, duration=0, flags=0
+[mpegts  read_frame_internal stream=2, pts=310366800, dts=310366800, size=15588, duration=0, flags=0
+[mpegts  read_frame_internal stream=2, pts=310363200, dts=NOPTS, size=21476, duration=0, flags=0
+[mpegts  read_frame_internal stream=2, pts=310370400, dts=NOPTS, size=276868, duration=3600, flags=1
+[mpegts  read_frame_internal stream=2, pts=NOPTS, dts=NOPTS, size=85884, duration=3600, flags=0
+[mpegts  read_frame_internal stream=2, pts=310388400, dts=NOPTS, size=45761, duration=3600, flags=0


[...]
Nicolas Gaullier Feb. 23, 2024, 12:52 p.m. UTC | #2
>De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Michael Niedermayer
>Envoyé : mercredi 21 février 2024 05:32
>On Tue, Feb 20, 2024 at 05:33:01PM +0100, Nicolas Gaullier wrote:
>> Fix fetch_timestamp when the frame start is in a previous packet.
>> 
>> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
>> ---
>>  libavcodec/parser.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>
>This change looses pts

I missed it : some broken streams are missing the zero_byte which makes the current h264 parser code to borrow a terminating null byte in the previous frame if available.
It seems there is currently no issue with that behaviour, but with my patch fixing the fetch_timestamp mechanism, it becomes one.
So, what is somewhat tricky is to guess if we are facing a broken stream or a conformant stream which actually has its zero_byte in the previous frame.
In my experience (including the sample from Michael and a sample of mine where there is no available null byte at the end of the frame),
the "usual broken streams" are missing the zero_byte for the first NAL unit which is an AUD, but the following NAL still has this zero_byte.
The following patch is a proposal to detect and overcome such a situation:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10877
At the end, this patch is required to handle broken streams and thus prepare the ground for the fetch_timestamp patch.

Another option would be for example to handle the data_alignment_indicator in the mpegts demuxer to force the alignment (ex: with a parser state reset),
but it seems it would involve some big unhappy changes in the code, with demux and parser tied together. Moreover, I don't think it is reliable and there might exists
broken stream with unaligned packets that we would still like to support.

Any inputs concerning broken streams for other codecs is welcome. For example, it may be required to handle broken hevc streams alike h264 ones: I have no opinion/ samples for that matter.

Nicolas
diff mbox series

Patch

diff --git a/libavcodec/parser.c b/libavcodec/parser.c
index efc28b8918..853b5323b0 100644
--- a/libavcodec/parser.c
+++ b/libavcodec/parser.c
@@ -153,11 +153,11 @@  int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx,
     }
 
     if (s->fetch_timestamp) {
-        s->fetch_timestamp = 0;
         s->last_pts        = s->pts;
         s->last_dts        = s->dts;
         s->last_pos        = s->pos;
-        ff_fetch_timestamp(s, 0, 0, 0);
+        ff_fetch_timestamp(s, FFMIN(s->fetch_timestamp, 0), 0, 0);
+        s->fetch_timestamp = 0;
     }
     /* WARNING: the returned index can be negative */
     index = s->parser->parser_parse(s, avctx, (const uint8_t **) poutbuf,
@@ -179,7 +179,7 @@  int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx,
 
         /* offset of the next frame */
         s->next_frame_offset = s->cur_offset + index;
-        s->fetch_timestamp   = 1;
+        s->fetch_timestamp   = index >= 0 ? 1 : index;
     } else {
         /* Don't return a pointer to dummy_buf. */
         *poutbuf = NULL;