suit: Decompression stream filter #20582
Conversation
github-actions
bot
added
the
changelog-entry-required
|
After documentation is built, you will find the preview for this PR here. |
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 642138ef4c5b66dcbdede23733286b7c8d2e505d more detailssdk-nrf:
suit-processor:
Github labels
List of changed files detected by CI (21)Outputs:ToolchainVersion: aedb4c0245 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
nordic-mik7
force-pushed
the
dev/decompress_filter_rebase
branch
from
01d2ca8 to
c3d8fb7
Compare
|
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
nordic-mik7
force-pushed
the
dev/decompress_filter_rebase
branch
2 times, most recently
from
9105b76 to
e10e118
Compare
| * @param[in] size Size of @a buf; data read size | ||
| */ | ||
| typedef suit_plat_err_t (*out_sink_readback_func)(void *sink_ctx, | ||
| size_t offset, uint8_t *buf, size_t size); |
There was a problem hiding this comment.
alignment is off here and below, and line length is 100, sink_ctx, offset and buf can all be on the top line and is under limit
| /** | ||
| * @brief Interface function type for output sink readback, passed to | ||
| * suit_decompress_filter_get() as a callback. Decompression process needs | ||
| * an access to unaltered destination component memory (see WARNING below). |
There was a problem hiding this comment.
| * an access to unaltered destination component memory (see WARNING below). | |
| * access to unaltered destination component memory (see WARNING below). |
| const struct suit_compression_info *compress_info, | ||
| const suit_manifest_class_id_t *class_id, | ||
| const struct stream_sink *out_sink, | ||
| out_sink_readback_func out_sink_readback); |
| * @param[in] offset Offset of output sink area to start reading from | ||
| * @param[out] buf Buffer to read into | ||
| * @param[in] size Size of @a buf; data read size | ||
| */ |
| * @param[in] out_sink Pointer to output stream_sink to be filled with decompressed data | ||
| * @param[in] out_sink_readback Output sink readback function | ||
| * | ||
| * @return SUIT_PLAT_SUCCESS if success otherwise error code |
There was a problem hiding this comment.
should be @retval and have multiple lines e.g.:
* @retval 0 on success.
* @retval -ENOMEM if LwM2M engine couldn't allocate its objects.
| /* Make sure we buffer CHUNK_BUFFER_SIZE bytes | ||
| * in ctx.last_chunk for the flush operation. | ||
| */ | ||
|
|
| return SUIT_PLAT_ERR_INVAL; | ||
| } | ||
|
|
||
| enum nrf_compress_types compress_type = NRF_COMPRESS_TYPE_COUNT; |
There was a problem hiding this comment.
declare variables at top of scope
| * of the stream_sink API. User that holds the flash_sink can readback the data during | ||
| * streaming, as flash_sink has the access to data destination in memory. | ||
| * | ||
| * @param[in] ctx context of the flash_sink |
| * of the stream_sink API. User that holds the ram_sink can readback the data during | ||
| * streaming, as ram_sink has the access to data destination in memory. | ||
| * | ||
| * @param[in] ctx Context of the ram_sink |
| } | ||
|
|
||
| if (ctx_found == false) { | ||
| LOG_ERR("readback with invalid sink_ctx called."); |
There was a problem hiding this comment.
capital first letters for messages
nordic-mik7
force-pushed
the
dev/decompress_filter_rebase
branch
from
e10e118 to
e5db0be
Compare
| rc = Lzma2Dec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize, | ||
| input, &chunk_size, | ||
| (last_part ? LZMA_FINISH_END : LZMA_FINISH_ANY), &status); | ||
| rc = Lzma2Dec_DecodeToDic(&lzma_decoder, decoder->dicHandle->dicBufSize, input, &chunk_size, |
There was a problem hiding this comment.
@nordicjm Please take a look at changes to lzma_decompress() function logic. I implemented them for the 'external dictionary' version but I believe they should be applied for all configurations.
LZMA_FINISH_END flag configures lzma decompression behavior to lookup for the 'end mark' if 'dicLimit' is reached during LzmaDec_DecodeToDic() call. If it is not found, LzmaDec_DecodeToDic() will return error.
If we set LZMA_FINISH_END when we deliver 'last_part', this means we must fit with the output of decompression of this last part in dictionary buffer that is currently available (lzma_decoder.dicBufSize - lzma_decoder.dicPos). As we can provide 'last_part' at any given point, we may exceed 'dicLimit' with decompression output. I believe we should not return an error here, just let the user provide the rest of the 'last_part' in the next call.
Also check on the line 553 is faulthy then - if we fit with our decompression output in dictionary buffer, we should always have chunk_size == input_size. If not, then we should not override offset but just signal that more processing is needed.
nordic-mik7
requested review from
tomchy,
SylwesterKonczyk,
ahasztag,
adsz-nordic and
nordicjm
This commit provides implementation of decompression stream filter module. For this filter, support for Flash sink and RAM sink was added by implementing required sink readback interfaces. Additionally, fixes to lzma.c were introduced as results of testing the filter (cache behavior and ending the decompression stream). Signed-off-by: Michal Kozikowski <[email protected]>
nordic-mik7
force-pushed
the
dev/decompress_filter_rebase
branch
from
e5db0be to
642138e
Compare
|
This PR provides implementation of decompression stream filter module.
For this filter, support for Flash sink and RAM sink was added by implementing required sink
readback interfaces.
Additionally, fixes to lzma.c were introduced as results of testing the filter (cache behavior and
ending the decompression stream).