diff mbox

[FFmpeg-devel] avfilter/f_cue: use internal fifo for queueing frames

Message ID 20180930161913.14006-1-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint Sept. 30, 2018, 4:19 p.m. UTC 
Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/f_cue.c | 90 +++++++++++++++++++----------------------------------
 1 file changed, 32 insertions(+), 58 deletions(-)

Comments

Nicolas George Sept. 30, 2018, 4:42 p.m. UTC | #1 
Marton Balint (2018-09-30):
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavfilter/f_cue.c | 90 +++++++++++++++++++----------------------------------
>  1 file changed, 32 insertions(+), 58 deletions(-)
> 
> diff --git a/libavfilter/f_cue.c b/libavfilter/f_cue.c
> index 732b5e218a..b2fff050da 100644
> --- a/libavfilter/f_cue.c
> +++ b/libavfilter/f_cue.c
> @@ -18,11 +18,13 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#define FF_INTERNAL_FIELDS 1
> +#include "framequeue.h"

No.

I think I already told you: use the API in filters.h, the various
ff_inlink_* functions, rather than accessing the queue directly. Better
abstraction, less maintenance later.

Regards,
Marton Balint Sept. 30, 2018, 4:50 p.m. UTC | #2 
On Sun, 30 Sep 2018, Nicolas George wrote:

> Marton Balint (2018-09-30):
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavfilter/f_cue.c | 90 +++++++++++++++++++----------------------------------
>>  1 file changed, 32 insertions(+), 58 deletions(-)
>>
>> diff --git a/libavfilter/f_cue.c b/libavfilter/f_cue.c
>> index 732b5e218a..b2fff050da 100644
>> --- a/libavfilter/f_cue.c
>> +++ b/libavfilter/f_cue.c
>> @@ -18,11 +18,13 @@
>>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>   */
>>
>> +#define FF_INTERNAL_FIELDS 1
>> +#include "framequeue.h"
>
> No.
>
> I think I already told you: use the API in filters.h, the various
> ff_inlink_* functions, rather than accessing the queue directly. Better
> abstraction, less maintenance later.

With filters.h I can only consume the oldest frame, I have no way to 
query the timestamp of the most recent frame in the fifo. Please point me 
to the API I should use.

Thanks,
Marton
Nicolas George Sept. 30, 2018, 5:21 p.m. UTC | #3 
Marton Balint (2018-09-30):
> With filters.h I can only consume the oldest frame, I have no way to query
> the timestamp of the most recent frame in the fifo. Please point me to the
> API I should use.

If some utility functions you need are missing, do not hesitate to add
them. It looks like ff_inlink_queued_frames() and ff_inlink_peek_frame()
would be just what you need.

Regards,
diff mbox

Patch

diff --git a/libavfilter/f_cue.c b/libavfilter/f_cue.c
index 732b5e218a..b2fff050da 100644
--- a/libavfilter/f_cue.c
+++ b/libavfilter/f_cue.c
@@ -18,11 +18,13 @@ 
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#define FF_INTERNAL_FIELDS 1
+#include "framequeue.h"
+
 #include "libavutil/opt.h"
 #include "libavutil/time.h"
 #include "avfilter.h"
 #include "filters.h"
-#include "framequeue.h"
 #include "internal.h"
 
 typedef struct CueContext {
@@ -32,73 +34,49 @@  typedef struct CueContext {
     int64_t preroll;
     int64_t buffer;
     int status;
-    FFFrameQueue queue;
 } CueContext;
 
-static av_cold int init(AVFilterContext *ctx)
-{
-    CueContext *s = ctx->priv;
-    ff_framequeue_init(&s->queue, &ctx->graph->internal->frame_queues);
-    return 0;
-}
-
-static av_cold void uninit(AVFilterContext *ctx)
-{
-    CueContext *s = ctx->priv;
-    ff_framequeue_free(&s->queue);
-}
-
 static int activate(AVFilterContext *ctx)
 {
     AVFilterLink *inlink = ctx->inputs[0];
     AVFilterLink *outlink = ctx->outputs[0];
     CueContext *s = ctx->priv;
-    int64_t pts;
-    AVFrame *frame = NULL;
 
     FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
 
-    if (s->status < 3 || s->status == 5) {
-        int ret = ff_inlink_consume_frame(inlink, &frame);
-        if (ret < 0)
-            return ret;
-        if (frame)
-            pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q);
-    }
+    if (ff_framequeue_queued_frames(&inlink->fifo)) {
+        AVFrame *frame = ff_framequeue_peek(&inlink->fifo, 0);
+        int64_t pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q);
 
-    if (!s->status && frame) {
-        s->first_pts = pts;
-        s->status++;
-    }
-    if (s->status == 1 && frame) {
-        if (pts - s->first_pts < s->preroll)
-            return ff_filter_frame(outlink, frame);
-        s->first_pts = pts;
-        s->status++;
-    }
-    if (s->status == 2 && frame) {
-        int ret = ff_framequeue_add(&s->queue, frame);
-        if (ret < 0) {
-            av_frame_free(&frame);
-            return ret;
+        if (!s->status) {
+            s->first_pts = pts;
+            s->status++;
         }
-        frame = NULL;
-        if (!(pts - s->first_pts < s->buffer && (av_gettime() - s->cue) < 0))
+        if (s->status == 1) {
+            if (pts - s->first_pts < s->preroll) {
+                ff_inlink_consume_frame(inlink, &frame);
+                return ff_filter_frame(outlink, frame);
+            }
+            s->first_pts = pts;
             s->status++;
+        }
+        if (s->status == 2) {
+            frame = ff_framequeue_peek(&inlink->fifo, ff_framequeue_queued_frames(&inlink->fifo) - 1);
+            pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q);
+            if (!(pts - s->first_pts < s->buffer && (av_gettime() - s->cue) < 0))
+                s->status++;
+        }
+        if (s->status == 3) {
+            int64_t diff;
+            while ((diff = (av_gettime() - s->cue)) < 0)
+                av_usleep(av_clip(-diff / 2, 100, 1000000));
+            s->status++;
+        }
+        if (s->status == 4) {
+            ff_inlink_consume_frame(inlink, &frame);
+            return ff_filter_frame(outlink, frame);
+        }
     }
-    if (s->status == 3) {
-        int64_t diff;
-        while ((diff = (av_gettime() - s->cue)) < 0)
-            av_usleep(av_clip(-diff / 2, 100, 1000000));
-        s->status++;
-    }
-    if (s->status == 4) {
-        if (ff_framequeue_queued_frames(&s->queue))
-            return ff_filter_frame(outlink, ff_framequeue_take(&s->queue));
-        s->status++;
-    }
-    if (s->status == 5 && frame)
-        return ff_filter_frame(outlink, frame);
 
     FF_FILTER_FORWARD_STATUS(inlink, outlink);
     FF_FILTER_FORWARD_WANTED(outlink, inlink);
@@ -140,8 +118,6 @@  AVFilter ff_vf_cue = {
     .description = NULL_IF_CONFIG_SMALL("Delay filtering to match a cue."),
     .priv_size   = sizeof(CueContext),
     .priv_class  = &cue_class,
-    .init        = init,
-    .uninit      = uninit,
     .inputs      = cue_inputs,
     .outputs     = cue_outputs,
     .activate    = activate,
@@ -173,8 +149,6 @@  AVFilter ff_af_acue = {
     .description = NULL_IF_CONFIG_SMALL("Delay filtering to match a cue."),
     .priv_size   = sizeof(CueContext),
     .priv_class  = &acue_class,
-    .init        = init,
-    .uninit      = uninit,
     .inputs      = acue_inputs,
     .outputs     = acue_outputs,
     .activate    = activate,