# HG changeset patch # User bae # Date 1361823317 -14400 # Node ID d868fe7c7618e5b55eea8dd69ee5d099c71816e0 # Parent 6784c9903db7f65a93279ac12b7fc00c57dbaaa5 8007667: Better image reading Reviewed-by: prr, jgodinez diff --git openjdk/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java openjdk/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java --- jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java +++ jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java @@ -281,12 +281,17 @@ * sending warnings to listeners. */ protected void warningOccurred(int code) { - if ((code < 0) || (code > MAX_WARNING)){ - throw new InternalError("Invalid warning index"); - } - processWarningOccurred - ("com.sun.imageio.plugins.jpeg.JPEGImageReaderResources", - Integer.toString(code)); + cbLock.lock(); + try { + if ((code < 0) || (code > MAX_WARNING)){ + throw new InternalError("Invalid warning index"); + } + processWarningOccurred + ("com.sun.imageio.plugins.jpeg.JPEGImageReaderResources", + Integer.toString(code)); + } finally { + cbLock.unlock(); + } } /** @@ -303,7 +308,12 @@ * library warnings from being printed to stderr. */ protected void warningWithMessage(String msg) { - processWarningOccurred(msg); + cbLock.lock(); + try { + processWarningOccurred(msg); + } finally { + cbLock.unlock(); + } } public void setInput(Object input, @@ -312,18 +322,55 @@ { setThreadLock(); try { + cbLock.check(); + super.setInput(input, seekForwardOnly, ignoreMetadata); this.ignoreMetadata = ignoreMetadata; resetInternalState(); iis = (ImageInputStream) input; // Always works - setSource(structPointer, iis); + setSource(structPointer); } finally { clearThreadLock(); } } - private native void setSource(long structPointer, - ImageInputStream source); + /** + * This method is called from native code in order to fill + * native input buffer. + * + * We block any attempt to change the reading state during this + * method, in order to prevent a corruption of the native decoder + * state. + * + * @return number of bytes read from the stream. + */ + private int readInputData(byte[] buf, int off, int len) throws IOException { + cbLock.lock(); + try { + return iis.read(buf, off, len); + } finally { + cbLock.unlock(); + } + } + + /** + * This method is called from the native code in order to + * skip requested number of bytes in the input stream. + * + * @param n + * @return + * @throws IOException + */ + private long skipInputBytes(long n) throws IOException { + cbLock.lock(); + try { + return iis.skipBytes(n); + } finally { + cbLock.unlock(); + } + } + + private native void setSource(long structPointer); private void checkTablesOnly() throws IOException { if (debug) { @@ -375,6 +422,8 @@ public int getNumImages(boolean allowSearch) throws IOException { setThreadLock(); try { // locked thread + cbLock.check(); + return getNumImagesOnThread(allowSearch); } finally { clearThreadLock(); @@ -574,8 +623,13 @@ if (debug) { System.out.println("pushing back " + num + " bytes"); } - iis.seek(iis.getStreamPosition()-num); - // The buffer is clear after this, so no need to set haveSeeked. + cbLock.lock(); + try { + iis.seek(iis.getStreamPosition()-num); + // The buffer is clear after this, so no need to set haveSeeked. + } finally { + cbLock.unlock(); + } } /** @@ -645,7 +699,12 @@ * Ignore this profile. */ iccCS = null; - warningOccurred(WARNING_IGNORE_INVALID_ICC); + cbLock.lock(); + try { + warningOccurred(WARNING_IGNORE_INVALID_ICC); + } finally { + cbLock.unlock(); + } return; } @@ -680,6 +739,7 @@ setThreadLock(); try { if (currentImage != imageIndex) { + cbLock.check(); readHeader(imageIndex, true); } return width; @@ -692,6 +752,7 @@ setThreadLock(); try { if (currentImage != imageIndex) { + cbLock.check(); readHeader(imageIndex, true); } return height; @@ -720,6 +781,8 @@ setThreadLock(); try { if (currentImage != imageIndex) { + cbLock.check(); + readHeader(imageIndex, true); } @@ -743,6 +806,7 @@ private Iterator getImageTypesOnThread(int imageIndex) throws IOException { if (currentImage != imageIndex) { + cbLock.check(); readHeader(imageIndex, true); } @@ -944,6 +1008,7 @@ setThreadLock(); try { if (!tablesOnlyChecked) { + cbLock.check(); checkTablesOnly(); } return streamMetadata; @@ -964,6 +1029,8 @@ return imageMetadata; } + cbLock.check(); + gotoImage(imageIndex); imageMetadata = new JPEGMetadata(false, false, iis, this); @@ -980,6 +1047,7 @@ throws IOException { setThreadLock(); try { + cbLock.check(); try { readInternal(imageIndex, param, false); } catch (RuntimeException e) { @@ -1209,58 +1277,63 @@ } target.setRect(destROI.x, destROI.y + y, raster); - processImageUpdate(image, - destROI.x, destROI.y+y, - raster.getWidth(), 1, - 1, 1, - destinationBands); - if ((y > 0) && (y%progInterval == 0)) { - int height = target.getHeight()-1; - float percentOfPass = ((float)y)/height; - if (progressive) { - if (knownPassCount != UNKNOWN) { - processImageProgress((pass + percentOfPass)*100.0F - / knownPassCount); - } else if (maxProgressivePass != Integer.MAX_VALUE) { - // Use the range of allowed progressive passes - processImageProgress((pass + percentOfPass)*100.0F - / (maxProgressivePass - minProgressivePass + 1)); + cbLock.lock(); + try { + processImageUpdate(image, + destROI.x, destROI.y+y, + raster.getWidth(), 1, + 1, 1, + destinationBands); + if ((y > 0) && (y%progInterval == 0)) { + int height = target.getHeight()-1; + float percentOfPass = ((float)y)/height; + if (progressive) { + if (knownPassCount != UNKNOWN) { + processImageProgress((pass + percentOfPass)*100.0F + / knownPassCount); + } else if (maxProgressivePass != Integer.MAX_VALUE) { + // Use the range of allowed progressive passes + processImageProgress((pass + percentOfPass)*100.0F + / (maxProgressivePass - minProgressivePass + 1)); + } else { + // Assume there are a minimum of MIN_ESTIMATED_PASSES + // and that there is always one more pass + // Compute the percentage as the percentage at the end + // of the previous pass, plus the percentage of this + // pass scaled to be the percentage of the total remaining, + // assuming a minimum of MIN_ESTIMATED_PASSES passes and + // that there is always one more pass. This is monotonic + // and asymptotic to 1.0, which is what we need. + int remainingPasses = // including this one + Math.max(2, MIN_ESTIMATED_PASSES-pass); + int totalPasses = pass + remainingPasses-1; + progInterval = Math.max(height/20*totalPasses, + totalPasses); + if (y%progInterval == 0) { + percentToDate = previousPassPercentage + + (1.0F - previousPassPercentage) + * (percentOfPass)/remainingPasses; + if (debug) { + System.out.print("pass= " + pass); + System.out.print(", y= " + y); + System.out.print(", progInt= " + progInterval); + System.out.print(", % of pass: " + percentOfPass); + System.out.print(", rem. passes: " + + remainingPasses); + System.out.print(", prev%: " + + previousPassPercentage); + System.out.print(", %ToDate: " + percentToDate); + System.out.print(" "); + } + processImageProgress(percentToDate*100.0F); + } + } } else { - // Assume there are a minimum of MIN_ESTIMATED_PASSES - // and that there is always one more pass - // Compute the percentage as the percentage at the end - // of the previous pass, plus the percentage of this - // pass scaled to be the percentage of the total remaining, - // assuming a minimum of MIN_ESTIMATED_PASSES passes and - // that there is always one more pass. This is monotonic - // and asymptotic to 1.0, which is what we need. - int remainingPasses = // including this one - Math.max(2, MIN_ESTIMATED_PASSES-pass); - int totalPasses = pass + remainingPasses-1; - progInterval = Math.max(height/20*totalPasses, - totalPasses); - if (y%progInterval == 0) { - percentToDate = previousPassPercentage + - (1.0F - previousPassPercentage) - * (percentOfPass)/remainingPasses; - if (debug) { - System.out.print("pass= " + pass); - System.out.print(", y= " + y); - System.out.print(", progInt= " + progInterval); - System.out.print(", % of pass: " + percentOfPass); - System.out.print(", rem. passes: " - + remainingPasses); - System.out.print(", prev%: " - + previousPassPercentage); - System.out.print(", %ToDate: " + percentToDate); - System.out.print(" "); - } - processImageProgress(percentToDate*100.0F); - } + processImageProgress(percentOfPass * 100.0F); } - } else { - processImageProgress(percentOfPass * 100.0F); - } + } + } finally { + cbLock.unlock(); } } @@ -1273,33 +1346,58 @@ } private void passStarted (int pass) { - this.pass = pass; - previousPassPercentage = percentToDate; - processPassStarted(image, - pass, - minProgressivePass, - maxProgressivePass, - 0, 0, - 1,1, - destinationBands); + cbLock.lock(); + try { + this.pass = pass; + previousPassPercentage = percentToDate; + processPassStarted(image, + pass, + minProgressivePass, + maxProgressivePass, + 0, 0, + 1,1, + destinationBands); + } finally { + cbLock.unlock(); + } } private void passComplete () { - processPassComplete(image); + cbLock.lock(); + try { + processPassComplete(image); + } finally { + cbLock.unlock(); + } } void thumbnailStarted(int thumbnailIndex) { - processThumbnailStarted(currentImage, thumbnailIndex); + cbLock.lock(); + try { + processThumbnailStarted(currentImage, thumbnailIndex); + } finally { + cbLock.unlock(); + } } // Provide access to protected superclass method void thumbnailProgress(float percentageDone) { - processThumbnailProgress(percentageDone); + cbLock.lock(); + try { + processThumbnailProgress(percentageDone); + } finally { + cbLock.unlock(); + } } // Provide access to protected superclass method void thumbnailComplete() { - processThumbnailComplete(); + cbLock.lock(); + try { + processThumbnailComplete(); + } finally { + cbLock.unlock(); + } } /** @@ -1323,6 +1421,11 @@ public void abort() { setThreadLock(); try { + /** + * NB: we do not check the call back lock here, + * we allow to abort the reader any time. + */ + super.abort(); abortRead(structPointer); } finally { @@ -1345,6 +1448,7 @@ setThreadLock(); Raster retval = null; try { + cbLock.check(); /* * This could be further optimized by not resetting the dest. * offset and creating a translated raster in readInternal() @@ -1384,6 +1488,8 @@ public int getNumThumbnails(int imageIndex) throws IOException { setThreadLock(); try { + cbLock.check(); + getImageMetadata(imageIndex); // checks iis state for us // Now check the jfif segments JFIFMarkerSegment jfif = @@ -1404,6 +1510,8 @@ throws IOException { setThreadLock(); try { + cbLock.check(); + if ((thumbnailIndex < 0) || (thumbnailIndex >= getNumThumbnails(imageIndex))) { throw new IndexOutOfBoundsException("No such thumbnail"); @@ -1422,6 +1530,8 @@ throws IOException { setThreadLock(); try { + cbLock.check(); + if ((thumbnailIndex < 0) || (thumbnailIndex >= getNumThumbnails(imageIndex))) { throw new IndexOutOfBoundsException("No such thumbnail"); @@ -1441,6 +1551,8 @@ throws IOException { setThreadLock(); try { + cbLock.check(); + if ((thumbnailIndex < 0) || (thumbnailIndex >= getNumThumbnails(imageIndex))) { throw new IndexOutOfBoundsException("No such thumbnail"); @@ -1481,6 +1593,7 @@ public void reset() { setThreadLock(); try { + cbLock.check(); super.reset(); } finally { clearThreadLock(); @@ -1492,6 +1605,8 @@ public void dispose() { setThreadLock(); try { + cbLock.check(); + if (structPointer != 0) { disposerRecord.dispose(); structPointer = 0; @@ -1553,4 +1668,34 @@ theThread = null; } } + + private CallBackLock cbLock = new CallBackLock(); + + private static class CallBackLock { + + private State lockState; + + CallBackLock() { + lockState = State.Unlocked; + } + + void check() { + if (lockState != State.Unlocked) { + throw new IllegalStateException("Access to the reader is not allowed"); + } + } + + private void lock() { + lockState = State.Locked; + } + + private void unlock() { + lockState = State.Unlocked; + } + + private static enum State { + Unlocked, + Locked + } + } } diff --git openjdk/jdk/src/share/native/sun/awt/image/jpeg/imageioJPEG.c openjdk/jdk/src/share/native/sun/awt/image/jpeg/imageioJPEG.c --- jdk/src/share/native/sun/awt/image/jpeg/imageioJPEG.c +++ jdk/src/share/native/sun/awt/image/jpeg/imageioJPEG.c @@ -57,8 +57,8 @@ #define MAX(a,b) ((a) > (b) ? (a) : (b)) /* Cached Java method ids */ -static jmethodID ImageInputStream_readID; -static jmethodID ImageInputStream_skipBytesID; +static jmethodID JPEGImageReader_readInputDataID; +static jmethodID JPEGImageReader_skipInputBytesID; static jmethodID JPEGImageReader_warningOccurredID; static jmethodID JPEGImageReader_warningWithMessageID; static jmethodID JPEGImageReader_setImageDataID; @@ -923,7 +923,7 @@ imageio_fill_input_buffer(j_decompress_p RELEASE_ARRAYS(env, data, src->next_input_byte); ret = (*env)->CallIntMethod(env, sb->stream, - ImageInputStream_readID, + JPEGImageReader_readInputDataID, sb->hstreamBuffer, 0, sb->bufferLength); if ((*env)->ExceptionOccurred(env) @@ -1013,7 +1013,7 @@ imageio_fill_suspended_buffer(j_decompre } ret = (*env)->CallIntMethod(env, sb->stream, - ImageInputStream_readID, + JPEGImageReader_readInputDataID, sb->hstreamBuffer, offset, buflen); if ((*env)->ExceptionOccurred(env) @@ -1107,7 +1107,7 @@ imageio_skip_input_data(j_decompress_ptr RELEASE_ARRAYS(env, data, src->next_input_byte); ret = (*env)->CallLongMethod(env, sb->stream, - ImageInputStream_skipBytesID, + JPEGImageReader_skipInputBytesID, (jlong) num_bytes); if ((*env)->ExceptionOccurred(env) || !GET_ARRAYS(env, data, &(src->next_input_byte))) { @@ -1382,13 +1382,13 @@ Java_com_sun_imageio_plugins_jpeg_JPEGIm jclass qTableClass, jclass huffClass) { - ImageInputStream_readID = (*env)->GetMethodID(env, - ImageInputStreamClass, - "read", + JPEGImageReader_readInputDataID = (*env)->GetMethodID(env, + cls, + "readInputData", "([BII)I"); - ImageInputStream_skipBytesID = (*env)->GetMethodID(env, - ImageInputStreamClass, - "skipBytes", + JPEGImageReader_skipInputBytesID = (*env)->GetMethodID(env, + cls, + "skipInputBytes", "(J)J"); JPEGImageReader_warningOccurredID = (*env)->GetMethodID(env, cls, @@ -1531,8 +1531,7 @@ Java_com_sun_imageio_plugins_jpeg_JPEGIm Java_com_sun_imageio_plugins_jpeg_JPEGImageReader_setSource (JNIEnv *env, jobject this, - jlong ptr, - jobject source) { + jlong ptr) { imageIODataPtr data = (imageIODataPtr)jlong_to_ptr(ptr); j_common_ptr cinfo; @@ -1546,7 +1545,7 @@ Java_com_sun_imageio_plugins_jpeg_JPEGIm cinfo = data->jpegObj; - imageio_set_stream(env, cinfo, data, source); + imageio_set_stream(env, cinfo, data, this); imageio_init_source((j_decompress_ptr) cinfo); }