Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] master from image-rs:master #7

Open
wants to merge 610 commits into
base: master
Choose a base branch
from

Conversation

pull[bot]
Copy link

@pull pull bot commented Mar 1, 2020

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added ⤵️ pull merge-conflict Resolve conflicts manually labels Mar 1, 2020
fintelia and others added 28 commits December 23, 2022 00:24
Optimize Sub and Paeth filtering
Consolidate Avg remainder loop to main loop
Use iterators in Up and Avg filtering
Drastically improve encoding performance
Add an initial roundtrip fuzz target
Shnatsel and others added 30 commits September 27, 2024 19:20
`unstable` feature doesn't affect encoding; + fix grammar
Replace handwritten SIMD implementation with autovectorization for +10% perf
AFAICT before this commit `cargo test` didn't cover the code path in
`Reader.next_frame` that results in `PolledAfterEndOfImage` error.  This
commit adds unit tests that provide such coverage.
This commit adds a test with the main assertions commented out
(because they would fail as-is).  A fix comes in the follow-up commit.
Before this commit, `next_frame` would take care to read till the end of
an `IDAT`/`fdAT` chunks sequence, by calling
`self.decoder.finish_decoding` when
`!self.subframe.consumed_and_flushed`.  `next_row` wouldn't do this and
therefore the next call to `next_frame` would be temporarily stuck on
the previous frame (i.e. finishing decoding after the previous frame
rather than advancing to the next frame).

After this commit, the `finish_decoding` code is extracted to a separate
helper function that is called by *both* `next_frame` and
`next_interlaced_row` (once they detect that all rows of a frame have
been already decoded).

This commit fixes the `test_row_by_row_then_next_frame` unit test.
In an earlier commit 278b1d4 we
stopped using `Reader.next_frame` for deciding whether to call
`read_until_image_data` from `next_frame`.  This commit goes one
step further and removes the `next_frame` field entirely.

There are no known cases where the previous code would result
in incorrect behavior, but this commit still seems desirable:

* It simplifies the code
* It ensures that a single "end-of-IDAT/fdAT-sequence" event kind
  controls both 1) `consumed_and_flushed` state, and 2) counting
  remaining frames.  (Before this commit `next_frame` would also be
  mutated after a separate "fcTL-encountered"  event.  And also after
  "end-of-IDAT/fdAT-sequence" but only from `next_frame` and not from
  `next_row`.)
When `StreamingDecoder` reports an error, it leaves `state` set to
`None`.  Before this commit, calling `next_frame` in this state would
have led to an infinite loop:

* `ReadDecoder::decode_next` would loop forever (`!self.at_eof` is true
  after an error) and would fail to make progress, because
* When `StreamingDecoder::update` sees `state` saw set to `None` then
  before this commit it wouldn't enter the `next_state` loop and would
  immediately return no progress
  (`Ok((/* consumer bytes = */ 0, Decoded::Nothing))`).

After this commit, `StreamingDecoder::update` checks if the `state` is
`None` and treats this as an error.
Before this commit calling `Reader.finish` a 2nd time would return a
random, accidental error (`UnexpectedEof`), because of how
`StreamingDecoder`'s `state` is set after processing the `IEND` chunk.
After this commit, `Reader.finish` will handle this condition
explicitly, in a similar same way to how `next_frame` handles being
called when we have already consumed all the frames.
After a recent commit, all public APIs of a `Reader` take care of not
going beyond the `IEND` chunk and returning `PolledAfterEndOfImage`
instead.  This means that tracking `at_eof` at the level of
`ReadDecoder` is obsolete and leads to unnecessary complexity.  This
commit refactors away this complexity.
`ReadDecoder::finish_decoding` now reuses `decode_next` instead of
duplicating some of its code.  Some duplication (e.g. handling of
`Decoded::Nothing` remains - this will be taken care of in a subsequent
commit).
This commit means that `decode_next` can be a private method of
`ReadDecoder` (this is not enforced yet, before a subsequent commit
moves `ReadDecoder` into a separate `mod`ule).
Before this commit `fn decode_next` would `loop` to skip
`Decoded::Nothing` events.  This is unnecessary, because all the callers
of `decode_next` already account for `Decoded::Nothing` (explicitly or
implicitly via a wildcard).
This commit helps to keep some aspects of `ReadDecoder` private
(e.g. its fields, `decode_next` methods, etc.).  This commit
also means that `mod.rs` no longer directly depends on `Decoded`
nor `StreamingDecoder`.
Simplify and encapsulate `ReadDecoder` implementation
Extract a separate `unfiltered_rows_buffer` module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⤵️ pull merge-conflict Resolve conflicts manually
Projects
None yet
Development

Successfully merging this pull request may close these issues.