Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e4c17ec
Increase Identify performance and reduce allocations
JimBobSquarePants Apr 28, 2018
2c0fb6d
Merge branch 'master' into js/better-identify
JimBobSquarePants Apr 28, 2018
f6b374d
Golang is skipping EXIF reading when it shouldn't.
JimBobSquarePants Apr 28, 2018
b44e30e
Pool buffer
JimBobSquarePants Apr 28, 2018
f64f2b0
Ensure buffer is always aligned and fix tests
JimBobSquarePants Apr 28, 2018
75050c5
Ben Adams is a wizard
JimBobSquarePants Apr 28, 2018
f598561
Merge branch 'master' into js/better-identify
JimBobSquarePants Apr 28, 2018
2662853
Faster multibyte reading and fix test
JimBobSquarePants Apr 29, 2018
e927ed1
Merge branch 'master' into js/better-identify
JimBobSquarePants Apr 29, 2018
b52b872
Slight perf tweak plus duplicate refactoring
JimBobSquarePants Apr 29, 2018
3ef9480
Merge branch 'master' into js/better-identify
JimBobSquarePants May 1, 2018
a20f5f7
Minor scan decoder optimization
JimBobSquarePants May 1, 2018
b1db217
Refactor buffer-filling to be more like libjpegturbo + add optimizati…
JimBobSquarePants May 1, 2018
b419eb6
Merge branch 'master' into js/better-identify
JimBobSquarePants May 2, 2018
97020f7
fix indentation
antonfirsov May 2, 2018
00fa494
better unit tests for Identify & MetaData parsing
antonfirsov May 2, 2018
0bae6ad
Update external refs
JimBobSquarePants May 3, 2018
934e2bc
Skip SOS contents when parsing metadata only
JimBobSquarePants May 3, 2018
803ce2a
Merge branch 'js/better-identify' of https://github.com/SixLabors/Ima…
JimBobSquarePants May 3, 2018
4a895b4
Better sanitation for scan decoder.
JimBobSquarePants May 4, 2018
2d2477b
Merge branch 'master' into js/better-identify
JimBobSquarePants May 4, 2018
694972d
Break out after SOS marker
JimBobSquarePants May 5, 2018
34dc609
Add skip test
JimBobSquarePants May 5, 2018
2e96ff6
Improve readability in scan decoder (less params, no discernable perf…
JimBobSquarePants May 7, 2018
5095426
Merge branch 'master' into js/better-identify
antonfirsov May 7, 2018
2f29567
Merge branch 'master' into js/better-identify
JimBobSquarePants May 8, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ public OrigComponent(byte identifier, int index)
/// </summary>
/// <param name="memoryManager">The <see cref="MemoryManager"/> to use for buffer allocations.</param>
/// <param name="decoder">The <see cref="OrigJpegDecoderCore"/> instance</param>
/// <param name="metadataOnly">Whether to decode metadata only. If this is true, memory allocation for SpectralBlocks will not be necessary</param>
public void InitializeDerivedData(MemoryManager memoryManager, OrigJpegDecoderCore decoder, bool metadataOnly)
public void InitializeDerivedData(MemoryManager memoryManager, OrigJpegDecoderCore decoder)
{
// For 4-component images (either CMYK or YCbCrK), we only support two
// hv vectors: [0x11 0x11 0x11 0x11] and [0x22 0x11 0x11 0x22].
Expand All @@ -81,10 +80,7 @@ public void InitializeDerivedData(MemoryManager memoryManager, OrigJpegDecoderCo
this.SubSamplingDivisors = c0.SamplingFactors.DivideBy(this.SamplingFactors);
}

if (!metadataOnly)
{
this.SpectralBlocks = memoryManager.Allocate2D<Block8x8>(this.SizeInBlocks.Width, this.SizeInBlocks.Height, true);
}
this.SpectralBlocks = memoryManager.Allocate2D<Block8x8>(this.SizeInBlocks.Width, this.SizeInBlocks.Height, true);
}

/// <summary>
Expand Down
82 changes: 44 additions & 38 deletions src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ public OrigJpegDecoderCore(Configuration configuration, IJpegDecoderOptions opti
{
this.IgnoreMetadata = options.IgnoreMetadata;
this.configuration = configuration ?? Configuration.Default;
this.HuffmanTrees = OrigHuffmanTree.CreateHuffmanTrees();
this.QuantizationTables = new Block8x8F[MaxTq + 1];
this.Temp = new byte[2 * Block8x8F.Size];
}

Expand All @@ -103,10 +101,10 @@ public OrigJpegDecoderCore(Configuration configuration, IJpegDecoderOptions opti
/// <summary>
/// Gets the huffman trees
/// </summary>
public OrigHuffmanTree[] HuffmanTrees { get; }
public OrigHuffmanTree[] HuffmanTrees { get; private set; }

/// <inheritdoc />
public Block8x8F[] QuantizationTables { get; }
public Block8x8F[] QuantizationTables { get; private set; }

/// <summary>
/// Gets the temporary buffer used to store bytes read from the stream.
Expand Down Expand Up @@ -193,7 +191,6 @@ public Image<TPixel> Decode<TPixel>(Stream stream)
where TPixel : struct, IPixel<TPixel>
{
this.ParseStream(stream);

return this.PostProcessIntoImage<TPixel>();
}

Expand All @@ -204,7 +201,6 @@ public Image<TPixel> Decode<TPixel>(Stream stream)
public IImageInfo Identify(Stream stream)
{
this.ParseStream(stream, true);

return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.MetaData);
}

Expand All @@ -215,7 +211,7 @@ public void Dispose()
{
foreach (OrigComponent component in this.Components)
{
component.Dispose();
component?.Dispose();
}
}

Expand All @@ -233,6 +229,12 @@ public void ParseStream(Stream stream, bool metadataOnly = false)
this.InputStream = stream;
this.InputProcessor = new InputProcessor(stream, this.Temp);

if (!metadataOnly)
{
this.HuffmanTrees = OrigHuffmanTree.CreateHuffmanTrees();
this.QuantizationTables = new Block8x8F[MaxTq + 1];
}

// Check for the Start Of Image marker.
this.InputProcessor.ReadFull(this.Temp, 0, 2);

Expand Down Expand Up @@ -332,10 +334,6 @@ public void ParseStream(Stream stream, bool metadataOnly = false)
case OrigJpegConstants.Markers.SOF2:
this.IsProgressive = marker == OrigJpegConstants.Markers.SOF2;
this.ProcessStartOfFrameMarker(remaining, metadataOnly);
if (metadataOnly && this.isJFif)
{
return;
}

break;
case OrigJpegConstants.Markers.DHT:
Expand All @@ -361,19 +359,24 @@ public void ParseStream(Stream stream, bool metadataOnly = false)

break;
case OrigJpegConstants.Markers.SOS:
if (metadataOnly)
if (!metadataOnly)
Copy link
Member

@antonfirsov antonfirsov May 2, 2018

Choose a reason for hiding this comment

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

After "ignoring" SOS, the decoder will start scanning through the contents of the SOS block using tens (hundreds? thousands?) of FindNextFileMarker() calls returning invalid "file markers". I don't think this is the expected behavior.

Can't this behavior push the decoder into a faulted state leading to ignore some further actual file markers it shouldn't ignore, even in metadata-only state?

Copy link
Member

Choose a reason for hiding this comment

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

Accidentally commented this on OrigJpegDecoderCore instead of PdfJsDecoderCore.

Actually I only investigetad the latter, and the issue is present here! Haven't checked OrigJpegDecoderCore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this marker declare it's length? If so we might be able to simply skip the length.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it should.

However, ProcessSOS does not depend on it as far as I understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can just skip the remaining value same as the other markers.

{
return;
this.ProcessStartOfScanMarker(remaining);
if (this.InputProcessor.ReachedEOF)
{
// If unexpected EOF reached. We can stop processing bytes as we now have the image data.
processBytes = false;
}
}

this.ProcessStartOfScanMarker(remaining);
if (this.InputProcessor.ReachedEOF)
else
{
// If unexpected EOF reached. We can stop processing bytes as we now have the image data.
// It's highly unlikely that APPn related data will be found after the SOS marker
// We should have gathered everything we need by now.
processBytes = false;
}

break;

case OrigJpegConstants.Markers.DRI:
if (metadataOnly)
{
Expand Down Expand Up @@ -425,7 +428,12 @@ public bool IsAtRestartInterval(int mcuCounter)
/// </summary>
private void InitDerivedMetaDataProperties()
{
if (this.isExif)
if (this.isJFif)
{
this.MetaData.HorizontalResolution = this.jFif.XDensity;
this.MetaData.VerticalResolution = this.jFif.YDensity;
}
else if (this.isExif)
{
double horizontalValue = this.MetaData.ExifProfile.TryGetValue(ExifTag.XResolution, out ExifValue horizonalTag)
? ((Rational)horizonalTag.Value).ToDouble()
Expand All @@ -441,11 +449,6 @@ private void InitDerivedMetaDataProperties()
this.MetaData.VerticalResolution = verticalValue;
}
}
else if (this.isJFif)
{
this.MetaData.HorizontalResolution = this.jFif.XDensity;
this.MetaData.VerticalResolution = this.jFif.YDensity;
}
}

/// <summary>
Expand Down Expand Up @@ -675,26 +678,29 @@ private void ProcessStartOfFrameMarker(int remaining, bool metadataOnly)
throw new ImageFormatException("SOF has wrong length");
}

this.Components = new OrigComponent[this.ComponentCount];

for (int i = 0; i < this.ComponentCount; i++)
if (!metadataOnly)
{
byte componentIdentifier = this.Temp[6 + (3 * i)];
var component = new OrigComponent(componentIdentifier, i);
component.InitializeCoreData(this);
this.Components[i] = component;
}
this.Components = new OrigComponent[this.ComponentCount];

int h0 = this.Components[0].HorizontalSamplingFactor;
int v0 = this.Components[0].VerticalSamplingFactor;
for (int i = 0; i < this.ComponentCount; i++)
{
byte componentIdentifier = this.Temp[6 + (3 * i)];
var component = new OrigComponent(componentIdentifier, i);
component.InitializeCoreData(this);
this.Components[i] = component;
}

this.ImageSizeInMCU = this.ImageSizeInPixels.DivideRoundUp(8 * h0, 8 * v0);
int h0 = this.Components[0].HorizontalSamplingFactor;
int v0 = this.Components[0].VerticalSamplingFactor;

this.ColorSpace = this.DeduceJpegColorSpace();
this.ImageSizeInMCU = this.ImageSizeInPixels.DivideRoundUp(8 * h0, 8 * v0);

foreach (OrigComponent component in this.Components)
{
component.InitializeDerivedData(this.configuration.MemoryManager, this, metadataOnly);
this.ColorSpace = this.DeduceJpegColorSpace();

foreach (OrigComponent component in this.Components)
{
component.InitializeDerivedData(this.configuration.MemoryManager, this);
}
}
}

Expand Down
Loading