Bug: wrong TrackNumber atom length in MP4

Started by dae65, August 05, 2020, 10:28:45 AM

Previous topic - Next topic

dae65

This looks like a minor issue, but I'd like to report it anyway. Who knows what problems could arise from it in the future.

ExifTool assumes that the DiskNumber and the TrackNumber tags in the QuickTime ItemList group have the same format, as it reads and writes them in the same way; namely, as a 30-byte atom with an embedded 22-byte data atom. This is a reasonable assumption, given that DiskNumber and TrackNumber are usually printed in the same way: "1 of 2", "1/2", etc.

DiskNumber is okay, but ExifTool should probably write TrackNumber as a 32-byte atom with an embedded 24-byte data atom instead. Compare the fourth and twelfth bytes below. These are files where TrackNumber was set to "1/10" with Apple iTunes (12.06), AtomicParsley (0.9.6). and ExifTool (12.03) respectively:


$ hexdump -C -s 77007 -n 32 itunes.m4a
00012ccf  00 00 00 20 74 72 6b 6e  00 00 00 18 64 61 74 61  |... trkn....data|
00012cdf  00 00 00 00 00 00 00 00  00 00 00 01 00 0a 00 00  |................|
00012cef

$ hexdump -C -s 78721 -n 32 atomicparsley.m4a
00013381  00 00 00 20 74 72 6b 6e  00 00 00 18 64 61 74 61  |... trkn....data|
00013391  00 00 00 00 00 00 00 00  00 00 00 01 00 0a 00 00  |................|
000133a1

$ hexdump -C -s 77014 -n 30 exiftool.m4a
00012cd6  00 00 00 1e 74 72 6b 6e  00 00 00 16 64 61 74 61  |....trkn....data|
00012ce6  00 00 00 00 00 00 55 c4  00 00 00 01 00 0a        |......U.......|
00012cf4


ExifTool output is 2 bytes shorter. According to AtomicParsley's documentation, those 2 extra null-bytes that ExifTool fails to write at the end of the trkn.data atom are presumably just "padding." I'm not convinced, though, if for no other reason than that what comes closest to trkn in terms of format is the disk atom, but disk has no such padding. Compare: disk length is 30 bytes, and data length is 22 bytes for a value meaning "2 of 3" on both iTunes and AtomicParsley:


$ hexdump -C -s 77039 -n 30 itunes.m4a
00012cef  00 00 00 1e 64 69 73 6b  00 00 00 16 64 61 74 61  |....disk....data|
00012cff  00 00 00 00 00 00 00 00  00 00 00 02 00 03        |..............|
00012d0d

$ hexdump -C -s 78753 -n 30 atomicparsley.m4a
000133a1  00 00 00 1e 64 69 73 6b  00 00 00 16 64 61 74 61  |....disk....data|
000133b1  00 00 00 00 00 00 00 00  00 00 00 02 00 03        |..............|
000133bf


Back to trkn, even if just padding, AtomicParsley makes sure to write it regardless (see main.cpp):


AtomicInfo* tracknumData_atom = APar_MetaData_atom_Init("moov.udta.meta.ilst.trkn.data", optarg, AtomFlags_Data_Binary);                                       
//tracknum: [0, 0, 0, 0,   0, 0, 0, pos_in_total, 0, the_total, 0, 0]; BUT that first uint32_t is already accounted for in APar_MetaData_atom_Init             
APar_Unified_atom_Put(tracknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, 0, 16);                                                                       
APar_Unified_atom_Put(tracknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, pos_in_total, 16);                                                           
APar_Unified_atom_Put(tracknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, the_total, 16);                                                               
APar_Unified_atom_Put(tracknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, 0, 16);                                                                                           

                                                                                                                                                                     
While the disk atom is written with no padding (here):                                                                                                                           
                                                                                                                                                                                   
AtomicInfo* disknumData_atom = APar_MetaData_atom_Init("moov.udta.meta.ilst.disk.data", optarg, AtomFlags_Data_Binary);                                                           
//disknum: [0, 0, 0, 0,   0, 0, 0, pos_in_total, 0, the_total]; BUT that first uint32_t is already accounted for in APar_MetaData_atom_Init                                       
APar_Unified_atom_Put(disknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, 0, 16);                                                                                           
APar_Unified_atom_Put(disknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, pos_in_total, 16);                                                                                                           
APar_Unified_atom_Put(disknumData_atom, NULL, UTF8_iTunesStyle_256glyphLimited, the_total, 16);                                                                                                             

                                                                                                                                                                                                                                         
It's true that ilst.trkn belongs to the reserved data-type class (0x00), which is "reserved for use where no type needs to be indicated" (QuickTime File Format Specification). Nevertheless, if I understand correctly, atoms under the moov.udta.meta.ilst hierarchy were introduced by iTunes, and have been reverse-engineered from iTunes output since then. And so, to be on the safe side, since we can't know what the Apple developer who implemented trkn had in mind with those extra bytes, maybe we should follow AtomicParsley here, and stick to some x0000 int16u int16u x0000 template when writing it as well.                                                                                                                                                                     
                                                                                                                                                                                                                                               
This may have ramifications for ValueConv and ValueConvInv: how they should unpack and pack things, but here we're in the dark as to what the leading and the trailing x0000 padding bytes could possibly be used for.                                     
                                                                                                                                                                                                                                               
Thank you. :)

dae65

I forgot to mention this has nothing to do with ExifTool 12.02 change in order "Allow integer QuickTime TrackNumber and DiskNumber values". This already happened as of ExifTool 12.01:

$ exiftool -ver
12.03 [Warning: Library version is 12.01]
$ exiftool -ItemList:TrackNumber='2 of 4' exiftool.m4a
    1 image files updated
$ hexdump -s 77014 -n 30 -C exiftool.m4a
00012cd6  00 00 00 1e 74 72 6b 6e  00 00 00 16 64 61 74 61  |....trkn....data|
00012ce6  00 00 00 00 00 00 55 c4  00 00 00 02 00 04        |......U.......|
00012cf4


dae65

Phil,

This is no minor issue. As I was looking into the data-type bug in the other thread, I noticed that Apple iTunes (12.06) actually ignores a trkn atom as written by ExifTool. Please have a look at the empty "track [   ] of [   ]" on the same screenshot. It seems it has to be 32 bytes long after all.

DiskNumber is good though.

Best,
d

dae65

iTunes will be able to read a trkn atom by ExifTool as long as you change ValueConvInv from:
return pack('n3', 0, @a)
to:
return pack('n4', 0, @a, 0);
or something along those lines.

See screenshot. Thanks.

Phil Harvey

Great, thanks!  I'll make this change too.

- Phil
...where DIR is the name of a directory/folder containing the images.  On Mac/Linux/PowerShell, use single quotes (') instead of double quotes (") around arguments containing a dollar sign ($).