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

Improve handling of incompatible published codecs #911

Merged
merged 5 commits into from
Oct 27, 2023
Merged

Conversation

davidzhao
Copy link
Member

@davidzhao davidzhao commented Oct 26, 2023

When the client publishes a codec that the server doesn't support (or if server determines the client isn't compatible with it), we would reject those track publications.

However, the better thing to do from a user experience standpoint is to publish with another compatible codec. This compatibility layer works with server v1.5.1+

This change will use the preferred codec as returned in TrackPublishedResponse. Client will update its state accordingly.

Tested: disabling VP8 and publishing VP8, H.264, and AV1, verified backup is chosen and publishing at expected bitrates.

When the client publishes a codec that the server doesn't support (or if
server determines the client isn't compatible with it), we would reject
those track publications.

However, the better thing to do from a user experience standpoint is to
publish with another compatible codec. This PR requires v1.5.1
@changeset-bot
Copy link

changeset-bot bot commented Oct 26, 2023

🦋 Changeset detected

Latest commit: 11da5e5

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

This PR includes changesets to release 1 package
Name Type
livekit-client 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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 27, 2023

size-limit report 📦

Path Size
dist/livekit-client.esm.mjs 76.98 KB (-0.15% 🔽)
dist/livekit-client.umd.js 82.49 KB (-0.2% 🔽)

Copy link
Contributor

@cnderrauber cnderrauber left a comment

Choose a reason for hiding this comment

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

looks good!

Copy link
Contributor

@lukasIO lukasIO left a comment

Choose a reason for hiding this comment

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

looks good to me!

log.debug(
`primary codec ${opts.videoCodec} not supported, fallback to ${backupCodec.codec}`,
if (primaryCodecMime && track.kind === Track.Kind.Video) {
const updatedCodec = primaryCodecMime.replace(/video\//y, '').toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as a suggestion, we have a mimeTypeToVideoCodecString util function as part of e2ee, maybe makes sense to re-use here?

Copy link
Member Author

Choose a reason for hiding this comment

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

awesome, this function is perfect. I've moved it to track/utils since it's now used outside of e2ee

@davidzhao davidzhao merged commit 4fb5a88 into main Oct 27, 2023
2 checks passed
@davidzhao davidzhao deleted the dz-improve-ms branch October 27, 2023 16:27
@github-actions github-actions bot mentioned this pull request Oct 27, 2023
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.

3 participants