Closed Bug 1357133 Opened 3 years ago Closed 3 years ago

Opus + Widevine fails to decode; CDM crashes in Nightly 55 [@ widevinecdm.dll@0xc82bf]

Categories

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

52 Branch
Unspecified
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 + fixed
firefox55 + fixed

People

(Reporter: joeyparrish, Assigned: cpearce)

Details

(Keywords: crash, reproducible)

Crash Data

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce:

Visit this page for a one-step repro:

https://v2-0-8-dot-shaka-player-demo.appspot.com/demo/?asset=//storage.googleapis.com/shaka-demo-assets/angel-one-widevine-webm-only/dash.mpd;license=//widevine-proxy.appspot.com/proxy;play


Actual results:

Decode error around 11 seconds (content has 10-second clear lead)


Expected results:

It should either play to the end (60 seconds) or Firefox should report that Opus is not supported
It should be noted that clear Opus is working fine.  Original bug report on Shaka Player: https://github.com/google/shaka-player/issues/755
[Tracking Requested - why for this release]:

Anthony, here is a reproducible Widevine CDM crash. The video stops midstream on Firefox 53-54, but crashes on start in Nightly 55. I am using Widevine version 1.4.8.903 on Windows 10.
Crash Signature: [@ widevinecdm.dll@0xc82bf]
Component: Untriaged → Audio/Video: Playback
Flags: needinfo?(ajones)
Keywords: crash, reproducible
Product: Firefox → Core
Hardware: Unspecified → x86_64
Summary: Opus + Widevine fails to decode → Opus + Widevine fails to decode; CDM crashes in Nightly 55 [@ widevinecdm.dll@0xc82bf]
I can repro this crash on both Mac and Windows Nightly 55. I bisected this crash regression to PChromiumCDM bug 1353929.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → All
Hardware: x86_64 → Unspecified
Tracking 54/55+ and adding one of the other top signatures so it gets picked up in crash stats.

If the bug in Comment 3 landed only in nightly 55, will this crash also affect 52 and other branches? There appear to be other crashes in crash stats correlated to 52.
(In reply to Marcia Knous [:marcia - use ni] from comment #4)
> If the bug in Comment 3 landed only in nightly 55, will this crash also
> affect 52 and other branches? There appear to be other crashes in crash
> stats correlated to 52.

It sounds like there might be two different problems in this one bug report: the playback error Joey reported and the Nightly 55 crash I saw. We might want to fork these problems into two separate bugs, but since the STR are the same, I think we should just keep this one bug report until cpearce investigates. (He is currently out on PTO.)
I filed a Mac bug earlier that I think is a dupe of this one: Bug 1356378
(In reply to Chris Peterson [:cpeterson] from comment #5)
> (In reply to Marcia Knous [:marcia - use ni] from comment #4)
> > If the bug in Comment 3 landed only in nightly 55, will this crash also
> > affect 52 and other branches? There appear to be other crashes in crash
> > stats correlated to 52.
> 
> It sounds like there might be two different problems in this one bug report:
> the playback error Joey reported and the Nightly 55 crash I saw.

Yup! I'm fixing the crash in bug 1359621.

(In reply to Marcia Knous [:marcia - use ni] from comment #6)
> I filed a Mac bug earlier that I think is a dupe of this one: Bug 1356378

Thanks Marcia! We can dupe bug 1356378 and any other duplicates onto bug 1359621. Hopefully there aren't too many dupes.
Flags: needinfo?(ajones)
Crash Signature: [@ widevinecdm.dll@0xc82bf] → [@ widevinecdm.dll@0xc82bf] [@ libwidevinecdm.dylib@0x33b5b ]
(In reply to Chris Pearce (:cpearce) from comment #7)
> Thanks Marcia! We can dupe bug 1356378 and any other duplicates onto bug
> 1359621. Hopefully there aren't too many dupes.

cpearce, the STR in comment 0 crashes Widevine in Nightly 55, but in 53 and 54 the video stream stops without crashing (AFAICT). Are those separate bugs?
Flags: needinfo?(cpearce)
Yes, I'm fixing the crash in bug 1358373 (not in bug 1359621 as I said in comment 7; I messed up the duping). I'd assume that Nightly will have the same behaviour as 53/54 once I fix bug 1359621. We should fix the playback failure in this bug.
Assignee: nobody → cpearce
Flags: needinfo?(cpearce)
The problem here is that the new ChromiumCDM IPC protocol (new in 55) is assuming the CDM will allocate 320x240 buffer, but it's allocating buffers with Y stride 384, U/V stride 192, so we don't have a shmem the right size as we assume the CDM allocates the minimal sized buffer required. The old WidevineAdapter works fine in 55 on this stream; so I don't understand why it doesn't work in 53 and 54.
Comment on attachment 8862674 [details]
Bug 1357133 - Recover from incorrectly guessing the CDM's shmem sizes.

https://reviewboard.mozilla.org/r/134540/#review137538

::: dom/media/gmp/ChromiumCDMParent.cpp:649
(Diff revision 1)
> +  // content process pre-allocate a set of shmems and give them to the CDM
> +  // process in advance of them being needed.
> +  //
> +  // When the CDM needs to allocate a buffer for storing a decoded video
> +  // frame, the CDM host gives it one of these shmems' buffers. When this
> +  // is sent back to the content process, we uploading to a GPU surface,

'uploading' -> 'upload it'

::: dom/media/gmp/GMPUtils.cpp:245
(Diff revision 1)
>  }
>  
> -static int32_t
> -Align16(int32_t aNumber)
> +static size_t
> +Align16(size_t aNumber)
>  {
>    const uint32_t mask = 15; // Alignment - 1.

uint32_t -> size_t
Attachment #8862674 - Flags: review?(gsquelart) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/22823b481c3c
Recover from incorrectly guessing the CDM's shmem sizes. r=gerald
https://hg.mozilla.org/mozilla-central/rev/22823b481c3c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I can reproduce the playback hang on ESR52 as well. ESR45 hits a "DRM.REQUESTED_KEY_SYSTEM_CONFIG_UNAVAILABLE" error immediately upon load. Given that we're not shipping anymore releases off ESR45 anyway, calling it unaffected.

Chris, please request Beta/ESR52 approval on this patch when you feel it's ready for uplift.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)
> I can reproduce the playback hang on ESR52 as well. ESR45 hits a
> "DRM.REQUESTED_KEY_SYSTEM_CONFIG_UNAVAILABLE" error immediately upon load.
> Given that we're not shipping anymore releases off ESR45 anyway, calling it
> unaffected.
> 
> Chris, please request Beta/ESR52 approval on this patch when you feel it's
> ready for uplift.

Firefox 55 has a new backend for decoding Widevine video, and this patch only applies to the new backend. So we can't uplift it.

I tested the old backend in Firefox 55 and that works fine. So that means both the old backend and the new backend failed on this content for different reasons, and both are fixed in 55.

So to fix this in ESR52 we'll need to uplift a different patch against the old backend. I'll see if I can figure out which patch we need.
Flags: needinfo?(cpearce)
This was fixed in the old Widevine backend by 4ba776d3f2d3 in bug 1306477. So it should be fixed in 54, i.e. on current beta.

Given that uplifting this to ESR52 would basically be uplifting a feature to ESR (support for WebM subsample encryption), I don't think we should do that.
Yeah, WFM on Beta as well. Thanks for tracking this down, Chris!
You need to log in before you can comment on or make changes to this bug.