Make SkAndroidCodec (optionally) respect origin
Bug: b/63909536
ImageDecoder will respect the origin, but BitmapFactory will maintain
its current behavior of not respecting it. Add an option to respect it.
In addition, add support for reading the EXIF data from a WEBP. This
seems to be an uncommon use case, but is occasionally used when
converting from a JPEG. Add 8 WEBPs, all converted (with cwebp) from
their analogous JPEG files already checked in.
Change-Id: I38afca58c86fa99ee9ab7d1dc83aaa4f23132c11
Reviewed-on: https://skia-review.googlesource.com/95300
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp
index 486257f..536e344 100644
--- a/src/codec/SkAndroidCodec.cpp
+++ b/src/codec/SkAndroidCodec.cpp
@@ -9,6 +9,8 @@
#include "SkCodec.h"
#include "SkCodecPriv.h"
#include "SkMakeUnique.h"
+#include "SkPixmap.h"
+#include "SkPixmapPriv.h"
#include "SkRawAdapterCodec.h"
#include "SkSampledCodec.h"
#include "SkWebpAdapterCodec.h"
@@ -57,19 +59,32 @@
return false;
}
-SkAndroidCodec::SkAndroidCodec(SkCodec* codec)
- : fInfo(codec->getInfo())
+static inline SkImageInfo adjust_info(SkCodec* codec,
+ SkAndroidCodec::ExifOrientationBehavior orientationBehavior) {
+ auto info = codec->getInfo();
+ if (orientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
+ || !SkPixmapPriv::ShouldSwapWidthHeight(codec->getOrigin())) {
+ return info;
+ }
+ return SkPixmapPriv::SwapWidthHeight(info);
+}
+
+SkAndroidCodec::SkAndroidCodec(SkCodec* codec, ExifOrientationBehavior orientationBehavior)
+ : fInfo(adjust_info(codec, orientationBehavior))
+ , fOrientationBehavior(orientationBehavior)
, fCodec(codec)
{}
SkAndroidCodec::~SkAndroidCodec() {}
-std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromStream(std::unique_ptr<SkStream> stream, SkPngChunkReader* chunkReader) {
+std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
+ SkPngChunkReader* chunkReader) {
auto codec = SkCodec::MakeFromStream(std::move(stream), nullptr, chunkReader);
return MakeFromCodec(std::move(codec));
}
-std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec) {
+std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec,
+ ExifOrientationBehavior orientationBehavior) {
if (nullptr == codec) {
return nullptr;
}
@@ -88,14 +103,16 @@
#ifdef SK_HAS_HEIF_LIBRARY
case SkEncodedImageFormat::kHEIF:
#endif
- return skstd::make_unique<SkSampledCodec>(codec.release());
+ return skstd::make_unique<SkSampledCodec>(codec.release(), orientationBehavior);
#ifdef SK_HAS_WEBP_LIBRARY
case SkEncodedImageFormat::kWEBP:
- return skstd::make_unique<SkWebpAdapterCodec>((SkWebpCodec*) codec.release());
+ return skstd::make_unique<SkWebpAdapterCodec>((SkWebpCodec*) codec.release(),
+ orientationBehavior);
#endif
#ifdef SK_CODEC_DECODES_RAW
case SkEncodedImageFormat::kDNG:
- return skstd::make_unique<SkRawAdapterCodec>((SkRawCodec*)codec.release());
+ return skstd::make_unique<SkRawAdapterCodec>((SkRawCodec*)codec.release(),
+ orientationBehavior);
#endif
default:
return nullptr;
@@ -325,24 +342,43 @@
get_scaled_dimension(subset.height(), sampleSize)};
}
-SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,
- size_t rowBytes, const AndroidOptions* options) {
- if (!pixels) {
+static bool acceptable_result(SkCodec::Result result) {
+ switch (result) {
+ // These results mean a partial or complete image. They should be considered
+ // a success by SkPixmapPriv.
+ case SkCodec::kSuccess:
+ case SkCodec::kIncompleteInput:
+ case SkCodec::kErrorInInput:
+ return true;
+ default:
+ return false;
+ }
+}
+
+SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
+ void* requestPixels, size_t requestRowBytes, const AndroidOptions* options) {
+ if (!requestPixels) {
return SkCodec::kInvalidParameters;
}
- if (rowBytes < info.minRowBytes()) {
+ if (requestRowBytes < requestInfo.minRowBytes()) {
return SkCodec::kInvalidParameters;
}
+ SkImageInfo adjustedInfo = fInfo;
+ if (ExifOrientationBehavior::kRespect == fOrientationBehavior
+ && SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
+ adjustedInfo = SkPixmapPriv::SwapWidthHeight(adjustedInfo);
+ }
+
AndroidOptions defaultOptions;
if (!options) {
options = &defaultOptions;
} else if (options->fSubset) {
- if (!is_valid_subset(*options->fSubset, fInfo.dimensions())) {
+ if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) {
return SkCodec::kInvalidParameters;
}
- if (SkIRect::MakeSize(fInfo.dimensions()) == *options->fSubset) {
+ if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) {
// The caller wants the whole thing, rather than a subset. Modify
// the AndroidOptions passed to onGetAndroidPixels to not specify
// a subset.
@@ -352,7 +388,27 @@
}
}
- return this->onGetAndroidPixels(info, pixels, rowBytes, *options);
+ if (ExifOrientationBehavior::kIgnore == fOrientationBehavior) {
+ return this->onGetAndroidPixels(requestInfo, requestPixels, requestRowBytes, *options);
+ }
+
+ SkCodec::Result result;
+ auto decode = [this, options, &result](const SkPixmap& pm) {
+ result = this->onGetAndroidPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), *options);
+ return acceptable_result(result);
+ };
+
+ SkPixmap dst(requestInfo, requestPixels, requestRowBytes);
+ if (SkPixmapPriv::Orient(dst, fCodec->getOrigin(), decode)) {
+ return result;
+ }
+
+ // Orient returned false. If onGetAndroidPixels succeeded, then Orient failed internally.
+ if (acceptable_result(result)) {
+ return SkCodec::kInternalError;
+ }
+
+ return result;
}
SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,
diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index 84215b9..1251d8d 100644
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -13,6 +13,7 @@
#include "SkColorSpaceXformPriv.h"
#include "SkColorTable.h"
#include "SkEncodedInfo.h"
+#include "SkEncodedOrigin.h"
#include "SkImageInfo.h"
#include "SkTypes.h"
@@ -295,4 +296,6 @@
return (kOpaque_SkAlphaType == srcAlphaType) ? kOpaque_SkAlphaType : dstAlphaType;
}
+bool is_orientation_marker(const uint8_t* data, size_t data_length, SkEncodedOrigin* orientation);
+
#endif // SkCodecPriv_DEFINED
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 80a32ad..d2c023b 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -49,32 +49,36 @@
return false;
}
- const uint8_t* data = marker->data;
constexpr uint8_t kExifSig[] { 'E', 'x', 'i', 'f', '\0' };
- if (memcmp(data, kExifSig, sizeof(kExifSig))) {
+ if (memcmp(marker->data, kExifSig, sizeof(kExifSig))) {
return false;
}
+ // Account for 'E', 'x', 'i', 'f', '\0', '<fill byte>'.
+ constexpr size_t kOffset = 6;
+ return is_orientation_marker(marker->data + kOffset, marker->data_length - kOffset,
+ orientation);
+}
+
+bool is_orientation_marker(const uint8_t* data, size_t data_length, SkEncodedOrigin* orientation) {
bool littleEndian;
- if (!is_valid_endian_marker(data + 6, &littleEndian)) {
+ if (!is_valid_endian_marker(data, &littleEndian)) {
return false;
}
// Get the offset from the start of the marker.
- // Account for 'E', 'x', 'i', 'f', '\0', '<fill byte>'.
// Though this only reads four bytes, use a larger int in case it overflows.
- uint64_t offset = get_endian_int(data + 10, littleEndian);
- offset += sizeof(kExifSig) + 1;
+ uint64_t offset = get_endian_int(data + 4, littleEndian);
// Require that the marker is at least large enough to contain the number of entries.
- if (marker->data_length < offset + 2) {
+ if (data_length < offset + 2) {
return false;
}
uint32_t numEntries = get_endian_short(data + offset, littleEndian);
// Tag (2 bytes), Datatype (2 bytes), Number of elements (4 bytes), Data (4 bytes)
const uint32_t kEntrySize = 12;
- const auto max = SkTo<uint32_t>((marker->data_length - offset - 2) / kEntrySize);
+ const auto max = SkTo<uint32_t>((data_length - offset - 2) / kEntrySize);
numEntries = SkTMin(numEntries, max);
// Advance the data to the start of the entries.
diff --git a/src/codec/SkRawAdapterCodec.cpp b/src/codec/SkRawAdapterCodec.cpp
index 4917091..d4b3149 100644
--- a/src/codec/SkRawAdapterCodec.cpp
+++ b/src/codec/SkRawAdapterCodec.cpp
@@ -9,8 +9,8 @@
#include "SkCodecPriv.h"
#include "SkRawAdapterCodec.h"
-SkRawAdapterCodec::SkRawAdapterCodec(SkRawCodec* codec)
- : INHERITED(codec)
+SkRawAdapterCodec::SkRawAdapterCodec(SkRawCodec* codec, ExifOrientationBehavior behavior)
+ : INHERITED(codec, behavior)
{}
SkISize SkRawAdapterCodec::onGetSampledDimensions(int sampleSize) const {
diff --git a/src/codec/SkRawAdapterCodec.h b/src/codec/SkRawAdapterCodec.h
index 29e817c..d4828ce 100644
--- a/src/codec/SkRawAdapterCodec.h
+++ b/src/codec/SkRawAdapterCodec.h
@@ -21,8 +21,7 @@
*/
class SkRawAdapterCodec : public SkAndroidCodec {
public:
-
- explicit SkRawAdapterCodec(SkRawCodec*);
+ explicit SkRawAdapterCodec(SkRawCodec*, ExifOrientationBehavior);
~SkRawAdapterCodec() override {}
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index b8e2a56..ac0539f 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -12,8 +12,8 @@
#include "SkSampler.h"
#include "SkTemplates.h"
-SkSampledCodec::SkSampledCodec(SkCodec* codec)
- : INHERITED(codec)
+SkSampledCodec::SkSampledCodec(SkCodec* codec, ExifOrientationBehavior behavior)
+ : INHERITED(codec, behavior)
{}
SkISize SkSampledCodec::accountForNativeScaling(int* sampleSizePtr, int* nativeSampleSize) const {
diff --git a/src/codec/SkSampledCodec.h b/src/codec/SkSampledCodec.h
index 4bcf5bc..faa955c 100644
--- a/src/codec/SkSampledCodec.h
+++ b/src/codec/SkSampledCodec.h
@@ -16,8 +16,7 @@
*/
class SkSampledCodec : public SkAndroidCodec {
public:
-
- explicit SkSampledCodec(SkCodec*);
+ explicit SkSampledCodec(SkCodec*, ExifOrientationBehavior);
~SkSampledCodec() override {}
diff --git a/src/codec/SkWebpAdapterCodec.cpp b/src/codec/SkWebpAdapterCodec.cpp
index 18e1a54..81011d0 100644
--- a/src/codec/SkWebpAdapterCodec.cpp
+++ b/src/codec/SkWebpAdapterCodec.cpp
@@ -9,8 +9,8 @@
#include "SkCodecPriv.h"
#include "SkWebpAdapterCodec.h"
-SkWebpAdapterCodec::SkWebpAdapterCodec(SkWebpCodec* codec)
- : INHERITED(codec)
+SkWebpAdapterCodec::SkWebpAdapterCodec(SkWebpCodec* codec, ExifOrientationBehavior behavior)
+ : INHERITED(codec, behavior)
{}
SkISize SkWebpAdapterCodec::onGetSampledDimensions(int sampleSize) const {
diff --git a/src/codec/SkWebpAdapterCodec.h b/src/codec/SkWebpAdapterCodec.h
index b2c6c7b..fc8328c 100644
--- a/src/codec/SkWebpAdapterCodec.h
+++ b/src/codec/SkWebpAdapterCodec.h
@@ -17,7 +17,7 @@
class SkWebpAdapterCodec : public SkAndroidCodec {
public:
- explicit SkWebpAdapterCodec(SkWebpCodec*);
+ explicit SkWebpAdapterCodec(SkWebpCodec*, ExifOrientationBehavior);
~SkWebpAdapterCodec() override {}
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index aa3547d..d9b13e8 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -87,14 +87,25 @@
}
}
- WebPChunkIterator chunkIterator;
- SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
sk_sp<SkColorSpace> colorSpace = nullptr;
- if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
- colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+ {
+ WebPChunkIterator chunkIterator;
+ SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
+ if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
+ colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+ }
+ if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
+ colorSpace = SkColorSpace::MakeSRGB();
+ }
}
- if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
- colorSpace = SkColorSpace::MakeSRGB();
+
+ SkEncodedOrigin origin = kDefault_SkEncodedOrigin;
+ {
+ WebPChunkIterator chunkIterator;
+ SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
+ if (WebPDemuxGetChunk(demux, "EXIF", 1, &chunkIterator)) {
+ is_orientation_marker(chunkIterator.chunk.bytes, chunkIterator.chunk.size, &origin);
+ }
}
// Get the first frame and its "features" to determine the color and alpha types.
@@ -156,10 +167,12 @@
return nullptr;
}
+
*result = kSuccess;
SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
return std::unique_ptr<SkCodec>(new SkWebpCodec(width, height, info, std::move(colorSpace),
- std::move(stream), demux.release(), std::move(data)));
+ std::move(stream), demux.release(), std::move(data),
+ origin));
}
SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const {
@@ -646,9 +659,9 @@
SkWebpCodec::SkWebpCodec(int width, int height, const SkEncodedInfo& info,
sk_sp<SkColorSpace> colorSpace, std::unique_ptr<SkStream> stream,
- WebPDemuxer* demux, sk_sp<SkData> data)
+ WebPDemuxer* demux, sk_sp<SkData> data, SkEncodedOrigin origin)
: INHERITED(width, height, info, SkColorSpaceXform::kBGRA_8888_ColorFormat, std::move(stream),
- std::move(colorSpace))
+ std::move(colorSpace), origin)
, fDemux(demux)
, fData(std::move(data))
, fFailed(false)
diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h
index 010771b..fdd5422 100644
--- a/src/codec/SkWebpCodec.h
+++ b/src/codec/SkWebpCodec.h
@@ -48,7 +48,7 @@
private:
SkWebpCodec(int width, int height, const SkEncodedInfo&, sk_sp<SkColorSpace>,
- std::unique_ptr<SkStream>, WebPDemuxer*, sk_sp<SkData>);
+ std::unique_ptr<SkStream>, WebPDemuxer*, sk_sp<SkData>, SkEncodedOrigin);
SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> fDemux;