beetbox / mediafile

elegant audio file tagging
http://mediafile.readthedocs.io/
MIT License
100 stars 26 forks source link

Add AcousticBrainz fields #59

Closed JOJ0 closed 2 years ago

JOJ0 commented 2 years ago

Add tag mappings to AcousticBrainz fields as discussed in beetbox/beets issue #3928.

sampsyo commented 2 years ago

Great; thanks for getting this started! To answer some questions from https://github.com/beetbox/beets/issues/3928#issuecomment-1014280384:

But what are the things like MP3DescStorageStyle and MP4StorageStyle and ASFStorageStyle?

These storage styles are format-specific (as all (most?) storage styles are). So:

Should I put the whole rather long "path" in there as well or is it sufficient to shorten stuff like I did in this example (leaving out AB, LO and DANCEABILITY)

Unfortunately, there is no general answer to this question… every format has its own weird quirks, and the only goal is to be compatible with what other software chooses to do. So figuring out what to do here typically involves a mixture of comparison with other tools' source code, guesswork, and creativity.

JOJ0 commented 2 years ago

Great; thanks for getting this started! To answer some questions from beetbox/beets#3928 (comment):

Sorry, I should have put my questions into this PR directly. Was not the best decision.

But what are the things like MP3DescStorageStyle and MP4StorageStyle and ASFStorageStyle?

These storage styles are format-specific (as all (most?) storage styles are). So:

  • MP3DescStorageStyle is the style used for MP3/ID3 tags that are disambiguated by the desc field of a more general ID3 frame type.
  • MP4StorageStyle is for MPEG-4 metadata (common for AAC music).
  • ASFStorageStyle is for ASF/WMA (Windows Media) files.

In the meantime reading some more MediaFile code I think I figured out what MP4... and ASF... where, and it's obvious. They inherit features of StorageStyle and extend it to be represent a specific format. Thanks a lot for your explanations. Clarifys!

I need to read a little more about ID3 formats to fully understand your answer about mp3. Will do that later today.

Should I put the whole rather long "path" in there as well or is it sufficient to shorten stuff like I did in this example (leaving out AB, LO and DANCEABILITY)

Unfortunately, there is no general answer to this question… every format has its own weird quirks, and the only goal is to be compatible with what other software chooses to do. So figuring out what to do here typically involves a mixture of comparison with other tools' source code, guesswork, and creativity.

LOL, saved my day :-))) Alright, fun aside I think I understand what my priorities should be! Thanks!

Also, I think we need @wargreen over here, he might have some answers. In the meantime I will do some more experiments with Picard. I could just see how tags on WMA and MP4 files end up being tagged with it. Got some ALAC files lying around somewhere I think.

JOJ0 commented 2 years ago

After reading your nicely written docstrings and sphinx docs of mediafile @sampsyo, digging into mutagen docs a little, especially some very interesting but hard to read design docs about id3v2/2.3/2.4, playing around with MP4 files and Picard, recalling that I used mp3tag regularly a few years ago to do all my tagging, giving Puddletag a try, deciding that mediafile is not helping me find out what tags "really" look like, I think I learned a lot but still are struggling.

The one thing I figured out: I finally found a tool that does help me a little to see what's behind the scenes: mutagen-inspect.

Let's focus on one format at a time here:

Here is a diff of an MP4/ALAC file before and after tagging with Picard. The File was not tagged with beets. I think back then I used mp3tag and the Discogs source plugin:

(beets) jojo@gin ~/Music/Techno $ diff -u <(mutagen-inspect 031-spin_14-04/Answer\ Code\ Request\ -\ Breathe\ Ep\ \[O-TON\ 075\]\ \(2014\)\ ALAC\ ORIG/01\ -\ Answer\ Code\ Request\ -\ The\ 4th\ Verdict.m4a) <(mutagen-inspect 031-spin_14-04/Answer\ Code\ Request\ -\ Breathe\ Ep\ \[O-TON\ 075\]\ \(2014\)\ ALAC/01\ -\ Answer\ Code\ Request\ -\ The\ 4th\ Verdict.m4a)
--- /dev/fd/63  2022-01-20 08:44:45.000000000 +0100
+++ /dev/fd/62  2022-01-20 08:44:45.000000000 +0100
@@ -1,26 +1,127 @@
--- 031-spin_14-04/Answer Code Request - Breathe Ep [O-TON 075] (2014) ALAC ORIG/01 - Answer Code Request - The 4th Verdict.m4a
+-- 031-spin_14-04/Answer Code Request - Breathe Ep [O-TON 075] (2014) ALAC/01 - Answer Code Request - The 4th Verdict.m4a
 - MPEG-4 audio (ALAC), 337.10 seconds, 934197 bps (audio/mp4)
+----:com.apple.iTunes:ARTISTS=MP4FreeForm(b'Answer Code Request', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Discogs_Artist_Name=MP4FreeForm(b'Answer Code Request', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Discogs_Catalog=MP4FreeForm(b'oton-75', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Discogs_Country=MP4FreeForm(b'Germany', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Discogs_Date=MP4FreeForm(b'2014-02-00', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Discogs_Release_ID=MP4FreeForm(b'5423397', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:LABEL=MP4FreeForm(b'Ostgut Ton', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MEDIA=MP4FreeForm(b'Digital Media', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Not acoustic', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Not aggressive', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Electronic', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Not happy', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Party', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Not relaxed', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MOOD=MP4FreeForm(b'Not sad', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Mediatype=MP4FreeForm(b'Vinyl 12", EP', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Album Artist Id=MP4FreeForm(b'e7583cdd-9548-467f-aa12-ea9882af7b92', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Album Id=MP4FreeForm(b'a9a50244-0ccd-4a9a-b871-8933bd8489f4', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Album Release Country=MP4FreeForm(b'US', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Album Status=MP4FreeForm(b'official', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Album Type=MP4FreeForm(b'single', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Artist Id=MP4FreeForm(b'e7583cdd-9548-467f-aa12-ea9882af7b92', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Release Group Id=MP4FreeForm(b'51da3ad5-7eb2-4fcb-b637-0909616e229d', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Release Track Id=MP4FreeForm(b'5108c9b5-f0a4-4835-8d1d-02b5415b053a', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:MusicBrainz Track Id=MP4FreeForm(b'e8bcd28d-2b3b-4534-9a8c-971c8ae24573', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Publisher=MP4FreeForm(b'Ostgut Ton', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:SCRIPT=MP4FreeForm(b'Latn', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:Style=MP4FreeForm(b'Techno', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:danceability:danceable=MP4FreeForm(b'0.999999880791', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:danceability:not danceable=MP4FreeForm(b'1.00000008274e-07', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:gender:female=MP4FreeForm(b'0.490105032921', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:gender:male=MP4FreeForm(b'0.509894967079', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:alternative=MP4FreeForm(b'1.71346326017e-10', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:blues=MP4FreeForm(b'1.84553025639e-10', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:electronic=MP4FreeForm(b'0.999991714954', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:folk/country=MP4FreeForm(b'2.45273639621e-07', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:funk/soul/rnb=MP4FreeForm(b'4.72483741021e-08', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:jazz=MP4FreeForm(b'6.91388504492e-06', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:pop=MP4FreeForm(b'4.87628213364e-08', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:rap/hiphop=MP4FreeForm(b'1.98116740791e-08', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_dortmund:rock=MP4FreeForm(b'1.02764363419e-06', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_electronic:ambient=MP4FreeForm(b'0.0720595642924', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_electronic:drum and bass=MP4FreeForm(b'0.0121761728078', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_electronic:house=MP4FreeForm(b'0.120231315494', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_electronic:techno=MP4FreeForm(b'0.619988739491', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_electronic:trance=MP4FreeForm(b'0.175544187427', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:classical=MP4FreeForm(b'0.00777373136953', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:dance=MP4FreeForm(b'0.260708212852', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:hiphop=MP4FreeForm(b'0.481227725744', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:jazz=MP4FreeForm(b'0.0165352486074', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:pop=MP4FreeForm(b'0.0264916606247', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:rhythm and blues=MP4FreeForm(b'0.183025106788', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:rock=MP4FreeForm(b'0.0136867687106', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_rosamerica:speech=MP4FreeForm(b'0.0105515616015', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:blues=MP4FreeForm(b'0.0620671063662', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:classical=MP4FreeForm(b'0.034474901855', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:country=MP4FreeForm(b'0.103433981538', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:disco=MP4FreeForm(b'0.0620971247554', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:hiphop=MP4FreeForm(b'0.15527240932', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:jazz=MP4FreeForm(b'0.310666948557', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:metal=MP4FreeForm(b'0.044346679002', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:pop=MP4FreeForm(b'0.0620793737471', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:reggae=MP4FreeForm(b'0.0620816536248', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:genre_tzanetakis:rock=MP4FreeForm(b'0.103479810059', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:chachacha=MP4FreeForm(b'0.246445417404', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:jive=MP4FreeForm(b'0.104635164142', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:quickstep=MP4FreeForm(b'0.038626562804', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:rumba-american=MP4FreeForm(b'0.0469643101096', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:rumba-international=MP4FreeForm(b'0.0729862898588', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:rumba-misc=MP4FreeForm(b'0.0556916445494', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:samba=MP4FreeForm(b'0.227213978767', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:tango=MP4FreeForm(b'0.131457850337', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:viennesewaltz=MP4FreeForm(b'0.0515687130392', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:ismir04_rhythm:waltz=MP4FreeForm(b'0.0244100671262', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_acoustic:acoustic=MP4FreeForm(b'1.29250798953e-09', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_acoustic:not acoustic=MP4FreeForm(b'1', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_aggressive:aggressive=MP4FreeForm(b'0.0348894521594', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_aggressive:not aggressive=MP4FreeForm(b'0.96511054039', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_electronic:electronic=MP4FreeForm(b'0.986752927303', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_electronic:not electronic=MP4FreeForm(b'0.0132470680401', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_happy:happy=MP4FreeForm(b'0.199092701077', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_happy:not happy=MP4FreeForm(b'0.800907313824', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_party:not party=MP4FreeForm(b'0.060668040067', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_party:party=MP4FreeForm(b'0.939331948757', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_relaxed:not relaxed=MP4FreeForm(b'0.653021454811', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_relaxed:relaxed=MP4FreeForm(b'0.346978545189', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_sad:not sad=MP4FreeForm(b'0.805326104164', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:mood_sad:sad=MP4FreeForm(b'0.194673895836', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:moods_mirex:aggressive, fiery, tense/anxious, intense, volatile, visceral=MP4FreeForm(b'0.621267855167', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:moods_mirex:humorous, silly, campy, quirky, whimsical, witty, wry=MP4FreeForm(b'0.0473020523787', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:moods_mirex:literate, poignant, wistful, bittersweet, autumnal, brooding=MP4FreeForm(b'0.203447043896', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:moods_mirex:passionate, rousing, confident, boisterous, rowdy=MP4FreeForm(b'0.0774111151695', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:moods_mirex:rollicking, cheerful, fun, sweet, amiable/good natured=MP4FreeForm(b'0.0505719520152', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:timbre:bright=MP4FreeForm(b'0.0618188902736', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:timbre:dark=MP4FreeForm(b'0.938181102276', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:tonal_atonal:atonal=MP4FreeForm(b'0.999999940395', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:tonal_atonal:tonal=MP4FreeForm(b'4.45173071739e-08', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:voice_instrumental:instrumental=MP4FreeForm(b'0.906327426434', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:hi:voice_instrumental:voice=MP4FreeForm(b'0.0936725661159', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:rhythm:bpm=MP4FreeForm(b'130.717025757', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:tonal:chords_changes_rate=MP4FreeForm(b'0.0676122307777', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:tonal:chords_key=MP4FreeForm(b'C#', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:tonal:chords_scale=MP4FreeForm(b'major', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:tonal:key_key=MP4FreeForm(b'A#', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:ab:lo:tonal:key_scale=MP4FreeForm(b'major', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:energylevel=MP4FreeForm(b'6', <AtomDataType.UTF8: 1>)
 ----:com.apple.iTunes:initialkey=MP4FreeForm(b'Gm', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:originaldate=MP4FreeForm(b'2014-02-24', <AtomDataType.UTF8: 1>)
+----:com.apple.iTunes:originalyear=MP4FreeForm(b'2014', <AtomDataType.UTF8: 1>)
 ----:com.serato.dj:markers=MP4FreeForm(b'YXBwbGljYXRpb24vb2N0ZXQtc3RyZWFtAABTZXJhdG8gTWFya2VyczIAAgUAAAAOAAAAn///\n//8A/////wDMAAABAAABWCL/////AP////8AzIgAAQAAAZFj/////wD/////AAAAzAEAAAI9\nJP////8A/////wDMzAABAAACdmX/////AP////8AAMwAAQD//////////wAEGoRXAMwAAAMA\n//////////8ABBqEVwDMAAADAP//////////AAQahFcAzAAAAwD//////////wAEGoRXAMwA\nAAMA//////////8ABBqEVwDMAAADAP//////////AAQahFcAzAAAAwD//////////wAEGoRX\nAMwAAAMA//////////8ABBqEVwDMAAADAP//////////AAQahFcAzAAAAwAA////\n', <AtomDataType.UTF8: 1>)
 ----:com.serato.dj:markersv2=MP4FreeForm(b'YXBwbGljYXRpb24vb2N0ZXQtc3RyZWFtAABTZXJhdG8gTWFya2VyczIAAQFBUUZEVlVVQUFB\nQUFGUUFBQUFBQW53RE1BQUFBQUVWdVpYSm5lU0EwQUVOVlJRQUFBQUFWQUFFQUFWZ2lBTXlJ\nQUFBQVJXNWwKY21kNUlEVUFRMVZGQUFBQUFCVUFBZ0FCa1dNQUFBRE1BQUJGYm1WeVoza2dO\nZ0JEVlVVQUFBQUFGUUFEQUFJOUpBRE16QUFBCkFFVnVaWEpuZVNBMUFFTlZSUUFBQUFBVkFB\nUUFBblpsQUFETUFBQUFSVzVsY21kNUlEY0FRMVZGQUFBQUFCVUFCUUFDNk9ZQQp6QURNQUFC\nRmJtVnlaM2tnTndCRFZVVUFBQUFBRlFBR0FBTWlKZ0FBek13QUFFVnVaWEpuZVNBMkFFTlZS\nUUFBQUFBVkFBY0EKQTF0bkFJZ0F6QUFBUlc1bGNtZDVJRFlBCg==\n', <AtomDataType.UTF8: 1>)
 aART=Answer Code Request
-covr=[93192 bytes of data]
+covr=[65612 bytes of data]
+disk=(1, 1)
+soaa=Answer Code Request
+soar=Answer Code Request
 tmpo=131
-trkn=(1, 0)
+trkn=(3, 3)
 ©ART=Answer Code Request
-©alb=Breathe Ep
-©day=2014
-©gen=Techno
-©gen=Ambient
+©alb=Breathe
+©day=2014-02-24
+©gen=Electronic
 ©nam=The 4th Verdict - Gm - 131
 ©too=X Lossless Decoder 20131102, QuickTime 7.6.6

(beets) jojo@gin ~/Music/Techno $

So what we see here is that Picard adds AcousticBrainz things simply like that, no fancy "long" name is used:

----:com.apple.iTunes:ab:lo:tonal:chords_key

So please @sampsyo I need your advice again (sorry for needing so much guidance here and thanks again for your patience and time): Is it true that if I would set MP4StorageStyle to:

    chords_key = MediaField(
        MP3DescStorageStyle(u'Acousticbrainz Chords Key'),
        MP4StorageStyle(
            '----:com.apple.iTunes:ab:lo:tonal:chords_key
        ),
        StorageStyle('AB:LO:TONAL:CHORDS_KEY'),
        ASFStorageStyle('Acousticbrainz/Chords Key'),
    )

we would get the same result when tagging with mediafile?

sampsyo commented 2 years ago

Great! Yes, that looks exactly right for the MP4 key (and I'm glad it looks feasible to match Picard in this respect).

JOJ0 commented 2 years ago

Thanks! MP4 [x] DONE :-)

Brings us to MP3/ID3:

After reading https://id3.org/id3v2.4.0-frames and looking at your docstring again (https://github.com/beetbox/mediafile/blob/master/mediafile.py#L883) I finally understand what you ment by:

MP3DescStorageStyle is the style used for MP3/ID3 tags that are disambiguated by the desc field of a more general ID3 frame type.

We are talking simply about a TXXX frame, a user-defined field. I was confused about TXXX meaning something like T??? like most tags look like and not being a special kind of tag name. Well, fortunately I now have the very basics of ID3v2 down ;-)))

So I guess Picard is storing ab:hi:..... named TXXX frames similar to MP4, and voila, a test reveals exactly that (snippet of diff before/after tagging):

TXXX=ab:hi:danceability:danceable=0.999972999096
TXXX=ab:hi:danceability:not danceable=2.69713837042e-05

Alright so all clear with ID3 tags, will do like that.

So what's left is WMA files. I can't find one on the computer I'm coding right now. Probably tags will look exactley the same there but want to make sure. @wargreen if you know by heart it would speed up things, if not I can make tests later when I'm on my Linux computer, I recall having found something in WMA there recently.

wargreen commented 2 years ago

Hi, so i move my reply from the issue :

Hi @JOJ0 So cool if you can implement this feature ! I'm not sure to understand the question (my english is pretty bad). But what i can say : All the sum of probability should be equal to 1. So if we have only two entry, the most probabilistic value can deduct the other. We have store both in this case to have an unified way to store all tags, store all the AB data and maybe simplify the search into tag. Eg: if we want filter music with NO male, it can be simpler to search ab:hi:gender:male < 0.5. And maybe in the future may be added other genders (for example) ?

But i don't understand why store as Acousticbrainz/Danceable. Isn't it possible in some file format to store in the original format ? My tests in Picard doesn't show me that case (but maybe i don't do enough tests...).

About your last question, I should admit I've don't test the picard plugin on wma files... And i've completely forgot this format ! I've just now transcode a tagged flac file in wma, and the tags seem stay "AB:HI:DANCEABILITY:DANCEABLE", according the mediainfo output. I assume that Picard should directly write it in the same way. I can try it if you want, but i haven't a very deep knowledge of how tags are write in the different files formats...

JOJ0 commented 2 years ago

Hi @wargreen I assume wma files will have the tag as you describe. But to make sure you could tag your testfile again using Picard and then run mutagen-inspect filenameon it to definitely see what is produced then. Thanks for helping out! Appreciated!

@sampsyo again I need your advice. Have a look at my latest commit. My idea is to define all the available attributes as Picard tags them and then map to shorter descriptor names like we use with the beets acousticbrainz plugin. I am not sure if I'm still on track here...

The idea is to make all or most data AcousticBrainz delivers available to be tagged. Later the acousticbrainz beets plugin could be extended to write all or most of them to files if the user wants that/confiugres that using acousticbrainz:tags directive.

Also I am not sure where the mapping between MediaFile.attribute and beets database column name is made. I was assuming it is 1:1 mapping since it looks like it when I have a look into the db. Eg. mb_albumid is the name of the descriptor as well as the database column name. That all might be a coincidence an these mappings have nothing to do with how the attribute/descriptor is called in mediafile.py??

I am reading over and over again these parts in the plugin but am not sure if I understand what it does entirely:

https://github.com/beetbox/beets/blob/master/beetsplug/acousticbrainz.py#L27 https://github.com/beetbox/beets/blob/master/beetsplug/acousticbrainz.py#L235

sampsyo commented 2 years ago

Also I am not sure where the mapping between MediaFile.attribute and beets database column name is made. I was assuming it is 1:1 mapping

Yes, the names must be identical. In other words, the fields in the beets database must exactly match a MediaFile field name in order to be written to files; otherwise, they only live in the database. This is the line in beets where that happens, FWIW: https://github.com/beetbox/beets/blob/19e4f41a72ade8e69dfb43b38a6569985ecefd79/beets/library.py#L770

…so yes, in short, matching the names we use within the beets AB plugin is the thing to do.

wargreen commented 2 years ago

@JOJ0 I had a try on wma file. So Picard don't add any tag from AB. That is not intentional, and i don't see any information about why in the Picard log... A look via mediainfo or mutagen confirm that is not write... That is strange, ffmpeg can add it but PIcard... !? Do you think that our format of the AB tags isn't compliant with the wma tags ?

JOJ0 commented 2 years ago

@wargreen I don't think that this is the issue but I actually don't have any idea :-) The WMA tag also is just a string and with AB:XX, even the colon is just a regular character. I am sure you double checked this but does your installed Picard write any tags at all on WMA files?

What I did now is using the same scheme as with flac files: Upper case letters and the colon. Eg. AB:HI:XXX I could easily change this to the lower case version, as we have it when tagging mp3 files, if we later find out that (a fixed) Picard does that.

@wargreen and @sampsyo, have a look: https://github.com/beetbox/mediafile/commit/c06819932ace24548d3daa91dbf7c89bff137cdc

I tried to explain my decisions in the commit message. Criticism and improvement suggestions welcome.

One more thing I didn't write in the commit message. From a DJ point of view, regarding bpm and initial_key fields: Yes these are regular mp3 tags and it's good that AcousticBrianz plugin can write to them but in reality I personally would want to have the possibilty to save this information from several sourcs. For example the widely used tools "KeyFinder" and "Mixed in Key" write to these too. Usually they are more accurate than AcousticBrainz and I use them primarily. But I do want to have the AcousticBrainz key too because sometimes it's interesting to compare them and make up your own mind which is the more correct one, or even decide that both are kind of correct harmonically, or tho see that AB often decides for the parallel major key instead of the minor key (which technically is the same but different from a musicians point of view, duh!).

Maybe that's all too theoretical and my own interest but in short: Does it hurt if additional fields, ab_lo_key_key, ab_lo_key_scale are saved to each file and I don't have to decide which source should write to my one and only key tag (being the initial_key one)?

Similar thoughts on saving multiple bpm fields.

wargreen commented 2 years ago

@JOJ0 I've just get a try, and Picard write a lot of tags in .wma, but not the AB ones... I don't know why... About your commit message, what's the kind of field that use 2 fields in Picard and one in Beets ? And about the Key and BPM fields, in the PIcard plugin, we have an option in the plugin UI to add this (or not), in addition of the tags from AB:LO:TONAL:* and AB:LO:RHYTHM:BPM. So the "simples" fields can be get from another source.

JOJ0 commented 2 years ago

@wargreen for example danceability is 2 tags in picard and 1 in beets acousticbrains plugin. I added a comment to each of those in the mediafile.py code. have a look.

wargreen commented 2 years ago

@JOJ0 Is the tags with NOT are not stored ? EDIT: It seem to me that is more than NOT removed. All the tags with only two choice are simplified. On can be deducted from other so it is not really mandatory, but the tags don't reflect all the content of the AB data... And as i say earlier, what we do if the scheme of AB change with more than "feature | !feature" ? Can we admit the AB data stable and won't change in the future ? EDIT: Another question, from the usage point of view : How can we use theses tags if we can't be sure what the tag removed ? Eg : on software remove TONAL and another ATONAL. We need to do more tests and deduct the other. Without a strong specification about how simplify the tags it seem to be complicated to implement.

Open questions of course :)

JOJ0 commented 2 years ago

Hi @wargreen I am not sure if I understand entirely what you are trying to say. This PR is about supporting the underlying "tagging infrastructure" provided by the mediafile library to the already existing "acousticbrainz beats" plugin.

Have a look at this scheme, it is a good overview about what acousticbrainz api endpoints the plugin uses to fetch data: https://github.com/beetbox/beets/blob/master/beetsplug/acousticbrainz.py#L27

Furthermore this should gives you an idea of how the plugin can be used and again which data it provides to the beets user, but I am sure you know that page already :-) https://beets.readthedocs.io/en/stable/plugins/acousticbrainz.html

And yes you are right: The beets plugin only relies on the "positive" fields of e.g dancability/danceable and it just ignores the existence of danceability/not_danceable at all. Same for other fields with that concept.

I think for now I want to focus on supporting the "writing to actual file tags" with the beets plugin. This PR is the foundation for that. There is a lot of room for enhancement in the beets AB plugin but for now let's focus on getting the actual tagging to work with the fields the current state of the AB plugin uses.

JOJ0 commented 2 years ago

And a word about API stability. You can never know :-) If an API provider changes something....then it changes.....you have to adapt apps. It is what it is :-))) But most of the time API providers try to stay stable for a very long time and IF they really want to kick out something, they usually DEPRECATE an API endpoint and then finally remove it, during that time the app coders have time to adapt to the changes. So in short: I am not afraid of API changes.

But again I am not sure if I understood your concerns about AB API changes correctly? Sorry if this was not what you were talking about / you are worried about :-)

wargreen commented 2 years ago

Sorry, my english is... terrible ! I will try to be better ! I understand now that this PR is not focused on parsing the AB data, but on how to tag the files with the data grab by the AB plugin. And it's a very cool step, thank you ! It's a different logic in our Picard plugin. We've try to be transparent about the AB data, and tag with all features. One question still for me (i try to rephrase): At the user side, if we need to use for exemple NOT_DANCEABLE we need to test the existence of DANCEABLE, NOT_DANCEABLE and calculate the nonexistent value. Is it a good way ? And does your code can be extended if the AB plugin don't ignore some features in the future ? Another question : is it an issue in the AB plugin or a good choice in order to keep the tags simpler ?

I hope be clearer this time :)

JOJ0 commented 2 years ago

@sampsyo, please have a look at my latest commit. I shortened my proposed ab_ names since they are pretty long anyway. I am aware of, that to make this work together with the acousticbrainz beets plugin, it should be taken in account there.

From reading code, docs and experimenting a little already, I understand that only if add the fieldnames to Item._media_fields I can actually write tags to them? So I need to add them to this list: https://github.com/beetbox/beets/blob/19e4f41a72ade8e69dfb43b38a6569985ecefd79/beets/library.py#L460

Or is there a better way to add tagnames that are only used when a certain plugin is used?

What do you think in general about my idea to advance the acousticbrainz plugin to support accessing attributes within beets with the current names (eg. danceability) but also with more descriptive names (abdanceability)? It would tell the user that it actually is an acousticbrainz field, similar to all the musicbrainz fields having the mb prefix, discogs fields having discogs_, and so on. The ugliness of this would be that, to not break current things, the fieldnames in above Item._fields list would have to be added in both formats (e.g danceability and ab_danceability).

I would accept an answer like: "This is a bad idea and further complicates things" as well :-)

JOJ0 commented 2 years ago

Aaah I overread the not in a sentence here: https://beets.readthedocs.io/en/stable/dev/library.html#beets.library.Item.write

So I think using the tags arg here is better than hardcoding ab plug fields into Item._fields. That correct?

sampsyo commented 2 years ago

Thanks for pushing this along! It looks like a lot of work to gather all these tag mappings. Nice work!

On the central question of whether to use an ab_ prefix: my currently leaning is that we probably should not. The reason is that these attributes, while their inspiration comes from AcousticBrainz, could in principle be provided by all manner of data sources. For example, Spotify's "audio features" API also provides scores for attributes like danceability, liveness, and speechiness. In a way, the story is similar to other, more boring tags like the "artist" tag: the artist name could come from MusicBrainz, but for interoperability's sake, we like to give it a generic, source-independent meaning. So I think it makes sense to keep generic names for these attributes, even at the cost of some clarity on the user's side. We could separately revisit the question of whether we want to add a feature to the plugin to "alias" the fields with an ab_ prefix so they're easier to find, but I think that should be a separate feature. Does that argument make sense?

It might also be good to look at a few open-source projects beyond Picard to see if they agree. Here are a few tagging libraries I found with GitHub search, for example, that could be relevant: https://github.com/goxr3plus/jaudiotagger/blob/0f7296c25e52c5d3b07f9f1daf14260d9a4363fb/src/org/jaudiotagger/tag/id3/ID3v24FieldKey.java#L82 https://github.com/ealva-com/ealvatag/blob/c03f5d5b4fc22872c61fb3c3e34a61a37562f734/ealvatag/src/main/java/ealvatag/tag/id3/ID3v22Frames.java#L227 https://github.com/Marekkon5/onetagger/blob/4b1649303151900a838a291c540079554c65b31a/client/src/views/AudioFeatures.vue#L154-L155

JOJ0 commented 2 years ago

Hi, before I answer to your last post, let me share a "now that's cool" moment: http://www.jthink.net/jaudiotagger/tagmapping.html :-)

JOJ0 commented 2 years ago

I agree that having prefixes like ab_ for the beets internal field names (db names) should be a different feature for another time. I also agree that interoperability is key and I am looking at those libraries you mentioned now. And I also think that you are aware of that you kind of opened pandoras box with that. Up to now we were talking about acousticbrainz only fieldnames, like Picard does it (and I didn't mention it yet, but had in mind that the xtractor plugin from adamjakab, which does exactly the same thing but offline, should use those AB:... tags as well), but now we are in a whole different game with finding the right tag names. It's a good thing though :-)

I think the mood... tags is something that is widely used and beets does it equally to other tools already. Only that not all those mood.. tags are available with acousticbrainz (e.g. moodvalence, mood

Onetagger is a very cool tool (nice find! never heard about it!) but it entirely does it's own thing. Tags look like this:

TXXX=1T_ACOUSTICNESS=61
TXXX=1T_DANCEABILITY=61
TXXX=1T_ENERGY=83
TXXX=1T_INSTRUMENTALNESS=90
TXXX=1T_LIVENESS=9
TXXX=1T_POPULARITY=30
TXXX=1T_SPEECHINESS=5
TXXX=1T_TAGGEDDATE=2022-02-10 08:09:53
TXXX=1T_VALENCE=78
TXXX=ALBUMARTISTSORT=AFX
TXXX=AUDIO_FEATURES=#dance-med, #energy-med, #vocal-low, #neutral

The AUDIO_FEATURES tag is something reminding me of like Picard does the TXXX:MOOD tag: A summary of all the mood tags. I think MOOD is one of the more common tags and I also a future feature idea of mine would be to steal Picards code and implement this in the acousticbrainz plugin. Again something for another time.....

Back to AUDIO_FEATURES: I am not sure how we could use those numbers to write into the same tag as eg. the acousticbrainz-filled danceability tag, it's a different system I think. You sure this could be translated into something general? Haven't found the time to think about that closely and would like to leave it out for now.

I will try to commit something in the next days keeping those things in mind.

sampsyo commented 2 years ago

Huh, that's interesting—yeah, it looks like the system in Onetagger is completely different (as indicated by the fact that they have also namespaced all the fields using a 1T_ prefix). If that's the case in general—that is, that no other project exists with tags that correspond to AB fields but don't match the formats in Picard—then I think that means we should just match Picard. Thanks for investigating!

JOJ0 commented 2 years ago

Huh, that's interesting—yeah, it looks like the system in Onetagger is completely different (as indicated by the fact that they have also namespaced all the fields using a 1T_ prefix).

Further investigation reveals that onetagger is flexible. Have a look at some screenshots. Tags to write to can be freely chosen.

By "different system" I did not only mean the tag names but the "numbers system" they use. Do you know by heart if the numbers the Spotify audiofeatures api retrieves are comparable to the numbers acousticbrainz gets us? Sorry I was to lazy to investigate this in detail. What I want to say is: I am not sure if spotify and acousticbrainz values are compatible in this regard, thus my conclusion would be: If any numbers system is saved to actual file tags, then they are kind of "proprietary", ooooor beets does the job about translating it to something "general", but that all sounds like too much hassle. Easier would be to save audiofeatures values to other tags then acousticbrainz values.

Interoperability possiblities I've found out so far:

If that's the case in general—that is, that no other project exists with tags that correspond to AB fields but don't atch the formats in Picard—then I think that means we should just match Picard. Thanks for investigating!

Just a little more investigation necessary to really make decissions. I have a feeling that once things are decided they will last forever ;-)))

Screen Shot 2022-02-12 at 11 35 06 Screen Shot 2022-02-12 at 11 34 34 Screen Shot 2022-02-12 at 11 31 40 Screen Shot 2022-02-12 at 11 29 30 Screen Shot 2022-02-12 at 11 19 48 Screen Shot 2022-02-11 at 07 49 59

Screen Shot 2022-02-12 at 11 36 03

JOJ0 commented 2 years ago

Just a quick note to myself, with the initial idea and and some changes in acousticbrainz.py (simply adding to _fields, ignoring that there might be a better way), tags would look like this:

TXXX=ab:hi:mood_happy:happy=0.0850781202316
TXXX=ab:hi:mood_party:party=1.57784197654e-05
TXXX=ab:hi:mood_sad:sad=0.109234951437
TXXX=ab:lo:tonal:chords_changes_rate=0.08
TXXX=ab:lo:tonal:chords_key=A#
TXXX=ab:lo:tonal:chords_scale=minor

Questions: Does beets even understand and handle the sciencific notation (e-05 meaning *10^-5 IIRC) we see in eg mood_party?

Update: Stupid question, sure there will be ways, it's Python. And actually it's more of the question if any other tool trying to use this tag can handle it. And this only if ever any tool will support readng this tag...

JOJ0 commented 2 years ago

Playing around with Sonkong which uses the jaudiotagger java library for tagging:

It uses musicbrainz but also discogs (for genre, style mainly I think) at the same time to fetch info about a release. I can't figure out where it has the "mood" information and things like "electronic=87" or acoustic=3" from. It usess numbers between -100 and +100 it seems, which unfortunately also is something not compatible with what we get from acousticbrainz. So does it even make sense to use the same filetag names is what I am asking myself/us.

_Note that Sonkong is commercial, I can run it in "preview mode" but I can't actually see how the tags on the files would end up. So we can only educated guess that what is eg. called "Mood Aggressive" here will be saved in a TXXX MOODAGGRESSIVE tag since it's what we see in the jaudiotagger source code.

Screen Shot 2022-02-12 at 18 39 59 Screen Shot 2022-02-12 at 18 40 49 Screen Shot 2022-02-12 at 18 41 34

:

JOJ0 commented 2 years ago

@wargreen I haven't forgotten about you but you see that all this might go in a different/broader direction than just tagging 1:1 what acousticbrainz delivers and how it is called there. We might come back to the initial idea but I am not sure at the moment where this is going... :-)

JOJ0 commented 2 years ago

Found a thread from 2016/2017 which discusses where Songkong and Jaikoz (commercial tools both using the jaudiotagger library) got their "mood information" from: Surprise, surprise, it also uses AcousticBrainz. But they definitely don't just save the AB data as-is, they process it somehow to get their numerical and also human readable textual values. For details and a lot of blabla have a read here: https://community.jthink.net/t/questions-about-mood/7761/28

We can't know what they are doing exactly, it's closed source. My conclusion is that we shouldn't bother that they write to an id3 tag TXXX:MOOD_SOMETHING, it's something else, we are here to discuss how to name tags that save unprocessed acousticbrainz data.

Fun fact / interesting unrelated sidenote: Above thread reveals that Songkong uses an intermediate query source that combines Discogs and MusicBrainz via one API, interesting approach: http://www.albunack.net/

Another unrelated sidenote, learned from above thread, I want to share: Another commercial tool also using AcousticBrainz and processing data similar to SongKong: https://www.beatunes.com/en/beatunes-buy.html

Alright I feel a little overinformed now :-))

@sampsyo and @wargreen: What should we do with all this information? Reinvent the wheel? Go back to the initial idea and just tag AcousticBrainz data as Picard does (AB:HI:..., etc.)?

sampsyo commented 2 years ago

All very good questions! I also don't have a strong opinion, given all the diverging approaches taken in other tools. Maybe the best things to do would be: (1) forge ahead with Picard compatibility as the main goal, (2) start a broader conversation with the Picard and/or AcousticBrainz people to ask what they think a good long-term strategy would be? I'm guessing that the MusicBrainz IRC channel(s) might be the best way to do that latter thing…

JOJ0 commented 2 years ago

Finalizing this PR atm following your last suggestion to forge ahead with Picard compatibility and plan a conversation with Picard/AB devs.

I am using as_type=float with all the storage types to really save exactely as Picard does (actually I think it saves exactely the same number as AB gives us), this works fine everywhere except with MP4 storage type. Am I hitting a mutagen bug?

Sending event: write
Traceback (most recent call last):
  File "/Users/jojo/.venvs/beets/bin/beet", line 33, in <module>
    sys.exit(load_entry_point('beets', 'console_scripts', 'beet')())
  File "/Users/jojo/git/beets/beets/ui/__init__.py", line 1285, in main
    _raw_main(args)
  File "/Users/jojo/git/beets/beets/ui/__init__.py", line 1272, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/Users/jojo/git/beets/beetsplug/acousticbrainz.py", line 162, in func
    self._fetch_info(items, ui.should_write(),
  File "/Users/jojo/git/beets/beetsplug/acousticbrainz.py", line 245, in _fetch_info
    item.try_write(tags=tags_to_write)
  File "/Users/jojo/git/beets/beets/library.py", line 797, in try_write
    self.write(*args, **kwargs)
  File "/Users/jojo/git/beets/beets/library.py", line 777, in write
    mediafile.update(item_tags)
  File "/Users/jojo/git/mediafile/mediafile.py", line 1759, in update
    setattr(self, field, dict[field])
  File "/Users/jojo/git/mediafile/mediafile.py", line 1288, in __set__
    style.set(mediafile.mgfile, value)
  File "/Users/jojo/git/mediafile/mediafile.py", line 558, in set
    self.store(mutagen_file, self.serialize(value))
  File "/Users/jojo/git/mediafile/mediafile.py", line 563, in store
    mutagen_file[self.key] = [value]
  File "/Users/jojo/.venvs/beets/lib/python3.9/site-packages/mutagen/_file.py", line 74, in __setitem__
    self.tags[key] = value
  File "/Users/jojo/.venvs/beets/lib/python3.9/site-packages/mutagen/mp4/__init__.py", line 374, in __setitem__
    self._render(key, value)
  File "/Users/jojo/.venvs/beets/lib/python3.9/site-packages/mutagen/mp4/__init__.py", line 390, in _render
    return render_func(self, key, value, *render_args)
  File "/Users/jojo/.venvs/beets/lib/python3.9/site-packages/mutagen/mp4/__init__.py", line 635, in __render_freeform
    ">I4s2I", len(v) + 16, b"data", version << 24 | flags, 0)
TypeError: object of type 'float' has no len()

As a workaround I save as a string and set float_places to something rather high.

@sampsyo best you have a look on the final code and not on the commit's diff: https://github.com/JOJ0/mediafile/blob/acousticbrainz/mediafile.py#L2140-L2344

On that note: If you want I could rebase the stuff into one or only several commits. Or we want to leave all the experimenting and "pathfinding" in the history. However you suggest is fine for me.

Left some comments in the code that I think descbribe the most important findings and differences to Picards tagging. Hope it's clear an makes sense?? Please review. If it's too much or something should rather go into the Sphinx docs, happy to move it.

Added 3 additional fields: ab_key_key, ab_key_scale, ab_bpm to be able to save the original values coming from AB next to initialkey and bpm (which could come from somewhere else if the user wants that). Tag names are exactely as Picard has them. Will respect this change in my upcoming PR to acoustricbrainz.py in beetbox/beets. I am not sure if the naming is fine for these fields. Not sure about the fieldnames though (Also having in mind the ab prefix/alias fieldname PR, not sure if that would clash with it, could just renme these to key_key_ab, key_scale_ab, bpm_ab, that better for now?)

sampsyo commented 2 years ago

I don't think most formats (and apparently not MPEG-4) can actually store proper float values. (You'll notice that no other fields currently use as_type=float, for instance.) Instead, most formats actually store a stringified version of the floating-point value.

It might be a good idea to follow the example of, for example, rg_track_gain, which is another float-valued field. There, we use the float_places parameter to decide how much precision to use when stringifying, and out_type=float to convert strings to floats externally.

JOJ0 commented 2 years ago

Picard seems to save the original number exactly as retrieved from AcousticBrainz, if present keeping the scientific notation.

Mere experimenting brought up the decision that as_type=float seems to be the best idea.

The original value (and what Picard saves) is: 3.00000096379e-14

### afx every day (mp3) experiments

as_type=float:

beets:
     mood_aggressive: 3.00000096379e-14
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=3.00000096379e-14

as_type=float, out_type=float:

beets:
     mood_aggressive: 3.00000096379
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=3.00000096379e-14

out_type=float:

beets:
     mood_aggressive: 0.0
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=0.00

out_type=float, float_places=13

beets:
     mood_aggressive: 0.0
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=0.0000000000000

out_type=float, float_places=20

beets:
     mood_aggressive: 3.000001e-14
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=0.00000000000003000001

out_type=float, float_places=30

beets:
     mood_aggressive: 3.00000096379e-14
tag:
     TXXX=ab:hi:mood_aggressive:aggressive=0.000000000000030000009637899998
JOJ0 commented 2 years ago

So if we really want to mirror what Picard is doing, we should actually just save it as a simple string, without any magic.

Tried as suggested here an put as_type=unicode: https://github.com/JOJ0/mediafile/blob/acousticbrainz/mediafile.py#L498-L499

but that can't work because unicode is not a function. The docstring might be outdated. It may be a Python 2 thing?, I don't know.

Tried to put as_type=str but that gives me 0.00 values again.

How would I just save as-is without any magic?

(On another note, my personal opinion is that if those value should be readable and most of all comparable to one another (eg. comparing two mood_ values) by a human being, the scientific notation does not help and having 0.000something values with the same amount of float_places everywhere would be better. But I am not sure anymore what the goal should be. Being exactly equal to Picard or making useful decisions.....I don't know...and if the latter should be the goal then we could actually go even back to the idea and translate those values to something like SongKong does (-100 to +100) - but let's forget about this personal opinion for now but keep it in mind for later discussions on IRC ;-))

sampsyo commented 2 years ago

Picard seems to save the original number exactly as retrieved from AcousticBrainz, if present keeping the scientific notation.

Hmm… is it possible to confirm whether the values stored in the actual files are literally float values (i.e., 32-bit IEEE 754 numbers) or are strings containing those values (i.e., many more bytes, ASCII-encoded)? It seems perhaps hard to tell because they print the same way, but the difference is pretty important…

Tried as suggested here an put as_type=unicode: https://github.com/JOJ0/mediafile/blob/acousticbrainz/mediafile.py#L498-L499

Ah yes, that should be str on Python 3. (Or rather six.text_type, since we're still using six.)

Unrelatedly, there is some bad news that may render this discussion moot: AcousticBrainz seems to be shutting down. This was just announced yesterday: https://blog.metabrainz.org/2022/02/16/acousticbrainz-making-a-hard-decision-to-end-the-project/

To be perfectly honest, this puts a bit of a damper on the urgency toward figuring out the "right" tag formats for these values that will soon no longer be available. :/

JOJ0 commented 2 years ago

Hi @sampsyo that is really sad news, certainly for the beets project but also a lot of others who used it. Also with my personal pet project github.com/joj0/discodos I heavily rely on AB to find key and bpm info for my Vinyl. I only have a handful users I guess but that was the killer feature actually, at least for me... Well....sad.....but what do you say if we still finish this and enjoy the last weeks of acousticbrainz and then forget about it and put all the motivation in something else (like spotify audio_features for example ;-))

I opened a PR for the changes required in the plugin already.

I agree that getting the best Picard compatibility is not of much importance anymore. I decided that setting float_places to 12 is more than enough accuracy.

But I left out out_type=float entirely this morning but I am not sure anymore why I did it. I think when I had it set to float beet commands then sometimes showed scientific notation values which I wanted to prevent....sorry not sure anymore if that's true. I understand that the out_type arg is used to translate data read from file (which potentially is saved as a a string/unicode) and then return it, ensuring a certain type, to eg. beets.

I also found out that the values in the beets database are actually saved in their original values. And now I realized that beet info -l query shows me the values with just 6 digits after the comma, and that definitely comes from the original plugin dev's decision to display 6 digits only: https://github.com/beetbox/beets/blob/master/beetsplug/acousticbrainz.py#L124-L130

Hmmm....should I increaes the digits count in the beets plugin to 12 or better reduce mediafile saving to 6 digits? To stay consistent.

Furthermore I noticed that when editing a beets item using beet edit queryI see the original value as-is in the database (which makes sense obviously), just saying.

And I decided that the newly invented extra-fields for key and bpm from AB are not worth the hassle anymore. Didn't find a quick but not-too-ugly way of implementing this in the plugin so let's screw the idea and move on :-)

JOJ0 commented 2 years ago

Sorry for all the confusion on my end...again...I tested with other files that did not have so much zeros, when I wrote above, thus no E- notation.......I couldn't remember what the difference was with or without the out_type option.

I finally could reproduce why I removed out_type=float. A beet info query gives me something like this then:

       mood_acoustic: 0.081802986562
     mood_aggressive: 0.0
     mood_electronic: 0.979390621185
          mood_happy: 0.085078120232
          mood_party: 1.577842e-05
        mood_relaxed: 0.808817088604
            mood_sad: 0.109234951437
         moods_mirex: Cluster5

As I understand now this is expected since mediafile is returning "a real number" and this is how beets interprets it.

I didn't like the notation and removed it again to get:

       mood_acoustic: 0.081802986562
     mood_aggressive: 0.000000000000
     mood_electronic: 0.979390621185
          mood_happy: 0.085078120232
          mood_party: 0.000015778420
        mood_relaxed: 0.808817088604
            mood_sad: 0.109234951437

I liked this output better but as I understand now, these values are strings. Is this bad?

JOJ0 commented 2 years ago

Just a quick test whether using beets ls filters still work with the current setup (float_places=12, out_type NOT set):

$ beet ls mood_relaxed:0.4..1
AFX - Hangable Auto Bulb EP.2 - 01 - Every Day (1995/GB) [WAP69]  275kbps  m:1970-01-01 01:00:00  a:2016-09-14 07:49:56 relaxed:0.808817
Adam Beyer - London - 01 - London (2009/) []  320kbps  m:1970-01-01 01:00:00  a:2016-09-14 07:49:55 relaxed:0.571618
sampsyo commented 2 years ago

Cool! I do, however, think we should be using out_type=float. That gives beets an actual floating-point number it can do things with, such as compare two values with a < b (that wouldn't work with strings). Then, if we want to format these numbers differently for printing, we can accomplish that separately in beets.

JOJ0 commented 2 years ago

Hi, just set out_type to float :-)

sampsyo commented 2 years ago

It looks like the tests could use some love (see the GitHub Actions CI output above). Maybe that would be a good place to look next?

JOJ0 commented 2 years ago

Yup, saw it, on it, sorry I am distracted atm with beetifying all my DJ collection and submitting it to AB to be able to use the resource then back in DiscoDOS...

JOJ0 commented 2 years ago

I think I fixed test_known_fields. Also Flake8 check should be ok now. Please approve runs (Not sure if I can test this offline, no experience with tox so far, tried but no luck with Python versions or so; Tried python -m unittest discover but complains that files are missing the new tags, not sure if I have to actually manipulate fixture files, had a look on other PR's and they didn't change testfiles when adding new fields, thus not sure, anyway, let's see what gh-actions triggered tests say, Thanks!)

sampsyo commented 2 years ago

(Actions approved.)

JOJ0 commented 2 years ago

Ok, hope I fixed tests now. Running unittest discover locally is fine at least.

JOJ0 commented 2 years ago

Hi @sampsyo, have you found the time to give this one a final review? In particular, have a quick look on how I implemented the tests. Is there anything else I can do? Thanks!

sampsyo commented 2 years ago

It's in my queue to take a closer look; sorry for the delay.

JOJ0 commented 2 years ago

Closing, since as discussed in https://github.com/beetbox/beets/pull/4287, tag mappings are now done within the acousticbrainz plugin.