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.

Sign up for GitHub

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

mimeTypeToVideoCodecString should not throw #1302

Merged
merged 2 commits into from
Oct 26, 2024
Merged

mimeTypeToVideoCodecString should not throw #1302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d41b9ec
mimeTypeToVideoCodecString should not throw
davidzhao Oct 25, 2024
aeb3d87
changeset
davidzhao Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/heavy-garlics-greet.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'livekit-client': patch
---

fix: mimeTypeToVideoCodecString should not throw
7 changes: 1 addition & 6 deletions src/room/track/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
type ScreenShareCaptureOptions,
type VideoCaptureOptions,
type VideoCodec,
videoCodecs,
} from './options';
import type { AudioTrack } from './types';

Expand Down Expand Up @@ -188,11 +187,7 @@ export function screenCaptureToDisplayMediaStreamOptions(
}

export function mimeTypeToVideoCodecString(mimeType: string) {
const codec = mimeType.split('/')[1].toLowerCase() as VideoCodec;
if (!videoCodecs.includes(codec)) {
throw Error(`Video codec not supported: ${codec}`);
}
return codec;
return mimeType.split('/')[1].toLowerCase() as VideoCodec;
Copy link
Contributor

@lukasIO lukasIO Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure whether forcing this cast is the right fix.
This incorrectly returns strings that are not part of the client defined VideoCodec type.

If a method is needed that just strips away video/ from the mimetype without throwing, I think that should return a string instead

Copy link
Member Author

Choose a reason for hiding this comment

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

that's a fair point. my main goal is to ensure it doesn't throwing just because there's a piece of data that it doesn't understand.

perhaps the more systematic question is how do we handle new video codecs that the server/browser might contain that our client isn't aware of. we'd have to be defensive about it

}

export function getTrackPublicationInfo<T extends TrackPublication>(
Expand Down
Loading