1654185 - Move webrtc code out of media/webrtc and into dom/media/webrtc

Status: RESOLVED FIXED

(Core :: WebRTC, enhancement, P2)

Description

[Nico Grunbaum [:ng, @chew:mozilla.org]]

While we are changing paths around for the libwebrtc update we should go ahead and collapse signaling/* into dom/media/webrtc. Doing so at the same time would create a single section of the Mercurial history that was polluted.
Edit: media/webrtc -> dom/media/webrtc

Comment 1

[Dan Minor [:dminor]]

If we look at what has been done for things like libdav1d and libaom, the upstream library lives under third_party and our vendoring code and build system stuff lives under media. So I'd propose that our vendoring scripts and moz.build files for libwebrtc would live under media/webrtc and the upstream code would be under third_party/libwebrtc.

Somewhere there is a standard process for how to handle vendoring libraries, we should be following that if we want to make changes. I'm not sure what the process is, because libwebrtc was exempted from it in the past.

Personally, I think it would make sense to move the signaling code to live with the rest of our webrtc code under dom/media/webrtc. I also think there's no reason to wait for an update to move the signaling code, this is the kind of thing we can work on now without worrying about breaking wfh stuff. We just need to agree where to move it :)

Comment 2

[Byron Campen [:bwc]]

So I have a small patch in signaling up for review (bug 1652426), but mjf is working on RTCDtlsTransport (bug 1307996) right now, so maybe we hold off until mjf is done with that?

I'm fine moving this stuff to dom/media/webrtc (I guess mtransport could eventually live somewhere like dom/media/webrtc/transport). There will probably be quite a few build-system simplifications we can do on the way.

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Byron, that sound reasonable.

Comment 4

[Michael Froman [:mjf]]

FYI: I've added a bug to account for adding the partial RTCDtlsTransport (just state and onstatechange), it is Bug 1654399. I'm hoping to push to review later this evening, so it should be soon.

The plan seems good to me.

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

(In reply to Dan Minor [:dminor] from comment #1)

Personally, I think it would make sense to move the signaling code to live with the rest of our webrtc code under dom/media/webrtc. I also think there's no reason to wait for an update to move the signaling code, this is the kind of thing we can work on now without worrying about breaking wfh stuff. We just need to agree where to move it :)

I agree about not needing to wait. I think it should happen before we start the merge process.

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

From the comments above, here is my proposed mapping:
media/webrtc/signaling/gtest -> dom/media/webrtc/tests/gtests <- note the plural
(DONE) media/webrtc/signaling/fuzztest -> dom/media/webrtc/tests/fuzztests <- note the plural
dom/media/tests/mochitest -> dom/media/webrtc/tests/mochitests <- note the plural
media/webrtc/signaling/src/common -> dom/media/webrtc/common
media/webrtc/signaling/src/jsep -> dom/media/webrtc/jsep
media/webrtc/signaling/src/media-conduit -> dom/media/webrtc/libwebrtcglue
media/webrtc/signaling/src/mediapipeline -> dom/media/webrtc/transportbridge
media/webrtc/signaling/src/peerconnection -> dom/media/webrtc/jsapi
media/webrtc/signaling/src/sdp -> dom/media/webrtc/sdp
media/mtransport -> dom/media/webrtc/transport <- note the dropped 'm'
(DONE) SIPCC components of media/webrtc/signaling/sdp directory -> third_party/sipcc
Some of these would benefit from and initial move of media/webrtc/signaling/src -> dom/media/webrtc
Move the vendoring (where one exists) and build scripts for nICER, sipcc, and libwebrtc into folders

• dom/media/webrtc/third_party_build/nICER
• dom/media/webrtc/third_party_build/sipcc
• dom/media/webrtc/third_party_build/libwebrtc
  Edited to reflect comments below

Comment 7

[Byron Campen [:bwc]]

(In reply to Nico Grunbaum [:ng, @chew:mozilla.org] from comment #6)

From the comments above, here is my proposed mapping:
media/webrtc/signaling/gtest -> dom/media/tests/gtests <- note the plural
media/webrtc/signaling/fuzztest -> dom/media/tests/fuzztests <- note the plural
media/webrtc/signaling/src/common -> dom/media/webrtc/common
media/webrtc/signaling/src/jsep -> dom/media/webrtc/jsep
media/webrtc/signaling/src/media-conduit -> dom/media/webrtc/mediaconduit <- note the removed dash
media/webrtc/signaling/src/mediapipeline -> dom/media/webrtc/mediapipeline
media/webrtc/signaling/src/peerconnection -> dom/media/webrtc/peerconnection
media/webrtc/signaling/src/sdp -> dom/media/webrtc/sdp
media/mtransport -> dom/media/webrtc/transport <- note the dropped 'm'
dom/media/tests/mochitest -> dom/media/tests/mochitests <- note the plural

I think I'd like the following variations:

• s/media-conduit/libwebrtcwrapper/, since that's what it is
• s/peerconnection/jsapi/ might be a good thing to do, even though there's some extra stuff in there that probably should not be
• s/mediapipeline/pipeline/ (I wish I could come up with a better name for this that indicates that it bridges between transport and libwebrtc wrapper, and also between libwebrtcwrapper and MediaTrackGraph)

Comment 8

[Dan Minor [:dminor]]

I think libwebrtcglue is a bit snappier and libwebrtcducttapeandbalingtwine is closer to the truth, but I'm ok with libwebrtcwrapper and don't want to bikeshed on this.

Comment 9

[Byron Campen [:bwc]]

(In reply to Dan Minor [:dminor] from comment #8)

I think libwebrtcglue is a bit snappier and libwebrtcducttapeandbalingtwine is closer to the truth, but I'm ok with libwebrtcwrapper and don't want to bikeshed on this.

I'm cool with any of these.

Comment 10

[Nico Grunbaum [:ng, @chew:mozilla.org]]

• s/mediapipeline/pipeline/ (I wish I could come up with a better name for this that indicates that it bridges between transport and libwebrtc wrapper, and also between libwebrtcwrapper and MediaTrackGraph)

How about transportbridge for now?

The stuff that is in peerconnection there that shouldn't be under the new scheme, would that largely move to common?

Comment 11

[Byron Campen [:bwc]]

(In reply to Nico Grunbaum [:ng, @chew:mozilla.org] from comment #10)

• s/mediapipeline/pipeline/ (I wish I could come up with a better name for this that indicates that it bridges between transport and libwebrtc wrapper, and also between libwebrtcwrapper and MediaTrackGraph)

How about transportbridge for now?

The stuff that is in peerconnection there that shouldn't be under the new scheme, would that largely move to common?

Not sure yet. Maybe in an /impl directory with the pipeline stuff. Not super important for this move.

Comment 12

[Michael Froman [:mjf]]

After some conversations and looking at the situation on the ground in dom/media, we'll do:
media/webrtc/signaling/fuzztest -> dom/media/webrtc/tests/fuzztests <- note the plural

Comment 13

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Byron, do the classes under mediapipeline need to be renamed to match transportbridge. I figure that now would be a good time, as we have almost all the work to move things done, and will be waiting until the end of soft freeze to land.

Comment 14

[Byron Campen [:bwc]]

I think let's leave the class names alone for now.