-
Notifications
You must be signed in to change notification settings - Fork 201
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
Don't use Identity matrix coefficients with 4:0:0 #1482
Don't use Identity matrix coefficients with 4:0:0 #1482
Conversation
2fe5ab8
to
5c5c485
Compare
Thx for the explanation in #1460 (comment) ! |
Chrome doesn't render 4:0:0 with Identity matrix coefficients correctly. Work around this Chrome issue by using matrix coefficients 6 instead with lossless 4:0:0. Fix AOMediaCodec#1481.
5c5c485
to
b810521
Compare
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.
Please review the two commits together rather than separately.
Vincent approved the first commit in this pull request before it was ready for review.
I tested this pull request with a grayscale PNG input and a monochrome y4m input, and added the second commit.
(input.requestedFormat != AVIF_PIXEL_FORMAT_YUV444)) { | ||
// User explicitly asked for non YUV444 yuvFormat, while matrixCoefficients was likely | ||
// set to AVIF_MATRIX_COEFFICIENTS_IDENTITY as a side effect of --lossless, | ||
// and Identity is only valid with YUV444. Set matrixCoefficients back to the default. |
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.
Only this change and the new if statement at line 1680 are needed to work around the Chrome issue.
@@ -1663,8 +1661,7 @@ int main(int argc, char * argv[]) | |||
} | |||
|
|||
// Check again for y4m input (y4m input ignores input.requestedFormat and retains the format in file). | |||
if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) && | |||
(image->yuvFormat != AVIF_PIXEL_FORMAT_YUV400)) { | |||
if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444)) { |
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.
This change satisfies the following requirement in the AV1 spec (on page 119):
If matrix_coefficients is equal to MC_IDENTITY, it is a requirement of bitstream conformance that subsampling_x is equal to 0 and subsampling_y is equal to 0.
In AV1, subsampling_x == 0 && subsampling_y == 0
is equivalent to YUV 4:4:4.
if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) && | ||
(image->yuvFormat != AVIF_PIXEL_FORMAT_YUV400)) { | ||
// Check again for -y auto or for y4m input (y4m input ignores input.requestedFormat and | ||
// retains the format in file). |
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.
The original comment may have been written before -y auto
was supported. A y4m input is not the only reason image->yuvFormat
may be different from input.requestedFormat
now. So I updated this comment.
Chrome doesn't render 4:0:0 with Identity matrix coefficients correctly. Work around this Chrome issue by using matrix coefficients 6 instead with 4:0:0.
Note: On the decoder side, avifImageYUVToRGB() accepts 4:0:0 with Identity matrix coefficients.
Fix #1481.