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.

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

year is shown as null in ID3v2.4 tagged file #1

Closed
tastytea opened this issue Dec 31, 2022 · 13 comments
Closed

year is shown as null in ID3v2.4 tagged file #1

tastytea opened this issue Dec 31, 2022 · 13 comments

Comments

@tastytea
Copy link

id3-json -r 10.\ the\ masochism\ tango.mp3 | jq . outputs this:

{
  "data": {
    "album": "An Evening Wasted With Tom Lehrer",
    "artist": "Tom Lehrer",
    "comment": null,
    "genre": "Comedy",
    "title": "The Masochism Tango",
    "track": 10,
    "year": null
  }
}

while picard shows this:

screenshot picard showing the date as 1990-04-12

It happens with this file: testfile.zip (it's in the public domain)

% ./id3-json -V
id3-json 0.1.2

I'm not very familiar with id3 tags, but the issue appears to be that v2.4 replaced the year tags (TORY and TYER) with time tags (TDOR and TDRL). Maybe they should be used if the year tags are not found?

@AndrewRadev
Copy link
Owner

AndrewRadev commented Jan 1, 2023

That's a reasonable request, I agree. I've just pushed some code that should use fallbacks. Here's what I put in the README for it:

When reading tags, the plugin will try to return "year" from the TYER tag, but if there is none, it'll look for the year in the "date released" (TDRL), "original date released" (TDOR) and "date recorded" (TRDC) tags, in that order. When writing, it'll still write in "TYER".

I'd appreciate if you test it out and share your thoughts on the order of fallbacks. If you think it makes sense, I'll publish a new minor release.

@tastytea
Copy link
Author

tastytea commented Jan 1, 2023

Reading works fine! I think the order makes sense. But i wonder if it is legal to write TYER into an id3v2.4 header? The spec doesn't mention it so i guess not? I think it should write into the tag the original value came from.

@AndrewRadev
Copy link
Owner

AndrewRadev commented Jan 2, 2023

Hmm, you're right. The reason it was working when I tested it is because I was writing the tags as v2.3. I've pushed a change that writes the version of the tags that was read, and updating the year no longer works for v2.4.

What I think I'll do is only return the year if the version is < 2.4 and return the full date otherwise. Same for writing. It might be best to also expose a --tag-version flag and add a Version: field in the Vim plugin. I'll work on it.

@AndrewRadev
Copy link
Owner

I've pushed some commit that implement this -- there's a "date" for v2.4 tags and a "year" for 2.3 and 2.2. These seem to be the only versions the rust library handles. If you install the latest git versions of the Vim plugin and the tool, you should be able to edit those consistently.

For the date, I've only taken "release date" to avoid mixing things up, now that we've got them separated.

Could you (carefully, on copies of music) try this out and let me know if it seems to work as you'd expect?

@tastytea
Copy link
Author

tastytea commented Jan 4, 2023

It looks like “release date” (TDRL) is not read and written by picard and EasyTag but they use “Recording time” (TDRC) instead¹, so it was a bit tricky to find a suitable file 😊.

id3-json reads and writes the files as expected. However, i get this error when trying to open a file with TDRL set in neovim:

Error detected while processing BufReadCmd Autocommands for "*.mp3"..function id3#ReadMp3[9]..id3#id3_json#Read[19]..<SNR>102_NullToString:
line    2:
E735: Can only compare Dictionary with Dictionary
Error detected while processing BufReadCmd Autocommands for "*.mp3"..function id3#ReadMp3[9]..id3#id3_json#Read:
line   44:
E731: using Dictionary as a String
E116: Invalid arguments for function extend

Files with no TDRL tag and TDRC instead trigger no error but the date isn't displayed, of course. Both the binary and the plugin were updated ~20 minutes before writing this.

¹ I gathered that by running strings <file> | less and searching for the tag names, not sure how accurate that is.

@AndrewRadev
Copy link
Owner

It looks like “release date” (TDRL) is not read and written by picard and EasyTag but they use “Recording time” (TDRC) instead¹, so it was a bit tricky to find a suitable file blush.

Hm, okay, "release date" was just my assumption, I've pushed a change that uses "recording date". Tested it locally with picard and it does seem like the updates are happening.

id3-json reads and writes the files as expected. However, i get this error when trying to open a file with TDRL set in neovim:

That's odd, I just tested it out with neovim and I'm not seeing this. I added some more precise checks, so if and when you see this error, maybe we'll get more context.

I also found a dumb bug with comments, which might have affected things, not sure. Either way, could you update both id3-json and id3.vim and give it another shot?

@tastytea
Copy link
Author

tastytea commented Jan 6, 2023

It works fine with files with “recording date” now but i still get the same error for files with “release date” but without “recording date”.

Error detected while processing BufReadCmd Autocommands for "*.mp3"..function id3#ReadMp3[9]..id
3#id3_json#Read[24]..<SNR>102_NullToString:
line    2:
E735: Can only compare Dictionary with Dictionary
Error detected while processing BufReadCmd Autocommands for "*.mp3"..function id3#ReadMp3[9]..id
3#id3_json#Read:
line   49:
E731: using Dictionary as a String
E116: Invalid arguments for function extend

@tastytea
Copy link
Author

tastytea commented Jan 6, 2023

Ah, the error is because of the comment! Both files i tested had http://www.jamendo.com Attribution-Noncommercial-No Derivative Works 3.0 set as comment. Setting it to test produced the same error. Removing the comment completely got rid of the error.

@AndrewRadev
Copy link
Owner

AndrewRadev commented Jan 7, 2023

Ah, the error is because of the comment!

Hm, I still can't replicate it :/. It seems the json result's comment key is a dictionary, but I don't understand why. Could you run id3-json on the file directly and paste the results? Also, inside neovim, could you try:

echo string(json_decode(system('id3-json '.shellescape('10. the masochism tango.mp3'))))

(or whatever the name of the file you're testing with)

@tastytea
Copy link
Author

tastytea commented Jan 7, 2023

Ah, the error is because of the comment!

Hm, I still can't replicate it :/. It seems the json result's comment key is a dictionary, but I don't understand why.

In picard i can add several comments to one file. But that should make it an array in my opinion… 🤔

Could you run id3-json on the file directly and paste the results?

{
  "data": {
    "album": "The Butcher's Ballroom",
    "artist": "Diablo Swing Orchestra",
    "comment": "test\u0000",
    "date": null,
    "genre": null,
    "title": "Balrog Boogie",
    "track": 1
  },
  "version": "ID3v2.4"
}

Also, inside neovim, could you try:

echo string(json_decode(system('id3-json '.shellescape('10. the masochism tango.mp3'))))

(or whatever the name of the file you're testing with)

{'data': {'genre': v:null, 'track': 1, 'comment': {'_TYPE': [], '_VAL': ['test
']}, 'album': 'The Butcher''s Ballroom', 'date': v:null, 'title': 'Balrog Boogie', 'artist': 'Diablo Swing Orchestra'}, 'version': 'ID3v2.4'}

(the newline is not because of my window size, it's in the output)

@AndrewRadev
Copy link
Owner

AndrewRadev commented Jan 7, 2023

I see now. This is a neovim thing. The help files for json_decode in neovim say this:

3. String contains NUL byte.  Two special dictionaries: for
   dictionary and for string will be emitted in case string
   with NUL byte was a dictionary key.

The comment string ends in a nul bytes (that's the \u{0000}). Vim just ignores it, but Neovim outputs a JSON object with those extra keys, something to do with MsgPack. 🤷

I've pushed a new id3-json that cleans up trailing nul bytes for all the output strings. Try with that?

@tastytea
Copy link
Author

tastytea commented Jan 7, 2023

Yep, it works now. 🎉

@AndrewRadev
Copy link
Owner

Awesome :). I've pushed a new release, 0.2.0, with compiled binaries and everything. With that, I'll close this issue, but please open a new one if you run into more problems.

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

No branches or pull requests

2 participants