Opened 12 years ago

Closed 12 years ago

#708 closed defect (fixed)

Bad computed size for atoms with size 0 in MOV files

Reported by: Mihnea Owned by:
Priority: normal Component: avformat
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

I have a MOV file written by a lazy program which can't be bothered to compute the size for the 'chan' atoms it writes, so it just places them at the end of the 'stsd' atom and writes 0 in the size field. I don't know which program it is, I just have the resulting file. According to the spec, size 0 is only allowed for top-level atoms, so strictly speaking the file is malformed, but I see that most parsers (Quicktime included) do not enforce this restriction and take size 0 to mean "extend to the end of the parent atom" instead of "to the end of the file". Ffmpeg does this too, but I think there's an error in the computation. Inside the function mov_read_default() in libavformat/mov.c you have this (slightly edited to show the relevant code path):

a.size = avio_rb32(pb);
a.type = avio_rl32(pb);
total_size += 8;
if (a.size == 0)
    a.size = atom.size - total_size;
a.size -= 8;

The computed size doesn't contain the header size because it's already skipped by incrementing total_size, but then it's skipped again in the last line. The atom comes out 8 bytes short and the function mov_read_chan() aborts the whole parsing process. I think the computed size should be atom.size - total_size + 8.

I've attached a sample MOV file which can be used to reproduce the error. Just do ffmpeg -i audio.mov -vn -acodec copy test.mp4 and ffmpeg will say "error reading header". If you apply the proposed patch, the file works correctly.

PS: the value of the layout tag in the 'chan' atom is 24, which doesn't correspond to anything. I don't know what the authoring program meant by that, but at least it wrote all 3 required fields, so the file can be parsed.

Attachments (2)

audio.mov (1.0 MB ) - added by Mihnea 12 years ago.
mov.patch (370 bytes ) - added by Mihnea 12 years ago.

Download all attachments as: .zip

Change History (8)

by Mihnea, 12 years ago

Attachment: audio.mov added

by Mihnea, 12 years ago

Attachment: mov.patch added

comment:1 by Carl Eugen Hoyos, 12 years ago

Please add complete, uncut ffmpeg -i audio.mov output

comment:2 by Mihnea, 12 years ago

Ok, the original file was about 5 MB and I see there's a 2.5 MB upload limit, so I truncated it after the first megabyte. It should still be enough to reproduce the problem.

Requested ouput:

[mihnea@mihnea-centos6-work ffmpeg-406e964]$ ffmpeg -i /mnt/hellgate_public/audio.mov
ffmpeg version 0.8.7.git-406e964, Copyright (c) 2000-2011 the FFmpeg developers
  built on Dec  5 2011 12:03:50 with gcc 4.4.4 20100726 (Red Hat 4.4.4-13)
  configuration: --prefix=/usr --enable-shared --enable-gpl --enable-libx264 --enable-nonfree --enable-libfaac --enable-debug=3 --disable-stripping
  libavutil    51. 30. 0 / 51. 30. 0
  libavcodec   53. 40. 0 / 53. 40. 0
  libavformat  53. 24. 0 / 53. 24. 0
  libavdevice  53.  4. 0 / 53.  4. 0
  libavfilter   2. 51. 0 /  2. 51. 0
  libswscale    2.  1. 0 /  2.  1. 0
  libpostproc  51.  2. 0 / 51.  2. 0
[mov,mp4,m4a,3gp,3g2,mj2 @ 0xea57a0] error reading header: -1
/mnt/hellgate_public/audio.mov: Operation not permitted

comment:3 by Mihnea, 12 years ago

I forgot to fill in the component and version fields, sorry about that. I don't know how to change them now, please set them to avformat and git-master, respectively.

comment:4 by Carl Eugen Hoyos, 12 years ago

Component: undeterminedavformat
Status: newopen
Version: unspecifiedgit-master

I just committed a change that fixes the sample you provided, unfortunately, I cannot comment on the patch you submitted, please consider sending it to ffmpeg-devel.

Afaict, the chan atom in your sample is definitely broken: Which channel layout does QuickTime report for the sample?

comment:5 by Mihnea, 12 years ago

The commit fixes the problem indeed, but the read position for the next atom will be wrong. Since it's off by only 8 bytes and the parent ends after the atom, ffmpeg won't try to parse the garbage and will go to the next atom correctly (one level up in the hierarchy), but still, atoms with size 0 will always be parsed wrong, since they will be missing 8 bytes.

The atom in the sample is broken indeed, and Quicktime doesn't say anything about channel layout (or I don't know where to look, I checked the "movie inspector" thing). Mediainfo says the audio is stereo with front left and right channel mappings, but I guess it reads that from the AAC stream, not from the MOV headers.

I'll submit my patch to ffmpeg-devel and see what gives, thanks.

comment:6 by Carl Eugen Hoyos, 12 years ago

Resolution: fixed
Status: openclosed

The patch has been applied, thank you!

Note: See TracTickets for help on using tickets.