-
Notifications
You must be signed in to change notification settings - Fork 3
[RTC-32] Fetch actual bitrate of a variant #21
Changes from 3 commits
6973c3a
0249e20
b7d82f6
1e37972
ed3079e
7a30bf2
65e1e3e
bc4b32e
0c7bbf6
9118e79
41c315b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -636,6 +636,8 @@ export class MembraneWebRTC { | |
simulcastConfig: SimulcastConfig = { enabled: false, active_encodings: [] }, | ||
maxBandwidth: TrackBandwidthLimit = 0 // unlimited bandwidth | ||
): string { | ||
if (!simulcastConfig.enabled && !(typeof maxBandwidth === "number")) | ||
throw "Invalid type of `maxBandwidth` argument for a non-simulcast track, expected: number"; | ||
if (this.getPeerId() === "") | ||
throw "Cannot add tracks before being accepted by the server"; | ||
const trackId = this.getTrackId(uuidv4()); | ||
|
@@ -882,6 +884,7 @@ export class MembraneWebRTC { | |
trackId: string, | ||
bandwidth: BandwidthLimit | ||
): Promise<boolean> { | ||
// FIXME: maxBandwidth in TrackContext is not updated | ||
const trackContext = this.localTrackIdToTrack.get(trackId); | ||
|
||
if (!trackContext) { | ||
|
@@ -894,6 +897,7 @@ export class MembraneWebRTC { | |
if (parameters.encodings.length === 0) { | ||
parameters.encodings = [{}]; | ||
} else { | ||
// TODO: sent MediaEvent with updated variant bitrates | ||
this.applyBandwidthLimitation(parameters.encodings, bandwidth); | ||
} | ||
|
||
|
@@ -934,6 +938,14 @@ export class MembraneWebRTC { | |
delete encoding.maxBitrate; | ||
} else { | ||
encoding.maxBitrate = bandwidth * 1024; | ||
let mediaEvent = generateCustomEvent({ | ||
type: "setTrackVariantBitrates", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would favor |
||
data: { | ||
trackId: trackId, | ||
variantBitrates: { [rid]: bandwidth }, | ||
}, | ||
}); | ||
this.sendMediaEvent(mediaEvent); | ||
} | ||
|
||
return sender | ||
|
@@ -1150,23 +1162,6 @@ export class MembraneWebRTC { | |
this.sendMediaEvent(mediaEvent); | ||
}; | ||
|
||
private getMidToTrackId = () => { | ||
const localTrackMidToTrackId = {} as any; | ||
|
||
if (!this.connection) return null; | ||
this.connection.getTransceivers().forEach((transceiver) => { | ||
const localTrackId = transceiver.sender.track?.id; | ||
const mid = transceiver.mid; | ||
if (localTrackId && mid) { | ||
const trackContext = Array.from(this.localTrackIdToTrack.values()).find( | ||
(trackContext) => trackContext!.track!!.id === localTrackId | ||
)!; | ||
localTrackMidToTrackId[mid] = trackContext.trackId; | ||
} | ||
}); | ||
return localTrackMidToTrackId; | ||
}; | ||
|
||
/** | ||
* Leaves the room. This function should be called when user leaves the room | ||
* in a clean way e.g. by clicking a dedicated, custom button `disconnect`. | ||
|
@@ -1252,8 +1247,7 @@ export class MembraneWebRTC { | |
type: "sdpOffer", | ||
data: { | ||
sdpOffer: offer, | ||
trackIdToTrackMetadata: this.getTrackIdToMetadata(), | ||
midToTrackId: this.getMidToTrackId(), | ||
trackIdToTrackInfo: this.getTrackIdToTrackInfo(), | ||
}, | ||
}); | ||
this.sendMediaEvent(mediaEvent); | ||
|
@@ -1262,14 +1256,42 @@ export class MembraneWebRTC { | |
} | ||
} | ||
|
||
private getTrackIdToMetadata = () => { | ||
const trackIdToMetadata = {} as any; | ||
private getTrackIdToTrackInfo = () => { | ||
const trackIdToMid = this.getTrackIdToMid(); | ||
|
||
const tracksInfo = {} as any; | ||
Array.from(this.localPeer.trackIdToMetadata.entries()).forEach( | ||
([trackId, metadata]) => { | ||
trackIdToMetadata[trackId] = metadata; | ||
const maxBandwidth = | ||
this.localTrackIdToTrack.get(trackId)!.maxBandwidth; | ||
tracksInfo[trackId] = { | ||
trackMetadata: metadata, | ||
maxBandwidth: | ||
maxBandwidth instanceof Map | ||
? Object.fromEntries(maxBandwidth) | ||
: maxBandwidth, | ||
mid: trackIdToMid ? trackIdToMid.get(trackId!) : null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should allow for sending |
||
}; | ||
} | ||
); | ||
return trackIdToMetadata; | ||
return tracksInfo; | ||
}; | ||
|
||
private getTrackIdToMid = () => { | ||
LVala marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const localTrackMidToTrackId = new Map<string, string>(); | ||
|
||
if (!this.connection) return null; | ||
this.connection.getTransceivers().forEach((transceiver) => { | ||
const localTrackId = transceiver.sender.track?.id; | ||
const mid = transceiver.mid; | ||
if (localTrackId && mid) { | ||
const trackContext = Array.from(this.localTrackIdToTrack.values()).find( | ||
(trackContext) => trackContext!.track!!.id === localTrackId | ||
)!; | ||
localTrackMidToTrackId.set(trackContext.trackId, mid); | ||
} | ||
}); | ||
return localTrackMidToTrackId; | ||
}; | ||
|
||
private checkIfTrackBelongToPeer = (trackId: string, peer: Peer) => | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,8 @@ | |
"moduleResolution": "node", | ||
"outDir": "dist/", | ||
"strict": true, | ||
"target": "es2018" | ||
"target": "es2018", | ||
"lib": ["dom", "es6", "es2019"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added that to be able to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Object.fromEntries was added in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This SO question misled me a bit (the "nothing imports |
||
}, | ||
"include": ["src/**/*"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending
TrackVariantBitrates
MediaEvent here would be quite troublesome (need to split the bandwidth before sending bitrates for each of the tracks), so I decided to leave it to the bigger refactor that we mentioned.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acctually, would it be hard to address this issue in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I did, but I would't call the code clean, the ambiguity of how track variant bandwidth is kept (sometimes as an aggregated value, sometimes bitrate per variant) and that the function responsible for splitting bitrate is very deep on the call stack make it hard to do it nicely without bigger refactor (which, as far as I understand, we don't want to do in this PR).