-
Notifications
You must be signed in to change notification settings - Fork 245
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
Try retry the download when CURLOpen fail on stream download #594
Conversation
e52bb32
to
7c99f2d
Compare
7c99f2d
to
3dd910d
Compare
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.
Could this be the cause for:
2021-02-02 20:56:20.607 T:1762808032 WARNING: AddOnLog: InputStream Adaptive: Initializing stream with unknown KID!
2021-02-02 20:56:21.843 T:1762808032 ERROR: CCurlFile::FillBuffer - Failed: HTTP returned error 403
2021-02-02 20:56:21.843 T:1762808032 ERROR: CCurlFile::Open failed with code 403 for https://wv-keyos.licensekeyserver.com/:
@MPParsley for error 403 sometimes means problem with DNS used or you are under VPN and the server refuse your connection, perhaps you can also try check possible http headers to add |
Thanks for the tip @CastagnaIT! In my case it's probably a real 403 since there's no KID, I opened another issue for it on #599 |
@CastagnaIT I don't think this is the right solution to the problem. AdaptiveStream already has a segment retrying capability, just that it isn't enabled for VOD streams: inputstream.adaptive/src/common/AdaptiveStream.cpp Lines 92 to 97 in 99d72cd
My guess would be that 404s are quite common on the very edge of a live stream but it's assumed (obviously not the case though) that VOD won't ever 404 or fail otherwise. If you remove the live stream requirement then all should be good :) |
oh thanks i will do |
This looks like a ugly hack. This implicates that if we need to retry because a server is already overloaded, we hammer it even more. And still: If we want something like this, I think it should go into CCurlfile where one has greater control over when this should kick in. |
3dd910d
to
e6288a4
Compare
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.
LGTM
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.
unsigned int retryCount(10);
Is 10 retries too many?
Do we need delay between?
Could we do 10 for live, 3 for vod for example?
I think the 1 second delay is appropriate, no delay would be just hammering the server. Remember the issues with Kayo - 1 second delay and retry was usually enough to get it resolved. The only time I see multiple retries in a log is when there's another issue causing wrong segment number or filename calculation, then we get the full 10 retries due to 404 and stream eventually stops. 10 was probably a number plucked out of the air and could be reduced. Is this outside the scope of the PR though (let VOD streams have the functionality to retry)? |
Is there already a delay here that missed? Or maybe its better to retry based on total time taken. So a slow server may only retry 3 times. But fast server could retry 10 Anyway. Something to keep in mind. |
I have noticed that on days with heavy server traffic (on december/jennuary for christmas days/new year's days) the stream download sometime failed causing unexpected behaviour to Kodi like my issue #583
the problem is that when CURLOpen fails the download will be interrupted
so let's make at least 2 attempts before breaking the playback