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

alternate implementation to deal with extraneous RST code in C++ generated lepton files #44

Merged
merged 6 commits into from
Sep 8, 2023

Conversation

mcroomp
Copy link
Collaborator

@mcroomp mcroomp commented Sep 7, 2023

I think this is slightly less intrusive way to support this weird corner case. Unfortunately the garbage RST code stuff is not very robust in the C++ version. For the Rust version I just encoded everything using the garbage data which is much more robust.

if self.rst_err.len() > 0 {
let cumulative_reset_markers = if self.jpeg_header.rsti != 0 {
((self.jpeg_header.mcuh * self.jpeg_header.mcuv) - 1) / self.jpeg_header.rsti
} else {
0
} as u8;
for i in 0..self.rst_err[0] as u8 {
// the C++ version will strangely sometimes ask for extra rst codes even if we are at the end of the file and shouldn't
// be emitting anything more, so if we are over or at the size limit then don't emit the RST code
if amount_written >= size_limit {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not it be more correct "amount_written + 2 > size_limit"?

Copy link
Contributor

@gbrovman gbrovman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just left tiny fix proposal, other than that it is good to go. I considered to do it in this way before going with the Seek option, but I missed the results write_all loop that allows to easily summarize the written amount.

@mcroomp mcroomp merged commit 62201ee into main Sep 8, 2023
3 checks passed
@mcroomp mcroomp deleted the kristof_outputsize branch September 8, 2023 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants