1597216 - createMediaElementSource(), mozCaptureStream() or setSinkId() for a media element displayed through picture-in-picture will stall the pip-player's video

Status: RESOLVED FIXED

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

Description

[Andreas Pehrson [:pehrsons]]

Looking at the code for how picture-in-picture hooks into the VideoSink, using any of the methods mentioned in the title, will stall the pip-player's video, because they all cause the MediaDecoderStateMachine to create a new MediaSink, and that seems like a case that's not covered.

Found while looking at bug 1596777.

Comment 1

[Andreas Pehrson [:pehrsons]]

This only affects picture-in-picture really. Mike, can you triage this?

I can try to provide a fix.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Thanks for filing!

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Actually, I'm not the triage owner for this component - so I'm resetting the priority to let the triage owner or their team set the priority.

Comment 4

[Andreas Pehrson [:pehrsons]]

(In reply to Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos) from comment #3)

Actually, I'm not the triage owner for this component - so I'm resetting the priority to let the triage owner or their team set the priority.

Well I mean, I am part of the triage team for this component, but seeing that this only impacts your feature, I'd like your input on the priority. The buggy code here exists for the sole purpose of supporting picture-in-picture.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

The buggy code here exists for the sole purpose of supporting picture-in-picture.

Fair, it just felt rather presumptuous of me to set the priority in someone else's component.

I think P3 makes sense.

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1597216 - Handle changes to mMediaSink in MediaDecoderStateMachine. r?alwu

Depends on D53711

Comment 7

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

Does it affect 71? (we ship pip on windows in 71)

Comment 8

[Mike Conley (:mconley) (:⚙️)]

From what I can tell, yes this affects 71 - but I suspect the occurrence of this bug in the wild will likely be infrequent (I don't believe any of the major sites we're targeting like YouTube, Netflix, Amazon Prime or Twitch use these APIs).

Comment 9

[Andreas Pehrson [:pehrsons]]

(In reply to Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos) from comment #8)

From what I can tell, yes this affects 71 - but I suspect the occurrence of this bug in the wild will likely be infrequent (I don't believe any of the major sites we're targeting like YouTube, Netflix, Amazon Prime or Twitch use these APIs).

I agree with this. I'm not aware of a large video site using any of these APIs.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01d2c84810f0
Handle changes to mMediaSink in MediaDecoderStateMachine. r=alwu

Comment 11

[Daniel Varga [:dvarga]]

Backed out 7 changesets (Bug 1597216, Bug 1596777, Bug 1536156) for reftest failures at reftest/bipbop_300_215kbps.mp4.lastframe.htm.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=a3fa99d936f30f37cdedba6092d7136dbdd96a16

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=277498214&repo=autoland&lineNumber=2191

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=adf7d49a44f8e4d91e687140aedd06721e6e45b0

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e01020298260
Backed out 7 changesets (bug 1597216, bug 1596777, bug 1536156) for reftest failures at reftest/bipbop_300_215kbps.mp4.lastframe.htm. On a CLOSED TREE

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e146778e544
Handle changes to mMediaSink in MediaDecoderStateMachine. r=alwu

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5e146778e544