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
Add avifImageYUVToRGBLibYUV() 10bpc support #838
Conversation
Call the libyuv functions that convert 10bpc YUV to 8bpc RGB. In libyuv, I410 means 10 bit 444 YUV, I210 means 10 bit 422 YUV, and I010 means 10 bit 420 YUV. I010ToARGBMatrix() and I210ToARGBMatrix() were added (exposed) in libyuv version 1755, which is the minimum libyuv version required by libavif: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2202748 I410ToARGBMatrix() was added in libyuv version 1780: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2707003 Fix AOMediaCodec#822.
@joedrago @tongyuantongyu Please review. Thanks. |
@@ -221,7 +249,8 @@ avifResult avifImageYUVToRGBLibYUV(const avifImage * image, avifRGBImage * rgb) | |||
return AVIF_RESULT_REFORMAT_FAILED; | |||
} | |||
return AVIF_RESULT_OK; | |||
} else if (image->yuvFormat == AVIF_PIXEL_FORMAT_YUV422) { | |||
} | |||
if (image->yuvFormat == AVIF_PIXEL_FORMAT_YUV422) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote these else-if statements as if statements, so that it will be easier to nest with #if LIBYUV_VERSION >= 1780
.
(const uint16_t *)image->yuvPlanes[AVIF_CHAN_U], | ||
image->yuvRowBytes[AVIF_CHAN_U] / 2, | ||
(const uint16_t *)image->yuvPlanes[AVIF_CHAN_V], | ||
image->yuvRowBytes[AVIF_CHAN_V] / 2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tongyuantongyu Please confirm that we must divide image->yuvPlanes[AVIF_CHAN_Y]
by 2 to get the src_stride_y
value for libyuv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I confirm it. Libyuv treats stride as ptrdiff_t and add it to uint16_t* pointer directly.
#endif | ||
} | ||
if (image->yuvFormat == AVIF_PIXEL_FORMAT_YUV400) { | ||
// This doesn't currently exist in libyuv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that unlike the above cases, I did not add an #if 0
block here with some code, because I don't know what 10 bit 400 YUV (grey) will be called in libyuv.
* Utilize bilinear chroma upsample in libyuv when possible * Add more info to comment about -Wnewline-eof * Add a comment about -Wnewline-eof * Edit comment Co-authored-by: Wan-Teh Chang <wtc@google.com>
I recreated this pull request based on the current source tree as #902. |
Call the libyuv functions that convert 10bpc YUV to 8bpc RGB.
In libyuv, I410 means 10 bit 444 YUV, I210 means 10 bit 422 YUV, and
I010 means 10 bit 420 YUV.
I010ToARGBMatrix() and I210ToARGBMatrix() were added (exposed) in libyuv
version 1755, which is the minimum libyuv version required by libavif:
https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2202748
I410ToARGBMatrix() was added in libyuv version 1780:
https://chromium-review.googlesource.com/c/libyuv/libyuv/+/2707003
Fix #822.