[FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts

2024-10-04 Thread Nicolas Gaullier
Pinging again...
post rebased to current master, issue still present and fixed with the patch.

Nicolas Gaullier (1):
  avfilter/framesync: fix forward EOF pts

 libavfilter/framesync.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts

2024-10-04 Thread Nicolas Gaullier
Note1: when the EOF pts is not accurate enough, the last frame
can be dropped by vf_fps with default rounding.

Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3,
so this is a very commonplace scenario.

For example:
./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \
  -count_frames -show_entries stream=nb_read_frames

Before:
streams.stream.0.nb_read_frames="24"

After:
streams.stream.0.nb_read_frames="25"
---
 libavfilter/framesync.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index 8e06e0e700..0d5779f830 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext 
*parent, unsigned nb_in)
 return 0;
 }
 
-static void framesync_eof(FFFrameSync *fs)
+static void framesync_eof(FFFrameSync *fs, int64_t pts)
 {
 fs->eof = 1;
 fs->frame_ready = 0;
-ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE);
+ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts);
 }
 
-static void framesync_sync_level_update(FFFrameSync *fs)
+static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts)
 {
 unsigned i, level = 0;
 
@@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs)
 if (level)
 fs->sync_level = level;
 else
-framesync_eof(fs);
+framesync_eof(fs, eof_pts);
 }
 
 int ff_framesync_configure(FFFrameSync *fs)
@@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs)
 for (i = 0; i < fs->nb_in; i++)
 fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE;
 fs->sync_level = UINT_MAX;
-framesync_sync_level_update(fs);
+framesync_sync_level_update(fs, AV_NOPTS_VALUE);
 
 return 0;
 }
@@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs)
 if (fs->in[i].have_next && fs->in[i].pts_next < pts)
 pts = fs->in[i].pts_next;
 if (pts == INT64_MAX) {
-framesync_eof(fs);
+framesync_eof(fs, AV_NOPTS_VALUE);
 break;
 }
 for (i = 0; i < fs->nb_in; i++) {
@@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs)
 fs->frame_ready = 1;
 if (fs->in[i].state == STATE_EOF &&
 fs->in[i].after == EXT_STOP)
-framesync_eof(fs);
+framesync_eof(fs, AV_NOPTS_VALUE);
 }
 }
 if (fs->frame_ready)
@@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, 
unsigned in, AVFrame *frame)
 fs->in[in].have_next  = 1;
 }
 
-static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, 
int64_t pts)
+static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, 
int64_t eof_pts)
 {
 av_assert0(!fs->in[in].have_next);
-pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY
-? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts);
 fs->in[in].sync = 0;
-framesync_sync_level_update(fs);
+framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : 
AV_NOPTS_VALUE);
 fs->in[in].frame_next = NULL;
-fs->in[in].pts_next   = pts;
+fs->in[in].pts_next   = fs->in[in].state != STATE_RUN || fs->in[in].after 
== EXT_INFINITY
+? INT64_MAX : framesync_pts_extrapolate(fs, in, 
fs->in[in].pts);
 fs->in[in].have_next  = 1;
 }
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes

2024-09-13 Thread Nicolas Gaullier
>De : ffmpeg-devel  De la part de martin 
>schitter
>Envoyé : jeudi 12 septembre 2024 13:54
>On 12.09.24 13:14, Nicolas Gaullier wrote:
>> The message "Recognize and Ignore" does not make it clear what issue or 
>> grave defect is solved here.
>> I see in the code that fill items are currently recognized as dark metadata 
>> and ignored likewise, but I don't see any issue here.
>> Maybe could you comment a little bit about your intent ?
>
>While developing this DNxUncompressed code I always got lots of this "Dark key 
>..." log messages in the debug output. This kind of output wouldn't be a 
>surprise, if the key belongs to some rare and utterly irrelevant data >box. 
>but in this particular case the key stands for empty "fill" blocks, which are 
>frequently used in various places in MXF files. They are an elementary 
>building block of this container format.
>
>On the muxer side of ffmpegs MXF code 'fill' is known and used in many places, 
>but the demuxer doesn't recognize this element and just always prints these 
>warnings about something "unknown". That's highly irritating and >also 
>inefficient, because 'fill' is used quite frequently e.g. as place holder to 
>align frame data on 256byte boundaries etc.
>
>It's really trivial to fix and I don't see, why we should debate any longer 
>about this obvious flaw instead of just quickly solving the issue.
>
>But if you want, you can rewrite the wording to "Recognize 'fill' in MXF data 
>and suppress output" or whatever you like...

I am not against the idea of the patch: filler items should not be logged as 
dark metadata. I just wanted to check with you that it was indeed the only 
issue.
So it is not a big issue, and I find the patch confusing both because of the 
commit msg and code:
- the commit msg should not claim it "adds support for" filler items: mxf files 
with fillers are already supported/playable.
Maybe just a single line like "avformat/mxfdec: suppress verbose log for 
fillers" ?
- why not using a simple "if" on the av_log rather than inserting a new block 
of code ?

Thank you,
Nicolas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes

2024-09-12 Thread Nicolas Gaullier
>De : ffmpeg-devel  De la part de martin 
>schitter
>Envoyé : jeudi 12 septembre 2024 02:36
>
>But an elementary feature like 'fill' should be simply supported by any MXF 
>demuxer in a suitable manner, otherwise it's IMHO a grave defect.

>If you don't see any further objections, please just merge the patch and 
>finally solve this issue by this trivial solution till someone actually 
>contributes a better alternative.

The message "Recognize and Ignore" does not make it clear what issue or grave 
defect is solved here.
I see in the code that fill items are currently recognized as dark metadata and 
ignored likewise, but I don't see any issue here.
Maybe could you comment a little bit about your intent ?

Thanks
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts

2024-09-05 Thread Nicolas Gaullier
Rebased with no change.

Pinging again ?

Nicolas Gaullier (1):
  avfilter/framesync: fix forward EOF pts

 libavfilter/framesync.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts

2024-09-05 Thread Nicolas Gaullier
Note1: when the EOF pts is not accurate enough, the last frame
can be dropped by vf_fps with default rounding.

Note2: vf_scale use framesync since e82a3997cdd6c0894869b33ba42430ac3,
so this is a very commonplace scenario.

For example:
./ffprobe -f lavfi testsrc=d=1,scale,fps -of flat \
  -count_frames -show_entries stream=nb_read_frames

Before:
streams.stream.0.nb_read_frames="24"

After:
streams.stream.0.nb_read_frames="25"
---
 libavfilter/framesync.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index 8e06e0e700..0d5779f830 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -103,14 +103,14 @@ int ff_framesync_init(FFFrameSync *fs, AVFilterContext 
*parent, unsigned nb_in)
 return 0;
 }
 
-static void framesync_eof(FFFrameSync *fs)
+static void framesync_eof(FFFrameSync *fs, int64_t pts)
 {
 fs->eof = 1;
 fs->frame_ready = 0;
-ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE);
+ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, pts);
 }
 
-static void framesync_sync_level_update(FFFrameSync *fs)
+static void framesync_sync_level_update(FFFrameSync *fs, int64_t eof_pts)
 {
 unsigned i, level = 0;
 
@@ -131,7 +131,7 @@ static void framesync_sync_level_update(FFFrameSync *fs)
 if (level)
 fs->sync_level = level;
 else
-framesync_eof(fs);
+framesync_eof(fs, eof_pts);
 }
 
 int ff_framesync_configure(FFFrameSync *fs)
@@ -179,7 +179,7 @@ int ff_framesync_configure(FFFrameSync *fs)
 for (i = 0; i < fs->nb_in; i++)
 fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE;
 fs->sync_level = UINT_MAX;
-framesync_sync_level_update(fs);
+framesync_sync_level_update(fs, AV_NOPTS_VALUE);
 
 return 0;
 }
@@ -200,7 +200,7 @@ static int framesync_advance(FFFrameSync *fs)
 if (fs->in[i].have_next && fs->in[i].pts_next < pts)
 pts = fs->in[i].pts_next;
 if (pts == INT64_MAX) {
-framesync_eof(fs);
+framesync_eof(fs, AV_NOPTS_VALUE);
 break;
 }
 for (i = 0; i < fs->nb_in; i++) {
@@ -222,7 +222,7 @@ static int framesync_advance(FFFrameSync *fs)
 fs->frame_ready = 1;
 if (fs->in[i].state == STATE_EOF &&
 fs->in[i].after == EXT_STOP)
-framesync_eof(fs);
+framesync_eof(fs, AV_NOPTS_VALUE);
 }
 }
 if (fs->frame_ready)
@@ -255,15 +255,14 @@ static void framesync_inject_frame(FFFrameSync *fs, 
unsigned in, AVFrame *frame)
 fs->in[in].have_next  = 1;
 }
 
-static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, 
int64_t pts)
+static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, 
int64_t eof_pts)
 {
 av_assert0(!fs->in[in].have_next);
-pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY
-? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts);
 fs->in[in].sync = 0;
-framesync_sync_level_update(fs);
+framesync_sync_level_update(fs, status == AVERROR_EOF ? eof_pts : 
AV_NOPTS_VALUE);
 fs->in[in].frame_next = NULL;
-fs->in[in].pts_next   = pts;
+fs->in[in].pts_next   = fs->in[in].state != STATE_RUN || fs->in[in].after 
== EXT_INFINITY
+? INT64_MAX : framesync_pts_extrapolate(fs, in, 
fs->in[in].pts);
 fs->in[in].have_next  = 1;
 }
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 3/3] fftools/ffmpeg: Fix honor -r output option with streamcopy

2024-09-03 Thread Nicolas Gaullier
Fix "ost->st->avg_frame_rate = ost->frame_rate" in streamcopy_init()
being reset to input's frame rate a few lines below.
Note that in current code, there are some discrepancies amongst the
muxers. For example, avienc relies on time_base, so it is not affected
by this patch, whereas mxfenc and matroskaenc do use avg_frame_rate,
so this patch fixes -r being honored.

In the updated fate test, the input is (wrongly) probed as 50fps. With
this patch, the correct value (25fps) is successfully forced with -r.

Signed-off-by: Nicolas Gaullier 
---
 fftools/ffmpeg_mux_init.c | 1 -
 tests/ref/fate/time_base  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index e84fa9719f..37d626add6 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1012,7 +1012,6 @@ static int streamcopy_init(const Muxer *mux, OutputStream 
*ost, AVDictionary **e
 else
 sar = par->sample_aspect_ratio;
 ost->st->sample_aspect_ratio = par->sample_aspect_ratio = sar;
-ost->st->avg_frame_rate = ist->st->avg_frame_rate;
 ost->st->r_frame_rate = ist->st->r_frame_rate;
 break;
 }
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index 23875d1fb8..ae96232e60 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-b28d4ca13029fdc80a114b56467be9d7
+69ffc45e19ab070bc3e964d7b718fe53
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 2/3] tests: Remove void -time_base overrides when streamcopying to mxf

2024-09-03 Thread Nicolas Gaullier
Signed-off-by: Nicolas Gaullier 
---
 tests/fate/ffmpeg.mak | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 9e0c68da20..3ffaaeb295 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov 
$(TARGET_SAMPLES)/mov/fcp_export8-236.mov
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER 
PCM_S16LE_DECODER) += fate-copy-trac4914
 fate-copy-trac4914: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
-  mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 
1001/3"
+  mxf "-c:a pcm_s16le -af aresample -c:v copy"
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER 
ARESAMPLE_FILTER) += fate-copy-trac4914-avi
 fate-copy-trac4914-avi: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
@@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, 
SETTS_BSF PIPE_PROTOCOL) += f
 fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v 
copy -bsf:v 
"setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2"
 -fflags +bitexact
 
 FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, 
MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base
-fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf
+fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -fflags +bitexact -f mxf
 
 FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes)
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 1/3] avformat/mxfenc: Fix guess frame_rate

2024-09-03 Thread Nicolas Gaullier
The time_base was a bad guess.

Currently, fate-time_base test data assumed that overriding the input
time_base would affect the frame_rate, but this behaviour is not
documented, so just fix the fate data now that this is fixed.

Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0:
previously, when streamcopying, the time_base was guessed from the
frame_rate considering it is often constant, so guessing the frame_rate
back from the time_base was often not a problem.

To reproduce:
ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf

Signed-off-by: Nicolas Gaullier 
---
 libavformat/mxfenc.c | 8 ++--
 tests/ref/fate/time_base | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 4ac6a2d715..57be9e6ef6 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s)
 
 if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
 const AVPixFmtDescriptor *pix_desc = 
av_pix_fmt_desc_get(st->codecpar->format);
-// TODO: should be avg_frame_rate
-AVRational tbc = st->time_base;
+AVRational tbc = (AVRational){ 0, 0 };
+if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0)
+tbc = av_inv_q(st->avg_frame_rate);
+else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0)
+tbc = av_inv_q(st->r_frame_rate);
+
 // Default component depth to 8
 sc->component_depth = 8;
 sc->h_chroma_sub_sample = 2;
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index fd6cac53fc..23875d1fb8 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-d408aba82d62a90ed7f46a1999b014f1
+b28d4ca13029fdc80a114b56467be9d7
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 0/3] framerate / streamcopy

2024-09-03 Thread Nicolas Gaullier
This is the whole serie properly posted with the latest addition, the
third patch related to ffmpeg. This last patch is independent but
somewhat related, so maybe it is a good thing to review it to possibly
apply the complete serie as a whole.
(the first two patches seem ready to apply)

Nicolas Gaullier (3):
  avformat/mxfenc: Fix guess frame_rate
  tests: Remove void -time_base overrides when streamcopying to mxf
  fftools/ffmpeg: Fix honor -r output option with streamcopy

 fftools/ffmpeg_mux_init.c | 1 -
 libavformat/mxfenc.c  | 8 ++--
 tests/fate/ffmpeg.mak | 4 ++--
 tests/ref/fate/time_base  | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 3/3] fftools/ffmpeg: Fix honor -r output option with streamcopy

2024-09-02 Thread Nicolas Gaullier
Fix "ost->st->avg_frame_rate = ost->frame_rate" in streamcopy_init()
being reset to input's frame rate a few lines below.
Note that in current code, there are some discrepancies amongst the
muxers. For example, avienc relies on time_base, so it is not affected
by this patch, whereas mxfenc and matroskaenc do use avg_frame_rate,
so this patch fixes -r being honored.

In the update fate test, the input is wrongly probeb as 50fps. With
this patch, the correct value (25fps) is forced with -r.

Signed-off-by: Nicolas Gaullier 
---
 fftools/ffmpeg_mux_init.c | 1 -
 tests/ref/fate/time_base  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index e84fa9719f..37d626add6 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1012,7 +1012,6 @@ static int streamcopy_init(const Muxer *mux, OutputStream 
*ost, AVDictionary **e
 else
 sar = par->sample_aspect_ratio;
 ost->st->sample_aspect_ratio = par->sample_aspect_ratio = sar;
-ost->st->avg_frame_rate = ist->st->avg_frame_rate;
 ost->st->r_frame_rate = ist->st->r_frame_rate;
 break;
 }
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index 23875d1fb8..ae96232e60 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-b28d4ca13029fdc80a114b56467be9d7
+69ffc45e19ab070bc3e964d7b718fe53
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v3 0/2] avformat/mxfenc: Fix guess frame_rate

2024-09-02 Thread Nicolas Gaullier
>De : Anton Khirnov  
>Envoyé : lundi 2 septembre 2024 13:26
>
>Quoting Nicolas Gaullier (2024-09-02 11:52:15)
>> Following yesterday posts by Anton/Tomas on version 1.
>> 
>> v3:
>> - do not fall back to time_base (that was my v2)
>> - do not remove "-r 25" in the fate test with streamcopy (see below)
>> 
>> What comes out:
>> - mxfenc behaves the same as matroskaenc for example
>> - avienc behaves differently: still sticked to time_base only
>> 
>> --
>> 
>> Another issue on this: "-r" as an output option with streamcopy is not 
>> honored with mxf/matroska, but it is with avienc. This is because
>> streamcopy_init() only overrides time_base, not *_frame_rate.
>
>Output -r should set avg_frame_rate, see line 958 in ffmpeg_mux_init.c.

Yes, but this is reset a few lines below.
So, I will send a third patch in the same serie to address this; but maybe it 
can be discussed afterwards independently...
Except that issue,  do the patches for mxfenc look good to you ?

Thanks
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 2/2] tests: Remove void -time_base overrides when streamcopying to mxf

2024-09-02 Thread Nicolas Gaullier
Signed-off-by: Nicolas Gaullier 
---
 tests/fate/ffmpeg.mak | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 9e0c68da20..3ffaaeb295 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov 
$(TARGET_SAMPLES)/mov/fcp_export8-236.mov
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER 
PCM_S16LE_DECODER) += fate-copy-trac4914
 fate-copy-trac4914: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
-  mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 
1001/3"
+  mxf "-c:a pcm_s16le -af aresample -c:v copy"
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER 
ARESAMPLE_FILTER) += fate-copy-trac4914-avi
 fate-copy-trac4914-avi: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
@@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, 
SETTS_BSF PIPE_PROTOCOL) += f
 fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v 
copy -bsf:v 
"setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2"
 -fflags +bitexact
 
 FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, 
MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base
-fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf
+fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -fflags +bitexact -f mxf
 
 FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes)
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 1/2] avformat/mxfenc: Fix guess frame_rate

2024-09-02 Thread Nicolas Gaullier
The time_base was a bad guess.

Currently, fate-time_base test data assumed that overriding the input
time_base would affect the frame_rate, but this behaviour is not
documented, so just fix the fate data now that this is fixed.

Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0:
previously, when streamcopying, the time_base was guessed from the
frame_rate considering it is often constant, so guessing the frame_rate
back from the time_base was often not a problem.

To reproduce:
ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf

Signed-off-by: Nicolas Gaullier 
---
 libavformat/mxfenc.c | 8 ++--
 tests/ref/fate/time_base | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 4ac6a2d715..57be9e6ef6 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s)
 
 if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
 const AVPixFmtDescriptor *pix_desc = 
av_pix_fmt_desc_get(st->codecpar->format);
-// TODO: should be avg_frame_rate
-AVRational tbc = st->time_base;
+AVRational tbc = (AVRational){ 0, 0 };
+if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0)
+tbc = av_inv_q(st->avg_frame_rate);
+else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0)
+tbc = av_inv_q(st->r_frame_rate);
+
 // Default component depth to 8
 sc->component_depth = 8;
 sc->h_chroma_sub_sample = 2;
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index fd6cac53fc..23875d1fb8 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-d408aba82d62a90ed7f46a1999b014f1
+b28d4ca13029fdc80a114b56467be9d7
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 0/2] avformat/mxfenc: Fix guess frame_rate

2024-09-02 Thread Nicolas Gaullier
Following yesterday posts by Anton/Tomas on version 1.

v3:
- do not fall back to time_base (that was my v2)
- do not remove "-r 25" in the fate test with streamcopy (see below)

What comes out:
- mxfenc behaves the same as matroskaenc for example
- avienc behaves differently: still sticked to time_base only

--

Another issue on this: "-r" as an output option with streamcopy
is not honored with mxf/matroska, but it is with avienc. This is because
streamcopy_init() only overrides time_base, not *_frame_rate.

In my understanding:
- streamcopy_init() should override avg_frame_rate/r_frame_rate
- avienc should be fixed similarly to this patch for mxfenc

[Note that if streamcopy_init() is fixed, fate-time_base data will be
updated because currently the input frame_rate is (wrongly) probed as 50.]

Nicolas Gaullier (2):
  avformat/mxfenc: Fix guess frame_rate
  tests: Remove void -time_base overrides when streamcopying to mxf

 libavformat/mxfenc.c | 8 ++--
 tests/fate/ffmpeg.mak| 4 ++--
 tests/ref/fate/time_base | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/mxfenc: Fix guess frame_rate

2024-08-29 Thread Nicolas Gaullier
>De : ffmpeg-devel  De la part de Tomas Härdin
>Envoyé : lundi 26 août 2024 11:52
>
>This is probably fine for now, but it should be said that frame rate and 
>EditRate are not necessarily the same. We might want an explicit EditRate 
>option. But we can wait for users to actually request that feature
>
>/Tomas

It seems nobody objected, can this patch be applied ?
There is also a second patch in this serie for the corresponding fate tests 
"cleanup"; this is just in my mind to avoid confusing command lines arguments 
that don't take effect. Can you review it ?

Thank you
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 0/2] avformat/mxfenc: Fix guess frame_rate

2024-08-23 Thread Nicolas Gaullier
v2:
- keep time_base as default if frame_rate is undetermined
- fix mixed declaration and code


Nicolas Gaullier (2):
  avformat/mxfenc: Fix guess frame_rate
  tests: Remove void time_base overrides when streamcopying to mxf

 libavformat/mxfenc.c | 6 +-
 tests/fate/ffmpeg.mak| 4 ++--
 tests/ref/fate/time_base | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 2/2] tests: Remove void time_base overrides when streamcopying to mxf

2024-08-23 Thread Nicolas Gaullier
For fate-copy-trac4914, this is a revert of
10185e2d4c1e9839bc58a1d6a63c861677b13fd0.

Signed-off-by: Nicolas Gaullier 
---
 tests/fate/ffmpeg.mak | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 9e0c68da20..4f15356e0e 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov 
$(TARGET_SAMPLES)/mov/fcp_export8-236.mov
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER 
PCM_S16LE_DECODER) += fate-copy-trac4914
 fate-copy-trac4914: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
-  mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 
1001/3"
+  mxf "-c:a pcm_s16le -af aresample -c:v copy"
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER 
ARESAMPLE_FILTER) += fate-copy-trac4914-avi
 fate-copy-trac4914-avi: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
@@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, 
SETTS_BSF PIPE_PROTOCOL) += f
 fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v 
copy -bsf:v 
"setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2"
 -fflags +bitexact
 
 FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, 
MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base
-fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf
+fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -fflags +bitexact -f mxf
 
 FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes)
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 1/2] avformat/mxfenc: Fix guess frame_rate

2024-08-23 Thread Nicolas Gaullier
The time_base was a bad guess.

Currently, fate-time_base test data assumed that overriding the input
time_base would affect the frame_rate, but this behaviour is not
documented, so just fix the fate data now that this is fixed.

Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0:
previously, when streamcopying, the time_base was guessed from the
frame_rate considering it is often constant, so guessing the frame_rate
back from the time_base was often not a problem.

To reproduce:
ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf

Signed-off-by: Nicolas Gaullier 
---
 libavformat/mxfenc.c | 6 +-
 tests/ref/fate/time_base | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 4ac6a2d715..57f4c674f3 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2894,8 +2894,12 @@ static int mxf_init(AVFormatContext *s)
 
 if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
 const AVPixFmtDescriptor *pix_desc = 
av_pix_fmt_desc_get(st->codecpar->format);
-// TODO: should be avg_frame_rate
 AVRational tbc = st->time_base;
+if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0)
+tbc = av_inv_q(st->avg_frame_rate);
+else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0)
+tbc = av_inv_q(st->r_frame_rate);
+
 // Default component depth to 8
 sc->component_depth = 8;
 sc->h_chroma_sub_sample = 2;
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index fd6cac53fc..23875d1fb8 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-d408aba82d62a90ed7f46a1999b014f1
+b28d4ca13029fdc80a114b56467be9d7
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 2/2] tests: Remove void time_base overrides when streamcopying to mxf

2024-08-22 Thread Nicolas Gaullier
For fate-copy-trac4914, this is a revert of
10185e2d4c1e9839bc58a1d6a63c861677b13fd0.

Signed-off-by: Nicolas Gaullier 
---
 tests/fate/ffmpeg.mak | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 9e0c68da20..4f15356e0e 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -146,7 +146,7 @@ fate-copy-trac236: CMD = transcode mov 
$(TARGET_SAMPLES)/mov/fcp_export8-236.mov
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, MXF, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER MP2_DECODER ARESAMPLE_FILTER 
PCM_S16LE_DECODER) += fate-copy-trac4914
 fate-copy-trac4914: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
-  mxf "-c:a pcm_s16le -af aresample -c:v copy -time_base 
1001/3"
+  mxf "-c:a pcm_s16le -af aresample -c:v copy"
 
 FATE_STREAMCOPY-$(call TRANSCODE, RAWVIDEO MPEG2VIDEO, AVI, MPEGTS_DEMUXER 
MPEGVIDEO_PARSER MPEGAUDIO_PARSER EXTRACT_EXTRADATA_BSF MP2_DECODER 
ARESAMPLE_FILTER) += fate-copy-trac4914-avi
 fate-copy-trac4914-avi: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpeg2/xdcam8mp2-1s_small.ts\
@@ -220,7 +220,7 @@ FATE_SAMPLES_FFMPEG-$(call DEMMUX, APNG, FRAMECRC, 
SETTS_BSF PIPE_PROTOCOL) += f
 fate-ffmpeg-setts-bsf: CMD = framecrc -i $(TARGET_SAMPLES)/apng/clock.png -c:v 
copy -bsf:v 
"setts=duration=if(eq(NEXT_PTS\,NOPTS)\,PREV_OUTDURATION\,(NEXT_PTS-PTS)/2):ts=PTS/2"
 -fflags +bitexact
 
 FATE_TIME_BASE-$(call PARSERDEMDEC, MPEGVIDEO, MPEGPS, MPEG2VIDEO, 
MPEGVIDEO_DEMUXER MXF_MUXER) += fate-time_base
-fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -r 25 -time_base 1001:3 -fflags +bitexact -f mxf
+fate-time_base: CMD = md5 -i $(TARGET_SAMPLES)/mpeg2/dvd_single_frame.vob -an 
-sn -c:v copy -fflags +bitexact -f mxf
 
 FATE_SAMPLES_FFMPEG-yes += $(FATE_TIME_BASE-yes)
 
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/2] avformat/mxfenc: Fix guess frame_rate

2024-08-22 Thread Nicolas Gaullier
The input time_base was a bad guess.

Currently, fate-time_base test data assumed that overriding the input
time_base would affect the frame_rate, but this behaviour is not
documented, so just fix the fate data now that this is fixed.

Fix regression since 10185e2d4c1e9839bc58a1d6a63c861677b13fd0:
previously, when streamcopying, the time_base was guessed from the
frame_rate considering it is often constant, so guessing the frame_rate
back from the time_base was often not a problem.

To reproduce:
ffmpeg -i fate-suite/mpeg2/dvd_still_frame.vob -an -c copy out.mxf

Signed-off-by: Nicolas Gaullier 
---
 libavformat/mxfenc.c | 9 +++--
 tests/ref/fate/time_base | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 4ac6a2d715..a814f15609 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2894,8 +2894,13 @@ static int mxf_init(AVFormatContext *s)
 
 if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
 const AVPixFmtDescriptor *pix_desc = 
av_pix_fmt_desc_get(st->codecpar->format);
-// TODO: should be avg_frame_rate
-AVRational tbc = st->time_base;
+AVRational frame_rate = (AVRational){ 0, 1 };
+if (st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0)
+frame_rate = st->avg_frame_rate;
+else if (st->r_frame_rate.num > 0 && st->r_frame_rate.den > 0)
+frame_rate = st->r_frame_rate;
+AVRational tbc = av_inv_q(frame_rate);
+
 // Default component depth to 8
 sc->component_depth = 8;
 sc->h_chroma_sub_sample = 2;
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index fd6cac53fc..23875d1fb8 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-d408aba82d62a90ed7f46a1999b014f1
+b28d4ca13029fdc80a114b56467be9d7
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts

2024-07-08 Thread Nicolas Gaullier
>Envoyé : vendredi 14 juin 2024 13:27
>>Envoyé : lundi 3 juin 2024 12:00
>>>Envoyé : mardi 28 mai 2024 11:11
>>>
>>>This a new ping but I post the patch again to get fate cleanly completed on 
>>>patchwork.
>>>
>>https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939
>
>To summarize, and for remembering, the initial commit was 4e0e9ce2dc, 7 years 
>ago, and inserted this:
>ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE)
>The target of this patch is to make it a:
>ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, eof_pts)
>
>My use case is that I frequently chain the scale filter with the fps filter, 
>and since the scale filter uses framesync since e82a3997cdd6c0 (3rd of May 
>this year), this is becoming a "full" issue

Pinging again, the patch still applies...

Thank you
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts

2024-06-14 Thread Nicolas Gaullier
>Envoyé : lundi 3 juin 2024 12:00
>>Envoyé : mardi 28 mai 2024 11:11
>>
>>This a new ping but I post the patch again to get fate cleanly completed on 
>>patchwork.
>>BTW, the initial design of framesync/EOF was in n3.4-dev-1799-g4e0e9ce2dc, so 
>>one can say that time has past...
>>
>>Thank you in advance for the review.
>>Nicolas
>
>Another ping.
>Patch still applies
>https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939

Still applies to current master.
Anyone to review ?
To summarize, and for remembering, the initial commit was 4e0e9ce2dc, 7 years 
ago, and inserted this:
ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE)
The target of this patch is to make it a:
ff_outlink_set_status(fs->parent->outputs[0], AVERROR_EOF, eof_pts)

My use case is that I frequently chain the scale filter with the fps filter,
and since the scale filter use framesync since e82a3997cdd6c0 (3rd of May this 
year),
this is becoming a full issue (yet I have not find any corresponding trac entry 
so far).

Thanks
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 0/1] avfilter/framesync: fix forward EOF pts

2024-06-03 Thread Nicolas Gaullier
>Envoyé : mardi 28 mai 2024 11:11
>
>This a new ping but I post the patch again to get fate cleanly completed on 
>patchwork.
>BTW, the initial design of framesync/EOF was in n3.4-dev-1799-g4e0e9ce2dc, so 
>one can say that time has past...
>
>Thank you in advance for the review.
>Nicolas

Another ping.
Patch still applies
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=11939

Thank you
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/1] avfilter/framesync: fix forward EOF pts

2024-05-28 Thread Nicolas Gaullier
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

2024-05-28 Thread Nicolas Gaullier
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

2024-05-24 Thread Nicolas Gaullier
>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

2024-05-24 Thread Nicolas Gaullier
>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

2024-05-23 Thread Nicolas Gaullier
>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

2024-05-21 Thread Nicolas Gaullier
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

2024-04-29 Thread Nicolas Gaullier
>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

2024-04-22 Thread Nicolas Gaullier
>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

2024-04-04 Thread Nicolas Gaullier
Fixes a regression due to the fact that the colorspace filter does
not use the new API introduced by 8c7934f73ab6c568acaa.
The scale filter uses it since 45e09a30419cc2a7251e, and the setparams
filter since 3bf80df3ccd32aed23f0.

Example:
ffprobe -f lavfi yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=
bt470bg:colorspace=bt470bg,colorspace=bt709:range=tv,scale,showinfo

Before:
  color_range:unknown color_space:bt470bg ...
After:
  color_range:tv color_space:bt709 ...

Signed-off-by: Nicolas Gaullier 
---
 libavfilter/vf_colorspace.c | 62 +
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index d181e81ace..7bacd7892a 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -433,8 +433,7 @@ static int create_filtergraph(AVFilterContext *ctx,
 if (out->color_trc   != s->out_trc) s->out_txchr = NULL;
 if (in->colorspace   != s->in_csp ||
 in->color_range  != s->in_rng)  s->in_lumacoef   = NULL;
-if (out->colorspace  != s->out_csp ||
-out->color_range != s->out_rng) s->out_lumacoef  = NULL;
+if (out->color_range != s->out_rng) s->rgb2yuv   = NULL;
 
 if (!s->out_primaries || !s->in_primaries) {
 s->in_prm = in->color_primaries;
@@ -563,26 +562,8 @@ static int create_filtergraph(AVFilterContext *ctx,
 redo_yuv2rgb = 1;
 }
 
-if (!s->out_lumacoef) {
-s->out_csp = out->colorspace;
+if (!s->rgb2yuv) {
 s->out_rng = out->color_range;
-s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
-if (!s->out_lumacoef) {
-if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
-if (s->user_all == CS_UNSPECIFIED) {
-av_log(ctx, AV_LOG_ERROR,
-   "Please specify output colorspace\n");
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output color property %d\n", 
s->user_all);
-}
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output colorspace %d (%s)\n", s->out_csp,
-   av_color_space_name(s->out_csp));
-}
-return AVERROR(EINVAL);
-}
 redo_rgb2yuv = 1;
 }
 
@@ -687,6 +668,26 @@ static av_cold int init(AVFilterContext *ctx)
 {
 ColorSpaceContext *s = ctx->priv;
 
+s->out_csp  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
+  default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
+s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
+if (!s->out_lumacoef) {
+if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
+if (s->user_all == CS_UNSPECIFIED) {
+av_log(ctx, AV_LOG_ERROR,
+   "Please specify output colorspace\n");
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output color property %d\n", s->user_all);
+}
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output colorspace %d (%s)\n", s->out_csp,
+   av_color_space_name(s->out_csp));
+}
+return AVERROR(EINVAL);
+}
+
 ff_colorspacedsp_init(&s->dsp);
 
 return 0;
@@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 return res;
 }
 
+out->colorspace =  s->out_csp;
+out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ?
+   in->color_range : s->user_rng;
 out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ?
default_prm[FFMIN(s->user_all, CS_NB)] : 
s->user_prm;
 if (s->user_trc == AVCOL_TRC_UNSPECIFIED) {
@@ -746,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 } else {
 out->color_trc   = s->user_trc;
 }
-out->colorspace  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
-   default_csp[FFMIN(s->user_all, CS_NB)] : 
s->user_csp;
-out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ?
-   in->color_range : s->user_rng;
 if (rgb_sz != s->rgb_sz) {
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format);
 int uvw = in->width >> desc->log2_chroma_w;
@@ -841,8 +841,18 @@ static int query_formats(AVFilterContext *ctx)
 };
 int res;
 ColorSpaceContext *s = ctx->priv;
+AVFilterLink *outlink = ctx->outputs[0];
 AVFilterFormats *formats = ff_make_format_list(pix_fmts);
 
+

[FFmpeg-devel] [PATCH v4 0/1] avfilter/vf_colorspace: use colorspace negotiation API

2024-04-04 Thread Nicolas Gaullier
v4:
- remove dynamic color_range pass-through (which requires changing outlink 
dynamically and is forbidden)
- nits&coding style
- commit msg: simplified example (+ removed example for dynamic color_range 
pass-through)

Nicolas Gaullier (1):
  avfilter/vf_colorspace: use colorspace negotiation API

 libavfilter/vf_colorspace.c | 62 +
 1 file changed, 36 insertions(+), 26 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API

2024-04-04 Thread Nicolas Gaullier
>De : ffmpeg-devel  De la part de Niklas Haas
>Envoyé : jeudi 4 avril 2024 14:44
>
>> >> @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame 
>> >> *in)
>> >>  return res;
>> >>  }
>> >>  
>> >> +out->colorspace = s->out_csp;
>> >> +outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ? 
>> >> s->user_rng : in->color_range;
>> >> +out->color_range = outlink->color_range;
>> >
>> >Changing outlink dynamically like this seems not correct to me (what if the 
>> >next filter in the chain only supports one color range?). Changing the 
>> >range on the fly would at the minimum require reconfiguring the filter 
>> >graph, and >>possibly insertion of more auto-scale filters.
>> This is the kind of questioning I had when working on this issue. This seems 
>> very annoying and overly complex for a very uncommon scenario; is it even 
>> possible within the filter to ask for a reconfiguration of the filter graph ?
>
>No, reconfiguring the filter graph is (currently) the API user's 
>responsibility. (See fftools/ffmpeg_filter.c for an example)
>
>vf_buffersrc even warns you if you try and change colorspace properties 
>mid-stream, and for good reason - IMO there is no general expectation that 
>filters should be able to handle dynamically changing colorspace properties. 
>(This is >a feature, not a bug)
>
>There *are* some filters that handle dynamically changing input properties 
>(e.g. scale, zscale, libplacebo), but these are the exception rather than the 
>norm, and it's only because they're already written in a way that can 
>trivially >handle dynamic conversions.
>
>If it's difficult to do from within vf_colorspace, there's no need to do so. 
>Feel free to assume that frame->colorspace == inlink->colorspace == constant. 
>(Ditto color_range)

Thank you, this is pretty clear. I agree dynamic change of color range sounds 
weird and I am pleased it can be assumed constant.
In my patch, it means the problematic "outlink->color_range = xxx" you pointed 
at can be dropped. Nice.

>> 
>> >The logic that is used in the other YUV negotiation aware filters is to 
>> >just set `out->colorspace = outlink->colorspace` and `out->color_range = 
>> >outlink->color_range`, since the link properties are authoritative.
>> This is the kind of easy logic I tried at the beginning. Some water has 
>> flowed under the bridge, notably b89ee26539, but I just tried at the moment 
>> (with current code) an easy patch with just these two lines, and it still 
>> does not >work as "I" expected.
>> More specifically:
>> When running: ./ffprobe -f lavfi -i 
>> yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:color
>> space=bt470bg,colorspace=bt709:range=tv,scale,showinfo
>> At the entry of filter_frame(), the outlink values are incorrect:
>> colorspace = AVCOL_SPC_BT470BG
>> And color_range = AVCOL_RANGE_UNSPECIFIED So, this is why I introduced 
>> the negotiation for the colorspace to early set and persist this outlink 
>> property.
>> But for the range, I am bit confused with the documentation, it is not clear 
>> to me, but possibly it is expected to pass through? so it cannot be 
>> negotiated, so I am sticked if the outlink range cannot be changed 
>> dynamically...
>
>Note: passing through the range untouched *is* the default behavior (via 
>ff_default_query_formats). So I think the correct logic can be condensed into 
>just:
>
>if (out_rng != UNSPEC) {
>ret = set_common_color_ranges(make_singleton(out_rng));
>if (ret < 0)
>return ret;
>}
>
>That way, if the user passes unspec, it's guaranteed that the input range == 
>output_range (but with no preference for any specific range); but if the user 
>passes a specific range, both the input and output of the filter are forced to 
>be >this range.

Well, this is where I still not feel confident. Dynamic input range is not 
expected but somewhat still supported.
First thing: in my understanding, the colorspace filter is aimed at converting 
from any input range to the desired output range (if specified), and I think my 
patch is ok with the current 
"ff_formats_ref(ff_make_formats_list_singleton(s->user_rng), 
&outlink->incfg.color_ranges)". I don't think they are issues against it, so I 
will keep it that way unless you object.
Second thing: I understand the default behaviour is to pass through (I 
mean/guess dynamically) the range, but it is not what I experience. To test 
this, my patch serie includes a first patch to make setparams support timeline 
and here it is when running a "dynamic input range input":
ffmpeg -f lavfi -i yuvtestsrc -vf 
"setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg:range=unknown,
setparams=range=pc:enable='between(n,1,2)',
setparams=range=tv:enable='between(n,2,3)',
colorspace=bt709,scale,showinfo" -f null -frames 3 - 2>&1|awk "/color_/ {print 
\$4 \" \" \$5}"

Current master (solely patched for timeline support in setparams):
color_range:tv color_spa

Re: [FFmpeg-devel] [PATCH v3 2/2] avfilter/vf_colorspace: Use colorspace negotiation API

2024-04-04 Thread Nicolas Gaullier
>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

2024-04-04 Thread Nicolas Gaullier
>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

2024-04-04 Thread Nicolas Gaullier
>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

2024-04-02 Thread Nicolas Gaullier
Two issues affect accuracy of duration in estimate_timings_from_pts():
- pkt->duration typically reports the duration of a single audio frame,
whereas a pes often contain several audio frames
- for video, compute_frame_duration() use r_frame_rate which is not
reliable; typically, it is the duration of a single field for an
interlaced video using two field pictures.

Packet splitting/parsing is required to get accurate durations, so this
patch replaces ff_read_packet() calls by av_read_frame() calls.

Note that concatdec makes use of avformat_find_stream_info() to stitch
correctly the files, so it benefits from this patch (typically, overlap
is avoided).
e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream
duration is now longer than that of the video, which results in
concatdec joining on audio after the patch instead of joining on video
before that.

Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  36 ++--
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +-
 tests/ref/fate/ts-opus-demux  |   4 +-
 3 files changed, 100 insertions(+), 110 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index abfd5fee7d..f017bae094 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1808,12 +1808,12 @@ static void 
estimate_timings_from_bit_rate(AVFormatContext *ic)
 #define DURATION_DEFAULT_MAX_RETRY 6
 #define DURATION_MAX_RETRY 1
 
-/* only usable for MPEG-PS streams */
+/* only usable for MPEG-PS/TS streams */
 static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
 {
 FFFormatContext *const si = ffformatcontext(ic);
 AVPacket *const pkt = si->pkt;
-int num, den, read_size, ret;
+int read_size, ret;
 int64_t duration_max_read_size = ic->duration_probesize ? 
ic->duration_probesize >> DURATION_MAX_RETRY : DURATION_DEFAULT_MAX_READ_SIZE;
 int duration_max_retry = ic->duration_probesize ? DURATION_MAX_RETRY : 
DURATION_DEFAULT_MAX_RETRY;
 int found_duration = 0;
@@ -1821,9 +1821,6 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 int64_t filesize, offset, duration;
 int retry = 0;
 
-/* flush packet queue */
-ff_flush_packet_queue(ic);
-
 for (unsigned i = 0; i < ic->nb_streams; i++) {
 AVStream *const st  = ic->streams[i];
 FFStream *const sti = ffstream(st);
@@ -1834,10 +1831,13 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 av_log(ic, AV_LOG_WARNING,
"start time for stream %d is not set in 
estimate_timings_from_pts\n", i);
 
-if (sti->parser) {
-av_parser_close(sti->parser);
-sti->parser = NULL;
-}
+/* Demuxer context updates may occur, particularly while seeking in 
mpegts,
+ * and this could loose codec parameters in avctx,
+ * so preserve them in codecpar.
+ */
+if (sti->avctx_inited &&
+avcodec_parameters_from_context(st->codecpar, sti->avctx))
+goto skip_duration_calc;
 }
 
 if (ic->skip_estimate_duration_from_pts) {
@@ -1855,6 +1855,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (offset < 0)
 offset = 0;
 
+ff_read_frame_flush(ic);
 avio_seek(ic->pb, offset, SEEK_SET);
 read_size = 0;
 for (;;) {
@@ -1864,7 +1865,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 break;
 
 do {
-ret = ff_read_packet(ic, pkt);
+ret = av_read_frame(ic, pkt);
 } while (ret == AVERROR(EAGAIN));
 if (ret != 0)
 break;
@@ -1874,15 +1875,6 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (pkt->pts != AV_NOPTS_VALUE &&
 (st->start_time != AV_NOPTS_VALUE ||
  sti->first_dts != AV_NOPTS_VALUE)) {
-if (pkt->duration == 0) {
-compute_frame_duration(ic, &num, &den, st, sti->parser, 
pkt);
-if (den && num) {
-pkt->duration = av_rescale_rnd(1,
-   num * (int64_t) st->time_base.den,
-   den * (int64_t) st->time_base.num,
-   AV_ROUND_DOWN);
-}
-}
 duration = pkt->pts + pkt->duration;
 found_duration = 1;
 if (st->start_time != AV_NOPTS_VALUE)
@@ -1938,15 +1930,13 @@ skip_duration_calc:
 fill_all_stream_timings(ic);
 
 avio_seek(ic->pb, old_offset, SEEK_SET);
+
+ff_read_frame_flush(ic);
 fo

[FFmpeg-devel] [PATCH v3 0/1] avformat/demux: fix accurate probing of durations in mpegts/ps

2024-04-02 Thread Nicolas Gaullier
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

2024-04-02 Thread Nicolas Gaullier
Fixes a regression due to the fact that the colorspace filter does
not use the new API introduced by 8c7934f73ab6c568acaa.
The scale filter uses it since 45e09a30419cc2a7251e, and the setparams
filter since 3bf80df3ccd32aed23f0.

Example 1 - color_range specified:
ffmpeg -f lavfi -i yuvtestsrc -vf setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
,showinfo -f null -frames 1 -

Before:
  color_range:unknown color_space:bt470bg ...
After:
  color_range:tv color_space:bt709 ...

Example 2 - color_range pass-through:
ffmpeg -f lavfi -i yuvtestsrc -vf "setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg:range=unknown,
setparams=range=pc:enable='between(n,1,2)',
setparams=range=tv:enable='between(n,2,3)',
colorspace=bt709,scale,showinfo"
-f null -frames 3 - 2>&1|awk "/color_/ {print \$4 \" \" \$5}"

Before:
color_range:tv color_space:bt470bg
color_range:tv color_space:bt470bg
color_range:tv color_space:bt470bg
After:
color_range:unknown color_space:bt709
color_range:pc color_space:bt709
color_range:tv color_space:bt709

Signed-off-by: Nicolas Gaullier 
---
 libavfilter/vf_colorspace.c | 63 +
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index d181e81ace..12a571172b 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -433,8 +433,7 @@ static int create_filtergraph(AVFilterContext *ctx,
 if (out->color_trc   != s->out_trc) s->out_txchr = NULL;
 if (in->colorspace   != s->in_csp ||
 in->color_range  != s->in_rng)  s->in_lumacoef   = NULL;
-if (out->colorspace  != s->out_csp ||
-out->color_range != s->out_rng) s->out_lumacoef  = NULL;
+if (out->color_range != s->out_rng) s->rgb2yuv   = NULL;
 
 if (!s->out_primaries || !s->in_primaries) {
 s->in_prm = in->color_primaries;
@@ -563,26 +562,8 @@ static int create_filtergraph(AVFilterContext *ctx,
 redo_yuv2rgb = 1;
 }
 
-if (!s->out_lumacoef) {
-s->out_csp = out->colorspace;
+if (!s->rgb2yuv) {
 s->out_rng = out->color_range;
-s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
-if (!s->out_lumacoef) {
-if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
-if (s->user_all == CS_UNSPECIFIED) {
-av_log(ctx, AV_LOG_ERROR,
-   "Please specify output colorspace\n");
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output color property %d\n", 
s->user_all);
-}
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output colorspace %d (%s)\n", s->out_csp,
-   av_color_space_name(s->out_csp));
-}
-return AVERROR(EINVAL);
-}
 redo_rgb2yuv = 1;
 }
 
@@ -687,6 +668,26 @@ static av_cold int init(AVFilterContext *ctx)
 {
 ColorSpaceContext *s = ctx->priv;
 
+s->out_csp  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
+  default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
+s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
+if (!s->out_lumacoef) {
+if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
+if (s->user_all == CS_UNSPECIFIED) {
+av_log(ctx, AV_LOG_ERROR,
+   "Please specify output colorspace\n");
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output color property %d\n", s->user_all);
+}
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output colorspace %d (%s)\n", s->out_csp,
+   av_color_space_name(s->out_csp));
+}
+return AVERROR(EINVAL);
+}
+
 ff_colorspacedsp_init(&s->dsp);
 
 return 0;
@@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 return res;
 }
 
+out->colorspace = s->out_csp;
+outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ? 
s->user_rng : in->color_range;
+out->color_range = outlink->color_range;
 out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ?
default_prm[FFMIN(s->user_all, CS_NB)] : 
s->user_prm;
 if (s->user_trc == AVCOL_TRC_UNSPECIFIED) {
@@ -746,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 } else {
 out->color_trc   = s->user_trc;
 }
-out->colorspace  = s-

[FFmpeg-devel] [PATCH v3 1/2] avfilter/vf_setparams: Add timeline support

2024-04-02 Thread Nicolas Gaullier
This is helpful at least for test purposes.

Signed-off-by: Nicolas Gaullier 
---
 libavfilter/vf_setparams.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
index c96f4d314b..1b5eb70344 100644
--- a/libavfilter/vf_setparams.c
+++ b/libavfilter/vf_setparams.c
@@ -198,7 +198,7 @@ const AVFilter ff_vf_setparams = {
 .description = NULL_IF_CONFIG_SMALL("Force field, or color property for 
the output video frame."),
 .priv_size   = sizeof(SetParamsContext),
 .priv_class  = &setparams_class,
-.flags   = AVFILTER_FLAG_METADATA_ONLY,
+.flags   = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | 
AVFILTER_FLAG_METADATA_ONLY,
 FILTER_INPUTS(inputs),
 FILTER_OUTPUTS(ff_video_default_filterpad),
 FILTER_QUERY_FUNC(query_formats),
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3 0/2] avfilter/vf_colorspace: Use colorspace negotiation API

2024-04-02 Thread Nicolas Gaullier
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

2024-03-29 Thread Nicolas Gaullier
This is helpful at least for test purposes.
---
 libavfilter/vf_setparams.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
index c96f4d314b..1b5eb70344 100644
--- a/libavfilter/vf_setparams.c
+++ b/libavfilter/vf_setparams.c
@@ -198,7 +198,7 @@ const AVFilter ff_vf_setparams = {
 .description = NULL_IF_CONFIG_SMALL("Force field, or color property for 
the output video frame."),
 .priv_size   = sizeof(SetParamsContext),
 .priv_class  = &setparams_class,
-.flags   = AVFILTER_FLAG_METADATA_ONLY,
+.flags   = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | 
AVFILTER_FLAG_METADATA_ONLY,
 FILTER_INPUTS(inputs),
 FILTER_OUTPUTS(ff_video_default_filterpad),
 FILTER_QUERY_FUNC(query_formats),
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 0/2] avfilter/vf_colorspace: Use colorspace negotiation API

2024-03-29 Thread Nicolas Gaullier
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

2024-03-29 Thread Nicolas Gaullier
Fixes a regression due to the fact that the colorspace filter does
not use the new API introduced by 8c7934f73ab6c568acaa.
The scale filter uses it since 45e09a30419cc2a7251e, and the setparams
filter since 3bf80df3ccd32aed23f0.

Example 1 - color_range specified and chained with scale filter:
ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
,showinfo -f null -frames 1 -

Before:
  color_range:unknown color_space:bt470bg ...
After:
  color_range:tv color_space:bt709 ...

Example 2 - color_range pass-through:
ffmpeg -f lavfi -i testsrc -vf "setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg:range=unknown,
setparams=range=pc:enable='between(n,1,2)',
setparams=range=tv:enable='between(n,2,3)',
colorspace=bt709,showinfo"
-f null -frames 3 - 2>&1|awk "/color_/ {print \$4}"

Before:
color_range:tv
color_range:tv
color_range:tv
After:
color_range:unknown
color_range:pc
color_range:tv

Signed-off-by: Nicolas Gaullier 
---
 libavfilter/vf_colorspace.c | 64 +
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index f367ce17c6..a006fd5aac 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -432,8 +432,7 @@ static int create_filtergraph(AVFilterContext *ctx,
 if (out->color_trc   != s->out_trc) s->out_txchr = NULL;
 if (in->colorspace   != s->in_csp ||
 in->color_range  != s->in_rng)  s->in_lumacoef   = NULL;
-if (out->colorspace  != s->out_csp ||
-out->color_range != s->out_rng) s->out_lumacoef  = NULL;
+if (out->color_range != s->out_rng) s->rgb2yuv   = NULL;
 
 if (!s->out_primaries || !s->in_primaries) {
 s->in_prm = in->color_primaries;
@@ -562,26 +561,8 @@ static int create_filtergraph(AVFilterContext *ctx,
 redo_yuv2rgb = 1;
 }
 
-if (!s->out_lumacoef) {
-s->out_csp = out->colorspace;
+if (!s->rgb2yuv) {
 s->out_rng = out->color_range;
-s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
-if (!s->out_lumacoef) {
-if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
-if (s->user_all == CS_UNSPECIFIED) {
-av_log(ctx, AV_LOG_ERROR,
-   "Please specify output colorspace\n");
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output color property %d\n", 
s->user_all);
-}
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output colorspace %d (%s)\n", s->out_csp,
-   av_color_space_name(s->out_csp));
-}
-return AVERROR(EINVAL);
-}
 redo_rgb2yuv = 1;
 }
 
@@ -686,6 +667,26 @@ static av_cold int init(AVFilterContext *ctx)
 {
 ColorSpaceContext *s = ctx->priv;
 
+s->out_csp  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
+  default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
+s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
+if (!s->out_lumacoef) {
+if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
+if (s->user_all == CS_UNSPECIFIED) {
+av_log(ctx, AV_LOG_ERROR,
+   "Please specify output colorspace\n");
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output color property %d\n", s->user_all);
+}
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output colorspace %d (%s)\n", s->out_csp,
+   av_color_space_name(s->out_csp));
+}
+return AVERROR(EINVAL);
+}
+
 ff_colorspacedsp_init(&s->dsp);
 
 return 0;
@@ -734,6 +735,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 return res;
 }
 
+out->colorspace = outlink->colorspace;
+if (s->user_rng == AVCOL_RANGE_UNSPECIFIED)
+outlink->color_range = link->src->inputs[0]->color_range;
+out->color_range = outlink->color_range;
 out->color_primaries = s->user_prm == AVCOL_PRI_UNSPECIFIED ?
default_prm[FFMIN(s->user_all, CS_NB)] : 
s->user_prm;
 if (s->user_trc == AVCOL_TRC_UNSPECIFIED) {
@@ -745,10 +750,6 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 } else {
 out->color_trc   = s->user_trc;
 }
-out->colorspace  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
-   default_csp[FFMIN(

[FFmpeg-devel] [PATCH v6] avformat/demux: Add duration_probesize AVOption

2024-03-29 Thread Nicolas Gaullier
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

2024-03-28 Thread Nicolas Gaullier
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

2024-03-28 Thread Nicolas Gaullier
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

2024-03-28 Thread Nicolas Gaullier
>>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

2024-03-28 Thread Nicolas Gaullier
>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

2024-03-26 Thread Nicolas Gaullier
>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

2024-03-26 Thread 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 0/1] avcodec/h264_parser: fix start of packet for some broken

2024-03-26 Thread Nicolas Gaullier
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

2024-03-26 Thread Nicolas Gaullier
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

2024-03-26 Thread Nicolas Gaullier
Thanks to Stefano for the precise inspection, I addressed all the points.

The question about what is specific to mpeg remains, so I will try to elaborate 
on this.
I don't see how duration_probesize could be needed in any way beyond 
estimate_timings_from_pts().
And it seems there is no other headerless format like mpeg's, byte seekable but 
especially dts-muxed, cbr-stuffinged with, potentially, a high I/O physical 
audio/video delay - or not.
It seems the probing of mpeg streams is very specific currently and I don't 
think this situation will change in the future.
Take mp4 kind formats: strictly index-based, and even for a truncated file 
(with moov at the head), no I/O is required to get the actual truncated stream 
durations.
Take mxf kind formats that can be streamed, or simply truncated/broken: the 
editunits are quite consistent, carrying both audio&video, so recovering the 
duration would not be very tricky.

This v4 is "an experimental try" to lift the AVOption in the demuxer (which 
mean no API change). The problem is that it makes the AVOption belong to 
mpegts.c (I will focus on mpegts, I think it is the only real use case) but 
unused by it, so I don't feel it is acceptable?
Any input welcome.

Nicolas Gaullier (1):
  avformat/mpegts: Add duration_probesize AVOption

 doc/demuxers.texi |  13 +
 doc/formats.texi  |   2 +-
 libavformat/demux.c   |  23 ++---
 libavformat/mpegts.c  | 104 +---
 libavformat/mpegts.h  | 108 +-
 libavformat/version.h |   2 +-
 6 files changed, 140 insertions(+), 112 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] avfilter/vf_colorspace: use colorspace negotiation API

2024-03-25 Thread Nicolas Gaullier
Fixes a regression due to the fact that the colorspace filter does
not use the new API introduced by 8c7934f73ab6c568acaa.
The scale filter uses it since 45e09a30419cc2a7251e, and the setparams
filter since 3bf80df3ccd32aed23f0.

Example:
ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
,showinfo -f null -frames 1 -

Before:
  color_range:unknown color_space:unknown color_primaries:bt709 ...
After:
  color_range:tv color_space:bt709 color_primaries:bt709 ...

There is still an issue with the color_range when it is not specified:
the documentation states the output defaults to the same value as the
input, but it does not seem possible to implement currently.

Signed-off-by: Nicolas Gaullier 
---
 libavfilter/vf_colorspace.c | 62 +
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index f367ce17c6..b1e1b9b719 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -432,8 +432,7 @@ static int create_filtergraph(AVFilterContext *ctx,
 if (out->color_trc   != s->out_trc) s->out_txchr = NULL;
 if (in->colorspace   != s->in_csp ||
 in->color_range  != s->in_rng)  s->in_lumacoef   = NULL;
-if (out->colorspace  != s->out_csp ||
-out->color_range != s->out_rng) s->out_lumacoef  = NULL;
+if (out->color_range != s->out_rng) s->rgb2yuv   = NULL;
 
 if (!s->out_primaries || !s->in_primaries) {
 s->in_prm = in->color_primaries;
@@ -562,26 +561,8 @@ static int create_filtergraph(AVFilterContext *ctx,
 redo_yuv2rgb = 1;
 }
 
-if (!s->out_lumacoef) {
-s->out_csp = out->colorspace;
+if (!s->rgb2yuv) {
 s->out_rng = out->color_range;
-s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
-if (!s->out_lumacoef) {
-if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
-if (s->user_all == CS_UNSPECIFIED) {
-av_log(ctx, AV_LOG_ERROR,
-   "Please specify output colorspace\n");
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output color property %d\n", 
s->user_all);
-}
-} else {
-av_log(ctx, AV_LOG_ERROR,
-   "Unsupported output colorspace %d (%s)\n", s->out_csp,
-   av_color_space_name(s->out_csp));
-}
-return AVERROR(EINVAL);
-}
 redo_rgb2yuv = 1;
 }
 
@@ -686,6 +667,26 @@ static av_cold int init(AVFilterContext *ctx)
 {
 ColorSpaceContext *s = ctx->priv;
 
+s->out_csp  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
+  default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
+s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
+if (!s->out_lumacoef) {
+if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
+if (s->user_all == CS_UNSPECIFIED) {
+av_log(ctx, AV_LOG_ERROR,
+   "Please specify output colorspace\n");
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output color property %d\n", s->user_all);
+}
+} else {
+av_log(ctx, AV_LOG_ERROR,
+   "Unsupported output colorspace %d (%s)\n", s->out_csp,
+   av_color_space_name(s->out_csp));
+}
+return AVERROR(EINVAL);
+}
+
 ff_colorspacedsp_init(&s->dsp);
 
 return 0;
@@ -745,10 +746,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 } else {
 out->color_trc   = s->user_trc;
 }
-out->colorspace  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
-   default_csp[FFMIN(s->user_all, CS_NB)] : 
s->user_csp;
-out->color_range = s->user_rng == AVCOL_RANGE_UNSPECIFIED ?
-   in->color_range : s->user_rng;
+// FIXME color_range must be set in query_formats for negotiation.
+// The value updated here will not be propagated further in the graph.
+if (s->user_rng == AVCOL_RANGE_UNSPECIFIED)
+out->color_range = outlink->color_range = in->color_range;
 if (rgb_sz != s->rgb_sz) {
 const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format);
 int uvw = in->width >> desc->log2_chroma_w;
@@ -838,10 +839,19 @@ static int query_formats(AVFilterContext *ctx)
 AV_PIX_FMT_YUVJ420P,  AV_PIX_FMT_YUVJ422P,  AV_PIX_FMT_YUVJ444P,
 AV_PIX_FMT_NONE
 };
-int res;
+int res,

[FFmpeg-devel] [PATCH v2 1/1] avformat/demux: Fix accurate probing of durations in mpegts/ps

2024-03-18 Thread Nicolas Gaullier
Two issues affect accuracy of duration in estimate_timings_from_pts():
- pkt->duration typically reports the duration of a single audio frame,
whereas a pes often contain several audio frames
- for video, compute_frame_duration() use r_frame_rate which is not
reliable; typically, it is the duration of a single field for an
interlaced video using two field pictures.

Packet splitting/parsing is required to get accurate durations, so this
patch replaces ff_read_packet() calls by av_read_frame() calls.

Note that concatdec makes use of avformat_find_stream_info() to stitch
correctly the files, so it benefits from this patch (typically, overlap
is avoided).
e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream
duration is now longer than that of the video, which results in
concatdec joining on audio after the patch instead of joining on video
before that.

Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  36 ++--
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +-
 tests/ref/fate/ts-opus-demux  |   4 +-
 3 files changed, 100 insertions(+), 110 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 4c50eb5568..5b89eb71c9 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1820,20 +1820,17 @@ static void 
estimate_timings_from_bit_rate(AVFormatContext *ic)
 #define DURATION_MAX_READ_SIZE 25LL
 #define DURATION_MAX_RETRY 6
 
-/* only usable for MPEG-PS streams */
+/* only usable for MPEG-PS/TS streams */
 static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
 {
 FFFormatContext *const si = ffformatcontext(ic);
 AVPacket *const pkt = si->pkt;
-int num, den, read_size, ret;
+int read_size, ret;
 int found_duration = 0;
 int is_end;
 int64_t filesize, offset, duration;
 int retry = 0;
 
-/* flush packet queue */
-ff_flush_packet_queue(ic);
-
 for (unsigned i = 0; i < ic->nb_streams; i++) {
 AVStream *const st  = ic->streams[i];
 FFStream *const sti = ffstream(st);
@@ -1844,10 +1841,13 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 av_log(ic, AV_LOG_WARNING,
"start time for stream %d is not set in 
estimate_timings_from_pts\n", i);
 
-if (sti->parser) {
-av_parser_close(sti->parser);
-sti->parser = NULL;
-}
+/* Demuxer context updates may occur, particularly while seeking in 
mpegts,
+ * and this could loose codec parameters in avctx,
+ * so preserve them in codecpar.
+ */
+if (sti->avctx_inited &&
+avcodec_parameters_from_context(st->codecpar, sti->avctx))
+goto skip_duration_calc;
 }
 
 if (ic->skip_estimate_duration_from_pts) {
@@ -1865,6 +1865,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (offset < 0)
 offset = 0;
 
+ff_read_frame_flush(ic);
 avio_seek(ic->pb, offset, SEEK_SET);
 read_size = 0;
 for (;;) {
@@ -1874,7 +1875,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 break;
 
 do {
-ret = ff_read_packet(ic, pkt);
+ret = av_read_frame(ic, pkt);
 } while (ret == AVERROR(EAGAIN));
 if (ret != 0)
 break;
@@ -1884,15 +1885,6 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (pkt->pts != AV_NOPTS_VALUE &&
 (st->start_time != AV_NOPTS_VALUE ||
  sti->first_dts != AV_NOPTS_VALUE)) {
-if (pkt->duration == 0) {
-compute_frame_duration(ic, &num, &den, st, sti->parser, 
pkt);
-if (den && num) {
-pkt->duration = av_rescale_rnd(1,
-   num * (int64_t) st->time_base.den,
-   den * (int64_t) st->time_base.num,
-   AV_ROUND_DOWN);
-}
-}
 duration = pkt->pts + pkt->duration;
 found_duration = 1;
 if (st->start_time != AV_NOPTS_VALUE)
@@ -1948,15 +1940,13 @@ skip_duration_calc:
 fill_all_stream_timings(ic);
 
 avio_seek(ic->pb, old_offset, SEEK_SET);
+
+ff_read_frame_flush(ic);
 for (unsigned i = 0; i < ic->nb_streams; i++) {
 AVStream *const st  = ic->streams[i];
 FFStream *const sti = ffstream(st);
 
 sti->cur_dts = sti->first_dts;
-sti->last_IP_pts = AV_NOPTS_VALUE;
-sti->last_dts_for_order_check = AV_NOPTS_VALUE;
-for (int j = 0; j < MAX_REORDER_D

[FFmpeg-devel] [PATCH v2 0/1] avformat/demux: Fix accurate probing of durations in mpegts/ps

2024-03-18 Thread Nicolas Gaullier
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

2024-03-18 Thread Nicolas Gaullier
>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

2024-03-14 Thread Nicolas Gaullier
Two issues affect accuracy of duration in estimate_timings_from_pts():
- pkt->duration typically reports the duration of a single audio frame,
whereas a pes often contain several audio frames
- for video, compute_frame_duration() use r_frame_rate which is not
reliable; typically, it is the duration of a single field for an
interlaced video using two field pictures.

Packet splitting/parsing is required to get accurate durations, so this
patch replaces ff_read_packet() calls by av_read_frame() calls.

Note that concatdec makes use of avformat_find_stream_info() to stitch
correctly the files, so it benefits from this patch (typically, overlap
is avoided).
e.g. in fate/concat-demuxer-simple2-lavf-ts: the input audio stream
duration is now longer than that of the video, which results in
concatdec joining on audio after the patch instead of joining on video
before that.

Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  30 +---
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 170 +-
 tests/ref/fate/ts-opus-demux  |   4 +-
 3 files changed, 93 insertions(+), 111 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 4c50eb5568..7e75e7149c 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1820,20 +1820,17 @@ static void 
estimate_timings_from_bit_rate(AVFormatContext *ic)
 #define DURATION_MAX_READ_SIZE 25LL
 #define DURATION_MAX_RETRY 6
 
-/* only usable for MPEG-PS streams */
+/* only usable for MPEG-PS/TS streams */
 static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
 {
 FFFormatContext *const si = ffformatcontext(ic);
 AVPacket *const pkt = si->pkt;
-int num, den, read_size, ret;
+int read_size, ret;
 int found_duration = 0;
 int is_end;
 int64_t filesize, offset, duration;
 int retry = 0;
 
-/* flush packet queue */
-ff_flush_packet_queue(ic);
-
 for (unsigned i = 0; i < ic->nb_streams; i++) {
 AVStream *const st  = ic->streams[i];
 FFStream *const sti = ffstream(st);
@@ -1843,11 +1840,6 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 st->codecpar->codec_type != AVMEDIA_TYPE_UNKNOWN)
 av_log(ic, AV_LOG_WARNING,
"start time for stream %d is not set in 
estimate_timings_from_pts\n", i);
-
-if (sti->parser) {
-av_parser_close(sti->parser);
-sti->parser = NULL;
-}
 }
 
 if (ic->skip_estimate_duration_from_pts) {
@@ -1865,6 +1857,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (offset < 0)
 offset = 0;
 
+ff_read_frame_flush(ic);
 avio_seek(ic->pb, offset, SEEK_SET);
 read_size = 0;
 for (;;) {
@@ -1874,7 +1867,7 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 break;
 
 do {
-ret = ff_read_packet(ic, pkt);
+ret = av_read_frame(ic, pkt);
 } while (ret == AVERROR(EAGAIN));
 if (ret != 0)
 break;
@@ -1884,15 +1877,6 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 if (pkt->pts != AV_NOPTS_VALUE &&
 (st->start_time != AV_NOPTS_VALUE ||
  sti->first_dts != AV_NOPTS_VALUE)) {
-if (pkt->duration == 0) {
-compute_frame_duration(ic, &num, &den, st, sti->parser, 
pkt);
-if (den && num) {
-pkt->duration = av_rescale_rnd(1,
-   num * (int64_t) st->time_base.den,
-   den * (int64_t) st->time_base.num,
-   AV_ROUND_DOWN);
-}
-}
 duration = pkt->pts + pkt->duration;
 found_duration = 1;
 if (st->start_time != AV_NOPTS_VALUE)
@@ -1948,15 +1932,13 @@ skip_duration_calc:
 fill_all_stream_timings(ic);
 
 avio_seek(ic->pb, old_offset, SEEK_SET);
+
+ff_read_frame_flush(ic);
 for (unsigned i = 0; i < ic->nb_streams; i++) {
 AVStream *const st  = ic->streams[i];
 FFStream *const sti = ffstream(st);
 
 sti->cur_dts = sti->first_dts;
-sti->last_IP_pts = AV_NOPTS_VALUE;
-sti->last_dts_for_order_check = AV_NOPTS_VALUE;
-for (int j = 0; j < MAX_REORDER_DELAY + 1; j++)
-sti->pts_buffer[j] = AV_NOPTS_VALUE;
 }
 }
 
diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts 
b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
index 548cab01c6..86e5e6670f 100644
--- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts
+++ b/tests/ref

[FFmpeg-devel] [PATCH 0/1] avformat/demux: Fix accurate probing of durations in mpegts/ps

2024-03-14 Thread Nicolas Gaullier
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

2024-03-14 Thread Nicolas Gaullier
>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

2024-03-12 Thread Nicolas Gaullier
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

2024-03-12 Thread Nicolas Gaullier
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

2024-03-11 Thread Nicolas Gaullier
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

2024-03-11 Thread Nicolas Gaullier
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

2024-03-11 Thread Nicolas Gaullier
Mantissas are the last data in the channel subsegment and it appears it
is sometimes missing a very few bits for the parsing to complete.
This must not be confused with data corruption.
In standard conditions with certified products, it has been observed
that the occurence of this issue is pretty steady and about once every 2
hours. The truncation is at about 950 out of the 1024 values (923 is the
minimum I have seen so far).
The current code raises a severe 'Read past end' error and all data is
lost resulting in 20ms(@25fps) of silence for the affected channel.
This patch introduces a tolerance: if 800 out of the 1024 mantissas have
been parsed, a simple warning is raised and the data is preserved.

Signed-off-by: Nicolas Gaullier 
---
 libavcodec/dolby_e.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavcodec/dolby_e.c b/libavcodec/dolby_e.c
index 9c3f6006c2..1f8442e2e9 100644
--- a/libavcodec/dolby_e.c
+++ b/libavcodec/dolby_e.c
@@ -845,6 +845,7 @@ static int parse_indices(DBEContext *s, DBEChannel *c)
 return 0;
 }
 
+#define MIN_MANTISSAS 800
 static int parse_mantissas(DBEContext *s, DBEChannel *c)
 {
 DBEGroup *g;
@@ -885,6 +886,13 @@ static int parse_mantissas(DBEContext *s, DBEChannel *c)
 }
 }
 } else {
+if (i == c->nb_groups - 1
+&& count * size1 > get_bits_left(&s->gb)
+&& get_bits_left(&s->gb) >= 0
+&& (int)(mnt - c->mantissas) >= MIN_MANTISSAS) {
+av_log(s->avctx, AV_LOG_WARNING, "Truncated mantissas @%d, 
highest frequencies not recoverable\n", (int)(mnt - c->mantissas));
+break;
+}
 for (k = 0; k < count; k++)
 mnt[k] = get_sbits(&s->gb, size1) * scale;
 }
-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 0/1] avcodec/dolby_e: Add error recovery when parse_mantissas run out of bits

2024-03-11 Thread Nicolas Gaullier
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

2024-03-11 Thread Nicolas Gaullier
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

2024-03-11 Thread Nicolas Gaullier
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

2024-03-05 Thread Nicolas Gaullier
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

2024-03-05 Thread Nicolas Gaullier
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

2024-03-05 Thread Nicolas Gaullier
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

2024-03-05 Thread Nicolas Gaullier
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

2024-03-05 Thread Nicolas Gaullier
>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

2024-03-04 Thread Nicolas Gaullier
>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()

2024-03-04 Thread Nicolas Gaullier
Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  25 ++-
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +-
 tests/ref/fate/ts-demux   |   8 +-
 3 files changed, 106 insertions(+), 91 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 2e1d0ed66d..299e693606 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt,
 FFStream *const sti = ffstream(st);
 const uint8_t *data = pkt->data;
 int size = pkt->size;
-int ret = 0, got_output = flush;
+int ret = 0, got_output = flush, pkt_side_data_consumed = 0;
 
 if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) {
 // preserve 0-size sync packets
@@ -1231,10 +1231,21 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 if (pkt->side_data) {
-out_pkt->side_data   = pkt->side_data;
-out_pkt->side_data_elems = pkt->side_data_elems;
-pkt->side_data  = NULL;
-pkt->side_data_elems= 0;
+/* for the first iteration, side_data are simply moved to output.
+ * in case of additional iterations, they are duplicated each 
time. */
+if (!pkt_side_data_consumed) {
+pkt_side_data_consumed = 1;
+out_pkt->side_data   = pkt->side_data;
+out_pkt->side_data_elems = pkt->side_data_elems;
+} else for (int i = 0; i < pkt->side_data_elems; i++) {
+const AVPacketSideData *const src_sd = &pkt->side_data[i];
+uint8_t *dst_data = av_packet_new_side_data(out_pkt, 
src_sd->type, src_sd->size);
+if (!dst_data) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(dst_data, src_sd->data, src_sd->size);
+}
 }
 
 /* set the duration */
@@ -1286,6 +1297,10 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 fail:
+if (pkt_side_data_consumed) {
+pkt->side_data  = NULL;
+pkt->side_data_elems= 0;
+}
 if (ret < 0)
 av_packet_unref(out_pkt);
 av_packet_unref(pkt);
diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts 
b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
index 548cab01c6..ee49e331f3 100644
--- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts
+++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
@@ -7,19 +7,19 @@ 
video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea
 video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS 
Stream ID|224
 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS 
Stream ID|224
 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192
-audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__
-audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__
-audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__
-audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__
-audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__
-audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__
-audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__
-audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__
-audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__
-audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__
-audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__
-audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__
-audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__
+audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS 

[FFmpeg-devel] [PATCH v3 2/5] avcodec/parser: reindent after previous commit

2024-03-04 Thread Nicolas Gaullier
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

2024-03-04 Thread Nicolas Gaullier
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

2024-03-04 Thread Nicolas Gaullier
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

2024-03-04 Thread Nicolas Gaullier
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

2024-03-04 Thread Nicolas Gaullier
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()

2024-03-01 Thread Nicolas Gaullier
Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  23 ++-
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +-
 tests/ref/fate/ts-demux   |   8 +-
 3 files changed, 104 insertions(+), 91 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 2e1d0ed66d..722bb35c4c 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt,
 FFStream *const sti = ffstream(st);
 const uint8_t *data = pkt->data;
 int size = pkt->size;
-int ret = 0, got_output = flush;
+int ret = 0, got_output = flush, pkt_side_data_consumed = 0;
 
 if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) {
 // preserve 0-size sync packets
@@ -1231,10 +1231,19 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 if (pkt->side_data) {
-out_pkt->side_data   = pkt->side_data;
-out_pkt->side_data_elems = pkt->side_data_elems;
-pkt->side_data  = NULL;
-pkt->side_data_elems= 0;
+if (!pkt_side_data_consumed) {
+out_pkt->side_data   = pkt->side_data;
+out_pkt->side_data_elems = pkt->side_data_elems;
+pkt_side_data_consumed = 1;
+} else for (int i = 0; i < pkt->side_data_elems; i++) {
+const AVPacketSideData *const src_sd = &pkt->side_data[i];
+uint8_t *dst_data = av_packet_new_side_data(out_pkt, 
src_sd->type, src_sd->size);
+if (!dst_data) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(dst_data, src_sd->data, src_sd->size);
+}
 }
 
 /* set the duration */
@@ -1286,6 +1295,10 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 fail:
+if (pkt_side_data_consumed) {
+pkt->side_data  = NULL;
+pkt->side_data_elems= 0;
+}
 if (ret < 0)
 av_packet_unref(out_pkt);
 av_packet_unref(pkt);
diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts 
b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
index 548cab01c6..ee49e331f3 100644
--- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts
+++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
@@ -7,19 +7,19 @@ 
video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea
 video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS 
Stream ID|224
 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS 
Stream ID|224
 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192
-audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__
-audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__
-audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__
-audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__
-audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__
-audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__
-audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__
-audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__
-audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__
-audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__
-audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__
-audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__
-audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__
+audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
 video|1|29782|0.330911|26182|0.290911|3600|0.04|14098|124268|___|MPEGTS 
Stream ID|224
 video|1|33382|0.370911|29782|0.330911|3600|0.04000

[FFmpeg-devel] [PATCH v2 4/5] avcodec/h264_parser: fix start of packet for some broken streams

2024-03-01 Thread Nicolas Gaullier
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

2024-03-01 Thread Nicolas Gaullier
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

2024-03-01 Thread Nicolas Gaullier
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

2024-03-01 Thread Nicolas Gaullier
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

2024-03-01 Thread Nicolas Gaullier
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

2024-02-23 Thread Nicolas Gaullier
>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

2024-02-23 Thread 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".


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

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

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

Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 2/2] lavf/demux: duplicate side_data in parse_packet()

2024-02-20 Thread Nicolas Gaullier
Signed-off-by: Nicolas Gaullier 
---
 libavformat/demux.c   |  23 ++-
 tests/ref/fate/concat-demuxer-simple2-lavf-ts | 164 +-
 tests/ref/fate/ts-demux   |   8 +-
 3 files changed, 104 insertions(+), 91 deletions(-)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index 2e1d0ed66d..722bb35c4c 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1186,7 +1186,7 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt,
 FFStream *const sti = ffstream(st);
 const uint8_t *data = pkt->data;
 int size = pkt->size;
-int ret = 0, got_output = flush;
+int ret = 0, got_output = flush, pkt_side_data_consumed = 0;
 
 if (!size && !flush && sti->parser->flags & PARSER_FLAG_COMPLETE_FRAMES) {
 // preserve 0-size sync packets
@@ -1231,10 +1231,19 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 if (pkt->side_data) {
-out_pkt->side_data   = pkt->side_data;
-out_pkt->side_data_elems = pkt->side_data_elems;
-pkt->side_data  = NULL;
-pkt->side_data_elems= 0;
+if (!pkt_side_data_consumed) {
+out_pkt->side_data   = pkt->side_data;
+out_pkt->side_data_elems = pkt->side_data_elems;
+pkt_side_data_consumed = 1;
+} else for (int i = 0; i < pkt->side_data_elems; i++) {
+const AVPacketSideData *const src_sd = &pkt->side_data[i];
+uint8_t *dst_data = av_packet_new_side_data(out_pkt, 
src_sd->type, src_sd->size);
+if (!dst_data) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(dst_data, src_sd->data, src_sd->size);
+}
 }
 
 /* set the duration */
@@ -1286,6 +1295,10 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt,
 }
 
 fail:
+if (pkt_side_data_consumed) {
+pkt->side_data  = NULL;
+pkt->side_data_elems= 0;
+}
 if (ret < 0)
 av_packet_unref(out_pkt);
 av_packet_unref(pkt);
diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts 
b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
index 548cab01c6..ee49e331f3 100644
--- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts
+++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
@@ -7,19 +7,19 @@ 
video|1|18982|0.210911|15382|0.170911|3600|0.04|13092|84788|___|MPEGTS Strea
 video|1|22582|0.250911|18982|0.210911|3600|0.04|12755|98700|___|MPEGTS 
Stream ID|224
 video|1|26182|0.290911|22582|0.250911|3600|0.04|12023|111860|___|MPEGTS 
Stream ID|224
 audio|0|0|0.00|0|0.00|2351|0.026122|208|152844|K__|MPEGTS Stream ID|192
-audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__
-audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__
-audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__
-audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__
-audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__
-audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__
-audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__
-audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__
-audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__
-audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__
-audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__
-audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__
-audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__
+audio|0|2351|0.026122|2351|0.026122|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|4702|0.052244|4702|0.052244|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|7053|0.078367|7053|0.078367|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|9404|0.104489|9404|0.104489|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|11755|0.130611|11755|0.130611|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|14106|0.156733|14106|0.156733|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|16457|0.182856|16457|0.182856|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|18808|0.208978|18808|0.208978|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|21159|0.235100|21159|0.235100|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|23510|0.261222|23510|0.261222|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|25861|0.287344|25861|0.287344|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|28212|0.313467|28212|0.313467|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
+audio|0|30563|0.339589|30563|0.339589|2351|0.026122|209|N/A|K__|MPEGTS Stream 
ID|192
 video|1|29782|0.330911|26182|0.290911|3600|0.04|14098|124268|___|MPEGTS 
Stream ID|224
 video|1|33382|0.370911|29782|0.330911|3600|0.04000

[FFmpeg-devel] [PATCH 0/2] fix an mpegts scenario with unaligned pes

2024-02-20 Thread Nicolas Gaullier
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

2024-02-20 Thread Nicolas Gaullier
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

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

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

Again, sorry. There are some implementers who take the standards very crude to 
"optimize" for every little byte unreasonably.
Fact is, here, the standard should not have allowed this!
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


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

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

Ping ?
If you think it would be better to have a related trac ticket, just tell me, I 
can do this of course.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 0/1] avformat/demux: Add durationprobesize AVOption

2024-02-06 Thread Nicolas Gaullier
I posted "avformat/demux: Add more retries to get more stream durations"
last friday and it is a complementary patch.
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10654
Note that, if this "Add more retries" patch is accepted, I would update
this patch to set DURATION_MAX_RETRY_USER to the value of 
MORE_DURATIONS_MAX_RETRY (3),
which means that if the user ask for 8M, the first step will be to try 1M,
then 2M, then up to 8M until all A/V durations are found.
Currently, since there is only one extra retry to get all durations,
if the user ask for 8M, the first step is to try with 4M.

The default behaviour remains unchanged as exact&complete stream durations
is only required for somes use cases and/or some specific files.

Here is a sample file (mpegts @15Mb/s, with A/V pts cleanly cut at the end):
https://0x0.st/HkxN.ts

If it can help, I can add a trac issue or build a patchset with the two patches.

Nicolas Gaullier (1):
  avformat/demux: Add durationprobesize AVOption

 libavformat/avformat.h  |  8 
 libavformat/demux.c | 13 -
 libavformat/options_table.h |  1 +
 3 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.30.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/1] avformat/demux: Add durationprobesize AVOption

2024-02-06 Thread Nicolas Gaullier
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

2024-02-02 Thread Nicolas Gaullier
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".


  1   2   3   4   >