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/seek near period end #3718

Conversation

lkinasiewicz
Copy link
Contributor

Addresses issue #3713

@dsilhavy
Copy link
Collaborator

@lkinasiewicz Thank you for the work on this. I had a quick look, and I am afraid this will not work for live multiperiod. You can check with this stream for instance: https://d24rwxnt7vw9qb.cloudfront.net/v1/dash/e6d234965645b411ad572802b6c9d5a10799c9c1/All_Reference_Streams/4577dca5f8a44756875ab5cc913cd1f1/index.mpd

We use the DashHandler logic to decide whether a period is finished or not. In the case of live the changes in DashHandler will never finish a period:

        // for dynamic manifest, more segments will be available after manifest update
        if (isDynamicManifest && !dynamicStreamCompleted) {
            return false;
        }

In addition, i think we break track switches with the changes made here.

I started improving the DashHandler logic as well, based on the email thread we had. My changes are visible here: https://github.com/Dash-Industry-Forum/dash.js/tree/feature/dashHandlerOptimizations
The changes I made there should fix parts of #3713.

I suggest we wait for my PR to be finished and merged and then check your changes regarding the seek on top of my PR.

@lkinasiewicz
Copy link
Contributor Author

Well... good point 🤦 Your changes look like they should fix the problem in DashHandler so let's wait with this one.

@lkinasiewicz
Copy link
Contributor Author

I have removed changes in DashHandler and left only the rest. When I applied my changes on top of #3727, I could not reproduce the problem reported in #3713.

@lkinasiewicz
Copy link
Contributor Author

@dsilhavy can you please have a look at the changes?

@dsilhavy
Copy link
Collaborator

@lkinasiewicz Thanks for your work on this, based on the recent changes we made to the scheduling I provided a first draft of a fix for the seeking problem here: #3772 . Can you please check on your side if this solves the problem.

We should only adjust the seek target if we dont find a valid request.

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.

2 participants