SimonSimCity / Xamarin-CrossDownloadManager

A cross platform download manager for Xamarin
MIT License
149 stars 68 forks source link

TotalBytesExpected and TotalBytesWritten is converted as integers not float #88

Closed large closed 6 years ago

large commented 6 years ago

To help us fix your issue, please provide the information in the below template.

Note: If you want to save time, please create a fork, reproduce the bug in the sample projects and add a link to the repository here.

Steps to reproduce

I am sorry for not forking, but I could not get it to work...

Use Crossdownloader in Android simulator on a big file. For instance a file with this size: 9,83 GB (10 558 486 413 bytes) is calculated as a 1,89GB file since the convert is a integer and not float.

I suggest using double instead float to ensure you have enough precision for the future 👍

Ref digits available: double has 52 mantissa bits + 1 hidden bit: log(253)÷log(10) = 15.95 digits float has 23 mantissa bits + 1 hidden bit: log(224)÷log(10) = 7.22 digits

Expected behavior

Main problem is "cursor.GetInt" in convert to float.

My suggested fix (not tested): IDownloadFile.cs line 89 and 95:

double TotalBytesExpected { get; }
double TotalBytesWritten { get; }

DownloadFileImplementation.cs line 47, 49, 60 and 62:

private double _totalBytesExpected;
public double TotalBytesExpected {
private double _totalBytesWritten;
public double TotalBytesWritten {

DownloadManagerImplementation.cs line 158 and 159:

downloadFile.TotalBytesWritten = cursor.GetDouble (cursor.GetColumnIndex (Android.App.DownloadManager.ColumnBytesDownloadedSoFar));
downloadFile.TotalBytesExpected = cursor.GetDouble(cursor.GetColumnIndex (Android.App.DownloadManager.ColumnTotalSizeBytes));

Actual behavior

Return actual filesize provided by the server in the Content-Length: HTTP-header

Configuration

Platform:

Device:

SimonSimCity commented 6 years ago

Thanks very much for reporting this error in detail. For now I've fixed it by just replacing the ICursor.getInt() by ICursor.getFloat(). Do you have the possibility to check it? This should already solve the major problem here.

I agree that we should rather use double here to keep the accuracy on a higher level, but this is something I want to keep for version 2.0, because it would require me to change the public interface of this library.

So far I do not have any plans on releasing 2.0 any time soon since I do not use this library myself anymore. I'm just focussed on fixing obvious bugs and implementing features that are worked out in a PR. If someone sees the need for a 2.0 and takes up the issues and keeps close contact with me, I'm very much willing to help him with all the knowledge I have and also merge it.

I'll await your feedback before publishing a new bugfix version 😃

large commented 6 years ago

@SimonSimCity I will check it later on. Out of curiosity; what library do you use for Xamarin since you abandoned this?

SimonSimCity commented 6 years ago

The current Android version is compiled against Android Nougat (7.0).

... and I didn't really abandon this project. It's just that I haven't had the need for any updates for quite a while already before leaving the project I used this package on, so there wasn't really a need for me to force this forward into any direction. I've added quite some features required by other people and helped them finalizing it to fit the design of all platforms though - even after I left the named project.

Looking forward to your test results 😃

SimonSimCity commented 6 years ago

Should be fixed in the version 1.4.0, which I'll release soon. Please reopen if the problem still remains.