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

Feature/451 keep seeding after closing player #474

Conversation

chtrinh
Copy link
Contributor

@chtrinh chtrinh commented Feb 10, 2017

'autoSeed' - starts up a forked process with a WebTorrent client which
enables this feature. Torrent files are create and saved in tmp folder
to be used later for seeding. Randomly selects files for join swarm.
'seedLimit' - starting limit for number of torrents connected.
Copy link
Contributor

@team-pct team-pct left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really interesting changes , can you let the user disable this on Settings? Also can you send this Pr into Butter ? https://github.com/butterproject/butter-desktop
Thank you for your contributions !

@xaiki
Copy link
Contributor

xaiki commented Feb 12, 2017

very interesting indeed ! a few comments though (a part from whitespaces nitpicks and unused variables):

  • we may want seeders to be named, because we'd much likely want to spawn a new worker to seed updates as well as the current movie.
  • this duplicate the streamer torrent code, why not always use a seeder (and kill the streamer code for this worker-enabled solution ?)
  • seedLimit seems a bit buggy in our usecase, we usually have 1 big file and a lot of small files, it only makes sense to seed the big file for our users, but we might as well seed the small ones too (also it allows you to remove the dep on underscore and make this an independent npm module <3)

thanks heaps !

@chtrinh
Copy link
Contributor Author

chtrinh commented Feb 12, 2017

@team-pct: Yes, users can disable this setting in the setting window. checkbox label "autoSeed". Still not happy with the naming of it. Suggestions welcomed.

Should PRs go to butter-desktop instead of this?

@xaiki: I'll fix the whitespace issues. Not sure which unused variables you are referring to.

  1. I'll add an optional 'name' property to the seeder constructor.

  2. Agreed, I just wanted some feedback before I do exactly that. The change I felt was too major. I think what you touch on also address this issue: using webworkers for some cpu-intensive tasks butterproject/butter-desktop#517 ('ll make my additional comments there)

  3. 'seedLimit' is more like amount of torrents to start connecting to. (again probably badly named).
    I just used underscore for the sampling of the array. I want to users to randomly select torrents from their cache and join the swarm. Or there be too many seeders/peers for torrents starting with the letter 'A'.

@team-pct team-pct merged commit 7dcca9e into popcorn-official:development Mar 10, 2018
team-pct added a commit that referenced this pull request Mar 18, 2018
…_after_closing_player"

This reverts commit 7dcca9e, reversing
changes made to 36e234e.
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.

3 participants