Skip to content

fix(parser): ensure dateRange uses startDate for condition#187

Open
pmendozav wants to merge 2 commits into
videojs:mainfrom
pmendozav:fix/dateRange-startDate-condition
Open

fix(parser): ensure dateRange uses startDate for condition#187
pmendozav wants to merge 2 commits into
videojs:mainfrom
pmendozav:fix/dateRange-startDate-condition

Conversation

@pmendozav

Copy link
Copy Markdown

related with issue #186

@ishish

ishish commented Jan 22, 2025

Copy link
Copy Markdown

Is there an update on this can we get it merged?

gkatsev
gkatsev previously approved these changes Jan 28, 2025
@gkatsev gkatsev dismissed their stale review January 28, 2025 20:31

not inherently wrong, but there's more to it

@adrums86 adrums86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for submitting a fix! I agree this could be handled better. I think we might just want to be careful about possibly creating out of spec dateRange tags so lets see if we can do this as close to spec as possible.

Comment thread src/parser.js
Triggers a warn event when an EXT-X-DATERANGE tag includes both DURATION and END-DATE but lacks the required START-DATE. Ensures better HLS spec compliance.

@gkatsev gkatsev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tests pass locally

@adrums86 adrums86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit: Obviously some karma issues that need fixed in the repo. Seems to pass locally, so its unrelated.

@pmendozav

Copy link
Copy Markdown
Author

Any update here? Can the change be merged?

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.

4 participants