Widevine / Audio Decoding error with Shaka player

VERIFIED FIXED in Firefox -esr60

Status

()

defect
P3
normal
Rank:
25
VERIFIED FIXED
a year ago
7 months ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks 2 bugs)

unspecified
mozilla62
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr6061+ verified, firefox60 wontfix, firefox61 verified, firefox62 verified)

Details

(URL)

Attachments

(4 attachments)

(Assignee)

Description

a year ago
Spawned from bug 1447821.

Open https://shaka-player-demo.appspot.com/demo/#asset=https://dai.google.com/ondemand/dash/content/2474151/vid/tosr2-widevine/manifest.mpd;license=https://proxy.uat.widevine.com/proxy;lang=en-US;build=uncompiled

After the google home demo another video starts, after a few seconds an audio decoding error appears

 error: 3 (NS_ERROR_DOM_MEDIA_DECODE_ERR (0x806e0004) - virtual mozilla::MediaResult mozilla::FFmpegAudioDecoder<57>::DoDecode(mozilla::MediaRawData*, uint8_t*, int, bool*, mozilla::MediaDataDecoder::DecodedData&): FFmpeg audio error:-1094995529)
Rank: 25
Priority: -- → P3

Comment 1

11 months ago
Is there any chance if this issue can be pushed up the priority?

We couldn't find any workarounds to get this content played back successfully on Firefox.

Comment 2

11 months ago
This affects content that transitions from clear to encrypted initialization segments.  For example, in multi-Period DASH if the first Period is "fully" clear and the second has encrypted content, it will fail to play.

Note this also affects the video streams, but the first error reported is for audio.  Here is another reproduction that is created from two known good assets.  The first minute is from the 'Sintel 4k (multicodec)' asset on this site and the second minute is from 'Sintel 4k (multicodec, Widevine)'.  Both assets play fine by themselves.  Both the links below work fine on Chrome.

https://shaka-player-demo.appspot.com/demo/#asset=https://storage.googleapis.com/shaka-demo-assets/_bugs/firefox-clear-then-encrypted/dash.mpd;license=https://cwip-shaka-proxy.appspot.com/no_auth;lang=en-US;build=uncompiled;play

And here is the same source material but using MP4 only.  Firefox doesn't report any errors but the video shows bad decoding.

https://shaka-player-demo.appspot.com/demo/#asset=https://storage.googleapis.com/shaka-demo-assets/_bugs/firefox-clear-then-encrypted/mp4.mpd;license=https://cwip-shaka-proxy.appspot.com/no_auth;lang=en-US;build=uncompiled;play

Updated

11 months ago
Duplicate of this bug: 1463213
(Assignee)

Comment 4

11 months ago
Any chances you could create a shorter demonstration, one where you don't have to wait several minutes to experience the problem?

thank you
Flags: needinfo?(modmaker)

Comment 5

11 months ago
You could also seek to near the end after it starts.  But here is one where it fails around 22 seconds.  (10 seconds clear segments, 20 seconds encrypted that has a 10 second clear lead)

https://shaka-player-demo.appspot.com/demo/#asset=https://storage.googleapis.com/shaka-demo-assets/_bugs/firefox-clear-then-encrypted/short.mpd;license=https://cwip-shaka-proxy.appspot.com/no_auth;lang=en-US;play;build=uncompiled

Also some more specifics on what Shaka Player is doing here:  
- Attach a MediaKeys object to the <video>
- Append the clear init segment, then the clear segments for the first Period
- Append the encrypted init segment and append the encrypted media segments. 

The encrypted segments have 10 seconds of clear lead and it fails when trying to decode the first encrypted frame.  It should have the required keys.

I know there is an EME conformance test that verifies clear then encrypted segments.  But in that test all the data is appended at once.  Maybe Firefox is failing because we are waiting a bit before we append the encrypted segments?
Flags: needinfo?(modmaker)
(Assignee)

Updated

11 months ago
Assignee: nobody → jyavenard
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 8

11 months ago
My gutfeel is that the special exception of the Android PDM that always re-use the decoder rather than creating a new one, will prevent this stream on an android device.... to be checked
(Assignee)

Comment 9

11 months ago
[Tracking Requested - why for this release]: We don't play content mixing encrypted / clear
Really wish there was something we could do to improve our test coverage here.
(Assignee)

Comment 11

11 months ago
There is a test, however we fell into the cracks due to how the test is set up.

I'll open a bug in the wpt to add coverage for this particular problem. Or maybe our friends at Google can take over, we're pretty far stretched resource wise right now
Flags: needinfo?(modmaker)
(Assignee)

Comment 12

11 months ago
I thought I had fixed the issue with Firefox on Android (forcing a new PDMFactory when the CDMProxy wasn't set for an encrypted stream), but always getting a decoder creation error when it needs to create a new one. Interestingly, Chrome for Android fails to decode those streams just the same.

Comment 13

11 months ago
mozreview-review
Comment on attachment 8980998 [details]
Bug 1454630 - P2. Use new PDMFactory whenever encryption type change.

https://reviewboard.mozilla.org/r/247124/#review253194

Shouldn't we be able to write a test for this simply by appending an encrypted stream after a non-encrypted stream to a MediaSource (or vice versa)?
Attachment #8980998 - Flags: review?(cpearce) → review+
(Assignee)

Updated

11 months ago
See Also: → 1464822
(Assignee)

Comment 14

11 months ago
The reason we fail to create the audio decoder for the encrypted content is that when the CDMProxy is set, the PDMFactory will only attempt to use the AndroidDecoderModule for all codecs, but opus is disable for the Android PDM (the sample is encrypted opus). I'll spawned another bug
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 18

11 months ago
With this new version, stream #1 on description plays on Android (we incorrectly attempted to reuse the same decoder on android even if encryption changed).

Different approach, now use two distinct PDMFactory, one for encrypted content, one for cleared.

Comment 19

11 months ago
mozreview-review
Comment on attachment 8980997 [details]
Bug 1454630 - P1. Simplify retrieval of current TrackInfo.

https://reviewboard.mozilla.org/r/247122/#review253552
Attachment #8980997 - Flags: review?(bvandyk) → review+

Comment 20

11 months ago
mozreview-review
Comment on attachment 8980998 [details]
Bug 1454630 - P2. Use new PDMFactory whenever encryption type change.

https://reviewboard.mozilla.org/r/247124/#review253554
Attachment #8980998 - Flags: review?(bvandyk) → review+

Comment 21

11 months ago
mozreview-review
Comment on attachment 8981142 [details]
Bug 1454630 - P3. Always recreate the PDMFactory when CDMProxy change.

https://reviewboard.mozilla.org/r/247238/#review253556
Attachment #8981142 - Flags: review?(bvandyk) → review+

Comment 22

11 months ago
mozreview-review
Comment on attachment 8981143 [details]
Bug 1454630 - P4. Only wait for CDMProxy if actually needed for the given decoder.

https://reviewboard.mozilla.org/r/247240/#review253558
Attachment #8981143 - Flags: review?(bvandyk) → review+
(Assignee)

Updated

11 months ago
Blocks: 1465073
(Assignee)

Comment 23

11 months ago
Thanks for the quick reviews

Comment 24

11 months ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c444cd068b2
P1. Simplify retrieval of current TrackInfo. r=bryce
https://hg.mozilla.org/integration/autoland/rev/16a0b5cfca59
P2. Use new PDMFactory whenever encryption type change. r=bryce,cpearce
https://hg.mozilla.org/integration/autoland/rev/250d768f4749
P3. Always recreate the PDMFactory when CDMProxy change. r=bryce
https://hg.mozilla.org/integration/autoland/rev/ed681c93dd5b
P4. Only wait for CDMProxy if actually needed for the given decoder. r=bryce

Comment 26

11 months ago
(In reply to Jean-Yves Avenard [:jya] from comment #11)
> There is a test, however we fell into the cracks due to how the test is set
> up.
> 
> I'll open a bug in the wpt to add coverage for this particular problem. Or
> maybe our friends at Google can take over, we're pretty far stretched
> resource wise right now

Sure, I'll write a wpt test for this.
Flags: needinfo?(modmaker)
Please request Beta approval on this when you feel that it has had sufficient bake time.
Flags: qe-verify+
Flags: needinfo?(jyavenard)
Flags: in-testsuite?
(Assignee)

Comment 28

11 months ago
Comment on attachment 8980997 [details]
Bug 1454630 - P1. Simplify retrieval of current TrackInfo.


Approval Request Comment
[Feature/Bug causing the regression]: not a new regression.
[User impact if declined]: Encrypted content with clear segment may not play on desktop, will certainly not play on Android
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: manually verified.
[Needs manual test from QE? If yes, steps to reproduce]: steps to reproduce are provided in the description
[List of other uplifts needed for the feature/fix]: all patches of this ticket
[Is the change risky?]: no
[Why is the change risky/not risky?]: we maintain two different decoding states rather than attempting to re-use the same one. It's far less risky that way than what it used to be. For the most popular sites using EME (Netflix, Amazon Prime, etc) they only use fully encrypted content, as such it's the same as before.
[String changes made/needed]: no
Flags: needinfo?(jyavenard)
Attachment #8980997 - Flags: approval-mozilla-esr60?
Attachment #8980997 - Flags: approval-mozilla-beta?
Comment on attachment 8980997 [details]
Bug 1454630 - P1. Simplify retrieval of current TrackInfo.

Fix for broken EME playback on some major sites. Approved for 61.0b11.
Attachment #8980997 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 31

11 months ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180605141053

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180604143143

Verified as fixed on Nightly build 62.0a1 (2018-06-05) and Beta 61.0b11 on MacOS 10.13.5.
At least Part 1 will need a rebased patch for ESR60 (unsure about the subsequent ones).
Flags: needinfo?(jyavenard)
(Assignee)

Comment 34

11 months ago
the conflicts were due to bug 1448222 and bug 1456743 (P2). I've merged that last one in while at it.
Comment on attachment 8980997 [details]
Bug 1454630 - P1. Simplify retrieval of current TrackInfo.

Thanks for doing the rebase. Approved for ESR 60.1.
Attachment #8980997 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

Comment 37

10 months ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180621121604

Verified as fixed on ESR build 60.1.0esr on MacOS 10.13.5.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
(Assignee)

Updated

7 months ago
Depends on: 1495735
You need to log in before you can comment on or make changes to this bug.