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.

Sign up for GitHub

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

Jump to bottom

More output formats #1244

Merged
merged 34 commits into from
May 24, 2021
Merged

More output formats #1244

merged 34 commits into from
May 24, 2021

Conversation

xnetcat
Copy link
Member

@xnetcat xnetcat commented Apr 1, 2021

Convert downloaded songs to user-specified format (m4a, mp3, ogg, flac, opus) with the --output-format parameter and tag them accordingly.

@xnetcat xnetcat requested review from aklajnert, Silverarmor and a user April 1, 2021 15:38
@xnetcat xnetcat changed the base branch from master to dev April 1, 2021 15:39
@KLVN KLVN mentioned this pull request Apr 4, 2021
Closed
@Silverarmor Silverarmor added the Enhancement Enhancing spotDL label Apr 4, 2021
aklajnert
aklajnert requested changes Apr 11, 2021
View reviewed changes
Copy link
Contributor

@aklajnert aklajnert left a comment

Choose a reason for hiding this comment

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

Can you also rename arguments for internal functions to follow PEP-8?

spotdl/download/downloader.py Outdated Show resolved Hide resolved
spotdl/download/downloader.py Outdated Show resolved Hide resolved
spotdl/download/downloader.py Outdated Show resolved Hide resolved
spotdl/download/embed_metadata.py Outdated Show resolved Hide resolved
@xnetcat
Copy link
Member Author

xnetcat commented Apr 11, 2021

Checks are failing for some reason

@xnetcat xnetcat requested a review from aklajnert April 29, 2021 14:44
@xnetcat
Copy link
Member Author

xnetcat commented Apr 29, 2021

I don't know how to fix tests... I didn't really change that much code since last successful ci check

@aklajnert
Copy link
Contributor

I don't know how to fix tests... I didn't really change that much code since last successful ci check

I think you should patch out set_id3_data function, as it seems to fail when the file is empty, which is the case in tests.

@xnetcat
Copy link
Member Author

xnetcat commented Apr 30, 2021 •
edited

I don't know how to fix tests... I didn't really change that much code since last successful ci check

I think you should patch out set_id3_data function, as it seems to fail when the file is empty, which is the case in tests.

It's already patched https://github.com/xnetcat/spotify-downloader/blob/more-output-formats/tests/test_downloader.py#L76. I've also tried patching other functions from embed_metadata.py but it didn't work

@aklajnert
Copy link
Contributor

It's already patched https://github.com/xnetcat/spotify-downloader/blob/more-output-formats/tests/test_downloader.py#L76. I've also tried patching other functions from embed_metadata.py but it didn't work

Right. Read this article to understand why it doesn't work, you can learn something useful from it.

To make it work, you need to patch set_id3_data from downloader module, not embed_metadata.

@Silverarmor
Copy link
Member

Silverarmor commented May 24, 2021 •
edited

@aklajnert please re-review, you have requested changes

I'm satisfied the changes have been made, I'm merging this PR as for 3.6.0

@Silverarmor Silverarmor merged commit c879885 into spotDL:dev May 24, 2021
@Silverarmor Silverarmor mentioned this pull request May 24, 2021
Closed
@xnetcat xnetcat deleted the more-output-formats branch May 24, 2021 22:11
Silverarmor added a commit that referenced this pull request May 25, 2021
@Silverarmor @xnetcat
@phcreery @AZMCode @aiden2480 @bovine3dom @aklajnert
Merge pull request #1291 from spotDL/dev
7e69cd7 
Publish v3.6.0

* ignore .cache and other hidden files (#1274)

* Bump minimal required python version to 3.6.1 (#1278)

* Remove FFmpeg normalization causing "quiet" songs. (#1276)

* Saved Songs Download and User Authentication (#1240)

* regenerate cassettes (#1290)

* Use ffmpeg_path to check for version (#1289)

* Skip already downloaded songs before doing youtube search (#1287)

* Fix security risk (#1285)

* Song matching improvements (#1279)

* Artist songs fixes (#1284)

* More output formats (#1244)

* Bump version number to 3.6.0

* Update .gitignore to remove duplicate cache

* docs update (#1293)


Co-authored-by: Silverarmor <23619946+Silverarmor@users.noreply.github.com>
Co-authored-by: Jakub Kot <42355410+xnetcat@users.noreply.github.com>
Co-authored-by: Peyton Creery <44987569+phcreery@users.noreply.github.com>
Co-authored-by: AZMCode <adrianozambrana@protonmail.com>
Co-authored-by: Aiden Gardner <19619206+aiden2480@users.noreply.github.com>
Co-authored-by: Oliver Blanthorn <freedom4cows@gmail.com>
Co-authored-by: Andrzej Klajnert <github@aklajnert.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Silverarmor Silverarmor Silverarmor left review comments

@aklajnert aklajnert Awaiting requested review from aklajnert

Assignees
No one assigned
Labels
Enhancement Enhancing spotDL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@xnetcat @aklajnert @Silverarmor