Move MediaStream and MediaStreamTrack implementation from modules/mediastream to core/mediastream.

Move MediaStream and MediaStreamTrack implementation from modules/mediastream to core/mediastream.
Move RTCPeerConnection implementation from modules/mediastream to modules/rtcpeerconnection.

The purpose of this move is to cleanup dependencies between modules and make it easier to implement
HTMLMediaElement::srcObject for MediaStreams.
It also prepare for adding a new module MediaRecorder that also will be dependent on core/mediastream.
Currently, WebAudio, WebSpeech and RTCPeerConnection depend on modules/mediaStream.

[perkj_chrome, 2014-10-13 08:35:54]

Patchset #3 (id:1680001) has been deleted

[perkj_chrome, 2014-10-13 08:55:23]

perkj@chromium.org changed reviewers:
+ jochen@chromium.org, philipj@opera.com

[perkj_chrome, 2014-10-13 08:55:26]

Hej guys.
This cl move MediaStream to core and thus clean up dependencies between the
modules. 

It should also mean that implementing HTMLMediaElement::srcObject should be
easier for at least MediaStreams. 

Further more, I have started on MediaRecorder that also will depend on
MediaStreams. 

Wdyt? 
Does it make sence? Can we take the decision to do it?

[philipj_slow, 2014-10-13 12:30:06]

I think that moving MediaStream into core/ makes sense given the dependency
situation. Did you figure out if the cross-module include in
webaudio/MediaStreamAudioSourceNode.h violates any rules or not?

How coupled is the user media stuff to MediaStream, would it make sense to have
it in a separate module?

There's a typo in the description: core/rtcpeerconnection. The "current"
core/mediaStream should also be modules/mediastream I guess.

Given that this creates a new module, it needs review from Source/OWNERS, which
is jochen :)

[perkj_chrome, 2014-10-13 14:57:37]

On 2014/10/13 12:30:06, philipj wrote:
> I think that moving MediaStream into core/ makes sense given the dependency
> situation. Did you figure out if the cross-module include in
> webaudio/MediaStreamAudioSourceNode.h violates any rules or not?
> 
> How coupled is the user media stuff to MediaStream, would it make sense to
have
> it in a separate module?
> 
> There's a typo in the description: core/rtcpeerconnection. The "current"
> core/mediaStream should also be modules/mediastream I guess.
> 
> Given that this creates a new module, it needs review from Source/OWNERS,
which
> is jochen :)

I think it should be doable to split out the parts affecting Navigator to a
separate module if we think that make sense. But I would rather not.. 
Regarding webaudio, it includes header files for MediaStream in
modules/mediastream. Speech also do that as seen from the cl.

[haraken, 2014-10-13 15:17:38]

On 2014/10/13 14:57:37, perkj wrote:
> On 2014/10/13 12:30:06, philipj wrote:
> > I think that moving MediaStream into core/ makes sense given the dependency
> > situation. Did you figure out if the cross-module include in
> > webaudio/MediaStreamAudioSourceNode.h violates any rules or not?
> > 
> > How coupled is the user media stuff to MediaStream, would it make sense to
> have
> > it in a separate module?
> > 
> > There's a typo in the description: core/rtcpeerconnection. The "current"
> > core/mediaStream should also be modules/mediastream I guess.
> > 
> > Given that this creates a new module, it needs review from Source/OWNERS,
> which
> > is jochen :)
> 
> I think it should be doable to split out the parts affecting Navigator to a
> separate module if we think that make sense. But I would rather not.. 
> Regarding webaudio, it includes header files for MediaStream in
> modules/mediastream. Speech also do that as seen from the cl.

We're moving more components from core/ to modules/, and thus it wouldn't make
sense to move modules/ back to core/ unless you have a strong reason to do so.
So I'd like to understand why you cannot avoid moving mediastream/ to core/.

Would you describe your plan to reorganize WebAudio, WebSpeech,
RTCPeerConnection, MediaSource, MediaStream etc?

[perkj_chrome, 2014-10-13 17:45:34]

perkj@chromium.org changed reviewers:
+ haraken@chromium.org

[perkj_chrome, 2014-10-13 17:47:28]

On 2014/10/13 15:17:38, haraken wrote:
> On 2014/10/13 14:57:37, perkj wrote:
> > On 2014/10/13 12:30:06, philipj wrote:
> > > I think that moving MediaStream into core/ makes sense given the
dependency
> > > situation. Did you figure out if the cross-module include in
> > > webaudio/MediaStreamAudioSourceNode.h violates any rules or not?
> > > 
> > > How coupled is the user media stuff to MediaStream, would it make sense to
> > have
> > > it in a separate module?
> > > 
> > > There's a typo in the description: core/rtcpeerconnection. The "current"
> > > core/mediaStream should also be modules/mediastream I guess.
> > > 
> > > Given that this creates a new module, it needs review from Source/OWNERS,
> > which
> > > is jochen :)
> > 
> > I think it should be doable to split out the parts affecting Navigator to a
> > separate module if we think that make sense. But I would rather not.. 
> > Regarding webaudio, it includes header files for MediaStream in
> > modules/mediastream. Speech also do that as seen from the cl.
> 
> We're moving more components from core/ to modules/, and thus it wouldn't make
> sense to move modules/ back to core/ unless you have a strong reason to do so.
> So I'd like to understand why you cannot avoid moving mediastream/ to core/.
> 
> Would you describe your plan to reorganize WebAudio, WebSpeech,
> RTCPeerConnection, MediaSource, MediaStream etc?

Hej
MediaStream is used  in the specs for RTCPeerConnection, WebAudio, MediaRecorder
and apparently Speech. Both WebAudio and Speech currently depend on
modules/mediastream. 
From what I understood from trying to implement HTMLMediaElement::srcObject for
MediaStreams, https://codereview.chromium.org/545933002/ core can not depend on
modules and modules should? not depend on other modules. Too me, jochens
suggestion in that cl therfore make sense. 

I donät plan to make any more large refactorings in this code. I just wanted to
make it possible to implement HTMLMediaElement::srcObject for MediaStreams and
at the same time prepare for adding a first version of MediaRecorder. 
Am I on the wrong path?

[jochen (gone - plz use gerrit), 2014-10-13 18:57:29]

modules/ must not depend on each other, furthermore, core/ now would have to
depend on modules (via HTMLMediaElement::srcObject).

I don't see a good way around this, other than moving media* to core. Given that
both are enabled on all platforms, and are an integral part of the platform, I
think that makes sense.

[philipj_slow, 2014-10-13 20:42:36]

On 2014/10/13 18:57:29, jochen wrote:
> modules/ must not depend on each other, furthermore, core/ now would have to
> depend on modules (via HTMLMediaElement::srcObject).
> 
> I don't see a good way around this, other than moving media* to core. Given
that
> both are enabled on all platforms, and are an integral part of the platform, I
> think that makes sense.

Right, modules/mediasource is also going to have to move into core/ when the
MediaSource bit of srcObject is implemented. That's a shame, but as a bonus the
extra HTMLMediaSource layer that (I think) exists only to avoid a core-modules
dependency can be retired.

[haraken, 2014-10-14 01:14:24]

On 2014/10/13 20:42:36, philipj wrote:
> On 2014/10/13 18:57:29, jochen wrote:
> > modules/ must not depend on each other, furthermore, core/ now would have to
> > depend on modules (via HTMLMediaElement::srcObject).
> > 
> > I don't see a good way around this, other than moving media* to core. Given
> that
> > both are enabled on all platforms, and are an integral part of the platform,
I
> > think that makes sense.
> 
> Right, modules/mediasource is also going to have to move into core/ when the
> MediaSource bit of srcObject is implemented. That's a shame, but as a bonus
the
> extra HTMLMediaSource layer that (I think) exists only to avoid a core-modules
> dependency can be retired.

The requirements are:

- modules/ shouldn't depend on other modules/.
- core/ shouldn't depend on modules/.
- core/ should be slimmed down.

As tasak@ suggested, how about putting all of RTCPeerConnection, WebAudio,
MediaRecorder
MediaStream, MediaSource and Speech into modules/media/ ? The approach would
increase modules/media/, but given the requirements, it seems better than
increasing core/.

[haraken, 2014-10-14 01:15:44]

haraken@chromium.org changed reviewers:
+ tasak@google.com

[perkj_chrome, 2014-10-14 06:14:39]

On 2014/10/14 01:15:46, haraken wrote:

haraken, tasak, would you also want to move  HTMLMediaElement to modules/media
(I have no idea if it is possible) or as tasak suggested in the
HTMLMediaElement::srcObject patch:
Create a partial interface in modules/media/.....

HTMLMediaElementMediaStream.idl:

typedef (MediaStream or MediaSource or Blob) MediaProvider;

[
  RuntimeEnabled=MediaStream,
] partial interface HTMLMediaElement {
    attribute MediaProvider? srcObject;
};

Even if we use a partial interface, we need more information about the srcObject
in the HTMLMediaElement in order to play it. HTMLMediaElement would at least
need to retrieve an objectURl to keep the current behavior so 
we need to ad an interface in core that MediaStream can implement.
We might need more information to implement for example
http://w3c.github.io/mediacapture-main/#loading-and-playing-a-mediastream-in-....

[haraken, 2014-10-14 06:41:44]

On 2014/10/14 06:14:39, perkj wrote:
> On 2014/10/14 01:15:46, haraken wrote:
> 
> haraken, tasak, would you also want to move  HTMLMediaElement to modules/media
> (I have no idea if it is possible) or as tasak suggested in the
> HTMLMediaElement::srcObject patch:
> Create a partial interface in modules/media/.....
> 
> HTMLMediaElementMediaStream.idl:
> 
> typedef (MediaStream or MediaSource or Blob) MediaProvider;
> 
> [
>   RuntimeEnabled=MediaStream,
> ] partial interface HTMLMediaElement {
>     attribute MediaProvider? srcObject;
> };
> 
> Even if we use a partial interface, we need more information about the
srcObject
> in the HTMLMediaElement in order to play it. HTMLMediaElement would at least
> need to retrieve an objectURl to keep the current behavior so 
> we need to ad an interface in core that MediaStream can implement.
> We might need more information to implement for example
>
http://w3c.github.io/mediacapture-main/#loading-and-playing-a-mediastream-in-....

Let's call the partial interface HTMLMediaElementMediaProvider.

Wouldn't it be possible to:

- Put HTMLMediaElementMediaProvider.{h,cpp} in modules/media/.
- HTMLMediaElementMediaProvider.{h,cpp} can touch srcObject.
- If HTMLMediaElementMediaProvider.{h,cpp} needs to access core/ things, it can
just call methods in core/.

I'm curious why you cannot restructure the code so that
HTMLMediaElementMediaProvider touches srcObject but core/ doesn't touch
srcObject.

[perkj_chrome, 2014-10-14 07:13:35]

On 2014/10/14 06:41:44, haraken wrote:
> On 2014/10/14 06:14:39, perkj wrote:
> > On 2014/10/14 01:15:46, haraken wrote:
> > 
> > haraken, tasak, would you also want to move  HTMLMediaElement to
modules/media
> > (I have no idea if it is possible) or as tasak suggested in the
> > HTMLMediaElement::srcObject patch:
> > Create a partial interface in modules/media/.....
> > 
> > HTMLMediaElementMediaStream.idl:
> > 
> > typedef (MediaStream or MediaSource or Blob) MediaProvider;
> > 
> > [
> >   RuntimeEnabled=MediaStream,
> > ] partial interface HTMLMediaElement {
> >     attribute MediaProvider? srcObject;
> > };
> > 
> > Even if we use a partial interface, we need more information about the
> srcObject
> > in the HTMLMediaElement in order to play it. HTMLMediaElement would at least
> > need to retrieve an objectURl to keep the current behavior so 
> > we need to ad an interface in core that MediaStream can implement.
> > We might need more information to implement for example
> >
>
http://w3c.github.io/mediacapture-main/#loading-and-playing-a-mediastream-in-....
> 
> Let's call the partial interface HTMLMediaElementMediaProvider.
> 
> Wouldn't it be possible to:
> 
> - Put HTMLMediaElementMediaProvider.{h,cpp} in modules/media/.
> - HTMLMediaElementMediaProvider.{h,cpp} can touch srcObject.
> - If HTMLMediaElementMediaProvider.{h,cpp} needs to access core/ things, it
can
> just call methods in core/.
> 
> I'm curious why you cannot restructure the code so that
> HTMLMediaElementMediaProvider touches srcObject but core/ doesn't touch
> srcObject.

Humm, right, true, my bad. HTMLMediaElementMediaProvider should be able to call
whatever the HTMLMediaElement expose and thus we should be able to structure the
code that way.

I don't have a strong opinion in either direction - but it would be nice with a
decision and a plan forward.
Further more- I will not be allowed to spend that much more time on this I am
afraid. 
Can we have subfolders beneath modules/media where each subfolder is allowed to
include from each other? WebAudio has quite a few files. 

If this is the path we want to go I suggest that:

I rename modules/mediastream to modules/media/mediastream now.

When time permits I will: 
- Ask owners of WebAudio and Speech to move to modules/media to cleanup the
dependency on MediaStreams, This is an already existing layer violation. 
- Implement a first version of MediaRecorder that I put in
modules/media/mediarecorder as an experimental feature.
- I will update my patch for HTMLMediaElement::srcObject as suggested by haraken
and tasak - it will initially only support MediaStreams and maybe blob?  The
code can be put in modules/media since it will be used by both Mediastream and
MediaSource. 

Philip would you mind sometime to:
- Move MediaSource to modules/media/mediasource ? 
- Implement srcObject for MediaSource based on my patch?

[haraken, 2014-10-14 07:29:57]

On 2014/10/14 07:13:35, perkj wrote:
> On 2014/10/14 06:41:44, haraken wrote:
> > On 2014/10/14 06:14:39, perkj wrote:
> > > On 2014/10/14 01:15:46, haraken wrote:
> > > 
> > > haraken, tasak, would you also want to move  HTMLMediaElement to
> modules/media
> > > (I have no idea if it is possible) or as tasak suggested in the
> > > HTMLMediaElement::srcObject patch:
> > > Create a partial interface in modules/media/.....
> > > 
> > > HTMLMediaElementMediaStream.idl:
> > > 
> > > typedef (MediaStream or MediaSource or Blob) MediaProvider;
> > > 
> > > [
> > >   RuntimeEnabled=MediaStream,
> > > ] partial interface HTMLMediaElement {
> > >     attribute MediaProvider? srcObject;
> > > };
> > > 
> > > Even if we use a partial interface, we need more information about the
> > srcObject
> > > in the HTMLMediaElement in order to play it. HTMLMediaElement would at
least
> > > need to retrieve an objectURl to keep the current behavior so 
> > > we need to ad an interface in core that MediaStream can implement.
> > > We might need more information to implement for example
> > >
> >
>
http://w3c.github.io/mediacapture-main/#loading-and-playing-a-mediastream-in-....
> > 
> > Let's call the partial interface HTMLMediaElementMediaProvider.
> > 
> > Wouldn't it be possible to:
> > 
> > - Put HTMLMediaElementMediaProvider.{h,cpp} in modules/media/.
> > - HTMLMediaElementMediaProvider.{h,cpp} can touch srcObject.
> > - If HTMLMediaElementMediaProvider.{h,cpp} needs to access core/ things, it
> can
> > just call methods in core/.
> > 
> > I'm curious why you cannot restructure the code so that
> > HTMLMediaElementMediaProvider touches srcObject but core/ doesn't touch
> > srcObject.
> 
> Humm, right, true, my bad. HTMLMediaElementMediaProvider should be able to
call
> whatever the HTMLMediaElement expose and thus we should be able to structure
the
> code that way.
> 
> I don't have a strong opinion in either direction - but it would be nice with
a
> decision and a plan forward.
> Further more- I will not be allowed to spend that much more time on this I am
> afraid. 
> Can we have subfolders beneath modules/media where each subfolder is allowed
to
> include from each other? WebAudio has quite a few files. 

Yes, this is allowed. modules/X is not allowed to depend on modules/Y, but
modules/X/Y is allowed to depend on modules/X/Z.


> If this is the path we want to go I suggest that:
> 
> I rename modules/mediastream to modules/media/mediastream now.
> 
> When time permits I will: 
> - Ask owners of WebAudio and Speech to move to modules/media to cleanup the
> dependency on MediaStreams, This is an already existing layer violation. 
> - Implement a first version of MediaRecorder that I put in
> modules/media/mediarecorder as an experimental feature.
> - I will update my patch for HTMLMediaElement::srcObject as suggested by
haraken
> and tasak - it will initially only support MediaStreams and maybe blob?  The
> code can be put in modules/media since it will be used by both Mediastream and
> MediaSource. 

I think this is a right way to go.

[philipj_slow, 2014-10-14 07:44:48]

It sounds like the end game is merging modules/mediasource, mediastream, speech
and webaudio into a single module. Are the module owners OK with that?

[haraken, 2014-10-14 07:46:32]

On 2014/10/14 07:44:48, philipj wrote:
> It sounds like the end game is merging modules/mediasource, mediastream,
speech
> and webaudio into a single module. Are the module owners OK with that?

I think we should raise this topic to blink-dev@.

[jochen (gone - plz use gerrit), 2014-10-14 07:47:58]

haraken, can you explain how core's version of HTMLMediaElement would work if
the media module is not compiled in?

I.e. how does it detect that srcObject is only a blob?

[haraken, 2014-10-14 07:52:08]

On 2014/10/14 07:47:58, jochen wrote:
> haraken, can you explain how core's version of HTMLMediaElement would work if
> the media module is not compiled in?
> 
> I.e. how does it detect that srcObject is only a blob?

In my understanding, that's the work of HTMLMediaElementMediaProvider (which is
a partial interface placed in modules/media/). The HTMLMediaElementMediaProvider
detects that and dispatches any core/ methods depending of the types of
srcObject. Wouldn't it work?

[tasak, 2014-10-14 08:11:55]

On 2014/10/14 07:46:32, haraken wrote:

> I think we should raise this topic to blink-dev@.

I think, this probably needs much more time.

So my suggestion is:

(1) firstly add MediaStream-only srcObject to HTMLMediaElement, i.e.
https://codereview.chromium.org/545933002/#ps1

(2) discuss the topic in blink-dev@.

# We can do (1) and (2) in parallel. After we finish (2),

(3) according to the discussion, do some refactoring to fix dependency
violation.

(4) change srcObject from MediaStream? to MediaProvider.

philipj, haraken, what do you think about this?

[jochen (gone - plz use gerrit), 2014-10-14 08:12:46]

if we don't compile the module in, HTMLMediaElement::srcObject should still work
(and just be a Blob).

So somehow we'd need a magic that does blobs in core/ and when you link in the
media module, it does blobs, and the two media types

[jochen (gone - plz use gerrit), 2014-10-14 08:18:07]

i think doing a mediastream only srcObject first while discussing this is fine.

but we shouldn't introduce any hacks or custom bindings for this (so the first
version will probably not have a srcObject if you compile without modules)

[haraken, 2014-10-14 08:24:23]

On 2014/10/14 08:12:46, jochen wrote:
> if we don't compile the module in, HTMLMediaElement::srcObject should still
work
> (and just be a Blob).
> 
> So somehow we'd need a magic that does blobs in core/ and when you link in the
> media module, it does blobs, and the two media types

I agree that we want to make core/ work without modules/ in the future, but
given that the current code base is far from a state where core/ can work
without modules/, I don't think that that requirement (i.e., core/ should work
without modules/) should block the HTMLMediaElement work.

I agree that we can move HTMLMediaElement's work forward while discussing the
topic in parallel.

[perkj_chrome, 2014-10-14 09:57:04]

On 2014/10/14 08:24:23, haraken wrote:
> On 2014/10/14 08:12:46, jochen wrote:
> > if we don't compile the module in, HTMLMediaElement::srcObject should still
> work
> > (and just be a Blob).
> > 
> > So somehow we'd need a magic that does blobs in core/ and when you link in
the
> > media module, it does blobs, and the two media types
> 
> I agree that we want to make core/ work without modules/ in the future, but
> given that the current code base is far from a state where core/ can work
> without modules/, I don't think that that requirement (i.e., core/ should work
> without modules/) should block the HTMLMediaElement work.
> 
> I agree that we can move HTMLMediaElement's work forward while discussing the
> topic in parallel.

I just posted
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/XQkaIlYpyxg.

[philipj_slow, 2014-10-16 18:56:33]

On 2014/10/14 07:13:35, perkj wrote:

> Philip would you mind sometime to:
> - Move MediaSource to modules/media/mediasource ? 
> - Implement srcObject for MediaSource based on my patch?

Yeah, I'm OK with doing srcObject only for MediaStream at this point. I should
weigh in on blink-dev as well, but regardless of the outcome we'll need an
Intent to Implement, if you just need this for running tests, or an Intent to
Implement and Ship, if you want to enable it everywhere. I'd probably lean
towards not shipping yet, by having a SrcObject status=experimental in
RuntimeEnabledFeatures.in. Ultimately that's up to the API OWNERS, though.

[jochen (gone - plz use gerrit), 2014-10-17 11:59:11]

yes, the feature should be marked as experimental before we have an
intent-to-ship

[perkj_chrome, 2015-10-12 06:16:27]

perkj@chromium.org changed reviewers:
- haraken@chromium.org, jochen@chromium.org, philipj@opera.com, tasak@google.com