przemyslawpluta / node-youtube-dl

youtube-dl driver for node
MIT License
1.71k stars 368 forks source link

feat: use custom url for binary download #351

Open n1ru4l opened 3 years ago

n1ru4l commented 3 years ago

This solves https://github.com/przemyslawpluta/node-youtube-dl/pull/338 which unfortunately got closed without being addressed.

This feature is necessary for people that are self-hosting the youtube-dl binary, e.g. on S3

cc @Kikobeats

Kikobeats commented 3 years ago

Hey, @n1ru4l I understand your use case but I prefer to don't add a new environment variable it could be a thing we can avoid.

What we can do to avoid the new environment variable is to check the response content type here:

https://github.com/przemyslawpluta/node-youtube-dl/blob/master/lib/downloader.js#L29

  1. If the content type is text, then the body contains the URL for fetching the resource.
  2. Otherwise the response is binary.

If you can make that change I will happy to merge :)

n1ru4l commented 3 years ago

That would not work as there are additional get parameter added here: https://github.com/przemyslawpluta/node-youtube-dl/blob/6131a9a050492ba26fe6e6c5e5213b5cb93295fc/lib/downloader.js#L89

Kikobeats commented 3 years ago

@n1ru4l not seeing that is an issue since query parameters are ignored by S3 files.

e.g., https://s.microlink.io/kdsy_4QyLo6Kh61e8SOtMcxCtWg_.png?foo=bar&platform=darwin

n1ru4l commented 3 years ago

But it will not append .exe properly?