tattwamasi / TeslaTunes

Copy your iTunes library, automatically converting Apple Lossless to flac, to a destination for use with your Tesla Model S
https://teslatunes.loci.net
43 stars 5 forks source link

Embed Album Art Cover #10

Closed chriseberlein closed 7 years ago

chriseberlein commented 7 years ago

Hey. First of all thanks for your perfect work, but have ever thought about including Album Art Cover? I began to implement it for PNG and will soon commit it as a branch. Feel free to include it in your Project.

tattwamasi commented 7 years ago

You're welcome and thanks for the thanks. Far from perfect, but it scratched an itch /shrug.

Subject to usefulness after considering the below, I'd be happy to include it after review if you implement it (and of course you are free to do whatever you want non-commercially with all this, subject to tag, Flac, etc. library licenses).

That said, did Tesla change their album art behavior? I thought they ignored embedded album art and looked it up (badly in some cases) on their own, then cached their results. This may have changed in the more recent media player.

Also, while I didn't myself go out and find then embed album art if it isn't there, I thought I copied over any album art in stuff I converted from apple lossless to Flac, and of course just copied the files (with optional name changes, etc.) in the other cases. If that's not the case for apple lossless to Flac, it should be and I'd welcome implementations.

tattwamasi commented 7 years ago

I looked into this a bit, and see where my misunderstanding was. I do take arbitrary file properties and attempt (naively, for the most part) to map them from apple lossless to Flac, but album art is not exposed in the same way. I do see what I should do, so I'll look at getting this implemented soon. If @chriseberlein has some of this done, I'm happy to look at that code, otherwise will take it on myself, maybe see how mp3tag and others do it.

chriseberlein commented 7 years ago

flac_utils 2.txt I've done some coding but only for PNG images. Feel free to include all you can need.

tattwamasi commented 7 years ago

Thanks. The PNG case is working? This is helpful, even if not complete, in that it validates how I was planning to read the cover art from the m4a files and write to the FLAC files, in terms of taglib and FLAC library calls.

chriseberlein commented 7 years ago

Yes the png case is working for me. Although it shows not the album cover in Finder, but in the car.

Den 23. feb. 2017 kl. 19.34 skrev tattwamasi notifications@github.com:

Thanks. The PNG case is working? This is helpful, even if not complete, in that it validates how I was planning to read the cover art from the m4a files and write to the FLAC files, in terms of taglib and FLAC library calls.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tattwamasi commented 7 years ago

I've begun working on it. I have a few unknowns though. I'll work through them but if any interested party knows the answers, I'll take them.

1) Can the Tesla handle both PNG and JPEG embedded cover art in the FLAC files? 2) Do Apple Lossless files ever get made through normal means with different types of art (PNG vs. JPEG), or do I really only need to support one? I expect to do both so far. 3) I'm only looking at copying over already embedded artwork, and only the first piece found per file. FLAC supports multiple pics with an actual type/purpose type field. MP4 also supports multiple pieces of artwork, but doesn't define what they would be (front cover vs. back vs. whatever). I doubt the Tesla would do anything but display the first / front cover one anyway. Anyone know?
4) There are other utils that will embed the artwork into the source files for you. For anyone worrying about this, I figure they are already doing that or won't mind using one of those tools. Is this a fair assumption? In theory (but would take substantially more work, complexity, likelihood of bugs) I could get the iTunes downloaded artwork and embed it in the new FLAC files (or at that point, offer it as an option on all destination files). I don't really want to take that on right now though. 5) It looks from sample code I find like there is a binary metadata block type for pics here that is used directly by the code, but when I look at docs here it clearly talks about the binary block getting base64 encoded and stuck within a VorbisComment with the tag name "METADATA_BLOCK_PICTURE". I haven't looked at the reference code deeply enough to see whether metaflac, @sbooth's code, the flac command line, etc. actually end up doing the base64 encoding and placing in a VorbisComment block at a level beneath my quick look - but doesn't seem like it. This seems important since only one VORBIS_COMMENT block (containing all the comments) is allowed in a stream

I'll try to get these things figured out in the next day or two, and get the feature added. May take longer though since it's been a long while since I've touched this at all, and I'll probably also end up updating the underlying frameworks/libraries while I'm at it, some of which were … finicky… to get to build or have enough changes to cause some other changes to my code.

tattwamasi commented 7 years ago

I pushed a barely tested (but hey, it worked once, on one file) quick update into the master branch, in case @chriseberlein or others want to test it a bit. It should support both png and jpeg embedded art. It won't take the non-embedded artwork iTunes displays and embed it.

My library doesn't seem to have actual embedded artwork for the apple lossless files I looked at, but I did go ahead and make one to test with and it seemed to work. Obviously it could use more testing.

@chriseberlein, note that I didn't actually decode the data as you and @sbooth did, but instead relied on the fact that the width/height/etc. fields are purely informational, and opted for the faster just plain copy of the image data after setting the correct mime type. If for some reason Tesla supported one but not the other format, and a lot of people had a lot of files with the other format, then converting would obviously be better, but unless that seems like an issue, I'd opt for doing less rather than more 😜

tattwamasi commented 7 years ago

I've made a new pre-release I'd appreciate anyone interested in this issue trying out before I put it up as a general update for everyone.

You can grab a zip with the app from the above release post, or directly from: https://github.com/tattwamasi/TeslaTunes/releases/download/1.2.4/TeslaTunes_1.2.4.zip

lraesly commented 7 years ago

I'll test it. I can analyze the album art with a couple of tag editors I have. Appreciate the work! I grabbed the code last night but struggled to compile it (I'm not a developer) so having the app is just what I need!

lraesly commented 7 years ago

I just tested a small 16 song sample. I used Metadatics to analyze the songs and then put them on a flash drive and loaded them into the Tesla. A couple of issues:

1) The track tags of the Apple Lossless songs are not being processed. So song name, album, artist, etc. are lost and what shows up in the Tesla is the file name. However, Genre shows as the playlist name. 2) The album art doesn't show in the Tesla for the flac songs but it does in Metadatics and another program called Tag Editor. I then entered the correct tag information (song name, etc.) and saved it to the flac song. I then tried it again in the Tesla and, for the songs where I added the tag data, the album art shows up. So my guess if the Tesla doesn't see the song/album/artist tag info it just skips the album art but I could be wrong on this.

Hope this helps.

lraesly commented 7 years ago

BTW, my images are jpeg format. And, using MediaInfo it does appear that the tags are missing from the flac files that are converted from Apple Lossless. And the images show up properly on the Tesla if the song tags are there.

tattwamasi commented 7 years ago

Regarding 1. Do you have 'strip tags in playlists' enabled? It is supposed to do exactly what you are saying. That's to keep the songs from showing up in the other listings.

Regarding 2. It might also be that the art was rewritten in a way that the Tesla likes.

To check, first make sure the strip tags option isn't on. Probably clear the other options too for good measure. Clear the destination directory and export again. Then, 1) are the tags back as expected? 2) does the album art show in the Tesla?

If the answers to both are yes, then cool. If not, then it'd be very helpful to get 3 copies of a file, the source file, one that I produced, one that works after you modified it.

tattwamasi commented 7 years ago

Just looked and recreated at least the tag portion of what you saw. So it wasn't the strip tags option. Looks like the update to taglib2 got in the way.

I'll get that fixed.

lraesly commented 7 years ago

Just double checked and agreed- no strip tags option. I have one lossless song where the album art is not processed, the others have artwork but no tags. MP4/MP3 are processed just fine. I can share the three if you need them via dropbox. Github is complaining about file size...

tattwamasi commented 7 years ago

Nah, something is clearly going on with the taglib2 update. My test files from the new cover code before the update show tags. Those after don't. I didn't look at the tests well enough.

No need for you to do more till I get this tracked down. Thanks for bringing it to my attention.

lraesly commented 7 years ago

Ok, and skip the one song without album art work. It was an issue with the original file.

tattwamasi commented 7 years ago

Found that issue. Was a dumb mistake on my part. In one of the few lines of code I had to update for the new taglib2 library, I checked to see if a tag was not empty before trying to do something with it. In the changed code, I accidentally reversed the logic, so it just wasn't copying any of the tags, other than the separately handled cover art.

I'll get an updated release made that'll correctly copy the tags.

Interestingly though, I don't think that should stop the art from showing in the Tesla. Will look forward to seeing what the car does with the files.

lraesly commented 7 years ago

Great! If you can post an updated app, I'll test it with the same files.

tattwamasi commented 7 years ago

Posted: https://github.com/tattwamasi/TeslaTunes/releases/download/1.2.5rc1/TeslaTunes_1.2.5rc1.zip

lraesly commented 7 years ago

Just tested on the original 16 song playlist and it worked flawlessly. I think that the Tesla media player is quirky. Once all of the tags were copied it recognized everything i.e. song name, album, artist, album art, etc.

I'm now testing on my full library but my expectation is that it will perform flawlessly.

Thanks!

lraesly commented 7 years ago

Everything looks great. Just finished converting 12,000 songs with 6,000 Apple Lossless to Flac files. Loaded them on to a flash drive and popped it into the Tesla. A little over 200 GB.

I browsed by Album and it looks like everything converted flawlessly. I also tried a few Flac songs (and standard mp3/4 and each one came up with Album Art.

Well done and thanks!

lraesly commented 7 years ago

BTW, it may help to re-post to Tesla Motors Club (maybe after the skip cloud song function is added?).

For any Tesla owner with a Mac and an appreciation of music, this is the "go to" program.

tattwamasi commented 7 years ago

Thanks for the testing! Out of curiosity, how did you get all your album art embedded into your files in the first place? Did you use doug's scripts, or some other util?

I want to do a test pass through the update code since I updated that component, then I'll push a release, update links, etc. and post a note on that forum thread.

lraesly commented 7 years ago

I use a couple of programs to embed the album art, clean up my library, etc. but in the end, it is a labor of love as nothing is completely automated or works consistently.

1) I start with MPFreaker (odd name) that will search for missing tag information and album art. It also has an embed album art function that works (most of the time but not every time) to embed the iTunes cached artwork into the file. I can sort by Album Art and then select groups of files to either lookup the artwork and/or embed iTunes artwork. 2) Metadatics allows you to quickly change tags and artwork for groups of files. It also has a pretty convenient art lookup function. Handy for changing a group of songs from an album or a group of tags.
3) Finally, Song Sergeant (odd name and graphics but it works well) will help clean up your iTunes library, identify duplicates, orphans, inconsistent titles etc.

I've heard good things about dbpoweramp but haven't used it. It has both a Windows and Mac version but I think the Windows version gets priority.

There may be better programs or newer ones but I've used these for some time now and the combination works well for me.

tattwamasi commented 7 years ago

Closing as handled in the current release, 1.2.6