Prototype of RAW decoding in Skia.

src/codec/SkRawCodec.cpp

Index: src/codec/SkRawCodec.cpp
diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..d9e37dd225176dd9ab568c73b1fac3d5642277f7
--- /dev/null
+++ b/src/codec/SkRawCodec.cpp
@@ -0,0 +1,303 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkCodec.h"
+#include "SkRawCodec.h"
+#include "SkCodecPriv.h"
+#include "SkColorPriv.h"
+#include "SkJpegCodec.h"
+#include "SkStream.h"
+#include "SkStreamPriv.h"
+#include "SkSwizzler.h"
+#include "SkTemplates.h"
+#include "SkTypes.h"
+
+#include "dng_color_space.h"
+#include "dng_exceptions.h"
+#include "dng_host.h"
+#include "dng_info.h"
+#include "dng_render.h"
+#include "dng_stream.h"
+
+#include "src/piex.h"
+
+#include <cmath>
+#include <cstring>
+#include <limits>
+#include <memory>
+#include <vector>
+
+namespace {
+
+// T must be unsigned type.
+template <class T>
+bool SafeAddToSizeT(T arg1, T arg2, size_t* result) {

[scroggo, 2016/01/07 20:57:42]

In Skia, we name static functions (or those in anonymous namespaces) as

safe_add_to_size_t

(I should have mentioned that for ReadDng etc)

[yujieqin, 2016/01/08 14:00:12]

Done.

+    SkASSERT(arg1 >= 0);
+    SkASSERT(arg2 >= 0);
+    if (arg1 >= 0 && arg2 <= std::numeric_limits<T>::max() - arg1) {
+        T sum = arg1 + arg2;
+        if (sum <= std::numeric_limits<size_t>::max()) {
+            *result = static_cast<size_t>(sum);
+            return true;
+        }
+    }
+    return false;
+}
+
+dng_negative* ReadDng(dng_stream* stream, dng_host* host, dng_info* info) {

[scroggo, 2016/01/07 20:57:43]

This should be read_dng

[yujieqin, 2016/01/08 14:00:12]

Done.

+    const uint32 kPreferredSize = 0;
+    const uint32 kMinimumSize = 0;
+    const uint32 kMaximumSize = 0;
+    SkAutoTDelete<dng_negative> negative;
+    try {
+        host->SetPreferredSize(kPreferredSize);
+        host->SetMinimumSize(kMinimumSize);
+        host->SetMaximumSize(kMaximumSize);
+
+        host->ValidateSizes();
+
+        // Read stage 1 image into the negative.
+        {
+            info->Parse(*host, *stream);
+            info->PostParse(*host);
+
+            if (!info->IsValidDNG()) {
+                return nullptr;
+            }
+
+            negative.reset(host->Make_dng_negative());
+            negative->Parse(*host, *stream, *info);
+            negative->PostParse(*host, *stream, *info);
+        }
+
+        negative->SynchronizeMetadata();
+    } catch (...) {
+        return nullptr;
+    }
+
+    return negative.release();
+}
+
+bool PrepareStage3(dng_stream* stream, dng_host* host, dng_info* info,

[scroggo, 2016/01/07 20:57:42]

prepare_stage_3

[yujieqin, 2016/01/08 14:00:12]

Done.

+                   dng_negative* negative) {
+    if (negative->Stage3Image() == nullptr) {
+        negative->ReadStage1Image(*host, *stream, *info);
+
+        if (info->fMaskIndex != -1) {
+            negative->ReadTransparencyMask(*host, *stream, *info);
+        }
+
+        negative->ValidateRawImageDigest(*host);
+        if (negative->IsDamaged()) {
+            return false;
+        }
+
+        const int32 kMosaicPlane = -1;
+        negative->BuildStage2Image(*host);
+        negative->BuildStage3Image(*host, kMosaicPlane);
+    }
+
+    return true;
+}
+
+}  // namespace
+
+// Note: this class could throw exception if it is used as dng_stream.
+class SkRawStream : public dng_stream, public ::piex::StreamInterface {

[scroggo, 2016/01/07 20:57:42]

We usually avoid multiple inheritance in Skia (see
https://groups.google.com/a/google.com/d/msg/skia-dev/gyRP6pHa154/_1L6Hb2oHrYJ).
Unless this is okay because one (or even better, both?) of these is an
interface?

[yujieqin, 2016/01/08 14:00:12]

Both dng_stream and ::piex::StreamInterface are indeed interfaces.

+public:
+    // Note that this call will take the ownership of stream.
+    explicit SkRawStream(SkStream* stream)
+        : fStream(stream), fWholeStreamRead(false), fStreamBufferSize(0)
+        // Arbitrary buffer size.
+#if defined(GOOGLE3)
+        // Stack frame size is limited in GOOGLE3.
+        , fReadBufferSize(8 * 1024) {} // 8KB
+#else
+        , fReadBufferSize(256 * 1024) {} // 256KB
+#endif
+
+    ~SkRawStream() override {}
+
+    // Creates a SkMemoryStream from the offset with size.
+    SkMemoryStream* CopyBuffer(size_t offset, size_t size) {

[scroggo, 2016/01/07 20:57:42]

style: (non-static) member functions are named like "copyBuffer". (I know some
of these are overrides, so you cannot change those.)

[yujieqin, 2016/01/08 14:00:12]

Done.

+        size_t sum;
+        if (!SafeAddToSizeT(offset, size, &sum) ||
+            !IncreaseStreamBufferSize(sum)) {
+            return nullptr;
+        }
+        return new SkMemoryStream(
+                reinterpret_cast<const uint8*>(fStreamBuffer.get()) + offset, size, true);

[scroggo, 2016/01/07 20:57:42]

You can get rid of these casts by making fStreamBuffer an SkAutoTMalloc, from
SkTemplates.h

[yujieqin, 2016/01/08 14:00:12]

Nice, thanks! :)

+    }
+
+    // For PIEX
+    ::piex::Error GetData(const size_t offset, const size_t length,
+                          uint8* data) override {
+        size_t sum;
+        if (!SafeAddToSizeT(offset, length, &sum) ||
+            !IncreaseStreamBufferSize(sum)) {
+          return ::piex::Error::kFail;
+        }
+        memcpy(data, reinterpret_cast<const uint8*>(fStreamBuffer.get()) + offset, length);
+        return ::piex::Error::kOk;
+    }
+
+protected:
+    // For dng_stream
+    uint64 DoGetLength() override {
+        if (!IncreaseStreamBufferSize(0, true)) {  // read whole stream
+            ThrowReadFile();
+        }
+        return fStreamBufferSize;
+    }
+
+    // For dng_stream
+    void DoRead(void* data, uint32 count, uint64 offset) override {
+        size_t sum;
+        if (!SafeAddToSizeT(static_cast<uint64>(count), offset, &sum) ||
+            !IncreaseStreamBufferSize(sum)) {
+            ThrowReadFile();
+        }
+        memcpy(data, reinterpret_cast<const uint8*>(fStreamBuffer.get()) + offset, count);
+    }
+
+private:
+    bool IncreaseStreamBufferSize(size_t size, bool readToEnd = false) {

[scroggo, 2016/01/07 20:57:42]

style: increaseStreamBufferSize

Also, I find this method confusing. It seems like what it does is read from the
internal stream into a buffer. Maybe a better name would be bufferMoreData?

The parameter name is also ambiguous. I think a better name would be newSize?

[yujieqin, 2016/01/08 14:00:12]

Done.

+        if (!readToEnd) {

[scroggo, 2016/01/07 20:57:43]

nit: I find it weird to say:

if (!condition) {
  ...
} else {
  ...
}

rather than saying if(condition) and switching the blocks. Maybe that's just me
though.

[yujieqin, 2016/01/08 14:00:12]

Done.

+            if (size <= fStreamBufferSize) {
+                return true;
+            } else if (fWholeStreamRead) {  // request more than available.

[scroggo, 2016/01/07 20:57:43]

no need for "else" here. We would have returned if the first condition was true.

[yujieqin, 2016/01/08 14:00:12]

Done.

+                return false;
+            }
+        } else {
+            if (fWholeStreamRead) {

[scroggo, 2016/01/07 20:57:43]

It seems weird to me that this is different depending on readToEnd. Is it okay
to ignore size here?

[yujieqin, 2016/01/08 14:00:12]

Yes, if readToEnd is "true", the newSize will be basically ignore. I added some
comment.

+                return true;
+            }
+        }
+
+        const size_t sizeToRead = readToEnd ? 0 : size - fStreamBufferSize;
+        const size_t readBufferSize =
+                readToEnd ? fReadBufferSize : std::min(fReadBufferSize, sizeToRead);

[scroggo, 2016/01/07 20:57:42]

nit: SkTMin

[yujieqin, 2016/01/08 14:00:12]

Done.

+
+        char buffer[readBufferSize];
+        size_t sizeRead = 0;
+        SkDynamicMemoryWStream tempStream;

[scroggo, 2016/01/07 20:57:42]

I think this code is made more complex by the fact that you're trying to handle
two cases:

1. Increase to a specific size.

This case does not need the SkDynamicMemoryStream. Instead, you can reallocate
(since you know the final size) and read (once) into the new memory:

fStreamBuffer.realloc(newSize);
uint8_t* dst = fStreamBuffer.get() + fStreamBufferSize;
size_t bytesRead = fStream->read(dst, sizeToRead);
if (bytesRead < sizeToRead) {
  // I'm guessing we'll no longer need it, so we could go ahead and free
  // the buffer
  fStreamBuffer.free();
  return false;
}
fStreamBufferSize = newSize;
return true;

This may allocate more memory than necessary on failure, but I'm assuming we
should optimize for success, where this saves allocations and copying.

2. Read the rest of the stream:
Here you'll need to loop over the rest of the stream, assuming the length is not
known, like you do here (and it's a little simpler because you don't need to
worry about the other case). But the code is more or less copied from
SkCopyStreamToStorage. What do you think about refactoring that so you can share
code?

FWIW, I think it is fine to special case where an SkMemoryStream is being used.
It might be better to leave that as a TODO though, so that running our tests
(which use SkMemoryStreams) more closely matches the common case on Android.

[yujieqin, 2016/01/08 14:00:12]

Done.

+        do {
+            size_t bytesRead = fStream->read(buffer, readBufferSize);
+            tempStream.write(buffer, bytesRead);
+            sizeRead = tempStream.bytesWritten();
+        } while (!fStream->isAtEnd() &&
+                 (readToEnd || sizeRead < sizeToRead));
+        if (sizeRead < sizeToRead) {
+            return false;
+        }
+
+        const size_t newStreamBufferSize = fStreamBufferSize + sizeRead;
+        SkAutoTDelete<uint8> oldBufferPtr(reinterpret_cast<uint8*>(fStreamBuffer.detach()));
+        void* newBufferPtr = fStreamBuffer.reset(newStreamBufferSize);
+
+        memcpy(reinterpret_cast<uint8*>(newBufferPtr), oldBufferPtr.get(), fStreamBufferSize);
+        tempStream.copyTo(reinterpret_cast<uint8*>(newBufferPtr) + fStreamBufferSize);
+        fStreamBufferSize = newStreamBufferSize;
+        fWholeStreamRead = fStream->isAtEnd();
+        return true;
+    }
+
+    SkAutoTDelete<SkStream> fStream;
+    bool fWholeStreamRead;
+
+    SkAutoMalloc fStreamBuffer;
+    size_t fStreamBufferSize;
+
+    const size_t fReadBufferSize;
+};
+
+SkCodec* SkRawCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkRawStream> rawStream(new SkRawStream(stream));
+    ::piex::PreviewImageData imageData;
+    if (::piex::IsRaw(rawStream.get()) &&
+        ::piex::GetPreviewImageData(rawStream.get(), &imageData) == ::piex::Error::kOk)
+    {
+        SkMemoryStream* memoryStream =
+                rawStream->CopyBuffer(imageData.jpeg_offset, imageData.jpeg_length);
+        return memoryStream ? SkJpegCodec::NewFromStream(memoryStream) : nullptr;
+    }
+
+    dng_host host;
+    dng_info info;
+    SkAutoTDelete<dng_negative> negative(ReadDng(rawStream.get(), &host, &info));
+    if (!negative) {
+        return nullptr;
+    }
+
+    const SkImageInfo& imageInfo = SkImageInfo::Make(
+        negative->DefaultCropSizeH().As_real64(),
+        negative->DefaultCropSizeV().As_real64(),
+        kN32_SkColorType, kOpaque_SkAlphaType);
+    return new SkRawCodec(imageInfo, rawStream.detach());
+}
+
+SkCodec::Result SkRawCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
+                                        size_t dstRowBytes, const Options& options,
+                                        SkPMColor ctable[], int* ctableCount,
+                                        int* rowsDecoded) {
+    const int width = requestedInfo.width();
+    const int height = requestedInfo.height();
+    SkAutoTDelete<dng_image> finalDngImage;
+    try {
+        dng_host host;
+        dng_info info;
+        SkAutoTDelete<dng_negative> negative(ReadDng(fRawStream, &host, &info));
+        if (!negative || !PrepareStage3(fRawStream, &host, &info, negative.get())) {
+            return kInvalidInput;
+        }
+
+        dng_render render(host, *negative);
+        render.SetFinalSpace(dng_space_sRGB::Get());
+        render.SetFinalPixelType(ttByte);
+
+        finalDngImage.reset(render.Render());
+    } catch (...) {
+        return kInvalidInput;
+    }
+
+    void* dstRow = dst;
+    uint8 srcRow[width * 3];
+    SkAutoTDelete<SkSwizzler> swizzler(SkSwizzler::CreateSwizzler(
+            SkSwizzler::kRGB, nullptr, requestedInfo, options));
+    SkASSERT(swizzler);
+    for (int i = 0; i < height; ++i) {
+        dng_pixel_buffer buffer;
+        buffer.fData = &srcRow[0];
+        buffer.fArea = dng_rect(i, 0, i + 1, width);
+        buffer.fPlane = 0;
+        buffer.fPlanes = 3;
+        buffer.fColStep = buffer.fPlanes;
+        buffer.fPlaneStep = 1;
+        buffer.fPixelType = ttByte;
+        buffer.fPixelSize = sizeof(uint8);
+        buffer.fRowStep = sizeof(srcRow);
+
+        try {
+            finalDngImage->Get(buffer, dng_image::edge_zero);
+        } catch (...) {
+            return kInvalidInput;
+        }
+
+        swizzler->swizzle(dstRow, &srcRow[0]);
+        dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
+    }
+    return kSuccess;
+}
+
+SkRawCodec::~SkRawCodec() {
+    if (fRawStream) {

[scroggo, 2016/01/07 20:57:42]

I think it's safe to delete fRawStream even if it's null? 

[yujieqin, 2016/01/08 14:00:12]

Done.

+        delete fRawStream;
+    }
+}
+
+SkRawCodec::SkRawCodec(const SkImageInfo& srcInfo, SkRawStream* stream)
+    : INHERITED(srcInfo, nullptr), fRawStream(stream) {}