Apply rotation and mirroring when decoding to PNG or JPEG.#3059
Apply rotation and mirroring when decoding to PNG or JPEG.#3059maryla-uc merged 3 commits intoAOMediaCodec:mainfrom
Conversation
| avifRWDataFree(&exif); | ||
| goto cleanup; | ||
| } | ||
| // Ignore errors: if the exif is invalid, we can consider it as equivalent to not having an orientation. |
There was a problem hiding this comment.
Maybe assert(result == AVIF_RESULT_INVALID_EXIF_PAYLOAD || result == AVIF_RESULT_NOT_IMPLEMENTED); to be safe
There was a problem hiding this comment.
Done, I added a check for those errors, and I return in other cases.
|
|
||
| if (rotation->angle == 0) { | ||
| const size_t bytesPerRow = (size_t)bytesPerPixel * srcImage->width; | ||
| // 0 degrees. Just copy the rows as is. |
There was a problem hiding this comment.
Nit: In this case the function could return right after *dstImage = *srcImage; (note that this branch is never reached when called from avifApplyTransforms() anyway)
There was a problem hiding this comment.
This would make the contract more complicated. The current contract says that dstImage owns the returned pixels. The RGBImage struct itself doesn't know whether it owns its pixels, so this would have to be communicated in some way. And in any case this path is indeed not used but just there for completeness.
| // May be less than image->rowBytes e.g. if image is a cropped view. | ||
| const size_t bytesPerRowToMove = (size_t)bytesPerPixel * image->width; | ||
| // Top-to-bottom | ||
| uint8_t * tempRow = (uint8_t *)avifAlloc(bytesPerRowToMove); |
There was a problem hiding this comment.
Nit: Not strictly necessary, is it faster and/or more convenient to use a temporary buffer? mirror=1 do it pixel by pixel anyway.
There was a problem hiding this comment.
Well it should be faster although I haven't benchmarked it.
| const uint8_t orientation = avifImageGetExifOrientationFromIrotImir(avif); | ||
| result = avifSetExifOrientation(&exif, orientation); | ||
| // We already rotated the pixels if necessary in avifApplyTransforms(), so we set the orientation to 1 (no rotation, no mirror). | ||
| result = avifSetExifOrientation(&exif, 1); |
There was a problem hiding this comment.
Maryla: This avifSetExifOrientation(&exif, 1) call was added to avifjpeg.c but was not added to avifpng.c. I can observe a difference when decoding the attached happy_dog.avif file to a JPEG output and a PNG output. The following patch (to illustrate the bug, not for check-in) fixes the difference:
diff --git a/apps/shared/avifpng.c b/apps/shared/avifpng.c
index 3e6b034f..826b4f2b 100644
--- a/apps/shared/avifpng.c
+++ b/apps/shared/avifpng.c
@@ -783,6 +783,7 @@ avifBool avifPNGWrite(const char * outputFilename, const avifImage * avif, uint3
fprintf(stderr, "Error writing PNG: Exif metadata is too big\n");
goto cleanup;
}
+ avifResult result = avifSetExifOrientation((avifRWData *)&avif->exif, 1);
png_set_eXIf_1(png, info, (png_uint_32)avif->exif.size, avif->exif.data);
}
if (avif->xmp.data && (avif->xmp.size > 0)) {
There was a problem hiding this comment.
Maryla: I filed issue #3070. Let's continue the discussion there.
3 participants
#2427