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

Resume retried downloads #39

Merged
merged 4 commits into from
Dec 18, 2016
Merged

Conversation

MicroDroid
Copy link

With this patch downloads are just resumed rather than started from scratch.

The patch makes use of the Range HTTP header

@carlosflorencio
Copy link
Owner

Hi,

Can you remove the .phpintel folder?
I will merge after that.

Thanks!

@MicroDroid
Copy link
Author

@iamfreee Done.

But, have you actually tested it? At the beginning I had some weird corruptions in the videos, and they automagically got fixed as I was refactoring my implementation a little bit.

After the final changes, I tested it with few videos, I killed the port forcefully with gdb during download, seems to work fine. Just curious if it could start corrupting things randomly out of nothing.

@MicroDroid
Copy link
Author

Hold on a bit, don't merge until I do some more testing

@carlosflorencio
Copy link
Owner

I dont have a subscription to test it.

So i hope you make it work good :D

@MicroDroid
Copy link
Author

@iamfree aww 🐙

Well, I tested it, it seems to work great, except there was a little issue in string formatting, it's fixed.

However, a new problem arises is that a timed out connection never breaks off and this causes the download process to hang overall. One should make guzzle timeout on reading the socket or something

@carlosflorencio
Copy link
Owner

@MicroDroid i see.

That timeout problem was there before your changes? I have a good internet so i cant test this edge cases.

@MicroDroid
Copy link
Author

@iamfreee Yeah, it's a guzzle thingy.

The summary of my changes is switching the file to append mode (a) and put the downloaded amount of bytes in the Range header. Most of everything else was bug fixes to my own implementation

@carlosflorencio
Copy link
Owner

Okay then, i will merge and then wait until someone want to work on the timeout problem.

Many thanks.

@carlosflorencio carlosflorencio merged commit c1a1594 into carlosflorencio:master Dec 18, 2016
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

2 participants