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

Jump to bottom

Fixed a bug regarding VOD videos #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed a bug regarding VOD videos #17

wants to merge 1 commit into from

Conversation

Kasher
Copy link

@Kasher Kasher commented Aug 14, 2016

No description provided.

@tjenkinson
Copy link

If there's the end list tag it still can be an EVENT stream, which isn't VOD.

On 14 Aug 2016, at 09:54, Kasher notifications@github.com wrote:

You can view, comment on, or merge this pull request online at:

#17

Commit Summary

Fixed a bug regarding VOD videos
File Changes

M parser.js (13)
Patch Links:

https://github.com/tedconf/node-m3u8/pull/17.patch
https://github.com/tedconf/node-m3u8/pull/17.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@Kasher
Copy link
Author

Kasher commented Aug 21, 2016

I saw that when threre is an end list tag, players don't treat the video as a live stream, but as a VOD stram. When this tag is missing, players might mistakenly treat videos as live streams.

If you look at the output of this NPM (i.e. after piping a m3u8 file and just stringify the output) sometimes it omits this end list tag, hence players play the video as if it was a live stream (even when the video is a VOD video).
I have tried many videos, and couldn't find any problem after making this change.
Do you have another idea of how fixing it?

@tjenkinson
Copy link

It should be classed as VOD if the X-PLAYLIST-TYPE header is VOD and I think that's the only case. If the tag isn't there then it should be treated as a live stream and sone players might decide to show the scrub bar if there's enough content that makes sense for a sliding window (dvr).

Have a read of https://developer.apple.com/library/ios/technotes/tn2288/_index.html :)

@Kasher
Copy link
Author

Kasher commented Aug 21, 2016

I agree.
My solution is not good enough, as it is just a heuristic, that was
written in order to fix a bug. That's totally not the right way to fix it,
as this is a patch that has some bad consequences for some cases.
With this being said, do you agree that there is a bug here?
I'll try to elaborate a bit: Lets go to twitch.tv and take a VOD video,
for instance this one:
https://www.twitch.tv/quin69/v/84093792
If we'll take the manifest file for this video, and we'll run it through
this NPM and print the output (without even modifying anything), we'll get
a different output than the original m3u8 file (the output will lack the
end list tag).
Hence, if we take the original m3u8 file and the output's m3u8 file and
give them both to the same player, we'll get different behaviours.
Is this being done by purpose? If so - that's my bad, I probably chose
the wrong package..

Anyhow, thanks a lot for your quick responses!!! They are really
appreciated :)

On Aug 21, 2016 20:29, "Tom Jenkinson" notifications@github.com wrote:

It should be classed as VOD if the X-PLAYLIST-TYPE header is VOD and I
think that's the only case. If the tag isn't there then it should be
treated as a live stream and sone players might decide to show the scrub
bar if there's enough content that makes sense for a sliding window (dvr).

Have a read of https://developer.apple.com/library/ios/technotes/tn2288/_
index.html :)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#17 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AE6w8wYMXF6qI06SM4N3AHTHleYTkhX9ks5qiIrjgaJpZM4Jj2t7
.

@tjenkinson
Copy link

tjenkinson commented Aug 21, 2016 •
edited

I just checked an example from twitch

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:2
#ID3-EQUIV-TDTG:2016-08-17T09:28:40
#EXT-X-PLAYLIST-TYPE:EVENT

#EXT-X-TWITCH-ELAPSED-SECS:0.0
#EXT-X-TWITCH-TOTAL-SECS:14947.112
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=0&end_offset=116371
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=116372&end_offset=283503
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=283504&end_offset=485039
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=485040&end_offset=677175
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=677176&end_offset=864047
#EXTINF:2.000,
...
#EXTINF:2.000,
index-0000007469-kXHB.ts?start_offset=5509340&end_offset=5713695
#EXTINF:2.000,
index-0000007469-kXHB.ts?start_offset=5713696&end_offset=5918051
#EXTINF:2.000,
index-0000007473-1uQC.ts?start_offset=0&end_offset=205295
#EXTINF:2.000,
index-0000007473-1uQC.ts?start_offset=205296&end_offset=409087
#EXTINF:2.000,
index-0000007473-1uQC.ts?start_offset=409088&end_offset=616639
#EXTINF:1.112,
index-0000007473-1uQC.ts?start_offset=616640&end_offset=741659
#EXT-X-ENDLIST

This is of type EVENT so that output should have type event as well and the end list should be included, so yes there is a bug here.

Does the output contain the EVENT type or is that missing as well?

Could you paste the output here as well?

@Kasher
Copy link
Author

Kasher commented Aug 28, 2016

Hey, sorry for the late response.
I used the following code, that just uses this package (pipes a m3u8 file through the parser), modifies nothing, and just prints the code in the end:

var m3u8 = require('m3u8');
var fs   = require('fs');

var parser = m3u8.createStream();
var file   = fs.createReadStream('~/twitch.m3u8');
file.pipe(parser);

parser.on('item', function(item) {
  // emits PlaylistItem, MediaItem, StreamItem, and IframeStreamItem
});
parser.on('m3u', function(m3u) {
  // fully parsed m3u file
  fs.writeFileSync("~/twitch2.m3u8", m3u.toString());
});

The input file:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:2
#ID3-EQUIV-TDTG:2016-08-17T09:28:40
#EXT-X-PLAYLIST-TYPE:EVENT

#EXT-X-TWITCH-ELAPSED-SECS:0.0
#EXT-X-TWITCH-TOTAL-SECS:14947.112
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=0&end_offset=116371
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=116372&end_offset=283503
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=283504&end_offset=485039
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=485040&end_offset=677175
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=677176&end_offset=864047
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=864048&end_offset=1050731
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1050732&end_offset=1244371
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1244372&end_offset=1419587
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1419588&end_offset=1612851
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1612852&end_offset=1794271
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1794272&end_offset=1988475
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=1988476&end_offset=2183619
#EXTINF:2.000,
index-0000000029-ogjS.ts?start_offset=2183620&end_offset=2342667
#EXTINF:2.000,
.
.
.
index-0000007473-1uQC.ts?start_offset=205296&end_offset=409087
#EXTINF:2.000,
index-0000007473-1uQC.ts?start_offset=409088&end_offset=616639
#EXTINF:1.112,
index-0000007473-1uQC.ts?start_offset=616640&end_offset=741659
#EXT-X-ENDLIST

And the output file:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:2
#ID3-EQUIV-TDTG:2016-08-17T09:28:40
#EXT-X-PLAYLIST-TYPE:EVENT
#EXT-X-TWITCH-ELAPSED-SECS:0.0
#EXT-X-TWITCH-TOTAL-SECS:14947.112
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=0&end_offset=116371
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=116372&end_offset=283503
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=283504&end_offset=485039
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=485040&end_offset=677175
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=677176&end_offset=864047
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=864048&end_offset=1050731
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1050732&end_offset=1244371
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1244372&end_offset=1419587
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1419588&end_offset=1612851
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1612852&end_offset=1794271
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1794272&end_offset=1988475
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=1988476&end_offset=2183619
#EXTINF:2.0000,
index-0000000029-ogjS.ts?start_offset=2183620&end_offset=2342667
#EXTINF:2.0000,
.
.
.
index-0000007473-1uQC.ts?start_offset=205296&end_offset=409087
#EXTINF:2.0000,
index-0000007473-1uQC.ts?start_offset=409088&end_offset=616639
#EXTINF:1.1120,
index-0000007473-1uQC.ts?start_offset=616640&end_offset=741659

(I removed a piece of the playlist, since it was too long...).
As you can see, in the output file the #EXT-X-ENDLIST tag is missing. The EVENT type tag is
there though.

What's the best way to fix it, now that we agree there is a bug?

Thanks!!!

@tjenkinson
Copy link

I might be wrong but I think there should be a new endList property here

Then here:

if (this.get('endList')) {
  output.push('#EXT-X-ENDLIST');
}

and here should be:

if (['', '#EXT-X-ENDLIST'].indexOf(line) > -1) {
  if (line === '#EXT-X-ENDLIST' && this.currentItem) {
     this.currentItem.set('endList', true);
  }
  return true;
}

Then to include the tag in the output here should be:

if (this.get('playlistType') === 'VOD' || this.get('endList')) {
  output.push('#EXT-X-ENDLIST');
}

and some tests should probably be updated for the new endList :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Kasher @tjenkinson