Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for getDisplayMedia audio capture #3173

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Added support for getDisplayMedia audio capture #3173

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4499f29
Added support for getDisplayMedia audio capture
rjnpnfigueiredo Feb 28, 2023
ac39e3a
Refactored getDisplayMedia audio into group
rjnpnfigueiredo Mar 4, 2023
17cf23e
Added back empty line at the end of file
rjnpnfigueiredo Mar 7, 2023
ccfc680
Improved coding style
rjnpnfigueiredo Mar 7, 2023
d5af913
Addressed coding style
rjnpnfigueiredo May 6, 2023
1b703bf
Merge pull request #1 from meetecho/master
rjnpnfigueiredo May 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 22 additions & 10 deletions html/janus.js
View file
Open in desktop
Expand Up @@ -2269,11 +2269,12 @@ function Janus(gatewayCallbacks) {
let groups = {};
for(let track of tracks) {
delete track.gumGroup;
if(!track.type || !['audio', 'video'].includes(track.type))
if(!track.type || !['audio', 'video', 'screen'].includes(track.type))
continue;
if(!track.capture || track.capture instanceof MediaStreamTrack)
continue;
let group = track.group ? track.group : 'default';
// Avoid assigning regular default group if it is a screen capture
let group = track.group ? track.group : track.type=='screen'?'sharescreendefault':'default';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put the second block of the ternary operator in brackets to avoid ambiguity, and space things accordingly to make it more readable, e.g.:

(track.type === 'screen' ? 'sharescreendefault' : 'default')

As a side note, I'm not fond sharescreendefault as a name, too long and overly verbose: maybe just screendefault? I was going to suggest updating the documentation in mainpage.dox accordingly, but I just noticed we don't have any text about group in there at all where we talk of tracks, so I can add something myself after this is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweaked code to better fit style as per comments.
Chained ternaries are not really readable but I don't think spinning it off into a function would fit into the current code style.
I'd say screendefault sounds good, lets keep that.

if(!groups[group])
groups[group] = {};
if(groups[group][track.type])
Expand All @@ -2284,11 +2285,13 @@ function Janus(gatewayCallbacks) {
let keys = Object.keys(groups);
for(let key of keys) {
let group = groups[key];
if(!group.audio || !group.video) {
if(!group.audio || (!group.video && !group.screen)) {
if(group.audio)
delete group.audio.gumGroup;
if(group.video)
delete group.video.gumGroup;
if(group.screen)
delete group.screen.gumGroup;
delete groups[key];
}
}
Expand Down Expand Up @@ -2369,7 +2372,7 @@ function Janus(gatewayCallbacks) {
await sender.replaceTrack(null);
} else if(track.capture) {
if(track.gumGroup && groups[track.gumGroup] && groups[track.gumGroup].stream) {
// We did a getUserMedia before already
// We did a getUserMedia/getDisplayMedia before already
let stream = groups[track.gumGroup].stream;
nt = (track.type === 'audio' ? stream.getAudioTracks()[0] : stream.getVideoTracks()[0]);
delete groups[track.gumGroup].stream;
Expand All @@ -2384,26 +2387,35 @@ function Janus(gatewayCallbacks) {
pluginHandle.consentDialog(true);
}
let constraints = Janus.trackConstraints(track), stream = null;
if(track.type === 'audio' || track.type === 'video') {
if(track.type === 'audio' || track.type === 'video' || track.type === 'screen') {
// Set if User Media should be used of if it should be Display Media
let captureUserMedia = track.type !== 'screen';
rjnpnfigueiredo marked this conversation as resolved.
Show resolved Hide resolved
rjnpnfigueiredo marked this conversation as resolved.
Show resolved Hide resolved
// Use getUserMedia: check if we need to group audio and video together
if(track.gumGroup) {
let otherType = (track.type === 'audio' ? 'video' : 'audio');
if(groups[track.gumGroup] && groups[track.gumGroup][otherType]) {
let otherTrack = groups[track.gumGroup][otherType];
if(!otherTrack && otherType === 'video') {
// If there other track is not video, then it is screen and should be a Display Media capture
rjnpnfigueiredo marked this conversation as resolved.
Show resolved Hide resolved
otherType = 'screen'
otherTrack = groups[track.gumGroup]['screen'];
captureUserMedia = false;
}
let otherConstraints = Janus.trackConstraints(otherTrack);
constraints[otherType] = otherConstraints[otherType];
}
}
stream = await navigator.mediaDevices.getUserMedia(constraints);
if(captureUserMedia) {
stream = await navigator.mediaDevices.getUserMedia(constraints);
} else {
stream = await navigator.mediaDevices.getDisplayMedia(constraints);
}
if(track.gumGroup && constraints.audio && constraints.video) {
// We just performed a grouped getUserMedia, keep track of the
// We just performed a grouped getUserMedia/getDisplayMedia, keep track of the
// stream so that we can immediately assign the track later
groups[track.gumGroup].stream = stream;
delete track.gumGroup;
}
} else {
// Use getDisplayMedia
stream = await navigator.mediaDevices.getDisplayMedia(constraints);
}
nt = (track.type === 'audio' ? stream.getAudioTracks()[0] : stream.getVideoTracks()[0]);
}
Expand Down
2 changes: 1 addition & 1 deletion html/screensharingtest.js
View file
Open in desktop
Expand Up @@ -594,4 +594,4 @@ function escapeXmlTags(value) {
escapedValue = escapedValue.replace(new RegExp('>', 'g'), '&gt');
return escapedValue;
}
}
}
rjnpnfigueiredo marked this conversation as resolved.
Show resolved Hide resolved