Commit Graph

4 Commits

Author SHA1 Message Date
Leon Scroggins III
600effbdc7 Improve the Codec_end test and add fixes
Better imitate the original Android bug. Create a stream with
multiple images in it, and verify that it successfully decodes after
decoding once.

This exposes a bug in SkPngCodec, which did not work for interlaced
images.

Test more formats that also happen to succeed: ICO, BMP, and WBMP

This explicitly does *not* attempt to fix sampled or subset
decodes, which already stopped early when decoding as an
optimization.

Change-Id: Ib0b8918f14ba3fb0fa31e9c71c8100dcbeeb465f
Reviewed-on: https://skia-review.googlesource.com/14104
Reviewed-by: Matt Sarett <msarett@google.com>
2017-04-25 11:45:15 +00:00
Leon Scroggins III
83239658f2 Reland "Make SkPngCodec only read as much of the stream as necessary"
(Originally uploaded as 13900.)

Previously, SkPngCodec assumed that the stream only contained one
image, which ended at the end of the stream. It read the stream in
arbitrarily-sized chunks, and then passed that data to libpng for
processing.

If a stream contains more than one image, this may result in reading
beyond the end of the image, making future reads read the wrong data.

Now, SkPngCodec starts by reading 8 bytes at a time. After the
signature, 8 bytes is enough to know which chunk is next and how many
bytes are in the chunk.

When decoding the size, we stop when we reach IDAT, and when decoding
the image, we stop when we reach IEND.

This manual parsing is necessary to support APNG, which is planned in
the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
was a workaround for reading more than necessary at the beginning of
the image.

Add a test that simulates the issue, by decoding a special stream that
reports an error if the codec attempts to read beyond the end.

Temporarily disable the partial decoding tests for png. A larger change
will be necessary to get those working again, and no clients are
currently relying on incrementally decoding PNGs (i.e. decode part of
an image, then decode further with more data).

Include a workaround for older versions of libpng (e.g. 1.2 in
Google3). In older versions, if the row callback is null when the
IDAT header is processed, reading the image will fail. When we see the
IDAT, we save the length and process a recreated IDAT header later,
after the row callback has been set.

Bug: skia:5368
Bug:b/34073812
Test: Existing tests, plus a new test in dm.

Change-Id: I293a4ddc013b82669a8b735062228b26d0bce933
Reviewed-on: https://skia-review.googlesource.com/13984
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
2017-04-21 20:49:55 +00:00
Leon Scroggins
434b6e81a5 Revert "Make SkPngCodec only read as much of the stream as necessary"
This reverts commit 2c65d51612.

Reason for revert: Causing failures in Google3 (https://test.corp.google.com/ui#cl=153703311&flags=CAMQAg==&id=OCL:153703311:BASE:153703364:1492695824938:4db2240d&t=//chrome/skia/dm_wrapper:dm_wrapper) and differences in Gold. This change was not intended to change the output.

Original change's description:
> Make SkPngCodec only read as much of the stream as necessary
> 
> Previously, SkPngCodec assumed that the stream only contained one
> image, which ended at the end of the stream. It read the stream in
> arbitrarily-sized chunks, and then passed that data to libpng for
> processing.
> 
> If a stream contains more than one image, this may result in reading
> beyond the end of the image, making future reads read the wrong data.
> 
> Now, SkPngCodec starts by reading 8 bytes at a time. After the
> signature, 8 bytes is enough to know which chunk is next and how many
> bytes are in the chunk.
> 
> When decoding the size, we stop when we reach IDAT, and when decoding
> the image, we stop when we reach IEND.
> 
> This manual parsing is necessary to support APNG, which is planned in
> the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
> was a workaround for reading more than necessary at the beginning of
> the image.
> 
> Add a test that simulates the issue, by decoding a special stream that
> reports an error if the codec attempts to read beyond the end.
> 
> Temporarily disable the partial decoding tests for png. A larger change
> will be necessary to get those working again, and no clients are
> currently relying on incrementally decoding PNGs (i.e. decode part of
> an image, then decode further with more data).
> 
> Bug: skia:5368
> BUG:34073812
> 
> Change-Id: If832f7b20565411226fb5be3c305a4d16bf9269d
> Reviewed-on: https://skia-review.googlesource.com/13900
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Matt Sarett <msarett@google.com>
> 

TBR=msarett@google.com,scroggo@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I2f82e9960dda7bf5c646774df84320dadb7b930e
Reviewed-on: https://skia-review.googlesource.com/13971
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
2017-04-20 15:07:23 +00:00
Leon Scroggins III
2c65d51612 Make SkPngCodec only read as much of the stream as necessary
Previously, SkPngCodec assumed that the stream only contained one
image, which ended at the end of the stream. It read the stream in
arbitrarily-sized chunks, and then passed that data to libpng for
processing.

If a stream contains more than one image, this may result in reading
beyond the end of the image, making future reads read the wrong data.

Now, SkPngCodec starts by reading 8 bytes at a time. After the
signature, 8 bytes is enough to know which chunk is next and how many
bytes are in the chunk.

When decoding the size, we stop when we reach IDAT, and when decoding
the image, we stop when we reach IEND.

This manual parsing is necessary to support APNG, which is planned in
the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
was a workaround for reading more than necessary at the beginning of
the image.

Add a test that simulates the issue, by decoding a special stream that
reports an error if the codec attempts to read beyond the end.

Temporarily disable the partial decoding tests for png. A larger change
will be necessary to get those working again, and no clients are
currently relying on incrementally decoding PNGs (i.e. decode part of
an image, then decode further with more data).

Bug: skia:5368
BUG:34073812

Change-Id: If832f7b20565411226fb5be3c305a4d16bf9269d
Reviewed-on: https://skia-review.googlesource.com/13900
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
2017-04-20 13:40:17 +00:00