[FFmpeg-devel] [PATCH] frame durations for framesync

[FFmpeg-devel] [PATCH] frame durations for framesync

[Paul B Mahol]

[-- Attachment #1: Type: text/plain, Size: 18 bytes --]

Patches attached.

[-- Attachment #2: 0001-avfilter-framesync-calculate-frame-duration-too.patch --]
[-- Type: text/x-patch, Size: 4500 bytes --]

From b4f835c4ef6e0e0bbe6adef8235381e56f3f91df Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Fri, 27 Jan 2023 23:34:02 +0100
Subject: [PATCH 1/4] avfilter/framesync: calculate frame duration too

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavfilter/framesync.c | 20 ++++++++++++++++----
 libavfilter/framesync.h | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index ee91e4cf68..71d67ca742 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -173,8 +173,10 @@ int ff_framesync_configure(FFFrameSync *fs)
                fs->time_base.num, fs->time_base.den);
     }
 
-    for (i = 0; i < fs->nb_in; i++)
+    for (i = 0; i < fs->nb_in; i++) {
         fs->in[i].pts = fs->in[i].pts_next = AV_NOPTS_VALUE;
+        fs->in[i].duration = fs->in[i].duration_next = 0;
+    }
     fs->sync_level = UINT_MAX;
     framesync_sync_level_update(fs);
 
@@ -184,7 +186,7 @@ int ff_framesync_configure(FFFrameSync *fs)
 static int framesync_advance(FFFrameSync *fs)
 {
     unsigned i;
-    int64_t pts;
+    int64_t pts, duration;
     int ret;
 
     while (!(fs->frame_ready || fs->eof)) {
@@ -193,9 +195,12 @@ static int framesync_advance(FFFrameSync *fs)
             return ret;
 
         pts = INT64_MAX;
+        duration = 0;
         for (i = 0; i < fs->nb_in; i++)
-            if (fs->in[i].have_next && fs->in[i].pts_next < pts)
+            if (fs->in[i].have_next && fs->in[i].pts_next < pts) {
                 pts = fs->in[i].pts_next;
+                duration = fs->in[i].duration_next;
+            }
         if (pts == INT64_MAX) {
             framesync_eof(fs);
             break;
@@ -211,8 +216,10 @@ static int framesync_advance(FFFrameSync *fs)
                 av_frame_free(&fs->in[i].frame);
                 fs->in[i].frame      = fs->in[i].frame_next;
                 fs->in[i].pts        = fs->in[i].pts_next;
+                fs->in[i].duration   = fs->in[i].duration_next;
                 fs->in[i].frame_next = NULL;
                 fs->in[i].pts_next   = AV_NOPTS_VALUE;
+                fs->in[i].duration_next = 0;
                 fs->in[i].have_next  = 0;
                 fs->in[i].state      = fs->in[i].frame ? STATE_RUN : STATE_EOF;
                 if (fs->in[i].sync == fs->sync_level && fs->in[i].frame)
@@ -228,6 +235,7 @@ static int framesync_advance(FFFrameSync *fs)
                      fs->in[i].before == EXT_STOP))
                     fs->frame_ready = 0;
         fs->pts = pts;
+        fs->duration = duration;
     }
     return 0;
 }
@@ -241,14 +249,17 @@ static int64_t framesync_pts_extrapolate(FFFrameSync *fs, unsigned in,
 
 static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame)
 {
-    int64_t pts;
+    int64_t pts, duration;
 
     av_assert0(!fs->in[in].have_next);
     av_assert0(frame);
     pts = av_rescale_q(frame->pts, fs->in[in].time_base, fs->time_base);
+    duration = av_rescale_q(frame->duration, fs->in[in].time_base, fs->time_base);
     frame->pts = pts;
+    frame->duration = duration;
     fs->in[in].frame_next = frame;
     fs->in[in].pts_next   = pts;
+    fs->in[in].duration_next = duration;
     fs->in[in].have_next  = 1;
 }
 
@@ -400,6 +411,7 @@ int ff_framesync_dualinput_get(FFFrameSync *fs, AVFrame **f0, AVFrame **f1)
     }
     av_assert0(mainpic);
     mainpic->pts = av_rescale_q(fs->pts, fs->time_base, ctx->outputs[0]->time_base);
+    mainpic->duration = av_rescale_q(fs->duration, fs->time_base, ctx->outputs[0]->time_base);
     if (ctx->is_disabled)
         secondpic = NULL;
     *f0 = mainpic;
diff --git a/libavfilter/framesync.h b/libavfilter/framesync.h
index 233f50a0eb..6a2866afc8 100644
--- a/libavfilter/framesync.h
+++ b/libavfilter/framesync.h
@@ -131,11 +131,21 @@ typedef struct FFFrameSyncIn {
      */
     int64_t pts;
 
+    /**
+     * duration of the current frame
+     */
+    int64_t duration;
+
     /**
      * PTS of the next frame, for internal use
      */
     int64_t pts_next;
 
+    /**
+     * duration of the next frame, for internal use
+     */
+    int64_t duration_next;
+
     /**
      * Boolean flagging the next frame, for internal use
      */
@@ -188,6 +198,11 @@ typedef struct FFFrameSync {
      */
     int64_t pts;
 
+    /**
+     * Duration of the current event
+     */
+    int64_t duration;
+
     /**
      * Callback called when a frame event is ready
      */
-- 
2.39.1


[-- Attachment #3: 0002-avfilter-vf_mix-set-output-frame-duration.patch --]
[-- Type: text/x-patch, Size: 1435 bytes --]

From b55f127bb87c50997b64e4e693ada41772a3e6c8 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Fri, 27 Jan 2023 23:35:02 +0100
Subject: [PATCH 2/4] avfilter/vf_mix: set output frame duration

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavfilter/vf_mix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavfilter/vf_mix.c b/libavfilter/vf_mix.c
index 64d07bbbbb..cf4c2d2a01 100644
--- a/libavfilter/vf_mix.c
+++ b/libavfilter/vf_mix.c
@@ -232,6 +232,7 @@ static int process_frame(FFFrameSync *fs)
         if (!out)
             return AVERROR(ENOMEM);
         out->pts = av_rescale_q(s->fs.pts, s->fs.time_base, outlink->time_base);
+        out->duration = av_rescale_q(s->fs.duration, s->fs.time_base, outlink->time_base);
         return ff_filter_frame(outlink, out);
     }
 
@@ -239,6 +240,7 @@ static int process_frame(FFFrameSync *fs)
     if (!out)
         return AVERROR(ENOMEM);
     out->pts = av_rescale_q(s->fs.pts, s->fs.time_base, outlink->time_base);
+    out->duration = av_rescale_q(s->fs.duration, s->fs.time_base, outlink->time_base);
 
     td.in = in;
     td.out = out;
@@ -438,6 +440,7 @@ static int tmix_filter_frame(AVFilterLink *inlink, AVFrame *in)
     if (!out)
         return AVERROR(ENOMEM);
     out->pts = s->frames[s->nb_frames - 1]->pts;
+    out->duration = s->frames[s->nb_frames - 1]->duration;
 
     td.out = out;
     td.in = s->frames;
-- 
2.39.1


[-- Attachment #4: 0003-avfilter-vf_mergeplanes-set-output-frame-duration.patch --]
[-- Type: text/x-patch, Size: 895 bytes --]

From 5d6278cd94c038d6a58d1271601e27e14551eba0 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Fri, 27 Jan 2023 23:36:34 +0100
Subject: [PATCH 3/4] avfilter/vf_mergeplanes: set output frame duration

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavfilter/vf_mergeplanes.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/vf_mergeplanes.c b/libavfilter/vf_mergeplanes.c
index 23b9f3d62f..d67e39a307 100644
--- a/libavfilter/vf_mergeplanes.c
+++ b/libavfilter/vf_mergeplanes.c
@@ -164,6 +164,7 @@ static int process_frame(FFFrameSync *fs)
     if (!out)
         return AVERROR(ENOMEM);
     out->pts = av_rescale_q(s->fs.pts, s->fs.time_base, outlink->time_base);
+    out->duration = av_rescale_q(s->fs.duration, s->fs.time_base, outlink->time_base);
 
     for (i = 0; i < s->nb_planes; i++) {
         const int input = s->map[i].input;
-- 
2.39.1


[-- Attachment #5: 0004-avfilter-vf_xmedian-set-output-frame-duration.patch --]
[-- Type: text/x-patch, Size: 1098 bytes --]

From 98ccb736ad5e3a4a65b8ea5840516cef47d3a9c5 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Fri, 27 Jan 2023 23:38:38 +0100
Subject: [PATCH 4/4] avfilter/vf_xmedian: set output frame duration

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavfilter/vf_xmedian.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_xmedian.c b/libavfilter/vf_xmedian.c
index 7f7f3de12a..1d11231e61 100644
--- a/libavfilter/vf_xmedian.c
+++ b/libavfilter/vf_xmedian.c
@@ -221,6 +221,7 @@ static int process_frame(FFFrameSync *fs)
     if (!out)
         return AVERROR(ENOMEM);
     out->pts = av_rescale_q(s->fs.pts, s->fs.time_base, outlink->time_base);
+    out->duration = av_rescale_q(s->fs.duration, s->fs.time_base, outlink->time_base);
 
     if (!ctx->is_disabled) {
         td.in = in;
@@ -426,6 +427,7 @@ static int tmedian_filter_frame(AVFilterLink *inlink, AVFrame *in)
     if (!out)
         return AVERROR(ENOMEM);
     out->pts = s->frames[0]->pts;
+    out->duration = s->frames[0]->duration;
 
     td.out = out;
     td.in = s->frames;
-- 
2.39.1


[-- Attachment #6: Type: text/plain, Size: 251 bytes --]

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Nicolas George]

Paul B Mahol (12023-01-27):
> From b4f835c4ef6e0e0bbe6adef8235381e56f3f91df Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda@gmail.com>
> Date: Fri, 27 Jan 2023 23:34:02 +0100
> Subject: [PATCH 1/4] avfilter/framesync: calculate frame duration too
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavfilter/framesync.c | 20 ++++++++++++++++----
>  libavfilter/framesync.h | 15 +++++++++++++++
>  2 files changed, 31 insertions(+), 4 deletions(-)

Looking into it right now, but I am pretty sure it cannot work.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Nicolas George]

Paul B Mahol (12023-01-27):
> From b4f835c4ef6e0e0bbe6adef8235381e56f3f91df Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda@gmail.com>
> Date: Fri, 27 Jan 2023 23:34:02 +0100
> Subject: [PATCH 1/4] avfilter/framesync: calculate frame duration too
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavfilter/framesync.c | 20 ++++++++++++++++----
>  libavfilter/framesync.h | 15 +++++++++++++++
>  2 files changed, 31 insertions(+), 4 deletions(-)

It does not work. For example, with the following filter graph:

testsrc2=r=1/3[a];testsrc2=r=1/5[b];[a][b]vstack

and adding a line of debug at the beginning of process_frame() in
vf_stack.c, you get:

got event: pts = 0, duration = 3
got event: pts = 3, duration = 3 ← should be 2
got event: pts = 5, duration = 5 ← should be 1
got event: pts = 6, duration = 3 …
got event: pts = 9, duration = 3
got event: pts = 10, duration = 5
got event: pts = 12, duration = 3
got event: pts = 15, duration = 3
got event: pts = 18, duration = 3
got event: pts = 20, duration = 5

I was wrong saying it cannot work, you can compute the duration of the
new frame without increasing the latency, but you have to duplicate the
whole frame synchronization logic.

I was against adding the duration field to libavfilter, and this is a
very good illustration of the reason. The duration field is redundant
with other information, it takes a lot of code to maintain it, and we
cannot trust it anyway.

For libavfilter, my advice would be to disregard it entirely. Let us
just clear it in buffersrc and be done with it.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Paul B Mahol]

On 1/30/23, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12023-01-27):
>> From b4f835c4ef6e0e0bbe6adef8235381e56f3f91df Mon Sep 17 00:00:00 2001
>> From: Paul B Mahol <onemda@gmail.com>
>> Date: Fri, 27 Jan 2023 23:34:02 +0100
>> Subject: [PATCH 1/4] avfilter/framesync: calculate frame duration too
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavfilter/framesync.c | 20 ++++++++++++++++----
>>  libavfilter/framesync.h | 15 +++++++++++++++
>>  2 files changed, 31 insertions(+), 4 deletions(-)
>
> It does not work. For example, with the following filter graph:
>
> testsrc2=r=1/3[a];testsrc2=r=1/5[b];[a][b]vstack
>
> and adding a line of debug at the beginning of process_frame() in
> vf_stack.c, you get:
>
> got event: pts = 0, duration = 3
> got event: pts = 3, duration = 3 ← should be 2
> got event: pts = 5, duration = 5 ← should be 1
> got event: pts = 6, duration = 3 …
> got event: pts = 9, duration = 3
> got event: pts = 10, duration = 5
> got event: pts = 12, duration = 3
> got event: pts = 15, duration = 3
> got event: pts = 18, duration = 3
> got event: pts = 20, duration = 5
>
> I was wrong saying it cannot work, you can compute the duration of the
> new frame without increasing the latency, but you have to duplicate the
> whole frame synchronization logic.
>
> I was against adding the duration field to libavfilter, and this is a
> very good illustration of the reason. The duration field is redundant
> with other information, it takes a lot of code to maintain it, and we
> cannot trust it anyway.

It is not redundant, it describes how much frame lasts.
There is no other way to derive it.

>
> For libavfilter, my advice would be to disregard it entirely. Let us
> just clear it in buffersrc and be done with it.
>

I think you do not understand why frame duration is needed.

> Regards,
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Nicolas George]

Paul B Mahol (12023-01-30):
> It is not redundant, it describes how much frame lasts.
> There is no other way to derive it.

The duration of a frame is equal to the difference between its timestamp
and the timestamp of the next frame.

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Paul B Mahol]

On 1/30/23, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12023-01-30):
>> It is not redundant, it describes how much frame lasts.
>> There is no other way to derive it.
>
> The duration of a frame is equal to the difference between its timestamp
> and the timestamp of the next frame.

Only in special cases.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Nicolas George]

Paul B Mahol (12023-01-30):
> > The duration of a frame is equal to the difference between its timestamp
> > and the timestamp of the next frame.
> Only in special cases.

No, in libavfilter, always; libavfilter does not support overlapping
frames nor gaps. And if you want to make a point, you will have to
explain your position with a lot more than four words.

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Paul B Mahol]

On 1/30/23, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12023-01-30):
>> > The duration of a frame is equal to the difference between its
>> > timestamp
>> > and the timestamp of the next frame.
>> Only in special cases.
>
> No, in libavfilter, always; libavfilter does not support overlapping
> frames nor gaps. And if you want to make a point, you will have to
> explain your position with a lot more than four words.

libavfilter support overlapping frames and frames with gaps, look into
aresample filter and its async options.

>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Nicolas George]

Paul B Mahol (12023-01-30):
> libavfilter support overlapping frames and frames with gaps, look into
> aresample filter and its async options.

aresample is a very special case. And for audio filters, the duration is
carried by nb_samples, not duration.

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

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

Re: [FFmpeg-devel] [PATCH] frame durations for framesync

[Paul B Mahol]

On 1/30/23, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12023-01-30):
>> libavfilter support overlapping frames and frames with gaps, look into
>> aresample filter and its async options.
>
> aresample is a very special case. And for audio filters, the duration is
> carried by nb_samples, not duration.

similar relates to video filters.

>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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