Skip to content

Commit

Permalink
Fail on reformat Identity with subsampling, make avifenc correct this…
Browse files Browse the repository at this point in the history
… before reformat (#517)

According to [AV1 Spec](https://aomediacodec.github.io/av1-spec/#color-config-semantics), Identity is only valid with YUV444:
> 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.

But `avifImageRGBToYUV` still tries to do the reformat. avifenc tries to correct this, but after reformat, which will produce an valid image but with wrong color.

This PR lets `avifImageRGBToYUV` failed on `image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444` so that libavif won't produce corrupted file. This PR also makes avifenc correct MC before reformat so that the output image has correct color.

I'm still thinking should we change MC or yuvFormat in this case? If user wants lossless, then maybe we should change yuvFormat instead, which makes output lossless as requested.

By the way, if we try to make an Identity monochrome image (forbid by spec), libavif, Chrome and Firefox all wouldn't reject decoding it. However libavif and Firefox will copy the Y(G) value to U(B) and V(R), but Chrome will fill U(B) and V(R) with 128.
  • Loading branch information
tongyuantongyu committed Feb 27, 2021
1 parent bc9730f commit 4f40299
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
31 changes: 19 additions & 12 deletions apps/avifenc.c
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,18 @@ int main(int argc, char * argv[])
image->yuvRange = requestedRange;
image->alphaPremultiplied = premultiplyAlpha;

if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (input.requestedFormat != AVIF_PIXEL_FORMAT_YUV444)) {
// matrixCoefficients was likely set to AVIF_MATRIX_COEFFICIENTS_IDENTITY as a side effect
// of --lossless, and Identity is only valid with YUV444. Set this back to the default.
image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_BT601;

if (cicpExplicitlySet) {
// Only warn if someone explicitly asked for identity.
printf("WARNING: matrixCoefficients may not be set to identity (0) when subsampling. Resetting MC to defaults (%d).\n",
image->matrixCoefficients);
}
}

avifInputFile * firstFile = avifInputGetNextFile(&input);
uint32_t sourceDepth = 0;
avifAppSourceTiming firstSourceTiming;
Expand All @@ -739,6 +751,13 @@ int main(int argc, char * argv[])
}
avifBool sourceWasRGB = (inputFormat != AVIF_APP_FILE_FORMAT_Y4M);

// 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)) {
fprintf(stderr, "matrixCoefficients may not be set to identity (0) when subsampling.\n");
returnCode = 1;
goto cleanup;
}

printf("Successfully loaded: %s\n", firstFile->filename);

// Prepare image timings
Expand All @@ -756,18 +775,6 @@ int main(int argc, char * argv[])
}
}

if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444)) {
// matrixCoefficients was likely set to AVIF_MATRIX_COEFFICIENTS_IDENTITY as a side effect
// of --lossless, and Identity is only valid with YUV444. Set this back to the default.
image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_BT601;

if (cicpExplicitlySet) {
// Only warn if someone explicitly asked for identity.
printf("WARNING: matrixCoefficients may not be set to identity (0) when subsampling. Resetting MC to defaults (%d).\n",
image->matrixCoefficients);
}
}

if (ignoreICC) {
avifImageSetProfileICC(image, NULL, 0);
}
Expand Down
5 changes: 5 additions & 0 deletions src/reformat.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,14 @@ static avifBool avifPrepareReformatState(const avifImage * image, const avifRGBI
return AVIF_FALSE;
}

if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444)) {
return AVIF_FALSE;
}

if (image->yuvFormat == AVIF_PIXEL_FORMAT_NONE) {
return AVIF_FALSE;
}

avifGetPixelFormatInfo(image->yuvFormat, &state->formatInfo);
avifCalcYUVCoefficients(image, &state->kr, &state->kg, &state->kb);
state->mode = AVIF_REFORMAT_MODE_YUV_COEFFICIENTS;
Expand Down

0 comments on commit 4f40299

Please sign in to comment.