diff --git a/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs b/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs index 09baf904b6..39ee6687b0 100644 --- a/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs +++ b/src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs @@ -356,21 +356,11 @@ namespace ImageSharp.Formats.Jpg throw new ImageFormatException("Excessive DC component"); } - // TODO: Handle the EOFException and piece together the final progressive scan. - try - { - int deltaDC = decoder.Bits.ReceiveExtend(value, decoder); - this.pointers.Dc[compIndex] += deltaDC; + 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); - } - catch (JpegDecoderCore.EOFException) - { - // Do something clever here. I'm just undoing ReceiveExtend - decoder.Bits.UnreadBits += value; - decoder.Bits.Mask >>= value; - } + // b[0] = dc[compIndex] << al; + Block8x8F.SetScalarAt(b, 0, this.pointers.Dc[compIndex] << this.al); } if (zig <= this.zigEnd && this.eobRun > 0) @@ -393,20 +383,7 @@ namespace ImageSharp.Formats.Jpg break; } - 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; - } + int ac = decoder.Bits.ReceiveExtend(val1, decoder); // 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 6ba578a3ae..2dd547c916 100644 --- a/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs @@ -186,162 +186,164 @@ namespace ImageSharp.Formats throw new ImageFormatException("Missing SOI marker."); } - while (true) + // 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) { - try + this.ReadFull(this.Temp, 0, 2); + while (this.Temp[0] != 0xff) { - 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(); - } + // 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; + } - this.ProcessStartOfScan(remaining); + // 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; + } - 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; - } - } - 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; + break; } } diff --git a/tests/ImageSharp.Tests/FileTestBase.cs b/tests/ImageSharp.Tests/FileTestBase.cs index a2d7397dab..e9343b20dd 100644 --- a/tests/ImageSharp.Tests/FileTestBase.cs +++ b/tests/ImageSharp.Tests/FileTestBase.cs @@ -22,11 +22,9 @@ 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), // Perf: Enable for local testing only + TestFile.Create(TestImages.Jpeg.Turtle), // 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 @@ -35,7 +33,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), // Perf: Enable for local testing only + TestFile.Create(TestImages.Png.Powerpoint), // 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 7948079596..9e2d21a9bd 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -52,7 +52,6 @@ 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 deleted file mode 100644 index 82a5707e05..0000000000 --- a/tests/ImageSharp.Tests/TestImages/Formats/Jpg/badeofprog.jpg +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:22697fcab865bda38c760c6402382bffd47dccdef34d93ce5f8ae63f9b7918aa -size 67503