Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quicktime/MP4 UUID atom issue #219

Closed
Kimossab opened this issue Dec 24, 2019 · 4 comments
Closed

Quicktime/MP4 UUID atom issue #219

Kimossab opened this issue Dec 24, 2019 · 4 comments

Comments

@Kimossab
Copy link

After this lastest update (version: 1.9.19-201912131005) we're experiencing errors in the analyse function.
After looking through the changes in the lastest release and the stack trace it seems to be related to the UUID atom unpack.

The videos that are causing problems were exported from Adobe After Effects, which uses the UUID atom to store XMP data in MP4 files. Since this last update the UUID atom started being analysed for 360Fly Sensor Data without any checks if the data is actually valid for 360Fly. As such, it crashes on the first unpack because the data is not as expected.

Error: unpack(): Type H: not enough input, need 4, have 0
File: module.audio-video.quicktime
Line: 1657

case 'uuid': // Atom holding 360fly spatial data??
	/* code in this block by Paul Lewis 2019-Oct-31 */
	/*	Sensor Timestamps need to be calculated using the recordings base time at ['quicktime']['moov']['subatoms'][0]['creation_time_unix']. */
	$atom_structure['title'] = '360Fly Sensor Data';

	//Get the UUID ID in first 16 bytes
	$uuid_bytes_read = unpack('H8time_low/H4time_mid/H4time_hi/H4clock_seq_hi/H12clock_seq_low', substr($atom_data, 0, 16));
	$atom_structure['uuid_field_id'] = print_r(implode('-', $uuid_bytes_read), true);

Is there any reason to assume that whenever there's an UUID atom it's related to 360Fly in a Quicktime audio-video file type?

Could there be a way to check that the data is related to 360Fly before running this new code?


Stack trace:

#0 [internal function]: Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(2, 'unpack(): Type ...', '/var/www/html/v...', 1657, Array)
#1 /var/www/html/vendor/james-heinrich/getid3/getid3/module.audio-video.quicktime.php(1657): unpack('H8time_low/H4ti...', '')
#2 /var/www/html/vendor/james-heinrich/getid3/getid3/module.audio-video.quicktime.php(2011): getid3_quicktime->QuicktimeParseAtom('uuid', 45180, '', 7710, Array, false)
#3 /var/www/html/vendor/james-heinrich/getid3/getid3/module.audio-video.quicktime.php(253): getid3_quicktime->QuicktimeParseContainerAtom('\x00\x00\x00lmvhd\x00\x00\x00\x00\xDA!s...', 32, Array, false)
#4 /var/www/html/vendor/james-heinrich/getid3/getid3/module.audio-video.quicktime.php(75): getid3_quicktime->QuicktimeParseAtom('moov', 7686, '\x00\x00\x00lmvhd\x00\x00\x00\x00\xDA!s...', 24, Array, false)
#5 /var/www/html/vendor/james-heinrich/getid3/getid3/getid3.php(634): getid3_quicktime->Analyze()
#6 /var/www/html/app/Http/Controllers/PubController.php(1197): getID3->analyze(Object(Illuminate\Http\UploadedFile))

More detailed stack trace in the HTML file in the following ZIP file:
stack-trace.zip

@JamesHeinrich
Copy link
Owner

Can you provide a small (preferably blank) sample video from After Effects that uses 'uusd' to store XMP data that I can test with?

JamesHeinrich added a commit that referenced this issue Dec 24, 2019
#219
Attempt to parse Quicktime 'uuid' atoms more appropriately. Currently
includes special handling for XML and 360fly data.
Note that [quicktime][uuid] is now returned as an array and may contain
multiple entries.
@JamesHeinrich
Copy link
Owner

I have made some changes to 'uuid' atom handling in ed22a3e
Note the [quicktime][uuid] section is now returned as an array and may contain multiple entries.
XML data should be detected and returned under an [xml] subkey.
I have attempted to isolate the 360fly codeblock under a version string check that works for the single sample file I have, it may need tweaking but should no longer be blindly executed on all 'uuid' atoms.

A blank sample file with XML uuid data would still be appreciated if it's easily created.

@Kimossab
Copy link
Author

I attempted using version 1.9.19-201912241213 and the error persisted.

After I analysed more deeply the code I've realized that the problem comes from having an empty UUID atom. The parser for UUID attempts to unpack the data without checking if it's empty or not and results in the error (noteworthy to point out that this is an error because the header says it contains data).

I've noticed that it comes empty from inside the MOOV atom, which was odd because it doesn't belong inside the MOOV and after looking a bit more to the code I think I've discovered the problem.

When parsing the atoms it read 8 bytes from the file to read the header, take the atom size from the header and then reads that amount of bytes. Here's the problem: it attempts to read the amount of bytes that is said in the header, but it doesn't take into account the 8 bytes already read for the header. This results in reading an additional 8 bytes, these are the header of the next atom in the file (in my specific case it's the UUID).

In the Analyze function of module.audio-video.quicktime.php I made some small changes:

while ($offset < $info['avdataend']) {
  if (!getid3_lib::intValueSupported($offset)) {
    $this->error('Unable to parse atom at offset '.$offset.' because beyond '.round(PHP_INT_MAX / 1073741824).'GB limit of PHP filesystem functions');
    break;
  }
  $this->fseek($offset);

  // header size to reduce from the atomsize to read the file
  $atomHeaderSize = 8; 
  $AtomHeader = $this->fread(8);

  $atomsize = getid3_lib::BigEndian2Int(substr($AtomHeader, 0, 4));
  $atomname = substr($AtomHeader, 4, 4);

  // 64-bit MOV patch by jlegateØktnc*com
  if ($atomsize == 1) {
    // additional 8 bytes read
    $atomHeaderSize += 8; 
    $atomsize = getid3_lib::BigEndian2Int($this->fread(8));
  }

  if (($offset + $atomsize) > $info['avdataend']) {
    $info['quicktime'][$atomname]['name']   = $atomname;
    $info['quicktime'][$atomname]['size']   = $atomsize;
    $info['quicktime'][$atomname]['offset'] = $offset;
    $this->error('Atom at offset '.$offset.' claims to go beyond end-of-file (length: '.$atomsize.' bytes)');
    return false;
  }
  if ($atomsize == 0) {
    // Furthermore, for historical reasons the list of atoms is optionally
    // terminated by a 32-bit integer set to 0. If you are writing a program
    // to read user data atoms, you should allow for the terminating 0.
    $info['quicktime'][$atomname]['name']   = $atomname;
    $info['quicktime'][$atomname]['size']   = $atomsize;
    $info['quicktime'][$atomname]['offset'] = $offset;
    break;
  }

  $atomHierarchy = array();
  // since 8 (16 in case of 64-bit MOV) bytes were already read from the file for the header
  // those have to be reduced from the atomsize to read only the data of the atom 
  // and to prevent reading the header of the next atom
  $parsedAtomData = $this->QuicktimeParseAtom($atomname, $atomsize, $this->fread(min($atomsize - $atomHeaderSize, $atom_data_read_buffer_size)), $offset, $atomHierarchy, $this->ParseAllPossibleAtoms);
  $parsedAtomData['name']   = $atomname;
  $parsedAtomData['size']   = $atomsize;
  $parsedAtomData['offset'] = $offset;
  if (in_array($atomname, array('uuid'))) {
    @$info['quicktime'][$atomname][] = $parsedAtomData;
  } else {
    $info['quicktime'][$atomname] = $parsedAtomData;
  }

  $offset += $atomsize;
  $atomcounter++;
}

After these changes it's working fine.

Also, here's a sample .mp4 file that can reproduce the error:
sample.zip

@JamesHeinrich
Copy link
Owner

@Kimossab: sorry, I didn't see this until now (the notification got sent to spam).
I believe all 'uuid' issues are resolved as of v1.9.19-201912291106 (most recent commit dee9eb7) but please reopen this if you're still having problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants