Closed Bug 1412558 Opened 7 years ago Closed 7 years ago

media.ffvpx.enabled=true (vp9) is too slow on 32bit Unix

Categories

(Core :: Audio/Video: Playback, defect, P5)

Product:
Core
Component:
Audio/Video: Playback
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: jbeich, Assigned: jya)

References

Details

Attachments

(5 files)

Bug 1412558 - Allow using flac decoder from system ffmpeg.
59 bytes, text/x-review-board-request
jya
: review-
Details
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.
59 bytes, text/x-review-board-request
jya
: review-
Details
Make Agnostic PDM used first on Unix 32 bits.
1.13 KB, patch
Details | Diff | Splinter Review
Bug 1412558 - P1. Don't export some unused symbols.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1412558 - P2. Disable ffvp8 and ffvp9 decoders on Unixes 32 bits.
59 bytes, text/x-review-board-request
mshal
: review+
Details 
0. Use hardware with at least AVX2 support
1. Open https://www.youtube.com/watch?v=1La4QzGeaaQ
2. Switch to 4k 60fps
3. Notice choppy playback unless media.ffvpx.enabled=false

Tested on c0eb1f08953b + 7d773bc1b9b8. ffvpx (even 3.4) is slower than libvpx, not to mention system ffmpeg.

47.08%  [25861508] put_8tap_2d_hv_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
09.46%  [5194161]  avg_8tap_2d_hv_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
06.60%  [3627802]  put_8tap_1d_h_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
05.75%  [3158648]  loop_filter_v_16_8_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
05.64%  [3100529]  put_8tap_1d_v_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
05.34%  [2933947]  loop_filter_h_16_8_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
04.02%  [2209240]  idct_idct_32x32_add_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
01.87%  [1026682]  decode_coeffs_8bpp @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
 00.00%  [36]        ff_vp9_decode_block
01.64%  [900038]   I422ToARGBRow_C @ objdir/toolkit/library/libxul.so
01.62%  [891969]   idct_idct_16x16_add_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
01.03%  [566575]   loop_filter_v_8_8_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
00.99%  [543723]   decode_mode @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
 00.01%  [48]        ff_vp9_decode_block
00.98%  [539160]   loop_filter_h_8_8_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
00.95%  [521312]   avg_8tap_1d_h_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
00.93%  [511182]   avg_8tap_1d_v_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
00.54%  [297149]   idct_idct_8x8_add_c @ objdir/media/ffvpx/libavcodec/libmozavcodec.so
Hardware: Unspecified → x86
Why does Firefox generate media/ffvpx/config_* with --enable-asm on Win32 but not on Linux32?
Flags: needinfo?(jyavenard)
OS: Unspecified → Linux
Our build system doesn't have a version of Yasm new enough to enable avx2; but more importantly, 32 bits compilation doesn't allow to build with the required -fPIC on Linux (but can on Windows).

In any case, 32 bits Linux won't receive much love anymore, as it should be :)

If libvpx is faster, we could default to that there.
Flags: needinfo?(jyavenard)
In that case ffvpx is useless on Linux32 thus can be disabled similar to libav FFT. Firefox would then default to ffvp9 from system FFmpeg if installed or libvpx (bundled or system) otherwise.
Comment on attachment 8923132 [details]
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.

https://reviewboard.mozilla.org/r/194322/#review199278

that would remove the ability to play flac on linux 32 bits.
Attachment #8923132 - Flags: review?(jyavenard) → review-
Comment on attachment 8923132 [details]
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.

https://reviewboard.mozilla.org/r/194322/#review199280

just change so that in 32 bits platforms, libvpx os preferred.
(In reply to Jan Beich from comment #3)
> In that case ffvpx is useless on Linux32 thus can be disabled similar to
> libav FFT. Firefox would then default to ffvp9 from system FFmpeg if
> installed or libvpx (bundled or system) otherwise.

We don't want that. There are too many issues with the various of ffmpeg/libav installed in the world to rely on them decoding vp9 properly.

This is why we stopped using system ffmpeg to play vp9, we can't reliably detect one that will work well or not (and why on Linux we ship our own version). And not everyone has ffmpeg installed.

default to libvpx for vp8/vp9.
Comment on attachment 8923131 [details]
Bug 1412558 - Allow using flac decoder from system ffmpeg.

https://reviewboard.mozilla.org/r/194320/#review199282
Attachment #8923131 - Flags: review?(jyavenard) → review-
Comment on attachment 8923131 [details]
Bug 1412558 - Allow using flac decoder from system ffmpeg.

https://reviewboard.mozilla.org/r/194320/#review199284

dont want to support one more decoding possibility/configuration just because...

using libvpx before using ffmpeg will solve what you want to achieve, with code we control and ship and have tested.
Comment on attachment 8923132 [details]
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.

https://reviewboard.mozilla.org/r/194322/#review199280

libvpx is 2x slower than ffvp9 from system FFmpeg.
Comment on attachment 8923132 [details]
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.

https://reviewboard.mozilla.org/r/194322/#review199278

Only if user doesn't have ffmpeg package installed.
(In reply to Jean-Yves Avenard [:jya] from comment #10)
> (In reply to Jan Beich from comment #3)
> > In that case ffvpx is useless on Linux32 thus can be disabled similar to
> > libav FFT. Firefox would then default to ffvp9 from system FFmpeg if
> > installed or libvpx (bundled or system) otherwise.
> 
> We don't want that. There are too many issues with the various of
> ffmpeg/libav installed in the world to rely on them decoding vp9 properly.

Why relying on system FFmpeg for H264 or MP3 decoding is OK then?

> This is why we stopped using system ffmpeg to play vp9, we can't reliably
> detect one that will work well or not (and why on Linux we ship our own
> version).

Can you name the rotten distros with broken ffmpeg package? I don't understand why the rest have to suffer.

- ffvpx doesn't "work well" on Linux/Unix x86... unlike system FFmpeg
- ffvpx won't have AVX2 support until bug 1412339... unlike system FFmpeg
- ffvpx isn't supported on non-x86 platforms... unlike system FFmpeg
- ffvpx is often out of date on release branches... unlike system FFmpeg (depends on distro)

> And not everyone has ffmpeg installed.

So? There's still libvpx to fall back to. For FLAC it'd require system FFmpeg just like MP3 but both FLAC and MP3 are optional for HTML5 audio.
Does Debian, Fedora, Gentoo, OpenBSD or NetBSD care about VP9 decoding performance on i386? Are you fine with libvpx being default but slower than ffmpeg? If you want to confirm:

- ffvpx via media.ffvpx.enabled=true (current default)
- ffmpeg via media.ffvpx.enabled=false (install ffmpeg package)
- libvpx via media.ffvpx.enabled=false + media.ffmpeg.enabled=false
(In reply to Jean-Yves Avenard [:jya] from comment #2)
> Our build system doesn't have a version of Yasm new enough to enable avx2;
> but more importantly, 32 bits compilation doesn't allow to build with the
> required -fPIC on Linux (but can on Windows).

Huh?
Comment on attachment 8923131 [details]
Bug 1412558 - Allow using flac decoder from system ffmpeg.

https://reviewboard.mozilla.org/r/194320/#review199454
(In reply to Jan Beich from comment #16)
> Does Debian, Fedora, Gentoo, OpenBSD or NetBSD care about VP9 decoding
> performance on i386? Are you fine with libvpx being default but slower than
> ffmpeg? If you want to confirm:
> 
> - ffvpx via media.ffvpx.enabled=true (current default)
> - ffmpeg via media.ffvpx.enabled=false (install ffmpeg package)
> - libvpx via media.ffvpx.enabled=false + media.ffmpeg.enabled=false

No, libvpx PDM needs to be made higher priority than ffmpeg, there's a parameter in PDMFactory to do that. Nothing else needs to change (nor the preferences)

Obviously, the other way would be to fix on why it cant be build with assembly accelerated code.
(In reply to Jan Beich from comment #13)

> libvpx is 2x slower than ffvp9 from system FFmpeg.

certainly not what our test shows, especially with newer version of libvpx . ffvp9 is indeed faster on average, but can only wish it was twice faster on average
(In reply to Jan Beich from comment #15)
> (In reply to Jean-Yves Avenard [:jya] from comment #10)
> > (In reply to Jan Beich from comment #3)
> > > In that case ffvpx is useless on Linux32 thus can be disabled similar to
> > > libav FFT. Firefox would then default to ffvp9 from system FFmpeg if
> > > installed or libvpx (bundled or system) otherwise.
> > 
> > We don't want that. There are too many issues with the various of
> > ffmpeg/libav installed in the world to rely on them decoding vp9 properly.
> 
> Why relying on system FFmpeg for H264 or MP3 decoding is OK then?
> 

H264/mp3 is fully supported by even very old version of Libav or Ffmpeg (before libavcodec v53)

VP9 is much more recent, and there were plenty of issues with Libav version (almost all of them) and some FFmpeg..
> 
> Can you name the rotten distros with broken ffmpeg package? I don't
> understand why the rest have to suffer.

all Libav based one: debian and ubuntu to name two.

> - ffvpx doesn't "work well" on Linux/Unix x86... unlike system FFmpeg
> - ffvpx won't have AVX2 support until bug 1412339... unlike system FFmpeg
> - ffvpx isn't supported on non-x86 platforms... unlike system FFmpeg
> - ffvpx is often out of date on release branches... unlike system FFmpeg
> (depends on distro)

how could ffvpx be out of date, we ship our own copy?
> So? There's still libvpx to fall back to. For FLAC it'd require system
> FFmpeg just like MP3 but both FLAC and MP3 are optional for HTML5 audio.

And there's no reason flac shouldn't play on all systems. H264 and Mp3 are patent encumbered. flac isn't.
now forcing uses to rely on an external package to have flac support would be a regression and isn't acceptable.

and i386 linux is going the way of the dodo anyway.
looking at telemetry, people still running linux 32 bits (if there are any left) just don't care about optimised video playback.
(In reply to Jan Beich from comment #15)
> Can you name the rotten distros with broken ffmpeg package? I don't
> understand why the rest have to suffer.

Fedora does not have ffmpeg, has to be installed from 3rd party repos. 
It's also unavailable for flatpak builds as it can't be a part of official platpak runtime.
 
> - ffvpx doesn't "work well" on Linux/Unix x86... unlike system FFmpeg
> - ffvpx won't have AVX2 support until bug 1412339... unlike system FFmpeg
> - ffvpx isn't supported on non-x86 platforms... unlike system FFmpeg
> - ffvpx is often out of date on release branches... unlike system FFmpeg
> (depends on distro)
(In reply to Martin Stránský [:stransky] from comment #22)
> platpak runtime.

Errr, flatpak runtime.
(In reply to Mike Hommey [:glandium] from comment #17)
> (In reply to Jean-Yves Avenard [:jya] from comment #2)
> > Our build system doesn't have a version of Yasm new enough to enable avx2;
> > but more importantly, 32 bits compilation doesn't allow to build with the
> > required -fPIC on Linux (but can on Windows).
> 
> Huh?

FFmpeg when built on linux 32 bits can't use -fPIC

run FFmpeg's configure with --cc="clang -m32"

and you'll see that it generates:
#define CONFIG_PIC 0

See also:
https://bugzilla.mozilla.org/show_bug.cgi?id=1157768#c49

and your explanation at the time: https://bugzilla.mozilla.org/show_bug.cgi?id=1157768#c61
FFmpeg assembly code doesn't support PIC mode which prevents from compiling it with assembly acceleration.

MozReview-Commit-ID: 7EELJr8Vpau
something like this should do...
Comment on attachment 8923336 [details] [diff] [review]
Make Agnostic PDM used first on Unix 32 bits.

Review of attachment 8923336 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/platforms/PDMFactory.cpp
@@ +377,5 @@
>    }
>  #endif
>  
>    m = new AgnosticDecoderModule();
> +#if defined(XP_UNIX) && defined(ARCH_CPU_32_BITS)

This should also check ARCH_CPU_X86 to avoid triggering on arm-android.

ARCH_CPU_* aren't currently available here; you'd need to export and include gfx/ycbcr/chromium_types.h or some other copy. I think it's cleaner just to use the gcc-style predefined macro directly:

#if defined(XP_UNIX) && defined(__i386__)
Comment on attachment 8923132 [details]
Bug 1412558 - Don't build ffvpx on 32bit Unix, similar to libav FFT.

https://reviewboard.mozilla.org/r/194322/#review199716

I don't disagree with jya here. You could still remove the darwin32 config if you like; we don't build for that target any more.
Attachment #8923132 - Flags: review?(giles)
Comment on attachment 8923131 [details]
Bug 1412558 - Allow using flac decoder from system ffmpeg.

https://reviewboard.mozilla.org/r/194320/#review199284

Would FLAC work without system FFmpeg on non-x86* Unix then?
(In reply to Jean-Yves Avenard [:jya] from comment #21)
> VP9 is much more recent, and there were plenty of issues with Libav version
> (almost all of them) and some FFmpeg..
> > 
> > Can you name the rotten distros with broken ffmpeg package? I don't
> > understand why the rest have to suffer.
> 
> all Libav based one: debian and ubuntu to name two.

Maybe a few years ago. 

https://lwn.net/Articles/650816/
https://repology.org/metapackage/ffmpeg/versions
https://repology.org/metapackage/libav/versions (notice absence of Debian or recent Ubuntu)

> > - ffvpx is often out of date on release branches... unlike system FFmpeg
>     (depends on distro)
> 
> how could ffvpx be out of date, we ship our own copy?

README_MOZILLA on ESR52 says n3.1.1-6-g86f9228. It's more than 1 year out of date. My system ffmpeg package is n3.4 on /latest and n3.3.5 on /quarterly.

> > So? There's still libvpx to fall back to. For FLAC it'd require system
> > FFmpeg just like MP3 but both FLAC and MP3 are optional for HTML5 audio.
> 
> And there's no reason flac shouldn't play on all systems. H264 and Mp3 are
> patent encumbered. flac isn't.

Not sure why Linux on armv7 or aarch64 aren't part of "all systems". And MP3 patents expired recently.
there's no ffvpx on arm , ffmpeg vp9 doesn't have neon optimizations, while libvpx does
(In reply to Jan Beich from comment #29)
> Comment on attachment 8923131 [details]
> Bug 1412558 - Allow using flac decoder from system ffmpeg.
> 
> https://reviewboard.mozilla.org/r/194320/#review199284
> 
> Would FLAC work without system FFmpeg on non-x86* Unix then?

not yet.. this is being worked on bug 1295886.
(In reply to Jan Beich from comment #30)
> (In reply to Jean-Yves Avenard [:jya] from comment #21)
> > VP9 is much more recent, and there were plenty of issues with Libav version
> > (almost all of them) and some FFmpeg..
> > > 
> > > Can you name the rotten distros with broken ffmpeg package? I don't
> > > understand why the rest have to suffer.
> > 
> > all Libav based one: debian and ubuntu to name two.
> 
> Maybe a few years ago. 

you think people still running 32 bits linux upgrade often ? :)

Unfortunately, we have to support a very broad range of users.
(In reply to Jean-Yves Avenard [:jya] from comment #35)
> you think people still running 32 bits linux upgrade often ? :)

We can prefer the system version of libavcodec when it is up to date (i.e. >=57) and fall back to the built in one otherwise.
Blocks: 1414440
Assignee: nobody → jyavenard
Comment on attachment 8925679 [details]
Bug 1412558 - P1. Don't export some unused symbols.

https://reviewboard.mozilla.org/r/196778/#review202026
Attachment #8925679 - Flags: review?(gsquelart) → review+
Comment on attachment 8925680 [details]
Bug 1412558 - P2. Disable ffvp8 and ffvp9 decoders on Unixes 32 bits.

https://reviewboard.mozilla.org/r/196780/#review202514
Attachment #8925680 - Flags: review+
Attachment #8925680 - Flags: review?(core-build-config-reviews)
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/41848d513f53
P1. Don't export some unused symbols. r=gerald
https://hg.mozilla.org/integration/autoland/rev/0754603ee7b0
P2. Disable ffvp8 and ffvp9 decoders on Unixes 32 bits. r=mshal
https://hg.mozilla.org/mozilla-central/rev/41848d513f53
https://hg.mozilla.org/mozilla-central/rev/0754603ee7b0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Firefox 58.0b3 appears to work as intended. Thank you. The video from comment 0 now plays smoothly, using ~27% CPU on i7-6700K which is close to x86_64 performance.
Status: RESOLVED → VERIFIED
thank you for reporting back...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: