Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1782063002:
Add a quick check to the TIFF header of DNG image (Closed)
DescriptionAdd a quick check to the TIFF header of DNG image
BUG=b/27475341
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1782063002
Committed: https://skia.googlesource.com/skia/+/d6215cf4a5f416cf0b64a4fbba95c519f03fe467
Patch Set 1 #
Total comments: 2
Messages
Total messages: 16 (7 generated)
Description was changed from ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 ========== to ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
yujieqin@google.com changed reviewers: + msarett@google.com, scroggo@google.com
The CQ bit was checked by yujieqin@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782063002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782063002/1
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2016-03-10 19:02 UTC
lgtm
The CQ bit was unchecked by yujieqin@google.com
The CQ bit was checked by yujieqin@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782063002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782063002/1
Message was sent while issue was closed.
Description was changed from ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add a quick check to the TIFF header of DNG image BUG=b/27475341 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/d6215cf4a5f416cf0b64a4fbba95c519f03fe467 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://skia.googlesource.com/skia/+/d6215cf4a5f416cf0b64a4fbba95c519f03fe467
Message was sent while issue was closed.
https://codereview.chromium.org/1782063002/diff/1/src/codec/SkRawCodec.cpp File src/codec/SkRawCodec.cpp (right): https://codereview.chromium.org/1782063002/diff/1/src/codec/SkRawCodec.cpp#ne... src/codec/SkRawCodec.cpp:450: if (!dngImage->isTiffHeaderValid()) { Just to make sure I understand: All dngs should have a TIFF header. So we fail immediately if there is no TIFF header. Otherwise, we go ahead and check PIEX/dng_sdk as usual. This should stop us from buffering video streams etc. because they likely won't match the TIFF header.
Message was sent while issue was closed.
https://codereview.chromium.org/1782063002/diff/1/src/codec/SkRawCodec.cpp File src/codec/SkRawCodec.cpp (right): https://codereview.chromium.org/1782063002/diff/1/src/codec/SkRawCodec.cpp#ne... src/codec/SkRawCodec.cpp:450: if (!dngImage->isTiffHeaderValid()) { On 2016/03/10 13:51:56, msarett wrote: > Just to make sure I understand: > > All dngs should have a TIFF header. So we fail immediately if there is no TIFF > header. Otherwise, we go ahead and check PIEX/dng_sdk as usual. > > This should stop us from buffering video streams etc. because they likely won't > match the TIFF header. Yes, DNG image is basically an extended TIFF. It should always contain the standard TIFF header. This check should block out most of the non-TIFF data (e.g. video), so that we don't even need to try to buffer the up-to-100MB data.
Message was sent while issue was closed.
LGTM |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
