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

Change so that default presentation delay for DASH is configurable #1234 #1235

Merged

Conversation

ghost
Copy link

@ghost ghost commented Jan 19, 2018

The DEFAULT_SUGGESTED_PRESENTATION_DELAY_ in DashParser is useful to have configurable
when the stream provider isn't able to add suggestedPresentationDelay in the manifest.

Fixes #1234

@ghost ghost force-pushed the feature/dash_presentation_delay_config branch from 293a14b to 3a7ac65 Compare January 19, 2018 09:27
@joeyparrish joeyparrish self-assigned this Jan 19, 2018
@joeyparrish joeyparrish added the type: enhancement New feature or request label Jan 19, 2018
@joeyparrish joeyparrish added this to the v2.4.0 milestone Jan 19, 2018
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks great otherwise! I'll go ahead and run the tests in the build bot.

@@ -508,6 +509,9 @@ shakaExtern.DrmConfiguration;
* existing contents. If false, xlink-related errors will be propagated
* to the application and will result in a playback failure. Defaults to
* false if not provided.
* @property {number} defaultPresentationDelay
* A default presentationDelay if suggestedPresentationDelay is missing
* in the MPEG DASH manifest, has to be biggar than minBufferTime * 1.5.
Copy link
Member

Choose a reason for hiding this comment

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

typo: "biggar" should be "bigger"

@shaka-bot
Copy link
Collaborator

Test Failure:

Generating Closure dependencies...
Running Closure linter...
205 files checked, no errors found.
Running htmlhint...

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Config loaded: /var/lib/jenkins/workspace/Manual PR Test (local-tests)/.htmlhintrc

Scanned 3 files, no errors found (28 ms).
Checking that the build files are complete...
Checking the tests for type errors...
Checking the usability of generated externs...
Compiling the library (debug)...
Compiling the demo app (debug)...
Compiling the receiver app (debug)...
Compiling the library (release)...
Compiling the demo app (release)...
Compiling the receiver app (release)...
Building the docs...
ERROR: Unable to create a Tag object for source file /var/lib/jenkins/workspace/Manual PR Test (local-tests)/externs/shaka/player.js with title "typedef" and body " {{
customScheme: shakaExtern.DashContentProtectionCallback,
clockSyncUri: string,
ignoreDrmInfo: boolean,
xlinkFailGracefully: boolean,
defaultPresentationDelay: number,
}}

": Invalid type expression "{
customScheme: shakaExtern.DashContentProtectionCallback,
clockSyncUri: string,
ignoreDrmInfo: boolean,
xlinkFailGracefully: boolean,
defaultPresentationDelay: number,
}": Expected "!", "$", "'", "(", "*", "...", "?", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter or Unicode uppercase letter but "}" found.
ERROR: Unable to create a Tag object for source file /var/lib/jenkins/workspace/Manual PR Test (local-tests)/externs/shaka/player.js with title "typedef" and body " {{
customScheme: shakaExtern.DashContentProtectionCallback,
clockSyncUri: string,
ignoreDrmInfo: boolean,
xlinkFailGracefully: boolean,
defaultPresentationDelay: number,
}}

": Invalid type expression "{
customScheme: shakaExtern.DashContentProtectionCallback,
clockSyncUri: string,
ignoreDrmInfo: boolean,
xlinkFailGracefully: boolean,
defaultPresentationDelay: number,
}": Expected "!", "$", "'", "(", "*", "...", "?", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter or Unicode uppercase letter but "}" found.
END-BUILD: FAILURE
Build step 'Execute shell' marked build as failure

@joeyparrish
Copy link
Member

It looks like jsdoc doesn't like your trailing comma in the doc string. I'll file a bug to make jsdoc more flexible, as trailing commas are recommended in other contexts.

The DEFAULT_SUGGESTED_PRESENTATION_DELAY_ in DashParser is useful to have configurable
when the stream provider isn't able to add `suggestedPresentationDelay` in the manifest
@ghost ghost force-pushed the feature/dash_presentation_delay_config branch from 3a7ac65 to 6f4f921 Compare January 22, 2018 08:21
@ghost
Copy link
Author

ghost commented Jan 22, 2018

@joeyparrish
I've amended the commit resolving the issues, removed the trailing comma and fixed the spelling mistake :)
Is there a command I should've run to see the jsdoc error? I ran ./build/all.py and ./build/test.py and didn't get that error 🤔

@joeyparrish
Copy link
Member

Yes ./build/docs.py will build the documentation. It is a little silly and counterintuitive that all.py doesn't include docs.py, but we reasoned that someone who wants to rebuild the code all the time during development won't want to rebuild the docs all the time. I'm honestly unsure what the best change would be to improve that.

@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish
Copy link
Member

Okay, everything looks good! Thanks!

@joeyparrish joeyparrish merged commit cf3e1e3 into shaka-project:master Jan 22, 2018
joeyparrish pushed a commit that referenced this pull request Jan 22, 2018
…1235)

It is useful to have the default presentation delay configurable when the stream provider isn't able to add `suggestedPresentationDelay` in the manifest

Fixes #1234
@joeyparrish
Copy link
Member

Cherry-picked for v2.3.2. Just barely missed the cut-off for v2.3.1. Thanks again for the contribution!

@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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

2 participants