Closed Bug 1663844 Opened 4 years ago Closed 1 year ago

Allow using OpenH264 GMP decoder as fallback for video decoding (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch )

Categories

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

Desktop
Linux
task

Tracking

()

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- verified

People

(Reporter: jya, Assigned: aosmond)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Currently the GMP OpenH264 decoder is disabled by default. Primary reason is that our version OpenH264 only supports the main profile.

Once the OpenH264 plugin is updated to 2.1.x we could theoretically use if for most h264 videos.
Though, there are likely performance problem as OpenH264 was geared toward low-latency and doesn't scale well speed-wise.

Some distros (Fedora for instance) ship system OpenH264 package which can be used by Firefox so this bug does not depend on the Mozilla copy.

So far (after some hacks) the GMP plugin (OpenH264 2.1.1) fails to play video content and the screen states

"Video can't be played because it's corrupted..."

GMP/Media log looks like:

[Parent 66952: GMPThread]: D/GMP GMPParent[0x7f9481e6a000|childPid=67420] LoadProcess: Sent StartPlugin to child process
[Child 67340: GMPThread]: D/GMP GMPContentParent::GMPContentParent(this=0x7fdcccd18400), aParent=(nil)
[Child 67340: GMPThread]: D/GMP GMPContentParent::GMPEventTarget(this=0x7fdcccd18400)
[Child 67340: GMPThread]: D/GMP GMPContentParent::AddCloseBlocker(this=0x7fdcccd18400) mCloseBlockerCount=1
[Child 67340: GMPThread]: D/GMP GMPContentParent::GetGMPVideoDecoder(this=0x7fdcccd18400)
[Child 67340, GMPThread] WARNING: Trying to use an dead GMP video decoder: file /home/komat/src/dom/media/gmp/GMPVideoDecoderParent.cpp, line 220
[Child 67340: GMPThread]: D/GMP GMPVideoDecoderParent[0x7fdcd0c63920]::InitDecode()
[Child 67340: GMPThread]: D/GMP GMPContentParent::RemoveCloseBlocker(this=0x7fdcccd18400) mCloseBlockerCount=0
[Child 67340: GMPThread]: D/GMP GMPContentParent::CloseIfUnused(this=0x7fdcccd18400) mVideoDecoders.IsEmpty=false, mVideoEncoders.IsEmpty=true, mChromiumCDMs.IsEmpty=true, mCloseBlockerCount=0
[Child 67340: GMPThread]: V/GMP GMPVideoDecoderParent[0x7fdcd0c63920]::Decode() timestamp=0 keyframe=1
[Child 67340: GMPThread]: V/GMP GMPVideoDecoderParent[0x7fdcd0c63920]::RecvInputDataExhausted()
[Child 67340: GMPThread]: V/GMP GMPVideoDecoderParent[0x7fdcd0c63920]::Decode() timestamp=133467 keyframe=0
[Child 67340: GMPThread]: V/GMP GMPVideoDecoderParent[0x7fdcd0c63920]::RecvDecoded() timestamp=133467 frameCount=1

and nothing more from GMP.

Note: Playback works when OpenH264 is used over ffvpx.

Hm, that may be caused by missing audio decoding.

Looks like the gmp-openh264.cpp module needs to be updated for the new codec version or gecko does something wrong. I'm getting called data exhausted callback from the module and it looks like gecko does not send frames to decoder.

Looks like https://github.com/cisco/openh264/issues/3079 ffmpeg has a similar bug but simple DecodeFrame2 -> DecodeFrameNoDelay replace in gmp-openh264.cpp does not work.

I wonder how the GMP plugin can signalize it needs more data to produce a frame.

Attached patch openh264-media-playback.patch (obsolete) — Splinter Review

There are two issues with OpenH264/GMP used for media playback which causes the plugin is unusable:

  • GMPDecoderModule::SupportsMimeType() returns always false so GMP is not used by PDMFactory
  • mDecodePromise (created at GMPVideoDecoder::Decode()) is newer resolved so the decode queue is not fed and decoding is blocked after first frame decode.

Jean-Yves, the patch works I'm not sure the promise resolve is correct there, can you take a look please?
Thanks.

Assignee: nobody → stransky
Attachment #9177627 - Flags: feedback?(jyavenard)
Comment on attachment 9177627 [details] [diff] [review]
openh264-media-playback.patch

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

::: dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
@@ +86,4 @@
>  
>  bool GMPDecoderModule::SupportsMimeType(
>      const nsACString& aMimeType, DecoderDoctorDiagnostics* aDiagnostics) const {
> +  return MP4Decoder::IsH264(aMimeType);

isn't there something available from the GMP API on what it does support?

that's kind of assume that the only plugin available is OpenH264 and that's it

::: dom/media/platforms/agnostic/gmp/GMPVideoDecoder.cpp
@@ +67,5 @@
>    RefPtr<GMPVideoDecoder> self = this;
>    if (v) {
>      mDecodedData.AppendElement(std::move(v));
> +    mDecodePromise.Resolve(std::move(mDecodedData), __func__);
> +    mDecodedData = DecodedData();

the normal flow of operation should be:
Decoded() - Decoded() ... - InputExhausted()/DrainCompleted()

So resolving a single frame as soon as it's returned will potentially means me missed a few. Particularly when draining, which will typically return more than a single frame.

The scenario you describe indicates that InputExhausted isn't called as it should. That's what should be fixed.
Attachment #9177627 - Flags: feedback?(jyavenard) → feedback-

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

Looks like the gmp-openh264.cpp module needs to be updated for the new codec version or gecko does something wrong. I'm getting called data exhausted callback from the module and it looks like gecko does not send frames to decoder.

Looks like https://github.com/cisco/openh264/issues/3079 ffmpeg has a similar bug but simple DecodeFrame2 -> DecodeFrameNoDelay replace in gmp-openh264.cpp does not work.

I wonder how the GMP plugin can signalize it needs more data to produce a frame.

InputExhausted is called when that happens.

The API mimics the Windows Media Foundation (WMF) decoder.

So currently when InputExhausted is called, the promise is resolved, if the DecodedData returned is empty, the MediaFormatReader will call the decoder with more compressed frames.
If the DecodedData returned a frame, the GMP decoder will be called again once a new frame is needed.

(In reply to Jean-Yves Avenard [:jya] from comment #6)

So currently when InputExhausted is called, the promise is resolved, if the DecodedData returned is empty, the MediaFormatReader will call the decoder with more compressed frames.
If the DecodedData returned a frame, the GMP decoder will be called again once a new frame is needed.

I tried to fiddle with InputExhausted on plugin side but without any luck. When InputExhausted is utilized then codec keep to decode the stream but nothing in shown on Firefox side. I'll investigate it more when Bug 1619988 (plugin update to 2.1.1) is fixed.

Severity: -- → S4
Priority: -- → P4

has this been fixed yet with latest releases of firefox?

Blocks: 1764436
OS: Unspecified → Linux
Hardware: Unspecified → Desktop
Summary: Allow using OpenH264 GMP decoder as fallback for video decoding → Allow using OpenH264 GMP decoder as fallback for video decoding (Consider upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch )
Summary: Allow using OpenH264 GMP decoder as fallback for video decoding (Consider upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch ) → Allow using OpenH264 GMP decoder as fallback for video decoding (Would be fixed by upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch )
Summary: Allow using OpenH264 GMP decoder as fallback for video decoding (Would be fixed by upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch ) → Allow using OpenH264 GMP decoder as fallback for video decoding (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1663844.patch )

Any improvement here with v2.3.0?

I have done local testing using 2.3.1. I have had success playing some H264 videos that I found randomly that the 1.8.1.2 plugin fails with today. I needed to incorporate the patch attached in order to get WebRTC/video playback to correct detect the plugin support.

I will fix the patch based on jya's comments and get something landable.

Assignee: stransky → aosmond

By default, decoding via GMP plugins is disabled by the pref
media.gmp.decoder.enabled. However if it is set to true, we never
actually exposed what codecs were supposed by the plugins (EME was
special cased). This patch corrects that.

This patch allows us to track multiple outstanding decodes and fulfill
the decode promise when they have all been completed. This fixes how the
media state machine would get stuck waiting for the decode promise to
fulfill when using the OpenH264 decoder.

Depends on D174354

Comment on attachment 9177627 [details] [diff] [review]
openh264-media-playback.patch

>diff --git a/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp b/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
>--- a/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
>+++ b/dom/media/platforms/agnostic/gmp/GMPDecoderModule.cpp
>@@ -86,7 +86,7 @@ bool GMPDecoderModule::SupportsMimeType(
> 
> bool GMPDecoderModule::SupportsMimeType(
>     const nsACString& aMimeType, DecoderDoctorDiagnostics* aDiagnostics) const {
>-  return false;
>+  return MP4Decoder::IsH264(aMimeType);
> }
> 
> }  // namespace mozilla
>diff --git a/dom/media/platforms/agnostic/gmp/GMPVideoDecoder.cpp b/dom/media/platforms/agnostic/gmp/GMPVideoDecoder.cpp
>--- a/dom/media/platforms/agnostic/gmp/GMPVideoDecoder.cpp
>+++ b/dom/media/platforms/agnostic/gmp/GMPVideoDecoder.cpp
>@@ -67,6 +67,8 @@ void GMPVideoDecoder::Decoded(GMPVideoi4
>   RefPtr<GMPVideoDecoder> self = this;
>   if (v) {
>     mDecodedData.AppendElement(std::move(v));
>+    mDecodePromise.Resolve(std::move(mDecodedData), __func__);
>+    mDecodedData = DecodedData();
>   } else {
>     mDecodedData.Clear();
>     mDecodePromise.RejectIfExists(
Attachment #9177627 - Attachment is obsolete: true
Pushed by aosmond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0da411d6fadb
Part 1. Expose GMP video decoder codec support if enabled. r=media-playback-reviewers,azebrowski
https://hg.mozilla.org/integration/autoland/rev/c948f6a27e6d
Part 2. Ensure we resolve decode promises with GMPVideoDecoder. r=media-playback-reviewers,azebrowski
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch

Thanks! Verified on Debian Testing:

Blocks: 1827453
Blocks: 1670333
No longer depends on: 1670333
No longer blocks: 1670333
Depends on: 1670333
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: