Closed Bug 1755361 Opened 3 years ago Closed 2 years ago

Don't accept OpenH264 as h264 decoder from ffmpeg by default

Categories

(Core :: Audio/Video: Playback, task, P3)

Product:
Core
Component:
Audio/Video: Playback
Platform:
Desktop
Linux
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
115 Branch
Tracking Flags:
Tracking Status
firefox113 --- unaffected
firefox114 --- wontfix
firefox115 --- fixed

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Bug 1755361 - Don't accept OpenH264 as h264 decoder from ffmpeg by default.
48 bytes, text/x-phabricator-request
Details | Review

OpenH264 currently has issues decoding some H264 content:
https://github.com/cisco/openh264/issues/3479
https://github.com/cisco/openh264/issues/3478

For now, it's probably better to stop using OpenH264 by default and hope that users are able to find a better h264 decoder.

We should be able to detect this here:
https://searchfox.org/mozilla-central/rev/69a482382823f42f482e840f65725218d7654cc4/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp#83

OpenH264 currently has issues decoding some H264 content:
https://github.com/cisco/openh264/issues/3479
https://github.com/cisco/openh264/issues/3478

For now, it's probably better to stop using OpenH264 by default and hope that users are able to find a better h264 decoder.

Assignee: nobody → jmuizelaar
Status: NEW → ASSIGNED

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)

https://github.com/cisco/openh264/issues/3478

That one has apparently been fixed by: https://github.com/cisco/openh264/pull/3482 ("fix issue-3478")

See Also: → openh264-2_6

Does this bug actually affects anybody? ffmpeg has its own (patented) h264 decoder which is used by default. When ffmpeg is build with openh264 decoder it means somebody does that on purpose and wants that as it needs extra effort to do so (ffmpeg does not build with openh264 by default). It may also mean the h264 content won't be available at all when we disable it.

As for the bugs here:

https://github.com/cisco/openh264/issues/3479
https://github.com/cisco/openh264/issues/3478

Firefox/Fedora (and probably other distros) use OpenH264 to decode h264 content but GMP interface (not ffmpeg). It's because we can't link OpenH264 with Firefox, OpenH264 is downloaded directly from Cisco servers after Firefox installation so the Firefox/Openh264 binding is dynamic (we also disable download OpenH264 from Mozilla as Mozilla provided H264 is pretty outdated - see Bug 1619988).

(In reply to Martin Stránský [:stransky] (ni? me) from comment #3)

Does this bug actually affects anybody? ffmpeg has its own (patented) h264 decoder which is used by default. When ffmpeg is build with openh264 decoder it means somebody does that on purpose and wants that as it needs extra effort to do so (ffmpeg does not build with openh264 by default). It may also mean the h264 content won't be available at all when we disable it.

The default Flatpak runtime has ffmpeg with openh264 and no builtin h264 decoder so this change would change the behaviour of the Firefox Flatpak package.

I'm not sure what the best thing for Fedora to do is. Ideally, Fedora wouldn't be using a broken H264 decoded by default but I don't know how often the brokenness shows up. Perhaps, Fedora could provide some resources to help improve the correctness and performance of OpenH264?

Not all h264 videos fail or have stuttering. We (community and Fedora) should instead file actionable bug reports on Github.

I would be more okay with this if there were a way to opt into testing this. As it is, I am using ffmpeg, which seems to work fine, and I have no idea how to opt into OpenH264.

Would merging this patch really be better than current situation?

Overall though, I do think this would be better than the current situation. OpenH264 seems to be known to be broken, and fixing it with ffmpeg is easy and fairly well documented across most distributions.

Unless Firefox also ships FDK-AAC-Stripped like Fedora and Flatpak (stripped ffmpeg with fdk_aac as part of freedesktop-sdk), there is little point in having OpenH264 for video decoding in Mozilla binaries (bug 1663844). Youtube could use H264+Opus, but prefers VP9 and AV1. Other websites require H264+AAC.

(In reply to Asif Youssuff from comment #7)

I have no idea how to opt into OpenH264.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #5)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #3)

Does this bug actually affects anybody? ffmpeg has its own (patented) h264 decoder which is used by default. When ffmpeg is build with openh264 decoder it means somebody does that on purpose and wants that as it needs extra effort to do so (ffmpeg does not build with openh264 by default). It may also mean the h264 content won't be available at all when we disable it.

The default Flatpak runtime has ffmpeg with openh264 and no builtin h264 decoder so this change would change the behaviour of the Firefox Flatpak package.

That's really interesting and I don't understand it. If default Flatpak runtime has ffmpeg with openh264 AFAIK it means it violates openh264 license as it's requested to get openh264 directly from Cisco (as Cisco has to counts download numbers for the license). That's reason why Fedora uses openh264 via GMP and not via ffmpeg (see Bug 1057646).

I'm not sure what the best thing for Fedora to do is. Ideally, Fedora wouldn't be using a broken H264 decoded by default but I don't know how often the brokenness shows up. Perhaps, Fedora could provide some resources to help improve the correctness and performance of OpenH264?

Fedora is not affected by this change as we don't use openh264 via. ffmpeg due to reason I wrote above.

(In reply to Darkspirit from comment #8)

Unless Firefox also ships FDK-AAC-Stripped like Fedora and Flatpak (stripped ffmpeg with fdk_aac as part of freedesktop-sdk), there is little point in having OpenH264 for video decoding in Mozilla binaries (bug 1663844). Youtube could use H264+Opus, but prefers VP9 and AV1. Other websites require H264+AAC.

YT uses Opus so generally OpenH264 can work there. But sure sites with H264+AAC may be broken unless AAC is also enabled in Flatpak/ffmpeg.

The Flatpak/Freedesktop SDK contains ffmpeg

Flatpak package org.freedesktop.Platform.openh264 seems to download libopenh264 to extend basic ffmpeg from Freedesktop SDK, it does not load gmp+openh264.
https://gitlab.com/freedesktop-sdk/openh264-extension/-/blob/master/elements/config.yml
https://gitlab.com/freedesktop-sdk/openh264-extension/-/blob/master/Readme.md#flatpak-openh264-extension

This extension provides bootstrapping mechanism which allows installing Cisco's
free to use OpenH264 codec on end-user machine. This system is intended to be
used in conjunction with noopenh264. Applications link against stubs in noopenh264
and as such it determines the ABI. Applications however use openh264 during
runtime so the two must be ABI-compatible.

OpenSuse's ffmpeg has at least FDK-AAC-Stripped.
https://build.opensuse.org/package/view_file/openSUSE:Factory/ffmpeg-4/ffmpeg-4.2-dlopen-fdk_aac.patch?expand=1
https://build.opensuse.org/package/show/openSUSE:Factory/fdk-aac-free

Debian's ffmpeg does not seem to have AAC by default, AAC/ADTS and H264 seem to be part of libavcodec-extra58.
There is only an uninteresting non-free package of the unstripped variant: https://packages.debian.org/stable/libfdk-aac2

(In reply to Martin Stránský [:stransky] (ni? me) from comment #9)

That's really interesting and I don't understand it. If default Flatpak runtime has ffmpeg with openh264 AFAIK it means it violates openh264 license as it's requested to get openh264 directly from Cisco (as Cisco has to counts download numbers for the license). That's reason why Fedora uses openh264 via GMP and not via ffmpeg (see Bug 1057646).

Flatpak provides a wrapper which downloads openh264 straight from cisco webpage on end-user host. This should comply with cisco license.

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jrmuizel, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Flags: needinfo?(jmuizelaar)
Flags: needinfo?(bvandyk)
Depends on: 1766638, 1827453
OS: Unspecified → Linux
Hardware: Unspecified → Desktop

This patch makes sense now with Bug 1831342 (GMP/OpenH264 decoding). GMP uses latest OpenH264 and has patches to handle broken OpenH264 behavior so we want to use OpenH264 via GMP and not via ffmpeg.

Andrew, what do you think?

Flags: needinfo?(aosmond)

mozilla-openh264 2.3.1 distro packages download distro-made gmp-openh264 binaries from Cisco. It's not worse if Firefox would do it instead.
That would relieve distros from their packaging burden of mozilla-openh264.

  • bug 1827333 should ship 2.3.2 to all Firefox 114+ Linux users now. Fedora(x86_64,i386,aarch64,ppc64le,s390x),OpenSuse,Gentoo already have 2.3.1. Flatpak has 2.2.0.
  • bug 1766638: Update GMP plugins right after startup (at least on MOZ_WIDGET_GTK).
  • Uplift a pref to ignore the MOZ_GMP_PATH env var by default.
  • bug 1827453: Uplift media.gmp.decoder.enabled=true for MOZ_WIDGET_GTK because distros are already shipping it. If it's not enabled, decoding would fail due to missing decoder.
  • this bug/Flatpak: Stop using openh264 via ffmpeg noopenh264 adapter. It has been affected by the seeking bug as well (bug 1670333 comment 26).

That would solve bug 1831342 and be a long-term fix at the same time.

Attachment #9263985 - Attachment description: Bug 1755361. Don't accept OpenH264 as h264 decoder from ffmpeg by default. → Bug 1755361 - Don't accept OpenH264 as h264 decoder from ffmpeg by default.

I agree, we can land this with the caveat that we disallow ffmpeg + openh264 on nightly, where we are currently shipping the updated plugin, but still allow it by default on beta and later. I may request uplift of this with several other patches to give distros more options on how they wish to go about the plugin update.

Severity: -- → S3
status-firefox113: --- → unaffected
status-firefox114: --- → affected
status-firefox115: --- → affected
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(aosmond)
Priority: -- → P3
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4471b79a945c Don't accept OpenH264 as h264 decoder from ffmpeg by default. r=bryce

Backed out for causing bustages on FFmpegDataDecoder.cpp

Backout link

Push with failures

Failure log

Flags: needinfo?(jmuizelaar)

Ah, it appears MOZ_FFVPX includes the decoder file even if MOZ_FFMPEG is disabled. I will fix.

Flags: needinfo?(jmuizelaar)
Pushed by aosmond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/46595343e742 Don't accept OpenH264 as h264 decoder from ffmpeg by default. r=bryce

https://hg.mozilla.org/mozilla-central/rev/46595343e742

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox115: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch

Is this something that needs to be backported to 114 before we build the RC next week or can it ride the trains?

Flags: needinfo?(jmuizelaar)

I think it's ok to ride. Andrew?

Flags: needinfo?(jmuizelaar) → needinfo?(aosmond)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)

Is this something that needs to be backported to 114 before we build the RC next week or can it ride the trains?

Not this bug, but bug 1831342 but with the pref set to false.

bug 1827703 (114) changed GMP-OpenH264 playback which caused bug 1831342: Playback becomes awful for versions older than latest OpenH264 2.3.2.

The regression only affects Linux distributions without ffmpeg h264 decoder (to avoid patent license fees).
Those distributions enable media.gmp.decoder.enabled, compile GMP-OpenH264 for Firefox themselves (because Mozilla's plugin version is too outdated), publish it as system package via Cisco, download it from Cisco, and load it into Firefox via MOZ_GMP_PATH environment variable.
Otherwise Firefox would fail to play Twitch streams etc.

Those Linux distributions need either
a) to create&publish their own GMP-OpenH264 2.3.2 package update before their users try to use Firefox 114+
or
b) they (or Mozilla) need to backport bug 1831342 (the pref) to 114, but set to false. <-----------------------------------------------------------------

I assume Flatpak is unaffected by the regression as it uses OpenH264 via ffmpeg noopenh264 (with its own problems) and not via GMP.
Only Stable is packaged for Flatpak. I would be able to test after its release.
comment 16 would be the long-term fix (hopefully sooner than later): Flatpak and distributions without ffmpeg h264 decoder would then use the same code path and every Nightly user could test the same thing.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #23)

Is this something that needs to be backported to 114 before we build the RC next week or can it ride the trains?

Uplift request: bug 1831342 comment 13

Flags: needinfo?(aosmond)
See Also: → 1850098
Regressions: 1898773
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: