* [FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: split loop for parsing and validation of -stats_* specifiers
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
@ 2023-12-11 15:07 ` Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils Thilo Borgmann via ffmpeg-devel
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-11 15:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
---
fftools/ffmpeg_mux_init.c | 40 ++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 63a25a350f..6c473a8f09 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -365,6 +365,26 @@ static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
c = &es->components[es->nb_components - 1];
+ for (size_t i = 0; i < FF_ARRAY_ELEMS(fmt_specs); i++) {
+ if (!strcmp(val, fmt_specs[i].str)) {
+ c->type = fmt_specs[i].type;
+ c->str = val;
+ c->str_len = val_len;
+ break;
+ }
+ }
+
+ if (!c->type) {
+ av_log(NULL, AV_LOG_ERROR, "Invalid format directive: %s\n", val);
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+ }
+
+ for (int j = 0; j < es->nb_components; j++) {
+ EncStatsComponent *c = &es->components[j];
+ char *val = c->str;
+
for (size_t i = 0; i < FF_ARRAY_ELEMS(fmt_specs); i++) {
if (!strcmp(val, fmt_specs[i].str)) {
if ((pre && fmt_specs[i].post_only) || (!pre && fmt_specs[i].pre_only)) {
@@ -375,8 +395,6 @@ static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
goto fail;
}
- c->type = fmt_specs[i].type;
-
if (fmt_specs[i].need_input_data && !ost->ist) {
av_log(ost, AV_LOG_WARNING,
"Format directive '%s' is unavailable, because "
@@ -387,20 +405,16 @@ static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
break;
}
}
-
- if (!c->type) {
- av_log(NULL, AV_LOG_ERROR, "Invalid format directive: %s\n", val);
- ret = AVERROR(EINVAL);
- goto fail;
- }
-
-fail:
- av_freep(&val);
- if (ret < 0)
- return ret;
}
ret = enc_stats_get_file(&es->io, path);
+fail:
+ for (int j = 0; j < es->nb_components; j++) {
+ EncStatsComponent *c = &es->components[j];
+ if (c->type != ENC_STATS_LITERAL) {
+ av_freep(&c->str);
+ }
+ }
if (ret < 0)
return ret;
--
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] 20+ messages in thread* [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: split loop for parsing and validation of -stats_* specifiers Thilo Borgmann via ffmpeg-devel
@ 2023-12-11 15:07 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:00 ` Anton Khirnov
2023-12-13 12:10 ` Nicolas George
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 3/5] reindent after last commit Thilo Borgmann via ffmpeg-devel
` (2 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-11 15:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
---
fftools/ffmpeg.h | 31 +------
fftools/ffmpeg_enc.c | 3 +-
fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
libavutil/parseutils.h | 102 ++++++++++++++++++++++
libavutil/version.h | 2 +-
6 files changed, 296 insertions(+), 170 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 1f11a2f002..cb4d90c7b2 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -42,6 +42,7 @@
#include "libavutil/eval.h"
#include "libavutil/fifo.h"
#include "libavutil/hwcontext.h"
+#include "libavutil/parseutils.h"
#include "libavutil/pixfmt.h"
#include "libavutil/rational.h"
#include "libavutil/thread.h"
@@ -437,36 +438,8 @@ enum forced_keyframes_const {
#define ABORT_ON_FLAG_EMPTY_OUTPUT (1 << 0)
#define ABORT_ON_FLAG_EMPTY_OUTPUT_STREAM (1 << 1)
-enum EncStatsType {
- ENC_STATS_LITERAL = 0,
- ENC_STATS_FILE_IDX,
- ENC_STATS_STREAM_IDX,
- ENC_STATS_FRAME_NUM,
- ENC_STATS_FRAME_NUM_IN,
- ENC_STATS_TIMEBASE,
- ENC_STATS_TIMEBASE_IN,
- ENC_STATS_PTS,
- ENC_STATS_PTS_TIME,
- ENC_STATS_PTS_IN,
- ENC_STATS_PTS_TIME_IN,
- ENC_STATS_DTS,
- ENC_STATS_DTS_TIME,
- ENC_STATS_SAMPLE_NUM,
- ENC_STATS_NB_SAMPLES,
- ENC_STATS_PKT_SIZE,
- ENC_STATS_BITRATE,
- ENC_STATS_AVG_BITRATE,
-};
-
-typedef struct EncStatsComponent {
- enum EncStatsType type;
-
- uint8_t *str;
- size_t str_len;
-} EncStatsComponent;
-
typedef struct EncStats {
- EncStatsComponent *components;
+ AVEncStatsComponent *components;
int nb_components;
AVIOContext *io;
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index fa4539664f..a499bc0c81 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -30,6 +30,7 @@
#include "libavutil/frame.h"
#include "libavutil/intreadwrite.h"
#include "libavutil/log.h"
+#include "libavutil/parseutils.h"
#include "libavutil/pixdesc.h"
#include "libavutil/rational.h"
#include "libavutil/timestamp.h"
@@ -499,7 +500,7 @@ void enc_stats_write(OutputStream *ost, EncStats *es,
}
for (size_t i = 0; i < es->nb_components; i++) {
- const EncStatsComponent *c = &es->components[i];
+ const AVEncStatsComponent *c = &es->components[i];
switch (c->type) {
case ENC_STATS_LITERAL: avio_write (io, c->str, c->str_len); continue;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 6c473a8f09..6acdf92c2c 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -242,152 +242,26 @@ void of_enc_stats_close(void)
nb_enc_stats_files = 0;
}
-static int unescape(char **pdst, size_t *dst_len,
- const char **pstr, char delim)
-{
- const char *str = *pstr;
- char *dst;
- size_t len, idx;
-
- *pdst = NULL;
-
- len = strlen(str);
- if (!len)
- return 0;
-
- dst = av_malloc(len + 1);
- if (!dst)
- return AVERROR(ENOMEM);
-
- for (idx = 0; *str; idx++, str++) {
- if (str[0] == '\\' && str[1])
- str++;
- else if (*str == delim)
- break;
-
- dst[idx] = *str;
- }
- if (!idx) {
- av_freep(&dst);
- return 0;
- }
-
- dst[idx] = 0;
-
- *pdst = dst;
- *dst_len = idx;
- *pstr = str;
-
- return 0;
-}
-
static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
const char *path, const char *fmt_spec)
{
- static const struct {
- enum EncStatsType type;
- const char *str;
- int pre_only:1;
- int post_only:1;
- int need_input_data:1;
- } fmt_specs[] = {
- { ENC_STATS_FILE_IDX, "fidx" },
- { ENC_STATS_STREAM_IDX, "sidx" },
- { ENC_STATS_FRAME_NUM, "n" },
- { ENC_STATS_FRAME_NUM_IN, "ni", 0, 0, 1 },
- { ENC_STATS_TIMEBASE, "tb" },
- { ENC_STATS_TIMEBASE_IN, "tbi", 0, 0, 1 },
- { ENC_STATS_PTS, "pts" },
- { ENC_STATS_PTS_TIME, "t" },
- { ENC_STATS_PTS_IN, "ptsi", 0, 0, 1 },
- { ENC_STATS_PTS_TIME_IN, "ti", 0, 0, 1 },
- { ENC_STATS_DTS, "dts", 0, 1 },
- { ENC_STATS_DTS_TIME, "dt", 0, 1 },
- { ENC_STATS_SAMPLE_NUM, "sn", 1 },
- { ENC_STATS_NB_SAMPLES, "samp", 1 },
- { ENC_STATS_PKT_SIZE, "size", 0, 1 },
- { ENC_STATS_BITRATE, "br", 0, 1 },
- { ENC_STATS_AVG_BITRATE, "abr", 0, 1 },
- };
- const char *next = fmt_spec;
-
int ret;
- while (*next) {
- EncStatsComponent *c;
- char *val;
- size_t val_len;
-
- // get the sequence up until next opening brace
- ret = unescape(&val, &val_len, &next, '{');
- if (ret < 0)
- return ret;
-
- if (val) {
- ret = GROW_ARRAY(es->components, es->nb_components);
- if (ret < 0) {
- av_freep(&val);
- return ret;
- }
-
- c = &es->components[es->nb_components - 1];
- c->type = ENC_STATS_LITERAL;
- c->str = val;
- c->str_len = val_len;
- }
-
- if (!*next)
- break;
- next++;
-
- // get the part inside braces
- ret = unescape(&val, &val_len, &next, '}');
- if (ret < 0)
- return ret;
-
- if (!val) {
- av_log(NULL, AV_LOG_ERROR,
- "Empty formatting directive in: %s\n", fmt_spec);
- return AVERROR(EINVAL);
- }
-
- if (!*next) {
- av_log(NULL, AV_LOG_ERROR,
- "Missing closing brace in: %s\n", fmt_spec);
- ret = AVERROR(EINVAL);
- goto fail;
- }
- next++;
-
- ret = GROW_ARRAY(es->components, es->nb_components);
- if (ret < 0)
- goto fail;
-
- c = &es->components[es->nb_components - 1];
-
- for (size_t i = 0; i < FF_ARRAY_ELEMS(fmt_specs); i++) {
- if (!strcmp(val, fmt_specs[i].str)) {
- c->type = fmt_specs[i].type;
- c->str = val;
- c->str_len = val_len;
- break;
- }
- }
-
- if (!c->type) {
- av_log(NULL, AV_LOG_ERROR, "Invalid format directive: %s\n", val);
- ret = AVERROR(EINVAL);
- goto fail;
- }
- }
+ ret = av_parse_enc_stats_components(&es->components, &es->nb_components, fmt_spec);
+ if (ret < 0)
+ goto fail;
for (int j = 0; j < es->nb_components; j++) {
- EncStatsComponent *c = &es->components[j];
+ AVEncStatsComponent *c = &es->components[j];
char *val = c->str;
- for (size_t i = 0; i < FF_ARRAY_ELEMS(fmt_specs); i++) {
- if (!strcmp(val, fmt_specs[i].str)) {
- if ((pre && fmt_specs[i].post_only) || (!pre && fmt_specs[i].pre_only)) {
+ for (size_t i = 0; i < av_get_nb_stats_fmt_specs(); i++) {
+ AVEncStatsFormatSpecifier f;
+ if (av_get_stats_fmt_spec(&f, i))
+ goto fail;
+
+ if (!strcmp(val, f.str)) {
+ if ((pre && f.post_only) || (!pre && f.pre_only)) {
av_log(NULL, AV_LOG_ERROR,
"Format directive '%s' may only be used %s-encoding\n",
val, pre ? "post" : "pre");
@@ -395,7 +269,7 @@ static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
goto fail;
}
- if (fmt_specs[i].need_input_data && !ost->ist) {
+ if (f.need_input_data && !ost->ist) {
av_log(ost, AV_LOG_WARNING,
"Format directive '%s' is unavailable, because "
"this output stream has no associated input stream\n",
@@ -410,7 +284,7 @@ static int enc_stats_init(OutputStream *ost, EncStats *es, int pre,
ret = enc_stats_get_file(&es->io, path);
fail:
for (int j = 0; j < es->nb_components; j++) {
- EncStatsComponent *c = &es->components[j];
+ AVEncStatsComponent *c = &es->components[j];
if (c->type != ENC_STATS_LITERAL) {
av_freep(&c->str);
}
diff --git a/libavutil/parseutils.c b/libavutil/parseutils.c
index 94e88e0a79..796811aa6e 100644
--- a/libavutil/parseutils.c
+++ b/libavutil/parseutils.c
@@ -788,3 +788,179 @@ int av_find_info_tag(char *arg, int arg_size, const char *tag1, const char *info
}
return 0;
}
+
+static const AVEncStatsFormatSpecifier fmt_specs[] = {
+ { ENC_STATS_FILE_IDX, "fidx" },
+ { ENC_STATS_STREAM_IDX, "sidx" },
+ { ENC_STATS_FRAME_NUM, "n" },
+ { ENC_STATS_FRAME_NUM_IN, "ni", 0, 0, 1 },
+ { ENC_STATS_TIMEBASE, "tb" },
+ { ENC_STATS_TIMEBASE_IN, "tbi", 0, 0, 1 },
+ { ENC_STATS_PTS, "pts" },
+ { ENC_STATS_PTS_TIME, "t" },
+ { ENC_STATS_PTS_IN, "ptsi", 0, 0, 1 },
+ { ENC_STATS_PTS_TIME_IN, "ti", 0, 0, 1 },
+ { ENC_STATS_DTS, "dts", 0, 1 },
+ { ENC_STATS_DTS_TIME, "dt", 0, 1 },
+ { ENC_STATS_SAMPLE_NUM, "sn", 1 },
+ { ENC_STATS_NB_SAMPLES, "samp", 1 },
+ { ENC_STATS_PKT_SIZE, "size", 0, 1 },
+ { ENC_STATS_BITRATE, "br", 0, 1 },
+ { ENC_STATS_AVG_BITRATE, "abr", 0, 1 },
+};
+
+size_t av_get_nb_stats_fmt_specs(void)
+{
+ return sizeof(fmt_specs) / sizeof(*fmt_specs);
+}
+
+int av_get_stats_fmt_spec(AVEncStatsFormatSpecifier *fmt_spec, int idx)
+{
+ if (!fmt_spec || idx >= av_get_nb_stats_fmt_specs())
+ return AVERROR(EINVAL);
+
+ *fmt_spec = fmt_specs[idx];
+ return 0;
+}
+
+static int unescape(char **pdst, size_t *dst_len,
+ const char **pstr, char delim)
+{
+ const char *str = *pstr;
+ char *dst;
+ size_t len, idx;
+
+ *pdst = NULL;
+
+ len = strlen(str);
+ if (!len)
+ return 0;
+
+ dst = av_malloc(len + 1);
+ if (!dst)
+ return AVERROR(ENOMEM);
+
+ for (idx = 0; *str; idx++, str++) {
+ if (str[0] == '\\' && str[1])
+ str++;
+ else if (*str == delim)
+ break;
+
+ dst[idx] = *str;
+ }
+ if (!idx) {
+ av_freep(&dst);
+ return 0;
+ }
+
+ dst[idx] = 0;
+
+ *pdst = dst;
+ *dst_len = idx;
+ *pstr = str;
+
+ return 0;
+}
+
+static int grow_array(void **array, int elem_size, int *size, int new_size)
+{
+ if (new_size >= INT_MAX / elem_size) {
+ av_log(NULL, AV_LOG_ERROR, "Array too big.\n");
+ return AVERROR(ERANGE);
+ }
+ if (*size < new_size) {
+ uint8_t *tmp = av_realloc_array(*array, new_size, elem_size);
+ if (!tmp)
+ return AVERROR(ENOMEM);
+ memset(tmp + *size*elem_size, 0, (new_size-*size) * elem_size);
+ *size = new_size;
+ *array = tmp;
+ return 0;
+ }
+ return 0;
+}
+
+#define GROW_ARRAY(array, nb_elems)\
+ grow_array((void**)&array, sizeof(*array), &nb_elems, nb_elems + 1)
+
+int av_parse_enc_stats_components(AVEncStatsComponent **components, int *nb_components, const char *fmt_spec)
+{
+ const char *next = fmt_spec;
+ int ret;
+
+ while (*next) {
+ AVEncStatsComponent *c;
+ char *val;
+ size_t val_len;
+
+ // get the sequence up until next opening brace
+ ret = unescape(&val, &val_len, &next, '{');
+ if (ret < 0)
+ return ret;
+
+ if (val) {
+ ret = GROW_ARRAY(*components, *nb_components);
+ if (ret < 0) {
+ av_freep(&val);
+ return ret;
+ }
+
+ c = &((*components)[*nb_components - 1]);
+ c->type = ENC_STATS_LITERAL;
+ c->str = val;
+ c->str_len = val_len;
+ }
+
+ if (!*next)
+ break;
+ next++;
+
+ // get the part inside braces
+ ret = unescape(&val, &val_len, &next, '}');
+ if (ret < 0)
+ return ret;
+
+ if (!val) {
+ av_log(NULL, AV_LOG_ERROR,
+ "Empty formatting directive in: %s\n", fmt_spec);
+ return AVERROR(EINVAL);
+ }
+
+ if (!*next) {
+ av_log(NULL, AV_LOG_ERROR,
+ "Missing closing brace in: %s\n", fmt_spec);
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+ next++;
+
+ ret = GROW_ARRAY(*components, *nb_components);
+ if (ret < 0)
+ goto fail;
+
+ c = &(*components)[*nb_components - 1];
+
+ for (int i = 0; i < FF_ARRAY_ELEMS(fmt_specs); i++) {
+ if (!strcmp(val, fmt_specs[i].str)) {
+ c->type = fmt_specs[i].type;
+ c->str = val;
+ c->str_len = val_len;
+ break;
+ }
+ }
+
+ if (!c->type) {
+ av_log(NULL, AV_LOG_ERROR, "Invalid format directive: %s\n", val);
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+
+ continue;
+fail:
+ av_freep(&val);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/libavutil/parseutils.h b/libavutil/parseutils.h
index dad5c2775b..d546d77de0 100644
--- a/libavutil/parseutils.h
+++ b/libavutil/parseutils.h
@@ -22,6 +22,7 @@
#include <time.h>
#include "rational.h"
+#include "mem.h"
/**
* @file
@@ -194,4 +195,105 @@ char *av_small_strptime(const char *p, const char *fmt, struct tm *dt);
*/
time_t av_timegm(struct tm *tm);
+typedef enum AVEncStatsType {
+ ENC_STATS_LITERAL = 0,
+ ENC_STATS_FILE_IDX,
+ ENC_STATS_STREAM_IDX,
+ ENC_STATS_FRAME_NUM,
+ ENC_STATS_FRAME_NUM_IN,
+ ENC_STATS_TIMEBASE,
+ ENC_STATS_TIMEBASE_IN,
+ ENC_STATS_PTS,
+ ENC_STATS_PTS_TIME,
+ ENC_STATS_PTS_IN,
+ ENC_STATS_PTS_TIME_IN,
+ ENC_STATS_DTS,
+ ENC_STATS_DTS_TIME,
+ ENC_STATS_SAMPLE_NUM,
+ ENC_STATS_NB_SAMPLES,
+ ENC_STATS_PKT_SIZE,
+ ENC_STATS_BITRATE,
+ ENC_STATS_AVG_BITRATE,
+} AVEncStatsType;
+
+/**
+ * Structure describing an encoding stats component.
+ */
+typedef struct AVEncStatsComponent {
+ /**
+ * The type of this component.
+ */
+ AVEncStatsType type;
+ /**
+ * The string representation of this component.
+ */
+ uint8_t *str;
+ /**
+ * The length of the string.
+ */
+ size_t str_len;
+} AVEncStatsComponent;
+
+/**
+ * Structure describing an encoding stats format specifier.
+ */
+typedef struct AVEncStatsFormatSpecifier {
+ /**
+ * The type of this format specifier.
+ */
+ enum AVEncStatsType type;
+ /**
+ * The string representation of this format specifier.
+ */
+ const char *str;
+ /**
+ * Flag if this format specifier is only valid before encoding.
+ */
+ int pre_only:1;
+ /**
+ * Flag if this format specifier is only valid after encoding.
+ */
+ int post_only:1;
+ /**
+ * Flag if this format specifier requires an associated input
+ * stream for it to be processed.
+ */
+ int need_input_data:1;
+} AVEncStatsFormatSpecifier;
+
+/**
+ * Get the number of available enc stats format specifiers.
+ */
+size_t av_get_nb_stats_fmt_specs(void);
+
+/**
+ * Get a copy of an enc stats format specifier.
+ *
+ * @param *fmt_spec Destination for the copy of the format specifier.
+ * Has to be previously allocated.
+ *
+ * @param idx Index to the table of format specifiers.
+ *
+ * @return Return 0 on success, a negative value corresponding
+ * to an AVERROR code otherwise.
+ */
+int av_get_stats_fmt_spec(AVEncStatsFormatSpecifier *fmt_spec, int idx);
+
+/**
+ * Parse an enc stats format string into an array of AVEncStatsComponent.
+ *
+ * @param components Pointer to the array of AVEncStatsComponent to store
+ * the parsed elements. The arrary will be reallocated
+ * in the process if any elements are found.
+ *
+ * @param nb_components Pointer to the number of components in the array.
+ *
+ * @param fmt_spec The format string to parse for components.
+ *
+ * @return Return 0 on success, a negative value corresponding
+ * to an AVERROR code otherwise.
+ *
+ */
+int av_parse_enc_stats_components(AVEncStatsComponent **components, int *nb_components, const char *fmt_spec);
+
#endif /* AVUTIL_PARSEUTILS_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index c5fa7c3692..0684996bf2 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 58
-#define LIBAVUTIL_VERSION_MINOR 32
+#define LIBAVUTIL_VERSION_MINOR 33
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
--
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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils Thilo Borgmann via ffmpeg-devel
@ 2023-12-13 12:00 ` Anton Khirnov
2023-12-13 12:05 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:10 ` Nicolas George
1 sibling, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-13 12:00 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
> ---
> fftools/ffmpeg.h | 31 +------
> fftools/ffmpeg_enc.c | 3 +-
> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> libavutil/version.h | 2 +-
> 6 files changed, 296 insertions(+), 170 deletions(-)
Absolutely not.
This is application code and does not belong in the libraries.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:00 ` Anton Khirnov
@ 2023-12-13 12:05 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:08 ` Anton Khirnov
0 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-13 12:05 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Am 13.12.23 um 13:00 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
>> ---
>> fftools/ffmpeg.h | 31 +------
>> fftools/ffmpeg_enc.c | 3 +-
>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
>> libavutil/version.h | 2 +-
>> 6 files changed, 296 insertions(+), 170 deletions(-)
>
> Absolutely not.
>
> This is application code and does not belong in the libraries.
How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
-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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:05 ` Thilo Borgmann via ffmpeg-devel
@ 2023-12-13 12:08 ` Anton Khirnov
2023-12-13 12:15 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-13 12:08 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
> > Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
> >> ---
> >> fftools/ffmpeg.h | 31 +------
> >> fftools/ffmpeg_enc.c | 3 +-
> >> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> >> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> >> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> >> libavutil/version.h | 2 +-
> >> 6 files changed, 296 insertions(+), 170 deletions(-)
> >
> > Absolutely not.
> >
> > This is application code and does not belong in the libraries.
>
> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
Why does that filter need to understand the same directives? No other
filter does.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:08 ` Anton Khirnov
@ 2023-12-13 12:15 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 12:39 ` Anton Khirnov
0 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-13 12:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Am 13.12.23 um 13:08 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
>> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
>>>> ---
>>>> fftools/ffmpeg.h | 31 +------
>>>> fftools/ffmpeg_enc.c | 3 +-
>>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
>>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
>>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
>>>> libavutil/version.h | 2 +-
>>>> 6 files changed, 296 insertions(+), 170 deletions(-)
>>>
>>> Absolutely not.
>>>
>>> This is application code and does not belong in the libraries.
>>
>> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
>
> Why does that filter need to understand the same directives? No other
> filter does.
Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
But that would also mean to update the filter (if someone realizes it) if the option ever changes.
-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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:15 ` Thilo Borgmann via ffmpeg-devel
@ 2023-12-13 12:39 ` Anton Khirnov
2023-12-13 12:50 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-13 12:39 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:15:27)
> Am 13.12.23 um 13:08 schrieb Anton Khirnov:
> > Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
> >> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
> >>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
> >>>> ---
> >>>> fftools/ffmpeg.h | 31 +------
> >>>> fftools/ffmpeg_enc.c | 3 +-
> >>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> >>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> >>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> >>>> libavutil/version.h | 2 +-
> >>>> 6 files changed, 296 insertions(+), 170 deletions(-)
> >>>
> >>> Absolutely not.
> >>>
> >>> This is application code and does not belong in the libraries.
> >>
> >> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
> >
> > Why does that filter need to understand the same directives? No other
> > filter does.
>
> Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
>
> Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
> But that would also mean to update the filter (if someone realizes it) if the option ever changes.
Why does it need a dynamic format at all? Just postulate a fixed format
for its input like every other filter and none of this complexity is
needed.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:39 ` Anton Khirnov
@ 2023-12-13 12:50 ` Thilo Borgmann via ffmpeg-devel
2023-12-13 16:28 ` Anton Khirnov
0 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-13 12:50 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
Am 13.12.23 um 13:39 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:15:27)
>> Am 13.12.23 um 13:08 schrieb Anton Khirnov:
>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
>>>> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
>>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
>>>>>> ---
>>>>>> fftools/ffmpeg.h | 31 +------
>>>>>> fftools/ffmpeg_enc.c | 3 +-
>>>>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
>>>>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
>>>>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
>>>>>> libavutil/version.h | 2 +-
>>>>>> 6 files changed, 296 insertions(+), 170 deletions(-)
>>>>>
>>>>> Absolutely not.
>>>>>
>>>>> This is application code and does not belong in the libraries.
>>>>
>>>> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
>>>
>>> Why does that filter need to understand the same directives? No other
>>> filter does.
>>
>> Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
>>
>> Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
>> But that would also mean to update the filter (if someone realizes it) if the option ever changes.
>
> Why does it need a dynamic format at all? Just postulate a fixed format
> for its input like every other filter and none of this complexity is
> needed.
That would unnecessarily limit the user of the -stats_* option to a specific format if the user also wants to use this filter later.
Either sacrificing the other info the user wanted to process from the file created or having to run the same command (with distinct format strings for the -stats_* option) twice. Or do the conversion from his extensive format into the fixed format manually - without errors.
-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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 12:50 ` Thilo Borgmann via ffmpeg-devel
@ 2023-12-13 16:28 ` Anton Khirnov
2023-12-13 18:17 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-13 16:28 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:50:09)
> Am 13.12.23 um 13:39 schrieb Anton Khirnov:
> > Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:15:27)
> >> Am 13.12.23 um 13:08 schrieb Anton Khirnov:
> >>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
> >>>> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
> >>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
> >>>>>> ---
> >>>>>> fftools/ffmpeg.h | 31 +------
> >>>>>> fftools/ffmpeg_enc.c | 3 +-
> >>>>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> >>>>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> >>>>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> >>>>>> libavutil/version.h | 2 +-
> >>>>>> 6 files changed, 296 insertions(+), 170 deletions(-)
> >>>>>
> >>>>> Absolutely not.
> >>>>>
> >>>>> This is application code and does not belong in the libraries.
> >>>>
> >>>> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
> >>>
> >>> Why does that filter need to understand the same directives? No other
> >>> filter does.
> >>
> >> Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
> >>
> >> Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
> >> But that would also mean to update the filter (if someone realizes it) if the option ever changes.
> >
> > Why does it need a dynamic format at all? Just postulate a fixed format
> > for its input like every other filter and none of this complexity is
> > needed.
>
> That would unnecessarily limit the user of the -stats_* option to a
> specific format if the user also wants to use this filter later.
> Either sacrificing the other info the user wanted to process from the
> file created or having to run the same command (with distinct format
> strings for the -stats_* option) twice. Or do the conversion from his
> extensive format into the fixed format manually - without errors.
It is bad practice to design library features around the needs and
limitations of a single specific caller.
Many of the directives supported by -stats* make sense only in the
context of the ffmpeg CLI, and we may want to add many more in the
future. We definitely do not want to hardcode them into libavutil public
API.
If the problem is limiting ffmpeg CLI users to a specific stats format,
then you could extend these options to allow writing multiple stats
files with different formats.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 16:28 ` Anton Khirnov
@ 2023-12-13 18:17 ` Thilo Borgmann via ffmpeg-devel
2023-12-14 5:23 ` Anton Khirnov
0 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-13 18:17 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
Am 13.12.23 um 17:28 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:50:09)
>> Am 13.12.23 um 13:39 schrieb Anton Khirnov:
>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:15:27)
>>>> Am 13.12.23 um 13:08 schrieb Anton Khirnov:
>>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 13:05:35)
>>>>>> Am 13.12.23 um 13:00 schrieb Anton Khirnov:
>>>>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-11 16:07:22)
>>>>>>>> ---
>>>>>>>> fftools/ffmpeg.h | 31 +------
>>>>>>>> fftools/ffmpeg_enc.c | 3 +-
>>>>>>>> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
>>>>>>>> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
>>>>>>>> libavutil/parseutils.h | 102 ++++++++++++++++++++++
>>>>>>>> libavutil/version.h | 2 +-
>>>>>>>> 6 files changed, 296 insertions(+), 170 deletions(-)
>>>>>>>
>>>>>>> Absolutely not.
>>>>>>>
>>>>>>> This is application code and does not belong in the libraries.
>>>>>>
>>>>>> How else do we not have a redundant copy of all that and make sure that -stats_* options and the filter understand the same {..} directives?
>>>>>
>>>>> Why does that filter need to understand the same directives? No other
>>>>> filter does.
>>>>
>>>> Because it is meant to use the file(s) the -stats_* option writes out. The most convenient and most error resilient way is to use the very same format string for -stats_* option as well as for the filter.
>>>>
>>>> Otherwise it could be a 'usual' scanf-format, but then the user has to translate it from one format into the other - without making mistakes.
>>>> But that would also mean to update the filter (if someone realizes it) if the option ever changes.
>>>
>>> Why does it need a dynamic format at all? Just postulate a fixed format
>>> for its input like every other filter and none of this complexity is
>>> needed.
>>
>> That would unnecessarily limit the user of the -stats_* option to a
>> specific format if the user also wants to use this filter later.
>> Either sacrificing the other info the user wanted to process from the
>> file created or having to run the same command (with distinct format
>> strings for the -stats_* option) twice. Or do the conversion from his
>> extensive format into the fixed format manually - without errors.
>
> It is bad practice to design library features around the needs and
> limitations of a single specific caller.
The callers here would be the CLI and this filter.
Very much like for av_parse_ratio().
In contrast to av_get_known_color_name() which actually has just one specific caller, the CLI.
Other parsing functions have more callers, some including the CLI.
And it makes only sense if we offer the same format anywhere for reading/writing the same thing.
This approach follows what we already do.
> Many of the directives supported by -stats* make sense only in the
> context of the ffmpeg CLI, and we may want to add many more in the
> future. We definitely do not want to hardcode them into libavutil public
> API.
At least three of them are already useful for this filter outside of the CLI.
Others might be useful for other/new uses cases as well and all of them would be useful if you'd want e.g. overlay them on top of the video.
> If the problem is limiting ffmpeg CLI users to a specific stats format,
> then you could extend these options to allow writing multiple stats
> files with different formats.
This would allow for the same use-case without moving the code to libavutil for the price of a less versatile filter.
And some non-standard behaviour in the CLI where multiple equal options override each other, usually.
I find this even less preferable than a fixed forced format.
-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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-13 18:17 ` Thilo Borgmann via ffmpeg-devel
@ 2023-12-14 5:23 ` Anton Khirnov
2023-12-14 10:34 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-14 5:23 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 19:17:04)
> Am 13.12.23 um 17:28 schrieb Anton Khirnov:
> > It is bad practice to design library features around the needs and
> > limitations of a single specific caller.
>
> The callers here would be the CLI and this filter.
First, public APIs should be designed for general classes of use cases,
not specific callers.
Second, you just said above that the reason you are moving this code
into libavutil is specifically because ffmpeg CLI currently behaves in a
certain way. This is the opposite of proper design, AKA an ad-hoc hack.
> Very much like for av_parse_ratio().
> In contrast to av_get_known_color_name() which actually has just one specific caller, the CLI.
No, not at all like any of these. A ratio or a color are generic
concepts that are not tied to a specific use. By contrast this code is
very much speficic to current ffmpeg CLI processing model. I know this,
because I wrote it.
> Other parsing functions have more callers, some including the CLI.
> And it makes only sense if we offer the same format anywhere for reading/writing the same thing.
>
> This approach follows what we already do.
It most certainly does not. It _might_ make slightly more sense if it
were a generic pattern decoupled from specific properties being parsed
and used across many filters and/or other modules. But that's now what
is happening here.
> > Many of the directives supported by -stats* make sense only in the
> > context of the ffmpeg CLI, and we may want to add many more in the
> > future. We definitely do not want to hardcode them into libavutil public
> > API.
>
> At least three of them are already useful for this filter outside of
> the CLI.
Out of 18.
> Others might be useful for other/new uses cases as well and
> all of them would be useful if you'd want e.g. overlay them on top of
> the video.
"Someone might want to use it somehow" is generically true of any
definable value at all, and so does not work as an argument for why
these specific values should be particularly useful to users of this
specific API.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-14 5:23 ` Anton Khirnov
@ 2023-12-14 10:34 ` Thilo Borgmann via ffmpeg-devel
2023-12-14 17:51 ` Anton Khirnov
0 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-14 10:34 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
Am 14.12.23 um 06:23 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 19:17:04)
>> Am 13.12.23 um 17:28 schrieb Anton Khirnov:
>>> It is bad practice to design library features around the needs and
>>> limitations of a single specific caller.
>>
>> The callers here would be the CLI and this filter.
>
> First, public APIs should be designed for general classes of use cases,
> not specific callers.
Parsing a "...{var0}...{varN}..." string for its variables appears quite generic to me.
IMO, especially for libavutil, you think a bit too ideological here.
A) How about putting it under avpriv_ then?
B) Not filibustering about lib design, nor contradictory support of this patch, but use a fixed format. IMHO just sad for the user and a drawback for us not being able to support the same format strings in the CLI & filters without redundant code.
-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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-14 10:34 ` Thilo Borgmann via ffmpeg-devel
@ 2023-12-14 17:51 ` Anton Khirnov
2023-12-14 18:42 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 1 reply; 20+ messages in thread
From: Anton Khirnov @ 2023-12-14 17:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Quoting Thilo Borgmann via ffmpeg-devel (2023-12-14 11:34:11)
> Am 14.12.23 um 06:23 schrieb Anton Khirnov:
> > Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 19:17:04)
> >> Am 13.12.23 um 17:28 schrieb Anton Khirnov:
> >>> It is bad practice to design library features around the needs and
> >>> limitations of a single specific caller.
> >>
> >> The callers here would be the CLI and this filter.
> >
> > First, public APIs should be designed for general classes of use cases,
> > not specific callers.
>
> Parsing a "...{var0}...{varN}..." string for its variables appears quite generic to me.
> IMO, especially for libavutil, you think a bit too ideological here.
You're not parsing generic variables though, but a predefined list of
highly specific ones.
> A) How about putting it under avpriv_ then?
1) fftools are not allowed to use avpriv functions.
2) It does not solve any of the problems I pointed out.
> B) Not filibustering
https://en.wiktionary.org/wiki/filibuster
2. (US politics) A tactic (such as giving long, often irrelevant
speeches) employed to delay the proceedings of, or the making of a
decision by, a legislative body,
Are you seriously calling my comments irrelevant stalling?
> about lib design, nor contradictory support of
> this patch, but use a fixed format. IMHO just sad
> for the user
This is an appeal to emotion along the lines of "THINK OF THE CHILDREN".
It is a fallacious argument that has no place in a technical discussion.
--
Anton Khirnov
_______________________________________________
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] 20+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-14 17:51 ` Anton Khirnov
@ 2023-12-14 18:42 ` Thilo Borgmann via ffmpeg-devel
0 siblings, 0 replies; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-14 18:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
Am 14.12.23 um 18:51 schrieb Anton Khirnov:
> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-14 11:34:11)
>> Am 14.12.23 um 06:23 schrieb Anton Khirnov:
>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-13 19:17:04)
>>>> Am 13.12.23 um 17:28 schrieb Anton Khirnov:
>>>>> It is bad practice to design library features around the needs and
>>>>> limitations of a single specific caller.
>>>>
>>>> The callers here would be the CLI and this filter.
>>>
>>> First, public APIs should be designed for general classes of use cases,
>>> not specific callers.
>>
>> Parsing a "...{var0}...{varN}..." string for its variables appears quite generic to me.
>> IMO, especially for libavutil, you think a bit too ideological here.
>
> You're not parsing generic variables though, but a predefined list of
> highly specific ones.
>
>> A) How about putting it under avpriv_ then?
>
> 1) fftools are not allowed to use avpriv functions.
> 2) It does not solve any of the problems I pointed out.
>
>> B) Not filibustering
>
> https://en.wiktionary.org/wiki/filibuster
> 2. (US politics) A tactic (such as giving long, often irrelevant
> speeches) employed to delay the proceedings of, or the making of a
> decision by, a legislative body,
>
> Are you seriously calling my comments irrelevant stalling?
No, sorry, I just mean endless discussing of this disagreement.
Going to send a fixed format version.
-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] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils Thilo Borgmann via ffmpeg-devel
2023-12-13 12:00 ` Anton Khirnov
@ 2023-12-13 12:10 ` Nicolas George
1 sibling, 0 replies; 20+ messages in thread
From: Nicolas George @ 2023-12-13 12:10 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Thilo Borgmann
Thilo Borgmann via ffmpeg-devel (12023-12-11):
> ---
> fftools/ffmpeg.h | 31 +------
> fftools/ffmpeg_enc.c | 3 +-
> fftools/ffmpeg_mux_init.c | 152 +++-----------------------------
> libavutil/parseutils.c | 176 ++++++++++++++++++++++++++++++++++++++
> libavutil/parseutils.h | 102 ++++++++++++++++++++++
> libavutil/version.h | 2 +-
> 6 files changed, 296 insertions(+), 170 deletions(-)
LGTM
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 3/5] reindent after last commit
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: split loop for parsing and validation of -stats_* specifiers Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 2/5] fftools/ffmpeg: move parsing of -stats_* specifiers to lavu/parseutils Thilo Borgmann via ffmpeg-devel
@ 2023-12-11 15:07 ` Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 4/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 5/5] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel
4 siblings, 0 replies; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-11 15:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
---
fftools/ffmpeg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index cb4d90c7b2..f169801366 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -440,7 +440,7 @@ enum forced_keyframes_const {
typedef struct EncStats {
AVEncStatsComponent *components;
- int nb_components;
+ int nb_components;
AVIOContext *io;
} EncStats;
--
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] 20+ messages in thread* [FFmpeg-devel] [PATCH 4/5] avfilter: Add fsync filter
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
` (2 preceding siblings ...)
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 3/5] reindent after last commit Thilo Borgmann via ffmpeg-devel
@ 2023-12-11 15:07 ` Thilo Borgmann via ffmpeg-devel
2023-12-11 15:14 ` Thilo Borgmann via ffmpeg-devel
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 5/5] fate: Add fsync filter tests Thilo Borgmann via ffmpeg-devel
4 siblings, 1 reply; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-11 15:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
pu
---
Changelog | 1 +
doc/filters.texi | 52 ++++++
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/vf_fsync.c | 376 +++++++++++++++++++++++++++++++++++++++
libavformat/version.h | 2 +-
6 files changed, 432 insertions(+), 1 deletion(-)
create mode 100644 libavfilter/vf_fsync.c
diff --git a/Changelog b/Changelog
index f00bc27ca4..9cc441e9d4 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version <next>:
- EVC encoding using external library libxeve
- QOA decoder and demuxer
- aap filter
+- fsync filter
version 6.1:
- libaribcaption decoder
diff --git a/doc/filters.texi b/doc/filters.texi
index 6d00ba2c3f..4ed12b83ac 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14681,6 +14681,58 @@ 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.
+
+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.
+Each line must contain at least one input PTS @code{@{ptsi@}}, one output PTS @code{@{pts@}} and one output timebase @code{@{tb@}}.
+Use the @code{format, fmt} option to specify which information is present in each line of the input file.
+
+@item format, fmt
+A format string describing the line format of the map file.
+It uses the same directives as the ffmpeg options @code{-stats_mux_pre_fmt}.
+The default value is @code{@{ptsi@} @{pts@} @{tb@}} which contains only the required information.
+@end table
+
+Some examples:
+@itemize
+@item Using the default format of the filter:
+@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
+
+@item Using a custom format:
+@example
+# Convert a video to 25 fps and record a MAP_FILE file with a custom line format
+ffmpeg -i INPUT -vf fps=fps=25 -stats_mux_pre MAP_FILE -stats_mux_pre_fmt "@{n@} @{pts@} @{tb@} @{ni@} @{ptsi@} @{tbi@}" OUTPUT
+
+# Sort MAP_FILE by increasing input PTS
+sort -k 4 -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:fmt=@{n@} @{pts@} @{tb@} @{ni@} @{ptsi@} @{tbi@}[ref];[1:v][ref]ssim' -f null -
+@end example
+@end itemize
+
@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/vf_fsync.c b/libavfilter/vf_fsync.c
new file mode 100644
index 0000000000..3d2027d007
--- /dev/null
+++ b/libavfilter/vf_fsync.c
@@ -0,0 +1,376 @@
+/*
+ * 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 "libavutil/parseutils.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 *format; // user-specified line format according to -stats_enc* options
+ char *format_str; // sscanf compatible user-specified line format of the 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
+ int64_t *param[4]; // mapping of ptsi, pts, tb_num, tb_den into user-specified format
+} 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 }, \
+ { "format", "set the line format", OFFSET(format), AV_OPT_TYPE_STRING, { .str = "{ptsi} {pts} {tb}" }, .flags=FLAGS }, \
+ { "fmt", "set the line format", OFFSET(format), AV_OPT_TYPE_STRING, { .str = "{ptsi} {pts} {tb}" }, .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) {
+ av_log(ctx, AV_LOG_DEBUG, "format = %s\n", s->format);
+
+ // default: av_sscanf(s->cur, "{ptsi} {pts} {tb}", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
+ ret = av_sscanf(s->cur, s->format_str, s->param[0], s->param[1], s->param[2], s->param[3]);
+ 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
+ // default: av_sscanf(s->cur, "{ptsi} {pts} {tb}", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
+ ret = av_sscanf(s->cur, s->format_str, s->param[0], s->param[1], s->param[2], s->param[3]);
+ 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;
+ AVEncStatsComponent *components = NULL;
+ int nb_components = 0;
+ int ret, i;
+ int j = 0;
+ int has_i = 0;
+ int has_o = 0;
+ int has_t = 0;
+
+ av_log(ctx, AV_LOG_DEBUG, "filename: %s\n", s->filename);
+
+ s->buf = av_malloc(BUF_SIZE);
+ if (!s->buf)
+ return AVERROR(ENOMEM);
+
+ s->format_str = av_mallocz(BUF_SIZE);
+ if (!s->format_str)
+ 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;
+
+ // parse format into format_str for av_sscanf
+ ret = av_parse_enc_stats_components(&components, &nb_components, s->format);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < nb_components; i++) {
+ AVEncStatsComponent *c = &components[i];
+ switch (c->type) {
+ default:
+ av_log(ctx, AV_LOG_ERROR, "Unknown format specifier: %i {%s}\n", c->type, c->str);
+ return AVERROR(EINVAL);
+ case ENC_STATS_LITERAL:
+ if (c->str) {av_strlcat(s->format_str, c->str, BUF_SIZE); continue;}
+ else continue;
+ case ENC_STATS_FILE_IDX: av_strlcat(s->format_str, "%*d", BUF_SIZE); continue;
+ case ENC_STATS_STREAM_IDX: av_strlcat(s->format_str, "%*d", BUF_SIZE); continue;
+ case ENC_STATS_FRAME_NUM: av_strlcat(s->format_str, "%*"PRIu64, BUF_SIZE); continue;
+ case ENC_STATS_FRAME_NUM_IN: av_strlcat(s->format_str, "%*"PRIu64, BUF_SIZE); continue;
+ case ENC_STATS_TIMEBASE: {av_strlcat(s->format_str, "%d/%d", BUF_SIZE);
+ if (!has_t) {s->param[j++] = &s->tb_num;
+ s->param[j++] = &s->tb_den;
+ has_t = 1;} continue;}
+ case ENC_STATS_TIMEBASE_IN: av_strlcat(s->format_str, "%*d/%*d", BUF_SIZE); continue;
+ case ENC_STATS_PTS: {av_strlcat(s->format_str, "%"PRId64, BUF_SIZE);
+ if (!has_o) {s->param[j++] = &s->pts;
+ has_o = 1;} continue;}
+ case ENC_STATS_PTS_TIME: av_strlcat(s->format_str, "%*g", BUF_SIZE); continue;
+ case ENC_STATS_PTS_IN: {av_strlcat(s->format_str, "%"PRId64, BUF_SIZE);
+ if (!has_i) {s->param[j++] = &s->ptsi;
+ has_i = 1;} continue;}
+ case ENC_STATS_PTS_TIME_IN: av_strlcat(s->format_str, "%*g", BUF_SIZE); continue;
+ case ENC_STATS_DTS: av_strlcat(s->format_str, "%*"PRId64, BUF_SIZE); continue;
+ case ENC_STATS_DTS_TIME: av_strlcat(s->format_str, "%*g", BUF_SIZE); continue;
+ case ENC_STATS_SAMPLE_NUM: av_strlcat(s->format_str, "%*"PRIu64, BUF_SIZE); continue;
+ case ENC_STATS_NB_SAMPLES: av_strlcat(s->format_str, "%*d", BUF_SIZE); continue;
+ case ENC_STATS_PKT_SIZE: av_strlcat(s->format_str, "%*d", BUF_SIZE); continue;
+ case ENC_STATS_BITRATE: av_strlcat(s->format_str, "%*g", BUF_SIZE); continue;
+ case ENC_STATS_AVG_BITRATE: av_strlcat(s->format_str, "%*g", BUF_SIZE); continue;
+ }
+ av_freep(c->str);
+ }
+ av_freep(&components);
+
+ // check for all necessary specifiers found
+ if (j != 4) {
+ if (!has_i) av_log(ctx, AV_LOG_ERROR, "Format specifier {ptsi} missing in format string\n");
+ if (!has_o) av_log(ctx, AV_LOG_ERROR, "Format specifier {pts} missing in format string\n");
+ if (!has_t) av_log(ctx, AV_LOG_ERROR, "Format specifier {tb} missing in format string\n");
+ return AVERROR(EINVAL);
+ }
+
+ return 0;
+}
+
+static av_cold void fsync_uninit(AVFilterContext *ctx)
+{
+ FsyncContext *s = ctx->priv;
+
+ avio_close(s->avio_ctx);
+ av_freep(&s->buf);
+ av_freep(&s->format_str);
+ 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,
+};
diff --git a/libavformat/version.h b/libavformat/version.h
index 6a80f3ac4e..e063e12b98 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -31,7 +31,7 @@
#include "version_major.h"
-#define LIBAVFORMAT_VERSION_MINOR 18
+#define LIBAVFORMAT_VERSION_MINOR 19
#define LIBAVFORMAT_VERSION_MICRO 100
#define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
--
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] 20+ messages in thread* [FFmpeg-devel] [PATCH 5/5] fate: Add fsync filter tests
2023-12-11 15:07 [FFmpeg-devel] [PATCH 0/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
` (3 preceding siblings ...)
2023-12-11 15:07 ` [FFmpeg-devel] [PATCH 4/5] avfilter: Add fsync filter Thilo Borgmann via ffmpeg-devel
@ 2023-12-11 15:07 ` Thilo Borgmann via ffmpeg-devel
4 siblings, 0 replies; 20+ messages in thread
From: Thilo Borgmann via ffmpeg-devel @ 2023-12-11 15:07 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Thilo Borgmann
---
tests/Makefile | 6 +++-
tests/fate/filter-video.mak | 8 +++++
tests/filtergraphs/fsync-down | 2 ++
tests/filtergraphs/fsync-up | 2 ++
tests/maps/fsync-down | 7 ++++
tests/maps/fsync-up | 57 +++++++++++++++++++++++++++++
tests/ref/fate/filter-fsync-down | 12 +++++++
tests/ref/fate/filter-fsync-up | 62 ++++++++++++++++++++++++++++++++
8 files changed, 155 insertions(+), 1 deletion(-)
create mode 100644 tests/filtergraphs/fsync-down
create mode 100644 tests/filtergraphs/fsync-up
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 e4bdf59db9..e5af5bf7a5 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -395,6 +395,14 @@ 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/filtergraphs/fsync-up
+fate-filter-fsync-up: tests/data/maps/fsync-up
+fate-filter-fsync-up: CMD = framecrc -filter_complex_script $(TARGET_PATH)/tests/data/filtergraphs/fsync-up
+fate-filter-fsync-down: tests/data/filtergraphs/fsync-down
+fate-filter-fsync-down: tests/data/maps/fsync-down
+fate-filter-fsync-down: CMD = framecrc -filter_complex_script $(TARGET_PATH)/tests/data/filtergraphs/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/filtergraphs/fsync-down b/tests/filtergraphs/fsync-down
new file mode 100644
index 0000000000..56df6a6d52
--- /dev/null
+++ b/tests/filtergraphs/fsync-down
@@ -0,0 +1,2 @@
+testsrc2=r=25:d=1 [ref];
+[ref] fsync=f=tests/data/maps/fsync-down:fmt={ptsi} {tbi} {pts} {tb}
diff --git a/tests/filtergraphs/fsync-up b/tests/filtergraphs/fsync-up
new file mode 100644
index 0000000000..e7a5c37728
--- /dev/null
+++ b/tests/filtergraphs/fsync-up
@@ -0,0 +1,2 @@
+testsrc2=r=25:d=1 [ref];
+[ref] fsync=f=tests/data/maps/fsync-up:fmt={pts} {tb} {ptsi} {tbi}
diff --git a/tests/maps/fsync-down b/tests/maps/fsync-down
new file mode 100644
index 0000000000..536a993dd0
--- /dev/null
+++ b/tests/maps/fsync-down
@@ -0,0 +1,7 @@
+1 1/25 0 1/7
+5 1/25 1 1/7
+8 1/25 2 1/7
+12 1/25 3 1/7
+16 1/25 4 1/7
+19 1/25 5 1/7
+23 1/25 6 1/7
diff --git a/tests/maps/fsync-up b/tests/maps/fsync-up
new file mode 100644
index 0000000000..7748b362e7
--- /dev/null
+++ b/tests/maps/fsync-up
@@ -0,0 +1,57 @@
+0 1/57 0 1/25
+1 1/57 0 1/25
+2 1/57 1 1/25
+3 1/57 1 1/25
+4 1/57 1 1/25
+5 1/57 2 1/25
+6 1/57 2 1/25
+7 1/57 3 1/25
+8 1/57 3 1/25
+9 1/57 4 1/25
+10 1/57 4 1/25
+11 1/57 5 1/25
+12 1/57 5 1/25
+13 1/57 5 1/25
+14 1/57 6 1/25
+15 1/57 6 1/25
+16 1/57 7 1/25
+17 1/57 7 1/25
+18 1/57 8 1/25
+19 1/57 8 1/25
+20 1/57 8 1/25
+21 1/57 9 1/25
+22 1/57 9 1/25
+23 1/57 10 1/25
+24 1/57 10 1/25
+25 1/57 11 1/25
+26 1/57 11 1/25
+27 1/57 12 1/25
+28 1/57 12 1/25
+29 1/57 12 1/25
+30 1/57 13 1/25
+31 1/57 13 1/25
+32 1/57 14 1/25
+33 1/57 14 1/25
+34 1/57 15 1/25
+35 1/57 15 1/25
+36 1/57 16 1/25
+37 1/57 16 1/25
+38 1/57 16 1/25
+39 1/57 17 1/25
+40 1/57 17 1/25
+41 1/57 18 1/25
+42 1/57 18 1/25
+43 1/57 19 1/25
+44 1/57 19 1/25
+45 1/57 19 1/25
+46 1/57 20 1/25
+47 1/57 20 1/25
+48 1/57 21 1/25
+49 1/57 21 1/25
+50 1/57 22 1/25
+51 1/57 22 1/25
+52 1/57 23 1/25
+53 1/57 23 1/25
+54 1/57 23 1/25
+55 1/57 24 1/25
+56 1/57 24 1/25
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] 20+ messages in thread