Last Comment Bug 771833 - drawImage doesn't work on MediaStream assigned to <video>
: drawImage doesn't work on MediaStream assigned to <video>
Status: VERIFIED FIXED
[qa!]
:
Product: Core
Classification: Components
Component: WebRTC (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Anant Narayanan [:anant]
: Jason Smith [:jsmith]
:
Mentors:
Depends on: 691234 879717
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-07 13:57 PDT by Anant Narayanan [:anant]
Modified: 2013-06-05 04:49 PDT (History)
10 users (show)
jsmith: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Assign the correct principal to nsDOMMediaStream (1.85 KB, patch)
2012-07-09 12:25 PDT, Anant Narayanan [:anant]
roc: review+
Details | Diff | Splinter Review
testcase (901 bytes, application/xhtml+xml)
2012-07-13 11:44 PDT, Sonny Piers [:sonny]
no flags Details
fix (5.25 KB, patch)
2012-07-26 16:03 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
no flags Details | Diff | Splinter Review
fix v2 (948 bytes, patch)
2012-07-26 16:05 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
anant: review+
Details | Diff | Splinter Review
testcase v2 (996 bytes, application/xhtml+xml)
2012-07-30 01:05 PDT, Sonny Piers [:sonny]
no flags Details
testcase v3 (1005 bytes, application/xhtml+xml)
2012-10-18 22:51 PDT, Henrik Skupin (:whimboo)
no flags Details
testcase v4 (1.01 KB, application/xhtml+xml)
2012-10-25 11:59 PDT, Henrik Skupin (:whimboo)
no flags Details

Description Anant Narayanan [:anant] 2012-07-07 13:57:47 PDT
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] 2012-07-08 21:29:47 PDT
Added roc to comment on mediastreams, canvases and principals.
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-08 23:20:43 PDT
Why isn't this working? What principal are we setting on the WebRTC MediaStream?
Comment 3 Anant Narayanan [:anant] 2012-07-08 23:25:11 PDT
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) 2012-07-08 23:31:22 PDT
(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] 2012-07-09 12:25:00 PDT
Created attachment 640324 [details] [diff] [review]
Assign the correct principal to nsDOMMediaStream

Thanks roc! This patch fixes the problem.
Comment 6 Randell Jesup [:jesup] 2012-07-09 13:46:01 PDT
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) 2012-07-09 18:03:34 PDT
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 9 Anant Narayanan [:anant] 2012-07-11 21:46:37 PDT
Backed out, B2G build was busted in the dependent patch.
Comment 11 Ed Morley [:emorley] 2012-07-12 09:35:25 PDT
https://hg.mozilla.org/mozilla-central/rev/64cff7aafcc4
Comment 12 Sonny Piers [:sonny] 2012-07-13 10:42:27 PDT
Throws a NS_ERROR_NOT_AVAILABLE: Component is not available

Testcase: http://jsbin.com/ixasep
Comment 13 Sonny Piers [:sonny] 2012-07-13 11:42:03 PDT
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] 2012-07-13 11:44:47 PDT
Created attachment 641978 [details]
testcase
Comment 15 Anant Narayanan [:anant] 2012-07-13 11:46:16 PDT
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] 2012-07-15 05:12:27 PDT
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) 2012-07-26 16:03:17 PDT
Created attachment 646393 [details] [diff] [review]
fix

This fixes it.

We really need an automated test here!
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-26 16:05:19 PDT
Created attachment 646396 [details] [diff] [review]
fix v2

Sorry, some unrelated stuff crept into the last patch.
Comment 21 Sonny Piers [:sonny] 2012-07-29 07:56:32 PDT
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] 2012-07-29 08:27:22 PDT
(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) 2012-07-29 15:28:46 PDT
(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] 2012-07-30 01:03:42 PDT
(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] 2012-07-30 01:05:45 PDT
Created attachment 647095 [details]
testcase v2
Comment 26 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-30 02:05:51 PDT
(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] 2012-07-30 03:40:08 PDT
Bug 778682
Comment 28 Jason Smith [:jsmith] 2012-07-30 08:23:07 PDT
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) 2012-10-18 22:51:21 PDT
Created attachment 673103 [details]
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) 2012-10-19 04:08:00 PDT
Try waiting for "playing" or "loadeddata" instead of "canplay"?
Comment 31 Henrik Skupin (:whimboo) 2012-10-25 11:59:57 PDT
Created attachment 675234 [details]
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 2013-01-15 09:03:15 PST
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) 2013-01-16 14:04:16 PST
Please file a new bug for that.
Comment 34 Michel Gutierrez 2013-03-23 05:48:43 PDT
I get the same issue as Dominic with remote streams. Bug #854094 has been filed.
Comment 35 Rob Manson 2013-06-05 04:40:15 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.