Skip to content

Conversation

@MasterFocus
Copy link
Contributor

Hello,

First of all, thanks in advance for this great tool. 😄
I'm really happy with all the work you and others have put into this and I hope my commits may be helpful.

Before going any further, it is worth mentioning how some special Bandcamp pages seem to work:

....bandcamp.com/releases    ==>    mirrors the page of the artist's latest album
....bandcamp.com/music       ==>    shows a grid with all albums of the band
....bandcamp.com/merch       ==>    shows a bunch of merchandise

The main URL (....bandcamp.com) usually mirrors one of those.

Here's an example where /releases is mirrored: https://mothermooch.bandcamp.com/
Here's an example where /music is mirrored: https://mississippibones.bandcamp.com/
(examples checked as of 2015/October/31 - may differ in the future)
(I can't find any examples right now, but I'm 100% sure some bands choose to mirror /merch)

While testing SoundScraper, I couldn't download all albums from a certain band.
This band's main URL was mirroring /music, so it couldn't be parsed correctly.
SoundScraper would also fail for any artists mirroring /merch, for obvious reasons.

So, these are my main proposed additions with this Pull Request:
[1] The /music page is now accessed when only the artist's username is provided
[2] The /music page is now parsed, recursively calling the relevant method for each album URL

This only introduces a small bug: the limit of tracks is applied for each album separately.
It's like "num_tracks" actually means "num_tracks_per_album".
(if num_tracks=2 and the band has 5 albums, you're going to download 10 tracks)
I'm sure it's an easy fix, but I didn't have time to give it a closer look. 😐

I'm kinda busy and had to use the browser editor to commit my changes, so I may have messed up the indentation. Sorry. 😑

Best regards,

Antonio

The Mixcloud section was commented as "Bandcamp". Also added more #'s for better visual distinction. 😁
When only the artist's username is provided, make sure we always visit the /music page instead of whatever the main page redirects us to (usually redirects to the /releases or the /merch page).
If the JSON routine fails, the new code attempts to parse the URL as a /music page, trying to return a list of album URLs.
The calling method now recursively downloads each album if the new code successfully returns a list.
Small fix regarding the saved files when using "-f".
Small note added to the Bandcamp section, regarding the newly introduced bug when using "-n".
@Miserlou
Copy link
Owner

Miserlou commented Nov 3, 2015

Hey MasterFocus!

Thanks for the PR! I'm happy to accept this update, although it seems like you have introduced some formatting errors which cause the tests to fail.

Can you please run your changes through PEP8ify and make sure that the tests pass? It'd also be great if you could write a test for the specific case you are adding support for.

Thanks!

@MasterFocus
Copy link
Contributor Author

This is indeed my first PR ever on GitHub, so I just noticed the indentation problem. I'll try to fix that ASAP. I'll also try to add tests as you mentioned in the near future. Once again, thanks for your work (and your patience 😅)!

Reminder: using the browser editor is usually a bad idea 😑
Miserlou pushed a commit that referenced this pull request Nov 3, 2015
Allow Bandcamp to parse and download all albums
@Miserlou Miserlou merged commit c134155 into Miserlou:master Nov 3, 2015
@Miserlou
Copy link
Owner

Miserlou commented Nov 3, 2015

Merged, and published as version 0.20.0

Thank you for your contribution! :D

@Miserlou
Copy link
Owner

Miserlou commented Nov 3, 2015

This has already been useful for me today!! Thanks!

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