[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(>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); +res =
[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 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), >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
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, , , 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); for (unsigned i = 0; i <
[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(>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->user_csp == AVCOL_SP
[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 = _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 = _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(>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(s->user_all, CS_NB)]
[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, 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(>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, ret; Colo
[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, , , 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_DELAY + 1; j++) -
[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, , , 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/fate/concat-demuxer-si
[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(>gb) +&& get_bits_left(>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(>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 = >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 Stream
[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 = >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.04|13329|139
[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 = >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.04|13329|139
[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 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".
[FFmpeg-devel] [PATCH] avformat/mpegts: fix first NAL start code splited in two different packets
When PES are not aligned and a tiny nal-encoded frame is contained in a single TS packet, the first NAL startcode may be split by the PES boundary, making the packet unworkable. This patch shift the PES boundaries to avoid this. Signed-off-by: Nicolas Gaullier --- libavformat/mpegts.c | 41 +++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index bef00c88e7..4b4cf82fb9 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -1149,6 +1149,7 @@ static int mpegts_push_data(MpegTSFilter *filter, PESContext *pes = filter->u.pes_filter.opaque; MpegTSContext *ts = pes->ts; const uint8_t *p; +int pes_align_shift = 0; int ret, len; if (!ts->pkt) @@ -1156,6 +1157,37 @@ static int mpegts_push_data(MpegTSFilter *filter, if (is_start) { if (pes->state == MPEGTS_PAYLOAD && pes->data_index > 0) { +if ((pes->st->codecpar->codec_id == AV_CODEC_ID_H264 + || pes->st->codecpar->codec_id == AV_CODEC_ID_HEVC) +&& pes->data_index < TS_PACKET_SIZE - 4 - PES_HEADER_SIZE +&& pes->data_index >= 4 +&& buf_size >= 4 ) { +/* check/avoid spliting the start code + first byte of the first NAL unit in two different packets. + * this could happen with a tiny unaligned PES that fits in a single ts packet. */ +uint8_t *last_p_end = pes->buffer->data + pes->data_index - 4; +p = buf + PES_HEADER_SIZE + buf[PES_HEADER_SIZE - 1]; +if (last_p_end[3] == 0x00 && AV_RB24(p) == 0x01) +pes_align_shift = 4; +else if (AV_RB16(last_p_end + 2)== 0x && AV_RB16(p) == 0x0001) +pes_align_shift = 3; +else if (AV_RB24(last_p_end + 1)== 0x00 && *p == 0x01) +pes_align_shift = 2; +else if (AV_RB32(last_p_end) == 0x0001) +pes_align_shift = 1; +if (pes_align_shift) +{ +last_p_end += 4; +if (pes_align_shift > 3) +*last_p_end++ = 0x00; +if (pes_align_shift > 2) +*last_p_end++ = 0x00; +if (pes_align_shift > 1) +*last_p_end++ = 0x01; +*last_p_end = *(p + pes_align_shift - 1); +pes->data_index += pes_align_shift; +buf_size -= pes_align_shift; +} +} ret = new_pes_packet(pes, ts->pkt); if (ret < 0) return ret; @@ -1301,6 +1333,7 @@ skip: /* we got the full header. We parse it and get the payload */ pes->state = MPEGTS_PAYLOAD; pes->data_index = 0; +p += pes_align_shift; if (pes->stream_type == 0x12 && buf_size > 0) { int sl_header_bytes = read_sl_header(pes, >sl, p, buf_size); @@ -1409,9 +1442,13 @@ skip: pes->data_index += buf_size; /* emit complete packets with known packet size * decreases demuxer delay for infrequent packets like subtitles from - * a couple of seconds to milliseconds for properly muxed files. */ + * a couple of seconds to milliseconds for properly muxed files. + * disabled for video/NALs because at this point it could split/break the first NAL start code. + */ if (!ts->stop_parse && pes->PES_packet_length && -pes->pes_header_size + pes->data_index == pes->PES_packet_length + PES_START_SIZE) { +pes->pes_header_size + pes->data_index == pes->PES_packet_length + PES_START_SIZE && +pes->st->codecpar->codec_id != AV_CODEC_ID_H264 && +pes->st->codecpar->codec_id != AV_CODEC_ID_HEVC) { ts->stop_parse = 1; ret = new_pes_packet(pes, ts->pkt); pes->state = MPEGTS_SKIP; -- 2.30.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [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(-) -- 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/s302m: enable non-PCM decoding
>On 2024-01-28 04:24 pm, Anton Khirnov wrote: >> Quoting Gyan Doshi (2024-01-26 05:23:50) >>> >>> On 2024-01-25 06:47 pm, Andreas Rheinhardt wrote: Gyan Doshi: >> On 2024-01-25 10:29 am, Andreas Rheinhardt wrote: >> Gyan Doshi: >>> Set up framework for non-PCM decoding in-place and add support >>> for Dolby-E decoding. >>> >>> Useful for direct transcoding of non-PCM audio in live inputs. >>> --- >>> configure | 1 + >>> doc/decoders.texi | 40 +++ >>> libavcodec/s302m.c | 609 >>> + >>> 3 files changed, 543 insertions(+), 107 deletions(-) >>> >>> diff --git a/configure b/configure index c8ae0a061d..8db3fa3f4b >>> 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -2979,6 +2979,7 @@ rv20_decoder_select="h263_decoder" >>> rv20_encoder_select="h263_encoder" >>> rv30_decoder_select="golomb h264pred h264qpel mpegvideodec rv34dsp" >>> rv40_decoder_select="golomb h264pred h264qpel mpegvideodec rv34dsp" >>> +s302m_decoder_select="dolby_e_decoder" >>> screenpresso_decoder_deps="zlib" >>> shorten_decoder_select="bswapdsp" >>> sipr_decoder_select="lsp" >>> diff --git a/doc/decoders.texi b/doc/decoders.texi index >>> 293c82c2ba..9f85c876bf 100644 >>> --- a/doc/decoders.texi >>> +++ b/doc/decoders.texi >>> @@ -347,6 +347,46 @@ configuration. You need to explicitly >>> configure the build with >>> An FFmpeg native decoder for Opus exists, so users can decode Opus >>> without this library. >>> +@section s302m >>> + >>> +SMPTE ST 302 decoder. >>> + >>> +SMPTE ST 302 is a method for storing AES3 data format within an >>> +MPEG >>> Transport >>> +Stream. AES3 streams can contain LPCM streams of 2, 4, 6 or 8 >>> channels with a >>> +bit depth of 16, 20 or 24-bits at a sample rate of 48 kHz. >>> +They can also contain non-PCM codec streams such as AC-3 or Dolby-E. >>> + >> This sounds like we should add bitstream filters to extract the >> proper underlying streams instead. >> (I see only two problems with this approach: The BSF API needs to >> set the CodecID of the output during init, but at this point no >> packet has reached the BSF to determine it. And changing codec IDs >> mid-stream is also not supported.) > In theory, this decoder shouldn't exist, as it is just a carrier, > whether of LPCM or non-PCM. > FFmpeg architecture also imposes a fundamental limitation in that > one s302m stream may carry multiple payload streams and we support > only one decoding context per input stream Then why does the demuxer not separate the data into multiple streams? >>> I didn't add demuxing support for this codec in MPEGTS, but I can >>> venture >>> >>> a) it would mean essentially inlining this decoder in the demuxer. >> Why is that a problem? This decoder seems like it shouldn't be a >> decoder. >> >> I agree with Andreas that this seems like it's a demuxer pretending to >> be a decoder. > >This module transforms the entire raw payload data to generate its output, >even if the syntax is simple which essentially makes it a de-coder. The >de-multiplexer aspect of multiple streams is an academic possibility allowed >by the >standard but not seen in any sample which makes me suspect it's used >for carriage between broadcast facilities rather than something ever sent to >an OTT provider, let alone an end user. > >Regards, >Gyan AFAIK, DolbyE may be found on satellite feeds, for carriage between broadcast facilities, and thus it makes them accessible so they may be grabbed by "end users". Otherwise, it is "broadcast professional end users", which are users too. AFAIK, its most common form is 20Bits and you simply "cannot" have a single stream in a 20Bit carrier; but indeed, most of the time only the first stream ("program") is used and the second is a downmix; but not always. For example, you can have a first program which is standard 5.1 and a second program with Audio Description. Anyway, I like this s302m patch support as it makes the existing DolbyE decoder really available (not requiring two pass). If this design is not acceptable and you want to get all the picture, then I don't think there is any other option as introducing a new filter, 1 pin in 48KHz, N pins out (XXKhz) with stream metadata inserted in each (downmix meta, dialnorm..). That would be nice, indeed from a user point of view, and this was already discussed in some forum discussions. But are you ready to go for it ? A filter, dedicated to DolbyE, and that would link to lavc ? 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] [PATCH] avformat/concatdec: fix seek with dts
Use same heuristic as in fftools. Signed-off-by: Nicolas Gaullier --- libavformat/concatdec.c | 17 +++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 40 ++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index ffa8ade25b..96de9117fe 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -384,7 +384,22 @@ static int open_file(AVFormatContext *avf, unsigned fileno) if ((ret = match_streams(avf)) < 0) return ret; if (file->inpoint != AV_NOPTS_VALUE) { - if ((ret = avformat_seek_file(cat->avf, -1, INT64_MIN, file->inpoint, file->inpoint, 0)) < 0) +int64_t seek_timestamp = file->inpoint; +if (!(cat->avf->iformat->flags & AVFMT_SEEK_TO_PTS)) { +int dts_heuristic = 0; +int i; +for (i=0; iavf->nb_streams; i++) { +const AVCodecParameters *par = cat->avf->streams[i]->codecpar; +if (par->video_delay) { +dts_heuristic = 1; +break; +} +} +if (dts_heuristic) { +seek_timestamp -= 3*AV_TIME_BASE / 23; +} +} +if ((ret = avformat_seek_file(cat->avf, -1, INT64_MIN, seek_timestamp, seek_timestamp, 0)) < 0) return ret; } return 0; diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts index 548cab01c6..329b42fbe9 100644 --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts +++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts @@ -126,6 +126,24 @@ audio|0|177970|1.977444|177970|1.977444|2351|0.026122|209|N/A|K__ audio|0|180321|2.003567|180321|2.003567|2351|0.026122|209|N/A|K__ video|1|174764|1.941822|171164|1.901822|3600|0.04|12678|347800|___|MPEGTS Stream ID|224 video|1|178364|1.981822|174764|1.941822|3600|0.04|24711|361336|K__ +video|1|125182|1.390911|121582|1.350911|3600|0.04|12398|254552|___|MPEGTS Stream ID|224 +video|1|128782|1.430911|125182|1.390911|3600|0.04|13455|267336|___|MPEGTS Stream ID|224 +audio|0|99515|1.105722|99515|1.105722|2351|0.026122|209|308508|K__|MPEGTS Stream ID|192 +audio|0|101866|1.131844|101866|1.131844|2351|0.026122|209|N/A|K__ +audio|0|104217|1.157967|104217|1.157967|2351|0.026122|209|N/A|K__ +audio|0|106568|1.184089|106568|1.184089|2351|0.026122|209|N/A|K__ +audio|0|108919|1.210211|108919|1.210211|2351|0.026122|209|N/A|K__ +audio|0|111270|1.236333|111270|1.236333|2351|0.026122|209|N/A|K__ +audio|0|113621|1.262456|113621|1.262456|2351|0.026122|209|N/A|K__ +audio|0|115972|1.288578|115972|1.288578|2351|0.026122|209|N/A|K__ +audio|0|118323|1.314700|118323|1.314700|2351|0.026122|209|N/A|K__ +audio|0|120674|1.340822|120674|1.340822|2351|0.026122|209|N/A|K__ +audio|0|123025|1.366944|123025|1.366944|2351|0.026122|209|N/A|K__ +audio|0|125376|1.393067|125376|1.393067|2351|0.026122|209|N/A|K__ +audio|0|127727|1.419189|127727|1.419189|2351|0.026122|209|N/A|K__ +audio|0|130078|1.445311|130078|1.445311|2351|0.026122|209|N/A|K__ +video|1|132382|1.470911|128782|1.430911|3600|0.04|13836|281624|___|MPEGTS Stream ID|224 +video|1|135982|1.510911|132382|1.470911|3600|0.04|12163|295912|___|MPEGTS Stream ID|224 video|1|139582|1.550911|135982|1.510911|3600|0.04|12692|311516|___|MPEGTS Stream ID|224 video|1|143182|1.590911|139582|1.550911|3600|0.04|10824|325052|___|MPEGTS Stream ID|224 video|1|146782|1.630911|143182|1.590911|3600|0.04|11286|336144|___|MPEGTS Stream ID|224 @@ -142,10 +160,28 @@ audio|0|153588|1.706533|153588|1.706533|2351|0.026122|209|N/A|K__ audio|0|155939|1.732656|155939|1.732656|2351|0.026122|209|N/A|K__ video|1|150382|1.670911|146782|1.630911|3600|0.04|12678|347800|___|MPEGTS Stream ID|224 video|1|153982|1.710911|150382|1.670911|3600|0.04|24711|361336|K__ +video|1|146782|1.630911|143182|1.590911|3600|0.04|12755|98700|___|MPEGTS Stream ID|224 +video|1|150382|1.670911|146782|1.630911|3600|0.04|12023|111860|___|MPEGTS Stream ID|224 +audio|0|124200|1.38|124200|1.38|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192 +audio|0|126551|1.406122|126551|1.406122|2351|0.026122|209|N/A|K__ +audio|0|128902|1.432244|128902|1.432244|2351|0.026122|209|N/A|K__ +audio|0|131253|1.458367|131253|1.458367|2351|0.026122|209|N/A|K__ +audio|0|133604|1.484489|133604|1.484489|2351|0.026122|209|N/A|K__ +audio|0|135955|1.510611|135955|1.510611|2351|0.026122|209|N/A|K__ +audio|0|138306|1.536733|138306|1.536733|2351|0.026122|209|N/A|K__ +audio|0|140657|1.562856|140657|1.562856|2351|0.026122|209|N/A|K__ +audio|0|143008|1.588978|143008|1.588978|2351|0.026122|209|N/A|K__ +audio|0|145359|1.615100|145359|1.615100|2351|0.026122|209|N/A|K__ +audio|0|147710|1.641222|147710|1.641222|2351|0.026122|209|N/A|K__ +audio|0|150061|1.667344|150061|1.667344|2351|0.026122|209|N/A|K__ +au
[FFmpeg-devel] [PATCH 0/1] avformat/concatdec: fix seek with dts
I have a bunch of patches about probing and making better concatdec, this is the first one. Unfortunately, this is mostly about heuristics and find_stream_info things no one like very much. This first patch seems very straightforward: there is already an heuristic in fftools for seeking, and the same one is required when opening a file indirectly through concatdec. (I can share a sample to reproduce the issue, but this is very commonplace). fate results is updated for one test that uses lavf.ts (two "inpoint" directives in the script, thus two modified blocks). Note: more packets are read but they are usually trimmed through segment_time_metadata etc. Nicolas Gaullier (1): avformat/concatdec: fix seek with dts libavformat/concatdec.c | 17 +++- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 40 ++- 2 files changed, 54 insertions(+), 3 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 1/2] avcodec/s302m: enable non-PCM decoding
>+#define IS_NONPCMSYNC_16(state) ((state & 0x00) == >NONPCMSYNC_16MARKER) Is this single 32 bits marker enough to avoid a fake detection ? 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
>fre 2023-03-10 klockan 10:17 +0100 skrev Nicolas Gaullier: >> + if (i == c->nb_groups - 1 >> + && count * size1 > get_bits_left(>gb) >> + && get_bits_left(>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; >> + } > >Surely there's a proper way to fix this rather than having an arbitrary >threshold. At the very least the get_bits_left() check could be moved to >before parse_mantissas(). If get_bits_left() is < 0 after >parse_mantissas() then a warning could be issued instead of erroring out, >which should have an effect similar to this. > >Is there a spec saying what to do with truncated packets? Since this is >Dolby-E I suspect the answer is "no". > >/Tomas Thank you for inspecting the patch. Concerning the arbitrary threshold: it may be understood as a very high level safeguard, but my intend was mostly to make the code more easily understandable. Actually, the fix is restricted to the last audio band (see "i == c->nb_groups - 1") and, at the end, the MIN_MANTISSAS condition could be simply removed with no audible risk, I guess. But to get this precision, the fix cannot be moved elsewhere. The idea here is to still be able to detect harsh packet truncation with full errors and keep current code behaviour in that case. Of course, this patch looks as a work-around, but a work-around that make it possible to decode the official reference bitstream implementation (dp600 at least, but maybe they are no different ones). So, this is a somewhat special scenario. Sadly, I don't see any another way to handle this. To conclude, I don't have any other idea than the complete removal of the arbitrary threshold. Would it make the patch more acceptable ? 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] decklink: Add support for compressed AC-3 output over SDI
>At this point I'm inclined to keep the code separate/private to decklink. >It's limited to AC-3 and only produces packets which are >s16 little endian. At some point I think it would be worthwhile to have some >common code that supports 20/24 bit, both endians, and other codecs like >Dolby-E, but my concern was at this point that would require a bunch of extra >testing (much of which I don't have the >streams or hardware to do), and I >didn't want to lock us permanently into an ABI that I didn't know would meet >our needs long term. > >Moving it from a static function to something that can be shared is a >relatively simple operation, but I want to wait until we have a second use >case prior to refactoring the code. Fully understand, yes you're right, thank you for the answer. 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/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 should not be confused with data corruption. For 5.1+2@25fps, 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 (never seen below 923 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 | 9 + 1 file changed, 9 insertions(+) diff --git a/libavcodec/dolby_e.c b/libavcodec/dolby_e.c index 921c33f3ba..a24edfcc93 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,14 @@ static int parse_mantissas(DBEContext *s, DBEChannel *c) } } } else { +if (i == c->nb_groups - 1 +&& count * size1 > get_bits_left(>gb) +&& get_bits_left(>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(>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
Follow up of https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220913213127.1756-1-nicolas.gaullier@cji.paris/ The only modification since this initial patch is a cosmetic line split to prevent an overly long line and the "Please wrap lines" warning. I did not get any message, I don't know what to do to get a review ? If I missed something obvious, tell me. The fact is the official maintainer disappeared, so it does not make this easy. The darkness of the official proprietary implementation is certainly an "issue", but the technology behind the dolby_e codec seems very similar to all other audio codecs, so it does not seem that critical for simple maintenance. Nicolas Gaullier (1): avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits libavcodec/dolby_e.c | 9 + 1 file changed, 9 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".
Re: [FFmpeg-devel] [PATCH] decklink: Add support for compressed AC-3 output over SDI
>+static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, >+uint8_t **outbuf, int *outsize) { This is very interesting in many other contexts. My current patch serie is about demuxing s337 (targeting dolby_e) from wav files, and it would be great to be able to re-mux back to s337 in output formats, be it decklink, wav or any broadcast format. So, maybe this belongs to s337m.c ? It could be later updated to support dolby_e. 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 7/7] doc: add doc about dolby_e decoder and wav support
--- doc/decoders.texi | 20 doc/demuxers.texi | 33 + 2 files changed, 53 insertions(+) diff --git a/doc/decoders.texi b/doc/decoders.texi index 5ba85cf9b1..750fb4bd7a 100644 --- a/doc/decoders.texi +++ b/doc/decoders.texi @@ -223,6 +223,26 @@ Loud sounds are fully compressed. Soft sounds are enhanced. @end table +@anchor{dolby_e} +@section dolby_e + +Dolby E audio decoder. + +This is a raw decoder without any further processing, which means the output +sample rate is different from the 48KHz input sample rate. +A dolby E stream is always carried in a s337m muxer, as a raw file +or muxed in any format that is able to transport an AES3 +or an equivalent uncompressed audio stream (stereo, 48KHz, 16/20/24 bits). +Currently, only wav and raw file formats are supported. Other formats require +two passes of processing: a first one to extract the s337m stream, +and another to decode it. +In case of AES3, or if the format itself supports it, the non-PCM mode may +be signaled, but in a more general manner, a stream has to be probed to be +handled as dolby E rather than pcm. +By default, stream probing is enabled which forbids pass-through as no s337m +muxer is implemented yet. In order to pass-through a dolby E stream, +@code{--non_pcm_mode copy} must be specified (see input muxer options). + @section flac FLAC audio decoder. diff --git a/doc/demuxers.texi b/doc/demuxers.texi index 3c81024f03..c86bd91461 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -928,4 +928,37 @@ which in turn, acts as a ceiling for the size of scripts that can be read. Default is 1 MiB. @end table +@section wav, w64 + +WAV/RF64/BW64/Wave64 demuxer. + +This demuxer supports spdif and s337m probing, as well as direct codec +probing in some cases (dts). +Concerning dolby E, see @ref{dolby_e,,the dolby E section in the ffmpeg-codecs manual,ffmpeg-codecs}. + +@subsection Options + +This demuxer supports the following options: + +@table @option + +@item ignore_length @var{bool} +Ignore the length field of the 'data' chunk: read everything as data up to eof. Default is disabled. +This option is not supported in the Wave64 demuxer. + +@item max_size +Max size of a single packet. Default is 4096. + +@item non_pcm_mode +This option is active only if s337m data is probed. +Specify how this data shall be handled. Default is to demux. +@table @samp +@item copy +Pass-through as pcm: s337m will not be reported +@item demux +Demux and report the encoded stream for later decoding +@end table + +@end table + @c man end DEMUXERS -- 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/7] avformat/wavdec: s337m support
Following of https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230213180936.815-5-nicolas.gaullier@cji.paris/ from the serie https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=8380 Should the version and changelog be updated too ? Nicolas Nicolas Gaullier (7): avformat/s337m: Split read_packet/get_packet avformat/s337m: Consider container bit resolution avformat/s337m: New ff_s337m_probe() avformat/wavdec: s337m support avformat/wavdec.c: Reindent after last commit avformat/wavdec: Test s337m doc: add doc about dolby_e decoder and wav support doc/decoders.texi| 20 +++ doc/demuxers.texi| 33 +++ libavformat/s337m.c | 71 libavformat/s337m.h | 54 ++ libavformat/wavdec.c | 61 +- tests/Makefile | 1 + tests/fate/audio.mak | 3 ++ tests/ref/fate/s337m-wav | 10 ++ 8 files changed, 230 insertions(+), 23 deletions(-) create mode 100644 libavformat/s337m.h create mode 100644 tests/ref/fate/s337m-wav -- 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] avformat/mxfenc: fix stored/sampled/displayed width/height
>>> The width is one thing; for whatever reason, there is a divergence between >>> DV100 on one hand and AVCI/XDCAMHD35 on the other. In my understanding, in >>> current practices, DV obey s337 (stored width includes scaling) but >>> >xdcam does not, so current code is fine >but maybe this is an >>> opportunity to document this ? >> >>AFAIK: >>- DV in MXF: found old Omneon with no scaling for stored value, no sampled >>value (so stored value), scaled value for displayed value, old Quantel with >>scaling everywhere. From my understanding of spec, I would keep the scaling. >>- MPEG-2 Video including XDCAMHD35 in MXF obey "including any decoder >>scaling or padding" wording with a 16x16 rounding for height, I have no >>file not 1920 or 3840 width >>- AVC in MXF: found old Omneon or old Quantel or old Telestream with no >>padding value for stored value (height of 540 for interlaced). I don't >>understand why it is not same as with MPEG-2 Video so I don't touch FFmpeg >>behavior >there (rounding). Actually checking >again SMPTE ST 381-2013, there >>is an explicit example: "1088: 1080-line progressive". > >I totally agree they are so many weird things in the wild, particularly >looking at some early implementations. I also have fully broken DV100 and >XDCAMHD35 Omneon records with release v6.1 (2010) at the early stages of HD, >but it was fixed afterwards (with many other >issues too!). Looking at GVG, >1440x1088i stored size was implemented from the early beginnings (2010 too) : >sample clips are still available here : >http://www.gvgdevelopers.com/concrete/products/k2/test_clips/ >There is also "kind of" reference sony implementation available here both for >xdcamhd35/avc-1440: https://www.sonycreativesoftware.com/catalystbrowse >Anyway, I think we all agree not to change anything related to MPEG2 and AVC. > >>I don't have DV in MXF with non multiple of 16 (I thought that DV is >>only 720x576 or 720x480 or 1280x720 or 1920x1080, all values multiple of >>16) and don't know about video encoding in DV so I didn't want to change the >>behavior of FFmpeg when I don't know, but case AV_CODEC_ID_DVVIDEO: >>line could be definitely removed if it is fine for you. >DV is questionable. Currently, the dust is under the rug (as a defaults >behaviour), which is an issue with very little concern. >Now, with the patch, the dust become visible, the DV rule is made explicit and >moreover it is presented as an exception, sharing code with macroblock >codecs... I think it is time to fix, even if it was not your prior intention. >I don't have an extensive experience with DV too, I just have samples here and >there like you, but it seems we share the same information. >Let see if someone else react and ask for keeping the current 1088 lines for >DV stored height, but if nobody does, I think it should be okay. > >> Do you want me to add a comment line e.g. "obey 'including any decoder >> scaling or padding' from SMPTE ST 377"? >I am not a core developer and will let others give their feedback. My personal >opinion is that the spec is supposed to be well known and does not require >commenting, but that it would be interesting to explicit why we make a >difference between DV and MPEG2/AVC. And >personally, I don't have the answer >to this question. If nobody has one, maybe a comment could say "because this >is the observed common practice". > >Nicolas Some weeks later now and no replies, maybe time to go on ? I think the "case AV_CODEC_ID_DVVIDEO:" can be removed as discussed, fate updated and that should be ok for everybody. (Ideally, it could have been an opportunity to document why we have this "DV exception", but I understand it is not very comfortable to write as there is no meaningful reason, so forget about this, this won't hold up the patch anyway) For information, there was a long thread recently on ffmpeg-user about a "bug" in dnxhd stored_height (will be fixed with your patch): https://ffmpeg.org/pipermail/ffmpeg-user/2023-February/056111.html 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 4/6] avformat/wavdec: s337m support
>This is why it is better to demand that these things be explicitly signalled. >At the same time a lot of users expect ffmpeg to Just Work, but that is not >always possible. Perhaps we should should put this in the manual and tell >>spdif/s377m/dolby-e users to RTFM? > >> And another point here in this latest edition on my patch serie is >> that the use of an existing AVOption makes it possible for users to >> upgrade their command lines just now by adding the option : >> ignored in previous version, it will take effect the day they upgrade >> their ffmpeg version, so the transition can be smooth... > >Assuming users read the documentation of course.. Yes, definitely, at the end, I don't see an option other than RTFM. Documentation : I could insert a "dolby_e" section between ac3 and flac in decoders.texi. But there is nothing specific to dolby_e for real. Maybe an overall "stream probing" chapter would be better in decoders.texi between "Decoders" and "Video Decoders" chapters; with a specific paragraph about the non_pcm_mode option for wav/s337 (and later for s302) ? 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 3/6] avformat/s337m: New ff_s337m_probe()
>> ff_s337m_probe is very similar to s337m_probe: what mainly differs is >> the input parameters. >> The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES. >> Currently it is set to 0, so has no effect (and of course I can remove >> it if someone object). >> There is two things to know about it: >> - one is that some DolbyE decoder implementations does not support the >> s337m sync word to be the first word, A minimal guard band (full of >> zero) is required in such a case : 1 word is enough in the cases I >> experimented. >> One developer might find it useful to set >> S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject >> such files. >> - one other thing is that, currently, the detection is based on 3 >> consecutive samples, But there are other implementations in the wild. >> A common single- sample implementation is to simply require a >> sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake >> detection. >> (for 16 bits, this is really dangerous!; for 24 bits, I think it is >> fair but would still require some little additions to be 100% sure). > >What a mess.. And there's no other way to reliably probe this? > >/Tomas No! But the statistics are such that at some time, a fake probe is not "possible". Anyway, we already have stream probings in current code, not only considering wav. I think my patch is similar to the spdif probe and safe. 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 2/6] avformat/s337m: Consider container bit resolution
>I haven't worked enough with S377m to really know, but I do know it's a mess. >Is there a way to differentiate "regular" packed 20-bit audio from S377m in >wav? > >/Tomas There is a relevant overview of S337m in this dolby paper: https://developer.dolby.com/globalassets/professional/dolby-e/dolby-e-tech-doc_1.2.pdf Page 25, one can read: SMPTE 337M is the primary method by which Dolby E is able to work in existing facilities and with existing devices. The standard is written such that the same AES3 interface can be shared with the normal PCM audio usage either by treating the AES3 channels independently or by alternating between data and linear PCM usage. Devices that are specifically designed for SMPTE 337M/Dolby E compatibility should be able to transition easily between both usages. The whole design is to not signal, not differentiate "normal PCM audio usage" from s337m. And indeed, manufacturers have implemented probing in all their hardware/software (be it linear/SDI input, or mxf/wav files input etc.). Note: ARD/ZDF mxf file format being "the" world exception here, as dolby_e/non-pcm must be signaled, made explicit: https://www.ard.de/ard/die-ard/ARD-ZDF-HDF02a-AVC-I-100-1080i-25-8-Audio-Tracks-100.pdf In ffmpeg, we already have spdif probing in wav, so there is nothing really new technically speaking. Quoting the previous dolby paper, about spdif (IEC61937): The IEC61937 format is similar to the SMPTE 337M format and can be considered a subset of SMPTE 337M for some data types (see SMPTE 337M Section 7), however there are significant differences in the two standards. In particular IEC61937 does not support the Dolby E data type. 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] avcodec/ac3: Remove unused fields
Signed-off-by: Nicolas Gaullier --- libavcodec/ac3dec.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/libavcodec/ac3dec.h b/libavcodec/ac3dec.h index 138b462abb..adf82b4a98 100644 --- a/libavcodec/ac3dec.h +++ b/libavcodec/ac3dec.h @@ -90,7 +90,6 @@ typedef struct AC3DecodeContext { int lfe_on; ///< lfe channel in use int dialog_normalization[2];///< dialog level in dBFS (dialnorm) int compression_exists[2]; ///< compression field is valid for frame (compre) -int compression_gain[2];///< gain to apply for heavy compression(compr) int channel_map;///< custom channel map (chanmap) int preferred_downmix; ///< Preferred 2-channel downmix mode (dmixmod) int center_mix_level; ///< Center mix level index @@ -100,7 +99,6 @@ typedef struct AC3DecodeContext { int lfe_mix_level_exists; ///< indicates if lfemixlevcod is specified (lfemixlevcode) int lfe_mix_level; ///< LFE mix level index (lfemixlevcod) int eac3; ///< indicates if current frame is E-AC-3 -int eac3_frame_dependent_found; ///< bitstream has E-AC-3 dependent frame(s) int eac3_subsbtreamid_found;///< bitstream has E-AC-3 additional substream(s) int dolby_surround_mode;///< dolby surround mode (dsurmod) int dolby_surround_ex_mode; ///< dolby surround ex mode (dsurexmod) -- 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 4/6] avformat/wavdec: s337m support
>> +#if CONFIG_S337M_DEMUXER >> + {"non_pcm_mode", "Chooses what to do with s337m", >> OFFSET(non_pcm_mode), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC, >> "non_pcm_mode"}, >> + {"copy" , "Pass s337m through unchanged", 0, >> AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 1, DEC, "non_pcm_mode"}, >> + {"demux" , "Demux s337m" , 0, >> AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"}, >> +#endif >> { NULL }, >> }; > >So default is to demux? Sounds OK and probably avoids eardrum destruction Well, to be honest, I am not very comfortable with that. The fact is, I fear that many users have scripts to mux wav/dolby_e into mxf outputs and they will be affected... But I completely understand the ffmpeg logic to "always decode", as is done currently in wav files to probe dts or even spdif which is really the same thing as s337. It does not seem reasonable to break this here. And another point here in this latest edition on my patch serie is that the use of an existing AVOption makes it possible for users to upgrade their command lines just now by adding the option : ignored in previous version, it will take effect the day they upgrade their ffmpeg version, so the transition can be smooth... >> + } else { >> + av_log(s, AV_LOG_INFO, "Passing >> through S337M data: codec will not be reported\n"); >> + } > >Perhaps the user should also be informed when S337m is demuxed rather than >passed through? > >/Tomas I could add a debug message if you think that could be helpful ? I think I cannot add an INFO log as spdif and other probe-mecanisms are not verbose in current code. 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 3/6] avformat/s337m: New ff_s337m_probe()
> The logic here is a bit hairy and I don't have time atm to digest it, but is > it entirely contained in S337m or would one need to read other specs too? > >/Tomas ff_s337m_probe is very similar to s337m_probe: what mainly differs is the input parameters. The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES. Currently it is set to 0, so has no effect (and of course I can remove it if someone object). There is two things to know about it: - one is that some DolbyE decoder implementations does not support the s337m sync word to be the first word, A minimal guard band (full of zero) is required in such a case : 1 word is enough in the cases I experimented. One developer might find it useful to set S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject such files. - one other thing is that, currently, the detection is based on 3 consecutive samples, But there are other implementations in the wild. A common single-sample implementation is to simply require a sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake detection. (for 16 bits, this is really dangerous!; for 24 bits, I think it is fair but would still require some little additions to be 100% sure). 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 2/6] avformat/s337m: Consider container bit resolution
>> @@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void *avc, >> avpriv_report_missing_feature(avc, "Data type %#x in >> SMPTE 337M", data_type & 0x1F); >> return AVERROR_PATCHWELCOME; >> } >> + if (container_word_bits && >> + !(container_word_bits == 16 && word_bits == 16) && >> + !(container_word_bits == 24 && word_bits == 20) && > >I presume 20/24 is intentional. Does WAV not support signalling 20-bit? > >The rest looks OK enough > >/Tomas You are true, I meant "container_word_bits" as "block_size" rather than "valid bits per sample" and I think this should be clarified is the latter integration code in "wavdec: s337m support" patch where I use par->bits_per_coded_sample... But I should have rather coded "AV_CODEC_ID_PCM_S16LE ? 16 : 24" This is in case a wav file would be detected as 20 bits in a 24 bits container (I don't think it is supported yet, but it could as bitspersample and validbitpersample are two different fields in WAVE_FORMAT_EXTENSIBLE). Are you ok with 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".
[FFmpeg-devel] [PATCH 6/6] avformat/wavdec: Test s337m
Test s337m probing in wav container. Test dolby_e demuxing for 20 bits with program config '5.1+2'. --- tests/Makefile | 1 + tests/fate/audio.mak | 3 +++ tests/ref/fate/s337m-wav | 10 ++ 3 files changed, 14 insertions(+) create mode 100644 tests/ref/fate/s337m-wav diff --git a/tests/Makefile b/tests/Makefile index 1d50e1d175..d78f78d1b8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -99,6 +99,7 @@ REMUX = $(call ALLYES, $(firstword $(1))_MUXER $(lastword $(1))_DEMUXER \ $(2) FILE_PROTOCOL PIPE_PROTOCOL FRAMECRC_MUXER) DEMDEC = $(call ALLYES, $(1)_DEMUXER $(2:%=%_DECODER) $(3) FILE_PROTOCOL) +DEMDEMDEC = $(call ALLYES, $(1)_DEMUXER $(2)_DEMUXER $(3:%=%_DECODER)) ENCMUX = $(call ALLYES, $(1:%=%_ENCODER) $(2)_MUXER $(3)) FRAMEMD5 = $(call ALLYES, $(1)_DEMUXER $(2:%=%_DECODER) $(3) \ diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak index 65317c8d45..1d1c75b0b3 100644 --- a/tests/fate/audio.mak +++ b/tests/fate/audio.mak @@ -31,6 +31,9 @@ fate-dolby-e: CMD = pcm -i $(TARGET_SAMPLES)/dolby_e/16-11 fate-dolby-e: CMP = oneoff fate-dolby-e: REF = $(SAMPLES)/dolby_e/16-11.pcm +FATE_SAMPLES_AUDIO-$(call DEMDEMDEC, WAV, S337M, DOLBY_E) += fate-s337m-wav +fate-s337m-wav: CMD = framecrc -i $(TARGET_SAMPLES)/dolby_e/512.wav -vn -c:a copy + FATE_SAMPLES_AUDIO-$(call DEMDEC, DSS, DSS_SP) += fate-dss-lp fate-dss-sp fate-dss-lp: CMD = framecrc -i $(TARGET_SAMPLES)/dss/lp.dss -frames 30 -af aresample fate-dss-sp: CMD = framecrc -i $(TARGET_SAMPLES)/dss/sp.dss -frames 30 diff --git a/tests/ref/fate/s337m-wav b/tests/ref/fate/s337m-wav new file mode 100644 index 00..768a6f0161 --- /dev/null +++ b/tests/ref/fate/s337m-wav @@ -0,0 +1,10 @@ +#tb 0: 1/48000 +#media_type 0: audio +#codec_id 0: dolby_e +#sample_rate 0: 44800 +#channel_layout_name 0: 7.1 +0, 0, 0, 1920,11496, 0x05a9c147 +0, 1920, 1920, 1920,11496, 0x1d44d2b4 +0, 3840, 3840, 1920,11496, 0x4e078953 +0, 5760, 5760, 1920,11496, 0x1c73b1a1 +0, 7680, 7680, 1920,11262, 0xfa179fc8 -- 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 5/6] avformat/wavdec.c: Reindent after last commit
--- libavformat/wavdec.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index fd9ca89880..29192e48f0 100644 --- a/libavformat/wavdec.c +++ b/libavformat/wavdec.c @@ -793,14 +793,14 @@ smv_out: size = FFMIN(S337M_MAX_OFFSET, left); ret = ff_s337m_get_packet(s->pb, pkt, size, NULL, s, st->codecpar->bits_per_coded_sample); } else { -size = wav->max_size; -if (st->codecpar->block_align > 1) { -if (size < st->codecpar->block_align) -size = st->codecpar->block_align; -size = (size / st->codecpar->block_align) * st->codecpar->block_align; -} -size = FFMIN(size, left); -ret = av_get_packet(s->pb, pkt, size); +size = wav->max_size; +if (st->codecpar->block_align > 1) { +if (size < st->codecpar->block_align) +size = st->codecpar->block_align; +size = (size / st->codecpar->block_align) * st->codecpar->block_align; +} +size = FFMIN(size, left); +ret = av_get_packet(s->pb, pkt, size); } if (ret < 0) -- 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 4/6] avformat/wavdec: s337m support
Add s337m probing and demuxing similarly to spdif. Add 'non_pcm_mode' option to disable s337m demuxing (pass-through). --- libavformat/wavdec.c | 47 +++- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index e3f790fcc9..fd9ca89880 100644 --- a/libavformat/wavdec.c +++ b/libavformat/wavdec.c @@ -45,6 +45,7 @@ #include "riff.h" #include "w64.h" #include "spdif.h" +#include "s337m.h" typedef struct WAVDemuxContext { const AVClass *class; @@ -61,9 +62,11 @@ typedef struct WAVDemuxContext { int ignore_length; int max_size; int spdif; +int s337m; int smv_given_first; int unaligned; // e.g. if an odd number of bytes ID3 tag was prepended int rifx; // RIFX: integer byte order for parameters is big endian +int non_pcm_mode; } WAVDemuxContext; #define OFFSET(x) offsetof(WAVDemuxContext, x) @@ -74,12 +77,18 @@ static const AVOption demux_options[] = { { "ignore_length", "Ignore length", OFFSET(ignore_length), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC }, #endif { "max_size", "max size of single packet", OFFSET(max_size), AV_OPT_TYPE_INT, { .i64 = 4096 }, 1024, 1 << 22, DEC }, +#if CONFIG_S337M_DEMUXER +{"non_pcm_mode", "Chooses what to do with s337m", OFFSET(non_pcm_mode), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"}, +{"copy", "Pass s337m through unchanged", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 1, DEC, "non_pcm_mode"}, +{"demux" , "Demux s337m" , 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"}, +#endif { NULL }, }; -static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav) +static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) { -if (CONFIG_SPDIF_DEMUXER && s->streams[0]->codecpar->codec_tag == 1) { +AVCodecParameters *par = s->streams[0]->codecpar; +if ((CONFIG_SPDIF_DEMUXER || CONFIG_S337M_DEMUXER) && par->codec_tag == 1) { enum AVCodecID codec; int len = 1<<16; int ret = ffio_ensure_seekback(s->pb, len); @@ -92,10 +101,24 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav) int64_t pos = avio_tell(s->pb); len = ret = avio_read(s->pb, buf, len); if (len >= 0) { -ret = ff_spdif_probe(buf, len, ); -if (ret > AVPROBE_SCORE_EXTENSION) { -s->streams[0]->codecpar->codec_id = codec; -wav->spdif = 1; +if (CONFIG_SPDIF_DEMUXER) { +ret = ff_spdif_probe(buf, len, ); +if (ret > AVPROBE_SCORE_EXTENSION) { +par->codec_id = codec; +wav->spdif = 1; +} +} +if (CONFIG_S337M_DEMUXER && !wav->spdif +&& (par->codec_id == AV_CODEC_ID_PCM_S16LE || par->codec_id == AV_CODEC_ID_PCM_S24LE) && par->ch_layout.nb_channels == 2) { +ret = ff_s337m_probe(buf, len, , par->bits_per_coded_sample); +if (ret > AVPROBE_SCORE_EXTENSION) { +if (wav->non_pcm_mode) { +par->codec_id = codec; +wav->s337m = 1; +} else { +av_log(s, AV_LOG_INFO, "Passing through S337M data: codec will not be reported\n"); +} +} } } avio_seek(s->pb, pos, SEEK_SET); @@ -104,7 +127,7 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav) } if (ret < 0) -av_log(s, AV_LOG_WARNING, "Cannot check for SPDIF\n"); +av_log(s, AV_LOG_WARNING, "Cannot check for SPDIF/S337M\n"); } } @@ -668,7 +691,7 @@ break_loop: ff_metadata_conv_ctx(s, NULL, wav_metadata_conv); ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv); -set_spdif(s, wav); +set_spdif_s337m(s, wav); return 0; } @@ -766,6 +789,10 @@ smv_out: wav->data_end = avio_tell(s->pb) + left; } +if (CONFIG_S337M_DEMUXER && wav->s337m == 1) { +size = FFMIN(S337M_MAX_OFFSET, left); +ret = ff_s337m_get_packet(s->pb, pkt, size, NULL, s, st->codecpar->bits_per_coded_sample); +} else { size = wav->max_size; if (st->codecpar->block_align > 1) { if (size < st->codecpar->block_align) @@ -774,6 +801,8 @@ smv_out: } size = FFMIN(size, left); ret = av_get_packet(s->pb, pkt, size); +} + if (ret < 0) return ret; pkt->stream_index = 0; @@ -960,7 +989,7 @@ static int w64_read_header(AVFormatContext *s) avio_seek(pb, data_ofs, SEEK_SET); -set_spdif(s, wav); +set_spdif_s337m(s, wav);