Open Bug 1670333 Opened 4 years ago Updated 1 year ago

When decoding video by OpenH264 (GMP or Flatpak openh264) and position at video stream is changed, decoding fails. (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch )

Categories

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

Desktop
Linux
defect

Tracking

()

ASSIGNED
Tracking Status
firefox112 --- affected

People

(Reporter: stransky, Assigned: aosmond)

References

(Blocks 2 open bugs, )

Details

Attachments

(1 file)

When user seeks in video during playback repeatedly, playback fails with "Media error". Happens also without a patch from Bug 1663844.

The failure comes from GMPVideoDecoderParent::Decode() where this condition fails:

if ((NumInUse(GMPSharedMem::kGMPFrameData) > 3 * GMPSharedMem::kGMPBufLimit) || (NumInUse(GMPSharedMem::kGMPEncodedData) > GMPSharedMem::kGMPBufLimit)) {

GMPSharedMem::kGMPBufLimit is 20 and NumInUse(GMPSharedMem::kGMPEncodedData is 21.

When the GMPSharedMem::kGMPBufLimit limit is raised to 40 the limit scenario won't happen but the openh264 decoder (gmp-openh264 plugin) fails at DecodeFrameNoDelay() with decoding error 34 (still investigating).

The error is:

dsRefLost = 0x02, ///< layer lost at reference frame with temporal id 0
dsDataErrorConcealed = 0x20, ///< current data error concealed specified

Bryce, would you mind taking a look?

Severity: -- → S3
Flags: needinfo?(bvandyk)

The decoding error is related to on-line playback (I use yotube). When I open the same video locally I can't reproduce it - I can seek in the video without any issue. So it looks like we serve incomplete data to the codec during on-line playback.

Could you post steps to repro so I can make sure I'm looking at the same thing? I'm assuming the cases where this happens without the patch mentioned in comment 0 are those where we have non-moz builds using OpenH264?

It sounds like we have both shared memory and decoding related issues.

Does the decoding problem only take place when changing position?

Flags: needinfo?(stransky)

I'm investigating that right now.

Looks like when video is played from YT we don't feed the decoder with correct data and/or Reset() call is not correctly implemented in gmp-openh264 plugin. The gmp-openh264 does not recognize the frames after seek as a new begin with a new start but tries to continue to decode the existing one and complains about a frame gap and throw an error.

The continuous frame stream uses NAL_UNIT_CODED_SLICE frames and the first frame (new start) uses NAL_UNIT_CODED_SLICE_IDR. After seek from disk we send NAL_UNIT_CODED_SLICE_IDR frame to decoder so it begins a new decoding but with seek on YT we send a new frame with NAL_UNIT_CODED_SLICE do the decoder tries to continue with the current stream.

Flags: needinfo?(stransky)

Bryce, how do we handle stream decode here? When we seek in stream and we get i-frames which can't be send to decoder - should we drop i-frames until we get a keyframe and start the decoding there?

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #6)

Bryce, how do we handle stream decode here? When we seek in stream and we get i-frames which can't be send to decoder - should we drop i-frames until we get a keyframe and start the decoding there?

That's what we should do. Explicitly

  • We begin a seek.
  • We flush the decoder[0] -- drop the frames we're currently holding a prepare to decode from the new seek point.
  • We seek the demuxer[1] to the desired time. The demuxer abstracts this operation and returns the actual time it has seeked to. This will normally be a random access point, which is expected to be a keyframe.
  • We start feeding the decoder with samples from that point.

(The MediaFormatReader's SeekInternal[2] is a good place to delve from if you want to look more).

Because the container can disagree with the h264 stream itself we attempt to determine the frame types by reading the stream[3].

Let me know if I can provide more information, or if it would help to have me debug this as well.

[0] https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/dom/media/platforms/PlatformDecoderModule.h#325
[1] https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/dom/media/MediaDataDemuxer.h#147
[2] https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/dom/media/MediaFormatReader.cpp#1952
[3] https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/dom/media/platforms/agnostic/bytestreams/H264.cpp#942

Flags: needinfo?(bvandyk)

Hi Bryce,

thanks a lot! There's a how-to enable openh264 with latest trunk:

  1. apply a patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1663844 to make open264 usable for media playback
  2. apply a patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1667096 and build with --with-system-fdk-aac, you need to have fdk-aac-free-devel or fdk-aac-devel packages installed. This may be optional as YT uses Opus afaik.
  3. set media.gmp.decoder.enabled to true
  4. disable ffmpeg playback by setting media.ffmpeg.enabled to false
  5. install openh264 system wide as Mozilla delivers 1.8.1 only (Fedora provides mozilla-openh264-2.1.1 package)
  6. install enhanced-h264ify extension and disable VP8/9/AV1 video formats.
  7. Run youtube, open h264 video, seek a video, you get the error.

NI me to try repro.

Flags: needinfo?(bvandyk)

I found the problem - we generally mark all mark i-frames as keyframes. FFmpeg accepts that but openh264 refuses to start decoding with i-frame.

Attached file a simple workaround (deleted) β€”

There's a simple workaround for the seek error. I wonder what's the original intention here.

Interesting. I wonder if one of the things we consider I-Frames in particular[0] is busting this.

[0] https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/dom/media/platforms/agnostic/bytestreams/H264.cpp#942

The problem is perhaps that we both H264_NAL_IDR_SLICE and H264_NAL_SLICE mark as I_FRAME and consider them as keyframe. But only H264_NAL_IDR_SLICE are the keyframes which we're looking for in seek:

if (nalType == H264_NAL_IDR_SLICE) {
  // IDR NAL.
  return FrameType::I_FRAME;
} else if (nalType == H264_NAL_SEI) {

} else if (nalType == H264_NAL_SLICE) {
  RefPtr<mozilla::MediaByteBuffer> decodedNAL = DecodeNALUnit(p, nalLen);
  if (DecodeISlice(decodedNAL)) {
    return FrameType::I_FRAME;
  }
}

I'm sure Jean-Yves should know more.

Flags: needinfo?(jyavenard)

The code comes from Bug 1300296.

We've done various relaxing and tweaking of the frames we consider key frames to handle the different content we get served. Bug 1539182 + Bug 1347518 are a couple of the major ones. If OpenH264 must start decoding on an IDR explicitly then I imagine the current code is not going to play nice with it.

(In reply to Bryce Seager van Dyk (:bryce) from comment #16)

We've done various relaxing and tweaking of the frames we consider key frames to handle the different content we get served. Bug 1539182 + Bug 1347518 are a couple of the major ones. If OpenH264 must start decoding on an IDR explicitly then I imagine the current code is not going to play nice with it.

Yes, that's exactly the case here.

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #13)

The problem is perhaps that we both H264_NAL_IDR_SLICE and H264_NAL_SLICE mark as I_FRAME and consider them as keyframe. But only H264_NAL_IDR_SLICE are the keyframes which we're looking for in seek:

if (nalType == H264_NAL_IDR_SLICE) {
  // IDR NAL.
  return FrameType::I_FRAME;
} else if (nalType == H264_NAL_SEI) {

} else if (nalType == H264_NAL_SLICE) {
  RefPtr<mozilla::MediaByteBuffer> decodedNAL = DecodeNALUnit(p, nalLen);
  if (DecodeISlice(decodedNAL)) {
    return FrameType::I_FRAME;
  }
}

H264_NAL_IDR_SLICE and H264_NAL_SEI must be handled. There's plenty of live content that only provides recovery point, without support for that we wouldn't be able to start decoding to start with as there's just no IDR at all, you can forget about seeking too.

We do not consider a H264_NAL_SLICE to be a keyframe, only I-SLICE (https://searchfox.org/mozilla-central/source/dom/media/platforms/agnostic/bytestreams/H264.cpp#1185-1199)

They are all technically keyframe, you can per spec be able to start decoding from them.

In my experience, you see one type but not the others, so that video where you had trouble seeking would typically have no strict IDR.

Flags: needinfo?(jyavenard)

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #11)

Created attachment 9181557 [details]
a simple workaround

There's a simple workaround for the seek error. I wonder what's the original intention here.

The intention is avoid badly muxed content that mark all frames as keyframe regardless of them being a keyframe.

Once upon a time, Chrome MSE stack would only allow to append a buffer if it started with a keyframe (even though there's no such requirement with MSE).
So some sites started to mark all frames as keyframe. Amazon and Twitch in particular did that. They still have such content available.

So while this change may work for that video, I can guarantee you that it would badly break with other videos (it will cause decoding error on both mac and android in particular).

FWIW, Chrome has since adopted the same approach as we do, they totally ignore the keyframe information provided by the container and rely on the H264 bytestream only.

All IDR frames are I-Frame, but not all I-Frame are IDR. Seeking using I-Frame is considered acceptable.

I feel like the only proper option that would cause severe regressions would be to fix OpenH264.

It's a surprising requirements they would have, as WebRTC produced by VideoToolbox encoder (Safari) in particular rarely use IDR

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #8)

  1. install openh264 system wide as Mozilla delivers 1.8.1 only (Fedora provides mozilla-openh264-2.1.1 package)

Can you confirm that the 2.x.y versions are actually used? I wouldn't expect us to use the non-GMP version.

Thanks a lot for the reply, looks like it needs more work to get openh264 in shape for media playback.

(In reply to Bryce Seager van Dyk (:bryce) from comment #20)

  1. install openh264 system wide as Mozilla delivers 1.8.1 only (Fedora provides mozilla-openh264-2.1.1 package)

Can you confirm that the 2.x.y versions are actually used? I wouldn't expect us to use the non-GMP version.

Yes, I can confirm that from GMP log.

Filed as openh264 issue: https://github.com/cisco/openh264/issues/3350

Looking at amount of opened issues there I'm rather skeptical of possible fixes from openh264 project. I'm afraid I'll need to create a downstream patch to take keyframes from a container when openh264 decoder is used.

One solution would be to modify the H264 MediaChangeMonitor: https://searchfox.org/mozilla-central/rev/61728de8273c04fe2417c475fc0637e8b79210d7/dom/media/platforms/wrappers/MediaChangeMonitor.cpp#28-148

And add a flag that would query if the decoder required strict IDR frame

right now the code ask for a keyframe ; special handling could be done there.

Another thing worth investigating is this bit:
https://searchfox.org/mozilla-central/rev/61728de8273c04fe2417c475fc0637e8b79210d7/dom/media/platforms/wrappers/MediaChangeMonitor.cpp#319-320

And the value of the need keyframe, what if this was always set to false ? IIRC it was required with Windows decoder only, without the extra SPS/PPS it would cause an error.

The GMP decoder takes some weird AVCC format where the size is in big-endian order ; I suspect it could be a reason there.

If you set that value to false all the time, how would that go?

How could I reproduce the issue locally?

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #21)

Thanks a lot for the reply, looks like it needs more work to get openh264 in shape for media playback.

(In reply to Bryce Seager van Dyk (:bryce) from comment #20)

  1. install openh264 system wide as Mozilla delivers 1.8.1 only (Fedora provides mozilla-openh264-2.1.1 package)

Can you confirm that the 2.x.y versions are actually used? I wouldn't expect us to use the non-GMP version.

Yes, I can confirm that from GMP log.

Do you need to set MOZ_GMP_PATH or link the newly installed lib? I was under the impression we wouldn't load GMPs unless they were in specific locations, but it sounds like I may need to reassess that.

Flags: needinfo?(bvandyk)

Ah, nevermind. I've had a look at some of the different packages and see they're doing this internally. I'd assumed they exposed a generic openh264 to the system, but there are various packages specifically targeted at Gecko. The above Fedora package seems like it will set the GMP path per the source[0].

[0] https://github.com/UnitedRPMs/openh264/blob/5e194f263cecc24f8279e230cc299184a15a28ab/openh264.spec#L86

Priority: -- → P3
Blocks: 1764436
Summary: When decoding video by OpenH264 GMP and position at video stream is changed, decoding fails. → When decoding video by OpenH264 GMP and position at video stream is changed, decoding fails. (Would be fixed by upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch )
Summary: When decoding video by OpenH264 GMP and position at video stream is changed, decoding fails. (Would be fixed by upstreaming Fedora's bug fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch ) → When decoding video by OpenH264 GMP and position at video stream is changed, decoding fails. (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch )
OS: Unspecified → Linux
Hardware: Unspecified → Desktop

KDE Wayland, Debian Testing
Seeking is broken in Twitter videos when using Openh264 via GMP or via Flatpak FFmpeg(noopenh264+flatpak openh264).
This problem hits Flatpak users by default because Firefox can't depend on Flatpak's ffmpeg-full h264 decoder.

STR:

  1. open $ MOZ_LOG="GMP:5,PlatformDecoderModule:5" mozregression --launch 2023-04-11 --pref media.gmp.decoder.enabled:true media.gmp.decoder.preferred:true media.gmp-manager.url:"https://aus5.mozilla.org/update/3/GMP/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/nightlytest/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml" -P stdout -a about:addons -a https://twitter.com/50Hertzcom/status/1628740928042065927

  2. Go to Plugins > click on wheel > Search for updates

  3. Switch to twitter tab, play and seek, also try Picture in Picture

    • sometimes seeking seems to work(?)
    • sometimes the video jumps to the beginning when seeking and seeking doesn't work afterwards
    • sometimes the video stops with an error

Also reproducible with Flatpak (freedesktop ffmpeg noopenh264+freedesktop openh264):
$ sudo apt install flatpak
$ sudo flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo
$ sudo flatpak install org.mozilla.firefox
$ flatpak run org.mozilla.firefox https://twitter.com/50Hertzcom/status/1628740928042065927

Blocks: flatpak
No longer blocks: 1663844
Depends on: 1663844
Summary: When decoding video by OpenH264 GMP and position at video stream is changed, decoding fails. (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch ) → When decoding video by OpenH264 (GMP or Flatpak openh264) and position at video stream is changed, decoding fails. (Mozilla could take Fedora's downstream fix: https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch )

Flatpak would be fixed by bug 1755361+bug 1827453 if this bug fixes GMP.

It sounds like for OpenH264 we should not consider frames other than IDR to be I_FRAME:
https://searchfox.org/mozilla-central/rev/ad732108b073742d7324f998c085f459674a6846/dom/media/platforms/agnostic/bytestreams/H264.cpp#983,988

As we promote that to a keyframe else where:
https://searchfox.org/mozilla-central/rev/ad732108b073742d7324f998c085f459674a6846/dom/media/mp4/MP4Demuxer.cpp#397

This unfortunately did not solve my other seeking problem, but I can look at fixing this as well.

Assignee: nobody → aosmond
Status: NEW → ASSIGNED
No longer depends on: 1663844

Reading the backlog of comments, it appears it was already figured out and we declined to resolve the issue. We may have to accept this is broken and either update OpenH264 ourselves or hope someone else does it in the future.

Are you sure? This bug did not discuss the final patch from https://src.fedoraproject.org/rpms/firefox/blob/rawhide/f/mozilla-1670333.patch which applies a workaround for GMP decoding only.
From my understanding, anything that a change could break is already broken for OpenH264 and this patch would at least fix one use case for it.

The following attachment was not restricted to GMP and therefore affected mac and android, but they are offtopic:
(Jean-Yves Avenard [:jya] from comment #19)

(In reply to Martin StrΓ‘nskΓ½ [:stransky] from comment #11)

Created attachment 9181557 [details]
a simple workaround

So while this change may work for that video, I can guarantee you that it would badly break with other videos (it will cause decoding error on both mac and android in particular).

Give me some time. I reproduced your twitter issue, thank you for the test case. I'll see what I can do regarding comment 23.

If I seek enough, I eventually generate this error or similar, with the playback/seeking improvements from other bugs:

[OpenH264] this = 0x0x7fc75dbd3000, Error:DecodeCurrentAccessUnit()::::::PrefetchPic ERROR, pSps->iNumRefFrames:8.

Turning on multithreading for the decoder did not fix it (maybe it made it less likely).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: