1200099 - Eliminate HTMLMediaElement::mPlaybackStream

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We don't really need this. Instead of blocking MediaStream playback when the media element is paused, we can just disconnect the MediaStream output, and then reconnect it if we play again.

This simplifies code and makes progress towards fixing bug 1189506.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/try/rev/a5efada89b46

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Green except where I'm hitting bug 1084185. I'll work around that by making test_streams_element_capture behave like test_streams_element_capture_reset.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Green on Linux64 that is...

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=64ea408dd482

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97619f506caa

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Ensure mSameOriginMedia is propagated to DecodedStream even if we don't get a watch notification. r=jwwang

Bug 1200099. Ensure mSameOriginMedia is propagated to DecodedStream even if we don't get a watch notification. r=jwwang

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Add a test that captured cross-origin video streams render black. r=jwwang

Bug 1200099. Add a test that captured cross-origin video streams render black. r=jwwang

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Ensure mSameOrigin is initialized. r=jwwang

Bug 1200099. Ensure mSameOrigin is initialized. r=jwwang

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Relax test assumptions to accommodate streams not blocking. r=jwwang

Bug 1200099. Relax test assumptions to accommodate streams not blocking. r=jwwang

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Stop using a distinct mPlaybackStream to play a media stream through an HTMLMediaElement. r=jwwang

Bug 1200099. Stop using a distinct mPlaybackStream to play a media stream through an HTMLMediaElement. r=jwwang

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Ensure mSameOriginMedia is propagated to DecodedStream even if we don't get a watch notification. r=jwwang

Bug 1200099. Ensure mSameOriginMedia is propagated to DecodedStream even if we don't get a watch notification. r=jwwang

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Add a test that captured cross-origin video streams render black. r=jwwang

Bug 1200099. Add a test that captured cross-origin video streams render black. r=jwwang

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Relax test assumptions to accommodate streams not blocking. r=jwwang

Bug 1200099. Relax test assumptions to accommodate streams not blocking. r=jwwang

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: MozReview Request: Bug 1200099. Stop using a distinct mPlaybackStream to play a media stream through an HTMLMediaElement. r=jwwang

Bug 1200099. Stop using a distinct mPlaybackStream to play a media stream through an HTMLMediaElement. r=jwwang

Comment 17

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8655728 [details]
MozReview Request: Bug 1200099. Ensure mSameOriginMedia is propagated to DecodedStream even if we don't get a watch notification. r=jwwang

https://reviewboard.mozilla.org/r/18007/#review16127

::: dom/media/MediaDecoderStateMachine.cpp:336
(Diff revision 1)
> +  SameOriginMediaChanged();

I believe this is fixed by bug 1199654 which give DecodedStream::mSameOrigin an initial false value.

Comment 18

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8655729 [details]
MozReview Request: Bug 1200099. Add a test that captured cross-origin video streams render black. r=jwwang

https://reviewboard.mozilla.org/r/18009/#review16131

Comment 19

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8655730 [details]
MozReview Request: Bug 1200099. Relax test assumptions to accommodate streams not blocking. r=jwwang

https://reviewboard.mozilla.org/r/18011/#review16129

::: dom/media/test/test_streams_element_capture.html:78
(Diff revision 1)
> -manager.runTests(gSmallTests, startTest);
> +var testVideo = getPlayableVideo(gSmallTests);

Might be worth a todo that we test only one file for the duration bug of gstreamer. Once it is fixed, we are able to run through all files in gSmallTests again.

Comment 20

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8655731 [details]
MozReview Request: Bug 1200099. Stop using a distinct mPlaybackStream to play a media stream through an HTMLMediaElement. r=jwwang

https://reviewboard.mozilla.org/r/18013/#review16133

::: dom/media/VideoFrameContainer.cpp
(Diff revision 1)
> -  mImageSizeChanged = false;

Why deleting this line?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://reviewboard.mozilla.org/r/18007/#review16127

> I believe this is fixed by bug 1199654 which give DecodedStream::mSameOrigin an initial false value.

That's probably true, but I think it's best to avoid assuming that mDecoder's mSameOriginMedia is false when InitializationTask runs.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://reviewboard.mozilla.org/r/18013/#review16165

::: dom/media/VideoFrameContainer.cpp
(Diff revision 1)
> -  mImageSizeChanged = false;

If the video frame changes size, and then someone calls ClearCurrentFrame, we should still ensure that InvalidateFrame gets called by VideoFrameContainer::InvalidateWithFlags. It's simpler and safer to only clear mImageSizeChanged when we actually call InvalidateFrame, anyway.

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/526a487b5ac8
https://hg.mozilla.org/integration/mozilla-inbound/rev/af77d3d2496d
https://hg.mozilla.org/integration/mozilla-inbound/rev/84cc83e991e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/e110acdf8e64

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/526a487b5ac8
https://hg.mozilla.org/mozilla-central/rev/af77d3d2496d
https://hg.mozilla.org/mozilla-central/rev/84cc83e991e0
https://hg.mozilla.org/mozilla-central/rev/e110acdf8e64

Comment 25

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue has caused a major smoketest blocker.