-
Notifications
You must be signed in to change notification settings - Fork 25
Fix audio/video sync: publish audio from camera when enabled and and … #119
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.
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
base: main
Are you sure you want to change the base?
Conversation
…made some changes in icon padding and text
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@konsalex, can you please review my PR? Thank you. |
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.
This is not really the direction we want to take. What we want to achieve here is the following:
- When the camera window is not open (which means no camera tracks exist) keep the implementation as is.
- When the camera window is open, disable
ListeToRemoteAudio
from call-center and enable it in the camera window.
</div> | ||
</div> | ||
<ListenToRemoteAudio /> | ||
{/* <ListenToRemoteAudio /> */} diable to solve the issue related to sync audio playback |
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.
We shouldn't commit commented out lines.
…state, remove commented code
} | ||
|
||
function CameraIcon() { | ||
function CameraIcon({ micEnabled }: { micEnabled: boolean }) { |
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.
No need for this. We don't want to change the way we capture the audio and camera. We only want to change the way the audio is played.
</div> | ||
<ListenToRemoteAudio /> | ||
{/* Render remote audio only if camera is NOT enabled */} | ||
{!callTokens?.hasCameraEnabled && <ListenToRemoteAudio />} diable to solve the issue related to sync audio playback |
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.
This is not 100% correct, hasCameraEnabled is set when the current user is publishing their local camera. We want to disable this when the camera window is open, which could also happen if another participant is publishing their camera. We could update CallState
to keep track of the camera window is opened or not.
@konsalex, can you review it? |
@Yasir761 I don't think all of my comments have been addressed. You are still using |
…made some changes in icon padding and text, this PR is related to issue #102 and #115