Move webrtc code out of media/webrtc and into dom/media/webrtc
Categories
(Core :: WebRTC, enhancement, P2)
Tracking
()
People
(Reporter: ng, Assigned: ng)
References
(Depends on 2 open bugs, Blocks 1 open bug)
Details
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•4 years ago
|
||
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•4 years ago
|
||
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.
Assignee | ||
Updated•4 years ago
|
Comment 4•4 years ago
|
||
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.
Assignee | ||
Comment 5•4 years ago
|
||
(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.
Assignee | ||
Comment 6•4 years ago
|
||
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•4 years ago
|
||
(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•4 years ago
|
||
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•4 years ago
|
||
(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.
Assignee | ||
Comment 10•4 years ago
|
||
- 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•4 years ago
|
||
(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 tocommon
?
Not sure yet. Maybe in an /impl directory with the pipeline stuff. Not super important for this move.
Comment 12•4 years ago
|
||
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
Assignee | ||
Comment 13•4 years ago
|
||
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•4 years ago
|
||
I think let's leave the class names alone for now.
Assignee | ||
Updated•3 years ago
|
Description
•