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

Brunsli for JPG? #97

Closed
M-Gonzalo opened this issue May 10, 2019 · 9 comments · Fixed by #101
Closed

Brunsli for JPG? #97

M-Gonzalo opened this issue May 10, 2019 · 9 comments · Fixed by #101
Assignees

Comments

@M-Gonzalo
Copy link

It's MIT licensed. And quite a bit faster than packJPG (a lot, actually, but a bit less strong). It is also under development, so it stands to reason we should recieve at least some improvements in the future...

What do you think, Christian?

@schnaader
Copy link
Owner

Thanks for this advice, tested it and it is much faster indeed (size different is about 1% in my tests, but it was about 5 times faster both compressing and decompressing). I'll have a look at the code and try to integrate it. I'm not sure if it will replace packJPG, however. Most likely using packJPG will stay an option (for slower, but better JPG compression).

@schnaader schnaader self-assigned this May 13, 2019
@schnaader schnaader added this to the Precomp v0.4.8 milestone May 13, 2019
@andrew-epstein
Copy link

I personally am in favor of integrating it, while leaving in packJPG as well, and then having one of them the default and the other toggleable (doesn't matter to me which is default). @schnaader please feel free to let me know if you want any help integrating / testing.

@M-Gonzalo
Copy link
Author

M-Gonzalo commented May 13, 2019

I agree. I don't believe there is any need to undo all the work put into packJPG.

In my tests, there are a few images in which brunsli fails. Maybe packJPG could be handling those when it's not being used as the default method too.

Thanks for the feedback BTW

@andrew-epstein
Copy link

Also, I found at least one, maybe more, issue(s) with packJPG by using clang's address sanitizer. I'll create GitHub issues when I have time.

@schnaader
Copy link
Owner

In my tests, there are a few images in which brunsli fails.

@M-Gonzalo : Could you share those images? They would be useful for testing.

@M-Gonzalo
Copy link
Author

In my tests, there are a few images in which brunsli fails.

@M-Gonzalo : Could you share those images? They would be useful for testing.

Sure thing! As soon as I find some shareable ones.

@andrew-epstein
Copy link

@M-Gonzalo how does Brunsli fail? It fails to make the file smaller, or it crashes?

@M-Gonzalo
Copy link
Author

@M-Gonzalo how does Brunsli fail? It fails to make the file smaller, or it crashes?

You know, I've been revisiting my quick tests to give you and Christian a proper answer, and I found that maybe it was a bug in the script what I saw. There was a couple of files without their .bru counterpart so I just assumed it was because of a crash. Now I'm thinking there was no crash at all. Sorry, it was a quick and crude test while I was away at work. I'll be doing a lengthier analysis in the following weeks.

@andrew-epstein
Copy link

I've got afl-fuzz running on it right now and it's only ~40% of the way through the first cycle, but so far I haven't seen any crashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants