Bug 1079653 (Part 3) - Make decoders track whether they need to flush data after getting a new frame. r=tn
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -22,16 +22,17 @@ Decoder::Decoder(RasterImage &aImage)
, mChunkCount(0)
, mDecodeFlags(0)
, mBytesDecoded(0)
, mDecodeDone(false)
, mDataError(false)
, mFrameCount(0)
, mFailCode(NS_OK)
, mNeedsNewFrame(false)
+ , mNeedsToFlushData(false)
, mInitialized(false)
, mSizeDecode(false)
, mInFrame(false)
, mIsAnimated(false)
{
}
Decoder::~Decoder()
@@ -100,16 +101,22 @@ Decoder::Write(const char* aBuffer, uint
// Begin recording telemetry data.
TimeStamp start = TimeStamp::Now();
mChunkCount++;
// Keep track of the total number of bytes written.
mBytesDecoded += aCount;
+ // If we're flushing data, clear the flag.
+ if (aBuffer == nullptr && aCount == 0) {
+ MOZ_ASSERT(mNeedsToFlushData, "Flushing when we don't need to");
+ mNeedsToFlushData = false;
+ }
+
// If a data error occured, just ignore future data.
if (HasDataError())
return;
if (IsSizeDecode() && HasSize()) {
// More data came in since we found the size. We have nothing to do here.
return;
}
@@ -245,16 +252,22 @@ Decoder::AllocateFrame()
} else if (NS_FAILED(rv)) {
PostDataError();
}
// Mark ourselves as not needing another frame before talking to anyone else
// so they can tell us if they need yet another.
mNeedsNewFrame = false;
+ // If we've received any data at all, we may have pending data that needs to
+ // be flushed now that we have a frame to decode into.
+ if (mBytesDecoded > 0) {
+ mNeedsToFlushData = true;
+ }
+
return rv;
}
void
Decoder::FlushInvalidations()
{
NS_ABORT_IF_FALSE(!HasDecoderError(),
"Not allowed to make more decoder calls after error!");
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -39,16 +39,21 @@ public:
*/
void InitSharedDecoder(uint8_t* imageData, uint32_t imageDataLength,
uint32_t* colormap, uint32_t colormapSize,
imgFrame* currentFrame);
/**
* Writes data to the decoder.
*
+ * If aBuffer is null and aCount is 0, Write() flushes any buffered data to
+ * the decoder. Data is buffered if the decoder wasn't able to completely
+ * decode it because it needed a new frame. If it's necessary to flush data,
+ * NeedsToFlushData() will return true.
+ *
* @param aBuffer buffer containing the data to be written
* @param aCount the number of bytes to write
*
* Any errors are reported by setting the appropriate state on the decoder.
*
* Notifications Sent: TODO
*/
void Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy);
@@ -159,16 +164,21 @@ public:
// will be called again with nullptr and 0 as arguments.
void NeedNewFrame(uint32_t frameNum, uint32_t x_offset, uint32_t y_offset,
uint32_t width, uint32_t height,
gfx::SurfaceFormat format,
uint8_t palette_depth = 0);
virtual bool NeedsNewFrame() const { return mNeedsNewFrame; }
+ // Returns true if we may have stored data that we need to flush now that we
+ // have a new frame to decode into. Callers can use Write() to actually
+ // flush the data; see the documentation for that method.
+ bool NeedsToFlushData() const { return mNeedsToFlushData; }
+
// Try to allocate a frame as described in mNewFrameData and return the
// status code from that attempt. Clears mNewFrameData.
virtual nsresult AllocateFrame();
already_AddRefed<imgFrame> GetCurrentFrame() const
{
nsRefPtr<imgFrame> frame = mCurrentFrame;
return frame.forget();
@@ -278,16 +288,17 @@ private:
uint32_t mOffsetY;
uint32_t mWidth;
uint32_t mHeight;
gfx::SurfaceFormat mFormat;
uint8_t mPaletteDepth;
};
NewFrameData mNewFrameData;
bool mNeedsNewFrame;
+ bool mNeedsToFlushData;
bool mInitialized;
bool mSizeDecode;
bool mInFrame;
bool mIsAnimated;
};
} // namespace image
} // namespace mozilla
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2391,21 +2391,19 @@ RasterImage::SyncDecode()
// with the new flags. If we can't discard then there isn't
// anything we can do.
if (!CanForciblyDiscardAndRedecode())
return NS_ERROR_NOT_AVAILABLE;
ForceDiscard();
}
}
- // If we're currently waiting on a new frame for this image, we have to create
- // it now.
+ // If we're currently waiting on a new frame for this image, create it now.
if (mDecoder && mDecoder->NeedsNewFrame()) {
mDecoder->AllocateFrame();
- mDecodeRequest->mAllocatedNewFrame = true;
}
// If we don't have a decoder, create one
if (!mDecoder) {
rv = InitDecoder(/* aDoSizeDecode = */ false);
CONTAINER_ENSURE_SUCCESS(rv);
}
@@ -2775,18 +2773,17 @@ RasterImage::DecodeSomeData(size_t aMaxB
{
MOZ_ASSERT(mDecoder, "Should have a decoder");
mDecodingMonitor.AssertCurrentThreadIn();
// First, if we've just been called because we allocated a frame on the main
// thread, let the decoder deal with the data it set aside at that time by
// passing it a null buffer.
- if (mDecodeRequest->mAllocatedNewFrame) {
- mDecodeRequest->mAllocatedNewFrame = false;
+ if (mDecoder->NeedsToFlushData()) {
nsresult rv = WriteToDecoder(nullptr, 0, aStrategy);
if (NS_FAILED(rv) || mDecoder->NeedsNewFrame()) {
return rv;
}
}
// If we have nothing else to decode, return.
if (mDecoder->BytesDecoded() == mSourceData.Length()) {
@@ -2822,18 +2819,17 @@ RasterImage::IsDecodeFinished()
}
} else if (mDecoder->GetDecodeDone()) {
return true;
}
// If the decoder returned because it needed a new frame and we haven't
// written to it since then, the decoder may be storing data that it hasn't
// decoded yet.
- if (mDecoder->NeedsNewFrame() ||
- (mDecodeRequest && mDecodeRequest->mAllocatedNewFrame)) {
+ if (mDecoder->NeedsNewFrame() || mDecoder->NeedsToFlushData()) {
return false;
}
// Otherwise, if we have all the source data and wrote all the source data,
// we're done.
//
// (NB - This can be the case even for non-erroneous images because
// Decoder::GetDecodeDone() might not return true until after we call
@@ -3412,19 +3408,17 @@ RasterImage::DecodePool::DecodeSomeOfIma
// example, a synchronous decode request came while the worker was pending).
if (!aImg->mDecoder || aImg->mDecoded)
return NS_OK;
// If we're doing synchronous decodes, and we're waiting on a new frame for
// this image, get it now.
if (aStrategy == DECODE_SYNC && aImg->mDecoder->NeedsNewFrame()) {
MOZ_ASSERT(NS_IsMainThread());
-
aImg->mDecoder->AllocateFrame();
- aImg->mDecodeRequest->mAllocatedNewFrame = true;
}
// If we're not synchronous, we can't allocate a frame right now.
else if (aImg->mDecoder->NeedsNewFrame()) {
return NS_OK;
}
nsRefPtr<Decoder> decoderKungFuDeathGrip = aImg->mDecoder;
@@ -3455,17 +3449,17 @@ RasterImage::DecodePool::DecodeSomeOfIma
// * we run out of time.
// We also try to decode at least one "chunk" if we've allocated a new frame,
// even if we have no more data to send to the decoder.
while ((aImg->mSourceData.Length() > aImg->mDecoder->BytesDecoded() &&
bytesToDecode > 0 &&
!aImg->IsDecodeFinished() &&
!(aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize) &&
!aImg->mDecoder->NeedsNewFrame()) ||
- (aImg->mDecodeRequest && aImg->mDecodeRequest->mAllocatedNewFrame)) {
+ aImg->mDecoder->NeedsToFlushData()) {
uint32_t chunkSize = std::min(bytesToDecode, maxBytes);
nsresult rv = aImg->DecodeSomeData(chunkSize, aStrategy);
if (NS_FAILED(rv)) {
aImg->DoError();
return rv;
}
bytesToDecode -= chunkSize;
@@ -3546,17 +3540,16 @@ RasterImage::FrameNeededWorker::Run()
{
ReentrantMonitorAutoEnter lock(mImage->mDecodingMonitor);
nsresult rv = NS_OK;
// If we got a synchronous decode in the mean time, we don't need to do
// anything.
if (mImage->mDecoder && mImage->mDecoder->NeedsNewFrame()) {
rv = mImage->mDecoder->AllocateFrame();
- mImage->mDecodeRequest->mAllocatedNewFrame = true;
}
if (NS_SUCCEEDED(rv) && mImage->mDecoder) {
// By definition, we're not done decoding, so enqueue us for more decoding.
DecodePool::Singleton()->RequestDecode(mImage);
}
return NS_OK;
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -325,17 +325,16 @@ private:
* Each RasterImage has a pointer to one or zero heap-allocated
* DecodeRequests.
*/
struct DecodeRequest
{
explicit DecodeRequest(RasterImage* aImage)
: mImage(aImage)
, mRequestStatus(REQUEST_INACTIVE)
- , mAllocatedNewFrame(false)
{
MOZ_ASSERT(aImage, "aImage cannot be null");
MOZ_ASSERT(aImage->mStatusTracker,
"aImage should have an imgStatusTracker");
mStatusTracker = aImage->mStatusTracker->CloneForRecording();
}
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest)
@@ -350,20 +349,16 @@ private:
{
REQUEST_INACTIVE,
REQUEST_PENDING,
REQUEST_ACTIVE,
REQUEST_WORK_DONE,
REQUEST_STOPPED
} mRequestStatus;
- /* True if a new frame has been allocated, but DecodeSomeData hasn't yet
- * been called to flush data to it */
- bool mAllocatedNewFrame;
-
private:
~DecodeRequest() {}
};
/*
* DecodePool is a singleton class we use when decoding large images.
*
* When we wish to decode an image larger than