Skip to content
Snippets Groups Projects

Rewrote VLCHTTPFileDownloader

Closed Justin Anderson requested to merge jay18001/vlc-ios:filedownloader-rewrite into master
All threads resolved!

Problem

Http downloads caused the ui to lock up and the caused the app to become unusable. Digging deeper it, it looks like NSUrlSession delegate methods were calling back on the main queue.

Solution

Rewrote Http file downloader to use background thread callbacks and use download session tasks over data tasks and writing to file. Also laid the ground work for simultaneous downloads.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Carola
  • This is a very good idea. This part was really too buggy.

  • Carola
  • Maintainer

    In general it looks really good. Especially the switch to download tasks is good here since they won't be killed in the background like the data tasks do. I still need to test it though

  • added 1 commit

    Compare with previous version

  • Justin Anderson added 7 commits

    added 7 commits

    Compare with previous version

  • Justin Anderson resolved all discussions

    resolved all discussions

  • I really like your MR, especially to move the downloads to the background where they belong. However, after the download is complete and while you download, you are now updating the UI from the background thread! so autolayout complains :)

    Additionally, there is a small bug which prevents the fileURL for the download task to be created correctly if no filename is provided as parameter to the function. This micro patch solves it:

    diff --git a/Sources/VLCHTTPFileDownloader.m b/Sources/VLCHTTPFileDownloader.m
    index ce4579b8..00093723 100644
    --- a/Sources/VLCHTTPFileDownloader.m
    +++ b/Sources/VLCHTTPFileDownloader.m
    @@ -110,10 +110,10 @@
         if (downloadFileName.pathExtension.length == 0 || ![downloadFileName isSupportedFormat]) {
             NSString *urlExtension = url.pathExtension;
             NSString *extension = urlExtension.length != 0 ? urlExtension : @"vlc";
    -        fileName = [fileName stringByAppendingPathExtension:extension];
    +        downloadFileName = [fileName stringByAppendingPathExtension:extension];
         }
         downloadTask.fileName = downloadFileName;
    -    downloadTask.fileURL = [NSURL fileURLWithPath:[libraryPath stringByAppendingPathComponent:fileName]];
    +    downloadTask.fileURL = [NSURL fileURLWithPath:[libraryPath stringByAppendingPathComponent:downloadFileName]];
    
         NSString *identifier = [[NSUUID UUID] UUIDString];

    Finally, please strip the unneeded white spaces.

    Edited by Felix Paul Kühne
  • Ehm, instead of doing this back and forth, I took the liberty to do those minor changes myself under your name. Merged as ff9a2c27.

    Thanks a lot for your work!

  • Please register or sign in to reply
    Loading