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

reset BlacklistController after url resolution failed #2539

Conversation

jeffcunat
Copy link
Contributor

@jeffcunat jeffcunat commented Apr 20, 2018

The use case is to be able to seek after a segment download has failed. At the moment, when this happens, an event Events.URL_RESOLUTION_FAILED is fired and stops the schedule controller.
If we want to seek further to bypass a missing segment, we can not start the playing again because the base url of the segment that failed is stored in the black list controller.
By resetting it after the event firing, it allows to try seeking further. Without this PR we can not seek at all after this event. So the only thing this PR changes is to at least let the client to jump over a missing segment.
If needed we can add a configuration to enable/disable this feature. What do you think ?

@jeffcunat
Copy link
Contributor Author

Hi @bbcrddave ! As you have done the whole part about BaseUrl management, could you review this PR ? Thanks

@davemevans
Copy link
Contributor

tl;dr this seems reasonable but it'd be nice to control it.


This seems to solve the problem you describe, but I think there's a wider conversation to be had around what to do in case of a failure. This was started at #651. It was also discussed at a F2F I seem to recall, but perhaps it's worth discussing again in May.

Anyway .... Most of the behaviour in this area is based around the DVB-DASH spec which clearly states what to do when a segment is missing, and what to do when all base URLs have been exhausted an error should be raised and the session terminated. Calling reset here marks all BaseURLs as selectable again, which may or may not be appropriate, so perhaps it would be worth asking the application (via a config option or something.

@jeffcunat
Copy link
Contributor Author

Thanks @bbcrddave for the review.
Yes @bbert and @nicosang will be to the meeting in May, so this subject may be added to the agenda and we will see what to do with this PR.

@jeffcunat
Copy link
Contributor Author

We can make it configurable but we can also restrict the reset only in the case of a BasicSelector and not in case of a DVBSelector). As far as I understand, the BasicSelector is not really an implementation of the DVB spec, just a really simple implementation that picks the first available baseurl. But most of the time, there is only one defined in a MPD, so as soon as we have a missing segment we are stucked forever. So my proposition is to modify the PR to do the reset only when having a BasicSelector.

@davemevans
Copy link
Contributor

This solves the problem, and is better, so ship it 👍

I'm still not convinced it's the right way to solve the true problem, but we can have that discussion at a later date.

@epiclabsDASH epiclabsDASH added this to the 2.6.8 milestone Apr 23, 2018
@epiclabsDASH epiclabsDASH merged commit c56b7e9 into Dash-Industry-Forum:development Apr 23, 2018
@bbert bbert deleted the allow-seeking-after-download-failure branch September 19, 2018 09:46
@bbert
Copy link
Contributor

bbert commented Oct 30, 2018

Hi @davemevans,
Coming back on this issue for which we still face some problems.
Question about basic selector: do we really need to blacklist base URL in case of basic selector? Since we know there is only one valid base URL? Do I miss something for which I am not aware?

If we do not blacklist the base URL, in case of segment failure, then we would be able to get around a missing segment and then seek further this missing segment at application level.
The reset() mechanism implemented in this PR does not resolve completely the issue, that's why I would suggest to simply not blacklisting base URL in case of basic selector.

I agree we should consider a wider conversation on segment failure, but meanwhile we need to enforce current version.

@davemevans
Copy link
Contributor

I'm not 100% sure of the use case but, if there is only one BaseURL, optionally not blacklisting seems like a pragmatic answer.

It isn't clear how you would ever fail at that point since you will retry the segment forever, but I suppose that is for the application to decide ...

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.

None yet

4 participants