1492915 - WebMDemuxer should not try and read keyframe information from partially encrypted packets

Status: RESOLVED FIXED

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

Description

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

When an unencrypted packet is read from a WebM, we attempt to determine if it is a keyframe by inspecting it. If a packet is detected as encrypted we do not perform this check and defer to the container, as the encrypted data cannot be sanely inspected.

When support for partially encrypted WebMs was added, the condition to avoid checking encrypted packets appears to have not been updated for the partial encryption case[0]. This should be updated to avoid misidentifying frames.

[0]: https://searchfox.org/mozilla-central/rev/94e37e71ffbfd39e6ad73ebcda5d77cce8d341ae/dom/media/webm/WebMDemuxer.cpp#679

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1492915 - Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=55d586859da040be3c2a5499fe48e6046786b2ca

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 9010710 [details]
Bug 1492915 - Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

Jean-Yves Avenard [:jya] has approved the revision.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6a202d7695b
Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/e6a202d7695b

Comment 6

[Julien Cristau [:jcristau] (back April 22)]

Calling this wontfix for 62, please request uplift to beta/esr60 if you think we should fix it there.

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 9010710 [details]
Bug 1492915 - Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

After some consideration, requesting uplift for beta/ESR.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: The change is low risk and impacts the playback of encrypted content, which is often paid content. It may also affect AV1 content that is encrypted, depending on how sites choose to serve such content. It would be desirable to avoid such bustages.
User impact if declined: Failure to playback media stored in WebM's with subsample encryption. As AV1 rolls out, sites serving encrypted AV1 in WebM may fail to playback.
Fix Landed on Version: Firefox 64 during nightly cycle
Risk to taking this patch (and alternatives if risky): Low, the change is small and in a well understood part of the codebase
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1306477
[User impact if declined]:Failure to playback media stored in WebM's with subsample encryption. As AV1 rolls out, sites serving encrypted AV1 in WebM may fail to playback.
[Is this code covered by automated tests?]: Partially, but 
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: No other changes are needed
[Is the change risky?]: No
[Why is the change risky/not risky?]: The change is small and in a well understood part of the codebase
[String changes made/needed]: None

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9010710 [details]
Bug 1492915 - Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

Fix landed to nightly 10 days ago with no reported regression, uplift approved for 63 beta 12, thanks.

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/948c74f2d3df

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9010710 [details]
Bug 1492915 - Stop WebMDemuxer from inspecting partially encrypted packets for keyframe information. r=jya

Fixes broken playback of some encrypted content. Approved for ESR 60.3.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/2d280e557331