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

feat(DASH): Add support for service descriptions #5394

Merged
merged 9 commits into from
Jul 5, 2023

Conversation

adgllorente
Copy link
Contributor

Changes

  • Parse ServiceDescription from Manifest
  • Use ServiceDescription for live syncrhonization

Doubts

  • I have added support for just one ServiceDescription. Low Latency spec seems that it supports more than one. If this support is mandatory, how can I know which ServiceDescription should I use?
  • I am executing onTimeUpdate when this.manifest_.serviceDescription OR this.config_.streaming.liveSync, should it be an AND?

@adgllorente adgllorente changed the title Service descriptions feat: Add support for service descriptions from MPD Jul 4, 2023
@avelad avelad added type: enhancement New feature or request priority: P3 Useful but not urgent labels Jul 4, 2023
@avelad avelad added this to the v4.4 milestone Jul 4, 2023
lib/player.js Outdated Show resolved Hide resolved
@avelad
Copy link
Collaborator

avelad commented Jul 4, 2023

@adgllorente

Doubts

I have added support for just one ServiceDescription. Low Latency spec seems that it supports more than one. If this support is mandatory, how can I know which ServiceDescription should I use?

Let's just take the first one, it's the same as we do with the trickplay track.

I am executing onTimeUpdate when this.manifest_.serviceDescription OR this.config_.streaming.liveSync, should it be an AND?

We don't want to break the old configuration, so OR is perfect in this case.

lib/player.js Outdated Show resolved Hide resolved
@adgllorente
Copy link
Contributor Author

adgllorente commented Jul 4, 2023

@avelad

I have parsed many values from Latency and PlaybackRate but we are using just 2 of them, do you agree in removing others?

@avelad
Copy link
Collaborator

avelad commented Jul 4, 2023

@avelad

I have parsed many values from Latency and PlaybackRate but we are using just 2 of them, do you agree in removing others?

In the context of this PR, yes.

@avelad
Copy link
Collaborator

avelad commented Jul 4, 2023

@adgllorente I forgot to comment before, add a parsing test of these new parameters in dash_parser_manifest_unit.js

@avelad avelad added the component: DASH The issue involves the MPEG DASH manifest format label Jul 4, 2023
@avelad avelad changed the title feat: Add support for service descriptions from MPD feat(DASH): Add support for service descriptions Jul 4, 2023
lib/hls/hls_parser.js Outdated Show resolved Hide resolved
lib/player.js Outdated Show resolved Hide resolved
lib/player.js Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Incremental code coverage: 63.46%

Copy link
Collaborator

@avelad avelad left a comment

Choose a reason for hiding this comment

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

LGTM. I'll wait to pass all tests to merge it. Thanks for your contribution!

@avelad avelad merged commit 693abd5 into shaka-project:main Jul 5, 2023
15 checks passed
@adgllorente adgllorente deleted the service-descriptions branch July 5, 2023 12:55
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Sep 3, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: DASH The issue involves the MPEG DASH manifest format priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants