commit 0b7c20ef8d929492fc675693616d422e9c5f373c
author Wan-Teh Chang <wtc@google.com> Fri Dec 18 16:21:57 2020 -0800
committer Joe Drago <joedrago@gmail.com> Fri Dec 18 18:59:03 2020 -0800
tree c7d3e28e31fc8f43294ea2d98f6c81a31fb60880
parent 8adbb97955f74ab3206207b90a1c6184de6ba858

Simplify avifEncoderAddImageInternal conditionals

Simplify some conditionals in avifEncoderAddImageInternal().

Since we require all cellImages have the same width, height, and depth,
move the checks for invalid values of width, height, and depth outside
the for loop and perform those checks on just firstCell.

Check gridCount > 1 instead of whether gridColorID or gridAlphaID is
nonzero. gridCount > 1 is an equivalent condition and is used elsewhere
in the function.

In the single-image case, we only need to check if the alpha planes of
cellImages have fully opaque values if firstCell has an alpha plane. (If
firstCell does not have an alpha plane, then no cellImages have an alpha
plane.)

src/write.c

diff --git a/src/write.c b/src/write.c
index 6ac1b51..a8f3198 100644
--- a/src/write.c
+++ b/src/write.c
@@ -402,22 +402,26 @@
     }
 
     const avifImage * firstCell = cellImages[0];
+    if ((firstCell->depth != 8) && (firstCell->depth != 10) && (firstCell->depth != 12)) {
+        return AVIF_RESULT_UNSUPPORTED_DEPTH;
+    }
+
+    if (!firstCell->width || !firstCell->height) {
+        return AVIF_RESULT_NO_CONTENT;
+    }
+
     if ((cellCount > 1) && ((firstCell->width < 64) || (firstCell->height < 64))) {
         return AVIF_RESULT_INVALID_IMAGE_GRID;
     }
 
     for (uint32_t cellIndex = 0; cellIndex < cellCount; ++cellIndex) {
         const avifImage * cellImage = cellImages[cellIndex];
-        if ((cellImage->depth != 8) && (cellImage->depth != 10) && (cellImage->depth != 12)) {
-            return AVIF_RESULT_UNSUPPORTED_DEPTH;
-        }
-
         if ((cellImage->depth != firstCell->depth) || (cellImage->width != firstCell->width) ||
             (cellImage->height != firstCell->height) || (!!cellImage->alphaPlane != !!firstCell->alphaPlane)) {
             return AVIF_RESULT_INVALID_IMAGE_GRID;
         }
 
-        if (!cellImage->width || !cellImage->height || !cellImage->yuvPlanes[AVIF_CHAN_Y]) {
+        if (!cellImage->yuvPlanes[AVIF_CHAN_Y]) {
             return AVIF_RESULT_NO_CONTENT;
         }
 
@@ -476,15 +480,15 @@
             }
             item->codec->csOptions = encoder->csOptions;
 
-            if (gridColorID) {
+            if (cellCount > 1) {
                 item->dimgFromID = gridColorID;
-            } else if (!encoder->data->primaryItemID) {
+            } else {
                 encoder->data->primaryItemID = item->id;
             }
         }
 
         encoder->data->alphaPresent = (firstCell->alphaPlane != NULL);
-        if (addImageFlags & AVIF_ADD_IMAGE_FLAG_SINGLE) {
+        if (encoder->data->alphaPresent && (addImageFlags & AVIF_ADD_IMAGE_FLAG_SINGLE)) {
             // If encoding a single image in which the alpha plane exists but is entirely opaque,
             // simply skip writing an alpha AV1 payload entirely, as it'll be interpreted as opaque
             // and is less bytes.
@@ -526,7 +530,7 @@
                 item->codec->csOptions = encoder->csOptions;
                 item->alpha = AVIF_TRUE;
 
-                if (gridAlphaID) {
+                if (cellCount > 1) {
                     item->dimgFromID = gridAlphaID;
                 } else {
                     item->irefToID = encoder->data->primaryItemID;
@@ -584,15 +588,10 @@
     } else {
         // Another frame in an image sequence
 
-        if (encoder->data->alphaPresent) {
-            for (uint32_t cellIndex = 0; cellIndex < cellCount; ++cellIndex) {
-                const avifImage * cellImage = cellImages[cellIndex];
-                if (!cellImage->alphaPlane) {
-                    // If the first image in the sequence had an alpha plane (even if fully opaque), all
-                    // subsequence images must have alpha as well.
-                    return AVIF_RESULT_ENCODE_ALPHA_FAILED;
-                }
-            }
+        if (encoder->data->alphaPresent && !firstCell->alphaPlane) {
+            // If the first image in the sequence had an alpha plane (even if fully opaque), all
+            // subsequence images must have alpha as well.
+            return AVIF_RESULT_ENCODE_ALPHA_FAILED;
         }
     }