feat(screenshare): Audio only screenshare #8922
Conversation
andrei-gavrilescu
force-pushed
the
feat/audio-only-screenshare
branch
from
1bc1895
to
55150f3
Compare
There was a problem hiding this comment.
@andrei-gavrilescu This LGTM overall, left 2 minor comments.
| @@ -1660,6 +1664,23 @@ export default { | |||
| = this._turnScreenSharingOff.bind(this, didHaveVideo); | |||
|
|
|||
| const desktopVideoStream = desktopStreams.find(stream => stream.getType() === MEDIA_TYPE.VIDEO); | |||
| const dekstopAudioStream = desktopStreams.find(stream => stream.getType() === MEDIA_TYPE.AUDIO); | |||
|
|
|||
| if (dekstopAudioStream) { | |||
There was a problem hiding this comment.
Do we want to stop screenshare here only when audio-only screenshare is in progress ? If we are using the audio mixer effect and have both video and audio streams from the desktop tracks, any issues with audio track will result in the share being turned off. Is that how we want it to work ?
There was a problem hiding this comment.
Imo this behaviour is fine. If the user initiates a screen sharing session and goes through the trouble of ticking the Share audio box and there's a problem with that audio track at some point, then the behaviour wouldn't be as expected same as what would happen if something went wrong with the video track.
Of course this is just my point of view, if you feel strongly about it we can think of another way to approach this scenario.
| && <OverflowMenuItem | ||
| accessibilityLabel = { t('toolbar.accessibilityLabel.shareaudio') } | ||
| icon = { IconShareAudio } | ||
| key = 'shareaudio' |
There was a problem hiding this comment.
I think the button name "Share audio" is little confusing. Do you think Start Audio Share and Stop Audio Share make more sense here ?
There was a problem hiding this comment.
As it stands the button doesn't have a state it's just Share audio for both stop and start. We're planning on adding a "How to" dialog when the user clicks that button and add that state then. This is meant to be an initial iteration. But if you think it's something we must have for this PR, I can add that state now.
andrei-gavrilescu
force-pushed
the
feat/audio-only-screenshare
branch
from
5c5eb38
to
97bb3f0
Compare
No description provided.