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

Mimic Exif hierarchy in CRX #224

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions MetadataExtractor/Formats/QuickTime/QuickTimeMetadataReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,34 +72,34 @@ static void SetRotation(QuickTimeTrackHeaderDirectory directory)
}
}

void UuidHandler(AtomCallbackArgs a)
void CrxUuidHandler(AtomCallbackArgs a)
{
switch (a.TypeString)
{
case "CMT1":
{
var handler = new QuickTimeTiffHandler<ExifIfd0Directory>(directories);
var handler = new ExifTiffHandler(directories);
var reader = new IndexedSeekingReader(a.Stream, (int)a.Reader.Position);
TiffReader.ProcessTiff(reader, handler);
break;
}
case "CMT2":
{
var handler = new QuickTimeTiffHandler<ExifSubIfdDirectory>(directories);
var handler = new QuickTimeTiffHandler<ExifSubIfdDirectory, ExifIfd0Directory>(directories);
var reader = new IndexedSeekingReader(a.Stream, (int)a.Reader.Position);
TiffReader.ProcessTiff(reader, handler);
break;
}
case "CMT3":
{
var handler = new QuickTimeTiffHandler<CanonMakernoteDirectory>(directories);
var handler = new QuickTimeTiffHandler<CanonMakernoteDirectory, ExifSubIfdDirectory>(directories);
var reader = new IndexedSeekingReader(a.Stream, (int)a.Reader.Position);
TiffReader.ProcessTiff(reader, handler);
break;
}
case "CMT4":
{
var handler = new QuickTimeTiffHandler<GpsDirectory>(directories);
var handler = new QuickTimeTiffHandler<GpsDirectory, ExifIfd0Directory>(directories);
var reader = new IndexedSeekingReader(a.Stream, (int)a.Reader.Position);
TiffReader.ProcessTiff(reader, handler);
break;
Expand Down Expand Up @@ -137,11 +137,14 @@ void MoovHandler(AtomCallbackArgs a)
}
case "uuid":
{
var CR3 = new byte[] { 0x85, 0xc0, 0xb6, 0x87, 0x82, 0x0f, 0x11, 0xe0, 0x81, 0x11, 0xf4, 0xce, 0x46, 0x2b, 0x6a, 0x48 };
var uuid = a.Reader.GetBytes(CR3.Length);
if (CR3.RegionEquals(0, CR3.Length, uuid))
var CRX = new byte[] { 0x85, 0xc0, 0xb6, 0x87, 0x82, 0x0f, 0x11, 0xe0, 0x81, 0x11, 0xf4, 0xce, 0x46, 0x2b, 0x6a, 0x48 };
if (a.BytesLeft >= CRX.Length)
{
QuickTimeReader.ProcessAtoms(stream, UuidHandler, a.BytesLeft);
var uuid = a.Reader.GetBytes(CRX.Length);
if (CRX.RegionEquals(0, CRX.Length, uuid))
{
QuickTimeReader.ProcessAtoms(stream, CrxUuidHandler, a.BytesLeft);
}
}
break;
}
Expand Down
10 changes: 8 additions & 2 deletions MetadataExtractor/Formats/QuickTime/QuickTimeTiffHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
using MetadataExtractor.Formats.Exif;
using MetadataExtractor.Formats.Tiff;
using System.Collections.Generic;
using System.Linq;

namespace MetadataExtractor.Formats.QuickTime
{
public sealed class QuickTimeTiffHandler<T> : ExifTiffHandler
public sealed class QuickTimeTiffHandler<T, TParent> : ExifTiffHandler
where T : Directory, new()
where TParent : Directory
{
public QuickTimeTiffHandler([NotNull] List<Directory> directories)
: base(directories)
Expand All @@ -22,7 +24,11 @@ public override void SetTiffMarker(int marker)
{
throw new TiffProcessingException($"Unexpected TIFF marker: 0x{marker:X}");
}
PushDirectory(new T());
var directory = new T
{
Parent = Directories.OfType<TParent>().FirstOrDefault()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is prone to selecting the wrong parent. The first one might not be the correct one. I would suggest adding the parent directory as a constructor parameter, storing it in a field, then adding it here.

You can then remove the TParent type parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean an additional directory stack in QucikTimeMetadataReader? I'm not sure if adding state to the latter is better than the current solution.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't SetTiffMarker only called once? I don't think a stack is needed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the state would be in this class (QuickTimeTiffHandler).

Copy link
Contributor Author

@dmitry-shechtman dmitry-shechtman Sep 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reader encounters the atoms in order:

  • CMT1
  • CMT2
  • CMT3
  • CMT4

For each of those, a new ITiffHandler instance is created, meaning a directory stack created from scratch every time.

N.B. I don't see a scenario in which more than one IFD0 directory exists (at least at this stage).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying. I still feel it should be possible to pass the parent forward somehow rather than looking it up, but will have to pull down your branch to play around with it.

};
PushDirectory(directory);
}
}
}