| Message ID | 20240220163302.1943993-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | fix an mpegts scenario with unaligned pes | expand |
| 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 |
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 [...]
>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 --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;
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(-)