mpatric / mp3agic

A java library for reading mp3 files and reading / manipulating the ID3 tags (ID3v1 and ID3v2.2 through ID3v2.4).
MIT License
1.2k stars 310 forks source link

Equals on Id3v2 tags does not work #177

Closed barbetb closed 4 years ago

barbetb commented 4 years ago

When I create Id3v2 tags, and set it to a file. Then save the file. Then retrieve the Id3v2 tags and compare it with the tags I used to create it with, it will not return equals. This is due to line: if (dataLength != other.dataLength) return false;

in AbstractID3v2Tag.java Data length stays 0 on the original tags.


Also:

            final ID3v2 id3v2 = this.idTagFactory.getId3Tags(mp3Dto);
            mp3File.setId3v1Tag(id3v2); //casting to Id3v1
            mp3File.setId3v2Tag(id3v2);
            mp3File.save(this.getNewFilenameForExportMp3(mp3)); 

Will result in a file with only an Id3v2Tag (though the object contains both and I see it written during save). However it saves to 1 slot. Is this the desired behaviour?

Thanks for the great library, but this was an issue I encountered.

mpatric commented 4 years ago

I've written a unit test using one of the test mp3s in src/test/resources to try reproduce what you describe and it passes fine..

public void shouldBeEqualTags() throws InvalidDataException, IOException, UnsupportedTagException, NotSupportedException {
  Mp3File mp3File = new Mp3File("src/test/resources/v1andv24tags.mp3");
  ID3v2 id3v23Tag = mp3File.getId3v2Tag();
  // save in a new file
  String newFilename = "src/test/resources/v1andv24tags-copy.mp3";
  mp3File.save(newFilename);
  // repoen file and read tags
  Mp3File mp3File2 = new Mp3File(newFilename);
  ID3v2 id3v23Tag2 = mp3File.getId3v2Tag();
  // compare tags from old and new file
  assertEquals(id3v23Tag, id3v23Tag2);
}
barbetb commented 4 years ago

Set both Idv1 and 2 fails:

@Test
    public void shouldSetIdv1and2Tags() throws Exception {
        File filename = new File(MP3_WITH_NO_TAGS);
        final ID3v24Tag id3v24Tag = new ID3v24Tag();
        id3v24Tag.setTrack("1");
        id3v24Tag.setTitle("title");
        id3v24Tag.setArtist("artist");
        id3v24Tag.setAlbum("album");
        Mp3File mp3File = new Mp3File(filename);
        mp3File.setId3v2Tag(id3v24Tag);
        mp3File.setId3v1Tag(id3v24Tag);
        final String newFilename = MP3_WITH_NO_TAGS+ ".copy";;
        mp3File.save(newFilename);
        Mp3File copy = new Mp3File(new File(newFilename));
        assertTrue(copy.hasId3v2Tag());
        assertTrue(copy.hasId3v1Tag()); //fails
    }

I now have a work around I use:

public Mp3File somesetmethod(){
   final Mp3File mp3File = new Mp3File(somefile);
   final ID3v2 id3v2 = this.idTagFactory.getId3Tags(newMp3);
   mp3File.setId3v1Tag(mapId3v1Tag(id3v2));
   mp3File.setId3v2Tag(id3v2);
   return mp3file;

}

    private ID3v1Tag mapId3v1Tag(final ID3v2 id3v2) {
        final ID3v1Tag id3v1Tag = new ID3v1Tag();
        new ModelMapper().map(id3v2, id3v1Tag);
        return id3v1Tag;
    }
barbetb commented 4 years ago

You are right on equals, it works:

    public void shouldEquals(){
        final ID3v24Tag tag1 = new ID3v24Tag();
        tag1.setTrack("1");
        tag1.setTitle("title");
        tag1.setArtist("artist");
        tag1.setAlbum("album");

        final ID3v24Tag tag2 = new ID3v24Tag();
        tag2.setTrack("1");
        tag2.setTitle("title");
        tag2.setArtist("artist");
        tag2.setAlbum("album");
        assertTrue(tag1.equals(tag2)); //success    }
mpatric commented 4 years ago

In the above unit test (shouldSetIdv1and2Tags):

mp3File.setId3v1Tag(id3v24Tag);

You can't set a id3v1 tag with a id3v2 tag as the argument