Netflix/vmaf

(FFmpeg) Getting identical VMAF scores for `pool=mean` and `pool=harmonic_mean`

t-nil opened this issue · 6 comments

t-nil commented 
for i in mean, min, harmonic_mean; ffmpeg -i test.mp4 -i ref.mp4 -filter_complex \
              "[0:v]setpts=PTS-STARTPTS[distorted]; \
               [1:v]setpts=PTS-STARTPTS[ref]; \
               [distorted][ref]libvmaf=n_threads=13:log_fmt=json:pool=$i" -f null - &| grep VMAF; end

The min setting produces a different, lower score, which seems sane. I tried three different samples and always got the exact same results for the other two pooling strategies.

I am using libvmaf v2.3.1 with FFmpeg 6.1 on latest manjaro Manjaro. The builds should be from official (archlinux) sources.

Can you reproduce? Could something be broken?

nilfm commented

Thanks for the report. I have been able to reproduce and I see what is wrong. In ffmpeg's vf_libvmaf filter, the function pool_method_map is wrong, see the implementation here. It's using av_stristr(s, t), which checks whether t is a substring of s. However, mean is a substring of harmonic_mean, so that branch gets executed.

@kylophone FYI. I think the fix just involves using strcmp to compare the strings directly, like this:

rom fbed385b2b9de3483c18ebe60f17e94f2ac1f5b8 Mon Sep 17 00:00:00 2001
From: nilfm <nilf@netflix.com>
Date: Mon, 27 Nov 2023 12:14:41 +0000
Subject: [PATCH] avfilter/libvmaf.c: fix pool_method_map for harmonic mean

---
 libavfilter/vf_libvmaf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 12810b7267..7b92d9199d 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -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.42.0
nilfm commented

BTW: there are a few other instances of av_stristr in the filter code, and I believe they should all be strcmp. This is technically a breaking change, but it is consistent with the documentation. The docs are also missing the pool parameter.

t-nil commented
kylophone commented

The official docs missing the pool parameter is an FFmpeg bug. I reported it and they promised to fix.

Thanks for the report, this is fixed now: https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/0ea9e26636387336c48d6d20872ddd277479817b

nilfm commented

I submitted a patch for the substring comparison bug: https://ffmpeg.org/pipermail/ffmpeg-devel/2023-December/317767.html

nilfm commented

The fix is merged: https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/5f4b7bf2b5b675a4a1dfc3b64c1c5dd03d80f278

Thanks again for raising this! Closing the issue as resolved.