8000 No longer required a segment to be mark as a discontinuity in order include program date time. by cameronmaske · Pull Request #115 · globocom/m3u8 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

No longer required a segment to be mark as a discontinuity in order include program date time. #115

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
Closed
wants to merge 1 commit into from
Closed

No longer required a segment to be mark as a discontinuity in order include program date time. #115

wants to merge 1 commit into from

Conversation

cameronmaske
Copy link

Hey there,
I'm not sure if this is a bug, or part of the HLS spec requirement (I tried googling around, but didn't come across anything relevant).

I came across this manipulating some m3u8 which have program dates for each segments, e.g.
Here is an example of the loading and dumping, that cause the #EXT-X-PROGRAM-DATE-TIME to be wiped on segments.

Input (original)

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:7
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-PLAYLIST-TYPE:EVENT
#EXTINF:6.473522,
#EXT-X-PROGRAM-DATE-TIME:2017-11-24T12:43:39.000+0000
003.ts
#EXTINF:5.866667,
#EXT-X-PROGRAM-DATE-TIME:2017-11-24T12:43:45.474+0000
004.ts
#EXT-X-ENDLIST

Output (output dumped)

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:7
#EXT-X-PROGRAM-DATE-TIME:2017-11-24T12:43:39+00:00
#EXT-X-PLAYLIST-TYPE:EVENT
#EXTINF:6.473522,
003.ts
#EXTINF:5.866667,
004.ts
#EXT-X-ENDLIST

Thoughts?

@leandromoreira
Copy link
Contributor
leandromoreira commented Nov 24, 2017 •
edited
Loading

Looking at the specs:

The EXT-X-PROGRAM-DATE-TIME tag associates the first sample of a Media Segment with an absolute date and/or time.

I think we can safely assume that is part of the hls spec.

@cameronmaske
Copy link
Author

@leandromoreira Sorry, my comment was confusing. I meant, specwise, if EXT-X-PROGRAM-DATE-TIME can only be used in segments, if used in conjunction with EXT-X-DISCONTINUITY and can, or if can stand alone.

@leandromoreira
Copy link
Contributor
leandromoreira commented Nov 27, 2017 •
edited
Loading

When I read the spec my understanding was: "We should apply the tag EXT-X-PROGRAM-DATE-TIME only for the first media segment or when a EXT-X-DISCONTINUITY happens".

If that's the case then your input example is wrong. ;/ I also take a look at some HLS generate by elemental, woowza and ffmpeg and notice that they all respect this rule.

IE: https://video-dev.github.io/streams/x36xhzz/x36xhzz.m3u8

@igorsobreira
Copy link
Contributor

I'm having the same issue, loading a m3u8 that contains #EXT-X-PROGRAM-DATE-TIME but not EXT-X-DISCONTINUITY. Then dumping will strip out the #EXT-X-PROGRAM-DATE-TIME.

The spec says:

The server MAY associate an absolute date and time with a Media
Segment by applying an EXT-X-PROGRAM-DATE-TIME tag to it. This
defines an informative mapping of the (wall-clock) date and time
specified by the tag to the first media timestamp in the segment,
which may be used as a basis for seeking, for display, or for other
purposes. If a server provides this mapping, it SHOULD apply an
EXT-X-PROGRAM-DATE-TIME tag to every segment that has an
EXT-X-DISCONTINUITY tag applied to it.
https://tools.ietf.org/html/draft-pantos-http-live-streaming-23#section-6.2.1

From my understanding, you can provide EXT-X-PROGRAM-DATE-TIME for segments to help seeking, display etc. But if you do then you should also include EXT-X-PROGRAM-DATE-TIME for every segment that has EXT-X-DISCONTINUITY

My manifest is coming from AWS MediaPackage

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:6
#EXT-X-PLAYLIST-TYPE:EVENT
#EXT-X-MEDIA-SEQUENCE:50
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:00.000Z
#EXTINF:6.000,
manifest_1_50.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:06.000Z
#EXTINF:6.000,
manifest_1_51.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:12.000Z
#EXTINF:6.000,
manifest_1_52.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:18.000Z
#EXTINF:6.000,
manifest_1_53.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:24.000Z
#EXTINF:6.000,
manifest_1_54.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:30.000Z
#EXTINF:6.000,
manifest_1_55.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:36.000Z
#EXTINF:6.000,
manifest_1_56.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:42.000Z
#EXTINF:6.000,
manifest_1_57.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:48.000Z
#EXTINF:6.000,
manifest_1_58.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:05:54.000Z
#EXTINF:6.000,
manifest_1_59.ts?m=1559946393
#EXT-X-PROGRAM-DATE-TIME:2019-06-10T00:06:00.000Z
#EXTINF:6.000,
manifest_1_60.ts?m=1559946393
#EXT-X-ENDLIST

I'm 👍 on this PR

@igorsobreira
Copy link
Contributor

Since the PR doesn't have tests I forked and will submit a new one with tests. Turns out this change breaks existing behavior because our Segment always has program_date_time field set, even when #EXT-X-PROGRAM-DATE-TIME only exists in the manifest for the first segment

The change as suggested in this PR will break our current dumps() because it will add #EXT-X-PROGRAM-DATE-TIME for all segments, even those that did not have when we loaded

I think the ideal solution would be to only set Segment.program_date_time if the segment actually has #EXT-X-PROGRAM-DATE-TIME. And maybe provide another field that always have a calculated datetime value, based on previous segments (which is the current behavior of program_date_time). But I don't want to break backwards compatibility.

@leandromoreira thoughts on this?

@leandromoreira
Copy link
Contributor
leandromoreira commented Jun 18, 2019 •
edited
Loading

I think the ideal solution would be to only set Segment.program_date_time if the segment actually has #EXT-X-PROGRAM-DATE-TIME.

I'm ok with this thus the loaded model will be the same when dumped! We should avoid to superficially add stuffs at the dump method or cutting out as the current state of the lib. Btw, nice to see you back 😄

igorsobreira added a commit to igorsobreira/m3u8 that referenced this pull request Jun 19, 2019
@igorsobreira
dumps() include EXT-X-PROGRAM-DATE-TIME consitent with loads()
7c0a498 
on Segment.dumps() instead of only writing EXT-X-PROGRAM-DATE-TIME
when EXT-X-DISCONTINUITY we always write when it was provided when
we parsed.

`Segment.program_date_time` is set only when EXT-X-PROGRAM-DATE-TIME is
present for that segment
Segment.current_program_date_time` is set when EXT-X-PROGRAM-DATE-TIME
is set for this segment or any previous ones

See discussion on globocom#115
@igorsobreira igorsobreira mentioned this pull request Jun 19, 2019
Merged
@igorsobreira
Copy link
Contributor

I'm ok with this thus the loaded model will be the same when dumped! We should avoid to superficially add stuffs at the dump method or cutting out as the current state of the lib. Btw, nice to see you back 😄

Thanks, great to be back working with video :-). Pretty cool that this project is still active and well maintained!

I created the pull request #142, added a new field to Segment called current_program_date_time, (not sold on the same). Let me know what you guys think

@igorsobreira
Copy link
Contributor

I believe this one is solved now that #142 was merged

ActuallyAdasi pushed a commit to ActuallyAdasi/m3u8 that referenced this pull request Oct 12, 2020
@igorsobreira @ActuallyAdasi
dumps() include EXT-X-PROGRAM-DATE-TIME consitent with loads()
7672e45 
on Segment.dumps() instead of only writing EXT-X-PROGRAM-DATE-TIME
when EXT-X-DISCONTINUITY we always write when it was provided when
we parsed.

`Segment.program_date_time` is set only when EXT-X-PROGRAM-DATE-TIME is
present for that segment
Segment.current_program_date_time` is set when EXT-X-PROGRAM-DATE-TIME
is set for this segment or any previous ones

See discussion on globocom#115
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.
4 participants
@cameronmaske @leandromoreira @igorsobreira @mauricioabreu
0