fix: export generateSidxKey, multiple audio/vtt playlists #123
Conversation
| @@ -3,6 +3,9 @@ import { findIndexes } from './utils/list'; | |||
| import { addSegmentsToPlaylist } from './segment/segmentBase'; | |||
| import { byteRangeToString } from './segment/urlType'; | |||
|
|
|||
| export const generateSidxKey = (sidx) => sidx && | |||
There was a problem hiding this comment.
We need this function in VHS, we re-implemented it there which was fairly pointless imo.
| @@ -40,25 +43,24 @@ const mergeDiscontiguousPlaylists = playlists => { | |||
| }); | |||
| }; | |||
|
|
|||
| const addSegmentInfoFromSidx = (playlists, sidxMapping = {}) => { | |||
| export const addSidxSegmentsToPlaylist = (playlist, sidxMapping) => { | |||
There was a problem hiding this comment.
Add Sidx segments to a single playlist. We used to do this only on an array of playlists, but it was weird because for audio/vtt playlists we needed to pass in a single playlist. The naming of these functions was also weird.
| a[label].playlists[0].attributes.BANDWIDTH > | ||
| playlist.attributes.bandwidth) { | ||
| return a; | ||
| if (!a[label]) { |
There was a problem hiding this comment.
This is the fix here, we should create the group, and add playlists to it. Not only create a group with one playlist ever.
| // skip if we already have subtitles | ||
| if (a[label]) { | ||
| return a; | ||
| if (!a[label]) { |
There was a problem hiding this comment.
same fix as audio playlists.
| @@ -237,6 +229,13 @@ export const formatVideoPlaylist = ({ attributes, segments, sidx }) => { | |||
| return playlist; | |||
| }; | |||
|
|
|||
| const videoOnly = ({ attributes }) => | |||
There was a problem hiding this comment.
We defined these in a function before, needlessly creating these functions on every mpd parse isn't something we should do.
| @@ -274,7 +266,7 @@ export const toM3u8 = (dashPlaylists, locations, sidxMapping = {}) => { | |||
| }, | |||
| uri: '', | |||
| duration, | |||
| playlists: addSegmentInfoFromSidx(videoPlaylists, sidxMapping) | |||
| playlists: addSidxSegmentsToPlaylists(videoPlaylists, sidxMapping) | |||
There was a problem hiding this comment.
new name, same function.
brandonocasey
force-pushed
the
fix/add-multiple-vtt-audio-playlist
branch
from
2109892 to
85d24be
Compare
| @@ -67,7 +67,7 @@ export const segmentsFromBase = (attributes) => { | |||
| * @param {Object} sidx the parsed sidx box | |||
| * @return {Object} the playlist object with the updated sidx information | |||
| */ | |||
| export const addSegmentsToPlaylist = (playlist, sidx, baseUrl) => { | |||
| export const addSidxSegmentsToPlaylist = (playlist, sidx, baseUrl) => { | |||
There was a problem hiding this comment.
Renamed this function, as it was named terribly. It only adds sidx segments to a playlist, and we have other functions that add segments from our dash manifest to a playlist object.
brandonocasey
force-pushed
the
fix/add-multiple-vtt-audio-playlist
branch
from
985ca65 to
7291421
Compare
brandonocasey
changed the title
brandonocasey
force-pushed
the
fix/add-multiple-vtt-audio-playlist
branch
from
7028b95 to
740f110
Compare
brandonocasey
force-pushed
the
fix/add-multiple-vtt-audio-playlist
branch
from
740f110 to
267dbcb
Compare
| @@ -1,10 +1,10 @@ | |||
| import { version } from '../package.json'; | |||
| import { toM3u8 } from './toM3u8'; | |||
| import { toM3u8, generateSidxKey } from './toM3u8'; | |||
There was a problem hiding this comment.
we duplicate this exact function in vhs right now. Seems better to have one source of truth.
| @@ -89,6 +91,11 @@ export const formatAudioPlaylist = ({ attributes, segments, sidx }) => { | |||
| playlist.sidx = sidx; | |||
| } | |||
|
|
|||
| if (isAudioOnly) { | |||
There was a problem hiding this comment.
if we are an audio only playlist, we still want subtitles and audio groups to work correctly.
Currently we drop multiple playlists for the same group, when we should allow multiple renditions for the same group. This allows that to happen.
Fixes: #93
Fixes: #105
Fixes: #77