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.

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

fix: merge dateRange tags with same IDs and no conflicting attributes #175

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

harisha-swaminathan
Copy link
Contributor

@harisha-swaminathan harisha-swaminathan commented Aug 4, 2023

In the PR that added the dateRange feature, the specification
If a Playlist contains two EXT-X-DATERANGE tags with the same ID attribute value, then any AttributeName that appears in both tags MUST have the same AttributeValue.
was misinterpreted as EXT-X-DATERANGE tags with the same ID in a playlist must have the same attributes and same attribute values.

This PR

  • fixes that by comparing attributes of dateRange tags with the same ID only if both the tags contain the attribute
  • merges dateRange tags with same IDs if no attribute has conflicting values (This will be useful when parsing SCTE35-IN/OUT pairs)

@harisha-swaminathan harisha-swaminathan changed the title fix: merge dateRange tags with same Id and no conflicting attributes fix: merge dateRange tags with same IDs and no conflicting attributes Aug 4, 2023
@@ -685,21 +685,28 @@ export default class Parser extends Stream {
}
if (dateRange.duration && dateRange.endDate) {
const startDate = dateRange.startDate;
const newDateInSeconds = startDate.setSeconds(startDate.getSeconds() + dateRange.duration);
const newDateInSeconds = startDate.getTime() + (dateRange.duration * 1000);
Copy link
Contributor Author

@harisha-swaminathan harisha-swaminathan Aug 4, 2023

Choose a reason for hiding this comment

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

Don't know why this was startDate.setSeconds(startDate.getSeconds() + dateRange.duration);
I think the setSeconds was a typo and was meant to be `getSeconds()'.

For more context on this block of code, the spec mentions
If a Date Range contains both a DURATION attribute and an END-DATE attribute, the value of the END-DATE attribute MUST be equal to the value of the START-DATE attribute plus the value of the DURATION attribute.
https://datatracker.ietf.org/doc/html/draft-pantos-http-live-streaming-23#section-4.3.2.7

Copy link
Contributor

@adrums86 adrums86 left a comment

Choose a reason for hiding this comment

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

Looks good, nice catch on the setSeconds 😄

@harisha-swaminathan harisha-swaminathan merged commit 73d934c into main Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants