1493400 - Vendor dav1d and create MediaDataDecoder wrapper

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Thomas Daede [:TD-Linux]]

Parse ninja files and generate mozbuild files.

Comment 1

[Jean-Yves Avenard [:jya]]

Thomas, is this something you're currently looking at?

Comment 2

[Thomas Daede [:TD-Linux]]

I'm not looking at it quite yet. That said, it should be significantly easier than libaom, because dav1d uses meson which can output ninja files, and we can use the same vendoring system as ANGLE, I think.

Comment 3

[Dan Minor [:dminor]]

(In reply to Thomas Daede [:TD-Linux] from comment #2)
> I'm not looking at it quite yet. That said, it should be significantly
> easier than libaom, because dav1d uses meson which can output ninja files,
> and we can use the same vendoring system as ANGLE, I think.

Unfortunately, it looks like ANGLE is relying upon 'gn' to generate json output to get the list of files to build [1], so I don't think meson's ninja backend will be enough to get this working. Adding a json or moz.build backend to Meson might be one way ahead, or perhaps we could parse the ninja output itself.

[1] https://searchfox.org/mozilla-central/rev/3a54520d8d2319a4116866371ed3d9ed2ec0cc2b/gfx/angle/update-angle.py#159

Comment 4

[Alex Chronopoulos [:achronop]]

Repo: https://code.videolan.org/videolan/dav1d

Comment 5

[Alex Chronopoulos [:achronop]]

In order to engage the fuzzy team we need to file a PI request (better in advance). We also need to mention that the existing AV1 decoder already has a fuzzing target in-tree and the new decoder may need a second one.

Comment 6

[Alex Chronopoulos [:achronop]]

:decoder pointed out that dav1d comes with a fuzzing target and it can be used with minor adaptations:
https://code.videolan.org/videolan/dav1d/blob/master/tests/libfuzzer/dav1d_fuzzer.c

Comment 7

[Thomas Daede [:TD-Linux]]

Attachment: Bug 1493400 - Import dav1d into tree.

Comment 8

[Alex Chronopoulos [:achronop]]

Attachment: add-media-libdav1d

WIP: This is for building in Linux. I am using config files from ninja parsing.

Comment 9

[Alex Chronopoulos [:achronop]]

Shared symbols with libaom
https://code.videolan.org/videolan/dav1d/issues/104 (fixed)

YASM vs NASM errors:
https://code.videolan.org/videolan/dav1d/issues/114

Comment 10

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Update dav1d upstream to 2cce1d2.

Depends on D9607

Comment 11

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Rename folder dav1d to libdav1d in media folder.

Comment 12

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=027943797230fe587b4f92313ded0d760987bb53

Comment 13

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3995f54a926d6b50cd5c68c00dcb3e69c3221ec4

Comment 14

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a6ce456d1a1411e87bea0e9c875963a9f11af3e

Comment 15

[Jean-Yves Avenard [:jya]]

:achronop, don't you plan to write the MediaDataDecoder wrapper for it?
pushing just dav1d to the tree like that would just take space for no reason.

Comment 16

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=15e38b107d4639a16369cae9c27614089bb96f2a

Comment 17

[Alex Chronopoulos [:achronop]]

Try run from comment 16 has NASM enabled as implemented in Bug 1501796. 

There are several build errors in *.asm files in Linux64. I suspect that the errors occur due to different NASM version used by try (2.10 on try, 2.13 locally). Locally I build successfully on Linux64.

Also, for windows errors, NASM is not enabled yet in window platform.

Comment 18

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ca4eda39a0f5d019b2e61e1e32e58c29af54f24

Comment 19

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e76515f5cc19c54405dbc9ef103f799f4484fa61

Comment 20

[Alex Chronopoulos [:achronop]]

dav1d issue about android link error:
https://code.videolan.org/videolan/dav1d/issues/136

Comment 21

[Thomas Daede [:TD-Linux]]

Because dav1d currently doesn't have arm assembly, we can probably just rely on libaom on Android for now.

Comment 22

[Alex Chronopoulos [:achronop]]

Also more link errors in Android:

[task 2018-10-31T17:53:43.520Z] 17:53:43     INFO -  /builds/worker/workspace/build/src/android-ndk/toolchains/x86-4.9/prebuilt/linux-x86_64/lib/gcc/i686-linux-android/4.9.x/../../../../i686-linux-android/bin/ld: error: read-only segment has dynamic relocations
[task 2018-10-31T17:53:43.520Z] 17:53:43     INFO -  /builds/worker/workspace/build/src/android-ndk/toolchains/x86-4.9/prebuilt/linux-x86_64/lib/gcc/i686-linux-android/4.9.x/../../../../i686-linux-android/bin/ld: error: hidden symbol 'posix_memalign' is not defined locally

We build with `D__ANDROID_API__=16` which corresponds to 4.1

According to https://github.com/android-ndk/ndk/issues/647 the method `posix_memalign` is available in 4.2 (api 17).

Comment 23

[Thomas Daede [:TD-Linux]]

I have filed an issue in dav1d to track adding a fallback for API 16: https://code.videolan.org/videolan/dav1d/issues/140

Comment 24

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49b13561ab74779f0b90b5f05b43d5b48677a805

Comment 25

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d03b9a0d564cf8183d463764d267727656f7a001

Comment 26

[Alex Chronopoulos [:achronop]]

That's a green try building dav1d without ASM:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b01756ed2e4ba246b141ad153dbfdbbd11a58e27

Comment 27

[Alex Chronopoulos [:achronop]]

The wrapper implementation is blocked on https://code.videolan.org/videolan/dav1d/merge_requests/246/diffs

Comment 28

[Alex Chronopoulos [:achronop]]

 We are not blocked by the merge request in previous comment, apparently.

This is a 1st version of the wrapper working with 8-bitdepth files:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=210390981&revision=c2e7add68a2d83d1ebdecc935e326e9baae5f475

Comment 29

[Alex Chronopoulos [:achronop]]

New try run for every platform (to help testing):
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=210653492&revision=8c2a59afaea28f27619d53aff1b659b022949bbc
In order to use dav1d you need to flip the pref media.av1.use-dav1d (on the top of media.av1.enabled)

P.S. the OSX failures seem unrelated to my changes.

Comment 30

[Alex Chronopoulos [:achronop]]

For the record, I tested a local build with asm on. Initially I hit the issue described in [1]. It proved to be a mistake in our moz.build files. After corrected that the build was successful and all 8 and 10 bitdepth videos played without a problem. Also, playback was very smooth, even on a debug build, without frames drops.

[1] https://code.videolan.org/videolan/dav1d/issues/157

Comment 31

[Alex Chronopoulos [:achronop]]

Hmm I am getting the same OSX failures after a rebase, which cannot be a coincident:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5c925e62a69ce335d171edc1e67dae9cfcfcd6a

Error:
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:434:5: error: unknown type name 'IOSurfacePtr'
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:434:28: error: use of undeclared identifier 'MacIOSurfaceLib'
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:437:12: error: unknown type name 'MacIOSurface'; did you mean '__IOSurface'?
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:437:43: error: unknown type name 'MacIOSurface'; did you mean '__IOSurface'?
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:439:39: error: unknown type name 'MacIOSurfaceImage'
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:641:35: error: use of undeclared identifier 'MacIOSurfaceLib'
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:643:17: error: no matching function for call to 'ArrayLength'
/builds/worker/workspace/build/src/dom/media/platforms/apple/AppleVTDecoder.cpp:651:24: error: no matching function for call to 'ArrayLength'
make[4]: *** [Unified_cpp_dom_media_platforms1.o] Error 1

Comment 32

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Update dav1d from upstream to d27598e. r?TD-Linux

Depends on D9607

Comment 33

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Create build files for dav1d. r?chmanchester

Comment 34

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Implement platform decoder for dav1d. r?jya

Comment 35

[Alex Chronopoulos [:achronop]]

Manual testing based on this run: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=475a9436e6e3bbb32ee7e2c5955130caffed249d

Comment 36

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Make DecoderDoctorLifeLogger destructor virtual. r?jya

DecoderDoctorLifeLogger is used as a base class in many classes thus the destructor has to be virtual.

Comment 37

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Change dav1d files to clang-format. r?jya

Comment 38

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493400 - Test av1 video using dav1d has the correct number of frames. r?jya

Comment 39

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94475f38ebe75b0612252e24df60f413d39fae9d

Comment 40

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=826987e03879d66771c5c0668226c3a985f5a1ad

Comment 41

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3350f94a3334cb55c718061c2e053d0b5408b2c4

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddfa91686df0
Import dav1d into tree. r=glob
https://hg.mozilla.org/integration/autoland/rev/94a729d77bc5
Update dav1d from upstream to d27598e. r=TD-Linux
https://hg.mozilla.org/integration/autoland/rev/5c4bf474ddb3
Create build files for dav1d. r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/8b2be3d7ece9
Implement platform decoder for dav1d. r=jya
https://hg.mozilla.org/integration/autoland/rev/04d915d32eea
Test av1 video using dav1d has the correct number of frames. r=jya

Comment 43

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ddfa91686df0
https://hg.mozilla.org/mozilla-central/rev/94a729d77bc5
https://hg.mozilla.org/mozilla-central/rev/5c4bf474ddb3
https://hg.mozilla.org/mozilla-central/rev/8b2be3d7ece9
https://hg.mozilla.org/mozilla-central/rev/04d915d32eea

Comment 44

[Paul Bone [:pbone]]

I can't build firefox since this landed.  Here's the error I see:

 0:02.56 In file included from /home/paul/dev/moz/dont-poison/third_party/dav1d/src/data.c:39:
 0:02.56 /home/paul/dev/moz/dont-poison/third_party/dav1d/src/ref.h:39:5: error: unknown type name 'atomic_int'
 0:02.56     atomic_int ref_cnt;
 0:02.56     ^

I'm using clang6 on amd64 Linux.

Comment 45

[Alex Chronopoulos [:achronop]]

(In reply to Paul Bone [:pbone] from comment #44)
> I can't build firefox since this landed.  Here's the error I see:
> 
>  0:02.56 In file included from
> /home/paul/dev/moz/dont-poison/third_party/dav1d/src/data.c:39:
>  0:02.56 /home/paul/dev/moz/dont-poison/third_party/dav1d/src/ref.h:39:5:
> error: unknown type name 'atomic_int'
>  0:02.56     atomic_int ref_cnt;
>  0:02.56     ^
> 
> I'm using clang6 on amd64 Linux.

Can you please open a new bug about it? Can you please build dav1d library alone from [1] and provide the config.h file found in the build directory.

[1] https://code.videolan.org/videolan/dav1d