Bug: standard genre codes written with wrong flag, format, and value in ItemList

[dae65]

Dear Phil,

I've found 2 issues with the Genre tag, more specifically with ItemList:ID-gnre:Genre, which is traditionally used to store standard genre codes. (User-defined genres are written to ©gen.)

1) Wrong value

If we set ItemList:ID-gnre:Genre to decimal 33, we get Classical. This is right. But if we set it to Classical, we get Trance. This is wrong.

Code Select Expand


$ exiftool -ItemList:ID-gnre:Genre=33 file.m4a
    1 image files updated
$ exiftool -ItemList:ID-gnre:Genre file.m4a
Genre                           : Classical
$ exiftool -ItemList:ID-gnre:Genre=Classical file.m4a
    1 image files updated
$ exiftool -ItemList:ID-gnre:Genre file.m4a
Genre                           : Trance


This is because PrintConv subtracts 1 from the value it passes as an argument to Image::ExifTool::ID3::PrintGenre, but PrintConvInv fails to add 1 to the value it gets from Image::ExifTool::ID3::GetGenreID. (see QuickTime.pl). A quick way to fix this is for PrintConvInv to return $id + 1 instead of $id. But this is not enough. Please bear with me.

2) Wrong flag and format

ExifTool 12.04 writes ilst.gnre data atoms as UTF-8 strings with a 0x1 data-type flag (utf8):

Code Select Expand


$ exiftool -v3 file.m4a
[...]
  | | | + [ItemList directory]
  | | | | Genre = 33
  | | | | - Tag 'gnre', Type='data', Flags=0x1, Lang=0x0000 (2 bytes):
  | | | |    12c11: 33 33                                           [33]


This is not news, and the source code acknowledges it:

Code Select Expand


        # (Note: not written as int16u if numerical, although it should be)


I'd like to request it to be written as int16u with 0x0 flag (binary), as it should be , and to request it always to be written in this way, please.  Otherwise, other applications and libraries (iTunes, Mp3Tag, puddletag, Mutagen, cmus, VLC, and Juk) will flatly ignore it, or else will misleadingly interpret it either literally as some UTF-8 decimal string (AtomicParsley), or as a hex string representing some 16-bit integer (MP4v2), or even as a hex string representing some 8-bit integer (mpv/mplayer). Compare:

Code Select Expand

$ AtomicParsley file.m4a -t
Atom "gnre" contains: 33

Note: It should say "... contains: Classical".

Code Select Expand

$ mp4info file.m4a
mp4info version -r
file.m4a:
Track   Type    Info
1       audio   MPEG-4 AAC LC, 221.030 secs, 0 kbps, 44100 Hz
GenreType: 13107, UNDEFINED(13107)
Note: 13107 equals 0x3333.

Code Select Expand

$ mpv file.m4a
Playing: file.m4a
(+) Audio --aid=1 --alang=und (*) (aac)
File tags:
Genre: Darkwave
AO: [pulse] 44100Hz stereo 2ch float
Note: Darkwave maps to MP4/M4A 0x33 (51) and ID3v1 0x32 (50).

All MP4/M4A players and tag editors I have, ExifTool included, are able to read 0x0 int16 gnre atoms. Moreover, this is how iTunes, AtomicParsley, and Mp3tag write it. MP4v2, puddletag, VLC, and Juk on the other hand, seem to avoid writing to gnre altogether, though they can read it; they write genre names, even standard ones such as Classical, to ©gen as UTF-8 strings. As far as I could ascertain, ExifTool is alone in writing standard genre numbers to gnre as UTF-8 strings.

Thank you

[dae65]

Dear Phil,

In order to fix this, I'd like to suggest gnre to be defined in ItemList as follows:

Code Select Expand


    gnre => { #10
        Name => 'Genre',
        Format => 'undef',
        ValueConv => q{ unpack 'n', $val },
        ValueConvInv => q{ $val =~ /^[0-9]+$/ ? pack 'n', $val : undef },
        PrintConv => q{
            return $val unless $val =~ /^[0-9]+$/;
            require Image::ExifTool::ID3;
            Image::ExifTool::ID3::PrintGenre($val - 1); # note the "- 1"
        },
        PrintConvInv => q{
            return $val if $val =~ /^[0-9]+$/;
            require Image::ExifTool::ID3;
            my $id = Image::ExifTool::ID3::GetGenreID($val);
            return defined $id ? $id + 1 : undef;
        },
    },

In this way, ExifTool writes standard genres with the right value, as int16u, and with a 0x0 data-type flag just like iTunes, AtomicParsley and Mp3tag do:

Code Select Expand


itunes/classical.m4a: gnre @ 0x12bf9 + 26
00012bf9  00 00 00 1a 67 6e 72 65  00 00 00 12 64 61 74 61  |....gnre....data|
00012c09  00 00 00 00 00 00 00 00  00 21                    |.........!|
00012c13
atomicparsley/classical.m4a: gnre @ 0x133c9 + 26
000133c9  00 00 00 1a 67 6e 72 65  00 00 00 12 64 61 74 61  |....gnre....data|
000133d9  00 00 00 00 00 00 00 00  00 21                    |.........!|
000133e3
mp3tag/classical.m4a: gnre @ 0x12bf1 + 26
00012bf1  00 00 00 1a 67 6e 72 65  00 00 00 12 64 61 74 61  |....gnre....data|
00012c01  00 00 00 00 00 00 00 00  00 21                    |.........!|
00012c0b
exiftool/classical.m4a: gnre @ 0x12bf9 + 26
00012bf9  00 00 00 1a 67 6e 72 65  00 00 00 12 64 61 74 61  |....gnre....data|
00012c09  00 00 00 00 00 00 00 00  00 21                    |.........!|
00012c13

Only standard genre codes are acceptable here. User-defined genre names should be written to ilst.©gen instead as UTF-8 strings.

As to reading:

Code Select Expand


$ exiftool -ItemList:ID-gnre:Genre {itunes,atomicparsley,mp3tag,exiftool}/classical.m4a
======== itunes/classical.m4a
Genre                           : Classical
======== atomicparsley/classical.m4a
Genre                           : Classical
======== mp3tag/classical.m4a
Genre                           : Classical
======== exiftool/classical.m4a
Genre                           : Classical
    4 image files read


I've tested files containing exiftool-written ilst.gnre with iTunes, AtomicParsley, Mp3tag, MP4v2, VLC, puddletag, Juk, and cmus. Please see the attached diff for what exactly I think needs our attention.

Thank you.

[dae65]

Oh, I see. Image::ExifTool::ID3::GetGenreID() returns non-numeric values for Cover and Remix. I suggest we ignore CR and RX as standard genre codes, since QuickTime is not ID3.

Code Select Expand


    PrintConvInv => q{
      return $val if $val =~ /^[0-9]+$/;
      require Image::ExifTool::ID3;
      my $id = Image::ExifTool::ID3::GetGenreID($val);
      return unless defined $id and $id =~ /^[0-9]+$/;
      return $id + 1;
    },


Thank you

[Phil Harvey]

Sorry for the delay in responding.  I've been rather busy lately.

I think you are right on with your analysis and patch / bug fix.  I'll apply it to the next release.

Thanks!!!

- Phil

[dae65]

Great! Don't worry. Thanks a lot.

ps: I forgot to add Avoid => 1 to the patch. Sorry.

[Phil Harvey]

ps: I forgot to add Avoid => 1 to the patch. Sorry.

Got that already, thanks.

- Phil