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

Fix for QuickTime #133

Merged
merged 1 commit into from
Mar 6, 2023
Merged

Fix for QuickTime #133

merged 1 commit into from
Mar 6, 2023

Conversation

sunfish-shogi
Copy link
Contributor

@sunfish-shogi sunfish-shogi commented Mar 3, 2023

#132

Fix not to return error when QuickTime atom has padding bytes.

@sunfish-shogi sunfish-shogi self-assigned this Mar 3, 2023
@sunfish-shogi sunfish-shogi marked this pull request as ready for review March 4, 2023 14:03
@mholt
Copy link

mholt commented Mar 6, 2023

Oh awesome, thanks for working on this!

I finally had a chance to try this with a batch of some .MOV files from the same dataset; however I still see the errors:

processing MP4 metadata: too large box size: type=0x00000020, size=3494, actualBufSize=4

I see it quite a bit on various files (they're from a personal camera roll though so I'm not sure I can share them, regrettably) -- the type values vary, sometimes they're 0x20, other times I see 0x2c, other times 0x19 or 0x1a... and so does the size value. But actualBufSize is always 4.

If it'd be helpful I'll try to look closer to see if we can send you at least one of the video files privately, but it's family media so I'd have to ask our family members.

If there's any other way I can help, or questions I can answer, please let me know! 👍

@sunfish-shogi
Copy link
Contributor Author

sunfish-shogi commented Mar 6, 2023

@mholt
Thank you for your confirmation!

I thought that this issue is related to only old QuickTime atom.
So, this PR changes implementation to allow padding bytes on QuickTime file type.

For example, my .mov file has "ftyp" (File Type) box containing "qt " ("qt" + double spaces) as major/compatible brand as follows:

Screen Shot 2023-03-06 at 12 57 00

I could not find any MP4 file occurring this error other than QuickTime's output.

I have two questions.

  1. What software/service has created your .mov files?
  2. What brand code does your .mov file specify? (major brand and compatible brand)

You can see brand codes by mp4tool(this project) or mp4dump(Bento4) like following commands.

# a. Use mp4tool
# install mp4tool
go install github.com/abema/go-mp4/mp4tool@latest
# dump box tree
mp4tool dump path/to/your.mov

# b. Use mp4dump (Bento4)
mp4dump path/to/your.mov

@mholt
Copy link

mholt commented Mar 6, 2023

I love mp4tool btw, I already have it installed :)

  1. What software/service has created your .mov files?

iPhone camera (iPhone 13 mini)

  1. What brand code does your .mov file specify? (major brand and compatible brand)

Running mp4tool on one of the video fields yields this output:

$ mp4tool dump iPhone\ Backup/Backup/00009234-0A1234A43C34821C/87/871a7e6441595645e74180d1fd1dc59e19f12cb3
[ftyp] Size=20 MajorBrand="qt  " MinorVersion=0 CompatibleBrands=[{CompatibleBrand="qt  "}]
...

The error message for this particular file was too large box size: type=0x0000002c, size=16670, actualBufSize=4 by the way.

It is interesting that I don't get this error message when running mp4tool command, just when calling mp4.ReadBoxStructure().

@sunfish-shogi
Copy link
Contributor Author

It is interesting that I don't get this error message when running mp4tool command, just when calling mp4.ReadBoxStructure().

Oh, I wonder why...
Did you use following commands?

# Use function
(at your go.mod's directory)
go get -u github.com/abema/go-mp4@fix-qt # ... No Error
go get -u github.com/abema/go-mp4@main # ... Error

# Use mp4tool
go install github.com/abema/go-mp4/mp4tool@fix-qt # ... No Error
go install github.com/abema/go-mp4/mp4tool@main # ... Error

@mholt
Copy link

mholt commented Mar 6, 2023

Oh! That did it:

$ go install github.com/abema/go-mp4/mp4tool@latest
$ mp4tool dump iPhone\ Backup/Backup/00009234-0A1234A43C34821C/87/871a7e6441595645e74180d1fd1dc59e19f12cb3
[ftyp] Size=20 MajorBrand="qt  " MinorVersion=0 CompatibleBrands=[{CompatibleBrand="qt  "}]
...
Error: too large box size: type=0x0000002c, size=16670, actualBufSize=4

$ go install github.com/abema/go-mp4/mp4tool@fix-qt
$ mp4tool dump iPhone\ Backup/Backup/00009234-0A1234A43C34821C/87/871a7e6441595645e74180d1fd1dc59e19f12cb3
[ftyp] Size=20 MajorBrand="qt  " MinorVersion=0 CompatibleBrands=[{CompatibleBrand="qt  "}]
...

$ 

I'm not sure why the error wasn't appearing earlier tonight. I know I didn't run that go install command before for the fix-qt branch. I wonder if go get that I ran for my project earlier did that automatically? But anyway, this does seem to indicate the error is fixed.

Does that mean it's able to extract metadata it wasn't able to before?

After re-running my program now too, I don't see the error anymore.

I'm not sure why it apparently compiled with the old/main branch before, even though I did go back and confirm I first ran go get github.com/abmea/go-mp4@fix-qt before running go run, so... anyway, this time it seems to be working!

Processed about 30,000 files now and I don't see any instances of "box size" so that's a good sign. 👍

@sunfish-shogi
Copy link
Contributor Author

@mholt
Thank you for trying again!

Does that mean it's able to extract metadata it wasn't able to before?

Yes. But there are some things to note.

I have not found official information about 4 bytes padding of Apple .mov file yet.
I try to find the information, if it is possible.
But I think detail of old QuickTime specification often is not public.

And this implementation do not keep those bytes.
It means go-mp4 will remove that for .mov file when you do both of reading and writing.

@mholt
Copy link

mholt commented Mar 6, 2023

And this implementation do not keep those bytes.
It means go-mp4 will remove that for .mov file when you do both of reading and writing.

Gotcha. That's OK for my use case since I'm just reading files, not writing.

Thanks again for the patch!

@sunfish-shogi sunfish-shogi merged commit b6a9288 into master Mar 6, 2023
@sunfish-shogi sunfish-shogi deleted the fix-qt branch March 6, 2023 16:54
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

Successfully merging this pull request may close these issues.

None yet

2 participants