Grab cover art

[thomas-mc-work]

I'd love to see whipper to automatically grab the album art from musicbrainz like beets with the FetchArt plugin is able to. This step shouldn't be to hard as whipper already detects the correct musicbrainz album ID und thus the corresponding URL. As Beets is also written in Python it's probably possible to get the source code from there.

[JoeLametta]

Good idea!
It will probably be implemented as a non default feature.

[pieqq]

Is it better to extract the cover art as a separate cover.jpg file in the album directory, or to insert it in each audio file?

[IvanDSM]

Is it better to extract the cover art as a separate cover.jpg file in the album directory, or to insert it in each audio file?

I'd recommend writing it as a cover.jpg/folder.jpg, since adding it to every file ends up bloating file sizes a bit.

[RecursiveForest]

The MB disc ID is output on both stdout and in the .log file-- it should be pretty trivial to write a small script that pulls cover art into the folder and (optionally) embeds it in each file.

[JoeLametta]

I'd recommend writing it as a cover.jpg/folder.jpg, since adding it to every file ends up bloating file sizes a bit.

👍

The MB disc ID is output on both stdout and in the .log file-- it should be pretty trivial to write a small script that pulls cover art into the folder and (optionally) embeds it in each file.

Are you suggesting not to add this feature to whipper?

[MerlijnWajer]

This is a tricky one. In general, whipper's metadata support is only automatic (there's no way to manually fill this in). What I am questioning here is the current state of the metadata features, and more specifically, where we want to go to. Do we want to add gracenote and freedb as well? Or do we want to separate this step?

For a real 'accurate' rip of the CD, this metadata is not required - I think. The only thing that is required, perhaps, is the CDTEXT? (Which we currently do not store, AFAIK). Although it is in the cdrdao output.

I would say this may not be a high priority right now, unless it blocks acceptance of whipper in certain projects/websites.

[MerlijnWajer]

By 'accurate' rip of the CD I mean: a bit exact copy of the original CD

[pieqq]

I understand the most important part of the rip is to get the accuracy of the content of the disc, but I'm always annoyed when I have a beautifully ripped disc that does not contain a simple covert art (or proper metadata). I don't understand how people who would go all these extra steps to provide an accurate rip don't care about the last part of the process, but I guess that's just me being anal :)

[ArchangeGabriel]

Because if they really care, they use something else after ripping. :p IMHO, whipper is not supposed to be a music library organizer, just an accurate ripper.

Personally, I used to work with ExFalso for that purpose, but will soon replace it with beets once the required feature for my workflow have landed.

I definitively don’t want whipper to try doing all this on his own, the current behaviour is perfectly fine (for my purpose mbids would even be enough).

I’ll not go against an optional feature to fetch cover art, but strongly encourage people to look at beets if they care about metadata and all the fancy things. ;)

[Freso]

I kind of agree that this might be one step on the path to feature bloat. Maybe take a good hard look at what you want whipper to be ("have a mission"), and if that includes being a bit more all-purpose, then adding an option to fetch the frontiest image from Cover Art Archive should be fairly straightforward (since both speak in MBIDs).

Do we want to add gracenote and freedb as well?

FreeDB's data is bad, and Gracenote is the reason MusicBrainz and FreeDB exist in the first place, as Gracenote were the ones who bought CDDB all those years ago and proceeded to close off all of their contributors' free contributions behind a paywall. Please don't even consider using a proprietary source like that in a FLOSS project. They're the antithesis of free, libre, and open.

[pieqq]

I kind of agree that this might be one step on the path to feature bloat.

Since my previous comment, I had a look at beets. I was shocked at how bloated this became (hell, it's now a full-featured HTML5 music player?!), so it made me think more and I don't think I want whipper to become like that. To me, a good software follows the UNIX approach of doing one thing, but doing it right.

The cover-art-fetching process can be automated with other software, so it might not be needed inside whipper.

[ArchangeGabriel]

Well at least they made it very modular, but I agree that in my opinion beets shouldn’t be a music player.

I’m glad to see we all agree whipper wouldn’t be a good init system. ;p

[IvanDSM]

@Freso brings up very good points. After thinking about those points, I think we should indeed keep whipper slim and true to the UNIX style (as @Pierrrrrrre said), and therefore not add this feature.

I’m glad to see we all agree whipper wouldn’t be a good init system. ;p

😆

[JoeLametta]

So, what's the summary of this issue report: do you prefer not to see this feature added to whipper or ... what?

[ArchangeGabriel]

Well, excepted for the OP that said nothing since its OP, everyone here is against this feature in whipper.

[thomas-mc-work]

In my opinion this wouldn't break any barrier because whipper already does more than just ripping which is grabbing the metadata. The cover art is just one additional information. I agree @Pierrrrrrre with stating that this is/should be a part of the whole process. Making this feature optional wouldn't annoy anyone in his well known workflow.

In the end I would also be happy by refusing this change because I can understand the fears for ending up with too many features beyond the core ripping process.

[MerlijnWajer]

I would like to delay the change until we get a "1.0" release. Then we can figure out where to go from here wrt metadata. Let's first get a 1.0 release, with links to a fixed cdparanoia, with downloadable releases, and so on.

[thomas-mc-work]

I think it's about time for a 1.0 release :-)

[MerlijnWajer]

When we have fetched the picture, using mutagen.flac.{FLAC,Picture} should make embedding the pictures relatively easy:

def make_flac_picture(image_filename):
    """Given a path to a jpeg or png file, return a flac.Picture

    Returns: a valid flac.Picture or None
    """
    if not image_filename:
        return None

    im = Image.open(image_filename)
    if im.format == u'JPEG':
        mime = u'image/jpeg'
    elif im.format == u'PNG':
        mime = u'image/png'
    else:
        return None         # we only support png and jpeg

    pic = Picture()         # flac.Picture
    with open(image_filename, 'rb') as f:
        pic.data = f.read() 

    pic.type = 3            # id3.PictureType.COVER_FRONT
    pic.mime = mime 
    pic.width, pic.height = im.size
    if im.mode == u'P':     # 8-bit pixels, mapped using color palette
        pic.colors = 256
    elif im.mode == u'RGB': # 3x8
        pic.depth = 24
    elif im.mode == u'RGBA':# 4x8
        pic.depth = 32
    else:
        return None         # unsupported mode

    return pic

And then just use something like:

    w = FLAC(flac_filename)
    if flac_pic:
        w.add_picture(flac_pic)

[thomas-mc-work]

Why only in milestone 2.0? Is it too late for 1.0?

[JoeLametta]

Why only in milestone 2.0? Is it too late for 1.0?

It wasn't a road blocker for 1.0 (so it was scheduled for 2.0) but it has been completed and already merged in develop so I've rescheduled it for milestone 1.0. 😉