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

Fix/doris 2701 webOS 5 startup improvements (unwanted seek) #102

Merged
merged 3 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "hls.js",
"version": "1.5.14",
"version": "1.5.15",
"license": "Apache-2.0",
"description": "JavaScript HLS client using MediaSourceExtension",
"homepage": "https://github.com/video-dev/hls.js",
Expand Down
17 changes: 11 additions & 6 deletions src/controller/base-stream-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,13 @@ export default class BaseStreamController
if (!media) {
return;
}

const liveSyncPosition = this.hls.liveSyncPosition;
const currentTime = media.currentTime;
const mediaCurrentTime = media.currentTime;
const currentTime =
this.loadedmetadata || mediaCurrentTime > 0
? mediaCurrentTime
: this.startPosition;
Comment on lines +1355 to +1358
Copy link

@robwalch robwalch Mar 4, 2025

Choose a reason for hiding this comment

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

Would getLoadPosition work here?

    const currentTime = this.getLoadPosition();

Copy link
Author

Choose a reason for hiding this comment

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

We might be able to, but we'd need to modify getLoadPosition() to check for media.currentTime as well, not sure if that'd have any impact on the rest of the code.

  protected getLoadPosition(): number {
    const { media } = this;
    // if we have not yet loaded any fragment, start loading from start position
    let pos = 0;
-   if (this.loadedmetadata && media) {
+   if (this.loadedmetadata && media && media.currentTime > 0) {
      pos = media.currentTime;
    } else if (this.nextLoadPosition) {
      pos = this.nextLoadPosition;
    }

    return pos;
  }

Copy link

Choose a reason for hiding this comment

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

I don't think so. getLoadPosition should return the position that should be buffered at any given time. If it determines that it should return media.currentTime and that has a value of 0 then that is the value that should be used, not startPosition or nextLoadPosition.

Copy link
Author

Choose a reason for hiding this comment

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

ah okay, I'll give it a try 👍

const start = levelDetails.fragments[0].start;
const end = levelDetails.edge;
const withinSlidingWindow =
Expand All @@ -1376,12 +1381,12 @@ export default class BaseStreamController
}
// Only seek if ready and there is not a significant forward buffer available for playback
if (media.readyState) {
const reason =
!withinSlidingWindow && media.readyState < 4
? 'outside'
: 'too far from the end';
this.warn(
`Playback: ${currentTime.toFixed(
3,
)} is located too far from the end of live sliding playlist: ${end}, reset currentTime to : ${liveSyncPosition.toFixed(
3,
)}`,
`Playback: ${currentTime.toFixed(3)} is located ${reason} of live sliding playlist: ${start.toFixed(3)} - ${end.toFixed(3)}, reset currentTime to : ${liveSyncPosition.toFixed(3)}`,
);
media.currentTime = liveSyncPosition;
}
Expand Down
Loading