Support for text information frames with multiple strings in ID3v2.4

[mpatric]

ID3v2.4 text information frames can contain multiple strings, whereas in ID3v2.3 and earlier they can only contain one. From the ID3v2.4 specification:

The text information frames are often the most important frames,
containing information like artist, album and more. There may only be
one text information frame of its kind in an tag. All text
information frames supports multiple strings, stored as a null
separated list, where null is reperesented by the termination code
for the charater encoding. All text frame identifiers begin with "T".
Only text frame identifiers begin with "T", with the exception of the
"TXXX" frame. All the text information frames have the following
format:

  <Header for 'Text information frame', ID: "T000" - "TZZZ",
  excluding "TXXX" described in 4.2.6.>
  Text encoding                $xx
  Information                  <text string(s) according to encoding>

Mp3agic will currently only return the first value when reading multi-values text information frames and will only allow you to set one value.

[gardnermj]

Any thoughts on implementing this? I might take a stab at it soon; any guidance would be helpful.

[gardnermj]

I've implemented multi-value support for ID3v2.4 text frames: https://github.com/oueqzoms/mp3agic/commit/c06d8da0b24969b98f41b90fdb06048dfac691df. Before I start working on tests, could you let me know what you think of the API changes?

[mpatric]

Hi, sorry for the slow response, I've been away from a good internet connection for the last week.

The approach you've taken is the same as what I was thinking; i.e. you've added various plural versions of the get and set methods to the ID3v24 interface that return/take arrays of Strings instead of Strings.

Thanks for working on this.

Cheers, Michael

[gardnermj]

Thanks for the feedback. Do you have any opinion on what type of collection the plurals should return? I used String[] because it was convenient, but I'm rusty with Java and don't know the current best practice for returning ordered collections.

[mpatric]

I prefer native arrays, i.e. String[]. Does anyone else have an opinion on native array vs a collection interface? @hennr - do you have an opinion on this?

[hennr]

Hi @oueqzoms thanks for implementing this!

I'm fine with String arrays as well, but for the setters I'd prefer varargs e.g. public void setArtists(String[] artists) { should become public void setArtists(String... artists) {

Also please avoid checked exception where possible, they are unhandy to code with.

[gardnermj]

@hennr, varargs would make it awkward to provide values from a collection, which seems likely to be the most common scenario. That doesn't feel like a good trade-off to me.

About the checked exceptions, I just followed the existing method signatures. I have no opinion on that, other than that things should be kept consistent either way.

[gardnermj]

Would it be better to put the plural getters in the ID3v2 interface, so that ID3v2.x tags can all be read in a consistent way?

The non-2.4 implementations would just return 1-element arrays (even though technically some text frames in ID3v2.3 do support multiple values separated by a forward slash, that just seems too error-prone to support at this level due to false positives like "AC/DC").

[mpatric]

I like your suggestion about plural getters in the ID3v2 interface, with the pre 2.4 implementations returning 1-element arrays.

[gardnermj]

After some more thought, I would now advocate having the plurals use Iterable<String>. But if the consensus is still for String[], that's fine too.

In the event that there is no corresponding frame, should the plural getters return null or an empty collection? I'd lean towards the empty collection, to reduce null-checking by clients.

[hennr]

I'd prefer an empty collection for the same reason.