fix: support automatic configuration of audio and video only DRM sources #1090
Conversation
src/videojs-http-streaming.js
Outdated
| codecs.video.split(',').forEach(function(codec) { | ||
| codec = codec.trim(); | ||
| if (mainPlaylist && mainPlaylist.attributes && mainPlaylist.attributes.CODECS) { | ||
| const mainCodecs = mainPlaylist.attributes.CODECS; |
There was a problem hiding this comment.
perhaps we should use parseCodecs from vhs utils and loop through that. Should we make sure that we can support the codec here as well?
There was a problem hiding this comment.
Will switch to using parseCodecs. On support, since this should be called on createdsourcebuffers, I think we should be safe.
src/videojs-http-streaming.js
Outdated
| if (audioPlaylist && audioPlaylist.attributes && audioPlaylist.attributes.CODECS) { | ||
| codecs.audio = audioPlaylist.attributes.CODECS; | ||
| } | ||
|
|
||
| const videoContentType = codecs.video ? `video/mp4;codecs="${codecs.video}"` : null; |
There was a problem hiding this comment.
perhaps we should use the vhs utils getMimeForCodec function here.
| if (!codecs.audio && codecs.video && codecs.video.split(',').length > 1) { | ||
| codecs.video.split(',').forEach(function(codec) { | ||
| codec = codec.trim(); | ||
| if (mainPlaylist && mainPlaylist.attributes && mainPlaylist.attributes.CODECS) { |
There was a problem hiding this comment.
should we use src/util/codecsForPlaylist for this instead?
There was a problem hiding this comment.
Looking into it, there is one slight difference in that codecsForPlaylist would grab the default audio codec, whereas this should grab the currently used audio playlist's codec. They should, in theory, be the same (or at least compatible), so it may not be a big issue, but in practice who knows what people do. I can see either side though. What do you think?
There was a problem hiding this comment.
I think we might need to use codecsForPlaylist for the edge case described here: https://github.com/videojs/http-streaming/blob/main/src/util/codecs.js#L116
There was a problem hiding this comment.
That should be covered by using the audio playlist below, since that playlist will have the attributes on it and will overwrite any audio codecs (or overwrite a missing codec if the video rendition doesn't list audio). And in the event that a non default alternate audio playlist is selected, we'd want to use the attributes there over the default one used by codecsForPlaylist
Codecov Report
@@ Coverage Diff @@
## main #1090 +/- ##
==========================================
- Coverage 86.13% 86.13% -0.01%
==========================================
Files 38 38
Lines 8967 8966 -1
Branches 2020 2017 -3
==========================================
- Hits 7724 7723 -1
Misses 1243 1243
Continue to review full report at Codecov.
|
Description
To test, encrypted video creator can be used to create three streams with the following options:
Requirements Checklist