mimeTypeToVideoCodecString should not throw #1302
Conversation
The current implementation is not resilient to codecs changes. If a new codec is introduced, previously working code would start failing.
🦋 Changeset detectedLatest commit: aeb3d87 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
size-limit report 📦
|
| throw Error(`Video codec not supported: ${codec}`); | ||
| } | ||
| return codec; | ||
| return mimeType.split('/')[1].toLowerCase() as VideoCodec; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
The current implementation is not resilient to codecs changes. If a new codec is introduced, previously working code would start failing.