feat:add crude CR3 support #624
Conversation
|
Sorry, there was an error in the code that caused misaligning of the atoms and some headers (i.e. the ExifSubIFD not being read). I've updated the PR, here's now the full parse result: |
|
@drewnoakes I'm not sure regarding required tests for the cr3 format parsing. I would be glad to support here but I guess I need some detail on best practices/required configuration, I'm not too deep into the libraries structure |
|
Thanks very much for tackling this @TSGames! I appreciate the comprehensive PR. The Java and .NET implementations for QuickTime handling are quite different which has historically made porting code between them difficult. I don't know which of the implementations produces the more preferable output either. At some point we should consolidate them. This will require diving into the specs, understanding both of the current implementations, selecting the best approach for both and moving forwards with what was learned. We'd also likely need more test data as we're pretty thin on that.
Generally, in this library, we use the regression tests images instead of unit tests. Unit tests are fine where we want to validate some small component, but generally most of the code here is well covered by the regression test suite. It's also not great to check tonnes of image data in alongside the source code. Running your PR against that regression test suite shows a changes that I'm unsure about and would require deeper investigation. For example: -[QuickTime - 0x1000] Major Brand = MP4 Base Media v1 [IS0 14496-12:2003]
-[QuickTime - 0x1001] Minor Version = 512
+[QuickTime - 0x1000] Major Brand = Unknown
+[QuickTime - 0x1001] Minor Version = 0
[QuickTime - 0x1002] Compatible Brands = [MP4 Base Media v1 [IS0 14496-12:2003], MP4 Base Media v2 [ISO 14496-12:2005], MP4 Base w/ AVC ext [ISO 14496-12:2005], MP4 v1 [ISO 14496-1:ch13]]
[QuickTime - 0x0100] Creation Time = Fri Jan 01 10:00:00 +10:00 1904
[QuickTime - 0x0101] Modification Time = Fri Jan 01 10:00:00 +10:00 1904
-[QuickTime - 0x0103] Duration = 0
+[QuickTime - 0x0103] Duration = 441
[QuickTime - 0x0102] Media Time Scale = 1000
-[QuickTime - 0x0104] Duration in Seconds = 00:00:00
-[QuickTime - 0x0105] Preferred Rate = 27.562
-[QuickTime - 0x0106] Preferred Volume = 0.125
-[QuickTime - 0x0107] Preview Time = 1073741824
+[QuickTime - 0x0104] Duration in Seconds = 00:00:01
+[QuickTime - 0x0105] Preferred Rate = 1
+[QuickTime - 0x0106] Preferred Volume = 1
+[QuickTime - 0x0107] Preview Time = 0
[QuickTime - 0x0108] Preview Duration = 0I'm not sure if this is more or less correct than before though! There are instructions on running the regression tests at https://github.com/drewnoakes/metadata-extractor/wiki/Working-with-test-images. Let me know if anything's unclear and I can improve the docs. Are you able to produce and donate a CR3 file to the project that we can use for regression testing? It's hard to validate the correctness of this PR without one. Also are you interested in digging more into the spec here and uncovering the approach we should take for both implementations? |
Actually I forgot that @thorsted kindly volunteered their data in drewnoakes/metadata-extractor-images#28 (comment). I'm working to bring them in now, in a submodule. |
|
Hi @drewnoakes , thanks for taking your time looking over the PR! |
|
Cloning https://github.com/drewnoakes/metadata-extractor-images/ (with submodules) will bring down some CR3 files to test with. I ran your PR against them a little earlier and there's a lot more metadata coming through now! We just need to understand why the other changes are there. Hopefully it's something straightforward. |
|
Sounds good, I'm just stuck a bit with the regression test still producing diffs even when running on master (Issue #626 ). Maybe you have any idea how do I get them running stable? Thanks! |
|
Hi @drewnoakes , the side effect is caused since there is the new My (naive?) approach was to check for the file type cr3/crx and only reading the header in this case - I'm not sure if that's valid in your opinion. The only other chance I see would be to "reset" the With this fix, the regression testing didn't caused any side effects for me, may you can check it as well. Thanks! |
|
Hi, just wanted to bump this, can I provide any more changes or can you help me on how I should proceed? @drewnoakes |
|
bump @drewnoakes |
|
Apologies @TSGames, holiday and the compression either side of it has taken up all my free time recently. I hope to get back to this soon. Thanks for your patience. |
|
No worries, let me know if you need any more information. |
This solves issue #374 by porting the Dotnet implementation to Java
Readout of a random CR3 file gave the following data:
All credits to @dmitry-shechtman