xiph / awcy

http://arewecompressedyet.com/
MIT License
72 stars 46 forks source link

Build and install VMAF #194

Closed ltrudeau-twoorioles closed 3 years ago

ltrudeau-twoorioles commented 4 years ago

Fixes #193

I'm not sure how to test this patch, so I can't guarantee that it works. It might also be worth it to use this version of VMAF when computing VMAF scores.

rzumer commented 4 years ago

Seems that libaom is not built with VMAF support, so I get the following error on a run with --tune=vmaf:

Error: Tried to set control 24 = 6
Failed to control codec: Invalid parameter
    This error may be related to the wrong configuration options: try to set -DCONFIG_TUNE_VMAF=1 at the time CMake is run.
ltrudeau-twoorioles commented 4 years ago

Seems that libaom is not built with VMAF support, so I get the following error on a run with --tune=vmaf:

Error: Tried to set control 24 = 6
Failed to control codec: Invalid parameter
    This error may be related to the wrong configuration options: try to set -DCONFIG_TUNE_VMAF=1 at the time CMake is run.

Yes, you need to build with DCONFIG_TUNE_VMAF=1

Screen Shot 2020-02-10 at 2 52 23 PM
rzumer commented 4 years ago

Sorry, what I meant is we should probably add that to the default configuration for AOM builds. I am pretty sure we are not just using default values, so it's probably defined somewhere in this repo.

If it has a significant impact on build time or performance, then leaving it off by default is fine.

rzumer commented 4 years ago

I built the wrong branch while testing, while correcting this I found that libvmaf doesn't build correctly.

test/meson.build:3:0: ERROR:  Include directory to be added is not an include directory object.

A full log can be found at /opt/app/vmaf/libvmaf/build/meson-logs/meson-log.txt
The command '/bin/sh -c apt-get install -y meson &&     git clone https://github.com/Netflix/vmaf.git &&         cd vmaf/libvmaf &&      meson build --buildtype release &&      ninja -vC build install' returned a non-zero code: 1

The full log does not actually exist, so I'm not exactly sure what is wrong.

ltrudeau-twoorioles commented 4 years ago

@rzumer please try again and let me know if it works now

rzumer commented 4 years ago

Still no luck, the same error comes up, just with the nonexistent log path starting with /opt/vmaf instead of /opt/app/vmaf.

ltrudeau-twoorioles commented 4 years ago

It might be the meson version

rzumer commented 4 years ago

The libvmaf readme says "0.47 or above", and the installed version at build time is 0.49, so it seems like it should work.

ltrudeau-twoorioles commented 4 years ago

Should now use the latest meson

rzumer commented 4 years ago

It now fails on dav1d build.

Successfully built meson
Installing collected packages: meson
  The script meson is installed in '/root/.local/bin' which is not on PATH.
  Consider adding this directory to PATH or, if you prefer to suppress this warning, use --no-warn-script-location.
Successfully installed meson-0.53.1
Removing intermediate container cd808b8c11ff
 ---> 8246cfd01079
Step 19/30 : ENV        DAV1D_DIR=/opt/dav1d
 ---> Running in de7ca417b40f
Removing intermediate container de7ca417b40f
 ---> 001f34dd6f6e
Step 20/30 : RUN        git clone https://code.videolan.org/videolan/dav1d.git ${DAV1D_DIR} &&  cd ${DAV1D_DIR} &&       mkdir build && cd build &&      meson .. &&     ninja
 ---> Running in bcab36d09301
Cloning into '/opt/dav1d'...
/bin/sh: 1: meson: not found
The command '/bin/sh -c git clone https://code.videolan.org/videolan/dav1d.git ${DAV1D_DIR} &&  cd ${DAV1D_DIR} &&       mkdir build && cd build &&      meson .. &&     ninja' returned a non-zero code: 127
ltrudeau-twoorioles commented 4 years ago

I did not know Docker always runs as root.

Now that dav1d is broken, feels like https://xkcd.com/349/ is appropriate.

rzumer commented 4 years ago

Thanks, the build is good now, I'll do a test run.

rzumer commented 4 years ago

So far no encode has managed to complete on default settings with --tune=vmaf, so I launched a new run using --cpu-used=8. If this does not generate results then it may be stuck somehow.

dwbuiten commented 4 years ago
[19:58] < Daemon404> Chagall, i tried it on a single 1080p intra frame just now. normal: 0m2.815s / tune=vmaf: 1m38s.
[19:58] < Daemon404> i understand what is going on now.
[19:58] < Daemon404> the speed of the VMAF step relies entirely on how many CPUs you have.
[19:59] < Daemon404> (libvmaf saturates cores)
[19:59] < Chagall> ah ok
[19:59] < Daemon404> hope you like DoSing awcy!
[...]
[20:01] < Daemon404> the inter frames will be even slower.
[20:01] < Daemon404> also note with 8 cores mine is ~50x slower on just an intra frame.
[20:02] < Chagall> ok
[20:02] < Daemon404> (on inter frames it calls vmaf motion on N frames to do its sketchy RD)
[20:02] < Chagall> then maybe we'll merge tonight or tomorrow
[20:02] < Daemon404> i dont see how this can work well on AWCY tbh
[20:02] < Daemon404> itll totally DoS the workers
[20:02] < Chagall> yeah true
[20:02] < Chagall> if google starts queuing 10 things with --tune=vmaf...
[20:02] < Daemon404> and libaom does not limit libvmaf's threads.
[20:02] < Daemon404> afaict
[20:03] < Chagall> how about we add pop-up confirmation dialogs
[20:03] < Daemon404> also it muddies up the filesystem it seems.
[20:03] < Chagall> are you sure you want to do this?
[20:03] < Chagall> are you sure you are sure you really want to do this?
[20:03] < Chagall> then if they press yes send them an invoice to obtain the password
[20:03] < Daemon404> https://aomedia.googlesource.com/aom/+/refs/heads/master/aom_dsp/vmaf.c#110
[20:04] < Daemon404> note it reads vmaf data from a xml file written by the lib
[20:04] < Daemon404> and also it does not unlink it.
[20:04] < Chagall> unlink?
[20:04] < Daemon404> delete
[20:04] < soichiro> remove
[20:04] < Chagall> ah
[20:04] < soichiro> I can only imagine if this code was submitted to the linux kernel
[20:04] < soichiro> the rant from Linus would be beautiful
[20:05] < Chagall> I wonder what happens if multiple files are encoded at the same time, what happens to vmaf_scores.xml...
[20:05] < soichiro> Probably bad things :)
[20:05] < Chagall> right
[20:06] < Chagall> can you comment about that in the awcy pull request?
[20:06] < Chagall> Daemon404
[20:06] < Chagall> https://github.com/xiph/awcy/pull/194
rzumer commented 4 years ago

Retracting my approval until we know how this would impact overall performance on AWCY.

rzumer commented 4 years ago

A test run on a VM shows encode times of about 50-75x those of the same files and configuration without --tune=vmaf. In my opinion this PR could be merged despite those performance concerns if libvmaf is compiled conditionally and disabled by default. I don't think it should be deployed in its current state on arewecompressedyet.com, because it has limited resource and a VMAF run would stall other people's work, but merging this with conditional support would allow others to deploy their own AWCY instance for it.

tdaede commented 4 years ago

Supposedly it's faster now. Can you retest?

rzumer commented 4 years ago

@ltrudeau-twoorioles It looks like VMAF also needs to be installed in Dockerfile.worker in order for external workers to work with it, can you please add the necessary lines there as well?

rzumer commented 4 years ago

If this PR gets updated please also add instructions for installing libvmaf in the readme, for standalone deployment.

rzumer commented 4 years ago

I retested it and can confirm that the encoding speed with --tune=vmaf is now much better. With otherwise default settings the first encode in objective-1-fast finishes in a more reasonable amount of time. That said it is still very slow and still saturates worker slots, so I think it might be dangerous to enable on the same AWCY instance as is used for less demanding runs.

As for the hardcoded stats file name, it should not be a concern on AWCY since each encode has its own directory.

Since awcy.com does not run off Docker, I think merging this is fine, once the worker installation also includes libvmaf. Making libvmaf optional through a command line setting would be a plus if that's possible.

tdaede commented 3 years ago

I am going to land this and then do another PR to fix up the worker Dockerfile.