* [FFmpeg-devel] [PATCH] avfilter/vf_libvmaf: fix string comparison bug
@ 2023-12-04 15:58 Nil Fons Miret via ffmpeg-devel
2023-12-06 17:54 ` Kyle Swanson
0 siblings, 1 reply; 3+ messages in thread
From: Nil Fons Miret via ffmpeg-devel @ 2023-12-04 15:58 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Nil Fons Miret, Kyle Swanson
The libvmaf filter was doing substring checks in place of string
equality comparisons. This led to a bug when the user specified the
pooling method "harmonic_mean", since "mean" was checked first and the
substring comparison returned true. This patch changes all substring
comparisons for string equality comparisons. This is both correct and
more efficient than the existing method.
Signed-off-by: nilfm <nilf@netflix.com>
---
libavfilter/vf_libvmaf.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 12810b7267..46ff8154ef 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -251,7 +251,7 @@ static int parse_features(AVFilterContext *ctx)
const AVDictionaryEntry *e = NULL;
while (e = av_dict_iterate(dict[i], e)) {
- if (av_stristr(e->key, "name")) {
+ if (!strcmp(e->key, "name")) {
feature_name = e->value;
continue;
}
@@ -312,29 +312,29 @@ static int parse_models(AVFilterContext *ctx)
char *path = NULL;
while (e = av_dict_iterate(dict[i], e)) {
- if (av_stristr(e->key, "disable_clip")) {
- model_cfg.flags |= av_stristr(e->value, "true") ?
+ if (!strcmp(e->key, "disable_clip")) {
+ model_cfg.flags |= !strcmp(e->value, "true") ?
VMAF_MODEL_FLAG_DISABLE_CLIP : 0;
continue;
}
- if (av_stristr(e->key, "enable_transform")) {
- model_cfg.flags |= av_stristr(e->value, "true") ?
+ if (!strcmp(e->key, "enable_transform")) {
+ model_cfg.flags |= !strcmp(e->value, "true") ?
VMAF_MODEL_FLAG_ENABLE_TRANSFORM : 0;
continue;
}
- if (av_stristr(e->key, "name")) {
+ if (!strcmp(e->key, "name")) {
model_cfg.name = e->value;
continue;
}
- if (av_stristr(e->key, "version")) {
+ if (!strcmp(e->key, "version")) {
version = e->value;
continue;
}
- if (av_stristr(e->key, "path")) {
+ if (!strcmp(e->key, "path")) {
path = e->value;
continue;
}
@@ -529,13 +529,13 @@ static int activate(AVFilterContext *ctx)
static enum VmafOutputFormat log_fmt_map(const char *log_fmt)
{
if (log_fmt) {
- if (av_stristr(log_fmt, "xml"))
+ if (!strcmp(log_fmt, "xml"))
return VMAF_OUTPUT_FORMAT_XML;
- if (av_stristr(log_fmt, "json"))
+ if (!strcmp(log_fmt, "json"))
return VMAF_OUTPUT_FORMAT_JSON;
- if (av_stristr(log_fmt, "csv"))
+ if (!strcmp(log_fmt, "csv"))
return VMAF_OUTPUT_FORMAT_CSV;
- if (av_stristr(log_fmt, "sub"))
+ if (!strcmp(log_fmt, "sub"))
return VMAF_OUTPUT_FORMAT_SUB;
}
@@ -545,11 +545,11 @@ static enum VmafOutputFormat log_fmt_map(const
char *log_fmt)
static enum VmafPoolingMethod pool_method_map(const char *pool_method)
{
if (pool_method) {
- if (av_stristr(pool_method, "min"))
+ if (!strcmp(pool_method, "min"))
return VMAF_POOL_METHOD_MIN;
- if (av_stristr(pool_method, "mean"))
+ if (!strcmp(pool_method, "mean"))
return VMAF_POOL_METHOD_MEAN;
- if (av_stristr(pool_method, "harmonic_mean"))
+ if (!strcmp(pool_method, "harmonic_mean"))
return VMAF_POOL_METHOD_HARMONIC_MEAN;
}
--
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] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/vf_libvmaf: fix string comparison bug
2023-12-04 15:58 [FFmpeg-devel] [PATCH] avfilter/vf_libvmaf: fix string comparison bug Nil Fons Miret via ffmpeg-devel
@ 2023-12-06 17:54 ` Kyle Swanson
2023-12-08 18:41 ` Kyle Swanson
0 siblings, 1 reply; 3+ messages in thread
From: Kyle Swanson @ 2023-12-06 17:54 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Nil Fons Miret, Kyle Swanson
Hi,
On Mon, Dec 4, 2023 at 7:59 AM Nil Fons Miret via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> The libvmaf filter was doing substring checks in place of string
> equality comparisons. This led to a bug when the user specified the
> pooling method "harmonic_mean", since "mean" was checked first and the
> substring comparison returned true. This patch changes all substring
> comparisons for string equality comparisons. This is both correct and
> more efficient than the existing method.
>
> Signed-off-by: nilfm <nilf@netflix.com>
> ---
> libavfilter/vf_libvmaf.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
> index 12810b7267..46ff8154ef 100644
> --- a/libavfilter/vf_libvmaf.c
> +++ b/libavfilter/vf_libvmaf.c
> @@ -251,7 +251,7 @@ static int parse_features(AVFilterContext *ctx)
> const AVDictionaryEntry *e = NULL;
>
> while (e = av_dict_iterate(dict[i], e)) {
> - if (av_stristr(e->key, "name")) {
> + if (!strcmp(e->key, "name")) {
> feature_name = e->value;
> continue;
> }
> @@ -312,29 +312,29 @@ static int parse_models(AVFilterContext *ctx)
> char *path = NULL;
>
> while (e = av_dict_iterate(dict[i], e)) {
> - if (av_stristr(e->key, "disable_clip")) {
> - model_cfg.flags |= av_stristr(e->value, "true") ?
> + if (!strcmp(e->key, "disable_clip")) {
> + model_cfg.flags |= !strcmp(e->value, "true") ?
> VMAF_MODEL_FLAG_DISABLE_CLIP : 0;
> continue;
> }
>
> - if (av_stristr(e->key, "enable_transform")) {
> - model_cfg.flags |= av_stristr(e->value, "true") ?
> + if (!strcmp(e->key, "enable_transform")) {
> + model_cfg.flags |= !strcmp(e->value, "true") ?
> VMAF_MODEL_FLAG_ENABLE_TRANSFORM : 0;
> continue;
> }
>
> - if (av_stristr(e->key, "name")) {
> + if (!strcmp(e->key, "name")) {
> model_cfg.name = e->value;
> continue;
> }
>
> - if (av_stristr(e->key, "version")) {
> + if (!strcmp(e->key, "version")) {
> version = e->value;
> continue;
> }
>
> - if (av_stristr(e->key, "path")) {
> + if (!strcmp(e->key, "path")) {
> path = e->value;
> continue;
> }
> @@ -529,13 +529,13 @@ static int activate(AVFilterContext *ctx)
> static enum VmafOutputFormat log_fmt_map(const char *log_fmt)
> {
> if (log_fmt) {
> - if (av_stristr(log_fmt, "xml"))
> + if (!strcmp(log_fmt, "xml"))
> return VMAF_OUTPUT_FORMAT_XML;
> - if (av_stristr(log_fmt, "json"))
> + if (!strcmp(log_fmt, "json"))
> return VMAF_OUTPUT_FORMAT_JSON;
> - if (av_stristr(log_fmt, "csv"))
> + if (!strcmp(log_fmt, "csv"))
> return VMAF_OUTPUT_FORMAT_CSV;
> - if (av_stristr(log_fmt, "sub"))
> + if (!strcmp(log_fmt, "sub"))
> return VMAF_OUTPUT_FORMAT_SUB;
> }
>
> @@ -545,11 +545,11 @@ static enum VmafOutputFormat log_fmt_map(const
> char *log_fmt)
> static enum VmafPoolingMethod pool_method_map(const char *pool_method)
> {
> if (pool_method) {
> - if (av_stristr(pool_method, "min"))
> + if (!strcmp(pool_method, "min"))
> return VMAF_POOL_METHOD_MIN;
> - if (av_stristr(pool_method, "mean"))
> + if (!strcmp(pool_method, "mean"))
> return VMAF_POOL_METHOD_MEAN;
> - if (av_stristr(pool_method, "harmonic_mean"))
> + if (!strcmp(pool_method, "harmonic_mean"))
> return VMAF_POOL_METHOD_HARMONIC_MEAN;
> }
>
> --
> 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".
Will push this tomorrow.
Thanks,
Kyle
_______________________________________________
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] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/vf_libvmaf: fix string comparison bug
2023-12-06 17:54 ` Kyle Swanson
@ 2023-12-08 18:41 ` Kyle Swanson
0 siblings, 0 replies; 3+ messages in thread
From: Kyle Swanson @ 2023-12-08 18:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Nil Fons Miret, Kyle Swanson
On Wed, Dec 6, 2023 at 9:54 AM Kyle Swanson <k@ylo.ph> wrote:
> Will push this tomorrow.
Pushed, thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-08 18:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-04 15:58 [FFmpeg-devel] [PATCH] avfilter/vf_libvmaf: fix string comparison bug Nil Fons Miret via ffmpeg-devel
2023-12-06 17:54 ` Kyle Swanson
2023-12-08 18:41 ` Kyle Swanson
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