diff --git a/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs b/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs index 39ee6687b..09baf904b 100644 --- a/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs +++ b/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs @@ -356,11 +356,21 @@ namespace ImageSharp.Formats.Jpg throw new ImageFormatException("Excessive DC component"); } - int deltaDC = decoder.Bits.ReceiveExtend(value, decoder); - this.pointers.Dc[compIndex] += deltaDC; + // TODO: Handle the EOFException and piece together the final progressive scan. + try + { + int deltaDC = decoder.Bits.ReceiveExtend(value, decoder); + this.pointers.Dc[compIndex] += deltaDC; - // b[0] = dc[compIndex] << al; - Block8x8F.SetScalarAt(b, 0, this.pointers.Dc[compIndex] << this.al); + // b[0] = dc[compIndex] << al; + Block8x8F.SetScalarAt(b, 0, this.pointers.Dc[compIndex] << this.al); + } + catch (JpegDecoderCore.EOFException) + { + // Do something clever here. I'm just undoing ReceiveExtend + decoder.Bits.UnreadBits += value; + decoder.Bits.Mask >>= value; + } } if (zig <= this.zigEnd && this.eobRun > 0) @@ -383,7 +393,20 @@ namespace ImageSharp.Formats.Jpg break; } - int ac = decoder.Bits.ReceiveExtend(val1, decoder); + int ac; // = decoder.Bits.ReceiveExtend(val1, decoder); + + // TODO: Handle the EOFException and piece together the final progressive scan. + try + { + ac = decoder.Bits.ReceiveExtend(val1, decoder); + } + catch (JpegDecoderCore.EOFException) + { + // Do something clever here. I'm just undoing ReceiveExtend + decoder.Bits.UnreadBits += val1; + decoder.Bits.Mask >>= val1; + break; + } // b[Unzig[zig]] = ac << al; Block8x8F.SetScalarAt(b, this.pointers.Unzig[zig], ac << this.al); diff --git a/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs b/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs index 2dd547c91..6ba578a3a 100644 --- a/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs @@ -186,164 +186,162 @@ namespace ImageSharp.Formats throw new ImageFormatException("Missing SOI marker."); } - // Process the remaining segments until the End Of Image marker. - bool processBytes = true; - - // we can't currently short circute progressive images so don't try. - while (processBytes) + while (true) { - this.ReadFull(this.Temp, 0, 2); - while (this.Temp[0] != 0xff) + try { - // Strictly speaking, this is a format error. However, libjpeg is - // liberal in what it accepts. As of version 9, next_marker in - // jdmarker.c treats this as a warning (JWRN_EXTRANEOUS_DATA) and - // continues to decode the stream. Even before next_marker sees - // extraneous data, jpeg_fill_bit_buffer in jdhuff.c reads as many - // bytes as it can, possibly past the end of a scan's data. It - // effectively puts back any markers that it overscanned (e.g. an - // "\xff\xd9" EOI marker), but it does not put back non-marker data, - // and thus it can silently ignore a small number of extraneous - // non-marker bytes before next_marker has a chance to see them (and - // print a warning). - // We are therefore also liberal in what we accept. Extraneous data - // is silently ignore - // This is similar to, but not exactly the same as, the restart - // mechanism within a scan (the RST[0-7] markers). - // Note that extraneous 0xff bytes in e.g. SOS data are escaped as - // "\xff\x00", and so are detected a little further down below. - this.Temp[0] = this.Temp[1]; - this.Temp[1] = this.ReadByte(); - } + this.ReadFull(this.Temp, 0, 2); + while (this.Temp[0] != 0xff) + { + // Strictly speaking, this is a format error. However, libjpeg is + // liberal in what it accepts. As of version 9, next_marker in + // jdmarker.c treats this as a warning (JWRN_EXTRANEOUS_DATA) and + // continues to decode the stream. Even before next_marker sees + // extraneous data, jpeg_fill_bit_buffer in jdhuff.c reads as many + // bytes as it can, possibly past the end of a scan's data. It + // effectively puts back any markers that it overscanned (e.g. an + // "\xff\xd9" EOI marker), but it does not put back non-marker data, + // and thus it can silently ignore a small number of extraneous + // non-marker bytes before next_marker has a chance to see them (and + // print a warning). + // We are therefore also liberal in what we accept. Extraneous data + // is silently ignore + // This is similar to, but not exactly the same as, the restart + // mechanism within a scan (the RST[0-7] markers). + // Note that extraneous 0xff bytes in e.g. SOS data are escaped as + // "\xff\x00", and so are detected a little further down below. + this.Temp[0] = this.Temp[1]; + this.Temp[1] = this.ReadByte(); + } - byte marker = this.Temp[1]; - if (marker == 0) - { - // Treat "\xff\x00" as extraneous data. - continue; - } + byte marker = this.Temp[1]; + if (marker == 0) + { + // Treat "\xff\x00" as extraneous data. + continue; + } - while (marker == 0xff) - { - // Section B.1.1.2 says, "Any marker may optionally be preceded by any - // number of fill bytes, which are bytes assigned code X'FF'". - marker = this.ReadByte(); - } + while (marker == 0xff) + { + // Section B.1.1.2 says, "Any marker may optionally be preceded by any + // number of fill bytes, which are bytes assigned code X'FF'". + marker = this.ReadByte(); + } - // End Of Image. - if (marker == JpegConstants.Markers.EOI) - { - break; - } + // End Of Image. + if (marker == JpegConstants.Markers.EOI) + { + break; + } - if (marker >= JpegConstants.Markers.RST0 && marker <= JpegConstants.Markers.RST7) - { - // Figures B.2 and B.16 of the specification suggest that restart markers should - // only occur between Entropy Coded Segments and not after the final ECS. - // However, some encoders may generate incorrect JPEGs with a final restart - // marker. That restart marker will be seen here instead of inside the ProcessSOS - // method, and is ignored as a harmless error. Restart markers have no extra data, - // so we check for this before we read the 16-bit length of the segment. - continue; - } + if (marker >= JpegConstants.Markers.RST0 && marker <= JpegConstants.Markers.RST7) + { + // Figures B.2 and B.16 of the specification suggest that restart markers should + // only occur between Entropy Coded Segments and not after the final ECS. + // However, some encoders may generate incorrect JPEGs with a final restart + // marker. That restart marker will be seen here instead of inside the ProcessSOS + // method, and is ignored as a harmless error. Restart markers have no extra data, + // so we check for this before we read the 16-bit length of the segment. + continue; + } - // Read the 16-bit length of the segment. The value includes the 2 bytes for the - // length itself, so we subtract 2 to get the number of remaining bytes. - this.ReadFull(this.Temp, 0, 2); - int remaining = (this.Temp[0] << 8) + this.Temp[1] - 2; - if (remaining < 0) - { - throw new ImageFormatException("Short segment length."); - } + // Read the 16-bit length of the segment. The value includes the 2 bytes for the + // length itself, so we subtract 2 to get the number of remaining bytes. + this.ReadFull(this.Temp, 0, 2); + int remaining = (this.Temp[0] << 8) + this.Temp[1] - 2; + if (remaining < 0) + { + throw new ImageFormatException("Short segment length."); + } - switch (marker) - { - case JpegConstants.Markers.SOF0: - case JpegConstants.Markers.SOF1: - case JpegConstants.Markers.SOF2: - this.IsProgressive = marker == JpegConstants.Markers.SOF2; - this.ProcessStartOfFrameMarker(remaining); - if (configOnly && this.isJfif) - { - return; - } + switch (marker) + { + case JpegConstants.Markers.SOF0: + case JpegConstants.Markers.SOF1: + case JpegConstants.Markers.SOF2: + this.IsProgressive = marker == JpegConstants.Markers.SOF2; + this.ProcessStartOfFrameMarker(remaining); + if (configOnly && this.isJfif) + { + return; + } - break; - case JpegConstants.Markers.DHT: - if (configOnly) - { - this.Skip(remaining); - } - else - { - this.ProcessDefineHuffmanTablesMarker(remaining); - } + break; + case JpegConstants.Markers.DHT: + if (configOnly) + { + this.Skip(remaining); + } + else + { + this.ProcessDefineHuffmanTablesMarker(remaining); + } - break; - case JpegConstants.Markers.DQT: - if (configOnly) - { - this.Skip(remaining); - } - else - { - this.ProcessDqt(remaining); - } + break; + case JpegConstants.Markers.DQT: + if (configOnly) + { + this.Skip(remaining); + } + else + { + this.ProcessDqt(remaining); + } - break; - case JpegConstants.Markers.SOS: - if (configOnly) - { - return; - } + break; + case JpegConstants.Markers.SOS: + if (configOnly) + { + return; + } - // when this is a progressive image this gets called a number of times - // need to know how many times this should be called in total. - this.ProcessStartOfScan(remaining); - if (!this.IsProgressive) - { - // if this is not a progressive image we can stop processing bytes as we now have the image data. - processBytes = false; - } + this.ProcessStartOfScan(remaining); - break; - case JpegConstants.Markers.DRI: - if (configOnly) - { - this.Skip(remaining); - } - else - { - this.ProcessDefineRestartIntervalMarker(remaining); - } + break; + case JpegConstants.Markers.DRI: + if (configOnly) + { + this.Skip(remaining); + } + else + { + this.ProcessDefineRestartIntervalMarker(remaining); + } - break; - case JpegConstants.Markers.APP0: - this.ProcessApplicationHeader(remaining); - break; - case JpegConstants.Markers.APP1: - this.ProcessApp1Marker(remaining, image); - break; - case JpegConstants.Markers.APP14: - this.ProcessApp14Marker(remaining); - break; - default: - if ((marker >= JpegConstants.Markers.APP0 && marker <= JpegConstants.Markers.APP15) - || marker == JpegConstants.Markers.COM) - { - this.Skip(remaining); - } - else if (marker < JpegConstants.Markers.SOF0) - { - // See Table B.1 "Marker code assignments". - throw new ImageFormatException("Unknown marker"); - } - else - { - throw new ImageFormatException("Unknown marker"); - } + break; + case JpegConstants.Markers.APP0: + this.ProcessApplicationHeader(remaining); + break; + case JpegConstants.Markers.APP1: + this.ProcessApp1Marker(remaining, image); + break; + case JpegConstants.Markers.APP14: + this.ProcessApp14Marker(remaining); + break; + default: + if ((marker >= JpegConstants.Markers.APP0 && marker <= JpegConstants.Markers.APP15) + || marker == JpegConstants.Markers.COM) + { + this.Skip(remaining); + } + else if (marker < JpegConstants.Markers.SOF0) + { + // See Table B.1 "Marker code assignments". + throw new ImageFormatException("Unknown marker"); + } + else + { + throw new ImageFormatException("Unknown marker"); + } - break; + break; + } + } + catch (EOFException) + { + // For non-progressive images this is a simple way to handle a missing EOI + // TODO: For progressive we still have to handle the exception within JpegScanDecoder to include last scan + break; } } diff --git a/tests/ImageSharp.Tests/FileTestBase.cs b/tests/ImageSharp.Tests/FileTestBase.cs index e9343b20d..a2d7397da 100644 --- a/tests/ImageSharp.Tests/FileTestBase.cs +++ b/tests/ImageSharp.Tests/FileTestBase.cs @@ -22,9 +22,11 @@ namespace ImageSharp.Tests // TestFile.Create(TestImages.Png.Pd), // Perf: Enable for local testing only // TestFile.Create(TestImages.Jpeg.Floorplan), // Perf: Enable for local testing only TestFile.Create(TestImages.Jpeg.Calliphora), + // TestFile.Create(TestImages.Jpeg.BadEOF), // Perf: Enable for local testing only + // TestFile.Create(TestImages.Jpeg.BadEOFProgressive), // TestFile.Create(TestImages.Jpeg.Ycck), // Perf: Enable for local testing only // TestFile.Create(TestImages.Jpeg.Cmyk), // Perf: Enable for local testing only - TestFile.Create(TestImages.Jpeg.Turtle), + // TestFile.Create(TestImages.Jpeg.Turtle), // Perf: Enable for local testing only // TestFile.Create(TestImages.Jpeg.Fb), // Perf: Enable for local testing only // TestFile.Create(TestImages.Jpeg.Progress), // Perf: Enable for local testing only // TestFile.Create(TestImages.Jpeg.GammaDalaiLamaGray), // Perf: Enable for local testing only @@ -33,7 +35,7 @@ namespace ImageSharp.Tests // TestFile.Create(TestImages.Png.Blur), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.Indexed), // Perf: Enable for local testing only TestFile.Create(TestImages.Png.Splash), - TestFile.Create(TestImages.Png.Powerpoint), + // TestFile.Create(TestImages.Png.Powerpoint), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.SplashInterlaced), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.Interlaced), // Perf: Enable for local testing only // TestFile.Create(TestImages.Png.Filter0), // Perf: Enable for local testing only diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 9e2d21a9b..794807959 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -52,6 +52,7 @@ namespace ImageSharp.Tests public const string Hiyamugi = "Jpg/Hiyamugi.jpg"; public const string BadEOF = "Jpg/badeof.jpg"; + public const string BadEOFProgressive = "Jpg/badeofprog.jpg"; public const string Snake = "Jpg/Snake.jpg"; public const string Lake = "Jpg/Lake.jpg"; diff --git a/tests/ImageSharp.Tests/TestImages/Formats/Jpg/badeofprog.jpg b/tests/ImageSharp.Tests/TestImages/Formats/Jpg/badeofprog.jpg new file mode 100644 index 000000000..6e8350820 Binary files /dev/null and b/tests/ImageSharp.Tests/TestImages/Formats/Jpg/badeofprog.jpg differ