Fix deezer extractor #4935

LucBerge · 2022-09-15T17:13:59Z

Description of the pull request and other information

This pull request fix the Deezer extractor with full song support and closes #1591.

Add blowfish encipher
Add deezer downloader
Add full song support
Add artist extractor
Add track extractor
Add episode extractor
Add show extractor

Before submitting this pull request I have:

At least skimmed through contributing guidelines including yt-dlp coding conventions
Searched the bugtracker for similar pull requests
Checked the code with flake8 and ran relevant tests

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check one of the following options:

I am the original author of the DeezerExtractor code and I am willing to release it under Unlicense. See youtube-dl.
I am not the original author of the Blowfish key computation and cipher code but it is in public domain or released under Unlicense (provide reliable evidence). See youtube-dl.

What is the purpose of my pull request?

Fix or improvement to an extractor (Make sure to add/update tests)
New extractor (Piracy websites will not be accepted)
Core bug fix/improvement
New feature (It is strongly recommended to open an issue first)

- Add blowfish encipher - Add deezer downloader - Add full song support - Add artist extractor - Add track extractor - Add episode extractor - Add show extractor

LucBerge · 2022-09-15T17:24:56Z

What about the failed core tests ?

pukkandan · 2022-09-15T17:26:44Z

Dont import legacy compat variables

LucBerge · 2022-09-15T17:31:27Z

Dont import legacy compat variables

So I copy the function code in the file ?

yt_dlp/extractor/deezer.py

Grub4K · 2022-09-15T19:14:42Z

So I copy the function code in the file ?

No, you use the functions provided by Python, for example the chr builtin instead of compat_chr. Compat functions were workarounds which are no longer needed since support for old Python versions is not required.

The decryption process could potentially be done more cleanly if the calculation of the key happens inside the downloader and is not dependant on the key property being present.

Also Cryptodome provides an internal Blowfish implementation (Cryptodome.Cipher.Blowfish). It might be worth using that one and fallback only if Cryptodome not available, like doing already for AES inside dependencies.py

- Remove utf-8 encoding sys reconfiguration - Do not use compat functions anymore - Blowfish key calculation in the extractor - Minor fixes

LucBerge · 2022-09-15T20:04:21Z

No, you use the functions provided by Python, for example the chr builtin instead of compat_chr. Compat functions were workarounds which are no longer needed since support for old Python versions is not required.

Done.

The decryption process could potentially be done more cleanly if the calculation of the key happens inside the downloader and is not dependant on the key property being present.

If by "inside the downloader" you mean the DeezerBaseInfoExtractor, then it's done.

Also Cryptodome provides an internal Blowfish implementation (Cryptodome.Cipher.Blowfish). It might be worth using that one and fallback only if Cryptodome not available, like doing already for AES inside dependencies.py

I'll do it later.

pukkandan · 2022-09-16T04:37:33Z

yt_dlp/extractor/deezer.py

+
+    GW_LIGHT_URL = "https://www.deezer.com/ajax/gw-light.php"
+    GET_URL = "https://media.deezer.com/v1/get_url"
+    BLOWFISH_KEY = b"g4el58wc0zvf9na1"


How is this key obtained?

From here : https://github.com/ytdl-org/youtube-dl/pull/13194/files#diff-301769c0cffc70e23f61421e11e68f1c77bf60db2f7547430bd3c4c894609699R40

Leaving the key in the repository may not be the best idea. Deezer has sent DMCAs to many repos containing it in the past. It specifically mentions "To do so, these repositories are sharing the private encryption keys retrieved maliciously to bypass Deezer's security measures to unlawfully download its music catalogue, directly from Deezer’s servers in total violation of our rights and those of our music licensors (phonographic producers, performing artists, songwriters and composers)."

It might be a better idea to dynamically generate the key or just make users provide it themselves.

@Ryan5453 I agree, this is a good idea. Note that this key is not directly used to decrypt the song. It is used to compute a new key that will decrypt the song.

@Ryan5453 I have no idea what this code returns. I can't run it on my windows. Do you know how to generate this key in python directly ?

yt_dlp/blowfish.py

gamer191 · 2022-09-18T15:06:11Z

Related PRs: ytdl-org/youtube-dl#13194 ytdl-org/youtube-dl#22074

EDIT: the 2nd PR I linked doesn't support full length songs, according to ytdl-org/youtube-dl#22074 (comment):
@Jan-NiklasB , note that the full length song support is available on the branch [deezer uncipher](https://github.com/LucBerge/youtube-dl/tree/deezer_uncipher).
Unfortunately the linked repo was DMCAed when youtube-dl was DMCAed, but it might be available on the wayback machine (which seems to be down right now)

LucBerge · 2022-09-20T11:11:04Z

Unfortunately the linked repo was DMCAed when youtube-dl was DMCAed, but it might be available on the wayback machine (which seems to be down right now)

Yes it is : https://web.archive.org/web/20200917071235/https://github.com/LucBerge/youtube-dl but the deezer_uncipher branch is not.

LucBerge · 2022-10-03T07:29:57Z

Regarding this comment (and a discussion I had with someone 😅) authentication is not used in my code. It explains why the MD5_ORIGIN key is not present and why MP3_64, MP3_128 and MP3_MISC are the only format I have access to even though all of them exists. Authentication code must be added in the futur for better song quality.

@gamer191 GitHub don't want to unlock my repo, they proposed to remove it so I can fork it again. My commits are still on my local computer, I can push it to a new github repo but it can be DMCA again for the same reason youtube-dl was. So maybe it's not the best idea.

yt_dlp/extractor/deezer.py

- Remove useless impot - Fix tracks number for extractors - Fix comment

Ryan5453

I haven't actually tested this code and therefore can not confirm if it actually works or not, but the new key extractor (in theory) is definitely a much better implementation than hardcoding it for legal reasons.

LucBerge · 2023-03-08T12:49:46Z

I haven't actually tested this code and therefore can not confirm if it actually works or not, but the new key extractor (in theory) is definitely a much better implementation than hardcoding it for legal reasons.

You mean generating the key dynamically based on the code you provided?

Ryan5453 · 2023-03-08T12:50:41Z

I haven't actually tested this code and therefore can not confirm if it actually works or not, but the new key extractor (in theory) is definitely a much better implementation than hardcoding it for legal reasons.

You mean generating the key dynamically based on the code you provided?

Yes. I'm saying you did a good job updating that.

LucBerge · 2023-03-08T12:51:25Z

job updating tha

Thanks. What can I do for core tests failing?
I have not even touch the files it fails on.

LucBerge · 2023-03-08T12:53:30Z

yt_dlp/blowfish.py

@@ -0,0 +1,309 @@
+from .compat import compat_struct_unpack, compat_struct_pack


I guess I should not import compat functions

pukkandan · 2023-03-08T13:31:52Z

Due to reasons we have discussed above (especially #4935 (comment)), I don't think it is a good idea to merge this code. In theory, we would have some protection by not hard-coding the key, but even so, it is a very risky move for us¹.

Since our plugin system has now matured, I recommend distributing this yourself as a plugin instead. This way, any DMCA will only affect your plugin, and not the entire project. While there are no downloader plugins yet, you can hack it in due to the flexibility of Python

from yt_dlp.downloader import PROTOCOL_MAP
PROTOCOL_MAP['deezer'] = DeezerFD

PS: Also, any PR adding new downloader is currently being held on pause since we are discussing re-working how IEs declare FDs

I have neither the time, the will, or the economic ability to fight it in court ↩

LucBerge · 2023-03-08T13:40:34Z

Due to reasons we have discussed above (especially #4935 (comment)), I don't think it is a good idea to merge this code. In theory, we would have some protection by not hard-coding the key, but even so, it is a very risky move for us.

It might be a better idea to dynamically generate the key or just make users provide it themselves.

The key is now dynamically generated as suggested

Since our plugin system has now matured, I recommend distributing this yourself as a plugin instead. This way, any DMCA will only affect your plugin, and not the entire project. While there are no downloader plugins yet, you can hack it in due to the flexibility of Python
from yt_dlp.downloader import PROTOCOL_MAP
PROTOCOL_MAP['deezer'] = DeezerFD

My goal in this PR is to share the extractor with others so people will be able to download too. Nobody will use my extractor as a plugin...

PS: Also, any PR adding new downloader is currently being held on pause since we are discussing re-working how IEs declare FDs

Any link to the issue/discussion ?

pukkandan · 2023-03-08T13:51:33Z

Nobody will use my extractor as a plugin...

People who need it will find it. It can be added to the wiki and I could change even the current extractor warning to point to plugins if desired.

PS: Also, any PR adding new downloader is currently being held on pause since we are discussing re-working how IEs declare FDs

Any link to the issue/discussion ?

A lot of the discussion happens on discord b/w maintainers. #3048 is one possible implementation that came out of it.

coletdjnz · 2023-03-08T18:57:43Z

Plugin repos can be tagged with yt-dlp-plugins to be discoverable. With the new plugin system we are aiming to make it a viable alternative to having extractor support in core. It's pretty new so not too many have heard of it yet nor is there many public plugins listed yet, so will take some time.

Also, after some investigation a while back, the origins of some of this code are potentially from DMCA'd sources and likely not under unlicense (it goes further back than the PR on youtube-dl), so we likely cannot include it anyway in it's current form.

LucBerge · 2023-03-08T21:32:58Z

So we likely cannot include it anyway in it's current form.

Ok. So in which form can you accept it ?

Fix deezer extractor

797283b

- Add blowfish encipher - Add deezer downloader - Add full song support - Add artist extractor - Add track extractor - Add episode extractor - Add show extractor

LucBerge mentioned this pull request Sep 15, 2022

deezer download ytdl-org/youtube-dl#31075

Open

LucBerge added 2 commits September 15, 2022 19:19

Code convention and flake8

2d06167

flake8

72df4f6

LucBerge commented Sep 15, 2022

View reviewed changes

yt_dlp/extractor/deezer.py Outdated Show resolved Hide resolved

LucBerge commented Sep 15, 2022

View reviewed changes

yt_dlp/extractor/deezer.py Outdated Show resolved Hide resolved

Reviews

34996fb

- Remove utf-8 encoding sys reconfiguration - Do not use compat functions anymore - Blowfish key calculation in the extractor - Minor fixes

pukkandan reviewed Sep 16, 2022

View reviewed changes

gamer191 reviewed Sep 18, 2022

View reviewed changes

yt_dlp/blowfish.py Outdated Show resolved Hide resolved

coletdjnz added the do-not-merge label Sep 22, 2022

LucBerge commented Oct 3, 2022

View reviewed changes

yt_dlp/extractor/deezer.py Show resolved Hide resolved

LucBerge added 2 commits March 8, 2023 12:30

Minor changes

98c9aee

- Remove useless impot - Fix tracks number for extractors - Fix comment

Generate key dynamically

122ff0a

LucBerge requested review from pukkandan, gamer191 and Ryan5453 and removed request for pukkandan and gamer191 March 8, 2023 12:39

Merge branch 'master' into deezer

d1f9f08

Ryan5453 approved these changes Mar 8, 2023

View reviewed changes

LucBerge commented Mar 8, 2023

View reviewed changes

LucBerge requested a review from pukkandan March 8, 2023 13:05

pukkandan force-pushed the master branch 2 times, most recently from ee280c7 to 7aeda6c Compare May 24, 2023 18:09

gamer191 mentioned this pull request Jun 25, 2023

Spotify support confusing ytdl-org/youtube-dl#31037

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix deezer extractor #4935

Fix deezer extractor #4935

LucBerge commented Sep 15, 2022 •

edited

LucBerge commented Sep 15, 2022

pukkandan commented Sep 15, 2022

LucBerge commented Sep 15, 2022 •

edited

Grub4K commented Sep 15, 2022

LucBerge commented Sep 15, 2022

pukkandan Sep 16, 2022

LucBerge Sep 20, 2022

Ryan5453 Sep 23, 2022

LucBerge Sep 23, 2022

LucBerge Sep 23, 2022

gamer191 commented Sep 18, 2022 •

edited

LucBerge commented Sep 20, 2022

LucBerge commented Oct 3, 2022

Ryan5453 left a comment

LucBerge commented Mar 8, 2023

Ryan5453 commented Mar 8, 2023

LucBerge commented Mar 8, 2023 •

edited

LucBerge Mar 8, 2023

pukkandan commented Mar 8, 2023 •

edited

LucBerge commented Mar 8, 2023

pukkandan commented Mar 8, 2023

coletdjnz commented Mar 8, 2023

LucBerge commented Mar 8, 2023

		@@ -0,0 +1,309 @@
		from .compat import compat_struct_unpack, compat_struct_pack

Fix deezer extractor #4935

Are you sure you want to change the base?

Fix deezer extractor #4935

Conversation

LucBerge commented Sep 15, 2022 • edited

Description of the pull request and other information

Before submitting this pull request I have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check one of the following options:

What is the purpose of my pull request?

LucBerge commented Sep 15, 2022

pukkandan commented Sep 15, 2022

LucBerge commented Sep 15, 2022 • edited

Grub4K commented Sep 15, 2022

LucBerge commented Sep 15, 2022

pukkandan Sep 16, 2022

Choose a reason for hiding this comment

LucBerge Sep 20, 2022

Choose a reason for hiding this comment

Ryan5453 Sep 23, 2022

Choose a reason for hiding this comment

LucBerge Sep 23, 2022

Choose a reason for hiding this comment

LucBerge Sep 23, 2022

Choose a reason for hiding this comment

gamer191 commented Sep 18, 2022 • edited

LucBerge commented Sep 20, 2022

LucBerge commented Oct 3, 2022

Ryan5453 left a comment

Choose a reason for hiding this comment

LucBerge commented Mar 8, 2023

Ryan5453 commented Mar 8, 2023

LucBerge commented Mar 8, 2023 • edited

LucBerge Mar 8, 2023

Choose a reason for hiding this comment

pukkandan commented Mar 8, 2023 • edited

Footnotes

LucBerge commented Mar 8, 2023

pukkandan commented Mar 8, 2023

coletdjnz commented Mar 8, 2023

LucBerge commented Mar 8, 2023

LucBerge commented Sep 15, 2022 •

edited

LucBerge commented Sep 15, 2022 •

edited

gamer191 commented Sep 18, 2022 •

edited

LucBerge commented Mar 8, 2023 •

edited

pukkandan commented Mar 8, 2023 •

edited