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

Add support for async download #38

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jaspervdm
Copy link

I added functionality to download youtube videos asynchronized, using react event loop and promises. Example code:

$loop = \React\EventLoop\Factory::create();
$timer = $loop->addPeriodicTimer(1, function () {
  static $i = 0;
  echo "Loop ".++$i."\n";
});
$loop->addTimer(1, function () use ($timer, $loop) {
  $dl = new \YoutubeDl\YoutubeDl([
    "extract-audio" => true,
    "audio-format" => "mp3",
    "audio-quality" => 0,
    "output" => "%(id)s.%(ext)s"
  ]);
  $dl->downloadAsync("URL", $loop)->then(function (\YoutubeDl\Entity\Video $video) use ($timer) {
    echo "Done, ".$video->getFilename()."\n";
    $timer->cancel();
  }, function (Exception $e) use ($timer) {
    echo get_class($e).": ".$e->getMessage()."\n";
    $timer->cancel();
  });
});
$loop->run();

@norkunas
Copy link
Owner

norkunas commented Sep 5, 2017

I don't think that this should be in core because I want this lib to be lightweight 🤔

@jaspervdm
Copy link
Author

jaspervdm commented Sep 5, 2017

Alright, thats fine by me, I am closing the PR. If anyone in the future is reading this, you can use this feature by using these lines in your composer.json:

{
  "require": {
    "norkunas/youtube-dl-php": "dev-async",
  },
  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/jaspervdm/youtube-dl-php"
    }
  ]
}

@jaspervdm jaspervdm closed this Sep 5, 2017
@norkunas
Copy link
Owner

norkunas commented Sep 6, 2017

@jaspervdm actually we could provide this method but just adding these dependencies to the composer suggest and if people would try async method and the dependencies wouldn't be installed then we could just throw exception

@norkunas norkunas reopened this Sep 6, 2017
@jaspervdm
Copy link
Author

Okay, I will modify the code to check for the dependencies.

@jaspervdm
Copy link
Author

I made the async functionality optional now, what do you think?

@@ -116,6 +122,77 @@ public function download(string $url): Video
return $this->processDownload($process);
}

public function downloadAsync(string $url, LoopInterface $loop): PromiseInterface
{
if (!class_exists('LoopInterface')) {
Copy link
Owner

Choose a reason for hiding this comment

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

Use LoopInterface::class

throw new \RuntimeException('React event loop package not installed.');
}

if (!class_exists('Deferred')) {
Copy link
Owner

Choose a reason for hiding this comment

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

Use Deferred::class

throw new UrlNotSupportedException(sprintf('Provided url "%s" is not supported.', $url));
}

$arguments = [
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe would be nice to make a private helper to share download process building?

@norkunas
Copy link
Owner

norkunas commented Sep 7, 2020

@jaspervdm are you still interested to finish this?

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.

2 participants