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.

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

core: use getTrack().some() to tell if screenshare has audio and use fallback id #186

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

nandito
Copy link
Contributor

@nandito nandito commented Apr 10, 2024

Description

Summary:

  • Replace the MediaStream.getAudioTracks().length > 0 with a MediaStream.getTrack().some() function, as some WebRTC implementation does not have .getAudioTracks() implemented.
  • Use fallback ids if the MediaStream has no id for screenshares: pres-<clientId> for remote participants and local-screenshare for local participant.

Related Issue:

Testing

  1. Start screenshare with and without audio
  2. They should work as before

Screenshots/GIFs (if applicable)

Checklist

  • My code follows the project's coding standards.
  • Prefixed the PR title and commit messages with the service or package name
  • I have written unit tests (if applicable).
  • I have updated the documentation (if applicable).
  • By submitting this pull request, I confirm that my contribution is made
    under the terms of the MIT license.

Additional Information

Replace the `MediaStream.getAudioTracks().length > 0` with a
`MediaStream.getTrack().some()` function, as some WebRTC implementation
does not have `.getAudioTracks()` implemented.
@nandito nandito requested a review from a team April 10, 2024 13:51
@nandito nandito self-assigned this Apr 10, 2024
Copy link

changeset-bot bot commented Apr 10, 2024

🦋 Changeset detected

Latest commit: 8f6987d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@whereby.com/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nandito nandito requested a review from kevinwhereby April 10, 2024 14:02
@nandito
Copy link
Contributor Author

nandito commented Apr 11, 2024

/canary

Copy link
Contributor

🚀 The canary release has been published to npm.

You can test the release by installing the newly published version:

yarn add @whereby.com/core@0.0.0-canary-20240411085142

Copy link
Contributor

@kevinwhereby kevinwhereby left a comment

Choose a reason for hiding this comment

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

🚀

@nandito
Copy link
Contributor Author

nandito commented Apr 11, 2024

/canary

Copy link
Contributor

🚀 The canary release has been published to npm.

You can test the release by installing the newly published version:

yarn add @whereby.com/core@0.0.0-canary-20240411113310

@nandito nandito changed the title core: use getTrack().some() to tell if screenshare has audio core: use getTrack().some() to tell if screenshare has audio and use fallback id Apr 11, 2024
We don't really use the `id` field on the screenshare MediaStream,
therefore it's `undefined`. Let's use the `pres-` prefixed clientId for
remote participants' screenshares and `local-screenshares` for local
participant's screenshare in this case, like we do in PWA.
@nandito nandito force-pushed the nandor/cob-617-make-screenshare-captions-work branch from d5b72c1 to 8f6987d Compare April 11, 2024 12:21
@nandito
Copy link
Contributor Author

nandito commented Apr 11, 2024

/canary

Copy link
Contributor

🚀 The canary release has been published to npm.

You can test the release by installing the newly published version:

yarn add @whereby.com/core@0.0.0-canary-20240411122200

@nandito nandito merged commit 78befe1 into main Apr 11, 2024
3 checks passed
@nandito nandito deleted the nandor/cob-617-make-screenshare-captions-work branch April 11, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants