[XS] Change in coreboot[main]: bootsplash: Increase heap from 1 MB to 4 MB when bootsplash is enabled

[Nigel Tao (Code Review)]

Attention is currently required from: Felix Singer, Jonathon Hall, Martin L Roth, Nico Huber, Paul Menzel.

Nigel Tao has posted comments on this change by Jonathon Hall. ( https://review.coreboot.org/c/coreboot/+/83476?usp=email )

Change subject: bootsplash: Increase heap from 1 MB to 4 MB when bootsplash is enabled
......................................................................

Patch Set 2:

(2 comments)

Patchset:

PS2:
> I guess I was thinking about the resolution (file(1) should be able to print it?). […]

The Wuffs calculation goes like this:

Usually (but not always), JPEG uses 4:2:0 chroma subsampling (https://en.wikipedia.org/wiki/Chroma_subsampling). For a 1024x768 JPEG, this means that there are:

- 1024x768 Y (luma) samples
- 512x384 Cb (chroma-blue) samples
- 512x384 Cr (chroma-red) samples

Each sample is one byte, hence ((1024x768) + (512x384) + (512x384)) = 1179648 bytes of "work buffer" memory is needed.

That's with Wuffs `0.4.0-alpha.2`. As I said in the other comment, if you upgrade to `0.4.0-alpha.6` then the required work buffer size can drop to zero and, IIUC, you won't need this Kconfig change, since `src/lib/jpeg.c` won't need to malloc or free any more.

PS2:
> But if overestimating memory requirements (by a few MB) is problematic for coreboot, I can do some thinking about a leaner (but more complicated) API.

OK, I have a leaner (and not actually much more complicated) API.

Coreboot's copy of Wuffs (`src/vendorcode/wuffs/wuffs-v0.4.c`) is currently at `0.4.0-alpha.2`. If you upgrade (copy `release/c/wuffs-v0.4.c` from https://github.com/google/wuffs) to the freshly git-pushed `0.4.0-alpha.6` then you can now opt in to a lower-quality JPEG decoding (specifically: box filtering instead of triangle filtering when chroma upsampling) which should drop the minimum Wuffs work area requirement (its "work buffer" length) for sequential (not progressive) JPEGs to zero: no heap allocation necessary anymore. I assume Coreboot can simply decide to not support progressive JPEGs in bootsplash.

If you upgrade, try this `src/lib/jpeg.c` patch (but I don't know how to test it myself):

```
diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 242cf0ca..2fe7ffdb 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -1,9 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

/*
- * Provide a simple API around the Wuffs JPEG decoder
- * Uses the heap (and lots of it) for the image-size specific
- * work buffer, so ramstage-only.
+ * Provide a simple API around the Wuffs JPEG decoder.
  */

#include <stdint.h>
@@ -84,6 +82,11 @@ int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic,
        if (status.repr) {
                return JPEG_DECODE_FAILED;
        }
+       status = wuffs_jpeg__decoder__set_quirk(&dec, WUFFS_BASE__QUIRK_QUALITY,
+               WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY);
+       if (status.repr) {
+               return JPEG_DECODE_FAILED;
+       }

wuffs_base__image_config imgcfg;
        wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true);
@@ -104,19 +107,16 @@ int jpeg_decode(unsigned char *filedata, size_t filesize, unsigned char *pic,
                return JPEG_DECODE_FAILED;
        }

-       uint64_t workbuf_len_min_incl = wuffs_jpeg__decoder__workbuf_len(&dec).min_incl;
-       uint8_t *workbuf_array = malloc(workbuf_len_min_incl);
-       if ((workbuf_array == NULL) && workbuf_len_min_incl) {
+       if (wuffs_jpeg__decoder__workbuf_len(&dec).min_incl > 0) {
+               // Setting WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY means that
+               // we can get a zero workbuf_len for common (sequential) JPEGs. No
+               // additional memory required. We choose not to support rarer
+               // (progressive) JPEGs.
                return JPEG_DECODE_FAILED;
        }

-       wuffs_base__slice_u8 workbuf =
-               wuffs_base__make_slice_u8(workbuf_array, workbuf_len_min_incl);
        status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src,
-                                                  WUFFS_BASE__PIXEL_BLEND__SRC, workbuf, NULL);
-
-       free(workbuf_array);
-
+               WUFFS_BASE__PIXEL_BLEND__SRC, wuffs_base__empty_slice_u8(), NULL);
        if (status.repr) {
                return JPEG_DECODE_FAILED;
        }
```