Closed Bug 771833 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla16

People

(Reporter: anant, Assigned: anant)

References

Details

(Whiteboard: [qa!])

Attachments

(3 files, 4 obsolete files)

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."
Added roc to comment on mediastreams, canvases and principals.
Why isn't this working? What principal are we setting on the WebRTC MediaStream?
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.
(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.
Thanks roc! This patch fixes the problem.
Assignee: nobody → anant
Status: NEW → ASSIGNED
Attachment #640324 - Flags: review?(roc)
Attachment #640324 - Flags: review?(rjesup)
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
Attachment #640324 - Flags: review?(rjesup)
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
Attachment #640324 - Flags: review?(roc) → review+
Backed out, B2G build was busted in the dependent patch.
https://hg.mozilla.org/mozilla-central/rev/64cff7aafcc4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Throws a NS_ERROR_NOT_AVAILABLE: Component is not available

Testcase: http://jsbin.com/ixasep
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attached file testcase (obsolete) —
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.
On a different machine (Linux) it throws:
NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.drawImage]
Attached patch fix (obsolete) — Splinter Review
This fixes it.

We really need an automated test here!
Attachment #646393 - Flags: review?(anant)
Attached patch fix v2Splinter Review
Sorry, some unrelated stuff crept into the last patch.
Attachment #646393 - Attachment is obsolete: true
Attachment #646393 - Flags: review?(anant)
Attachment #646396 - Flags: review?(anant)
Attachment #646396 - Flags: review?(anant) → review+
https://hg.mozilla.org/mozilla-central/rev/deb98a757d4a
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
QA Contact: jsmith
Whiteboard: [qa+]
Target Milestone: mozilla16 → mozilla17
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?
Target Milestone: mozilla17 → mozilla16
(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.
(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.
(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.
Attached file testcase v2 (obsolete) —
Attachment #641978 - Attachment is obsolete: true
(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.
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.
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
Flags: in-testsuite?
Attached file testcase v3 (obsolete) —
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?
Attachment #647095 - Attachment is obsolete: true
Try waiting for "playing" or "loadeddata" instead of "canplay"?
Attached file 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?
Attachment #673103 - Attachment is obsolete: true
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.
I get the same issue as Dominic with remote streams. Bug #854094 has been filed.
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.
Depends on: 879717
You need to log in before you can comment on or make changes to this bug.