A small cleanup in AudioVideoPlaybackBot #306
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…k for OnAudioMediaReceived.
pcdeadeasy
reviewed
Aug 13, 2020
Hmm we should ask @zihzhan what he thinks about that, since the OnDominantSpeakerChanged handler does things related to the CallHandler (e.g. it might end up unsubscribing from one person's video and subscribing to the dominant speaker's video instead). It seems appropriately scoped to the CallHandler class. If we were to move it to BotMediaStream then all it would do is raise an event that CallHandler would end up listening to & handling anyway. |
Identifier
added
the
review requested
pcdeadeasy
approved these changes
Aug 14, 2020
Samples/V1.0Samples/LocalMediaSamples/AudioVideoPlaybackBot/FrontEnd/Bot/BotMediaStream.cs
Show resolved
Hide resolved
zihzhan-msft
approved these changes
Aug 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this pull request I've moved the OnVideoMediaReceived function from CallHandler to BotMediaStream since BotMediaStream handles all other media related events. I also added a callback for OnAudioMediaReceived to the same class to make it easy for people to see where both Audio and Video can be handled.