Skip to content

Conversation

@cotid-qualabs
Copy link

No description provided.

@cotid-qualabs cotid-qualabs changed the title seekDisable flag implementation Alternative media presentation/skipAfter with seekDisable flag implementation Jun 17, 2025
@sebastianpiq
Copy link

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a 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,

Choose a reason for hiding this comment

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

high

The new seekDisabled flag is not reset in the reset() function. This can lead to unexpected behavior where seeking remains disabled after a player reset (e.g., when loading a new source). Please ensure seekDisabled is set to false within the reset() function to restore the default state.

Comment on lines +778 to +780
function setSeekDisabled(value){
playbackController.setSeekDisabled(value)
}

Choose a reason for hiding this comment

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

medium

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);
    }

Comment on lines +618 to +620
function setSeekDisabled(value){
seekDisabled = value;
}

Choose a reason for hiding this comment

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

medium

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;
    }

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.

3 participants