792665 - Separate HTMLMediaElement.src from HTMLMediaElement.srcObject

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Comment 1

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

Maybe we should put .srcObject behind a pref?

Comment 2

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

Or else we should prefix it.

Comment 3

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

I suppose prefix it, though the entire way we (all the browsers) handle not-yet-agreed-to specs makes me want to pull my hair out....

I'm tempted to use what Google is in PeerConnection: add a suffix that gets bumped whenever the API changes, so if two browsers implement the same API, you're good.  It also handles the case of our first implementation is replaced with a newer-but-not-final one.

So that would be video.srcObject01 = stream;

Comment 4

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

That style is relatively unknown and I fear would create much confusion. I'll go with prefixing for now.

Anant, Mike: I'm worried that this change might break some B2G apps. OTOH it'll be good to have this API aligned with standards before B2G apps get locked down. Can you notify whoever needs to know that it's happening?

Comment 5

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

Attachment: fix

Comment 6

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

Comment on attachment 662814 [details] [diff] [review]
fix

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

Also, we'll need URL.createObjectURL(stream) and support for handling those URLs in video.src.  (This can be a separate bug)

r+ if you're happy with your responses to my questions.

::: content/html/content/src/nsHTMLMediaElement.cpp
@@ +463,5 @@
>      }
>    }
> +  // Should we store unsupported values on the element's attribute anyway?
> +  // Let's not.
> +  return NS_OK;

What if you do:

video.src = foo;
video.srcObject = stream;
video.srcObject = null;

Should it stop playing?  (I think so; is null an object here?).

Should it restart src? (Good question.  Currently I don't think it does, but what happens if you fiddle controls?)

Similar questions for the other way around.

Comment 7

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

(In reply to Randell Jesup [:jesup] from comment #6)
> Also, we'll need URL.createObjectURL(stream) and support for handling those
> URLs in video.src.  (This can be a separate bug)

Yes, I'm working on that in bug 792675.

At some stage we should add support for element.srcObject = blob as well.

> What if you do:
> 
> video.src = foo;
> video.srcObject = stream;
> video.srcObject = null;
> 
> Should it stop playing?  (I think so; is null an object here?).

Yes, it should accept null and stop playing the stream (and start playing foo again). I need to fix that, right now setting to null would just be ignored. If you do these three statements all at once then the overall effect would be the same as video.src = foo --- restarting playback of foo.

> Similar questions for the other way around.

You mean 'video.srcObject = stream; video.src = foo; video.src = ""'? That's less interesting; srcObject always wins, so the assignments to src don't change anything. We would restart playback of 'stream', but that's basically a no-op anyway.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 662814 [details] [diff] [review]
fix

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

::: content/media/test/Makefile.in
@@ +129,5 @@
>  		test_seekLies.html \
>  		test_media_sniffer.html \
>  		contentType.sjs \
> +		test_streams_srcObject.html \
> +		test_streams_element_capture_bug752784.html \

test_streams_element_capture_bug752784.html isn't included in the patch?

Comment 9

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

(In reply to Chris Pearce (:cpearce) from comment #8)
> test_streams_element_capture_bug752784.html isn't included in the patch?

It's already in the tree but someone forgot to add it Makefile.in.

Comment 10

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

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9)
> (In reply to Chris Pearce (:cpearce) from comment #8)
> > test_streams_element_capture_bug752784.html isn't included in the patch?
> 
> It's already in the tree but someone forgot to add it Makefile.in.

Actually it's not. It's only in my tree and we already added that test as a crashtest elsewhere. So I removed that line.

Comment 11

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

Attachment: updated patch

Fixed the issues in comment #7. This is ready for landing.

Comment 12

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/3425c7e124b0

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3425c7e124b0

Comment 14

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

Comment on attachment 663186 [details] [diff] [review]
updated patch

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

This is an API change. We should take it on Aurora before the uplift to beta, to head off people trying to use the old API. Very few people are using it yet so I don't expect this to cause any compatibility issues right now, but we don't want any to develop.

Comment 15

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/9878b1edba48

Comment 16

[Jean-Yves Perrier [:teoli]]

I was trying to document this but wasn't able to fully understand the change here.

I see in the MDN doc that .src is already correctly documented (regarding its behavior in HTML5). By looking at the tests in the patch here I see that .srcObject deals with streams, and that this is a different spec (AFAIK there is no notion of media streams in the HTML5 spec). 

I'm a little bit lost with all the "streams" I see. There's is roc's proposal [1] but also those I found in WebRTC [2]. Are they the same or, if not, which one are we talking about for .srcObject?


[1] https://dvcs.w3.org/hg/audio/raw-file/tip/streams/StreamProcessing.html
[2] https://developer.mozilla.org/en-US/docs/WebRTC/MediaStream_API

Comment 17

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

They are the same. For now, #1 is not on standards track, so feel free to refer to #2. Thanks a ton!

Comment 18

[Jean-Yves Perrier [:teoli]]

Updated:
https://developer.mozilla.org/en-US/docs/DOM/HTMLMediaElement and
https://developer.mozilla.org/en-US/docs/Firefox_18_for_developers