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

feat: bitdepth option for avif/heif #4036

Merged
merged 1 commit into from
Mar 21, 2024
Merged

feat: bitdepth option for avif/heif #4036

merged 1 commit into from
Mar 21, 2024

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Mar 20, 2024

Following #4031, this PR exposes a bitdepth setting to allow outputting 10-bit or 12-bit AVIF/HEIF files. The default remains 8, so this doesn't change the current behavior.

Fixes #4031

@lovell
Copy link
Owner

lovell commented Mar 20, 2024

Thank you for the PR. The prebuilt binaries only include support for a bitdepth of 8, so you won't be able to add a unit test for this path, but you can check valid values are accepted.

It looks like lots of the tests are failing as the new heifBitdepth property needs adding somewhere around here in constructor.js.

@mertalev
Copy link
Contributor Author

Oh, right! I forgot the default build only supports 8-bit output.

add validation unit tests

add ushort unit test

add test for valid bitdepth, make test formatting consistent

consistent test wording

add setting to constructor

remove ushort test

fix unit tests

bitdepth is not a boolean
@mertalev
Copy link
Contributor Author

I got the tests working locally after adding --build-from-source and fixing a few bugs. Bit depth as a boolean didn't work too well haha

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, all the tests are passing now, this is good to merge.

(I'll also add something to the docs to highlight that the prebuilt binaries only support a bitdepth of 8.)

@lovell lovell added this to the v0.33.3 milestone Mar 21, 2024
@lovell lovell merged commit 3c26080 into lovell:main Mar 21, 2024
15 checks passed
lovell added a commit that referenced this pull request Mar 21, 2024
@lovell
Copy link
Owner

lovell commented Mar 21, 2024

I added a guard against this property when using the prebuilt binaries via commit aa1bbcb as the failure condition could otherwise lead to a segfault.

@mertalev
Copy link
Contributor Author

Yeah, that makes sense.

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.

Support outputting higher bit-depth AVIF
2 participants