Extra 0 at the end of ID3v2 comment

[gino0631]

It looks like the library returns extra 0 at the end of ID3v2 comment, which gets converted to � in tags_html.

For example, with this file, containing COMMENT123456789012345678901 as a comment:

  $getID3 = new getID3;
  $out = $getID3->analyze('sample_id3v1_id3v23.mp3');

  $comment = $out['tags_html']['id3v2']['comment'];
  error_log($comment[0]);		// COMMENT123456789012345678901�

  $comment = $out['tags']['id3v2']['comment'];
  error_log(strlen($comment[0]));	// 29
  error_log($comment[0][28] == 0);	// 1

  $comment = 'COMMENT123456789012345678901';
  error_log(strlen($comment));		// 28

[JamesHeinrich]

getID3 is returning the comment with a trailing null because that's what the comment contains:

Note the encoding is UTF-16 so two bytes for each character. Note the last two bytes of the selected section (the 68 bytes of data for the COMM frame) are 00 00, a UTF-16 encoded null character.

I'm not sure what program you used to create those tags, perhaps it's incorrectly null-terminating the comment (the description should be null-terminated, the actual comment should not be).
http://id3.org/id3v2.3.0#Comments

[gino0631]

Thanks for looking into this. I've used Mp3tag v2.83.

[JamesHeinrich]

Let me know if the Mp3tag authors have a different opinion about the issue.

[gino0631]

This post by Florian explains the reasons behind his implementation.

[cbj4074]

I'm using the mid3v2 CLI tool provided with https://github.com/quodlibet/mutagen , and its tags exhibit the same problem when parsed with getID3.

It's frustrating that the relevant specification is written in such a way so as to be ambiguous, thereby causing one developer to implement it differently than another.

To quote Florian, from https://community.mp3tag.de/t/x-trailing-nulls-in-id3v2-comments/19227/4 :

This can give the impression that all strings are terminated. In fact, some developers implemented in exactly this way (e.g., Apple iTunes or foobar2000), whereas others are not terminating the strings (e.g., Windows Media Player).

Given this most unfortunate state of affairs, what are end-users to do?

I'd "complain" to the Mutagen author, but I suspect I'll be met with insistence that the specification is being followed, and that getID3 has it wrong.

Curious if your thoughts regarding the specific nature of this debate have changed since your last post in this thread, @JamesHeinrich . Have you read the comment that @gino0631 cited? Are you willing to make the concession put forward therein?

Thanks in advance!

[JamesHeinrich]

I have made change 127aac6 to remove null terminator if present.

[cbj4074]

Thanks so much for lightning-fast reply and remediation @JamesHeinrich ! I really appreciate it!

Just for the record, and so I understand the specification, is your contention still that the libraries that include the trailing null are in violation of the spec? Or is the spec ambiguous?

Thanks again!

[JamesHeinrich]

The ID3v2.3 was less ambiguous in this point, but ID3v2.4 spec that Florian quoted makes it less clear. I still think the ID3v2 author's intent was that the comment string not be null-terminated, but since there are multiple variants in the wild I guess it's best to support what's out there, regardless of what the specs intended.

For a format that has always had quite extensive documentation, ID3v2 must hold some kind of record for implementations in violation of the specs and/or variant interpretations of said specs.

[cbj4074]

Excellent; the clarification is hugely helpful. And somewhat comical. :D

One other thought/question:

As I describe in quodlibet/mutagen#379 , I notice that the frame values suffer from the same problem. For example, the value at ['id3v2']['TPE1'][0]['data'].

Do these warrant the same "triage"/workaround?

[JamesHeinrich]

In 4c2c774 I have extended the remove-trailing-nulls-if-applicable code to all other ID3v2 frame types where I think it's applicable. If I missed something please let me know.

[cbj4074]

Perfect! Every instance I had noticed now appears to be fixed, but I'll post an update if I happen upon any others.

Thanks again for your prompt replies and willingness to compromise with the other implementations that are in play.