[FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
Pinging again... post rebased to current master, issue still present and fixed with the patch. Nicolas Gaullier (1): avfilter/framesync: fix forward EOF pts libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts
Note1: when the EOF pts is not accurate enough, the last frame can be dropped by vf_fps with default rounding. Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3, so this is a very commonplace scenario. For example: ./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \ -count_frames -show_entries stream=nb_read_frames Before: streams.stream.0.nb_read_frames="24" After: streams.stream.0.nb_read_frames="25" --- libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c index 8e06e0e700..0d5779f830 100644 --- a/libavfilter/framesync.c +++ b/libavfilter/framesync.c @@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext *parent, unsigned nb_in) return 0; } -static void framesync_eof(FFFrameSync *fs) +static void framesync_eof(FFFrameSync *fs, int64_t pts) { fs->eof = 1; fs->frame_ready = 0; -ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); +ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts); } -static void framesync_sync_level_update(FFFrameSync *fs) +static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts) { unsigned i, level = 0; @@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs) if (level) fs->sync_level = level; else -framesync_eof(fs); +framesync_eof(fs, eof_pts); } int ff_framesync_configure(FFFrameSync *fs) @@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs) for (i = 0; i < fs->nb_in; i++) fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE; fs->sync_level = UINT_MAX; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, AV_NOPTS_VALUE); return 0; } @@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs) if (fs->in[i].have_next && fs->in[i].pts_next < pts) pts = fs->in[i].pts_next; if (pts == INT64_MAX) { -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); break; } for (i = 0; i < fs->nb_in; i++) { @@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs) fs->frame_ready = 1; if (fs->in[i].state == STATE_EOF && fs->in[i].after == EXT_STOP) -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); } } if (fs->frame_ready) @@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame) fs->in[in].have_next = 1; } -static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts) +static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t eof_pts) { av_assert0(!fs->in[in].have_next); -pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY -? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].sync = 0; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : AV_NOPTS_VALUE); fs->in[in].frame_next = NULL; -fs->in[in].pts_next = pts; +fs->in[in].pts_next = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY +? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].have_next = 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes
>De : ffmpeg-devel De la part de martin >schitter >Envoyé : jeudi 12 septembre 2024 13:54 >On 12.09.24 13:14, Nicolas Gaullier wrote: >> The message "Recognize and Ignore" does not make it clear what issue or >> grave defect is solved here. >> I see in the code that fill items are currently recognized as dark metadata >> and ignored likewise, but I don't see any issue here. >> Maybe could you comment a little bit about your intent ? > >While developing this DNxUncompressed code I always got lots of this "Dark key >..." log messages in the debug output. This kind of output wouldn't be a >surprise, if the key belongs to some rare and utterly irrelevant data >box. >but in this particular case the key stands for empty "fill" blocks, which are >frequently used in various places in MXF files. They are an elementary >building block of this container format. > >On the muxer side of ffmpegs MXF code 'fill' is known and used in many places, >but the demuxer doesn't recognize this element and just always prints these >warnings about something "unknown". That's highly irritating and >also >inefficient, because 'fill' is used quite frequently e.g. as place holder to >align frame data on 256byte boundaries etc. > >It's really trivial to fix and I don't see, why we should debate any longer >about this obvious flaw instead of just quickly solving the issue. > >But if you want, you can rewrite the wording to "Recognize 'fill' in MXF data >and suppress output" or whatever you like... I am not against the idea of the patch: filler items should not be logged as dark metadata. I just wanted to check with you that it was indeed the only issue. So it is not a big issue, and I find the patch confusing both because of the commit msg and code: - the commit msg should not claim it "adds support for" filler items: mxf files with fillers are already supported/playable. Maybe just a single line like "avformat/mxfdec: suppress verbose log for fillers" ? - why not using a simple "if" on the av_log rather than inserting a new block of code ? Thank you, 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes
>De : ffmpeg-devel De la part de martin >schitter >Envoyé : jeudi 12 septembre 2024 02:36 > >But an elementary feature like 'fill' should be simply supported by any MXF >demuxer in a suitable manner, otherwise it's IMHO a grave defect. >If you don't see any further objections, please just merge the patch and >finally solve this issue by this trivial solution till someone actually >contributes a better alternative. The message "Recognize and Ignore" does not make it clear what issue or grave defect is solved here. I see in the code that fill items are currently recognized as dark metadata and ignored likewise, but I don't see any issue here. Maybe could you comment a little bit about your intent ? Thanks 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
Rebased with no change. Pinging again ? Nicolas Gaullier (1): avfilter/framesync: fix forward EOF pts libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts
Note1: when the EOF pts is not accurate enough, the last frame can be dropped by vf_fps with default rounding. Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3, so this is a very commonplace scenario. For example: ./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \ -count_frames -show_entries stream=nb_read_frames Before: streams.stream.0.nb_read_frames="24" After: streams.stream.0.nb_read_frames="25" --- libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c index 8e06e0e700..0d5779f830 100644 --- a/libavfilter/framesync.c +++ b/libavfilter/framesync.c @@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext *parent, unsigned nb_in) return 0; } -static void framesync_eof(FFFrameSync *fs) +static void framesync_eof(FFFrameSync *fs, int64_t pts) { fs->eof = 1; fs->frame_ready = 0; -ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); +ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts); } -static void framesync_sync_level_update(FFFrameSync *fs) +static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts) { unsigned i, level = 0; @@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs) if (level) fs->sync_level = level; else -framesync_eof(fs); +framesync_eof(fs, eof_pts); } int ff_framesync_configure(FFFrameSync *fs) @@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs) for (i = 0; i < fs->nb_in; i++) fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE; fs->sync_level = UINT_MAX; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, AV_NOPTS_VALUE); return 0; } @@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs) if (fs->in[i].have_next && fs->in[i].pts_next < pts) pts = fs->in[i].pts_next; if (pts == INT64_MAX) { -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); break; } for (i = 0; i < fs->nb_in; i++) { @@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs) fs->frame_ready = 1; if (fs->in[i].state == STATE_EOF && fs->in[i].after == EXT_STOP) -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); } } if (fs->frame_ready) @@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame) fs->in[in].have_next = 1; } -static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts) +static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t eof_pts) { av_assert0(!fs->in[in].have_next); -pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY -? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].sync = 0; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : AV_NOPTS_VALUE); fs->in[in].frame_next = NULL; -fs->in[in].pts_next = pts; +fs->in[in].pts_next = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY +? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].have_next = 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 3/3] fftools/ffmpeg: Fix honor -r output option with streamcopy
Fix "ost->st->avg_frame_rate = ost->frame_rate" in streamcopy_init() being reset to input's frame rate a few lines below. Note that in current code, there are some discrepancies amongst the muxers. For example, avienc relies on time_base, so it is not affected by this patch, whereas mxfenc and matroskaenc do use avg_frame_rate, so this patch fixes -r being honored. In the updated fate test, the input is (wrongly) probed as 50fps. With this patch, the correct value (25fps) is successfully forced with -r. Signed-off-by: Nicolas Gaullier --- fftools/ffmpeg_mux_init.c | 1 - tests/ref/fate/time_base | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c index e84fa9719f..37d626add6 100644 --- a/fftools/ffmpeg_mux_init.c +++ b/fftools/ffmpeg_mux_init.c @@ -1012,7 +1012,6 @@ static int streamcopy_init(const Muxer *mux, OutputStream *ost, AVDictionary **e else sar = par->sample_aspect_ratio; ost->st->sample_aspect_ratio = par->sample_aspect_ratio = sar; -ost->st->avg_frame_rate = ist->st->avg_frame_rate; ost->st->r_frame_rate = ist->st->r_frame_rate; break; } diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index 23875d1fb8..ae96232e60 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -b28d4ca13029fdc80a114b56467be9d7 +69ffc45e19ab070bc3e964d7b718fe53 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 2/3] tests: Remove void -time_base overrides when streamcopying to mxf
Signed-off-by: Nicolas Gaullier --- tests/fate/ffmpeg.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 9e0c68da20..3ffaaeb295 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov $(TARGET_SAMPLES)/mov/fcp_export8-236.mov FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER PCM_S16LE_DECODER) += fate-copy-trac4914 fate-copy-trac4914: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ - mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 1001/3" + mxf "-c:a pcm_s16le -af aresample -c:v copy" FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER ARESAMPLE_FILTER) += fate-copy-trac4914-avi fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ @@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, SETTS_BSF PIPE_PROTOCOL) += f fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v copy -bsf:v "setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2" -fflags +bitexact FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base -fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf +fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -fflags +bitexact -f mxf FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/3] avformat/mxfenc: Fix guess frame_rate
The time_base was a bad guess. Currently, fate-time_base test data assumed that overriding the input time_base would affect the frame_rate, but this behaviour is not documented, so just fix the fate data now that this is fixed. Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: previously, when streamcopying, the time_base was guessed from the frame_rate considering it is often constant, so guessing the frame_rate back from the time_base was often not a problem. To reproduce: ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf Signed-off-by: Nicolas Gaullier --- libavformat/mxfenc.c | 8 ++-- tests/ref/fate/time_base | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 4ac6a2d715..57be9e6ef6 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(st->codecpar->format); -// TODO: should be avg_frame_rate -AVRational tbc = st->time_base; +AVRational tbc = (AVRational){ 0, 0 }; +if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) +tbc = av_inv_q(st->avg_frame_rate); +else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) +tbc = av_inv_q(st->r_frame_rate); + // Default component depth to 8 sc->component_depth = 8; sc->h_chroma_sub_sample = 2; diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index fd6cac53fc..23875d1fb8 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -d408aba82d62a90ed7f46a1999b014f1 +b28d4ca13029fdc80a114b56467be9d7 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 0/3] framerate / streamcopy
This is the whole serie properly posted with the latest addition, the third patch related to ffmpeg. This last patch is independent but somewhat related, so maybe it is a good thing to review it to possibly apply the complete serie as a whole. (the first two patches seem ready to apply) Nicolas Gaullier (3): avformat/mxfenc: Fix guess frame_rate tests: Remove void -time_base overrides when streamcopying to mxf fftools/ffmpeg: Fix honor -r output option with streamcopy fftools/ffmpeg_mux_init.c | 1 - libavformat/mxfenc.c | 8 ++-- tests/fate/ffmpeg.mak | 4 ++-- tests/ref/fate/time_base | 2 +- 4 files changed, 9 insertions(+), 6 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 3/3] fftools/ffmpeg: Fix honor -r output option with streamcopy
Fix "ost->st->avg_frame_rate = ost->frame_rate" in streamcopy_init() being reset to input's frame rate a few lines below. Note that in current code, there are some discrepancies amongst the muxers. For example, avienc relies on time_base, so it is not affected by this patch, whereas mxfenc and matroskaenc do use avg_frame_rate, so this patch fixes -r being honored. In the update fate test, the input is wrongly probeb as 50fps. With this patch, the correct value (25fps) is forced with -r. Signed-off-by: Nicolas Gaullier --- fftools/ffmpeg_mux_init.c | 1 - tests/ref/fate/time_base | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c index e84fa9719f..37d626add6 100644 --- a/fftools/ffmpeg_mux_init.c +++ b/fftools/ffmpeg_mux_init.c @@ -1012,7 +1012,6 @@ static int streamcopy_init(const Muxer *mux, OutputStream *ost, AVDictionary **e else sar = par->sample_aspect_ratio; ost->st->sample_aspect_ratio = par->sample_aspect_ratio = sar; -ost->st->avg_frame_rate = ist->st->avg_frame_rate; ost->st->r_frame_rate = ist->st->r_frame_rate; break; } diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index 23875d1fb8..ae96232e60 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -b28d4ca13029fdc80a114b56467be9d7 +69ffc45e19ab070bc3e964d7b718fe53 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 0/2] avformat/mxfenc: Fix guess frame_rate
>De : Anton Khirnov >Envoyé : lundi 2 septembre 2024 13:26 > >Quoting Nicolas Gaullier (2024-09-02 11:52:15) >> Following yesterday posts by Anton/Tomas on version 1. >> >> v3: >> - do not fall back to time_base (that was my v2) >> - do not remove "-r 25" in the fate test with streamcopy (see below) >> >> What comes out: >> - mxfenc behaves the same as matroskaenc for example >> - avienc behaves differently: still sticked to time_base only >> >> -- >> >> Another issue on this: "-r" as an output option with streamcopy is not >> honored with mxf/matroska, but it is with avienc. This is because >> streamcopy_init() only overrides time_base, not *_frame_rate. > >Output -r should set avg_frame_rate, see line 958 in ffmpeg_mux_init.c. Yes, but this is reset a few lines below. So, I will send a third patch in the same serie to address this; but maybe it can be discussed afterwards independently... Except that issue, do the patches for mxfenc look good to you ? Thanks 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 2/2] tests: Remove void -time_base overrides when streamcopying to mxf
Signed-off-by: Nicolas Gaullier --- tests/fate/ffmpeg.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 9e0c68da20..3ffaaeb295 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov $(TARGET_SAMPLES)/mov/fcp_export8-236.mov FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER PCM_S16LE_DECODER) += fate-copy-trac4914 fate-copy-trac4914: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ - mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 1001/3" + mxf "-c:a pcm_s16le -af aresample -c:v copy" FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER ARESAMPLE_FILTER) += fate-copy-trac4914-avi fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ @@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, SETTS_BSF PIPE_PROTOCOL) += f fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v copy -bsf:v "setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2" -fflags +bitexact FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base -fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf +fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -fflags +bitexact -f mxf FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/2] avformat/mxfenc: Fix guess frame_rate
The time_base was a bad guess. Currently, fate-time_base test data assumed that overriding the input time_base would affect the frame_rate, but this behaviour is not documented, so just fix the fate data now that this is fixed. Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: previously, when streamcopying, the time_base was guessed from the frame_rate considering it is often constant, so guessing the frame_rate back from the time_base was often not a problem. To reproduce: ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf Signed-off-by: Nicolas Gaullier --- libavformat/mxfenc.c | 8 ++-- tests/ref/fate/time_base | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 4ac6a2d715..57be9e6ef6 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(st->codecpar->format); -// TODO: should be avg_frame_rate -AVRational tbc = st->time_base; +AVRational tbc = (AVRational){ 0, 0 }; +if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) +tbc = av_inv_q(st->avg_frame_rate); +else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) +tbc = av_inv_q(st->r_frame_rate); + // Default component depth to 8 sc->component_depth = 8; sc->h_chroma_sub_sample = 2; diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index fd6cac53fc..23875d1fb8 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -d408aba82d62a90ed7f46a1999b014f1 +b28d4ca13029fdc80a114b56467be9d7 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 0/2] avformat/mxfenc: Fix guess frame_rate
Following yesterday posts by Anton/Tomas on version 1. v3: - do not fall back to time_base (that was my v2) - do not remove "-r 25" in the fate test with streamcopy (see below) What comes out: - mxfenc behaves the same as matroskaenc for example - avienc behaves differently: still sticked to time_base only -- Another issue on this: "-r" as an output option with streamcopy is not honored with mxf/matroska, but it is with avienc. This is because streamcopy_init() only overrides time_base, not *_frame_rate. In my understanding: - streamcopy_init() should override avg_frame_rate/r_frame_rate - avienc should be fixed similarly to this patch for mxfenc [Note that if streamcopy_init() is fixed, fate-time_base data will be updated because currently the input frame_rate is (wrongly) probed as 50.] Nicolas Gaullier (2): avformat/mxfenc: Fix guess frame_rate tests: Remove void -time_base overrides when streamcopying to mxf libavformat/mxfenc.c | 8 ++-- tests/fate/ffmpeg.mak| 4 ++-- tests/ref/fate/time_base | 2 +- 3 files changed, 9 insertions(+), 5 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/mxfenc: Fix guess frame_rate
>De : ffmpeg-devel De la part de Tomas Härdin >Envoyé : lundi 26 août 2024 11:52 > >This is probably fine for now, but it should be said that frame rate and >EditRate are not necessarily the same. We might want an explicit EditRate >option. But we can wait for users to actually request that feature > >/Tomas It seems nobody objected, can this patch be applied ? There is also a second patch in this serie for the corresponding fate tests "cleanup"; this is just in my mind to avoid confusing command lines arguments that don't take effect. Can you review it ? Thank you 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 0/2] avformat/mxfenc: Fix guess frame_rate
v2: - keep time_base as default if frame_rate is undetermined - fix mixed declaration and code Nicolas Gaullier (2): avformat/mxfenc: Fix guess frame_rate tests: Remove void time_base overrides when streamcopying to mxf libavformat/mxfenc.c | 6 +- tests/fate/ffmpeg.mak| 4 ++-- tests/ref/fate/time_base | 2 +- 3 files changed, 8 insertions(+), 4 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/2] tests: Remove void time_base overrides when streamcopying to mxf
For fate-copy-trac4914, this is a revert of 10185e2d4c1e9839bc58a1d6a63c861677b13fd0. Signed-off-by: Nicolas Gaullier --- tests/fate/ffmpeg.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 9e0c68da20..4f15356e0e 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov $(TARGET_SAMPLES)/mov/fcp_export8-236.mov FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER PCM_S16LE_DECODER) += fate-copy-trac4914 fate-copy-trac4914: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ - mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 1001/3" + mxf "-c:a pcm_s16le -af aresample -c:v copy" FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER ARESAMPLE_FILTER) += fate-copy-trac4914-avi fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ @@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, SETTS_BSF PIPE_PROTOCOL) += f fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v copy -bsf:v "setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2" -fflags +bitexact FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base -fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf +fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -fflags +bitexact -f mxf FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/2] avformat/mxfenc: Fix guess frame_rate
The time_base was a bad guess. Currently, fate-time_base test data assumed that overriding the input time_base would affect the frame_rate, but this behaviour is not documented, so just fix the fate data now that this is fixed. Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: previously, when streamcopying, the time_base was guessed from the frame_rate considering it is often constant, so guessing the frame_rate back from the time_base was often not a problem. To reproduce: ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf Signed-off-by: Nicolas Gaullier --- libavformat/mxfenc.c | 6 +- tests/ref/fate/time_base | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 4ac6a2d715..57f4c674f3 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(st->codecpar->format); -// TODO: should be avg_frame_rate AVRational tbc = st->time_base; +if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) +tbc = av_inv_q(st->avg_frame_rate); +else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) +tbc = av_inv_q(st->r_frame_rate); + // Default component depth to 8 sc->component_depth = 8; sc->h_chroma_sub_sample = 2; diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index fd6cac53fc..23875d1fb8 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -d408aba82d62a90ed7f46a1999b014f1 +b28d4ca13029fdc80a114b56467be9d7 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] tests: Remove void time_base overrides when streamcopying to mxf
For fate-copy-trac4914, this is a revert of 10185e2d4c1e9839bc58a1d6a63c861677b13fd0. Signed-off-by: Nicolas Gaullier --- tests/fate/ffmpeg.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 9e0c68da20..4f15356e0e 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov $(TARGET_SAMPLES)/mov/fcp_export8-236.mov FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER PCM_S16LE_DECODER) += fate-copy-trac4914 fate-copy-trac4914: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ - mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 1001/3" + mxf "-c:a pcm_s16le -af aresample -c:v copy" FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER ARESAMPLE_FILTER) += fate-copy-trac4914-avi fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\ @@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, SETTS_BSF PIPE_PROTOCOL) += f fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v copy -bsf:v "setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2" -fflags +bitexact FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base -fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf +fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an -sn -c:v copy -fflags +bitexact -f mxf FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] avformat/mxfenc: Fix guess frame_rate
The input time_base was a bad guess. Currently, fate-time_base test data assumed that overriding the input time_base would affect the frame_rate, but this behaviour is not documented, so just fix the fate data now that this is fixed. Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0: previously, when streamcopying, the time_base was guessed from the frame_rate considering it is often constant, so guessing the frame_rate back from the time_base was often not a problem. To reproduce: ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf Signed-off-by: Nicolas Gaullier --- libavformat/mxfenc.c | 9 +++-- tests/ref/fate/time_base | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 4ac6a2d715..a814f15609 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2894,8 +2894,13 @@ static int mxf_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(st->codecpar->format); -// TODO: should be avg_frame_rate -AVRational tbc = st->time_base; +AVRational frame_rate = (AVRational){ 0, 1 }; +if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0) +frame_rate = st->avg_frame_rate; +else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0) +frame_rate = st->r_frame_rate; +AVRational tbc = av_inv_q(frame_rate); + // Default component depth to 8 sc->component_depth = 8; sc->h_chroma_sub_sample = 2; diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base index fd6cac53fc..23875d1fb8 100644 --- a/tests/ref/fate/time_base +++ b/tests/ref/fate/time_base @@ -1 +1 @@ -d408aba82d62a90ed7f46a1999b014f1 +b28d4ca13029fdc80a114b56467be9d7 -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
>Envoyé : vendredi 14 juin 2024 13:27 >>Envoyé : lundi 3 juin 2024 12:00 >>>Envoyé : mardi 28 mai 2024 11:11 >>> >>>This a new ping but I post the patch again to get fate cleanly completed on >>>patchwork. >>> >>https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939 > >To summarize, and for remembering, the initial commit was 4e0e9ce2dc, 7 years >ago, and inserted this: >ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE) >The target of this patch is to make it a: >ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, eof_pts) > >My use case is that I frequently chain the scale filter with the fps filter, >and since the scale filter uses framesync since e82a3997cdd6c0 (3rd of May >this year), this is becoming a "full" issue Pinging again, the patch still applies... Thank you 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
>Envoyé : lundi 3 juin 2024 12:00 >>Envoyé : mardi 28 mai 2024 11:11 >> >>This a new ping but I post the patch again to get fate cleanly completed on >>patchwork. >>BTW, the initial design of framesync/EOF was in n3.4-dev-1799-g4e0e9ce2dc, so >>one can say that time has past... >> >>Thank you in advance for the review. >>Nicolas > >Another ping. >Patch still applies >https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939 Still applies to current master. Anyone to review ? To summarize, and for remembering, the initial commit was 4e0e9ce2dc, 7 years ago, and inserted this: ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE) The target of this patch is to make it a: ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, eof_pts) My use case is that I frequently chain the scale filter with the fps filter, and since the scale filter use framesync since e82a3997cdd6c0 (3rd of May this year), this is becoming a full issue (yet I have not find any corresponding trac entry so far). Thanks 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
>Envoyé : mardi 28 mai 2024 11:11 > >This a new ping but I post the patch again to get fate cleanly completed on >patchwork. >BTW, the initial design of framesync/EOF was in n3.4-dev-1799-g4e0e9ce2dc, so >one can say that time has past... > >Thank you in advance for the review. >Nicolas Another ping. Patch still applies https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939 Thank you 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts
Note1: when the EOF pts is not accurate enough, the last frame can be dropped by vf_fps with default rounding. Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3, so this is a very commonplace scenario. For example: ./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \ -count_frames -show_entries stream=nb_read_frames Before: streams.stream.0.nb_read_frames="24" After: streams.stream.0.nb_read_frames="25" --- libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c index 535fbe9c7c..28a992ba6d 100644 --- a/libavfilter/framesync.c +++ b/libavfilter/framesync.c @@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext *parent, unsigned nb_in) return 0; } -static void framesync_eof(FFFrameSync *fs) +static void framesync_eof(FFFrameSync *fs, int64_t pts) { fs->eof = 1; fs->frame_ready = 0; -ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); +ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts); } -static void framesync_sync_level_update(FFFrameSync *fs) +static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts) { unsigned i, level = 0; @@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs) if (level) fs->sync_level = level; else -framesync_eof(fs); +framesync_eof(fs, eof_pts); } int ff_framesync_configure(FFFrameSync *fs) @@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs) for (i = 0; i < fs->nb_in; i++) fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE; fs->sync_level = UINT_MAX; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, AV_NOPTS_VALUE); return 0; } @@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs) if (fs->in[i].have_next && fs->in[i].pts_next < pts) pts = fs->in[i].pts_next; if (pts == INT64_MAX) { -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); break; } for (i = 0; i < fs->nb_in; i++) { @@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs) fs->frame_ready = 1; if (fs->in[i].state == STATE_EOF && fs->in[i].after == EXT_STOP) -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); } } if (fs->frame_ready) @@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame) fs->in[in].have_next = 1; } -static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts) +static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t eof_pts) { av_assert0(!fs->in[in].have_next); -pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY -? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].sync = 0; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : AV_NOPTS_VALUE); fs->in[in].frame_next = NULL; -fs->in[in].pts_next = pts; +fs->in[in].pts_next = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY +? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].have_next = 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts
This a new ping but I post the patch again to get fate cleanly completed on patchwork. BTW, the initial design of framesync/EOF was in n3.4-dev-1799-g4e0e9ce2dc, so one can say that time has past... Thank you in advance for the review. Nicolas Nicolas Gaullier (1): avfilter/framesync: fix forward EOF pts libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avcodec/h264_parser: fix start of packet for some broken
>Envoyé : mardi 26 mars 2024 16:10 > >This is a patch from my patch serie >https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10999 >Maybe it is easier to review it this way, independantly. >This patch shows some benefits by itself, but most importantly, it is required >for my patch serie to avoid any regression with some invalid streams. > >This patch is active in mpegts/h264 when the NAL Access Unit Delimiter is >missing the zero_byte (= a invalid stream case). >In such a case, if it happens that the last data byte from the previous frame >is a null byte, this byte is "kidnaped" to form the full NAL_AUD... >This is not good, but even worser, with my patch serie above applied, it means >that the start of the editunit is in the previous frame, which means the pts >has to be taken in it, which is not the expected behaviour in such a >missleading >scenario. > >Michael sent me a sample from the wild but it can't be shared, so here it is: >I have another sample of my own with NAL_AUD missing zero_byte similarly, but >it is missing the ending null byte, so I have patched/inserted the null byte >(I shrinked the adaptation field) to show how the code behave. > >Sample original (invalid NAL_AUDs): >https://0x0.st/Xs9Q.ts >Same sample modified to exhibit the issue (invalid NAL_AUDs + an available >null ending byte at 0x291F): >https://0x0.st/Xs9j.ts > >I use this simple command line and then compare the xml, it seems quite clear: >ffprobe xxx.ts -show_packets -show_data -print_format xml > >Nicolas Gaullier (1): > avcodec/h264_parser: fix start of packet for some broken streams > > libavcodec/h264_parser.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) Ping ? ___ 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".
Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in mpegts/ps
>Envoyé : lundi 29 avril 2024 19:35 >À : FFmpeg development discussions and patches >Objet : Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing >of durations in mpegts/ps > >>Envoyé : lundi 22 avril 2024 14:32 >>À : ffmpeg-devel@ffmpeg.org >>Objet : Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate >>probing of durations in mpegts/ps >> >>>De : Nicolas Gaullier Envoyé : mardi 2 >>>avril 2024 23:26 Objet : [PATCH v3 0/1] avformat/demux: fix accurate >>>probing of durations in mpegts/ps >>> >>>v3: rebased after ed9363052f4b8b8 applied tonight (add >>>duration_probesize AVOption) >>> >>>Note: I have no other plan for demux/probing; with these two patches, I can >>>cover my use cases, especially mpegts-concats. >>> >>>For remembering, previous cover-letters: >>> >>>v1 >>>ff_read_packet() is more lightweight, but it leads to important issues when >>>looking for accurate durations. >>>As a side effect, the code looks also simpler with regular av_read_frame() >>>calls. >>>1)Updates in the fate tests do exhibit most of the results. >>> >>>2)See also more directly the case of an audio PES containing many frames: >>>>ffprobe tests/data/lavf/lavf.ts -select_streams a -show_entries >>>>stream=duration -of flat >>>Before patch: >>> streams.stream.0.duration="0.757556" >>>After patch: >>> streams.stream.0.duration="1.018778" >>> >>>3)Here is an additional (commonplace) sample to demonstrate the benefit for >>>twofields-encoded video: >>>>https://0x0.st/HFbm.ts (say h264-50i_mp2.ts) >>> >>>>ffprobe h264-50i_mp2.ts -show_entries stream=duration -of flat >>>Before patch: >>> streams.stream.0.duration="2.06" >>> streams.stream.1.duration="1.176000" >>>After patch: >>> streams.stream.0.duration="2.08" >>> streams.stream.1.duration="1.20" >>> >>> >>>v2 >>>v1: There was an issue with teletext where resolution is set just once at >>>decoder init (teletext resolution is fixed/hard coded), so it is somewhat >>>fragile: when a demuxer context update occurs, it is lost/overriden by >>>>>>avcodec_parameters_to_context(sti->avctx, st->codecpar) in >>>read_frame_internal. >>>They could have been other scenario besides teletext, I don't know. >>>v2: So now at estimate_timings_from_pts, with one or more seeking involved >>>(seeking is detected by the mpegts demuxer and set last_vn=-1, so pmt is >>>forced/updated and results in demuxer context update), it is required to >>>>>>preserve the info in codecpar at first. >>>Thanks to Michael for reporting the issue. >>> >>> >>>Nicolas Gaullier (1): >>> avformat/demux: Fix accurate probing of durations in mpegts/ps >>> >>> libavformat/demux.c | 36 ++-- >>> tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- >>> tests/ref/fate/ts-opus-demux | 4 +- >>> 3 files changed, 100 insertions(+), 110 deletions(-) >> >>Ping for review ? >>The patch (https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11386) >>still applies on current master. >>Thanks. Another ping... Thanks ___ 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".
Re: [FFmpeg-devel] [PATCH] avfilter/framesync: fix forward EOF pts
>Envoyé : mardi 21 mai 2024 21:39 > >Note1: when the EOF pts is not accurate enough, the last frame can be dropped >by vf_fps with default rounding. > >Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3, >so this is a very commonplace scenario. > >For example: >./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \ > -count_frames -show_entries stream=nb_read_frames > >Before: >streams.stream.0.nb_read_frames="24" > >After: >streams.stream.0.nb_read_frames="25" >--- > libavfilter/framesync.c | 23 +++ > 1 file changed, 11 insertions(+), 12 deletions(-) Ping ? Here is another straight way to highlight the issue with a format filter auto-inserting a scale filter thus implying framesync: >ffprobe -v debug -f lavfi testsrc=d=1,format=yuv420p,fps -count_frames shows: Before: [auto_scale_0 @ X] [framesync @ X] Sync level 0 [Parsed_fps_2 @ X] EOF is at pts 24 [Parsed_fps_2 @ X] Dropping frame with pts 24 [Parsed_fps_2 @ X] 25 frames in, 24 frames out; 1 frames dropped, 0 frames duplicated. After: [auto_scale_0 @ X] [framesync @ X] Sync level 0 [Parsed_fps_2 @ X] EOF is at pts 25 [Parsed_fps_2 @ X] Writing frame with pts 24 to pts 24 [Parsed_fps_2 @ X] 25 frames in, 25 frames out; 0 frames dropped, 0 frames duplicated. ___ 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".
[FFmpeg-devel] [PATCH] avfilter/framesync: fix forward EOF pts
Note1: when the EOF pts is not accurate enough, the last frame can be dropped by vf_fps with default rounding. Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3, so this is a very commonplace scenario. For example: ./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \ -count_frames -show_entries stream=nb_read_frames Before: streams.stream.0.nb_read_frames="24" After: streams.stream.0.nb_read_frames="25" --- libavfilter/framesync.c | 23 +++ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c index 535fbe9c7c..28a992ba6d 100644 --- a/libavfilter/framesync.c +++ b/libavfilter/framesync.c @@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext *parent, unsigned nb_in) return 0; } -static void framesync_eof(FFFrameSync *fs) +static void framesync_eof(FFFrameSync *fs, int64_t pts) { fs->eof = 1; fs->frame_ready = 0; -ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); +ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts); } -static void framesync_sync_level_update(FFFrameSync *fs) +static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts) { unsigned i, level = 0; @@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs) if (level) fs->sync_level = level; else -framesync_eof(fs); +framesync_eof(fs, eof_pts); } int ff_framesync_configure(FFFrameSync *fs) @@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs) for (i = 0; i < fs->nb_in; i++) fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE; fs->sync_level = UINT_MAX; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, AV_NOPTS_VALUE); return 0; } @@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs) if (fs->in[i].have_next && fs->in[i].pts_next < pts) pts = fs->in[i].pts_next; if (pts == INT64_MAX) { -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); break; } for (i = 0; i < fs->nb_in; i++) { @@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs) fs->frame_ready = 1; if (fs->in[i].state == STATE_EOF && fs->in[i].after == EXT_STOP) -framesync_eof(fs); +framesync_eof(fs, AV_NOPTS_VALUE); } } if (fs->frame_ready) @@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame) fs->in[in].have_next = 1; } -static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts) +static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t eof_pts) { av_assert0(!fs->in[in].have_next); -pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY -? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].sync = 0; -framesync_sync_level_update(fs); +framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : AV_NOPTS_VALUE); fs->in[in].frame_next = NULL; -fs->in[in].pts_next = pts; +fs->in[in].pts_next = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY +? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts); fs->in[in].have_next = 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in mpegts/ps
>Envoyé : lundi 22 avril 2024 14:32 >À : ffmpeg-devel@ffmpeg.org >Objet : Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing >of durations in mpegts/ps > >>De : Nicolas Gaullier Envoyé : mardi 2 >>avril 2024 23:26 Objet : [PATCH v3 0/1] avformat/demux: fix accurate >>probing of durations in mpegts/ps >> >>v3: rebased after ed9363052f4b8b8 applied tonight (add >>duration_probesize AVOption) >> >>Note: I have no other plan for demux/probing; with these two patches, I can >>cover my use cases, especially mpegts-concats. >> >>For remembering, previous cover-letters: >> >>v1 >>ff_read_packet() is more lightweight, but it leads to important issues when >>looking for accurate durations. >>As a side effect, the code looks also simpler with regular av_read_frame() >>calls. >>1)Updates in the fate tests do exhibit most of the results. >> >>2)See also more directly the case of an audio PES containing many frames: >>>ffprobe tests/data/lavf/lavf.ts -select_streams a -show_entries >>>stream=duration -of flat >>Before patch: >> streams.stream.0.duration="0.757556" >>After patch: >> streams.stream.0.duration="1.018778" >> >>3)Here is an additional (commonplace) sample to demonstrate the benefit for >>twofields-encoded video: >>>https://0x0.st/HFbm.ts (say h264-50i_mp2.ts) >> >>>ffprobe h264-50i_mp2.ts -show_entries stream=duration -of flat >>Before patch: >> streams.stream.0.duration="2.06" >> streams.stream.1.duration="1.176000" >>After patch: >> streams.stream.0.duration="2.08" >> streams.stream.1.duration="1.20" >> >> >>v2 >>v1: There was an issue with teletext where resolution is set just once at >>decoder init (teletext resolution is fixed/hard coded), so it is somewhat >>fragile: when a demuxer context update occurs, it is lost/overriden by >>>>avcodec_parameters_to_context(sti->avctx, st->codecpar) in >>read_frame_internal. >>They could have been other scenario besides teletext, I don't know. >>v2: So now at estimate_timings_from_pts, with one or more seeking involved >>(seeking is detected by the mpegts demuxer and set last_vn=-1, so pmt is >>forced/updated and results in demuxer context update), it is required to >>>>preserve the info in codecpar at first. >>Thanks to Michael for reporting the issue. >> >> >>Nicolas Gaullier (1): >> avformat/demux: Fix accurate probing of durations in mpegts/ps >> >> libavformat/demux.c | 36 ++-- >> tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- >> tests/ref/fate/ts-opus-demux | 4 +- >> 3 files changed, 100 insertions(+), 110 deletions(-) > >Ping for review ? >The patch (https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11386) >still applies on current master. >Thanks. Ping ? ___ 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".
Re: [FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in mpegts/ps
>De : Nicolas Gaullier >Envoyé : mardi 2 avril 2024 23:26 >Objet : [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in >mpegts/ps > >v3: rebased after ed9363052f4b8b8 applied tonight (add duration_probesize >AVOption) > >Note: I have no other plan for demux/probing; with these two patches, I can >cover my use cases, especially mpegts-concats. > >For remembering, previous cover-letters: > >v1 >ff_read_packet() is more lightweight, but it leads to important issues when >looking for accurate durations. >As a side effect, the code looks also simpler with regular av_read_frame() >calls. >1)Updates in the fate tests do exhibit most of the results. > >2)See also more directly the case of an audio PES containing many frames: >>ffprobe tests/data/lavf/lavf.ts -select_streams a -show_entries >>stream=duration -of flat >Before patch: > streams.stream.0.duration="0.757556" >After patch: > streams.stream.0.duration="1.018778" > >3)Here is an additional (commonplace) sample to demonstrate the benefit for >twofields-encoded video: >>https://0x0.st/HFbm.ts (say h264-50i_mp2.ts) > >>ffprobe h264-50i_mp2.ts -show_entries stream=duration -of flat >Before patch: > streams.stream.0.duration="2.06" > streams.stream.1.duration="1.176000" >After patch: > streams.stream.0.duration="2.08" > streams.stream.1.duration="1.20" > > >v2 >v1: There was an issue with teletext where resolution is set just once at >decoder init (teletext resolution is fixed/hard coded), so it is somewhat >fragile: when a demuxer context update occurs, it is lost/overriden by >>avcodec_parameters_to_context(sti->avctx, st->codecpar) in >read_frame_internal. >They could have been other scenario besides teletext, I don't know. >v2: So now at estimate_timings_from_pts, with one or more seeking involved >(seeking is detected by the mpegts demuxer and set last_vn=-1, so pmt is >forced/updated and results in demuxer context update), it is required to >>preserve the info in codecpar at first. >Thanks to Michael for reporting the issue. > > >Nicolas Gaullier (1): > avformat/demux: Fix accurate probing of durations in mpegts/ps > > libavformat/demux.c | 36 ++-- > tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- > tests/ref/fate/ts-opus-demux | 4 +- > 3 files changed, 100 insertions(+), 110 deletions(-) Ping for review ? The patch (https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11386) still applies on current master. Thanks. 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4 1/1] avfilter/vf_colorspace: use colorspace negotiation API
Fixes a regression due to the fact that the colorspace filter does not use the new API introduced by 8c7934f73ab6c568acaa. The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter since 3bf80df3ccd32aed23f0. Example: ffprobe -f lavfi yuvtestsrc,setparams=color_primaries=bt470bg:color_trc= bt470bg:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo Before: color_range:unknown color_space:bt470bg ... After: color_range:tv color_space:bt709 ... Signed-off-by: Nicolas Gaullier --- libavfilter/vf_colorspace.c | 62 + 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index d181e81ace..7bacd7892a 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -433,8 +433,7 @@ static int create_filtergraph(AVFilterContext *ctx, if (out->color_trc != s->out_trc) s->out_txchr = NULL; if (in->colorspace != s->in_csp || in->color_range != s->in_rng) s->in_lumacoef = NULL; -if (out->colorspace != s->out_csp || -out->color_range != s->out_rng) s->out_lumacoef = NULL; +if (out->color_range != s->out_rng) s->rgb2yuv = NULL; if (!s->out_primaries || !s->in_primaries) { s->in_prm = in->color_primaries; @@ -563,26 +562,8 @@ static int create_filtergraph(AVFilterContext *ctx, redo_yuv2rgb = 1; } -if (!s->out_lumacoef) { -s->out_csp = out->colorspace; +if (!s->rgb2yuv) { s->out_rng = out->color_range; -s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); -if (!s->out_lumacoef) { -if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { -if (s->user_all == CS_UNSPECIFIED) { -av_log(ctx, AV_LOG_ERROR, - "Please specify output colorspace\n"); -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output color property %d\n", s->user_all); -} -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output colorspace %d (%s)\n", s->out_csp, - av_color_space_name(s->out_csp)); -} -return AVERROR(EINVAL); -} redo_rgb2yuv = 1; } @@ -687,6 +668,26 @@ static av_cold int init(AVFilterContext *ctx) { ColorSpaceContext *s = ctx->priv; +s->out_csp = s->user_csp == AVCOL_SPC_UNSPECIFIED ? + default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; +s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); +if (!s->out_lumacoef) { +if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { +if (s->user_all == CS_UNSPECIFIED) { +av_log(ctx, AV_LOG_ERROR, + "Please specify output colorspace\n"); +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output color property %d\n", s->user_all); +} +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output colorspace %d (%s)\n", s->out_csp, + av_color_space_name(s->out_csp)); +} +return AVERROR(EINVAL); +} + ff_colorspacedsp_init(&s->dsp); return 0; @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) return res; } +out->colorspace = s->out_csp; +out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ? + in->color_range : s->user_rng; out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ? default_prm[FFMIN(s->user_all, CS_NB)] : s->user_prm; if (s->user_trc == AVCOL_TRC_UNSPECIFIED) { @@ -746,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } else { out->color_trc = s->user_trc; } -out->colorspace = s->user_csp == AVCOL_SPC_UNSPECIFIED ? - default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; -out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ? - in->color_range : s->user_rng; if (rgb_sz != s->rgb_sz) { const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format); int uvw = in->width >> desc->log2_chroma_w; @@ -841,8 +841,18 @@ static int query_formats(AVFilterContext *ctx) }; int res; ColorSpaceContext *s = ctx->priv; +AVFilterLink *outlink = ctx->outputs[0]; AVFilterFormats *formats = ff_make_format_list(pix_fmts); +
[FFmpeg-devel] [PATCH v4 0/1] avfilter/vf_colorspace: use colorspace negotiation API
v4: - remove dynamic color_range pass-through (which requires changing outlink dynamically and is forbidden) - nits&coding style - commit msg: simplified example (+ removed example for dynamic color_range pass-through) Nicolas Gaullier (1): avfilter/vf_colorspace: use colorspace negotiation API libavfilter/vf_colorspace.c | 62 + 1 file changed, 36 insertions(+), 26 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API
>De : ffmpeg-devel De la part de Niklas Haas >Envoyé : jeudi 4 avril 2024 14:44 > >> >> @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame >> >> *in) >> >> return res; >> >> } >> >> >> >> +out->colorspace = s->out_csp; >> >> +outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ? >> >> s->user_rng : in->color_range; >> >> +out->color_range = outlink->color_range; >> > >> >Changing outlink dynamically like this seems not correct to me (what if the >> >next filter in the chain only supports one color range?). Changing the >> >range on the fly would at the minimum require reconfiguring the filter >> >graph, and >>possibly insertion of more auto-scale filters. >> This is the kind of questioning I had when working on this issue. This seems >> very annoying and overly complex for a very uncommon scenario; is it even >> possible within the filter to ask for a reconfiguration of the filter graph ? > >No, reconfiguring the filter graph is (currently) the API user's >responsibility. (See fftools/ffmpeg_filter.c for an example) > >vf_buffersrc even warns you if you try and change colorspace properties >mid-stream, and for good reason - IMO there is no general expectation that >filters should be able to handle dynamically changing colorspace properties. >(This is >a feature, not a bug) > >There *are* some filters that handle dynamically changing input properties >(e.g. scale, zscale, libplacebo), but these are the exception rather than the >norm, and it's only because they're already written in a way that can >trivially >handle dynamic conversions. > >If it's difficult to do from within vf_colorspace, there's no need to do so. >Feel free to assume that frame->colorspace == inlink->colorspace == constant. >(Ditto color_range) Thank you, this is pretty clear. I agree dynamic change of color range sounds weird and I am pleased it can be assumed constant. In my patch, it means the problematic "outlink->color_range = xxx" you pointed at can be dropped. Nice. >> >> >The logic that is used in the other YUV negotiation aware filters is to >> >just set `out->colorspace = outlink->colorspace` and `out->color_range = >> >outlink->color_range`, since the link properties are authoritative. >> This is the kind of easy logic I tried at the beginning. Some water has >> flowed under the bridge, notably b89ee26539, but I just tried at the moment >> (with current code) an easy patch with just these two lines, and it still >> does not >work as "I" expected. >> More specifically: >> When running: ./ffprobe -f lavfi -i >> yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:color >> space=bt470bg,colorspace=bt709:range=tv,scale,showinfo >> At the entry of filter_frame(), the outlink values are incorrect: >> colorspace = AVCOL_SPC_BT470BG >> And color_range = AVCOL_RANGE_UNSPECIFIED So, this is why I introduced >> the negotiation for the colorspace to early set and persist this outlink >> property. >> But for the range, I am bit confused with the documentation, it is not clear >> to me, but possibly it is expected to pass through? so it cannot be >> negotiated, so I am sticked if the outlink range cannot be changed >> dynamically... > >Note: passing through the range untouched *is* the default behavior (via >ff_default_query_formats). So I think the correct logic can be condensed into >just: > >if (out_rng != UNSPEC) { >ret = set_common_color_ranges(make_singleton(out_rng)); >if (ret < 0) >return ret; >} > >That way, if the user passes unspec, it's guaranteed that the input range == >output_range (but with no preference for any specific range); but if the user >passes a specific range, both the input and output of the filter are forced to >be >this range. Well, this is where I still not feel confident. Dynamic input range is not expected but somewhat still supported. First thing: in my understanding, the colorspace filter is aimed at converting from any input range to the desired output range (if specified), and I think my patch is ok with the current "ff_formats_ref(ff_make_formats_list_singleton(s->user_rng), &outlink->incfg.color_ranges)". I don't think they are issues against it, so I will keep it that way unless you object. Second thing: I understand the default behaviour is to pass through (I mean/guess dynamically) the range, but it is not what I experience. To test this, my patch serie includes a first patch to make setparams support timeline and here it is when running a "dynamic input range input": ffmpeg -f lavfi -i yuvtestsrc -vf "setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg:range=unknown, setparams=range=pc:enable='between(n,1,2)', setparams=range=tv:enable='between(n,2,3)', colorspace=bt709,scale,showinfo" -f null -frames 3 - 2>&1|awk "/color_/ {print \$4 \" \" \$5}" Current master (solely patched for timeline support in setparams): color_range:tv color_spa
Re: [FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API
>De : Nicolas Gaullier >Envoyé : jeudi 4 avril 2024 14:02 > >>The logic that is used in the other YUV negotiation aware filters is to just >>set `out->colorspace = outlink->colorspace` and `out->color_range = >>outlink->color_range`, since the link properties are authoritative. >This is the kind of easy logic I tried at the beginning. Some water has flowed >under the bridge, notably b89ee26539, but I just tried at the moment (with >current code) an easy patch with just these two lines, and it still does not >work >as "I" expected. >More specifically: >When running: ./ffprobe -f lavfi -i >yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo >At the entry of filter_frame(), the outlink values are incorrect: >colorspace = AVCOL_SPC_BT470BG >And color_range = AVCOL_RANGE_UNSPECIFIED So, this is why I introduced the >negotiation for the colorspace to early set and persist this outlink property. >But for the range, I am bit confused with the documentation, it is not clear >to me, but possibly it is expected to pass through? so it cannot be >negotiated, so I am sticked if the outlink range cannot be changed >dynamically... I was too hasty, sorry, let me precise what is running wrong: the "out" frame colorspace/range is fine And when running: /ffprobe -f lavfi -i yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,showinfo 2>&1|grep color_space you actually get, as expected: color_range:tv color_space:bt709 The issue is the link, and it is raised when chaining with the scale filter for example here: ./ffprobe -f lavfi -i yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo 2>&1|grep color_space color_range:unknown color_space:bt470bg So, I don't know, maybe my approach (fixing vf_colorspace) is wrong ? Anyway, the "out->color_range = outlink->color_range" logic does not seem to be the kind of things to do here. Thank you again 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API
>De : Niklas Haas >Envoyé : jeudi 4 avril 2024 12:18 >> --- a/libavfilter/vf_colorspace.c >> +++ b/libavfilter/vf_colorspace.c >> @@ -433,8 +433,7 @@ static int create_filtergraph(AVFilterContext *ctx, >> if (out->color_trc != s->out_trc) s->out_txchr = NULL; >> if (in->colorspace != s->in_csp || >> in->color_range != s->in_rng) s->in_lumacoef = NULL; >> -if (out->colorspace != s->out_csp || >> -out->color_range != s->out_rng) s->out_lumacoef = NULL; >> +if (out->color_range != s->out_rng) s->rgb2yuv = NULL; > >Can you explain this change to me? This is how I understand things: the output colorspace is a mandatory parameter of the filter, so it can be set early and negotiated, So I lifted some part of the code from the "dynamic" part (filter_frame/create_filtergraph) to upstream "init/query_formats". out_lumacoef depends on colorspace solely, so it seems there is no point to unset it and re-set it again. rgb2yuv is dynamic, dependent on the range, so this is the new trigger, the pointer that has to be updated. >> @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) >> return res; >> } >> >> +out->colorspace = s->out_csp; >> +outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ? >> s->user_rng : in->color_range; >> +out->color_range = outlink->color_range; > >Changing outlink dynamically like this seems not correct to me (what if the >next filter in the chain only supports one color range?). Changing the range >on the fly would at the minimum require reconfiguring the filter graph, and >>possibly insertion of more auto-scale filters. This is the kind of questioning I had when working on this issue. This seems very annoying and overly complex for a very uncommon scenario; is it even possible within the filter to ask for a reconfiguration of the filter graph ? >The logic that is used in the other YUV negotiation aware filters is to just >set `out->colorspace = outlink->colorspace` and `out->color_range = >outlink->color_range`, since the link properties are authoritative. This is the kind of easy logic I tried at the beginning. Some water has flowed under the bridge, notably b89ee26539, but I just tried at the moment (with current code) an easy patch with just these two lines, and it still does not work as "I" expected. More specifically: When running: ./ffprobe -f lavfi -i yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo At the entry of filter_frame(), the outlink values are incorrect: colorspace = AVCOL_SPC_BT470BG And color_range = AVCOL_RANGE_UNSPECIFIED So, this is why I introduced the negotiation for the colorspace to early set and persist this outlink property. But for the range, I am bit confused with the documentation, it is not clear to me, but possibly it is expected to pass through? so it cannot be negotiated, so I am sticked if the outlink range cannot be changed dynamically... >Nit: Why introduce a new result variable instead of re-using the one that's >already declared? >IMO this logic would look cleaner if you assigned ret before testing it, >especially since it's nested. Thanks, ok, will fix this in the next version. Thank you for your review; as you can see, I have no certainty, rather many questions... 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 0/2] avfilter/vf_colorspace: Use colorspace negotiation API
>Envoyé : mardi 2 avril 2024 15:06 > >v3: >- Fixes case where colorspace is the first filter (no inlink) >- Illustrates with proper examples in commit msg (use yuvtestsrc instead of >testsrc) > >Please note that it is a regression compared to the previous release: >both examples (see commit msg) behave the same way as 6.1 after this patch. > >Nicolas Gaullier (2): > avfilter/vf_setparams: Add timeline support > avfilter/vf_colorspace: Use colorspace negotiation API Any chance to get this regression fixed for 7.0 one way or another? For remembering: ./ffprobe -f lavfi -i yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo 2>&1|grep color_space Current code: color_range:unknown color_space:bt470bg ... Release 6.1: (expected values) [or current code with my proposed patch] color_range:tv color_space:bt709 ... Thanks 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3] avformat/demux: fix accurate probing of durations in mpegts/ps
Two issues affect accuracy of duration in estimate_timings_from_pts(): - pkt->duration typically reports the duration of a single audio frame, whereas a pes often contain several audio frames - for video, compute_frame_duration() use r_frame_rate which is not reliable; typically, it is the duration of a single field for an interlaced video using two field pictures. Packet splitting/parsing is required to get accurate durations, so this patch replaces ff_read_packet() calls by av_read_frame() calls. Note that concatdec makes use of avformat_find_stream_info() to stitch correctly the files, so it benefits from this patch (typically, overlap is avoided). e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream duration is now longer than that of the video, which results in concatdec joining on audio after the patch instead of joining on video before that. Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 36 ++-- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 100 insertions(+), 110 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index abfd5fee7d..f017bae094 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1808,12 +1808,12 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) #define DURATION_DEFAULT_MAX_RETRY 6 #define DURATION_MAX_RETRY 1 -/* only usable for MPEG-PS streams */ +/* only usable for MPEG-PS/TS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) { FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; -int num, den, read_size, ret; +int read_size, ret; int64_t duration_max_read_size = ic->duration_probesize ? ic->duration_probesize >> DURATION_MAX_RETRY : DURATION_DEFAULT_MAX_READ_SIZE; int duration_max_retry = ic->duration_probesize ? DURATION_MAX_RETRY : DURATION_DEFAULT_MAX_RETRY; int found_duration = 0; @@ -1821,9 +1821,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) int64_t filesize, offset, duration; int retry = 0; -/* flush packet queue */ -ff_flush_packet_queue(ic); - for (unsigned i = 0; i < ic->nb_streams; i++) { AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); @@ -1834,10 +1831,13 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) av_log(ic, AV_LOG_WARNING, "start time for stream %d is not set in estimate_timings_from_pts\n", i); -if (sti->parser) { -av_parser_close(sti->parser); -sti->parser = NULL; -} +/* Demuxer context updates may occur, particularly while seeking in mpegts, + * and this could loose codec parameters in avctx, + * so preserve them in codecpar. + */ +if (sti->avctx_inited && +avcodec_parameters_from_context(st->codecpar, sti->avctx)) +goto skip_duration_calc; } if (ic->skip_estimate_duration_from_pts) { @@ -1855,6 +1855,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (offset < 0) offset = 0; +ff_read_frame_flush(ic); avio_seek(ic->pb, offset, SEEK_SET); read_size = 0; for (;;) { @@ -1864,7 +1865,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) break; do { -ret = ff_read_packet(ic, pkt); +ret = av_read_frame(ic, pkt); } while (ret == AVERROR(EAGAIN)); if (ret != 0) break; @@ -1874,15 +1875,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (pkt->pts != AV_NOPTS_VALUE && (st->start_time != AV_NOPTS_VALUE || sti->first_dts != AV_NOPTS_VALUE)) { -if (pkt->duration == 0) { -compute_frame_duration(ic, &num, &den, st, sti->parser, pkt); -if (den && num) { -pkt->duration = av_rescale_rnd(1, - num * (int64_t) st->time_base.den, - den * (int64_t) st->time_base.num, - AV_ROUND_DOWN); -} -} duration = pkt->pts + pkt->duration; found_duration = 1; if (st->start_time != AV_NOPTS_VALUE) @@ -1938,15 +1930,13 @@ skip_duration_calc: fill_all_stream_timings(ic); avio_seek(ic->pb, old_offset, SEEK_SET); + +ff_read_frame_flush(ic); fo
[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in mpegts/ps
v3: rebased after ed9363052f4b8b8 applied tonight (add duration_probesize AVOption) Note: I have no other plan for demux/probing; with these two patches, I can cover my use cases, especially mpegts-concats. For remembering, previous cover-letters: v1 ff_read_packet() is more lightweight, but it leads to important issues when looking for accurate durations. As a side effect, the code looks also simpler with regular av_read_frame() calls. 1)Updates in the fate tests do exhibit most of the results. 2)See also more directly the case of an audio PES containing many frames: >ffprobe tests/data/lavf/lavf.ts -select_streams a -show_entries >stream=duration -of flat Before patch: streams.stream.0.duration="0.757556" After patch: streams.stream.0.duration="1.018778" 3)Here is an additional (commonplace) sample to demonstrate the benefit for twofields-encoded video: https://0x0.st/HFbm.ts (say h264-50i_mp2.ts) >ffprobe h264-50i_mp2.ts -show_entries stream=duration -of flat Before patch: streams.stream.0.duration="2.06" streams.stream.1.duration="1.176000" After patch: streams.stream.0.duration="2.08" streams.stream.1.duration="1.20" v2 v1: There was an issue with teletext where resolution is set just once at decoder init (teletext resolution is fixed/hard coded), so it is somewhat fragile: when a demuxer context update occurs, it is lost/overriden by avcodec_parameters_to_context(sti->avctx, st->codecpar) in read_frame_internal. They could have been other scenario besides teletext, I don't know. v2: So now at estimate_timings_from_pts, with one or more seeking involved (seeking is detected by the mpegts demuxer and set last_vn=-1, so pmt is forced/updated and results in demuxer context update), it is required to preserve the info in codecpar at first. Thanks to Michael for reporting the issue. Nicolas Gaullier (1): avformat/demux: Fix accurate probing of durations in mpegts/ps libavformat/demux.c | 36 ++-- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 100 insertions(+), 110 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API
Fixes a regression due to the fact that the colorspace filter does not use the new API introduced by 8c7934f73ab6c568acaa. The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter since 3bf80df3ccd32aed23f0. Example 1 - color_range specified: ffmpeg -f lavfi -i yuvtestsrc -vf setparams=color_primaries=bt470bg: color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale ,showinfo -f null -frames 1 - Before: color_range:unknown color_space:bt470bg ... After: color_range:tv color_space:bt709 ... Example 2 - color_range pass-through: ffmpeg -f lavfi -i yuvtestsrc -vf "setparams=color_primaries=bt470bg: color_trc=smpte170m:colorspace=bt470bg:range=unknown, setparams=range=pc:enable='between(n,1,2)', setparams=range=tv:enable='between(n,2,3)', colorspace=bt709,scale,showinfo" -f null -frames 3 - 2>&1|awk "/color_/ {print \$4 \" \" \$5}" Before: color_range:tv color_space:bt470bg color_range:tv color_space:bt470bg color_range:tv color_space:bt470bg After: color_range:unknown color_space:bt709 color_range:pc color_space:bt709 color_range:tv color_space:bt709 Signed-off-by: Nicolas Gaullier --- libavfilter/vf_colorspace.c | 63 + 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index d181e81ace..12a571172b 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -433,8 +433,7 @@ static int create_filtergraph(AVFilterContext *ctx, if (out->color_trc != s->out_trc) s->out_txchr = NULL; if (in->colorspace != s->in_csp || in->color_range != s->in_rng) s->in_lumacoef = NULL; -if (out->colorspace != s->out_csp || -out->color_range != s->out_rng) s->out_lumacoef = NULL; +if (out->color_range != s->out_rng) s->rgb2yuv = NULL; if (!s->out_primaries || !s->in_primaries) { s->in_prm = in->color_primaries; @@ -563,26 +562,8 @@ static int create_filtergraph(AVFilterContext *ctx, redo_yuv2rgb = 1; } -if (!s->out_lumacoef) { -s->out_csp = out->colorspace; +if (!s->rgb2yuv) { s->out_rng = out->color_range; -s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); -if (!s->out_lumacoef) { -if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { -if (s->user_all == CS_UNSPECIFIED) { -av_log(ctx, AV_LOG_ERROR, - "Please specify output colorspace\n"); -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output color property %d\n", s->user_all); -} -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output colorspace %d (%s)\n", s->out_csp, - av_color_space_name(s->out_csp)); -} -return AVERROR(EINVAL); -} redo_rgb2yuv = 1; } @@ -687,6 +668,26 @@ static av_cold int init(AVFilterContext *ctx) { ColorSpaceContext *s = ctx->priv; +s->out_csp = s->user_csp == AVCOL_SPC_UNSPECIFIED ? + default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; +s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); +if (!s->out_lumacoef) { +if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { +if (s->user_all == CS_UNSPECIFIED) { +av_log(ctx, AV_LOG_ERROR, + "Please specify output colorspace\n"); +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output color property %d\n", s->user_all); +} +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output colorspace %d (%s)\n", s->out_csp, + av_color_space_name(s->out_csp)); +} +return AVERROR(EINVAL); +} + ff_colorspacedsp_init(&s->dsp); return 0; @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) return res; } +out->colorspace = s->out_csp; +outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ? s->user_rng : in->color_range; +out->color_range = outlink->color_range; out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ? default_prm[FFMIN(s->user_all, CS_NB)] : s->user_prm; if (s->user_trc == AVCOL_TRC_UNSPECIFIED) { @@ -746,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } else { out->color_trc = s->user_trc; } -out->colorspace = s-
[FFmpeg-devel] [PATCH v3 1/2] avfilter/vf_setparams: Add timeline support
This is helpful at least for test purposes. Signed-off-by: Nicolas Gaullier --- libavfilter/vf_setparams.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c index c96f4d314b..1b5eb70344 100644 --- a/libavfilter/vf_setparams.c +++ b/libavfilter/vf_setparams.c @@ -198,7 +198,7 @@ const AVFilter ff_vf_setparams = { .description = NULL_IF_CONFIG_SMALL("Force field, or color property for the output video frame."), .priv_size = sizeof(SetParamsContext), .priv_class = &setparams_class, -.flags = AVFILTER_FLAG_METADATA_ONLY, +.flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | AVFILTER_FLAG_METADATA_ONLY, FILTER_INPUTS(inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_QUERY_FUNC(query_formats), -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 0/2] avfilter/vf_colorspace: Use colorspace negotiation API
v3: - Fixes case where colorspace is the first filter (no inlink) - Illustrates with proper examples in commit msg (use yuvtestsrc instead of testsrc) Please note that it is a regression compared to the previous release: both examples (see commit msg) behave the same way as 6.1 after this patch. Nicolas Gaullier (2): avfilter/vf_setparams: Add timeline support avfilter/vf_colorspace: Use colorspace negotiation API libavfilter/vf_colorspace.c | 63 + libavfilter/vf_setparams.c | 2 +- 2 files changed, 37 insertions(+), 28 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/2] avfilter/vf_setparams: Add timeline support
This is helpful at least for test purposes. --- libavfilter/vf_setparams.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c index c96f4d314b..1b5eb70344 100644 --- a/libavfilter/vf_setparams.c +++ b/libavfilter/vf_setparams.c @@ -198,7 +198,7 @@ const AVFilter ff_vf_setparams = { .description = NULL_IF_CONFIG_SMALL("Force field, or color property for the output video frame."), .priv_size = sizeof(SetParamsContext), .priv_class = &setparams_class, -.flags = AVFILTER_FLAG_METADATA_ONLY, +.flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | AVFILTER_FLAG_METADATA_ONLY, FILTER_INPUTS(inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_QUERY_FUNC(query_formats), -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 0/2] avfilter/vf_colorspace: Use colorspace negotiation API
I thought the issue was fixed with b89ee26539 but it is not. Note that I have introduced timeline support in vf_setparams in order to make testing easier. v2: fixed color_range pass-through Nicolas Gaullier (2): avfilter/vf_setparams: Add timeline support avfilter/vf_colorspace: Use colorspace negotiation API libavfilter/vf_colorspace.c | 64 + libavfilter/vf_setparams.c | 2 +- 2 files changed, 38 insertions(+), 28 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/2] avfilter/vf_colorspace: Use colorspace negotiation API
Fixes a regression due to the fact that the colorspace filter does not use the new API introduced by 8c7934f73ab6c568acaa. The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter since 3bf80df3ccd32aed23f0. Example 1 - color_range specified and chained with scale filter: ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg: color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale ,showinfo -f null -frames 1 - Before: color_range:unknown color_space:bt470bg ... After: color_range:tv color_space:bt709 ... Example 2 - color_range pass-through: ffmpeg -f lavfi -i testsrc -vf "setparams=color_primaries=bt470bg: color_trc=smpte170m:colorspace=bt470bg:range=unknown, setparams=range=pc:enable='between(n,1,2)', setparams=range=tv:enable='between(n,2,3)', colorspace=bt709,showinfo" -f null -frames 3 - 2>&1|awk "/color_/ {print \$4}" Before: color_range:tv color_range:tv color_range:tv After: color_range:unknown color_range:pc color_range:tv Signed-off-by: Nicolas Gaullier --- libavfilter/vf_colorspace.c | 64 + 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index f367ce17c6..a006fd5aac 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -432,8 +432,7 @@ static int create_filtergraph(AVFilterContext *ctx, if (out->color_trc != s->out_trc) s->out_txchr = NULL; if (in->colorspace != s->in_csp || in->color_range != s->in_rng) s->in_lumacoef = NULL; -if (out->colorspace != s->out_csp || -out->color_range != s->out_rng) s->out_lumacoef = NULL; +if (out->color_range != s->out_rng) s->rgb2yuv = NULL; if (!s->out_primaries || !s->in_primaries) { s->in_prm = in->color_primaries; @@ -562,26 +561,8 @@ static int create_filtergraph(AVFilterContext *ctx, redo_yuv2rgb = 1; } -if (!s->out_lumacoef) { -s->out_csp = out->colorspace; +if (!s->rgb2yuv) { s->out_rng = out->color_range; -s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); -if (!s->out_lumacoef) { -if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { -if (s->user_all == CS_UNSPECIFIED) { -av_log(ctx, AV_LOG_ERROR, - "Please specify output colorspace\n"); -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output color property %d\n", s->user_all); -} -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output colorspace %d (%s)\n", s->out_csp, - av_color_space_name(s->out_csp)); -} -return AVERROR(EINVAL); -} redo_rgb2yuv = 1; } @@ -686,6 +667,26 @@ static av_cold int init(AVFilterContext *ctx) { ColorSpaceContext *s = ctx->priv; +s->out_csp = s->user_csp == AVCOL_SPC_UNSPECIFIED ? + default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; +s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); +if (!s->out_lumacoef) { +if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { +if (s->user_all == CS_UNSPECIFIED) { +av_log(ctx, AV_LOG_ERROR, + "Please specify output colorspace\n"); +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output color property %d\n", s->user_all); +} +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output colorspace %d (%s)\n", s->out_csp, + av_color_space_name(s->out_csp)); +} +return AVERROR(EINVAL); +} + ff_colorspacedsp_init(&s->dsp); return 0; @@ -734,6 +735,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) return res; } +out->colorspace = outlink->colorspace; +if (s->user_rng == AVCOL_RANGE_UNSPECIFIED) +outlink->color_range = link->src->inputs[0]->color_range; +out->color_range = outlink->color_range; out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ? default_prm[FFMIN(s->user_all, CS_NB)] : s->user_prm; if (s->user_trc == AVCOL_TRC_UNSPECIFIED) { @@ -745,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } else { out->color_trc = s->user_trc; } -out->colorspace = s->user_csp == AVCOL_SPC_UNSPECIFIED ? - default_csp[FFMIN(
[FFmpeg-devel] [PATCH v6] avformat/demux: Add duration_probesize AVOption
Yet another probesize used to get the durations when estimate_timings_from_pts is required. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step). Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/APIchanges | 3 +++ doc/formats.texi| 19 ++- libavformat/avformat.h | 16 ++-- libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index aa102b4925..e7677658b1 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07 API changes, most recent first: +2024-03-29 - xx - lavf 61.3.100 - avformat.h + Add AVFormatContext.duration_probesize. + 2024-03-27 - xx - lavu 59.10.100 - frame.h Add AVSideDataDescriptor, enum AVSideDataProps, and av_frame_side_data_desc(). diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..876a9e92b3 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -225,9 +225,26 @@ Specifies the maximum number of streams. This can be used to reject files that would require too many resources due to a large number of streams. @item skip_estimate_duration_from_pts @var{bool} (@emph{input}) -Skip estimation of input duration when calculated using PTS. +Skip estimation of input duration if it requires an additional probing for PTS at end of file. At present, applicable for MPEG-PS and MPEG-TS. +@item duration_probesize @var{integer} (@emph{input}) +Set probing size, in bytes, for input duration estimation when it actually requires +an additional probing for PTS at end of file (at present: MPEG-PS and MPEG-TS). +It is aimed at users interested in better durations probing for itself, or indirectly +because using the concat demuxer, for example. +The typical use case is an MPEG-TS CBR with a high bitrate, high video buffering and +ending cleaning with similar PTS for video and audio: in such a scenario, the large +physical gap between the last video packet and the last audio packet makes it necessary +to read many bytes in order to get the video stream duration. +Another use case is where the default probing behaviour only reaches a single video frame which is +not the last one of the stream due to frame reordering, so the duration is not accurate. +Setting this option has a performance impact even for small files because the probing +size is fixed. +Default behaviour is a general purpose trade-off, largely adaptive, but the probing size +will not be extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. + @item strict, f_strict @var{integer} (@emph{input/output}) Specify how strictly to follow the standards. @code{f_strict} is deprecated and should be used only via the @command{ffmpeg} tool. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index de40397676..8afdcd9fd0 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1439,7 +1439,7 @@ typedef struct AVFormatContext { * * @note this is \e not used for determining the \ref AVInputFormat * "input format" - * @sa format_probesize + * @see format_probesize */ int64_t probesize; @@ -1667,6 +1667,8 @@ typedef struct AVFormatContext { * Skip duration calcuation in estimate_timings_from_pts. * - encoding: unused * - decoding: set by user + * + * @see duration_probesize */ int skip_estimate_duration_from_pts; @@ -1729,7 +1731,7 @@ typedef struct AVFormatContext { * * Demuxing only, set by the caller before avformat_open_input(). * - * @sa probesize + * @see probesize */ int format_probesize; @@ -1870,6 +1872,16 @@ typedef struct AVFormatContext { * @return 0 on success, a negative AVERROR code on failure */ int (*io_c
[FFmpeg-devel] [PATCH v5 1/1] avformat/demux: Add duration_probesize AVOption
Yet another probesize used to get the durations when estimate_timings_from_pts is required. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step). Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/APIchanges | 3 +++ doc/formats.texi| 19 ++- libavformat/avformat.h | 16 ++-- libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index aa102b4925..f709db536d 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07 API changes, most recent first: +2024-03-28 - xx - lavf 61.3.100 - avformat.h + Add AVFormatContext.duration_probesize. + 2024-03-27 - xx - lavu 59.10.100 - frame.h Add AVSideDataDescriptor, enum AVSideDataProps, and av_frame_side_data_desc(). diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..3fe7fa9d8d 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -225,9 +225,26 @@ Specifies the maximum number of streams. This can be used to reject files that would require too many resources due to a large number of streams. @item skip_estimate_duration_from_pts @var{bool} (@emph{input}) -Skip estimation of input duration when calculated using PTS. +Skip estimation of input duration if it requires an additional probing for PTS at end of file. At present, applicable for MPEG-PS and MPEG-TS. +@item duration_probesize @var{integer} (@emph{input}) +Set probing size, in bytes, for input duration estimation when it actually requires +an additional probing for PTS at end of file (at present: MPEG-PS and MPEG-TS). +It is aimed at users interested in better durations probing for itself, or indirectly +because using the concat demuxer, for example. +The typical use case is an MPEG-TS CBR with a high bitrate, high video buffering and +ending cleaning with similar PTS for video and audio: in such a scenario, the large +physical gap between the last video packet and the last audio packet makes it necessary +to read many bytes in order to get the video stream duration. +Another use case is where the default probing behaviour only reaches a single video frame which is +not the last one of the stream due to frame reordering, so the duration is not accurate. +Setting the duration_probesize has a performance impact even for small files because the probing +size is fixed. +Default behaviour is a general purpose trade-off, largely adaptive: the probing size may range from +25 up to 16M, but it is not extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. + @item strict, f_strict @var{integer} (@emph{input/output}) Specify how strictly to follow the standards. @code{f_strict} is deprecated and should be used only via the @command{ffmpeg} tool. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index de40397676..8afdcd9fd0 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1439,7 +1439,7 @@ typedef struct AVFormatContext { * * @note this is \e not used for determining the \ref AVInputFormat * "input format" - * @sa format_probesize + * @see format_probesize */ int64_t probesize; @@ -1667,6 +1667,8 @@ typedef struct AVFormatContext { * Skip duration calcuation in estimate_timings_from_pts. * - encoding: unused * - decoding: set by user + * + * @see duration_probesize */ int skip_estimate_duration_from_pts; @@ -1729,7 +1731,7 @@ typedef struct AVFormatContext { * * Demuxing only, set by the caller before avformat_open_input(). * - * @sa probesize + * @see probesize */ int format_probesize; @@ -1870,6 +1872,16 @@ typedef struct AVFormatContext { * @return 0 on success, a
[FFmpeg-devel] [PATCH v5 0/1] avformat/demux: Add duration_probesize AVOption
V5 including all Stefano's feedback. Notes: - I have largely reworked the texi, I hope the use case is more clear. - I have rewrote "250K up to 16M" to "25 up to 16M" to be clear about the K/M units. - @seealso seems not supported in my doxy environment, so I simply moved all the @sa to @see >> +int64_t duration_max_read_size = ic->duration_probesize ? >> + ic->duration_probesize >> DURATION_MAX_RETRY_USER : >> + DURATION_MAX_READ_SIZE_DEFAULT; > >nit: I find the right shift followed by the leftshift a bit confusing, but >probably there is no simple way to prevent it Apart code simplification, there is a least one reason for that: an odd number would not be satisfactory for the algorithm. Each seek/retry has to join with no overlap. Here, if the user set duration_probesize=17, the first seek will be at filesize-8 / 8 bytes and the next-unique retry at filesize-16 / 8 bytes. Thank you for your time, Nicolas Nicolas Gaullier (1): avformat/demux: Add duration_probesize AVOption doc/APIchanges | 3 +++ doc/formats.texi| 19 ++- libavformat/avformat.h | 16 ++-- libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 45 insertions(+), 9 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_colorspace: use colorspace negotiation API
>>Example: >>ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg: >>color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale >>,showinfo -f null -frames 1 - >> >>Before: >> color_range:unknown color_space:unknown color_primaries:bt709 ... >>After: >> color_range:tv color_space:bt709 color_primaries:bt709 ... >> >>There is still an issue with the color_range when it is not specified: >>the documentation states the output defaults to the same value as the input, >>but it does not seem possible to implement currently. > >Ping ? I think it should be fixed one way or another, and backported to 7.0... Forget about it, It has been fixed late yesterday with b89ee26539 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_colorspace: use colorspace negotiation API
>De : Nicolas Gaullier >Envoyé : lundi 25 mars 2024 18:59 > >Fixes a regression due to the fact that the colorspace filter does not use the >new API introduced by 8c7934f73ab6c568acaa. >The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter >since 3bf80df3ccd32aed23f0. > >Example: >ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg: >color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale >,showinfo -f null -frames 1 - > >Before: > color_range:unknown color_space:unknown color_primaries:bt709 ... >After: > color_range:tv color_space:bt709 color_primaries:bt709 ... > >There is still an issue with the color_range when it is not specified: >the documentation states the output defaults to the same value as the input, >but it does not seem possible to implement currently. Ping ? I think it should be fixed one way or another, and backported to 7.0... Thanks 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits
>De : Nicolas Gaullier >Envoyé : lundi 11 mars 2024 15:01 > >Mantissas are the last data in the channel subsegment and it appears it is >sometimes missing a very few bits for the parsing to complete. >This must not be confused with data corruption. >In standard conditions with certified products, it has been observed that the >occurence of this issue is pretty steady and about once every 2 hours. The >truncation is at about 950 out of the 1024 values (923 is the minimum I have >seen so far). >The current code raises a severe 'Read past end' error and all data is lost >resulting in 20ms(@25fps) of silence for the affected channel. >This patch introduces a tolerance: if 800 out of the 1024 mantissas have been >parsed, a simple warning is raised and the data is preserved. Ping ? still applies last posted here: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11090 Samples (source/before patch/after patch) here: https://0x0.st/oOvv.zip 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avcodec/h264_parser: fix start of packet for some broken streams
--- libavcodec/h264_parser.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 94cfbc481e..6b721ec253 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -124,7 +124,16 @@ static int h264_find_frame_end(H264ParseContext *p, const uint8_t *buf, if (nalu_type == H264_NAL_SEI || nalu_type == H264_NAL_SPS || nalu_type == H264_NAL_PPS || nalu_type == H264_NAL_AUD) { if (pc->frame_start_found) { -i++; +/* Some streams in the wild are missing the zero_byte at the NAL_AUD: + * it is following just afterwards. + * To avoid any accidental borrowing of a byte in the previous frame + * (which would return a negative index and indicate that fetch_timestamps + * has to get the pts from the previous frame), + * better have the start of packet strictly aligned. + * To make it a more general rule, just test the following three bytes are null. + */ +i += 1 + (!p->is_avc && state == 5 && i == 3 && nalu_type == H264_NAL_AUD && +buf_size >= 9 && !AV_RB24(buf + 5)); goto found; } } else if (nalu_type == H264_NAL_SLICE || nalu_type == H264_NAL_DPA || -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] avcodec/h264_parser: fix start of packet for some broken
This is a patch from my patch serie https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10999 Maybe it is easier to review it this way, independantly. This patch shows some benefits by itself, but most importantly, it is required for my patch serie to avoid any regression with some invalid streams. This patch is active in mpegts/h264 when the NAL Access Unit Delimiter is missing the zero_byte (= a invalid stream case). In such a case, if it happens that the last data byte from the previous frame is a null byte, this byte is "kidnaped" to form the full NAL_AUD... This is not good, but even worser, with my patch serie above applied, it means that the start of the editunit is in the previous frame, which means the pts has to be taken in it, which is not the expected behaviour in such a missleading scenario. Michael sent me a sample from the wild but it can't be shared, so here it is: I have another sample of my own with NAL_AUD missing zero_byte similarly, but it is missing the ending null byte, so I have patched/inserted the null byte (I shrinked the adaptation field) to show how the code behave. Sample original (invalid NAL_AUDs): https://0x0.st/Xs9Q.ts Same sample modified to exhibit the issue (invalid NAL_AUDs + an available null ending byte at 0x291F): https://0x0.st/Xs9j.ts I use this simple command line and then compare the xml, it seems quite clear: ffprobe xxx.ts -show_packets -show_data -print_format xml Nicolas Gaullier (1): avcodec/h264_parser: fix start of packet for some broken streams libavcodec/h264_parser.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4 1/1] avformat/mpegts: Add duration_probesize AVOption
Yet another probesize option aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step). Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/demuxers.texi | 13 + doc/formats.texi | 2 +- libavformat/demux.c | 23 ++--- libavformat/mpegts.c | 104 +--- libavformat/mpegts.h | 108 +- libavformat/version.h | 2 +- 6 files changed, 140 insertions(+), 112 deletions(-) diff --git a/doc/demuxers.texi b/doc/demuxers.texi index b70f3a38d7..f88ae18a6e 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -992,6 +992,19 @@ streams move to different PIDs. Default value is 0. @item max_packet_size Set maximum size, in bytes, of packet emitted by the demuxer. Payloads above this size are split across multiple packets. Range is 1 to INT_MAX/2. Default is 204800 bytes. + +@item duration_probesize +Set probing size, in bytes, for input duration estimation which requires a specific probing +for PTS at end of file. +It is aimed at users interested in better durations probing for itself, or indirectly +for specific use cases like using the concat demuxer. +Files with high bitrates and ending cleanly (as opposed to live captures), can lead +to a large physical gap between the last video packet and the last audio packet, +so many bytes have to be read in order to get a video stream duration. +Setting this value has a performance impact even for small files because the probing size is fixed. +Default behaviour is a trade-off, largely adaptive: the probing size may range from +25 up to 16M, but it is not extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. @end table @section mpjpeg diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..e16cd88b2c 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -225,7 +225,7 @@ Specifies the maximum number of streams. This can be used to reject files that would require too many resources due to a large number of streams. @item skip_estimate_duration_from_pts @var{bool} (@emph{input}) -Skip estimation of input duration when calculated using PTS. +Skip estimation of input duration if it requires an additional probing for pts at end of file. At present, applicable for MPEG-PS and MPEG-TS. @item strict, f_strict @var{integer} (@emph{input/output}) diff --git a/libavformat/demux.c b/libavformat/demux.c index 147f3b93ac..bc23786410 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -47,6 +47,7 @@ #include "id3v2.h" #include "internal.h" #include "url.h" +#include "mpegts.h" static int64_t wrap_timestamp(const AVStream *st, int64_t timestamp) { @@ -1803,20 +1804,30 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) "Estimating duration from bitrate, this may be inaccurate\n"); } -#define DURATION_MAX_READ_SIZE 25LL -#define DURATION_MAX_RETRY 6 +#define DURATION_DEFAULT_MAX_READ_SIZE 25LL +#define DURATION_DEFAULT_MAX_RETRY 6 -/* only usable for MPEG-PS streams */ +/* only usable for MPEG-PS/TS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) { FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; int num, den, read_size, ret; +int64_t duration_max_read_size = DURATION_DEFAULT_MAX_READ_SIZE; +int duration_max_retry = DURATION_DEFAULT_MAX_RETRY; int found_duration = 0; int is_end; int64_t filesize, offset, duration; int retry = 0; +if (!strcmp(ic->iformat->name, "mpegts")) { +MpegTSContext *ts = ic->priv_data; +if (ts->duration_probesize) { +duration_max_retry = 1; +duration_max_read_size = ts->duration_probesize >> duration_max_retry; +} +} + /* flush packet queue */ ff_flush_packet_queue(ic); @@ -1847,7 +1858,7
[FFmpeg-devel] [PATCH v4 0/1] avformat/mpegts: Add duration_probesize AVOption
Thanks to Stefano for the precise inspection, I addressed all the points. The question about what is specific to mpeg remains, so I will try to elaborate on this. I don't see how duration_probesize could be needed in any way beyond estimate_timings_from_pts(). And it seems there is no other headerless format like mpeg's, byte seekable but especially dts-muxed, cbr-stuffinged with, potentially, a high I/O physical audio/video delay - or not. It seems the probing of mpeg streams is very specific currently and I don't think this situation will change in the future. Take mp4 kind formats: strictly index-based, and even for a truncated file (with moov at the head), no I/O is required to get the actual truncated stream durations. Take mxf kind formats that can be streamed, or simply truncated/broken: the editunits are quite consistent, carrying both audio&video, so recovering the duration would not be very tricky. This v4 is "an experimental try" to lift the AVOption in the demuxer (which mean no API change). The problem is that it makes the AVOption belong to mpegts.c (I will focus on mpegts, I think it is the only real use case) but unused by it, so I don't feel it is acceptable? Any input welcome. Nicolas Gaullier (1): avformat/mpegts: Add duration_probesize AVOption doc/demuxers.texi | 13 + doc/formats.texi | 2 +- libavformat/demux.c | 23 ++--- libavformat/mpegts.c | 104 +--- libavformat/mpegts.h | 108 +- libavformat/version.h | 2 +- 6 files changed, 140 insertions(+), 112 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/vf_colorspace: use colorspace negotiation API
Fixes a regression due to the fact that the colorspace filter does not use the new API introduced by 8c7934f73ab6c568acaa. The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter since 3bf80df3ccd32aed23f0. Example: ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg: color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale ,showinfo -f null -frames 1 - Before: color_range:unknown color_space:unknown color_primaries:bt709 ... After: color_range:tv color_space:bt709 color_primaries:bt709 ... There is still an issue with the color_range when it is not specified: the documentation states the output defaults to the same value as the input, but it does not seem possible to implement currently. Signed-off-by: Nicolas Gaullier --- libavfilter/vf_colorspace.c | 62 + 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index f367ce17c6..b1e1b9b719 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -432,8 +432,7 @@ static int create_filtergraph(AVFilterContext *ctx, if (out->color_trc != s->out_trc) s->out_txchr = NULL; if (in->colorspace != s->in_csp || in->color_range != s->in_rng) s->in_lumacoef = NULL; -if (out->colorspace != s->out_csp || -out->color_range != s->out_rng) s->out_lumacoef = NULL; +if (out->color_range != s->out_rng) s->rgb2yuv = NULL; if (!s->out_primaries || !s->in_primaries) { s->in_prm = in->color_primaries; @@ -562,26 +561,8 @@ static int create_filtergraph(AVFilterContext *ctx, redo_yuv2rgb = 1; } -if (!s->out_lumacoef) { -s->out_csp = out->colorspace; +if (!s->rgb2yuv) { s->out_rng = out->color_range; -s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); -if (!s->out_lumacoef) { -if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { -if (s->user_all == CS_UNSPECIFIED) { -av_log(ctx, AV_LOG_ERROR, - "Please specify output colorspace\n"); -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output color property %d\n", s->user_all); -} -} else { -av_log(ctx, AV_LOG_ERROR, - "Unsupported output colorspace %d (%s)\n", s->out_csp, - av_color_space_name(s->out_csp)); -} -return AVERROR(EINVAL); -} redo_rgb2yuv = 1; } @@ -686,6 +667,26 @@ static av_cold int init(AVFilterContext *ctx) { ColorSpaceContext *s = ctx->priv; +s->out_csp = s->user_csp == AVCOL_SPC_UNSPECIFIED ? + default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; +s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp); +if (!s->out_lumacoef) { +if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { +if (s->user_all == CS_UNSPECIFIED) { +av_log(ctx, AV_LOG_ERROR, + "Please specify output colorspace\n"); +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output color property %d\n", s->user_all); +} +} else { +av_log(ctx, AV_LOG_ERROR, + "Unsupported output colorspace %d (%s)\n", s->out_csp, + av_color_space_name(s->out_csp)); +} +return AVERROR(EINVAL); +} + ff_colorspacedsp_init(&s->dsp); return 0; @@ -745,10 +746,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } else { out->color_trc = s->user_trc; } -out->colorspace = s->user_csp == AVCOL_SPC_UNSPECIFIED ? - default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp; -out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ? - in->color_range : s->user_rng; +// FIXME color_range must be set in query_formats for negotiation. +// The value updated here will not be propagated further in the graph. +if (s->user_rng == AVCOL_RANGE_UNSPECIFIED) +out->color_range = outlink->color_range = in->color_range; if (rgb_sz != s->rgb_sz) { const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format); int uvw = in->width >> desc->log2_chroma_w; @@ -838,10 +839,19 @@ static int query_formats(AVFilterContext *ctx) AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_NONE }; -int res; +int res,
[FFmpeg-devel] [PATCH v2 1/1] avformat/demux: Fix accurate probing of durations in mpegts/ps
Two issues affect accuracy of duration in estimate_timings_from_pts(): - pkt->duration typically reports the duration of a single audio frame, whereas a pes often contain several audio frames - for video, compute_frame_duration() use r_frame_rate which is not reliable; typically, it is the duration of a single field for an interlaced video using two field pictures. Packet splitting/parsing is required to get accurate durations, so this patch replaces ff_read_packet() calls by av_read_frame() calls. Note that concatdec makes use of avformat_find_stream_info() to stitch correctly the files, so it benefits from this patch (typically, overlap is avoided). e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream duration is now longer than that of the video, which results in concatdec joining on audio after the patch instead of joining on video before that. Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 36 ++-- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 100 insertions(+), 110 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 4c50eb5568..5b89eb71c9 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1820,20 +1820,17 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) #define DURATION_MAX_READ_SIZE 25LL #define DURATION_MAX_RETRY 6 -/* only usable for MPEG-PS streams */ +/* only usable for MPEG-PS/TS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) { FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; -int num, den, read_size, ret; +int read_size, ret; int found_duration = 0; int is_end; int64_t filesize, offset, duration; int retry = 0; -/* flush packet queue */ -ff_flush_packet_queue(ic); - for (unsigned i = 0; i < ic->nb_streams; i++) { AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); @@ -1844,10 +1841,13 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) av_log(ic, AV_LOG_WARNING, "start time for stream %d is not set in estimate_timings_from_pts\n", i); -if (sti->parser) { -av_parser_close(sti->parser); -sti->parser = NULL; -} +/* Demuxer context updates may occur, particularly while seeking in mpegts, + * and this could loose codec parameters in avctx, + * so preserve them in codecpar. + */ +if (sti->avctx_inited && +avcodec_parameters_from_context(st->codecpar, sti->avctx)) +goto skip_duration_calc; } if (ic->skip_estimate_duration_from_pts) { @@ -1865,6 +1865,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (offset < 0) offset = 0; +ff_read_frame_flush(ic); avio_seek(ic->pb, offset, SEEK_SET); read_size = 0; for (;;) { @@ -1874,7 +1875,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) break; do { -ret = ff_read_packet(ic, pkt); +ret = av_read_frame(ic, pkt); } while (ret == AVERROR(EAGAIN)); if (ret != 0) break; @@ -1884,15 +1885,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (pkt->pts != AV_NOPTS_VALUE && (st->start_time != AV_NOPTS_VALUE || sti->first_dts != AV_NOPTS_VALUE)) { -if (pkt->duration == 0) { -compute_frame_duration(ic, &num, &den, st, sti->parser, pkt); -if (den && num) { -pkt->duration = av_rescale_rnd(1, - num * (int64_t) st->time_base.den, - den * (int64_t) st->time_base.num, - AV_ROUND_DOWN); -} -} duration = pkt->pts + pkt->duration; found_duration = 1; if (st->start_time != AV_NOPTS_VALUE) @@ -1948,15 +1940,13 @@ skip_duration_calc: fill_all_stream_timings(ic); avio_seek(ic->pb, old_offset, SEEK_SET); + +ff_read_frame_flush(ic); for (unsigned i = 0; i < ic->nb_streams; i++) { AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); sti->cur_dts = sti->first_dts; -sti->last_IP_pts = AV_NOPTS_VALUE; -sti->last_dts_for_order_check = AV_NOPTS_VALUE; -for (int j = 0; j < MAX_REORDER_D
[FFmpeg-devel] [PATCH v2 0/1] avformat/demux: Fix accurate probing of durations in mpegts/ps
v1: There was an issue with teletext where resolution is set just once at decoder init (teletext resolution is fixed/hard coded), so it is somewhat fragile: when a demuxer context update occurs, it is lost/overriden by avcodec_parameters_to_context(sti->avctx, st->codecpar) in read_frame_internal. They could have been other scenario besides teletext, I don't know. v2: So now at estimate_timings_from_pts, with one or more seeking involved (seeking is detected by the mpegts demuxer and set last_vn=-1, so pmt is forced/updated and results in demuxer context update), it is required to preserve the info in codecpar at first. Thanks to Michael for reporting the issue. Nicolas Gaullier (1): avformat/demux: Fix accurate probing of durations in mpegts/ps libavformat/demux.c | 36 ++-- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 100 insertions(+), 110 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] doc/fate: advise on --assert-level=2
>De : Nicolas Gaullier >Envoyé : mardi 12 mars 2024 13:09 >Objet : [PATCH] doc/fate: advise on --assert-level=2 > >diff --git a/doc/fate.texi b/doc/fate.texi index 2fa8c34c2d..17644ce65a 100644 >--- a/doc/fate.texi >+++ b/doc/fate.texi >@@ -79,6 +79,14 @@ Do not put a '~' character in the samples path to indicate >a home directory. Because of shell nuances, this will cause FATE to fail. > @end float > >+Beware that some assertions are disabled by default, so mind setting >+@option{--assert-level=} at configuration time, e.g. when >+seeking the highest possible test coverage: >+@example >+./configure --assert-level=2 >+@end example >+Note that raising the assert level could have a performance impact. Ping ? (I forgot to raise the version to v2, but it actually includes a notice on performance impact following Michael's comment) Feel free to amend/reauthor if my english is not good enough. 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avformat/demux: Fix accurate probing of durations in mpegts/ps
Two issues affect accuracy of duration in estimate_timings_from_pts(): - pkt->duration typically reports the duration of a single audio frame, whereas a pes often contain several audio frames - for video, compute_frame_duration() use r_frame_rate which is not reliable; typically, it is the duration of a single field for an interlaced video using two field pictures. Packet splitting/parsing is required to get accurate durations, so this patch replaces ff_read_packet() calls by av_read_frame() calls. Note that concatdec makes use of avformat_find_stream_info() to stitch correctly the files, so it benefits from this patch (typically, overlap is avoided). e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream duration is now longer than that of the video, which results in concatdec joining on audio after the patch instead of joining on video before that. Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 30 +--- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 93 insertions(+), 111 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 4c50eb5568..7e75e7149c 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1820,20 +1820,17 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) #define DURATION_MAX_READ_SIZE 25LL #define DURATION_MAX_RETRY 6 -/* only usable for MPEG-PS streams */ +/* only usable for MPEG-PS/TS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) { FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; -int num, den, read_size, ret; +int read_size, ret; int found_duration = 0; int is_end; int64_t filesize, offset, duration; int retry = 0; -/* flush packet queue */ -ff_flush_packet_queue(ic); - for (unsigned i = 0; i < ic->nb_streams; i++) { AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); @@ -1843,11 +1840,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) st->codecpar->codec_type != AVMEDIA_TYPE_UNKNOWN) av_log(ic, AV_LOG_WARNING, "start time for stream %d is not set in estimate_timings_from_pts\n", i); - -if (sti->parser) { -av_parser_close(sti->parser); -sti->parser = NULL; -} } if (ic->skip_estimate_duration_from_pts) { @@ -1865,6 +1857,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (offset < 0) offset = 0; +ff_read_frame_flush(ic); avio_seek(ic->pb, offset, SEEK_SET); read_size = 0; for (;;) { @@ -1874,7 +1867,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) break; do { -ret = ff_read_packet(ic, pkt); +ret = av_read_frame(ic, pkt); } while (ret == AVERROR(EAGAIN)); if (ret != 0) break; @@ -1884,15 +1877,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) if (pkt->pts != AV_NOPTS_VALUE && (st->start_time != AV_NOPTS_VALUE || sti->first_dts != AV_NOPTS_VALUE)) { -if (pkt->duration == 0) { -compute_frame_duration(ic, &num, &den, st, sti->parser, pkt); -if (den && num) { -pkt->duration = av_rescale_rnd(1, - num * (int64_t) st->time_base.den, - den * (int64_t) st->time_base.num, - AV_ROUND_DOWN); -} -} duration = pkt->pts + pkt->duration; found_duration = 1; if (st->start_time != AV_NOPTS_VALUE) @@ -1948,15 +1932,13 @@ skip_duration_calc: fill_all_stream_timings(ic); avio_seek(ic->pb, old_offset, SEEK_SET); + +ff_read_frame_flush(ic); for (unsigned i = 0; i < ic->nb_streams; i++) { AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); sti->cur_dts = sti->first_dts; -sti->last_IP_pts = AV_NOPTS_VALUE; -sti->last_dts_for_order_check = AV_NOPTS_VALUE; -for (int j = 0; j < MAX_REORDER_DELAY + 1; j++) -sti->pts_buffer[j] = AV_NOPTS_VALUE; } } diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts index 548cab01c6..86e5e6670f 100644 --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts +++ b/tests/ref
[FFmpeg-devel] [PATCH 0/1] avformat/demux: Fix accurate probing of durations in mpegts/ps
ff_read_packet() is more lightweight, but it leads to important issues when looking for accurate durations. As a side effect, the code looks also simpler with regular av_read_frame() calls. 1) Updates in the fate tests do exhibit most of the results. 2) See also more directly the case of an audio PES containing many frames: >ffprobe tests/data/lavf/lavf.ts -select_streams a -show_entries >stream=duration -of flat Before patch: streams.stream.0.duration="0.757556" After patch: streams.stream.0.duration="1.018778" 3) Here is an additional (commonplace) sample to demonstrate the benefit for twofields-encoded video: https://0x0.st/HFbm.ts (say h264-50i_mp2.ts) >ffprobe h264-50i_mp2.ts -show_entries stream=duration -of flat Before patch: streams.stream.0.duration="2.06" streams.stream.1.duration="1.176000" After patch: streams.stream.0.duration="2.08" streams.stream.1.duration="1.20" Nicolas Gaullier (1): avformat/demux: Fix accurate probing of durations in mpegts/ps libavformat/demux.c | 30 +--- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +- tests/ref/fate/ts-opus-demux | 4 +- 3 files changed, 93 insertions(+), 111 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 0/5] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
>De : Nicolas Gaullier >Envoyé : lundi 4 mars 2024 18:32 >Objet : [PATCH v3 0/5] avcodec/parser: fix fetch_timestamp in a scenario with >unaligned packets > >Updated from v2: >patch 1: fix audio case where pts=AV_NOPTS_VALUE but dts exists (thanks to >Michael) >now pass fate with --assert-level=2 >patch 5: add inline comments and moved a line to make it more easy to read >(thanks to James) > >Nicolas Gaullier (5): > avcodec/parser: merge packets from the same frame > avcodec/parser: reindent after previous commit > avcodec/parser: fix fetch_timestamp in a scenario with unaligned >packets > avcodec/h264_parser: fix start of packet for some broken streams > lavf/demux: duplicate side_data in parse_packet() Ping ? https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10999 still apply (auto-merge) on current master Note this is presented as a patch serie because it is a use case, but it can be split. Notably: - patch 4/5 is an independent fix for the h264 parser (actually a hack to support corrupted streams properly instead of relying on a faulty implementation) - patch 5/5 is also an independent fix These two patches could possibly be reviewed independently, and that would prepare the ground for the parser patches. Thank you 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] doc/fate: advise on --assert-level=2
Signed-off-by: Nicolas Gaullier --- doc/fate.texi | 8 1 file changed, 8 insertions(+) diff --git a/doc/fate.texi b/doc/fate.texi index 2fa8c34c2d..17644ce65a 100644 --- a/doc/fate.texi +++ b/doc/fate.texi @@ -79,6 +79,14 @@ Do not put a '~' character in the samples path to indicate a home directory. Because of shell nuances, this will cause FATE to fail. @end float +Beware that some assertions are disabled by default, so mind setting +@option{--assert-level=} at configuration time, e.g. when seeking +the highest possible test coverage: +@example +./configure --assert-level=2 +@end example +Note that raising the assert level could have a performance impact. + To get the complete list of tests, run the command: @example make fate-list -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] fate: fix generating references when sh=dash
Regression since 0b98f28c46a7e3e914c51debc461 Signed-off-by: Nicolas Gaullier --- tests/fate-run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fate-run.sh b/tests/fate-run.sh index 0fead78c58..9863e4f2d9 100755 --- a/tests/fate-run.sh +++ b/tests/fate-run.sh @@ -672,7 +672,7 @@ else fi echo "${test}:${sig:-$err}:$cmpo:$erro" >$repfile -if test $err != 0 && test $gen != "no" && test "${ref#tests/data/}" == "$ref" ; then +if test $err != 0 && test $gen != "no" && test "${ref#tests/data/}" = "$ref" ; then echo "GEN $ref" cp -f "$outfile" "$ref" err=$? -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: Add durationprobesize AVOption
Ping ? v3 "bis" rebased and wrapped lines in commit msg Nicolas Gaullier (1): avformat/demux: Add durationprobesize AVOption doc/APIchanges | 3 +++ doc/formats.texi| 16 +++- libavformat/avformat.h | 12 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 40 insertions(+), 7 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/1] avformat/demux: Add durationprobesize AVOption
Yet another probesize used to get the durations when estimate_timings_from_pts is required. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step). Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/APIchanges | 3 +++ doc/formats.texi| 16 +++- libavformat/avformat.h | 12 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index cf58c8c5f0..c97acf60d1 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07 API changes, most recent first: +2024-03-11 - xx - lavf 61.1.100 - avformat.h + Add AVFormatContext.duration_probesize + 2024-03-08 - xx - lavc 61.1.100 - avcodec.h Add AVCodecContext.[nb_]side_data_prefer_packet. diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..9cada03a6e 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -225,9 +225,23 @@ Specifies the maximum number of streams. This can be used to reject files that would require too many resources due to a large number of streams. @item skip_estimate_duration_from_pts @var{bool} (@emph{input}) -Skip estimation of input duration when calculated using PTS. +Skip estimation of input duration if it requires an additional probing for pts at end of file. At present, applicable for MPEG-PS and MPEG-TS. +@item durationprobesize @var{integer} (@emph{input}) +Set probing size for input duration estimation when it actually requires an additional probing +for pts at end of file. +At present, applicable for MPEG-PS and MPEG-TS. +It is aimed at users interested in better durations probing for itself, or indirectly +for specific use cases like using the concat demuxer. +Files with high bitrates and ending cleanly (as opposed to live captures), can lead +to a large physical gap between the last video packet and the last audio packet, +so many bytes have to be read in order to get a video stream duration. +Setting this value has a performance impact even for small files because the probing size is fixed. +Default behaviour is a trade-off, largely adaptive: the probing size may range from +250K up to 16M, but it is not extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. + @item strict, f_strict @var{integer} (@emph{input/output}) Specify how strictly to follow the standards. @code{f_strict} is deprecated and should be used only via the @command{ffmpeg} tool. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index de40397676..9042a62b70 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1667,6 +1667,8 @@ typedef struct AVFormatContext { * Skip duration calcuation in estimate_timings_from_pts. * - encoding: unused * - decoding: set by user + * + * @sa duration_probesize */ int skip_estimate_duration_from_pts; @@ -1870,6 +1872,16 @@ typedef struct AVFormatContext { * @return 0 on success, a negative AVERROR code on failure */ int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb); + +/** + * Maximum number of bytes read from input in order to determine stream durations + * when using estimate_timings_from_pts in avformat_find_stream_info(). + * Demuxing only, set by the caller before avformat_find_stream_info(). + * Can be set to 0 to let avformat choose using a heuristic. + * + * @sa skip_estimate_duration_from_pts + */ +int64_t duration_probesize; } AVFormatContext; /** diff --git a/libavformat/demux.c b/libavformat/demux.c index 4c50eb5568..68b0c51720 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1817,8 +1817,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) "Estimating duration from bitrate, this may be in
[FFmpeg-devel] [PATCH 1/1] avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits
Mantissas are the last data in the channel subsegment and it appears it is sometimes missing a very few bits for the parsing to complete. This must not be confused with data corruption. In standard conditions with certified products, it has been observed that the occurence of this issue is pretty steady and about once every 2 hours. The truncation is at about 950 out of the 1024 values (923 is the minimum I have seen so far). The current code raises a severe 'Read past end' error and all data is lost resulting in 20ms(@25fps) of silence for the affected channel. This patch introduces a tolerance: if 800 out of the 1024 mantissas have been parsed, a simple warning is raised and the data is preserved. Signed-off-by: Nicolas Gaullier --- libavcodec/dolby_e.c | 8 1 file changed, 8 insertions(+) diff --git a/libavcodec/dolby_e.c b/libavcodec/dolby_e.c index 9c3f6006c2..1f8442e2e9 100644 --- a/libavcodec/dolby_e.c +++ b/libavcodec/dolby_e.c @@ -845,6 +845,7 @@ static int parse_indices(DBEContext *s, DBEChannel *c) return 0; } +#define MIN_MANTISSAS 800 static int parse_mantissas(DBEContext *s, DBEChannel *c) { DBEGroup *g; @@ -885,6 +886,13 @@ static int parse_mantissas(DBEContext *s, DBEChannel *c) } } } else { +if (i == c->nb_groups - 1 +&& count * size1 > get_bits_left(&s->gb) +&& get_bits_left(&s->gb) >= 0 +&& (int)(mnt - c->mantissas) >= MIN_MANTISSAS) { +av_log(s->avctx, AV_LOG_WARNING, "Truncated mantissas @%d, highest frequencies not recoverable\n", (int)(mnt - c->mantissas)); +break; +} for (k = 0; k < count; k++) mnt[k] = get_sbits(&s->gb, size1) * scale; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits
A new ping for this same old topic. Previously with no feedback at all: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=7482 I just "rebased"-retested it. Updates: wrapped lines of commit msg The samples (source/before patch/after patch) are still available on https://0x0.st/oOvv.zip Nicolas Gaullier (1): avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits libavcodec/dolby_e.c | 8 1 file changed, 8 insertions(+) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] doc/fate: advise on --assert-level=2
Signed-off-by: Nicolas Gaullier --- doc/fate.texi | 7 +++ 1 file changed, 7 insertions(+) diff --git a/doc/fate.texi b/doc/fate.texi index 2fa8c34c2d..2fa7c70251 100644 --- a/doc/fate.texi +++ b/doc/fate.texi @@ -79,6 +79,13 @@ Do not put a '~' character in the samples path to indicate a home directory. Because of shell nuances, this will cause FATE to fail. @end float +Beware that some assertions are disabled by default, so mind setting +@option{--assert-level=} at configuration time, e.g. when seeking +the highest possible test coverage: +@example +./configure --assert-level=2 +@end example + To get the complete list of tests, run the command: @example make fate-list -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] doc/fate: advise on --assert-level=2
In my personnal experience, when running fate, --assert-level=2 has a very limited performance impact, so I think it can be recommended without further attention. Nicolas Gaullier (1): doc/fate: advise on --assert-level=2 doc/fate.texi | 7 +++ 1 file changed, 7 insertions(+) -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/1] avformat/demux: Add durationprobesize AVOption
Yet another probesize used to get the durations when estimate_timings_from_pts is required. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step) Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/APIchanges | 3 +++ doc/formats.texi| 16 +++- libavformat/avformat.h | 12 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 523945e511..c87d52fdbc 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2024-03-05 - xx - lavf 60.25.100 - avformat.h + Add AVFormatContext.duration_probesize + 2024-03-05 - xx - lavf 60.24.100 - avformat.h Add avformat_stream_group_name(). diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..9cada03a6e 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -225,9 +225,23 @@ Specifies the maximum number of streams. This can be used to reject files that would require too many resources due to a large number of streams. @item skip_estimate_duration_from_pts @var{bool} (@emph{input}) -Skip estimation of input duration when calculated using PTS. +Skip estimation of input duration if it requires an additional probing for pts at end of file. At present, applicable for MPEG-PS and MPEG-TS. +@item durationprobesize @var{integer} (@emph{input}) +Set probing size for input duration estimation when it actually requires an additional probing +for pts at end of file. +At present, applicable for MPEG-PS and MPEG-TS. +It is aimed at users interested in better durations probing for itself, or indirectly +for specific use cases like using the concat demuxer. +Files with high bitrates and ending cleanly (as opposed to live captures), can lead +to a large physical gap between the last video packet and the last audio packet, +so many bytes have to be read in order to get a video stream duration. +Setting this value has a performance impact even for small files because the probing size is fixed. +Default behaviour is a trade-off, largely adaptive: the probing size may range from +250K up to 16M, but it is not extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. + @item strict, f_strict @var{integer} (@emph{input/output}) Specify how strictly to follow the standards. @code{f_strict} is deprecated and should be used only via the @command{ffmpeg} tool. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 3a584993dd..d904ff0cd3 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1952,6 +1952,8 @@ typedef struct AVFormatContext { * Skip duration calcuation in estimate_timings_from_pts. * - encoding: unused * - decoding: set by user + * + * @sa duration_probesize */ int skip_estimate_duration_from_pts; @@ -1994,6 +1996,16 @@ typedef struct AVFormatContext { * Freed by libavformat in avformat_free_context(). */ AVStreamGroup **stream_groups; + +/** + * Maximum number of bytes read from input in order to determine stream durations + * when using estimate_timings_from_pts in avformat_find_stream_info(). + * Demuxing only, set by the caller before avformat_find_stream_info(). + * Can be set to 0 to let avformat choose using a heuristic. + * + * @sa skip_estimate_duration_from_pts + */ +int64_t duration_probesize; } AVFormatContext; /** diff --git a/libavformat/demux.c b/libavformat/demux.c index 2e1d0ed66d..4a570ca2ce 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1836,8 +1836,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) "Estimating duration from bitrate, this may be inaccurate\n"); } -#define DURATION
[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: Add durationprobesize AVOption
Thanks to Anton: - fixed APIchanges - reworded the nebulous "calculated using PTS" both for duration_probesize and older skip_estimate_duration_from_pts - documented avformat.h for the special 0 value meaning default behaviour Plus: Added "see also" cross links in apidoc between skip_estimate_duration_from_pts and duration_probesize Added version.h lost previously Nicolas Gaullier (1): avformat/demux: Add durationprobesize AVOption doc/APIchanges | 3 +++ doc/formats.texi| 16 +++- libavformat/avformat.h | 12 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + libavformat/version.h | 2 +- 6 files changed, 40 insertions(+), 7 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 0/1] avformat/demux: Add durationprobesize AVOption
Thanks to Stefano for the review. So v2 updated with: - doc/APIchanges - doc/formats.texi - revised wording It remains a question about the naming: duration plural or not ? The avoption base name is currently durationprobesize / s->duration_probesize to mimic formatprobesize / s->format_probesize. But we also have skip_estimate_duration_from_pts / s->skip_estimate_duration_from_pts, so there is nothing obvious. Nicolas Gaullier (1): avformat/demux: Add durationprobesize AVOption doc/APIchanges | 3 +++ doc/formats.texi| 13 + libavformat/avformat.h | 9 + libavformat/demux.c | 13 - libavformat/options_table.h | 1 + 5 files changed, 34 insertions(+), 5 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/1] avformat/demux: Add durationprobesize AVOption
Yet another probesize used to get the durations when estimate_timings_from_pts is required. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for example, especially if streamcopying above it. The current code is a performance trade-off that can fail to get video stream durations in a scenario with high bitrates and buffering for files ending cleanly (as opposed to live captures): the physical gap between the last video packet and the last audio packet is very high in such a case. Default behaviour is unchanged: 250k up to 250k << 6 (step by step) Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Even if a single audio stream duration is found but not the other audio/video stream durations, there will be a retry, so at the end the full user-overriden probesize will be used as expected by the user. Signed-off-by: Nicolas Gaullier --- doc/APIchanges | 3 +++ doc/formats.texi| 13 + libavformat/avformat.h | 9 + libavformat/demux.c | 13 - libavformat/options_table.h | 1 + 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index 7d46ebb006..548c91effe 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2024-03-05 - xx - lavf 60.23.100 - options_table.h + Add AVOption durationprobesize + 2024-02-28 - xx - swr 4.14.100 - swresample.h swr_convert() now accepts arrays of const pointers (to input and output). diff --git a/doc/formats.texi b/doc/formats.texi index 69fc1457a4..b9feef5d15 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -228,6 +228,19 @@ would require too many resources due to a large number of streams. Skip estimation of input duration when calculated using PTS. At present, applicable for MPEG-PS and MPEG-TS. +@item durationprobesize @var{integer} (@emph{input}) +Set probing size in bytes for estimating durations when calculated using PTS. +At present, applicable for MPEG-PS and MPEG-TS. +It is aimed at users interested in better durations probing for itself, or indirectly +for specific use cases like using the concat demuxer. +Files with high bitrates and ending cleanly (as opposed to live captures), can lead +to a large physical gap between the last video packet and the last audio packet, +so many bytes have to be read in order to get a video stream duration. +Setting this value has a performance impact even for small files because the probing size is fixed. +Default behaviour is a trade-off, largely adaptive: the probing size may range from +250K up to 16M, but it is not extended to get streams durations at all costs. +Must be an integer not lesser than 1, or 0 for default behaviour. + @item strict, f_strict @var{integer} (@emph{input/output}) Specify how strictly to follow the standards. @code{f_strict} is deprecated and should be used only via the @command{ffmpeg} tool. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index f4506f4cf1..7b714f3c65 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1994,6 +1994,15 @@ typedef struct AVFormatContext { * Freed by libavformat in avformat_free_context(). */ AVStreamGroup **stream_groups; + +/** + * Maximum number of bytes read from input in order to determine stream durations + * when using estimate_timings_from_pts in avformat_find_stream_info(). + * + * - encoding: unused + * - decoding: set by user + */ +int64_t duration_probesize; } AVFormatContext; /** diff --git a/libavformat/demux.c b/libavformat/demux.c index 2e1d0ed66d..4a570ca2ce 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1836,8 +1836,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) "Estimating duration from bitrate, this may be inaccurate\n"); } -#define DURATION_MAX_READ_SIZE 25LL -#define DURATION_MAX_RETRY 6 +#define DURATION_MAX_READ_SIZE_DEFAULT 25LL +#define DURATION_MAX_RETRY_DEFAULT 6 +#define DURATION_MAX_RETRY_USER 1 /* only usable for MPEG-PS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) @@ -1845,6 +1846,8 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; int num, den, read_size, ret; +int64_t duration_max_read_size = ic->duration_probesize ? ic->duration_probesize >> DURATION_MAX_RETRY_USER : DURATION_MAX_READ_SIZE_DEFAULT; +
Re: [FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption
>De : Stefano Sabatini >Envoyé : mercredi 7 février 2024 00:52 > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h index >> + * stream durations. Used by avformat_find_stream_info() for MPEG-TS/PS. > >let's clarify this: is there any reason why this should not be used with other >formats? If this the case, probably a private option would be best. If not, >probably we should amend the doxy as it suggests it is only useful with MPEG >TS/PS. There is already an AVOption in the same case: skip_estimate_duration_from_pts, but indeed, it is much more appropriate to mention estimate_timings_from_pts rather than referring to mpeg directly. The texi says "At present, applicable for MPEG-PS and MPEG-TS". So, I will just try to go in the same logic. >> diff --git a/libavformat/options_table.h b/libavformat/options_table.h >> index 91708de453..c2bdb484a7 100644 >> --- a/libavformat/options_table.h >> +++ b/libavformat/options_table.h > >> +{"durationprobesize", "maximum number of bytes to probe the stream >> +durations", OFFSET(duration_probesize), AV_OPT_TYPE_INT64, {.i64 = 0 >> +}, 0, INT64_MAX, D}, > >duration_probesize? ... to probe the stream duration (why the plural?) The option affects the probing of all the streams and then these are computed to get the overall file duration. I will update all the wording. The naming of the avoption itself is a big worry for me. I tried to mimic format_probesize, but plural or not, I don't know what is best? I will send a v2 with same code but all revised wordings and doc. Thank you very much for the review. Sorry for the delay, I was very busy with my other patch serie. 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/5] avcodec/parser: merge packets from the same frame
>De : ffmpeg-devel De la part de Michael >Niedermayer >Envoyé : dimanche 3 mars 2024 23:07 >À : FFmpeg development discussions and patches >Objet : Re: [FFmpeg-devel] [PATCH v2 1/5] avcodec/parser: merge packets from >the same frame > >On Fri, Mar 01, 2024 at 02:39:19PM +0100, Nicolas Gaullier wrote: >> The mpegts demuxer splits packets according to its max_packet_size. >> This currently fills the AVCodecParserContext s->cur_frame_* arrays >> with kind of 'empty' entries: no pts/dts. >> This patch merges these entries, so the parser behaviour is >> independent from the demuxer settings. >> This patch is required for the following patch which will fetch 'past' >> timestamps from past cur_frames. >> >> Signed-off-by: Nicolas Gaullier >> --- >> libavcodec/parser.c | 4 >> 1 file changed, 4 insertions(+) > >Breaks fate-seek-lavf-as Really sorry about that, I was not aware that assert-level=2 had to be enabled to get full fate tests. Maybe this should be documented in the fate.texi. I also checked the green light on patchwork... It seems at least assert-level=1 is not supposed to consume too much cpu, so maybe it could be enabled for patchwork? Thank you very much for taking time for this review. 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 5/5] lavf/demux: duplicate side_data in parse_packet()
Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 25 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 3 files changed, 106 insertions(+), 91 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 2e1d0ed66d..299e693606 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, FFStream *const sti = ffstream(st); const uint8_t *data = pkt->data; int size = pkt->size; -int ret = 0, got_output = flush; +int ret = 0, got_output = flush, pkt_side_data_consumed = 0; if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) { // preserve 0-size sync packets @@ -1231,10 +1231,21 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } if (pkt->side_data) { -out_pkt->side_data = pkt->side_data; -out_pkt->side_data_elems = pkt->side_data_elems; -pkt->side_data = NULL; -pkt->side_data_elems= 0; +/* for the first iteration, side_data are simply moved to output. + * in case of additional iterations, they are duplicated each time. */ +if (!pkt_side_data_consumed) { +pkt_side_data_consumed = 1; +out_pkt->side_data = pkt->side_data; +out_pkt->side_data_elems = pkt->side_data_elems; +} else for (int i = 0; i < pkt->side_data_elems; i++) { +const AVPacketSideData *const src_sd = &pkt->side_data[i]; +uint8_t *dst_data = av_packet_new_side_data(out_pkt, src_sd->type, src_sd->size); +if (!dst_data) { +ret = AVERROR(ENOMEM); +goto fail; +} +memcpy(dst_data, src_sd->data, src_sd->size); +} } /* set the duration */ @@ -1286,6 +1297,10 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } fail: +if (pkt_side_data_consumed) { +pkt->side_data = NULL; +pkt->side_data_elems= 0; +} if (ret < 0) av_packet_unref(out_pkt); av_packet_unref(pkt); diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts index 548cab01c6..ee49e331f3 100644 --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts +++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts @@ -7,19 +7,19 @@ video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS Stream ID|224 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS Stream ID|224 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192 -audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__ -audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__ -audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__ -audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__ -audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__ -audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__ -audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__ -audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__ -audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__ -audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__ -audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__ -audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__ -audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__ +audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS
[FFmpeg-devel] [PATCH v3 2/5] avcodec/parser: reindent after previous commit
Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index 90461075fd..496ebbc231 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -144,14 +144,14 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, } else if (s->cur_offset + buf_size != s->cur_frame_end[s->cur_frame_start_index]) { /* skip remainder packets */ if (pos != s->cur_frame_pos[s->cur_frame_start_index] || pos <= 0 || pts != AV_NOPTS_VALUE || dts != AV_NOPTS_VALUE ) { -/* add a new packet descriptor */ -i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); -s->cur_frame_start_index = i; -s->cur_frame_offset[i] = s->cur_offset; -s->cur_frame_end[i] = s->cur_offset + buf_size; -s->cur_frame_pts[i] = pts; -s->cur_frame_dts[i] = dts; -s->cur_frame_pos[i] = pos; +/* add a new packet descriptor */ +i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); +s->cur_frame_start_index = i; +s->cur_frame_offset[i] = s->cur_offset; +s->cur_frame_end[i] = s->cur_offset + buf_size; +s->cur_frame_pts[i] = pts; +s->cur_frame_dts[i] = dts; +s->cur_frame_pos[i] = pos; } else { s->cur_frame_end[s->cur_frame_start_index] = s->cur_offset + buf_size; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 0/5] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
Updated from v2: patch 1: fix audio case where pts=AV_NOPTS_VALUE but dts exists (thanks to Michael) now pass fate with --assert-level=2 patch 5: add inline comments and moved a line to make it more easy to read (thanks to James) Thank you for this review Nicolas Gaullier (5): avcodec/parser: merge packets from the same frame avcodec/parser: reindent after previous commit avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets avcodec/h264_parser: fix start of packet for some broken streams lavf/demux: duplicate side_data in parse_packet() libavcodec/h264_parser.c | 11 +- libavcodec/parser.c | 30 ++-- libavformat/demux.c | 25 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 5 files changed, 134 insertions(+), 104 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 3/5] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
Fix fetch_timestamp when the frame start is in a previous packet. Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index 496ebbc231..3c22fdcc2f 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -102,7 +102,7 @@ void ff_fetch_timestamp(AVCodecParserContext *s, int off, int remove, int fuzzy) s->dts= s->cur_frame_dts[i]; s->pts= s->cur_frame_pts[i]; s->pos= s->cur_frame_pos[i]; -s->offset = s->next_frame_offset - s->cur_frame_offset[i]; +s->offset = FFMAX( 0, s->next_frame_offset - s->cur_frame_offset[i]); } if (remove) s->cur_frame_offset[i] = INT64_MAX; @@ -158,11 +158,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,12 +179,13 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, /* update the file pointer */ if (*poutbuf_size) { +s->fetch_timestamp = index >= 0 || !s->frame_offset ? 1 : index; + /* fill the data for the current frame */ s->frame_offset = s->next_frame_offset; /* offset of the next frame */ s->next_frame_offset = s->cur_offset + index; -s->fetch_timestamp = 1; } else { /* Don't return a pointer to dummy_buf. */ *poutbuf = NULL; -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/5] avcodec/parser: merge packets from the same frame
The mpegts demuxer splits packets according to its max_packet_size. This currently fills the AVCodecParserContext s->cur_frame_* arrays with kind of 'empty' entries: no pts/dts. This patch merges these entries, so the parser behaviour is independent from the demuxer settings. This patch is required for the following patch which will fetch 'past' timestamps from past cur_frames. Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index efc28b8918..90461075fd 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -142,6 +142,8 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, memset(dummy_buf, 0, sizeof(dummy_buf)); buf = dummy_buf; } else if (s->cur_offset + buf_size != s->cur_frame_end[s->cur_frame_start_index]) { /* skip remainder packets */ +if (pos != s->cur_frame_pos[s->cur_frame_start_index] || pos <= 0 || +pts != AV_NOPTS_VALUE || dts != AV_NOPTS_VALUE ) { /* add a new packet descriptor */ i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); s->cur_frame_start_index = i; @@ -150,6 +152,9 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, s->cur_frame_pts[i] = pts; s->cur_frame_dts[i] = dts; s->cur_frame_pos[i] = pos; +} else { +s->cur_frame_end[s->cur_frame_start_index] = s->cur_offset + buf_size; +} } if (s->fetch_timestamp) { -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 4/5] avcodec/h264_parser: fix start of packet for some broken streams
Signed-off-by: Nicolas Gaullier --- libavcodec/h264_parser.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 94cfbc481e..6b721ec253 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -124,7 +124,16 @@ static int h264_find_frame_end(H264ParseContext *p, const uint8_t *buf, if (nalu_type == H264_NAL_SEI || nalu_type == H264_NAL_SPS || nalu_type == H264_NAL_PPS || nalu_type == H264_NAL_AUD) { if (pc->frame_start_found) { -i++; +/* Some streams in the wild are missing the zero_byte at the NAL_AUD: + * it is following just afterwards. + * To avoid any accidental borrowing of a byte in the previous frame + * (which would return a negative index and indicate that fetch_timestamps + * has to get the pts from the previous frame), + * better have the start of packet strictly aligned. + * To make it a more general rule, just test the following three bytes are null. + */ +i += 1 + (!p->is_avc && state == 5 && i == 3 && nalu_type == H264_NAL_AUD && +buf_size >= 9 && !AV_RB24(buf + 5)); goto found; } } else if (nalu_type == H264_NAL_SLICE || nalu_type == H264_NAL_DPA || -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 5/5] lavf/demux: duplicate side_data in parse_packet()
Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 23 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 3 files changed, 104 insertions(+), 91 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 2e1d0ed66d..722bb35c4c 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, FFStream *const sti = ffstream(st); const uint8_t *data = pkt->data; int size = pkt->size; -int ret = 0, got_output = flush; +int ret = 0, got_output = flush, pkt_side_data_consumed = 0; if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) { // preserve 0-size sync packets @@ -1231,10 +1231,19 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } if (pkt->side_data) { -out_pkt->side_data = pkt->side_data; -out_pkt->side_data_elems = pkt->side_data_elems; -pkt->side_data = NULL; -pkt->side_data_elems= 0; +if (!pkt_side_data_consumed) { +out_pkt->side_data = pkt->side_data; +out_pkt->side_data_elems = pkt->side_data_elems; +pkt_side_data_consumed = 1; +} else for (int i = 0; i < pkt->side_data_elems; i++) { +const AVPacketSideData *const src_sd = &pkt->side_data[i]; +uint8_t *dst_data = av_packet_new_side_data(out_pkt, src_sd->type, src_sd->size); +if (!dst_data) { +ret = AVERROR(ENOMEM); +goto fail; +} +memcpy(dst_data, src_sd->data, src_sd->size); +} } /* set the duration */ @@ -1286,6 +1295,10 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } fail: +if (pkt_side_data_consumed) { +pkt->side_data = NULL; +pkt->side_data_elems= 0; +} if (ret < 0) av_packet_unref(out_pkt); av_packet_unref(pkt); diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts index 548cab01c6..ee49e331f3 100644 --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts +++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts @@ -7,19 +7,19 @@ video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS Stream ID|224 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS Stream ID|224 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192 -audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__ -audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__ -audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__ -audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__ -audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__ -audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__ -audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__ -audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__ -audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__ -audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__ -audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__ -audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__ -audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__ +audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 video|1|29782|0.330911|26182|0.290911|3600|0.04|14098|124268|___|MPEGTS Stream ID|224 video|1|33382|0.370911|29782|0.330911|3600|0.04000
[FFmpeg-devel] [PATCH v2 4/5] avcodec/h264_parser: fix start of packet for some broken streams
Signed-off-by: Nicolas Gaullier --- libavcodec/h264_parser.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 94cfbc481e..6b721ec253 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -124,7 +124,16 @@ static int h264_find_frame_end(H264ParseContext *p, const uint8_t *buf, if (nalu_type == H264_NAL_SEI || nalu_type == H264_NAL_SPS || nalu_type == H264_NAL_PPS || nalu_type == H264_NAL_AUD) { if (pc->frame_start_found) { -i++; +/* Some streams in the wild are missing the zero_byte at the NAL_AUD: + * it is following just afterwards. + * To avoid any accidental borrowing of a byte in the previous frame + * (which would return a negative index and indicate that fetch_timestamps + * has to get the pts from the previous frame), + * better have the start of packet strictly aligned. + * To make it a more general rule, just test the following three bytes are null. + */ +i += 1 + (!p->is_avc && state == 5 && i == 3 && nalu_type == H264_NAL_AUD && +buf_size >= 9 && !AV_RB24(buf + 5)); goto found; } } else if (nalu_type == H264_NAL_SLICE || nalu_type == H264_NAL_DPA || -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/5] avcodec/parser: merge packets from the same frame
The mpegts demuxer splits packets according to its max_packet_size. This currently fills the AVCodecParserContext s->cur_frame_* arrays with kind of 'empty' entries: no pts/dts. This patch merges these entries, so the parser behaviour is independent from the demuxer settings. This patch is required for the following patch which will fetch 'past' timestamps from past cur_frames. Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 4 1 file changed, 4 insertions(+) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index efc28b8918..249f81d4bb 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -142,6 +142,7 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, memset(dummy_buf, 0, sizeof(dummy_buf)); buf = dummy_buf; } else if (s->cur_offset + buf_size != s->cur_frame_end[s->cur_frame_start_index]) { /* skip remainder packets */ +if (pos != s->cur_frame_pos[s->cur_frame_start_index] || pos <= 0 || pts != AV_NOPTS_VALUE ) { /* add a new packet descriptor */ i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); s->cur_frame_start_index = i; @@ -150,6 +151,9 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, s->cur_frame_pts[i] = pts; s->cur_frame_dts[i] = dts; s->cur_frame_pos[i] = pos; +} else { +s->cur_frame_end[s->cur_frame_start_index] = s->cur_offset + buf_size; +} } if (s->fetch_timestamp) { -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 3/5] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
Fix fetch_timestamp when the frame start is in a previous packet. Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index ede9e7e8b4..e4e9fbf48f 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -102,7 +102,7 @@ void ff_fetch_timestamp(AVCodecParserContext *s, int off, int remove, int fuzzy) s->dts= s->cur_frame_dts[i]; s->pts= s->cur_frame_pts[i]; s->pos= s->cur_frame_pos[i]; -s->offset = s->next_frame_offset - s->cur_frame_offset[i]; +s->offset = FFMAX( 0, s->next_frame_offset - s->cur_frame_offset[i]); } if (remove) s->cur_frame_offset[i] = INT64_MAX; @@ -157,11 +157,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, @@ -178,12 +178,13 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, /* update the file pointer */ if (*poutbuf_size) { +s->fetch_timestamp = index >= 0 || !s->frame_offset ? 1 : index; + /* fill the data for the current frame */ s->frame_offset = s->next_frame_offset; /* offset of the next frame */ s->next_frame_offset = s->cur_offset + index; -s->fetch_timestamp = 1; } else { /* Don't return a pointer to dummy_buf. */ *poutbuf = NULL; -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 0/5] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
This is the following of https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10843 The file submited by Michael highlighted 3 different and independent issues I missed in the first version: - some corrupt mpegts files are missing the zero_byte at the NAL, but a "full" NAL start including the zero_byte can be found just afterwards, so they can be identified. In some cases, there might be a terminal null byte at the end of the previous frame and thus it is "borrowed" by current code to form a complete, full NAL start: in such a case, the fix fetch_timestamp now get the pts of the previous pes, which is not expected. So, I've added a h264_parser patch (new patch 4) - the index may be negative even for the very first packet, despite pointing to no data, so I've added the "!s->frame_offset" condition to enable the "fetch timestamp in the past" patch - the mpegts demuxer has a "split packets" logic according to its max_packet_size, and in my understanding, the behavious of the current code does not look good, the cur_frame_arrays are fed with kind of "empty entries", which now raises an issue because some "past entries" are required to get proper timestamps from the previous frame, so this had to be fixed (new patch 1) : the entries of the splited packets should be merged to get consistent entries Patch 3: I forced the AVCodecParserContext offset to be positive. Patch 5: unchanged. Still not sure whether this patch is somewhat "required", "useless", or "bad". What possibly remains: as seen with some h264 broken streams in the wild, there might be other broken stream issues (hevc?). If such issues currently exists (but currently with no serious effect), there would be a regression in the PTS/DTS values. So any testing with corrupt streams beyond h264 is wellcome to see if other parsers require a fix (say a hack). For remembering, sample files and cover letter of the first version: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321819.html Nicolas Gaullier (5): avcodec/parser: merge packets from the same frame avcodec/parser: reindent after previous commit avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets avcodec/h264_parser: fix start of packet for some broken streams lavf/demux: duplicate side_data in parse_packet() libavcodec/h264_parser.c | 11 +- libavcodec/parser.c | 29 ++-- libavformat/demux.c | 23 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 5 files changed, 131 insertions(+), 104 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/5] avcodec/parser: reindent after previous commit
Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index 249f81d4bb..ede9e7e8b4 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -143,14 +143,14 @@ int av_parser_parse2(AVCodecParserContext *s, AVCodecContext *avctx, buf = dummy_buf; } else if (s->cur_offset + buf_size != s->cur_frame_end[s->cur_frame_start_index]) { /* skip remainder packets */ if (pos != s->cur_frame_pos[s->cur_frame_start_index] || pos <= 0 || pts != AV_NOPTS_VALUE ) { -/* add a new packet descriptor */ -i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); -s->cur_frame_start_index = i; -s->cur_frame_offset[i] = s->cur_offset; -s->cur_frame_end[i] = s->cur_offset + buf_size; -s->cur_frame_pts[i] = pts; -s->cur_frame_dts[i] = dts; -s->cur_frame_pos[i] = pos; +/* add a new packet descriptor */ +i = (s->cur_frame_start_index + 1) & (AV_PARSER_PTS_NB - 1); +s->cur_frame_start_index = i; +s->cur_frame_offset[i] = s->cur_offset; +s->cur_frame_end[i] = s->cur_offset + buf_size; +s->cur_frame_pts[i] = pts; +s->cur_frame_dts[i] = dts; +s->cur_frame_pos[i] = pos; } else { s->cur_frame_end[s->cur_frame_start_index] = s->cur_offset + buf_size; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
>De : ffmpeg-devel 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 >> --- >> 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 ___ 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".
[FFmpeg-devel] [PATCH 1/3] avcodec/h264_parser: fix start of packet for some broken streams
--- libavcodec/h264_parser.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 94cfbc481e..6b721ec253 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -124,7 +124,16 @@ static int h264_find_frame_end(H264ParseContext *p, const uint8_t *buf, if (nalu_type == H264_NAL_SEI || nalu_type == H264_NAL_SPS || nalu_type == H264_NAL_PPS || nalu_type == H264_NAL_AUD) { if (pc->frame_start_found) { -i++; +/* Some streams in the wild are missing the zero_byte at the NAL_AUD: + * it is following just afterwards. + * To avoid any accidental borrowing of a byte in the previous frame + * (which would return a negative index and indicate that fetch_timestamps + * has to get the pts from the previous frame), + * better have the start of packet strictly aligned. + * To make it a more general rule, just test the following three bytes are null. + */ +i += 1 + (!p->is_avc && state == 5 && i == 3 && nalu_type == H264_NAL_AUD && +buf_size >= 9 && !AV_RB24(buf + 5)); goto found; } } else if (nalu_type == H264_NAL_SLICE || nalu_type == H264_NAL_DPA || -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets
>> 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] lavf/demux: duplicate side_data in parse_packet()
Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 23 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 3 files changed, 104 insertions(+), 91 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 2e1d0ed66d..722bb35c4c 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, FFStream *const sti = ffstream(st); const uint8_t *data = pkt->data; int size = pkt->size; -int ret = 0, got_output = flush; +int ret = 0, got_output = flush, pkt_side_data_consumed = 0; if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) { // preserve 0-size sync packets @@ -1231,10 +1231,19 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } if (pkt->side_data) { -out_pkt->side_data = pkt->side_data; -out_pkt->side_data_elems = pkt->side_data_elems; -pkt->side_data = NULL; -pkt->side_data_elems= 0; +if (!pkt_side_data_consumed) { +out_pkt->side_data = pkt->side_data; +out_pkt->side_data_elems = pkt->side_data_elems; +pkt_side_data_consumed = 1; +} else for (int i = 0; i < pkt->side_data_elems; i++) { +const AVPacketSideData *const src_sd = &pkt->side_data[i]; +uint8_t *dst_data = av_packet_new_side_data(out_pkt, src_sd->type, src_sd->size); +if (!dst_data) { +ret = AVERROR(ENOMEM); +goto fail; +} +memcpy(dst_data, src_sd->data, src_sd->size); +} } /* set the duration */ @@ -1286,6 +1295,10 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, } fail: +if (pkt_side_data_consumed) { +pkt->side_data = NULL; +pkt->side_data_elems= 0; +} if (ret < 0) av_packet_unref(out_pkt); av_packet_unref(pkt); diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts index 548cab01c6..ee49e331f3 100644 --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts +++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts @@ -7,19 +7,19 @@ video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS Stream ID|224 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS Stream ID|224 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192 -audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__ -audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__ -audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__ -audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__ -audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__ -audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__ -audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__ -audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__ -audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__ -audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__ -audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__ -audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__ -audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__ +audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 +audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS Stream ID|192 video|1|29782|0.330911|26182|0.290911|3600|0.04|14098|124268|___|MPEGTS Stream ID|224 video|1|33382|0.370911|29782|0.330911|3600|0.04000
[FFmpeg-devel] [PATCH 0/2] fix an mpegts scenario with unaligned pes
This is the scenario: - 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... The parser can usually deal with unaligned packets thanks to the parser state, but here a new PES starts just right after the split and fetch_timestamp() does not know that the start of the PES was on the previous frame. An alternative straightforward fix directly in the mpegts demuxer is possible but really ugly, see: https://pastebin.com/J286CXDr I hope this proposal is looking better. It consists in two patches to get an identical output. The first patch fixes the fetch_timestamp mechanism. The second patch is to make parse_packet duplicate the side_data when spliting packets. It is not clear to me if this is required nor correct in a general manner? Nicolas Gaullier (2): avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets lavf/demux: duplicate side_data in parse_packet() libavcodec/parser.c | 6 +- libavformat/demux.c | 23 ++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +- tests/ref/fate/ts-demux | 8 +- 4 files changed, 107 insertions(+), 94 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] avcodec/parser: fix fetch_timestamp in a scenario with unaligned packets
Fix fetch_timestamp when the frame start is in a previous packet. Signed-off-by: Nicolas Gaullier --- libavcodec/parser.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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; -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets
>> 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/1] avformat/mpegts: fix first NAL start code splited in two different packets
>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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/1] avformat/demux: Add durationprobesize AVOption
I posted "avformat/demux: Add more retries to get more stream durations" last friday and it is a complementary patch. https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10654 Note that, if this "Add more retries" patch is accepted, I would update this patch to set DURATION_MAX_RETRY_USER to the value of MORE_DURATIONS_MAX_RETRY (3), which means that if the user ask for 8M, the first step will be to try 1M, then 2M, then up to 8M until all A/V durations are found. Currently, since there is only one extra retry to get all durations, if the user ask for 8M, the first step is to try with 4M. The default behaviour remains unchanged as exact&complete stream durations is only required for somes use cases and/or some specific files. Here is a sample file (mpegts @15Mb/s, with A/V pts cleanly cut at the end): https://0x0.st/HkxN.ts If it can help, I can add a trac issue or build a patchset with the two patches. Nicolas Gaullier (1): avformat/demux: Add durationprobesize AVOption libavformat/avformat.h | 8 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + 3 files changed, 17 insertions(+), 5 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption
Yet another probesize used to get the last pts (and thus the duration) of mpegts/ps files. It is aimed at users interested in better durations probing for itself, or because using avformat_find_stream_info indirectly and requiring exact values: for concatdec for exemple, especially if streamcopying above it. The current code does not behave well with high bitrates and high video buffering (physical gap between the last video packet and the last audio packet). Default behaviour is unchanged: 250k up to 250k << 6 (step by step) Setting this new option has two effects: - override the maximum probesize (currently 250k << 6) - reduce the number of steps to 1 instead of 6, this is to avoid detecting the audio "too early" and failing to reach a video packet. Here, even if audio duration is found but not the video duration, there will be a retry, so at the end the full user-overriden probesize will be used. Signed-off-by: Nicolas Gaullier --- libavformat/avformat.h | 8 libavformat/demux.c | 13 - libavformat/options_table.h | 1 + 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 5d0fe82250..533f5a963d 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1823,6 +1823,14 @@ typedef struct AVFormatContext { * Freed by libavformat in avformat_free_context(). */ AVStreamGroup **stream_groups; + +/** + * Maximum number of bytes read at the end of input in order to determine the + * stream durations. Used by avformat_find_stream_info() for MPEG-TS/PS. + * + * Demuxing only, set by the caller before avformat_open_input(). + */ +int64_t duration_probesize; } AVFormatContext; /** diff --git a/libavformat/demux.c b/libavformat/demux.c index 6f640b92b1..798b44c979 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1740,8 +1740,9 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) "Estimating duration from bitrate, this may be inaccurate\n"); } -#define DURATION_MAX_READ_SIZE 25LL -#define DURATION_MAX_RETRY 6 +#define DURATION_MAX_READ_SIZE_DEFAULT 25LL +#define DURATION_MAX_RETRY_DEFAULT 6 +#define DURATION_MAX_RETRY_USER 1 /* only usable for MPEG-PS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) @@ -1749,6 +1750,8 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) FFFormatContext *const si = ffformatcontext(ic); AVPacket *const pkt = si->pkt; int num, den, read_size, ret; +int64_t duration_max_read_size = ic->duration_probesize ? ic->duration_probesize >> DURATION_MAX_RETRY_USER : DURATION_MAX_READ_SIZE_DEFAULT; +int duration_max_retry = ic->duration_probesize ? DURATION_MAX_RETRY_USER : DURATION_MAX_RETRY_DEFAULT; int found_duration = 0; int is_end; int64_t filesize, offset, duration; @@ -1784,7 +1787,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) filesize = ic->pb ? avio_size(ic->pb) : 0; do { is_end = found_duration; -offset = filesize - (DURATION_MAX_READ_SIZE << retry); +offset = filesize - (duration_max_read_size << retry); if (offset < 0) offset = 0; @@ -1793,7 +1796,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) for (;;) { AVStream *st; FFStream *sti; -if (read_size >= DURATION_MAX_READ_SIZE << (FFMAX(retry - 1, 0))) +if (read_size >= duration_max_read_size << (FFMAX(retry - 1, 0))) break; do { @@ -1847,7 +1850,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) } } while (!is_end && offset && - ++retry <= DURATION_MAX_RETRY); + ++retry <= duration_max_retry); av_opt_set_int(ic, "skip_changes", 0, AV_OPT_SEARCH_CHILDREN); diff --git a/libavformat/options_table.h b/libavformat/options_table.h index 91708de453..c2bdb484a7 100644 --- a/libavformat/options_table.h +++ b/libavformat/options_table.h @@ -108,6 +108,7 @@ static const AVOption avformat_options[] = { {"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = 1000 }, 0, INT_MAX, D }, {"skip_estimate_duration_from_pts", "skip duration calculation in estimate_timings_from_pts", OFFSET(skip_estimate_duration_from_pts), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D}, {"max_probe_packets", "Maximum number of packets to probe a codec", OFFSET(max_probe_packets), AV_OPT_TYPE_INT, { .i64 = 2500 }, 0, INT_MAX, D }, +{"durationprobesize", "maximum number of bytes to probe the stream du
[FFmpeg-devel] [PATCH] avformat/demux: Add more retries to get more stream durations
The number of extra retries to get more than a single duration is currently limited to 1 (see commit 77a0df4b5). This patch raises this number of retries to 3 (amongst the overall 6 max retries). Exemple: this sample requires 3 retries to get all durations: http://samples.ffmpeg.org/archive/extension/ts/ mpegts+mpeg2video+ac3++mpegts_multiple_ts_packets_pmt_pid_0x50.ts Moreover, when an mpegts file is cleany cut at a precise pts independantly for audio and video, it results in a big gap between the last video packet and the last audio packet. And above the probing benefits themselves, having all durations is somewhat recommanded for using concatdec, and even required when using concatdec with streamcopy. Signed-off-by: Nicolas Gaullier --- libavformat/demux.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavformat/demux.c b/libavformat/demux.c index 6f640b92b1..f489868d34 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -1742,6 +1742,7 @@ static void estimate_timings_from_bit_rate(AVFormatContext *ic) #define DURATION_MAX_READ_SIZE 25LL #define DURATION_MAX_RETRY 6 +#define MORE_DURATIONS_MAX_RETRY 3 /* only usable for MPEG-PS streams */ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) @@ -1753,6 +1754,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) int is_end; int64_t filesize, offset, duration; int retry = 0; +int retry_get_more_durations = 0; /* flush packet queue */ ff_flush_packet_queue(ic); @@ -1783,7 +1785,6 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) /* XXX: may need to support wrapping */ filesize = ic->pb ? avio_size(ic->pb) : 0; do { -is_end = found_duration; offset = filesize - (DURATION_MAX_READ_SIZE << retry); if (offset < 0) offset = 0; @@ -1833,7 +1834,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) } /* check if all audio/video streams have valid duration */ -if (!is_end) { +if (found_duration) { is_end = 1; for (unsigned i = 0; i < ic->nb_streams; i++) { const AVStream *const st = ic->streams[i]; @@ -1846,6 +1847,7 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset) } } } while (!is_end && + (!found_duration || ++retry_get_more_durations <= MORE_DURATIONS_MAX_RETRY) && offset && ++retry <= DURATION_MAX_RETRY); -- 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-requ...@ffmpeg.org with subject "unsubscribe".