optimize and trak.udta.name

[GoogleCodeExporter]

What steps will reproduce the problem?
1. have an MP4 with trak.udta.name (i.e. any Handbrake encode will does this, 
as it sets the 
Quicktime "name' of an audio track to Stereo or Surround)
2. run mp4file --optimize on said MP4
3. open up the MP4 in Quicktime, and the track name will no longer be present 
(replaced by 
Sound Track 1), and opening it up in a hex editor, you'll see that track name 
is gone as 
compared to the original MP4

a few ancillary notes
*) mp4creator --optimize (1.5.0.1, available as a source tarball at 
http://downloads.sourceforge.net/mpeg4ip/mpeg4ip-1.5.0.1.tar.gz?
modtime=1149927918&big_mirror=0 or on  macports) does not have this issue
*) if I add a fairly "complex" set of metadata (basic set of information for a 
TV show) and 
optimize, it will crash atomic parsley/metax/mp4creator/muxo with various 
errors (memory 
overflow for atomic parsley/metax, MP4ERROR: Atom ReadProperties: atom is too 
small for 
mp4creator/muxo). Running mp4creator verbosely reveals its traversing the atom 
tree and trying 
to read the udta.name atom and finding it has "insufficient data" (1). I'm not 
sure why this only 
happens with a complex set of metadata, I might do some tests later to see if 
there's a field 
causing this, but I doubt it and it would be very time consuming to find out.
*) some notes from konab1end 
KonaB1end: libmp4v2 *thinks* a name atom is .. shall we say a Atom -- ie: just 
other ITMF ilst 
stuff. this is *not* a "FullAtom" which has more fields.
KonaB1end: so since name directly beneath udta (not in ilst), is really a 
FullAtom, mp4optimize 
unfortunately nukes it down to Atom,
KonaB1end: which is a lean-and-mean style.
KonaB1end: hence the error 'too small'

(1)
ReadAtom: type = "name" data-size = 0 (0x0) hdr 8
ReadProperties: insufficient data for property: version pos 0x646a atom end 
0x6469
MP4ERROR: Atom ReadProperties: atom is too small

Original issue reported on code.google.com by buffalob...@gmail.com on 23 Dec 2008 at 8:12

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I believe r13 exposes this issue:
- the atom 'name' is FullAtom when direct child of udta
- the atom 'name' is Atom when direct child of ilst

So we need context-aware NameAtom as some code already attempts to implement 
but it is not activated. eddy 
seemed to be working on this stuff; maybe he can shed some light on the 
situation.

Original comment by Kona8l...@gmail.com on 23 Dec 2008 at 8:27

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I'm incorrect about mp4creator 1.5.0.1 not having the same error at this point 
-- I'm not sure why, but at one 
point it seemed like it didn't. 

mp4creator --optimize or mp4file --optimize on a file containing any audio 
track names will wipe out 
trak.udta.name, but will not cause crashes in MP4 utilities (atomicparsley, 
mp4creator, mp4tags, metaX, 
muxo).

mp4creator --optimize or mp4file --optimize on a file containing any audio 
track names and/or any "reverse 
DNS" style atoms (http://atomicparsley.sourceforge.net/mpeg-4files.html) which 
consist of parental rating 
(i.e. G/PG/PG-13/R) in com.apple.iTunes;iTunEXTC and 
actors/producers/directors/writers in 
com.apple.iTunes;iTunMOVI will crash the MP4 utilities mentioned above and wipe 
out trak.udta.name.

This explains why crashes were occuring only sometimes, it only happens if 
optimization is done on a file 
with the reverse DNS style atoms.

I have 7 sample files (1 clean file prefixed by 0 with an audio track name, 2 
files prefixed by 1 each run 
through mp4file or mp4creator optimization, and 4 files prefixed by 2 each run 
through optimization and one 
set of the reverse DNS tags) available at public.me.com/jpo/. Each file is only 
1.8 MB

Original comment by refulgentis@gmail.com on 23 Dec 2008 at 10:05

[GoogleCodeExporter]

Some more digging for '----' atoms:
        ExpectChildAtom("mean", Required, OnlyOne);
        ExpectChildAtom("name", Required, OnlyOne);
        ExpectChildAtom("data", Required, OnlyOne);

but this seems to be in slight conflict with the spec. 'name' should be 
Optional/OnlyOne and 'data' should be 
Required/Many. not sure if this is a contributor to the issue as I have no mp4 
files with these atoms.

Original comment by Kona8l...@gmail.com on 23 Dec 2008 at 4:18

[GoogleCodeExporter]

Another bit of information as related to 'reverse DNS' style atoms and the iTMF 
spec:
The '----' is a Atom containing:
    a) MeaningAtom  which is a FullAtom
    b) NameAtom which is a FullAtom
    c) DataAtom which is a Atom

and the code is not consistent (b) and (c). Frankly I'm just blown away by 
this; is it really possible that we are 
writing out FullAtom structure for  iTMF data atoms when they should really 
only be Atom ? Need another set 
of eyes on this to verify.

Original comment by Kona8l...@gmail.com on 23 Dec 2008 at 4:46

[GoogleCodeExporter]

Verified. DataAtom is *not* a FullAtom. libmp4v2 has this wrong, and so does 
AtomicParseley. The only 
reason it works is because the FullAtom (a versioned atom) uses 4 bytes for 
version/flags which just so 
happens to be exactly the same size as type_indicator field for DataAtom; and a 
zero value means implicit. 
How lucky. In any case, the way we use it is wrong; and it's extremely poor 
form.

However, the problem with NameAtom is more pronounced, there is no lucky field 
overlap. The bytes used in 
NameAtom (ie: string), end up taking the place of what is supposed to be 
version/flags. For comparison 
purposes, AtomicParsley defines NameAtom as a VERSIONED_ATOM in their 
terminology, which is a FullAtom.

If I had to guess, the effect of this would be; if libmp4v2 is used to create a 
NameAtom (eg. from HB), and 
then the m4v file is inspected from QuickTime player, it might very well be 
missing the first 4 bytes 
(characters). If the value is 4 chars or less, then maybe this is when the 
issue becomes more evident.

Original comment by Kona8l...@gmail.com on 23 Dec 2008 at 5:22

[GoogleCodeExporter]

marking as fixed; relevant changesets: r191,r194 .

Original comment by Kona8l...@gmail.com on 10 Jan 2009 at 9:09

• Changed state: Fixed