Conversation
We currently have one type in decoder/image.rs and another in decoder/mod.rs which each contributes part of the computation for laying out the output buffer. Confusingly though they rely on shared implementation details such as determining the number of channels that are written and their bit depth. We should have one shared method and resulting struct instead.
197g
force-pushed
the
decode-planar-data
branch
from
bc97880 to
c3882b2
Compare
2 tasks
This is well-defined even if we add sub-sampling support later on. We can adjust the layout type accordingly with factors per each plane. Note that the layout, in theory, does not depend on the index of the unit if we describe all planes for a multi-planar layout. The specification requires that the subsampling factors divide the tile sizes. However we have to handle non-planar images here and this leaves us open to other potential parameters.
By adding the type interpretation to `BufferLayoutPreference` (simply computed from the same fields that are trivial to fetch in `ReadoutLayout` with the rest) we can allocate an appropriate `DecoderResult` without any `Decoder` instance. We still want a Limits instance though. This lets a caller read _typed_ data from the `_bytes` methods which perform less allocations. It also lets them read multiple planes in a more convenient interface. See, `read_image` will keep its planar treatment bug for a little bit so you don't accidentally break during an upgrade. This also implies less code is being monomorphized, just a bonus.
We previously handed it a wrapper that was supposedly type-safe but really internally cast the buffer to bytes. That was rather pointless as the `_bytes` method could always be used instead. We replace the semantics by a utility which allocates an appropriate buffer with the internal limits, wrapping the exact sequence identified previously in the example as a pattern.
Member
Author
|
Hm, I wasn't aware we made use of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft because the interface is a bit odd.
read_image_bytesis enhanced to read additional planes if the buffer is large enough.PreferredBufferLayoutis augmented with fields to describe those additional requirements, with the compatibility that previous fields encode only the first plane. This should be changed in a next major version.image.rsso we have one source of truth for the layout of sub-byte color components, planes, chunk sizes, and strides. All relevant values are computed in one function and then used.expand_chunkis prepared for custom strides which it previously supported in a parameter but that would not scale to multiple planes; also support did not seem consistent? It was not exercised in the public interface and I would not have trusted that..