Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 0/2] avfilter: Add fsync filter
@ 2023-12-15  7:24 Thilo Borgmann via ffmpeg-devel
  2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 1/2] " Thilo Borgmann via ffmpeg-devel
  2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 2/2] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel
  0 siblings, 2 replies; 5+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-15  7:24 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Thilo Borgmann

Synchronize video frames with an external mapping from a file.
Follows up on the idea in https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-January/305986.html
implemented as a filter.

Not storing the frame map in a probably huge string but buffering
piece-wise. Using a fixed format string.

Thilo Borgmann (2):
  avfilter: Add fsync filter
  fate: Add fsync filter tests

 Changelog                        |   1 +
 MAINTAINERS                      |   1 +
 doc/filters.texi                 |  33 ++++
 libavfilter/Makefile             |   1 +
 libavfilter/allfilters.c         |   1 +
 libavfilter/version.h            |   2 +-
 libavfilter/vf_fsync.c           | 304 +++++++++++++++++++++++++++++++
 tests/Makefile                   |   6 +-
 tests/fate/filter-video.mak      |   6 +
 tests/maps/fsync-down            |   7 +
 tests/maps/fsync-up              |  57 ++++++
 tests/ref/fate/filter-fsync-down |  12 ++
 tests/ref/fate/filter-fsync-up   |  62 +++++++
 13 files changed, 491 insertions(+), 2 deletions(-)
 create mode 100644 libavfilter/vf_fsync.c
 create mode 100644 tests/maps/fsync-down
 create mode 100644 tests/maps/fsync-up
 create mode 100644 tests/ref/fate/filter-fsync-down
 create mode 100644 tests/ref/fate/filter-fsync-up

-- 
2.37.1 (Apple Git-137.1)

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [FFmpeg-devel] [PATCH 1/2] avfilter: Add fsync filter
  2023-12-15  7:24 [FFmpeg-devel] [PATCH 0/2] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
@ 2023-12-15  7:24 ` Thilo Borgmann via ffmpeg-devel
  2023-12-15 14:17   ` Andreas Rheinhardt
  2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 2/2] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel
  1 sibling, 1 reply; 5+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-15  7:24 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Thilo Borgmann

---
 Changelog                |   1 +
 MAINTAINERS              |   1 +
 doc/filters.texi         |  33 +++++
 libavfilter/Makefile     |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/version.h    |   2 +-
 libavfilter/vf_fsync.c   | 304 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 342 insertions(+), 1 deletion(-)
 create mode 100644 libavfilter/vf_fsync.c

diff --git a/Changelog b/Changelog
index 67ef92eb02..a25278d227 100644
--- a/Changelog
+++ b/Changelog
@@ -9,6 +9,7 @@ version <next>:
 - aap filter
 - demuxing, decoding, filtering, encoding, and muxing in the
   ffmpeg CLI now all run in parallel
+- fsync filter
 
 version 6.1:
 - libaribcaption decoder
diff --git a/MAINTAINERS b/MAINTAINERS
index 39b37ee0c5..4257fcad98 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -343,6 +343,7 @@ Filters:
   vf_delogo.c                           Jean Delvare (CC <jdelvare@suse.com>)
   vf_drawbox.c/drawgrid                 Andrey Utkin
   vf_extractplanes.c                    Paul B Mahol
+  vf_fsync.c                            Thilo Borgmann
   vf_histogram.c                        Paul B Mahol
   vf_hqx.c                              Clément Bœsch
   vf_idet.c                             Pascal Massimino
diff --git a/doc/filters.texi b/doc/filters.texi
index 6d00ba2c3f..9f19cba9df 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14681,6 +14681,39 @@ option may cause flicker since the B-Frames have often larger QP. Default is
 
 @end table
 
+@anchor{fsync}
+@section fsync
+
+Synchronize video frames with an external mapping from a file.
+
+For each input PTS given in the map file it either drops or creates as many frames as necessary to recreate the sequence of output frames given in the map file.
+
+This filter is useful to recreate the output frames of a framerate conversion by the @ref{fps} filter, recorded into a map file using the ffmpeg option @code{-stats_mux_pre}, and do further processing to the corresponding frames e.g. quality comparison.
+
+Each line of the map file must contain three items per input frame, the input PTS (decimal), the output PTS (decimal) and the output TIMEBASE (decimal/decimal), seperated by a space.
+This file format corresponds to the output of @code{-stats_mux_pre_fmt="@{ptsi@} @{pts@} @{tb@}"}.
+
+The filter assumes the map file is sorted by increasing input PTS.
+
+The filter accepts the following options:
+@table @option
+
+@item file, f
+The filename of the map file to be used.
+@end table
+
+Example:
+@example
+# Convert a video to 25 fps and record a MAP_FILE file with the default format of this filter
+ffmpeg -i INPUT -vf fps=fps=25 -stats_mux_pre MAP_FILE -stats_mux_pre_fmt "@{ptsi@} @{pts@} @{tb@}" OUTPUT
+
+# Sort MAP_FILE by increasing input PTS
+sort -n MAP_FILE
+
+# Use INPUT, OUTPUT and the MAP_FILE from above to compare the corresponding frames in INPUT and OUTPUT via SSIM
+ffmpeg -i INPUT -i OUTPUT -filter_complex '[0:v]fsync=file=MAP_FILE[ref];[1:v][ref]ssim' -f null -
+@end example
+
 @section gblur
 
 Apply Gaussian blur filter.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 63725f91b4..612616dfb4 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -323,6 +323,7 @@ OBJS-$(CONFIG_FREEZEDETECT_FILTER)           += vf_freezedetect.o
 OBJS-$(CONFIG_FREEZEFRAMES_FILTER)           += vf_freezeframes.o
 OBJS-$(CONFIG_FREI0R_FILTER)                 += vf_frei0r.o
 OBJS-$(CONFIG_FSPP_FILTER)                   += vf_fspp.o qp_table.o
+OBJS-$(CONFIG_FSYNC_FILTER)                  += vf_fsync.o
 OBJS-$(CONFIG_GBLUR_FILTER)                  += vf_gblur.o
 OBJS-$(CONFIG_GBLUR_VULKAN_FILTER)           += vf_gblur_vulkan.o vulkan.o vulkan_filter.o
 OBJS-$(CONFIG_GEQ_FILTER)                    += vf_geq.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index ed7c32be94..b32ffb2d71 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -299,6 +299,7 @@ extern const AVFilter ff_vf_freezedetect;
 extern const AVFilter ff_vf_freezeframes;
 extern const AVFilter ff_vf_frei0r;
 extern const AVFilter ff_vf_fspp;
+extern const AVFilter ff_vf_fsync;
 extern const AVFilter ff_vf_gblur;
 extern const AVFilter ff_vf_gblur_vulkan;
 extern const AVFilter ff_vf_geq;
diff --git a/libavfilter/version.h b/libavfilter/version.h
index 7642b670d1..59330858bd 100644
--- a/libavfilter/version.h
+++ b/libavfilter/version.h
@@ -31,7 +31,7 @@
 
 #include "version_major.h"
 
-#define LIBAVFILTER_VERSION_MINOR  14
+#define LIBAVFILTER_VERSION_MINOR  15
 #define LIBAVFILTER_VERSION_MICRO 100
 
 
diff --git a/libavfilter/vf_fsync.c b/libavfilter/vf_fsync.c
new file mode 100644
index 0000000000..3ce6f22d06
--- /dev/null
+++ b/libavfilter/vf_fsync.c
@@ -0,0 +1,304 @@
+/*
+ * Copyright (c) 2023 Thilo Borgmann <thilo.borgmann _at_ mail.de>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Filter for syncing video frames from external source
+ *
+ * @author Thilo Borgmann <thilo.borgmann _at_ mail.de>
+ */
+
+#include "libavutil/avstring.h"
+#include "libavutil/error.h"
+#include "libavutil/opt.h"
+#include "libavformat/avio.h"
+#include "video.h"
+#include "filters.h"
+
+#define BUF_SIZE 256
+
+typedef struct FsyncContext {
+    const AVClass *class;
+    AVIOContext *avio_ctx; // reading the map file
+    AVFrame *last_frame;   // buffering the last frame for duplicating eventually
+    char *filename;        // user-specified map file
+    char *buf;             // line buffer for the map file
+    char *cur;             // current position in the line buffer
+    char *end;             // end pointer of the line buffer
+    int64_t ptsi;          // input pts to map to [0-N] output pts
+    int64_t pts;           // output pts
+    int64_t tb_num;        // output timebase num
+    int64_t tb_den;        // output timebase den
+} FsyncContext;
+
+#define OFFSET(x) offsetof(FsyncContext, x)
+#define DEFINE_OPTIONS(filt_name, FLAGS)                                                                                        \
+static const AVOption filt_name##_options[] = {                                                                                 \
+    { "file",   "set the file name to use for frame sync", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = "" }, .flags=FLAGS }, \
+    { "f",      "set the file name to use for frame sync", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = "" }, .flags=FLAGS }, \
+    { NULL }                                                                                                                    \
+}
+
+// fills the buffer from cur to end, add \0 at EOF
+static int buf_fill(FsyncContext *ctx)
+{
+    int ret;
+    int num = ctx->end - ctx->cur;
+
+    ret = avio_read(ctx->avio_ctx, ctx->cur, num);
+    if (ret < 0)
+        return ret;
+    if (ret < num) {
+        *(ctx->cur + ret) = '\0';
+    }
+
+    return ret;
+}
+
+// copies cur to end to the beginning and fills the rest
+static int buf_reload(FsyncContext *ctx)
+{
+    int i, ret;
+    int num = ctx->end - ctx->cur;
+
+    for (i = 0; i < num; i++) {
+        ctx->buf[i] = *ctx->cur++;
+    }
+
+    ctx->cur = ctx->buf + i;
+    ret = buf_fill(ctx);
+    if (ret < 0)
+        return ret;
+    ctx->cur = ctx->buf;
+
+    return ret;
+}
+
+// skip from cur over eol
+static void buf_skip_eol(FsyncContext *ctx)
+{
+    char *i;
+    for (i = ctx->cur; i < ctx->end; i++) {
+        if (*i != '\n')// && *i != '\r')
+            break;
+    }
+    ctx->cur = i;
+}
+
+// get number of bytes from cur until eol
+static int buf_get_line_count(FsyncContext *ctx)
+{
+    int ret = 0;
+    char *i;
+    for (i = ctx->cur; i < ctx->end; i++, ret++) {
+        if (*i == '\0' || *i == '\n')
+            return ret;
+    }
+
+    return -1;
+}
+
+// get number of bytes from cur to '\0'
+static int buf_get_zero(FsyncContext *ctx)
+{
+    int ret = 0;
+    char *i;
+    for (i = ctx->cur; i < ctx->end; i++, ret++) {
+        if (*i == '\0')
+            return ret;
+    }
+
+    return ret;
+}
+
+static int activate(AVFilterContext *ctx)
+{
+    FsyncContext *s       = ctx->priv;
+    AVFilterLink *inlink  = ctx->inputs[0];
+    AVFilterLink *outlink = ctx->outputs[0];
+
+    int ret, line_count;
+    AVFrame *frame;
+
+    FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
+
+    buf_skip_eol(s);
+    line_count = buf_get_line_count(s);
+    if (line_count < 0) {
+        line_count = buf_reload(s);
+        if (line_count < 0)
+            return line_count;
+        line_count = buf_get_line_count(s);
+        if (line_count < 0)
+            return line_count;
+    }
+
+    if (avio_feof(s->avio_ctx) && buf_get_zero(s) < 3) {
+        av_log(ctx, AV_LOG_DEBUG, "End of file. To zero = %i\n", buf_get_zero(s));
+        goto end;
+    }
+
+    if (s->last_frame) {
+        ret = av_sscanf(s->cur, "%"PRId64" %"PRId64" %d/%d", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
+        if (ret != 4) {
+            av_log(ctx, AV_LOG_ERROR, "Unexpected format found (%i / 4).\n", ret);
+            ff_outlink_set_status(outlink, AVERROR_INVALIDDATA, AV_NOPTS_VALUE);
+            return AVERROR_INVALIDDATA;
+        }
+
+        av_log(ctx, AV_LOG_DEBUG, "frame %lli ", s->last_frame->pts);
+
+        if (s->last_frame->pts >= s->ptsi) {
+            av_log(ctx, AV_LOG_DEBUG, ">= %lli: DUP LAST with pts = %lli\n", s->ptsi, s->pts);
+
+            // clone frame
+            frame = av_frame_clone(s->last_frame);
+            if (!frame) {
+                ff_outlink_set_status(outlink, AVERROR(ENOMEM), AV_NOPTS_VALUE);
+                return AVERROR(ENOMEM);
+            }
+            av_frame_copy_props(frame, s->last_frame);
+
+            // set output pts and timebase
+            frame->pts = s->pts;
+            frame->time_base = av_make_q((int)s->tb_num, (int)s->tb_den);
+
+            // advance cur to eol, skip over eol in the next call
+            s->cur += line_count;
+
+            // call again
+            if (ff_inoutlink_check_flow(inlink, outlink))
+                ff_filter_set_ready(ctx, 100);
+
+            // filter frame
+            return ff_filter_frame(outlink, frame);
+        } else if (s->last_frame->pts < s->ptsi) {
+            av_log(ctx, AV_LOG_DEBUG, "<  %lli: DROP\n", s->ptsi);
+            av_frame_free(&s->last_frame);
+
+            // call again
+            if (ff_inoutlink_check_flow(inlink, outlink))
+                ff_filter_set_ready(ctx, 100);
+
+            return 0;
+        }
+    }
+
+end:
+    ret = ff_inlink_consume_frame(inlink, &s->last_frame);
+    if (ret < 0)
+        return ret;
+
+    FF_FILTER_FORWARD_STATUS(inlink, outlink);
+    FF_FILTER_FORWARD_WANTED(outlink, inlink);
+
+    return FFERROR_NOT_READY;
+}
+
+static int fsync_config_props(AVFilterLink* outlink)
+{
+    AVFilterContext *ctx = outlink->src;
+    FsyncContext    *s   = ctx->priv;
+    int ret;
+
+    // read first line to get output timebase
+    ret = av_sscanf(s->cur, "%"PRId64" %"PRId64" %d/%d", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
+    if (ret != 4) {
+        av_log(ctx, AV_LOG_ERROR, "Unexpected format found (%i of 4).\n", ret);
+        ff_outlink_set_status(outlink, AVERROR_INVALIDDATA, AV_NOPTS_VALUE);
+        return AVERROR_INVALIDDATA;
+    }
+
+    outlink->frame_rate = av_make_q(1, 0); // unknown or dynamic
+    outlink->time_base  = av_make_q((int)s->tb_num, (int)s->tb_den);
+
+    return 0;
+}
+
+static av_cold int fsync_init(AVFilterContext *ctx)
+{
+    FsyncContext *s = ctx->priv;
+    int ret;
+
+    av_log(ctx, AV_LOG_DEBUG, "filename: %s\n", s->filename);
+
+    s->buf = av_malloc(BUF_SIZE);
+    if (!s->buf)
+        return AVERROR(ENOMEM);
+
+    ret = avio_open(&s->avio_ctx, s->filename, AVIO_FLAG_READ);
+    if (ret < 0)
+        return ret;
+
+    s->cur = s->buf;
+    s->end = s->buf + BUF_SIZE;
+
+    ret = buf_fill(s);
+    if (ret < 0)
+        return ret;
+
+    return 0;
+}
+
+static av_cold void fsync_uninit(AVFilterContext *ctx)
+{
+    FsyncContext *s = ctx->priv;
+
+    avio_close(s->avio_ctx);
+    av_freep(&s->buf);
+    av_frame_unref(s->last_frame);
+}
+
+DEFINE_OPTIONS(fsync, AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM);
+AVFILTER_DEFINE_CLASS(fsync);
+
+static const enum AVPixelFormat pix_fmts[] = {
+    AV_PIX_FMT_GRAY8,
+    AV_PIX_FMT_GBRP,     AV_PIX_FMT_GBRAP,
+    AV_PIX_FMT_YUV422P,  AV_PIX_FMT_YUV420P,
+    AV_PIX_FMT_YUV444P,  AV_PIX_FMT_YUV440P,
+    AV_PIX_FMT_YUV411P,  AV_PIX_FMT_YUV410P,
+    AV_PIX_FMT_YUVJ440P, AV_PIX_FMT_YUVJ411P, AV_PIX_FMT_YUVJ420P,
+    AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
+    AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVA422P, AV_PIX_FMT_YUVA420P,
+    AV_PIX_FMT_NONE
+};
+
+static const AVFilterPad avfilter_vf_fsync_outputs[] = {
+    {
+        .name          = "default",
+        .type          = AVMEDIA_TYPE_VIDEO,
+        .config_props  = fsync_config_props,
+    },
+};
+
+const AVFilter ff_vf_fsync = {
+    .name          = "fsync",
+    .description   = NULL_IF_CONFIG_SMALL("Synchronize video frames from external source."),
+    .init          = fsync_init,
+    .uninit        = fsync_uninit,
+    .priv_size     = sizeof(FsyncContext),
+    .priv_class    = &fsync_class,
+    .activate      = activate,
+    FILTER_PIXFMTS_ARRAY(pix_fmts),
+    FILTER_INPUTS(ff_video_default_filterpad),
+    FILTER_OUTPUTS(avfilter_vf_fsync_outputs),
+    .flags         = AVFILTER_FLAG_METADATA_ONLY,
+};
-- 
2.37.1 (Apple Git-137.1)

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] fate: Add fsync filter tests
  2023-12-15  7:24 [FFmpeg-devel] [PATCH 0/2] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
  2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 1/2] " Thilo Borgmann via ffmpeg-devel
@ 2023-12-15  7:24 ` Thilo Borgmann via ffmpeg-devel
  1 sibling, 0 replies; 5+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-15  7:24 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Thilo Borgmann

---
 tests/Makefile                   |  6 +++-
 tests/fate/filter-video.mak      |  6 ++++
 tests/maps/fsync-down            |  7 ++++
 tests/maps/fsync-up              | 57 +++++++++++++++++++++++++++++
 tests/ref/fate/filter-fsync-down | 12 +++++++
 tests/ref/fate/filter-fsync-up   | 62 ++++++++++++++++++++++++++++++++
 6 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 tests/maps/fsync-down
 create mode 100644 tests/maps/fsync-up
 create mode 100644 tests/ref/fate/filter-fsync-down
 create mode 100644 tests/ref/fate/filter-fsync-up

diff --git a/tests/Makefile b/tests/Makefile
index 444c09b3de..c7892a9313 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -23,7 +23,7 @@ FFMPEG=ffmpeg$(PROGSSUF)$(EXESUF)
 $(AREF): CMP=
 
 APITESTSDIR := tests/api
-FATE_OUTDIRS = tests/data tests/data/fate tests/data/filtergraphs tests/data/lavf tests/data/lavf-fate tests/data/pixfmt tests/vsynth1 $(APITESTSDIR)
+FATE_OUTDIRS = tests/data tests/data/fate tests/data/filtergraphs tests/data/maps tests/data/lavf tests/data/lavf-fate tests/data/pixfmt tests/vsynth1 $(APITESTSDIR)
 OUTDIRS += $(FATE_OUTDIRS)
 
 $(VREF): tests/videogen$(HOSTEXESUF) | tests/vsynth1
@@ -66,6 +66,10 @@ tests/data/filtergraphs/%: TAG = COPY
 tests/data/filtergraphs/%: $(SRC_PATH)/tests/filtergraphs/% | tests/data/filtergraphs
 	$(M)cp $< $@
 
+tests/data/maps/%: TAG = COPY
+tests/data/maps/%: $(SRC_PATH)/tests/maps/% | tests/data/maps
+	$(M)cp $< $@
+
 RUNNING_FATE := $(filter check fate%,$(filter-out fate-rsync,$(MAKECMDGOALS)))
 
 # Check sanity of dependencies when running FATE tests.
diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index b57ef88c9b..072169ac7e 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -395,6 +395,12 @@ FATE_FILTER_SAMPLES-$(call FILTERDEMDEC, FPS SCALE, MOV, QTRLE) += fate-filter-f
 fate-filter-fps-cfr: CMD = framecrc -auto_conversion_filters -i $(TARGET_SAMPLES)/qtrle/apple-animation-variable-fps-bug.mov -r 30 -fps_mode cfr -pix_fmt yuv420p
 fate-filter-fps:     CMD = framecrc -auto_conversion_filters -i $(TARGET_SAMPLES)/qtrle/apple-animation-variable-fps-bug.mov -vf fps=30 -pix_fmt yuv420p
 
+FATE_FILTER_SAMPLES-$(call FILTERFRAMECRC, TESTSRC2 FSYNC, FILE_PROTOCOL) += fate-filter-fsync-up fate-filter-fsync-down
+fate-filter-fsync-up: tests/data/maps/fsync-up
+fate-filter-fsync-up: CMD = framecrc -lavfi testsrc2=r=25:d=1,fsync=f=tests/data/maps/fsync-up
+fate-filter-fsync-down: tests/data/maps/fsync-down
+fate-filter-fsync-down: CMD = framecrc -lavfi testsrc2=r=25:d=1,fsync=f=tests/data/maps/fsync-down
+
 FATE_FILTER_ALPHAEXTRACT_ALPHAMERGE := $(addprefix fate-filter-alphaextract_alphamerge_, rgb yuv)
 FATE_FILTER_VSYNTH_PGMYUV-$(call ALLYES, SCALE_FILTER FORMAT_FILTER SPLIT_FILTER ALPHAEXTRACT_FILTER ALPHAMERGE_FILTER) += $(FATE_FILTER_ALPHAEXTRACT_ALPHAMERGE)
 $(FATE_FILTER_ALPHAEXTRACT_ALPHAMERGE): fate-filter-alphaextract_alphamerge_%: tests/data/filtergraphs/alphamerge_alphaextract_%
diff --git a/tests/maps/fsync-down b/tests/maps/fsync-down
new file mode 100644
index 0000000000..107761ecbe
--- /dev/null
+++ b/tests/maps/fsync-down
@@ -0,0 +1,7 @@
+1 0 1/7
+5 1 1/7
+8 2 1/7
+12 3 1/7
+16 4 1/7
+19 5 1/7
+23 6 1/7
diff --git a/tests/maps/fsync-up b/tests/maps/fsync-up
new file mode 100644
index 0000000000..c37a1fae14
--- /dev/null
+++ b/tests/maps/fsync-up
@@ -0,0 +1,57 @@
+0 0 1/57
+0 1 1/57
+1 2 1/57
+1 3 1/57
+1 4 1/57
+2 5 1/57
+2 6 1/57
+3 7 1/57
+3 8 1/57
+4 9 1/57
+4 10 1/57
+5 11 1/57
+5 12 1/57
+5 13 1/57
+6 14 1/57
+6 15 1/57
+7 16 1/57
+7 17 1/57
+8 18 1/57
+8 19 1/57
+8 20 1/57
+9 21 1/57
+9 22 1/57
+10 23 1/57
+10 24 1/57
+11 25 1/57
+11 26 1/57
+12 27 1/57
+12 28 1/57
+12 29 1/57
+13 30 1/57
+13 31 1/57
+14 32 1/57
+14 33 1/57
+15 34 1/57
+15 35 1/57
+16 36 1/57
+16 37 1/57
+16 38 1/57
+17 39 1/57
+17 40 1/57
+18 41 1/57
+18 42 1/57
+19 43 1/57
+19 44 1/57
+19 45 1/57
+20 46 1/57
+20 47 1/57
+21 48 1/57
+21 49 1/57
+22 50 1/57
+22 51 1/57
+23 52 1/57
+23 53 1/57
+23 54 1/57
+24 55 1/57
+24 56 1/57
diff --git a/tests/ref/fate/filter-fsync-down b/tests/ref/fate/filter-fsync-down
new file mode 100644
index 0000000000..d3f04060ad
--- /dev/null
+++ b/tests/ref/fate/filter-fsync-down
@@ -0,0 +1,12 @@
+#tb 0: 1/7
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 320x240
+#sar 0: 1/1
+0,          0,          0,        1,   115200, 0x7ed43658
+0,          1,          1,        1,   115200, 0x5418f45b
+0,          2,          2,        1,   115200, 0x9872fad9
+0,          3,          3,        1,   115200, 0x4dbbf2e0
+0,          4,          4,        1,   115200, 0xcce711f5
+0,          5,          5,        1,   115200, 0xaa341025
+0,          6,          6,        1,   115200, 0xb41eeaac
diff --git a/tests/ref/fate/filter-fsync-up b/tests/ref/fate/filter-fsync-up
new file mode 100644
index 0000000000..ea7f7efe2d
--- /dev/null
+++ b/tests/ref/fate/filter-fsync-up
@@ -0,0 +1,62 @@
+#tb 0: 1/57
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 320x240
+#sar 0: 1/1
+0,          0,          0,        1,   115200, 0xeba70ff3
+0,          1,          1,        1,   115200, 0xeba70ff3
+0,          2,          2,        1,   115200, 0x7ed43658
+0,          3,          3,        1,   115200, 0x7ed43658
+0,          4,          4,        1,   115200, 0x7ed43658
+0,          5,          5,        1,   115200, 0x8cd87e03
+0,          6,          6,        1,   115200, 0x8cd87e03
+0,          7,          7,        1,   115200, 0xbb1ca0c4
+0,          8,          8,        1,   115200, 0xbb1ca0c4
+0,          9,          9,        1,   115200, 0x5fdfd474
+0,         10,         10,        1,   115200, 0x5fdfd474
+0,         11,         11,        1,   115200, 0x5418f45b
+0,         12,         12,        1,   115200, 0x5418f45b
+0,         13,         13,        1,   115200, 0x5418f45b
+0,         14,         14,        1,   115200, 0xb16cf929
+0,         15,         15,        1,   115200, 0xb16cf929
+0,         16,         16,        1,   115200, 0xe1f7f824
+0,         17,         17,        1,   115200, 0xe1f7f824
+0,         18,         18,        1,   115200, 0x9872fad9
+0,         19,         19,        1,   115200, 0x9872fad9
+0,         20,         20,        1,   115200, 0x9872fad9
+0,         21,         21,        1,   115200, 0x02a4f220
+0,         22,         22,        1,   115200, 0x02a4f220
+0,         23,         23,        1,   115200, 0x9ae2fcc9
+0,         24,         24,        1,   115200, 0x9ae2fcc9
+0,         25,         25,        1,   115200, 0x9b56f029
+0,         26,         26,        1,   115200, 0x9b56f029
+0,         27,         27,        1,   115200, 0x4dbbf2e0
+0,         28,         28,        1,   115200, 0x4dbbf2e0
+0,         29,         29,        1,   115200, 0x4dbbf2e0
+0,         30,         30,        1,   115200, 0x1953f828
+0,         31,         31,        1,   115200, 0x1953f828
+0,         32,         32,        1,   115200, 0xc42403b8
+0,         33,         33,        1,   115200, 0xc42403b8
+0,         34,         34,        1,   115200, 0xeb4615f6
+0,         35,         35,        1,   115200, 0xeb4615f6
+0,         36,         36,        1,   115200, 0xcce711f5
+0,         37,         37,        1,   115200, 0xcce711f5
+0,         38,         38,        1,   115200, 0xcce711f5
+0,         39,         39,        1,   115200, 0x297b12cf
+0,         40,         40,        1,   115200, 0x297b12cf
+0,         41,         41,        1,   115200, 0x625f10e9
+0,         42,         42,        1,   115200, 0x625f10e9
+0,         43,         43,        1,   115200, 0xaa341025
+0,         44,         44,        1,   115200, 0xaa341025
+0,         45,         45,        1,   115200, 0xaa341025
+0,         46,         46,        1,   115200, 0x139821b1
+0,         47,         47,        1,   115200, 0x139821b1
+0,         48,         48,        1,   115200, 0x1e7e09a0
+0,         49,         49,        1,   115200, 0x1e7e09a0
+0,         50,         50,        1,   115200, 0xa7d80776
+0,         51,         51,        1,   115200, 0xa7d80776
+0,         52,         52,        1,   115200, 0xb41eeaac
+0,         53,         53,        1,   115200, 0xb41eeaac
+0,         54,         54,        1,   115200, 0xb41eeaac
+0,         55,         55,        1,   115200, 0xe00dd55d
+0,         56,         56,        1,   115200, 0xe00dd55d
-- 
2.37.1 (Apple Git-137.1)

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avfilter: Add fsync filter
  2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 1/2] " Thilo Borgmann via ffmpeg-devel
@ 2023-12-15 14:17   ` Andreas Rheinhardt
  2023-12-16  8:13     ` Thilo Borgmann via ffmpeg-devel
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Rheinhardt @ 2023-12-15 14:17 UTC (permalink / raw)
  To: ffmpeg-devel

Thilo Borgmann via ffmpeg-devel:
> ---
>  Changelog                |   1 +
>  MAINTAINERS              |   1 +
>  doc/filters.texi         |  33 +++++
>  libavfilter/Makefile     |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/version.h    |   2 +-
>  libavfilter/vf_fsync.c   | 304 +++++++++++++++++++++++++++++++++++++++
>  7 files changed, 342 insertions(+), 1 deletion(-)
>  create mode 100644 libavfilter/vf_fsync.c
> 
> diff --git a/Changelog b/Changelog
> index 67ef92eb02..a25278d227 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -9,6 +9,7 @@ version <next>:
>  - aap filter
>  - demuxing, decoding, filtering, encoding, and muxing in the
>    ffmpeg CLI now all run in parallel
> +- fsync filter
>  
>  version 6.1:
>  - libaribcaption decoder
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 39b37ee0c5..4257fcad98 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -343,6 +343,7 @@ Filters:
>    vf_delogo.c                           Jean Delvare (CC <jdelvare@suse.com>)
>    vf_drawbox.c/drawgrid                 Andrey Utkin
>    vf_extractplanes.c                    Paul B Mahol
> +  vf_fsync.c                            Thilo Borgmann
>    vf_histogram.c                        Paul B Mahol
>    vf_hqx.c                              Clément Bœsch
>    vf_idet.c                             Pascal Massimino
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 6d00ba2c3f..9f19cba9df 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -14681,6 +14681,39 @@ option may cause flicker since the B-Frames have often larger QP. Default is
>  
>  @end table
>  
> +@anchor{fsync}
> +@section fsync
> +
> +Synchronize video frames with an external mapping from a file.
> +
> +For each input PTS given in the map file it either drops or creates as many frames as necessary to recreate the sequence of output frames given in the map file.
> +
> +This filter is useful to recreate the output frames of a framerate conversion by the @ref{fps} filter, recorded into a map file using the ffmpeg option @code{-stats_mux_pre}, and do further processing to the corresponding frames e.g. quality comparison.
> +
> +Each line of the map file must contain three items per input frame, the input PTS (decimal), the output PTS (decimal) and the output TIMEBASE (decimal/decimal), seperated by a space.
> +This file format corresponds to the output of @code{-stats_mux_pre_fmt="@{ptsi@} @{pts@} @{tb@}"}.
> +
> +The filter assumes the map file is sorted by increasing input PTS.
> +
> +The filter accepts the following options:
> +@table @option
> +
> +@item file, f
> +The filename of the map file to be used.
> +@end table
> +
> +Example:
> +@example
> +# Convert a video to 25 fps and record a MAP_FILE file with the default format of this filter
> +ffmpeg -i INPUT -vf fps=fps=25 -stats_mux_pre MAP_FILE -stats_mux_pre_fmt "@{ptsi@} @{pts@} @{tb@}" OUTPUT
> +
> +# Sort MAP_FILE by increasing input PTS
> +sort -n MAP_FILE
> +
> +# Use INPUT, OUTPUT and the MAP_FILE from above to compare the corresponding frames in INPUT and OUTPUT via SSIM
> +ffmpeg -i INPUT -i OUTPUT -filter_complex '[0:v]fsync=file=MAP_FILE[ref];[1:v][ref]ssim' -f null -
> +@end example
> +
>  @section gblur
>  
>  Apply Gaussian blur filter.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 63725f91b4..612616dfb4 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -323,6 +323,7 @@ OBJS-$(CONFIG_FREEZEDETECT_FILTER)           += vf_freezedetect.o
>  OBJS-$(CONFIG_FREEZEFRAMES_FILTER)           += vf_freezeframes.o
>  OBJS-$(CONFIG_FREI0R_FILTER)                 += vf_frei0r.o
>  OBJS-$(CONFIG_FSPP_FILTER)                   += vf_fspp.o qp_table.o
> +OBJS-$(CONFIG_FSYNC_FILTER)                  += vf_fsync.o
>  OBJS-$(CONFIG_GBLUR_FILTER)                  += vf_gblur.o
>  OBJS-$(CONFIG_GBLUR_VULKAN_FILTER)           += vf_gblur_vulkan.o vulkan.o vulkan_filter.o
>  OBJS-$(CONFIG_GEQ_FILTER)                    += vf_geq.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index ed7c32be94..b32ffb2d71 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -299,6 +299,7 @@ extern const AVFilter ff_vf_freezedetect;
>  extern const AVFilter ff_vf_freezeframes;
>  extern const AVFilter ff_vf_frei0r;
>  extern const AVFilter ff_vf_fspp;
> +extern const AVFilter ff_vf_fsync;
>  extern const AVFilter ff_vf_gblur;
>  extern const AVFilter ff_vf_gblur_vulkan;
>  extern const AVFilter ff_vf_geq;
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index 7642b670d1..59330858bd 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -31,7 +31,7 @@
>  
>  #include "version_major.h"
>  
> -#define LIBAVFILTER_VERSION_MINOR  14
> +#define LIBAVFILTER_VERSION_MINOR  15
>  #define LIBAVFILTER_VERSION_MICRO 100
>  
>  
> diff --git a/libavfilter/vf_fsync.c b/libavfilter/vf_fsync.c
> new file mode 100644
> index 0000000000..3ce6f22d06
> --- /dev/null
> +++ b/libavfilter/vf_fsync.c
> @@ -0,0 +1,304 @@
> +/*
> + * Copyright (c) 2023 Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Filter for syncing video frames from external source
> + *
> + * @author Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + */
> +
> +#include "libavutil/avstring.h"
> +#include "libavutil/error.h"
> +#include "libavutil/opt.h"
> +#include "libavformat/avio.h"
> +#include "video.h"
> +#include "filters.h"
> +
> +#define BUF_SIZE 256
> +
> +typedef struct FsyncContext {
> +    const AVClass *class;
> +    AVIOContext *avio_ctx; // reading the map file
> +    AVFrame *last_frame;   // buffering the last frame for duplicating eventually
> +    char *filename;        // user-specified map file
> +    char *buf;             // line buffer for the map file
> +    char *cur;             // current position in the line buffer
> +    char *end;             // end pointer of the line buffer
> +    int64_t ptsi;          // input pts to map to [0-N] output pts
> +    int64_t pts;           // output pts
> +    int64_t tb_num;        // output timebase num
> +    int64_t tb_den;        // output timebase den
> +} FsyncContext;
> +
> +#define OFFSET(x) offsetof(FsyncContext, x)
> +#define DEFINE_OPTIONS(filt_name, FLAGS)

Why is this a macro?


               \
> +static const AVOption filt_name##_options[] = {                                                                                 \
> +    { "file",   "set the file name to use for frame sync", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = "" }, .flags=FLAGS }, \
> +    { "f",      "set the file name to use for frame sync", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = "" }, .flags=FLAGS }, \
> +    { NULL }                                                                                                                    \
> +}
> +
> +// fills the buffer from cur to end, add \0 at EOF
> +static int buf_fill(FsyncContext *ctx)
> +{
> +    int ret;
> +    int num = ctx->end - ctx->cur;
> +
> +    ret = avio_read(ctx->avio_ctx, ctx->cur, num);
> +    if (ret < 0)
> +        return ret;
> +    if (ret < num) {
> +        *(ctx->cur + ret) = '\0';
> +    }
> +
> +    return ret;
> +}
> +
> +// copies cur to end to the beginning and fills the rest
> +static int buf_reload(FsyncContext *ctx)
> +{
> +    int i, ret;
> +    int num = ctx->end - ctx->cur;
> +
> +    for (i = 0; i < num; i++) {
> +        ctx->buf[i] = *ctx->cur++;
> +    }
> +
> +    ctx->cur = ctx->buf + i;
> +    ret = buf_fill(ctx);
> +    if (ret < 0)
> +        return ret;
> +    ctx->cur = ctx->buf;

I wonder whether you should not just use avio_read_to_bprint() for all
of this.

> +
> +    return ret;
> +}
> +
> +// skip from cur over eol
> +static void buf_skip_eol(FsyncContext *ctx)
> +{
> +    char *i;
> +    for (i = ctx->cur; i < ctx->end; i++) {
> +        if (*i != '\n')// && *i != '\r')
> +            break;
> +    }
> +    ctx->cur = i;
> +}
> +
> +// get number of bytes from cur until eol
> +static int buf_get_line_count(FsyncContext *ctx)
> +{
> +    int ret = 0;
> +    char *i;
> +    for (i = ctx->cur; i < ctx->end; i++, ret++) {
> +        if (*i == '\0' || *i == '\n')
> +            return ret;

If you unconditionally added a single \0 to the end of the buffer, you
could use strchr() here.

> +    }
> +
> +    return -1;
> +}
> +
> +// get number of bytes from cur to '\0'
> +static int buf_get_zero(FsyncContext *ctx)
> +{
> +    int ret = 0;
> +    char *i;
> +    for (i = ctx->cur; i < ctx->end; i++, ret++) {
> +        if (*i == '\0')
> +            return ret;

strnlen?

> +    }
> +
> +    return ret;
> +}
> +
> +static int activate(AVFilterContext *ctx)
> +{
> +    FsyncContext *s       = ctx->priv;
> +    AVFilterLink *inlink  = ctx->inputs[0];
> +    AVFilterLink *outlink = ctx->outputs[0];
> +
> +    int ret, line_count;
> +    AVFrame *frame;
> +
> +    FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
> +
> +    buf_skip_eol(s);
> +    line_count = buf_get_line_count(s);
> +    if (line_count < 0) {
> +        line_count = buf_reload(s);
> +        if (line_count < 0)
> +            return line_count;
> +        line_count = buf_get_line_count(s);
> +        if (line_count < 0)
> +            return line_count;
> +    }
> +
> +    if (avio_feof(s->avio_ctx) && buf_get_zero(s) < 3) {
> +        av_log(ctx, AV_LOG_DEBUG, "End of file. To zero = %i\n", buf_get_zero(s));
> +        goto end;
> +    }
> +
> +    if (s->last_frame) {
> +        ret = av_sscanf(s->cur, "%"PRId64" %"PRId64" %d/%d", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
> +        if (ret != 4) {
> +            av_log(ctx, AV_LOG_ERROR, "Unexpected format found (%i / 4).\n", ret);
> +            ff_outlink_set_status(outlink, AVERROR_INVALIDDATA, AV_NOPTS_VALUE);
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        av_log(ctx, AV_LOG_DEBUG, "frame %lli ", s->last_frame->pts);
> +
> +        if (s->last_frame->pts >= s->ptsi) {
> +            av_log(ctx, AV_LOG_DEBUG, ">= %lli: DUP LAST with pts = %lli\n", s->ptsi, s->pts);
> +
> +            // clone frame
> +            frame = av_frame_clone(s->last_frame);
> +            if (!frame) {
> +                ff_outlink_set_status(outlink, AVERROR(ENOMEM), AV_NOPTS_VALUE);
> +                return AVERROR(ENOMEM);
> +            }
> +            av_frame_copy_props(frame, s->last_frame);

Unnecessary, as the properties of frame are already equivalent to the
properties of last_frame.

> +
> +            // set output pts and timebase
> +            frame->pts = s->pts;
> +            frame->time_base = av_make_q((int)s->tb_num, (int)s->tb_den);
> +
> +            // advance cur to eol, skip over eol in the next call
> +            s->cur += line_count;
> +
> +            // call again
> +            if (ff_inoutlink_check_flow(inlink, outlink))
> +                ff_filter_set_ready(ctx, 100);
> +
> +            // filter frame
> +            return ff_filter_frame(outlink, frame);
> +        } else if (s->last_frame->pts < s->ptsi) {
> +            av_log(ctx, AV_LOG_DEBUG, "<  %lli: DROP\n", s->ptsi);
> +            av_frame_free(&s->last_frame);
> +
> +            // call again
> +            if (ff_inoutlink_check_flow(inlink, outlink))
> +                ff_filter_set_ready(ctx, 100);
> +
> +            return 0;
> +        }
> +    }
> +
> +end:
> +    ret = ff_inlink_consume_frame(inlink, &s->last_frame);
> +    if (ret < 0)
> +        return ret;
> +
> +    FF_FILTER_FORWARD_STATUS(inlink, outlink);
> +    FF_FILTER_FORWARD_WANTED(outlink, inlink);
> +
> +    return FFERROR_NOT_READY;
> +}
> +
> +static int fsync_config_props(AVFilterLink* outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    FsyncContext    *s   = ctx->priv;
> +    int ret;
> +
> +    // read first line to get output timebase
> +    ret = av_sscanf(s->cur, "%"PRId64" %"PRId64" %d/%d", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);

tb_num, tb_den are int64_t, yet you use %d to read into them.

> +    if (ret != 4) {
> +        av_log(ctx, AV_LOG_ERROR, "Unexpected format found (%i of 4).\n", ret);
> +        ff_outlink_set_status(outlink, AVERROR_INVALIDDATA, AV_NOPTS_VALUE);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    outlink->frame_rate = av_make_q(1, 0); // unknown or dynamic
> +    outlink->time_base  = av_make_q((int)s->tb_num, (int)s->tb_den);
> +
> +    return 0;
> +}
> +
> +static av_cold int fsync_init(AVFilterContext *ctx)
> +{
> +    FsyncContext *s = ctx->priv;
> +    int ret;
> +
> +    av_log(ctx, AV_LOG_DEBUG, "filename: %s\n", s->filename);
> +
> +    s->buf = av_malloc(BUF_SIZE);
> +    if (!s->buf)
> +        return AVERROR(ENOMEM);
> +
> +    ret = avio_open(&s->avio_ctx, s->filename, AVIO_FLAG_READ);

Given that you are using avio, you probably need a avformat dependency
in configure.

> +    if (ret < 0)
> +        return ret;
> +
> +    s->cur = s->buf;
> +    s->end = s->buf + BUF_SIZE;
> +
> +    ret = buf_fill(s);
> +    if (ret < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
> +static av_cold void fsync_uninit(AVFilterContext *ctx)
> +{
> +    FsyncContext *s = ctx->priv;
> +
> +    avio_close(s->avio_ctx);

avio_closep()

> +    av_freep(&s->buf);
> +    av_frame_unref(s->last_frame);

I expect that this needs to be changed to av_frame_free(). Anyway, you
should run your tests via valgrind/asan.

> +}
> +
> +DEFINE_OPTIONS(fsync, AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM);
> +AVFILTER_DEFINE_CLASS(fsync);
> +
> +static const enum AVPixelFormat pix_fmts[] = {
> +    AV_PIX_FMT_GRAY8,
> +    AV_PIX_FMT_GBRP,     AV_PIX_FMT_GBRAP,
> +    AV_PIX_FMT_YUV422P,  AV_PIX_FMT_YUV420P,
> +    AV_PIX_FMT_YUV444P,  AV_PIX_FMT_YUV440P,
> +    AV_PIX_FMT_YUV411P,  AV_PIX_FMT_YUV410P,
> +    AV_PIX_FMT_YUVJ440P, AV_PIX_FMT_YUVJ411P, AV_PIX_FMT_YUVJ420P,
> +    AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
> +    AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVA422P, AV_PIX_FMT_YUVA420P,
> +    AV_PIX_FMT_NONE
> +};

Why is this filter limited to specific pixel formats? Shouldn't it
accept all of them?

> +
> +static const AVFilterPad avfilter_vf_fsync_outputs[] = {

Don't use a name that indicates that this is exported. (I know that
other filters have the same issue...)

> +    {
> +        .name          = "default",
> +        .type          = AVMEDIA_TYPE_VIDEO,
> +        .config_props  = fsync_config_props,
> +    },
> +};
> +
> +const AVFilter ff_vf_fsync = {
> +    .name          = "fsync",
> +    .description   = NULL_IF_CONFIG_SMALL("Synchronize video frames from external source."),
> +    .init          = fsync_init,
> +    .uninit        = fsync_uninit,
> +    .priv_size     = sizeof(FsyncContext),
> +    .priv_class    = &fsync_class,
> +    .activate      = activate,
> +    FILTER_PIXFMTS_ARRAY(pix_fmts),
> +    FILTER_INPUTS(ff_video_default_filterpad),
> +    FILTER_OUTPUTS(avfilter_vf_fsync_outputs),
> +    .flags         = AVFILTER_FLAG_METADATA_ONLY,
> +};

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avfilter: Add fsync filter
  2023-12-15 14:17   ` Andreas Rheinhardt
@ 2023-12-16  8:13     ` Thilo Borgmann via ffmpeg-devel
  0 siblings, 0 replies; 5+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-16  8:13 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Thilo Borgmann

Am 15.12.23 um 15:17 schrieb Andreas Rheinhardt:
> Thilo Borgmann via ffmpeg-devel:
>> ---
>>   Changelog                |   1 +
>>   MAINTAINERS              |   1 +
>>   doc/filters.texi         |  33 +++++
>>   libavfilter/Makefile     |   1 +
>>   libavfilter/allfilters.c |   1 +
>>   libavfilter/version.h    |   2 +-
>>   libavfilter/vf_fsync.c   | 304 +++++++++++++++++++++++++++++++++++++++
>>   7 files changed, 342 insertions(+), 1 deletion(-)
>>   create mode 100644 libavfilter/vf_fsync.c
>>
>> [...]
>> +// fills the buffer from cur to end, add \0 at EOF
>> +static int buf_fill(FsyncContext *ctx)
>> +{
>> +    int ret;
>> +    int num = ctx->end - ctx->cur;
>> +
>> +    ret = avio_read(ctx->avio_ctx, ctx->cur, num);
>> +    if (ret < 0)
>> +        return ret;
>> +    if (ret < num) {
>> +        *(ctx->cur + ret) = '\0';
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +// copies cur to end to the beginning and fills the rest
>> +static int buf_reload(FsyncContext *ctx)
>> +{
>> +    int i, ret;
>> +    int num = ctx->end - ctx->cur;
>> +
>> +    for (i = 0; i < num; i++) {
>> +        ctx->buf[i] = *ctx->cur++;
>> +    }
>> +
>> +    ctx->cur = ctx->buf + i;
>> +    ret = buf_fill(ctx);
>> +    if (ret < 0)
>> +        return ret;
>> +    ctx->cur = ctx->buf;
> 
> I wonder whether you should not just use avio_read_to_bprint() for all
> of this.

I tested a bit. It appears good for filling the buffer with one function call, getting the stuff out for scanf appears not as well as when and how to refill the buffer. Comparing a bit to f_metadata where this is used, the necessary function get bigger and more complex than this easily.
Maybe its just I didn't use it often enough, but this char* handling appears easier and less complex to me.
So if you don't insist I'd leave it that way.

>> +
>> +    return ret;
>> +}
>> +
>> +// skip from cur over eol
>> +static void buf_skip_eol(FsyncContext *ctx)
>> +{
>> +    char *i;
>> +    for (i = ctx->cur; i < ctx->end; i++) {
>> +        if (*i != '\n')// && *i != '\r')
>> +            break;
>> +    }
>> +    ctx->cur = i;
>> +}
>> +
>> +// get number of bytes from cur until eol
>> +static int buf_get_line_count(FsyncContext *ctx)
>> +{
>> +    int ret = 0;
>> +    char *i;
>> +    for (i = ctx->cur; i < ctx->end; i++, ret++) {
>> +        if (*i == '\0' || *i == '\n')
>> +            return ret;
> 
> If you unconditionally added a single \0 to the end of the buffer, you
> could use strchr() here.

I'd need two strchr() calls, for \0 and \n, plus the interpretation of the resulting pointers.
Where \0 would always be found at the end of the buffer which needs another if to see if its what I need or do need to load more data.


>> [...]
>> +
>> +static av_cold void fsync_uninit(AVFilterContext *ctx)
>> +{
>> +    FsyncContext *s = ctx->priv;
>> +
>> +    avio_close(s->avio_ctx);
> 
> avio_closep()
> 
>> +    av_freep(&s->buf);
>> +    av_frame_unref(s->last_frame);
> 
> I expect that this needs to be changed to av_frame_free(). Anyway, you
> should run your tests via valgrind/asan.

Asan is fine both ways, so I assume it wouldn't catch it?
Changed to av_frame_free anyways.

There appears no Valgrind nor msan to be available for OSX.
'leaks' also even complains it cannot really do its job:

"Can't examine target process's malloc zone asan_0x10c134950, so memory analysis will be incomplete or incorrect.
Reason: for security, cannot load non-system library /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib

Process 49784 is not debuggable. Due to security restrictions, leaks can only show or save contents of readonly memory of restricted processes."

Am I out of options on OSX? Seems like the first real drawback when on a mac...


Fixed all the other things for v3.

-Thilo

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-12-16  8:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15  7:24 [FFmpeg-devel] [PATCH 0/2] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 1/2] " Thilo Borgmann via ffmpeg-devel
2023-12-15 14:17   ` Andreas Rheinhardt
2023-12-16  8:13     ` Thilo Borgmann via ffmpeg-devel
2023-12-15  7:24 ` [FFmpeg-devel] [PATCH 2/2] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git