Last Comment Bug 792665 - Separate HTMLMediaElement.src from HTMLMediaElement.srcObject
: Separate HTMLMediaElement.src from HTMLMediaElement.srcObject
Status: RESOLVED FIXED
[qa-]
: dev-doc-complete
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla18
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-19 20:32 PDT by Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
Modified: 2012-12-17 06:10 PST (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
fix (12.46 KB, patch)
2012-09-19 22:16 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
cpearce: review+
rjesup: review+
Details | Diff | Review
updated patch (12.83 KB, patch)
2012-09-20 15:40 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Review

Description Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-19 20:32:48 PDT

    
Comment 1 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-19 20:51:22 PDT
Maybe we should put .srcObject behind a pref?
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-19 20:51:34 PDT
Or else we should prefix it.
Comment 3 [:jesup] on pto until 2016/7/5 Randell Jesup 2012-09-19 21:03:42 PDT
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) (Exited; email my personal email if necessary) 2012-09-19 22:11:38 PDT
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) (Exited; email my personal email if necessary) 2012-09-19 22:16:29 PDT
Created attachment 662814 [details] [diff] [review]
fix
Comment 6 [:jesup] on pto until 2016/7/5 Randell Jesup 2012-09-19 23:21:42 PDT
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) (Exited; email my personal email if necessary) 2012-09-20 01:50:30 PDT
(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) 2012-09-20 15:11:57 PDT
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) (Exited; email my personal email if necessary) 2012-09-20 15:14:50 PDT
(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) (Exited; email my personal email if necessary) 2012-09-20 15:37:32 PDT
(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) (Exited; email my personal email if necessary) 2012-09-20 15:40:01 PDT
Created attachment 663186 [details] [diff] [review]
updated patch

Fixed the issues in comment #7. This is ready for landing.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-24 02:01:08 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3425c7e124b0
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-09-24 17:56:53 PDT
https://hg.mozilla.org/mozilla-central/rev/3425c7e124b0
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-10-03 15:00:37 PDT
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) (Exited; email my personal email if necessary) 2012-10-03 21:49:39 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/9878b1edba48
Comment 16 Jean-Yves Perrier [:teoli] 2012-12-12 05:49:57 PST
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) (Exited; email my personal email if necessary) 2012-12-16 20:38:49 PST
They are the same. For now, #1 is not on standards track, so feel free to refer to #2. Thanks a ton!

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