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

file.deselect supposedly not working #164

Open
feross opened this Issue Oct 25, 2014 · 7 comments

Comments

7 participants
@feross
Member

feross commented Oct 25, 2014

Heard from someone on IRC that after calling file.deselect on a file, it still gets fully downloaded.

@feross feross added bug node and removed bug labels Oct 25, 2014

@SanyaZol

This comment has been minimized.

Show comment
Hide comment
@SanyaZol

SanyaZol Dec 8, 2014

I also didn't see an option to download torrent metadata only as well as a ways to decide which files/parts should (not) be downloaded.

SanyaZol commented Dec 8, 2014

I also didn't see an option to download torrent metadata only as well as a ways to decide which files/parts should (not) be downloaded.

@Wingman4l7

This comment has been minimized.

Show comment
Hide comment
@Wingman4l7

Wingman4l7 Jun 2, 2015

@feross The problem is in the conditional in torrent.deselect(), which needs to be rewritten. Alternatively, you can possibly rewrite the code that handles the default selection & storage of the entire piece range in the torrent when client.add() is called.

If you put a debug line inside the conditional and then attempt to download only particular files in a torrent via deselecting all of the files which you do not want, you will never see it -- because the conditional is never met!

The issue appears to be that the conditional is trying to match the piece range of the file you wish to deselect, against the stored piece range. However, it will never find a match, as the only stored range is the piece range of the torrent in its entirety.

A way to demonstrate how this is broken (and a hacky fix to get the effective functionality of deselect() in the meantime) is to deselect the entire piece range, then select the files that you want. However, the piece count is not exposed in the library, so you can't call deselect(), because you don't know the end of the piece range! So, in lieu of modifying deselect(), setting torrent._selections = []; will do the trick.

You also might want to change the readme section for torrent.deselect(). Right now it only mentions that it can "deprioritize a range". Perhaps say something along the lines of "Use a non-zero value for priority -- zero means that the pieces will not be downloaded."

I did notice that unfortunately, you will get "false" copies of the files on either side of the piece range you selected, due to the nature of files not beginning or ending exactly where pieces do. This should probably be opened as a separate bug unless you can easily add in code for cleanup of those / prevent the creation of those false-files during this bugfix.

Credit goes to AlliedEnvy for figuring this out.

Wingman4l7 commented Jun 2, 2015

@feross The problem is in the conditional in torrent.deselect(), which needs to be rewritten. Alternatively, you can possibly rewrite the code that handles the default selection & storage of the entire piece range in the torrent when client.add() is called.

If you put a debug line inside the conditional and then attempt to download only particular files in a torrent via deselecting all of the files which you do not want, you will never see it -- because the conditional is never met!

The issue appears to be that the conditional is trying to match the piece range of the file you wish to deselect, against the stored piece range. However, it will never find a match, as the only stored range is the piece range of the torrent in its entirety.

A way to demonstrate how this is broken (and a hacky fix to get the effective functionality of deselect() in the meantime) is to deselect the entire piece range, then select the files that you want. However, the piece count is not exposed in the library, so you can't call deselect(), because you don't know the end of the piece range! So, in lieu of modifying deselect(), setting torrent._selections = []; will do the trick.

You also might want to change the readme section for torrent.deselect(). Right now it only mentions that it can "deprioritize a range". Perhaps say something along the lines of "Use a non-zero value for priority -- zero means that the pieces will not be downloaded."

I did notice that unfortunately, you will get "false" copies of the files on either side of the piece range you selected, due to the nature of files not beginning or ending exactly where pieces do. This should probably be opened as a separate bug unless you can easily add in code for cleanup of those / prevent the creation of those false-files during this bugfix.

Credit goes to AlliedEnvy for figuring this out.

@dcposch

This comment has been minimized.

Show comment
Hide comment
@dcposch

dcposch Sep 20, 2016

Contributor

Yeah, the deselect() API is janky.

You have to deselect the whole torrent first, then select individual files.

Here's how we made it work in WebTorrent Desktop:

  // Remove default selection (whole torrent)
  torrent.deselect(0, torrent.pieces.length - 1, false)

  // Add selections (individual files)
  for (let i = 0; i < selections.length; i++) {
    const file = torrent.files[i]
    if (selections[i]) {
      file.select()
    } else {
      console.log('deselecting file ' + i + ' of torrent ' + torrent.name)
      file.deselect()
    }
  }

@feross we should probably improve this API before WebTorrent 1.0, because afterward we don't want to change it.

Is there a reason to let people deselect individual blocks within a file? What if the API only allowed selecting which files in a torrent to download?

Here's an API proposal

Remove the select and deselect methods from File and Torrent.

Let users specify which files they want when adding a torrent:

client.add('magnet://...', { fileSelections: [true, true, false, true] })

When you do that, the files that aren't selected are never created on disk.

Let users specify which files they want after adding a torrent:

torrent.setFileSelections([true, true, false, true])

Doing that doesn't delete any files. Partially downloaded files just stay there, unless the user deletes them separately. This lets you pause and resume individual files in a torrent.

This alternate API would require the library to do a bit more work to keep track of pieces overlapping a file boundary. It's simpler, though. You always declare what you want to download for the whole torrent, instead of calling select() and deselect() on individual files and piece ranges. This API also lets you download a torrent without creating all the files on disk.

@feross

Contributor

dcposch commented Sep 20, 2016

Yeah, the deselect() API is janky.

You have to deselect the whole torrent first, then select individual files.

Here's how we made it work in WebTorrent Desktop:

  // Remove default selection (whole torrent)
  torrent.deselect(0, torrent.pieces.length - 1, false)

  // Add selections (individual files)
  for (let i = 0; i < selections.length; i++) {
    const file = torrent.files[i]
    if (selections[i]) {
      file.select()
    } else {
      console.log('deselecting file ' + i + ' of torrent ' + torrent.name)
      file.deselect()
    }
  }

@feross we should probably improve this API before WebTorrent 1.0, because afterward we don't want to change it.

Is there a reason to let people deselect individual blocks within a file? What if the API only allowed selecting which files in a torrent to download?

Here's an API proposal

Remove the select and deselect methods from File and Torrent.

Let users specify which files they want when adding a torrent:

client.add('magnet://...', { fileSelections: [true, true, false, true] })

When you do that, the files that aren't selected are never created on disk.

Let users specify which files they want after adding a torrent:

torrent.setFileSelections([true, true, false, true])

Doing that doesn't delete any files. Partially downloaded files just stay there, unless the user deletes them separately. This lets you pause and resume individual files in a torrent.

This alternate API would require the library to do a bit more work to keep track of pieces overlapping a file boundary. It's simpler, though. You always declare what you want to download for the whole torrent, instead of calling select() and deselect() on individual files and piece ranges. This API also lets you download a torrent without creating all the files on disk.

@feross

@kocoten1992

This comment has been minimized.

Show comment
Hide comment
@kocoten1992

kocoten1992 Jan 12, 2017

Contributor

Or maybe:

torrent.setFileSelections([12,23]);

So if a torrent have big number of files, you don't have to do [false, false, false, false, false, false, ... , true];

torrent.setFileDeselections([4,6,9]);
Contributor

kocoten1992 commented Jan 12, 2017

Or maybe:

torrent.setFileSelections([12,23]);

So if a torrent have big number of files, you don't have to do [false, false, false, false, false, false, ... , true];

torrent.setFileDeselections([4,6,9]);
@grantholle

This comment has been minimized.

Show comment
Hide comment
@grantholle

grantholle Apr 22, 2017

I like the idea that @kocoten1992 has, which is how the transmission rpc is also. You define an array of wanted/unwanted file indices. Having this option when adding torrents would be amazing!

grantholle commented Apr 22, 2017

I like the idea that @kocoten1992 has, which is how the transmission rpc is also. You define an array of wanted/unwanted file indices. Having this option when adding torrents would be amazing!

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 7, 2018

Some news?

ghost commented Jan 7, 2018

Some news?

@pldubouilh

This comment has been minimized.

Show comment
Hide comment
@pldubouilh

pldubouilh Jan 23, 2018

Contributor

I started implementing @dcposch proposal, in the fashion described by @kocoten1992, with a list of wanted files.
It's over there - it needs a bit more testing but seem to work fine on my side.

Contributor

pldubouilh commented Jan 23, 2018

I started implementing @dcposch proposal, in the fashion described by @kocoten1992, with a list of wanted files.
It's over there - it needs a bit more testing but seem to work fine on my side.

@feross feross added accepted and removed accepted labels May 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment