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

Fix #132 #135

Closed
wants to merge 0 commits into from
Closed

Fix #132 #135

wants to merge 0 commits into from

Conversation

lovasoa
Copy link
Contributor

@lovasoa lovasoa commented Feb 13, 2020

Criterion report on my computer:

 Benchmarking decode a 512x512 progressive JPEG: Collecting 100 samples in estimated 27.108 s (5050 i                                                                                                    decode a 512x512 progressive JPEG
                         time:   [5.3440 ms 5.3549 ms 5.3669 ms]
                         change: [-24.307% -23.085% -21.920%] (p = 0.00 < 0.05)
                         Performance has improved.
 Found 3 outliers among 100 measurements (3.00%)
   2 (2.00%) high mild
   1 (1.00%) high severe

@fintelia
Copy link
Contributor

Just to confirm, are you sure that the logical OR operation what this code is supposed to be doing? I posted that as a guess in the issue thread, but I don't actually know how jpeg decoding works so I was just speculating based on the shape of the surrounding code.

@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 13, 2020

No, I did not check... The specification for the part in the code in in G 1.2.3 of the JPEG specification, if you want to have a look yourself : https://books.google.fr/books?id=AepB_PZ_WMkC&pg=PA484&lpg=PA484&dq=Jpeg+Section+G.1.2.3&source=bl&ots=USKKer5RuP&sig=ACfU3U27xg2SO6Xh3LzRCXmCkqZxwFmspg&hl=en&sa=X&ved=2ahUKEwjv4bb11s7nAhUDXqwKHXnjClQQ6AEwCnoECAsQAQ#v=onepage&q&f=false

@lovasoa
Copy link
Contributor Author

lovasoa commented Feb 13, 2020

I also gave maintainers edit rights on this pull request, so if you want to fix something, you can commit directly on lovasoa:master if you want

src/decoder.rs Outdated Show resolved Hide resolved
@lovasoa
Copy link
Contributor Author

lovasoa commented Mar 3, 2020

I added the whitespace. But what I think we need more is someone reviewing that against the JPEG specification.

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.

Panic: attempt to subtract with overflow
3 participants