-
Notifications
You must be signed in to change notification settings - Fork 0
Alternative media presentation/skipAfter with seekDisable flag implementation #108
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
base: feature/alternative-media-presentations
Are you sure you want to change the base?
Conversation
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a seekDisabled flag to prevent seeking operations. The implementation is straightforward, adding checks in the seek() and _onPlaybackSeeking() methods and exposing the functionality through the MediaPlayer API. I've identified a potential issue with state management during player reset and some missing documentation that should be addressed to improve maintainability.
| playbackStalled, | ||
| manifestUpdateInProgress, | ||
| initialCatchupModeActivated, | ||
| seekDisabled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function setSeekDisabled(value){ | ||
| playbackController.setSeekDisabled(value) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new function is missing JSDoc documentation. Adding documentation improves code clarity and maintainability for future developers. Also, for consistency with the project's coding style, please add a semicolon at the end of the statement.
/**
* Use this method to disable seeking. When disabled, any calls to seek() will be ignored.
* @param {boolean} value - A boolean to enable/disable seeking.
* @memberof module:MediaPlayer
* @instance
*/
function setSeekDisabled(value) {
playbackController.setSeekDisabled(value);
}| function setSeekDisabled(value){ | ||
| seekDisabled = value; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add JSDoc documentation for the new setSeekDisabled function to explain its purpose and parameters. This will help with code maintainability.
/**
* Sets the seek disabled state. If true, seeking is disabled.
* @param {boolean} value
*/
function setSeekDisabled(value) {
seekDisabled = value;
}
No description provided.