1609417 - Some ogg files don't play

Status: RESOLVED FIXED

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

Description

[Krzysztof]

Attachment: photo_sound.ogg.oga

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

1. Open the attached ogg file
2. Open WWW Console (ctrl + shift + K)
3. Press the Play button.

Actual results:

The file doesn't play, console warnings show up.

The file is playing on Firefox for Android. It's also OK after conversion to mp3.

I tried to flip media.rdd* preferences on nightly, but it doesn't help.

Expected results:

See also this report: https://support.mozilla.org/en-US/questions/1277244

User claims that this is a regression in version 72.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Martin McNickle]

I can reproduce in the nightly.

User Agent (Nightly): Mozilla/5.0 (X11; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Debug Log (from file attached above):

[Child 9792, MediaDecoderStateMachine #1] WARNING:
Decoder=7f1ea97ed800 Decode error: NS_ERROR_OUT_OF_MEMORY (0x8007000e) - DecodeNextSample: 
file /home/mmcnickle/projects/firefox/mozilla-central/dom/media/MediaDecoderStateMachine.cpp, line 3358

Debug Log (from file in forum)

[Child 9792, MediaDecoderStateMachine #3] WARNING:
Decoder=7f1ea8ee7c00 Decode error: NS_ERROR_OUT_OF_MEMORY (0x8007000e) - DecodeNextSample:
file /home/mmcnickle/projects/firefox/mozilla-central/dom/media/MediaDecoderStateMachine.cpp, line 3358

Comment 3

[Martin McNickle]

I wasn't able to find a version that would play this file so not sure if this is a regression, at least on linux, going as far back as Firefox/60.0. The error does change in these earlier versions though:

Console Log (Firefox/60.0):

Media resource http://edapskov.ru/temp/photo_sound.ogg could not be decoded,
error: Error Code: NS_ERROR_DOM_MEDIA_DECODE_ERR (0x806e0004)
Details: RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > mozilla::VorbisDataDecoder::ProcessDecode(mozilla::MediaRawData*): vorbis_synthesis:-135

Comment 4

[Martin McNickle]

(In reply to Martin McNickle from comment #3)

I wasn't able to find a version that would play this file so not sure if this is a regression, at least on linux, going as far back as Firefox/60.0. The error does change in these earlier versions though:

Console Log (Firefox/60.0):

Media resource http://edapskov.ru/temp/photo_sound.ogg could not be decoded,
error: Error Code: NS_ERROR_DOM_MEDIA_DECODE_ERR (0x806e0004)
Details: RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > mozilla::VorbisDataDecoder::ProcessDecode(mozilla::MediaRawData*): vorbis_synthesis:-135

My mistake. The mozregression builds wouldn't work, but the publicly available binaries did. Can confirm it's a regression between Firefox/69.0.3 and Firefox/70.0.1.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This is a regression from bug 1524049 which enabled media.rdd-vorbis.enabled on Windows non-AArch. For bisection on Nightly, it was necessary to switch media.audio-max-decode-error from 0 (Nightly) to 3 (Release or Beta). It got enabled for Linux and macOS before that: https://hg.mozilla.org/mozilla-central/log/ce600e54dfb93a80b01c4874f6467fcae16d5e92/modules/libpref/init/StaticPrefList.yaml?patch=&linerange=5808:5819

[Tracking Requested - why for this release]:
Audio playback broken, unknown how widespread. This might be fixable by increasing the allowed exception count, or switching media.rdd-vorbis.enabled back to disabled.

Comment 6

[Michael Froman [:mjf]]

I've had a chance to investigate. The issue is that the last sample is zero length and when the content process uses the vorbis decoder directly, the decoder returns NS_ERROR_DOM_MEDIA_DECODE_ERR here[1]. I don't whether that is a legit audio file or not, but what happens is that when decoding vorbis on RDD, the zero length packet is causing triggering NS_ERROR_OUT_OF_MEMORY because RemoteDecoderParent::DecodeNextSample thinks we asked for Shmem that we didn't receive here[2]. I'll look at why we're getting into that state.

[1] https://searchfox.org/mozilla-central/source/dom/media/platforms/agnostic/VorbisDecoder.cpp#163-166
[2] https://searchfox.org/mozilla-central/source/dom/media/ipc/RemoteDecoderParent.cpp#85-90

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Too late to fix in 72 at this point, but it would be good if we could get a fix in time for 73.

Comment 8

[Michael Froman [:mjf]]

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #5)

This is a regression from bug 1524049 which enabled media.rdd-vorbis.enabled on Windows non-AArch. For bisection on Nightly, it was necessary to switch media.audio-max-decode-error from 0 (Nightly) to 3 (Release or Beta). It got enabled for Linux and macOS before that: https://hg.mozilla.org/mozilla-central/log/ce600e54dfb93a80b01c4874f6467fcae16d5e92/modules/libpref/init/StaticPrefList.yaml?patch=&linerange=5808:5819

[Tracking Requested - why for this release]:
Audio playback broken, unknown how widespread. This might be fixable by increasing the allowed exception count, or switching media.rdd-vorbis.enabled back to disabled.

The identified regression is incorrect, although turning off RDD for vorbis does cause the audio to play. The actual regression is Bug 1562910, where I added ShmemPool usage to RDD processing.

Comment 9

[Michael Froman [:mjf]]

Attachment: Bug 1609417 - don't treat 0 size samples as an error in RemoteDecoderParent::DecodeNextSample. r?jya!

Vorbis can send a 0-length sample, and we want to let the decoder deal
with the handling the error returning a NS_ERROR_DOM_MEDIA_DECODE_ERR, instead
of RemoteDecoderParent::DecodeNextSample returning NS_ERROR_OUT_OF_MEMORY. Only
return NS_ERROR_OUT_OF_MEMORY if MediaRawData is smaller than the actual sample
size recorded in MediaRawDataIPDL that comes from RemoteDecoderChild::Decode.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4aa7350e4b0f
don't treat 0 size samples as an error in RemoteDecoderParent::DecodeNextSample. r=jya

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4aa7350e4b0f

Comment 12

[Michael Froman [:mjf]]

Comment on attachment 9122165 [details]
Bug 1609417 - don't treat 0 size samples as an error in RemoteDecoderParent::DecodeNextSample. r?jya!

Beta/Release Uplift Approval Request

• User impact if declined: Some ogg files will not play when RDD decoding is enabled.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is an extremely small change (3 lines, 2 are comments) and simply allows the decoder to deal with a 0-length sample rather than incorrectly consider it an OOM error.
• String changes made/needed: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9122165 [details]
Bug 1609417 - don't treat 0 size samples as an error in RemoteDecoderParent::DecodeNextSample. r?jya!

Deal better with ogg files with 0-length sample to avoid playback errors. Approved for 73.0b9.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/14886e51af43