Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Duration should work when video or audio is missing #348

Merged
merged 1 commit into from
Jul 14, 2015

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented Jul 13, 2015

If audio or video data is missing for an HLS stream, duration calculations should just use the info that is available. Fixes #341.

@gkatsev
Copy link
Member

gkatsev commented Jul 13, 2015

LGTM

strictEqual(player.hls.playlists.media().segments[0].maxAudioPts, undefined, 'max audio pts is undefined');
});

test('records PTS values for video-only segments', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

audio-only

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops

If audio or video data is missing for an HLS stream, duration calculations should just use the info that is available. Fixes videojs#341.
dmlap added a commit to dmlap/videojs-contrib-hls that referenced this pull request Jul 14, 2015
@dmlap dmlap merged commit b58a7fc into videojs:master Jul 14, 2015
@dmlap dmlap deleted the no-audio-is-ok branch July 14, 2015 14:47
@@ -91,7 +107,8 @@
// available PTS information
for (left = range.start; left < range.end; left++) {
segment = playlist.segments[left];
if (segment.minVideoPts !== undefined) {
if (segment.minVideoPts !== undefined ||
Copy link

Choose a reason for hiding this comment

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

if I add .hls.playlists.media().segments = new Array(1) I will get error TypeError: Cannot read property 'minVideoPts' of undefined

Copy link
Member Author

Choose a reason for hiding this comment

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

The entries of a media playlist should be segment objects-- [undefined] is not a legal value for that array. Side note: it would be better to file this sort of thing as an issue. Line comments might get overlooked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants