771833 - drawImage doesn't work on MediaStream assigned to <video>

Status: VERIFIED FIXED

(Core :: WebRTC, defect)

Description

[Anant Narayanan [:anant]]

From Paul's comment in bug 691234:

"When I set the stream as the source of a video element, and then try to draw a frame of this video to a canvas, it does not work.

The drawImage methods calls nsCanvasRenderingContext2D::DrawImage, that calls nsLayoutUtils::SurfaceFromElement, that bails if the element does not have a current principal, hence the failure. When I comment the relevant lines, everything works fine, though.

I've no idea of the security implications, I just though it would be fun to have real time preview, and to be able to take still picture from my webcam to a canvas and to be able to process the image afterwards."

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Added roc to comment on mediastreams, canvases and principals.

Comment 2

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

Why isn't this working? What principal are we setting on the WebRTC MediaStream?

Comment 3

[Anant Narayanan [:anant]]

I don't think we're setting a principal, the drawImage call fails here: https://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#4286

Also, the comment at https://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLMediaElement.cpp#2541 indicates that we'll need to set one, but I'm not sure.

Comment 4

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

(In reply to Anant Narayanan [:anant] from comment #3)
> Also, the comment at
> https://mxr.mozilla.org/mozilla-central/source/content/html/content/src/
> nsHTMLMediaElement.cpp#2541 indicates that we'll need to set one, but I'm
> not sure.

That comment isn't relevant here.

You need to call nsDOMMediaStream::CombineWithPrincipal, passing a principal that must be subsumed by any principal that can read the stream contents. If you want the page calling getUserMedia to be able to read the stream contents, pass the page's principal.

Comment 5

[Anant Narayanan [:anant]]

Attachment: Assign the correct principal to nsDOMMediaStream

Thanks roc! This patch fixes the problem.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 640324 [details] [diff] [review]
Assign the correct principal to nsDOMMediaStream

I don't feel I have the knowledge to r+ this; leaving it to roc

Comment 7

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

Comment on attachment 640324 [details] [diff] [review]
Assign the correct principal to nsDOMMediaStream

Review of attachment 640324 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that

::: dom/media/MediaManager.cpp
@@ +150,4 @@
>    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
>    nsRefPtr<MediaEngineSource> mSource;
>    StreamListeners* mListeners;
> +  nsPIDOMWindow* mWindow;

nsRefPtr in case mWindow goes away

Comment 8

[Anant Narayanan [:anant]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39fe6a5577d7

Comment 9

[Anant Narayanan [:anant]]

Backed out, B2G build was busted in the dependent patch.

Comment 10

[Anant Narayanan [:anant]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64cff7aafcc4

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/64cff7aafcc4

Comment 12

[Sonny Piers [:sonny]]

Throws a NS_ERROR_NOT_AVAILABLE: Component is not available

Testcase: http://jsbin.com/ixasep

Comment 13

[Sonny Piers [:sonny]]

Paul told me that for him the error is thrown despite the fact the video is correctly drew within the canvas.

For me the frame isn't draw.

OSX 10.7

Comment 14

[Sonny Piers [:sonny]]

Attachment: testcase

Comment 15

[Anant Narayanan [:anant]]

Yes, in my test I was able to get the frame drawn (which is why I committed the patch and marked as RESOLVED), but now I'm able to reproduce your problem on a different machine. Something subtle is going on here, definitely needs to be investigated.

Comment 16

[Sonny Piers [:sonny]]

On a different machine (Linux) it throws:
NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.drawImage]

Comment 17

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

Attachment: fix

This fixes it.

We really need an automated test here!

Comment 18

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

Attachment: fix v2

Sorry, some unrelated stuff crept into the last patch.

Comment 19

[Anant Narayanan [:anant]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/deb98a757d4a

Comment 20

[(no longer active)]

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

Comment 21

[Sonny Piers [:sonny]]

It still doesn't works with the testcase but if I register the listener on 'canplay' event instead of 'play' it works and it doesn't throws 'NS_ERROR_NOT_AVAILABLE: Component is not available' anymore.
Should it be kept that way?

Comment 22

[Jason Smith [:jsmith]]

(In reply to Sonny Piers [:sonny] from comment #21)
> It still doesn't works with the testcase but if I register the listener on
> 'canplay' event instead of 'play' it works and it doesn't throws
> 'NS_ERROR_NOT_AVAILABLE: Component is not available' anymore.
> Should it be kept that way?

I can also reproduce this error on the latest nightly by doing the following with the test case:

1. Load the test case
2. Press back to the previous page in firefox and forward back to the test case

Result - NS_ERROR_NOT_AVAILABLE: Component is not available is thrown, the video fails to stream.

Comment 23

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

(In reply to Sonny Piers [:sonny] from comment #21)
> It still doesn't works with the testcase but if I register the listener on
> 'canplay' event instead of 'play' it works and it doesn't throws
> 'NS_ERROR_NOT_AVAILABLE: Component is not available' anymore.
> Should it be kept that way?

Ah, so that testcase is incorrect because the "play" event fires as soon as we call play(), which may be before video data is actually in the stream. Listening on "canplay" or "loadeddata" is the right thing to do.

Comment 24

[Sonny Piers [:sonny]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23) 
> Ah, so that testcase is incorrect because the "play" event fires as soon as
> we call play(), which may be before video data is actually in the stream.
> Listening on "canplay" or "loadeddata" is the right thing to do.

Right, makes sense.
Actually I tried with listening on loadeddata, it doesn't works, still throws the error.
The only one that works in our case is canplay.
Should I file a bug? I believe loadeddata for this use case makes more sense than canplay.

Comment 25

[Sonny Piers [:sonny]]

Attachment: testcase v2

Comment 26

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

(In reply to Sonny Piers [:sonny] from comment #24)
> Should I file a bug? I believe loadeddata for this use case makes more sense
> than canplay.

yes please.

Comment 27

[Sonny Piers [:sonny]]

Bug 778682

Comment 28

[Jason Smith [:jsmith]]

The original bug verifies here. However - There is a problem that was detected as a result of this bug fix in this use case:

1. Load the test case and wait for it to load
2. Close Firefox

Result - Firefox crashes on shutdown

Will file a followup bug for this.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: testcase v3

Update for the testcase to match the latest specs. 

Jason, when I run this testcase it does not work for me. I get an error:

[07:48:34.592] NS_ERROR_NOT_AVAILABLE: Component is not available @ ixasep.xhtml:20

Which is: context.drawImage(video, 0, 0);

Do you see the same?

Comment 30

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

Try waiting for "playing" or "loadeddata" instead of "canplay"?

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: testcase v4

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #30)
> Try waiting for "playing" or "loadeddata" instead of "canplay"?

It doesn't change anything. But what I have seen is that when I reload the page, the video and canvas elements are playing the video stream. I would expect that with the testcase we only create two shots. And the canvas does not show the stream. What's wrong here?

Comment 32

[Dominic Alie]

The bug is stil there for MediaStream coming from a PeerConnection.

If I have two video, one with a mozSrcObject from getUserMedia and one with mozSrcObject from a PeerConnection and I try to drawImage, the local getUserMedia works fine but the one from PeerConnection still throws a NS_ERROR_NOT_AVAILABLE exception.

Comment 33

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

Please file a new bug for that.

Comment 34

[Michel Gutierrez]

I get the same issue as Dominic with remote streams. Bug #854094 has been filed.

Comment 35

[Rob Manson]

Doh - testcase v4 attached to this bug now fails from v21.0 onwards on Ubuntu 12.04 LTS.  

Here's the latest UA I have tested:

Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130529 Firefox/24.0

This was working fine in v20.n but stopped after upgrading to 21.  No other apps/libs were updated on this Ubuntu box and this gUM/drawImage flow definitely works on Chrome so there's no systemic issues.

If you need any other info please let me know.