Improve gopro support

[vdeconinck]

tl;dr

GoPro files (at least on Hero 8, 9 and 12) cause crashes of IsoViewer. I tracked down the issues to parsing typos or misconfigurations and I propose this PR which combines the different fixes. The text below lists the issues, root cause of each problem and fix applied. Each one corresponds to one commit in the PR.

The big drawback of this is that I had to disable the test "TimeCodeBoxTest" (more details below). If there is a way to adapt that test, I'd be glad to implement it...

Here are the details, please take a coffee ;-)

0) Support blanks in project path.

This is totally unrelated but all tests failed on my machine because my projects reside in a path with blanks.

Problem: Many tests use getLocation().getFile() in which blanks are encoded as %20, resulting in FileNotFoundException failures.

Fix: Call URLDecode on the resulting String to restore the blanks.

Now for the parsing issues:

1) getContentSize() returns the size required to export the contents, not the size read when importing the contents.

For some rare items, the value is different because the source file includes useless "stuffing". This was the case for AudioSpecificConfig, where GoPro files contain 5 bytes for the bitmap while 4 would be sufficient. In itself, it's not a big deal: when calling getSize(), which is based on getContentSize(), you get the required size, not the parsed one.

Problem: IsoParser stops parsing only when the number of bytes read equals the full size of the contents (computed by using getSize()). As getSize() of AudioSpecificConfig returns one byte less than actual the size in the file, the sum of contents parsed is wrongly considered one byte less than the file size, So the parser tries to read one more byte after the end of the file and fails with an EOFException.

Fix: in AudioSpecificConfig, change getContentSize() so that, if the bitmap was parsed from a source and that size was larger than the minimum required, the size in the source is returned. Note1: this fix is safe because getContents() allocate the bitmap based on getContentSize(), so rewriting a file previously parsed including stuffing will result in the newly written file also including stuffing. Note2: this change could potentially be applied everywhere and not only in this specific case, but I'm not in a position to decide such a refactoring :-)

2) GoPro files make heavy usage of "timecode" information, and 'tmcd' parsing is not configured correctly.

Herited from Quicktime, several parts of an MPEG4 file can relate to TimeCode, See https://developer.apple.com/documentation/quicktime-file-format/timecode_media Namely, in GoPro files, one can find 'tmcd':

a) under moov\trak[0]\tref or moov\trak[1]\tref, as follows:

     00 00 00 14 74 72 65 66 00 00 00 0c 74 6d 63 64
     00 00 00 03

which must be interpreted as a timecode "track reference type" on those tracks, referencing the third track containing timecode:

   00 00 00 14 74 72 65 66
=> size = 0x14  t  r  e  f
   00 00 00 0c 74 6d 63 64
=> size = 0x0c  t  m  c  d
   00 00 00 03
=> trackIds = [3]

Problem: the 'tmcd' box being linked to TimeCodeBox in isoparser2-default.properties, that box parsing gets triggered instead of a TrackReferenceTypeBox, and it crashes because there aren't enough bytes.

b) under moov\trak[2]\mdia\hdlr, as follows:

   00 00 00 2c 68 64 6c 72 00 00 00 00 6d 68 6c 72
   74 6d 63 64 00 00 00 00 00 00 00 00 00 00 00 00
   0b 47 6f 50 72 6f 20 54 43 44 20 20

which is correctly interpreted as an unknown 'tmcd' handler inside a handler box

   00 00 00 2c 68 64 6c 72 00 00 00 00
=> size = 0x2c  h  d  l  r (flags = 0)
   6d 68 6c 72
=>  m  h  l  r (?)
   74 6d 63 64
=>  t  m  c  d (= handler type)
   00 00 00 00 00 00 00 00 00 00 00 00
=> a = 0       b = 0       c = 0
   0b 47 6f 50 72 6f 20 54 43 44 20 20
=>l=11 G  o  P  r  o     T  C  D       (name="GoPro TDC  ")

Problem: None.

c) under moov\trak[2]\mdia\minf\gmhd, as follows:

   00 00 00 32 74 6d 63 64 00 00 00 2a 74 63 6d 69
   00 00 00 01 00 15 00 00 00 0a 00 00 00 00 00 00
   00 00 ff ff ff ff ff ff 09 48 65 6c 76 65 74 69
   63 61

which must be interpreted as a tmcd containing a tcmi - https://developer.apple.com/documentation/quicktime-file-format/timecode_media_information_atom

   00 00 00 32 74 6d 63 64
=> size = 0x32  t  m  c  d
   00 00 00 2a 74 63 6d 69
=> size = 0x2a  t  c  m  i
   00
=> version = 0
   00 00 01
=> flags=01
   00 15
=> font=0x15
   00 00
=> face=0x00 (plain)
   00 0a
=> size=0x0a
   00 00
=> (reserved)
   00 00 00 00 00 00
=> color = (00,00,00)
   ff ff ff ff ff ff
=> bgcolor = (ff,ff,ff)
   09 48 65 6c 76 65 74 69 63 61
=> l=9 H  e  l  v  e  t  i  c  a (font name, Pascal style)

Problem: the 'tmcd' triggers the parsing by TimeCodeBox and, while it does not crash, the fields displayed are meaningless because data is not in that format

d) under moov\trak[2]\mdia\minf\stbl\stsd\tmcd, as follows:

   00 00 00 22 74 6d 63 64 00 00 00 00 00 00 00 01 
   00 00 00 00 00 00 00 02 00 01 5f 90 00 00 0b bb 
   1d 00

which is (almost) correctly interpreted as an Apple TimeCodeBox:

   00 00 00 22 74 6d 63 64
=> size = 0x22  t  m  c  d
   00 00 00 00 00 00
=> (reserved)
   00 01 
=> dataReferenceIndex=1
   00 00 00 00 
=> (reserved)
   00 00 00 02 
=> Timecode flags = 0x2 (Indicates the timecode wraps after 24 hours)
   00 01 5f 90
=> Timescale = 15F90h = 90000d
   00 00 0b bb
=> Frame duration = BBBh = 3003d
   1d
=> Number of frames = 1Dh = 29d 
   00
=> (reserved)
=> (no additional source reference information)

Problem: the last (reserved) field is actually on 1 byte, not 3 (According to Apple Quicktime docs, to ffmpeg source and to GoPro samples). See commit message for more details and references.

Fixes: Case b) is OK Case d) is an easy fix (read and write 1 byte instead of 3) But for case a) and c), the source of the issue comes from the fact that they are all interpreted as an Apple TimeCodeBox while they are not. This is due to the following mapping in isoparser2-default.properties: tmcd=org.mp4parser.boxes.apple.TimeCodeBox which must be edited to add the stsd- suffix, as follows: stsd-tmcd=org.mp4parser.boxes.apple.TimeCodeBox

Unfortunately, the TimeCodeBoxTest now fails miserably, because the TimeCodeBox is parsed as an UnknownBox when not inside an stsd. Modifying the hex string in the test class to add an enclosing stsd is not sufficient, because the BoxWriteReadBase.roundtrip() wants to write and read the single box under test, and I have no idea how to pretend it's inside a stsd... The test was thus disabled (renamed) for the time being...

Once TimeCodeBox is limited to tmcd inside stsd, no crash happens anymore, but the 'tmcd' as track reference and the 'tmcd' as a container for 'tcmi' are considered Unknown. case a): the following line was added to isoparser2-default.properties: tref-tmcd=org.mp4parser.boxes.iso14496.part12.TrackReferenceTypeBox(type) case c): two classes TimeCodeContainerBox and TimeCodeMediaInformationBox are now implemented and associated in the isoparser2-default.properties file to gmhd-tmcd and tcmi, respectively.

3) Additionally, the current TimecodeBox implementation uses a field called "long flags;"

Problem: IsoViewer introspects fields and matches any "flags" field by name, casting it to an Int (in org/mp4parser/isoviewer/views/BoxPane.kt lines 137-138). This causes a crash (java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer) Fix: solved by renaming 'flags' to 'lFlags'

Wow, that was long.

Sorry for that. But I really needed to analyze and try to fix corrupt MPEG4 files, and your work clearly is the best open-source Java MPEG4 parser. So although I see there was not much recent activity on it, I really hope you will consider merging this PR to hopefully make mp4parser even better :-)

Have a nice day,

Vincent