Introduce BufferReader and eliminate a bunch of allocations #393

iamcarbon · 2024-01-31T18:23:58Z

This PR introduces a ref struct based BufferReader that can operate directly over a span -- and applies it to eliminate a bunch of random allocations.

This same approach can also be used to optimize various Extract functions with a public API change.

iamcarbon · 2024-01-31T18:25:34Z

@drewnoakes Ready for feedback.

MetadataExtractor/Formats/Png/PngChromaticities.cs

MetadataExtractor/Formats/Png/PngDescriptor.cs

drewnoakes

Looks like a promising approach!

MetadataExtractor/IO/BufferReader.cs

drewnoakes · 2024-01-31T20:32:58Z

MetadataExtractor/IO/BufferReader.cs

+        if (_position + n > _bytes.Length)
+            throw new IOException("End of data reached.");
+
+        _position += unchecked((int)n);


Maybe checked would be better here. What do you think?

That would be safer, yes. Let me spent some time on this file, and go through usual diligence. I was a bit haphazard with copying / pasting here from SequentialByteReader, and overly confident on our test coverage.

I generally rely upon the regression test suite to validate changes, as it flexes loads more code paths than unit tests do. That's how I found the regressions fixed in #391. Do you have that repo cloned locally? Let me know if you have any questions about getting that running locally. There are some docs at https://github.com/drewnoakes/metadata-extractor/wiki/Working-with-test-images but they might be out of date. The source to run is actually in the metadata-extractor-images repo. If you don't have the Java version built locally, you can comment out the JavaRunner line and rely upon the files already present in the repo. Note that the output files will vary based on Java/.NET version. In general, the main branch should produce the output in that repo (unless I forget to push).

drewnoakes · 2024-01-31T20:33:24Z

MetadataExtractor/IO/BufferReader.cs

+        if (n < 0)
+            throw new ArgumentException("n must be zero or greater.");
+
+        _position += unchecked((int)n);


drewnoakes · 2024-01-31T20:35:00Z

MetadataExtractor/IO/BufferReader.cs

+    {
+        Span<byte> bytes = stackalloc byte[2];
+
+        GetBytes(bytes);


This copy could be avoided I think. Can't we just slice the inner array? Ditto elsewhere.

drewnoakes · 2024-01-31T20:39:18Z

MetadataExtractor/IO/BufferReader.cs

+    private int _position = 0;
+    private bool _isBigEndian = isBigEndian;
+
+    public long Position => _position;


Consider returning int here. We don't really support long values.

Agreed. This was modeled after SequentialByteReader, but this only works on 32 bit spans.

Arrays don't support long indexing, but Streams do. Some image files can be over 2GB, so it's important that we handle long streams correctly.

drewnoakes · 2024-01-31T20:40:52Z

MetadataExtractor/IO/BufferReader.cs

+
+    public bool TrySkip(long n)
+    {
+        if (n < 0)


If we are keeping this type internal, these argument validations could be debug asserts, to avoid overhead in release binaries.

Agreed. No reason for this to be public, so we can continue to improve.

drewnoakes · 2024-01-31T20:44:05Z

Did my comments come through? GitHub mobile issues again? Will be at a desktop shortly.

drewnoakes · 2024-02-01T05:32:23Z

I just ran your PR branch (merged with main) across the regression test suite and hit this assert:

---------------------------
Assertion Failed: Abort=Quit, Retry=Debug, Ignore=Continue
---------------------------
attempted to read past end of data



   at MetadataExtractor.IO.BufferReader.GetByte() in D:\repos\metadata-extractor-dotnet\MetadataExtractor\IO\BufferReader.cs:line 17

   at MetadataExtractor.Formats.Jpeg.JpegReader.Extract(JpegSegment segment) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\Formats\Jpeg\JpegReader.cs:line 52

   at MetadataExtractor.Formats.Jpeg.JpegReader.<ReadJpegSegments>b__3_0(JpegSegment segment) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\Formats\Jpeg\JpegReader.cs:line 21

   at System.Linq.Enumerable.WhereSelectListIterator`2.MoveNext()

   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)

   at MetadataExtractor.Formats.Jpeg.JpegMetadataReader.ProcessJpegSegments(IEnumerable`1 readers, ICollection`1 segments) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\Formats\Jpeg\JpegMetadataReader.cs:line 88

   at MetadataExtractor.Formats.Jpeg.JpegMetadataReader.Process(Stream stream, ICollection`1 readers) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\Formats\Jpeg\JpegMetadataReader.cs:line 77

   at MetadataExtractor.Formats.Jpeg.JpegMetadataReader.ReadMetadata(Stream stream, ICollection`1 readers) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\Formats\Jpeg\JpegMetadataReader.cs:line 47

   at MetadataExtractor.ImageMetadataReader.ReadMetadata(Stream stream) in D:\repos\metadata-extractor-dotnet\MetadataExtractor\ImageMetadataReader.cs:line 81

   at MetadataExtractor.MediaLibraryProcessor.DotNetRunner.ProcessDirectory(String path, IReadOnlyList`1 handlers, String relativePath, TextWriter log) in D:\repos\metadata-extractor-images\src\dotnet\MetadataExtractor.MediaLibraryProcessor\DotNet\DotNetRunner.cs:line 79

   at MetadataExtractor.MediaLibraryProcessor.DotNetRunner.ProcessDirectory(String path, IReadOnlyList`1 handlers, String relativePath, TextWriter log) in D:\repos\metadata-extractor-images\src\dotnet\MetadataExtractor.MediaLibraryProcessor\DotNet\DotNetRunner.cs:line 62

   at MetadataExtractor.MediaLibraryProcessor.DotNetRunner.ProcessDirectory(String path, IReadOnlyList`1 handlers, String relativePath, TextWriter log) in D:\repos\metadata-extractor-images\src\dotnet\MetadataExtractor.MediaLibraryProcessor\DotNet\DotNetRunner.cs:line 62

   at MetadataExtractor.MediaLibraryProcessor.DotNetRunner.<RunAsync>d__0.MoveNext() in D:\repos\metadata-extractor-images\src\dotnet\MetadataExtractor.MediaLibraryProcessor\DotNet\DotNetRunner.cs:line 38

   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)

   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)

   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()

   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()

   at System.Threading.ThreadPoolWorkQueue.Dispatch()


---------------------------
Abort   Retry   Ignore   
---------------------------

drewnoakes · 2024-02-01T06:15:29Z

The failing file is metadata-extractor-images\jpg\ImageTestSuite\c8bc97335529d069a753c67475b8c82c.jpg. It's failing here:

https://github.com/iamcarbon/metadata-extractor-dotnet/blob/db65fa5c81af218adfe9e6b2645860ff2b58f76d/MetadataExtractor/Formats/Jpeg/JpegReader.cs#L52

Looking at that code, we don't validate that the segment span is long enough for the read operations we want to perform. We need to either add that validation, or reintroduce the precondition and throw an IOException. I think the former is reasonable.

drewnoakes · 2024-02-01T06:24:20Z

Another hit in metadata-extractor-images\png\ImageTestSuite\e76546768d4a8f2f4c39339345c7614c.png here: https://github.com/iamcarbon/metadata-extractor-dotnet/blob/db65fa5c81af218adfe9e6b2645860ff2b58f76d/MetadataExtractor/Formats/Png/PngMetadataReader.cs#L322

drewnoakes · 2024-02-01T07:01:20Z

I was able to fix all asserts in the regression suite with the following diff:

diff --git a/MetadataExtractor/Formats/Jpeg/JpegReader.cs b/MetadataExtractor/Formats/Jpeg/JpegReader.cs
index 78499c0a..60a02fd2 100644
--- a/MetadataExtractor/Formats/Jpeg/JpegReader.cs
+++ b/MetadataExtractor/Formats/Jpeg/JpegReader.cs
@@ -31,6 +31,14 @@ namespace MetadataExtractor.Formats.Jpeg

             var reader = new BufferReader(segment.Span, isBigEndian: true);

+            const int JpegHeaderSize = 1 + 2 + 2 + 1;
+
+            if (segment.Span.Length < JpegHeaderSize)
+            {
+                directory.AddError("Insufficient bytes for JPEG segment header.");
+                return directory;
+            }
+
             try
             {
                 directory.Set(JpegDirectory.TagDataPrecision, reader.GetByte());
@@ -41,6 +49,14 @@ namespace MetadataExtractor.Formats.Jpeg

                 directory.Set(JpegDirectory.TagNumberOfComponents, componentCount);

+                const int JpegComponentSize = 1 + 1 + 1;
+
+                if (segment.Span.Length < JpegHeaderSize + componentCount * JpegComponentSize)
+                {
+                    directory.AddError("Insufficient bytes for JPEG the requested number of JPEG components.");
+                    return directory;
+                }
+
                 // For each component, there are three bytes of data:
                 // 1 - Component ID: 1 = Y, 2 = Cb, 3 = Cr, 4 = I, 5 = Q
                 // 2 - Sampling factors: bit 0-3 vertical, 4-7 horizontal
diff --git a/MetadataExtractor/Formats/Png/PngMetadataReader.cs b/MetadataExtractor/Formats/Png/PngMetadataReader.cs
index 21a0742a..40e7d8bf 100644
--- a/MetadataExtractor/Formats/Png/PngMetadataReader.cs
+++ b/MetadataExtractor/Formats/Png/PngMetadataReader.cs
@@ -295,35 +295,52 @@ namespace MetadataExtractor.Formats.Png
             }
             else if (chunkType == PngChunkType.tIME)
             {
-                var reader = new BufferReader(bytes, isBigEndian: true);
-                var year = reader.GetUInt16();
-                var month = reader.GetByte();
-                int day = reader.GetByte();
-                int hour = reader.GetByte();
-                int minute = reader.GetByte();
-                int second = reader.GetByte();
                 var directory = new PngDirectory(PngChunkType.tIME);
-                if (DateUtil.IsValidDate(year, month, day) && DateUtil.IsValidTime(hour, minute, second))
+
+                if (bytes.Length < 2 + 1 + 1 + 1 + 1 + 1)
                 {
-                    var time = new DateTime(year, month, day, hour, minute, second, DateTimeKind.Unspecified);
-                    directory.Set(PngDirectory.TagLastModificationTime, time);
+                    directory.AddError("Insufficient bytes for PNG tIME chunk.");
                 }
                 else
                 {
-                    directory.AddError($"PNG tIME data describes an invalid date/time: year={year} month={month} day={day} hour={hour} minute={minute} second={second}");
+                    var reader = new BufferReader(bytes, isBigEndian: true);
+                    var year = reader.GetUInt16();
+                    var month = reader.GetByte();
+                    int day = reader.GetByte();
+                    int hour = reader.GetByte();
+                    int minute = reader.GetByte();
+                    int second = reader.GetByte();
+                    if (DateUtil.IsValidDate(year, month, day) && DateUtil.IsValidTime(hour, minute, second))
+                    {
+                        var time = new DateTime(year, month, day, hour, minute, second, DateTimeKind.Unspecified);
+                        directory.Set(PngDirectory.TagLastModificationTime, time);
+                    }
+                    else
+                    {
+                        directory.AddError($"PNG tIME data describes an invalid date/time: year={year} month={month} day={day} hour={hour} minute={minute} second={second}");
+                    }
                 }
                 yield return directory;
             }
             else if (chunkType == PngChunkType.pHYs)
             {
-                var reader = new BufferReader(bytes, isBigEndian: true);
-                var pixelsPerUnitX = reader.GetInt32();
-                var pixelsPerUnitY = reader.GetInt32();
-                var unitSpecifier = reader.GetSByte();
                 var directory = new PngDirectory(PngChunkType.pHYs);
-                directory.Set(PngDirectory.TagPixelsPerUnitX, pixelsPerUnitX);
-                directory.Set(PngDirectory.TagPixelsPerUnitY, pixelsPerUnitY);
-                directory.Set(PngDirectory.TagUnitSpecifier, unitSpecifier);
+
+                if (bytes.Length < 4 + 4 + 1)
+                {
+                    directory.AddError("Insufficient bytes for PNG pHYs chunk.");
+                }
+                else
+                {
+                    var reader = new BufferReader(bytes, isBigEndian: true);
+                    var pixelsPerUnitX = reader.GetInt32();
+                    var pixelsPerUnitY = reader.GetInt32();
+                    var unitSpecifier = reader.GetSByte();
+                    directory.Set(PngDirectory.TagPixelsPerUnitX, pixelsPerUnitX);
+                    directory.Set(PngDirectory.TagPixelsPerUnitY, pixelsPerUnitY);
+                    directory.Set(PngDirectory.TagUnitSpecifier, unitSpecifier);
+                }
+
                 yield return directory;
             }
             else if (chunkType.Equals(PngChunkType.sBIT))

That said, there are other usages of the reader that aren't validating byte length, so maybe a debug assertion is not enough.

Also, there are a few diffs created by this PR that need to be investigated before we can merge this.

Full regression diff

diff --git a/jpg/ImageTestSuite/metadata/dotnet/c8bc97335529d069a753c67475b8c82c.jpg.txt b/jpg/ImageTestSuite/metadata/dotnet/c8bc97335529d069a753c67475b8c82c.jpg.txt
index 8a968f55..120d8200 100644
--- a/jpg/ImageTestSuite/metadata/dotnet/c8bc97335529d069a753c67475b8c82c.jpg.txt
+++ b/jpg/ImageTestSuite/metadata/dotnet/c8bc97335529d069a753c67475b8c82c.jpg.txt
@@ -1,14 +1,13 @@
 FILE: c8bc97335529d069a753c67475b8c82c.jpg
 TYPE: JPEG
 
-[ERROR: JPEG] End of data reached.
+[ERROR: JPEG] Insufficient bytes for JPEG the requested number of JPEG components.
 
 [JPEG - 0xfffffffd] Compression Type = Baseline
 [JPEG - 0x0000] Data Precision = 159 bits
 [JPEG - 0x0001] Image Height = 256 pixels
 [JPEG - 0x0003] Image Width = 529 pixels
 [JPEG - 0x0005] Number of Components = 3
-[JPEG - 0x0006] Component 1 = Unknown (17) component: Quantization table 63, Sampling factors 0 horiz/0 vert
 
 [Ducky - 0x0001] Quality = 60
 
diff --git a/jpg/metadata/diff/Issue 339.jpg.txt b/jpg/metadata/diff/Issue 339.jpg.txt
index 71f926d8..89cd66af 100644
--- a/jpg/metadata/diff/Issue 339.jpg.txt	
+++ b/jpg/metadata/diff/Issue 339.jpg.txt	
@@ -110,7 +110,8 @@
        [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
        [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
        [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-       [Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+JAVA   [Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+DOTNET [Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
        [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
        [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
        [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/jpg/metadata/dotnet/Issue 339.jpg.txt b/jpg/metadata/dotnet/Issue 339.jpg.txt
index 11af269d..7a08413b 100644
--- a/jpg/metadata/dotnet/Issue 339.jpg.txt	
+++ b/jpg/metadata/dotnet/Issue 339.jpg.txt	
@@ -110,7 +110,7 @@ TYPE: JPEG
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
 [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/jpg/metadata/dotnet/Olympus E-M1.jpg.txt b/jpg/metadata/dotnet/Olympus E-M1.jpg.txt
index efb01697..acbde5bc 100644
--- a/jpg/metadata/dotnet/Olympus E-M1.jpg.txt	
+++ b/jpg/metadata/dotnet/Olympus E-M1.jpg.txt	
@@ -105,7 +105,7 @@ TYPE: JPEG
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto (Keep Warm Color Off)
diff --git a/jpg/metadata/dotnet/Olympus E-M5.jpg.txt b/jpg/metadata/dotnet/Olympus E-M5.jpg.txt
index 9a484704..6a7b3ef8 100644
--- a/jpg/metadata/dotnet/Olympus E-M5.jpg.txt	
+++ b/jpg/metadata/dotnet/Olympus E-M5.jpg.txt	
@@ -105,7 +105,7 @@ TYPE: JPEG
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/jpg/metadata/dotnet/Olympus PEN E-P3.jpg.txt b/jpg/metadata/dotnet/Olympus PEN E-P3.jpg.txt
index be2c3e57..8d67c7e7 100644
--- a/jpg/metadata/dotnet/Olympus PEN E-P3.jpg.txt	
+++ b/jpg/metadata/dotnet/Olympus PEN E-P3.jpg.txt	
@@ -106,7 +106,7 @@ TYPE: JPEG
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/orf/metadata/diff/OM Sytem OM-1.orf.txt b/orf/metadata/diff/OM Sytem OM-1.orf.txt
index cf88dbeb..fe44b12c 100644
--- a/orf/metadata/diff/OM Sytem OM-1.orf.txt	
+++ b/orf/metadata/diff/OM Sytem OM-1.orf.txt	
@@ -134,7 +134,7 @@ DOTNET [Olympus Camera Settings - 0x0400] Flash Mode = Fill-in
 DOTNET [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 DOTNET [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 DOTNET [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-DOTNET [Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+DOTNET [Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 DOTNET [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 DOTNET [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 DOTNET [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/OM Sytem OM-1.orf.txt b/orf/metadata/dotnet/OM Sytem OM-1.orf.txt
index 847d43fb..c329c63d 100644
--- a/orf/metadata/dotnet/OM Sytem OM-1.orf.txt	
+++ b/orf/metadata/dotnet/OM Sytem OM-1.orf.txt	
@@ -125,7 +125,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus E-M10.orf.txt b/orf/metadata/dotnet/Olympus E-M10.orf.txt
index ff61af1b..18b6103b 100644
--- a/orf/metadata/dotnet/Olympus E-M10.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E-M10.orf.txt	
@@ -104,7 +104,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus E-M5.orf.txt b/orf/metadata/dotnet/Olympus E-M5.orf.txt
index d10f6976..0b6aa19e 100644
--- a/orf/metadata/dotnet/Olympus E-M5.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E-M5.orf.txt	
@@ -104,7 +104,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus E-PL7.orf.txt b/orf/metadata/dotnet/Olympus E-PL7.orf.txt
index c10d5e3b..d21aaded 100644
--- a/orf/metadata/dotnet/Olympus E-PL7.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E-PL7.orf.txt	
@@ -104,7 +104,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/orf/metadata/dotnet/Olympus E-PM1.orf.txt b/orf/metadata/dotnet/Olympus E-PM1.orf.txt
index 8941edf6..7935e23f 100644
--- a/orf/metadata/dotnet/Olympus E-PM1.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E-PM1.orf.txt	
@@ -101,7 +101,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus E30.orf.txt b/orf/metadata/dotnet/Olympus E30.orf.txt
index 7c287341..74e11895 100644
--- a/orf/metadata/dotnet/Olympus E30.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E30.orf.txt	
@@ -98,7 +98,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus E600.orf.txt b/orf/metadata/dotnet/Olympus E600.orf.txt
index 6c989653..413f76a7 100644
--- a/orf/metadata/dotnet/Olympus E600.orf.txt	
+++ b/orf/metadata/dotnet/Olympus E600.orf.txt	
@@ -98,7 +98,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
 [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/orf/metadata/dotnet/Olympus PEN E-P3.orf.txt b/orf/metadata/dotnet/Olympus PEN E-P3.orf.txt
index a5c8d1df..d4651b43 100644
--- a/orf/metadata/dotnet/Olympus PEN E-P3.orf.txt	
+++ b/orf/metadata/dotnet/Olympus PEN E-P3.orf.txt	
@@ -101,7 +101,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = 5300K (Fine Weather)
diff --git a/orf/metadata/dotnet/Olympus Pen E-PL1.orf.txt b/orf/metadata/dotnet/Olympus Pen E-PL1.orf.txt
index f2a07917..206eabed 100644
--- a/orf/metadata/dotnet/Olympus Pen E-PL1.orf.txt	
+++ b/orf/metadata/dotnet/Olympus Pen E-PL1.orf.txt	
@@ -100,7 +100,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus TG-4.orf.txt b/orf/metadata/dotnet/Olympus TG-4.orf.txt
index 5c92ea2d..0466c55a 100644
--- a/orf/metadata/dotnet/Olympus TG-4.orf.txt	
+++ b/orf/metadata/dotnet/Olympus TG-4.orf.txt	
@@ -86,7 +86,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = 0
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a (x4)
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a (x4)
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/orf/metadata/dotnet/Olympus XZ1.orf.txt b/orf/metadata/dotnet/Olympus XZ1.orf.txt
index 7bb5b45c..885ea28c 100644
--- a/orf/metadata/dotnet/Olympus XZ1.orf.txt	
+++ b/orf/metadata/dotnet/Olympus XZ1.orf.txt	
@@ -89,7 +89,7 @@ TYPE: ORF
 [Olympus Camera Settings - 0x0401] Flash Exposure Comp = -2
 [Olympus Camera Settings - 0x0402] Unknown tag (0x0402) = 0
 [Olympus Camera Settings - 0x0403] Flash Remote Control = Off
-[Olympus Camera Settings - 0x0404] Flash Control Mode = Off; 0; 0
+[Olympus Camera Settings - 0x0404] Flash Control Mode = OffOff; 0; 0
 [Olympus Camera Settings - 0x0405] Flash Intensity = n/a
 [Olympus Camera Settings - 0x0406] Manual Flash Strength = n/a
 [Olympus Camera Settings - 0x0500] White Balance 2 = Auto
diff --git a/png/ImageTestSuite/metadata/dotnet/e76546768d4a8f2f4c39339345c7614c.png.txt b/png/ImageTestSuite/metadata/dotnet/e76546768d4a8f2f4c39339345c7614c.png.txt
index a37226bf..ac79ab99 100644
--- a/png/ImageTestSuite/metadata/dotnet/e76546768d4a8f2f4c39339345c7614c.png.txt
+++ b/png/ImageTestSuite/metadata/dotnet/e76546768d4a8f2f4c39339345c7614c.png.txt
@@ -1,7 +1,7 @@
 FILE: e76546768d4a8f2f4c39339345c7614c.png
 TYPE: PNG
 
-[ERROR: Error] Exception reading PNG chunk: End of data reached.
+[ERROR: PNG-pHYs] Insufficient bytes for PNG pHYs chunk.
 
 [PNG-IHDR - 0x0001] Image Width = 32
 [PNG-IHDR - 0x0002] Image Height = 32
@@ -27,7 +27,7 @@ TYPE: PNG
 - PNG-IHDR
 - PNG-gAMA
 - PNG-sBIT
-- Error
+- PNG-pHYs
 - File Type
 - File

The error message diffs would be ok. We could add the same error messages to the Java library too. But the tag value diff is weird and needs fixing.

MetadataExtractor/IO/BufferReader.cs

MetadataExtractor/Formats/Apple/BplistReader.cs

MetadataExtractor/IO/BufferReader.cs

Encoding.ASCII converts non-printable characters to '?', yet 4CC may contain non-printable.

Use `Skip` to skip data and completely ignore it.

drewnoakes · 2024-02-04T11:50:11Z

Thanks again! Great stuff here. I pushed a few changes as I worked with the regression test suite to preserve output parity.

iamcarbon added 12 commits January 30, 2024 10:01

Eliminate allocation in PhotoshopReader

6392506

Use switch expressions

722774a

Add BufferReader

4df2738

Eliminate two allocations in FlirReader

224eade

Eliminate allocation in OlympusMakernoteDirectory

461a6b4

Eliminate various allocations in BplistReader

cbe2d09

Eliminate allocation in JpegReader

a26f6f3

Eliminate allocation in PngChromaticities

d84ae43

Eliminate allocation in PngDescriptor

6f33bc8

Eliminate allocation in PngHeader

5d34c55

Eliminate a few allocations in PngMetadataReader

189d66f

Eliminate allocation in WavFactHandler

76c687e

iamcarbon commented Jan 31, 2024

View reviewed changes

MetadataExtractor/Formats/Png/PngChromaticities.cs Show resolved Hide resolved

iamcarbon commented Jan 31, 2024

View reviewed changes

MetadataExtractor/Formats/Png/PngDescriptor.cs Show resolved Hide resolved

drewnoakes reviewed Jan 31, 2024

View reviewed changes

iamcarbon added 5 commits January 31, 2024 13:01

Use Assert.Debug

27c9846

Ensure BufferReader operates on buffers <= 2GB

5bb02b1

Fix offset in BufferReader.GetBytes

f53892b

Rename BufferReader.Skip -> Advance

a89e1f0

Fix assert order

db65fa5

iamcarbon added 4 commits February 1, 2024 09:13

Verify JpegSegment sizes before reading data

f7ee272

Bring back exceptions on BufferReader

94eec47

Fix switch statement

73143a7

Slice span to avoid extra copy in BufferReader

945686f

Validate buffer length when reading PNG chunks

5205495

drewnoakes reviewed Feb 2, 2024

View reviewed changes

MetadataExtractor/IO/BufferReader.cs Outdated Show resolved Hide resolved

drewnoakes reviewed Feb 2, 2024

View reviewed changes

MetadataExtractor/IO/BufferReader.cs Outdated Show resolved Hide resolved

drewnoakes reviewed Feb 2, 2024

View reviewed changes

MetadataExtractor/Formats/Apple/BplistReader.cs Outdated Show resolved Hide resolved

drewnoakes reviewed Feb 2, 2024

View reviewed changes

MetadataExtractor/IO/BufferReader.cs Show resolved Hide resolved

drewnoakes reviewed Feb 2, 2024

View reviewed changes

MetadataExtractor/IO/BufferReader.cs Outdated Show resolved Hide resolved

iamcarbon and others added 9 commits February 2, 2024 11:00

Ensure the BufferReader can't advance past the end of data

e6d86bd

Improve code readability in BplistReader

f4437f5

Fix length check in PngMetadataReader

59f4294

Fix typo

509973c

Revert change in 4CC strings

a332d84

Encoding.ASCII converts non-printable characters to '?', yet 4CC may contain non-printable.

Make field readonly

7173e0a

Make property readonly

b983a8f

Change Advance to return a span

6c398fd

Use `Skip` to skip data and completely ignore it.

Merge remote-tracking branch 'origin/main' into cq4

1b38f68

drewnoakes merged commit 1f990bf into drewnoakes:main Feb 4, 2024
2 checks passed

drewnoakes added the performance label Feb 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce BufferReader and eliminate a bunch of allocations #393

Introduce BufferReader and eliminate a bunch of allocations #393

iamcarbon commented Jan 31, 2024

iamcarbon commented Jan 31, 2024

drewnoakes left a comment

drewnoakes Jan 31, 2024

iamcarbon Jan 31, 2024 •

edited

Loading

drewnoakes Feb 1, 2024

drewnoakes Jan 31, 2024

drewnoakes Jan 31, 2024

iamcarbon Feb 1, 2024

drewnoakes Jan 31, 2024

iamcarbon Jan 31, 2024

drewnoakes Feb 1, 2024

drewnoakes Jan 31, 2024 •

edited

Loading

iamcarbon Jan 31, 2024

drewnoakes commented Jan 31, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 4, 2024

Introduce BufferReader and eliminate a bunch of allocations #393

Introduce BufferReader and eliminate a bunch of allocations #393

Conversation

iamcarbon commented Jan 31, 2024

iamcarbon commented Jan 31, 2024

drewnoakes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamcarbon Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewnoakes Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewnoakes commented Jan 31, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 1, 2024

drewnoakes commented Feb 4, 2024

iamcarbon Jan 31, 2024 •

edited

Loading

drewnoakes Jan 31, 2024 •

edited

Loading