libchdr: decode CHD Zstandard hunks through rzstd as well

libchdr's Zstandard codec was reachable only through the reference
library. Everything gating it - the wrapper's include, the eight sites
in libchdr_chd.c that declare the codec data, register the two codecs
and select them for a hunk, the object in Makefile.common and the
griffin include - tested HAVE_ZSTD alone. A build configured with
HAVE_RZSTD instead therefore compiled no zstd codec into libchdr at
all, and chd_open() rejected any image using CHD_CODEC_ZSTD or
CHD_CODEC_CD_ZSTD as an unsupported format. That is not a decode that
comes out wrong somewhere deep in a hunk; the image does not open, so
it neither runs nor survives a playlist scan.

That combination is not unusual, it is the default. qb/config.params.sh
ships HAVE_CHD=yes, HAVE_RCHD=no, HAVE_ZSTD=no, HAVE_RZSTD=yes, so a
plain ./configure selects libchdr as the reader and rzstd as the only
Zstandard decoder present. pkg/android's Android.mk defines HAVE_CHD
and HAVE_RZSTD without HAVE_RCHD and lands in the same place. Both have
been unable to open a Zstandard image. Makefile.webos sets HAVE_RCHD=1
and was unaffected, as is anything else on the built-in reader.

rchd has had this since it landed, through rzstd. libchdr is what a
build gets without HAVE_RCHD, and both readers are configurable, so the
codec has to be reachable from either one.

Widen every gate to test both, matching the shape already used for
FLAC, and give the wrapper an rzstd path. rzstd wins when both are
configured: it is where the tree is going, and it keeps <zstd.h> - six
long long declarations under -std=c89 -pedantic - out of the
translation unit. No configuration in the tree currently sets both, so
that precedence changes nothing today.

The rzstd path decodes in one call rather than streaming. Both callers
hand over one whole frame and already know how large it expands to:
zstd_codec_decompress() gets a hunk, and cdzs_codec_decompress() splits
its blob on the stored base length and passes each part exactly. That
matters because rzstd_decode() treats bytes left over after a frame as
an error instead of ignoring them, so the lengths have to be exact, and
they are. Nothing is carried between hunks, so init and free have
nothing to do and the codec struct holds a placeholder member, an empty
struct not being C89.

With <zstd.h> gone from an HAVE_RZSTD build, one mixed declaration in
cdzs_codec_decompress() was all that stood between this file and the
standard the tree targets, so complen_base moves to the top of its
block. The assignment stays where it was, after the bounds check that
has to run before the value is read. That arm now builds clean under
-std=c89 -pedantic.

The comment in rzstd.h claiming no caller can be served without a
streaming interface described this file and was wrong about it: a hunk
is one whole frame however it is fetched. Corrected rather than left to
justify an absence it no longer explains.

Verified by round-tripping reference-compressed hunks - 8 CD frames,
mixed raw, RLE and match-heavy content - through both builds of the
wrapper, plain and cdzs. Output is byte-identical: plain digest
8576c74f8291f69d, cdzs digest 9ad1f68b3999d76c on both. Reuse of a
codec across hunks, truncated frames, oversized stored base lengths and
short blobs all behave the same. ASan, UBSan and LSan clean; TSan
clean; no leaks. In the default configuration libchdr_zstd.o resolves
rzstd_decode against encoding_rzstd.o and retains no ZSTD_ symbol, and
libchdr_chd.o still references all six codec entry points, so the table
registration is live rather than compiled out.

One deliberate difference in strictness. Asked to decode a frame into a
destlen smaller than its true content, the reference path stops when
the output buffer fills, sees output.pos == output.size and reports
success, silently truncating; rzstd rejects it. Valid images always
pass the true hunk size, so this cannot fire on one, but on a corrupt
image the rzstd build fails the hunk where the reference build returns
a partial one as good.

Makefile.common

@@ -2240,7 +2240,10 @@ else
       OBJ += $(LIBRETRO_COMM_DIR)/formats/7z/r7z_lzma.o
    endif
 
-   ifeq ($(HAVE_ZSTD), 1)
+   # The CHD Zstandard codec decodes through whichever backend is
+   # configured, rzstd first. Same shape as the FLAC test above: either
+   # one gives libchdr a decoder, so build the wrapper for both.
+   ifneq ($(filter 1,$(HAVE_ZSTD) $(HAVE_RZSTD)),)
       OBJ += $(LIBRETRO_COMM_DIR)/formats/libchdr/libchdr_zstd.o
    endif
 endif

griffin/griffin.c

@@ -1548,7 +1548,7 @@ DEPENDENCIES
 #define GRIFFIN_HAVE_R7Z_LZMA 1
 #include "../libretro-common/formats/7z/r7z_lzma.c"
 
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 #include "../libretro-common/formats/libchdr/libchdr_zstd.c"
 #endif
 #endif  /* !HAVE_RCHD */

libretro-common/formats/libchdr/libchdr_chd.c

@@ -61,7 +61,7 @@
 #include <libchdr/lzma.h>
 #endif
 
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 #include <libchdr/libchdr_zstd.h>
 #endif
 
@@ -237,7 +237,7 @@ struct _chd_file
 	flac_codec_data			flac_codec_data;		/* flac codec data */
 	cdfl_codec_data			cdfl_codec_data;		/* cdfl codec data */
 #endif
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 	zstd_codec_data			zstd_codec_data;		/* zstd codec data */
 	cdzs_codec_data			cdzs_codec_data;		/* cdzs codec data */
 #endif
@@ -500,7 +500,7 @@ static const codec_interface codec_interfaces[] =
 		NULL
 	},
 #endif
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 	/* V5 zstd compression */
 	{
 		CHD_CODEC_ZSTD,
@@ -1140,7 +1140,7 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par
 						break;
 
 					case CHD_CODEC_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 						codec = &newchd->zstd_codec_data;
 #endif
 						break;
@@ -1164,7 +1164,7 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par
 						break;
 
 					case CHD_CODEC_CD_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 						codec = &newchd->cdzs_codec_data;
 #endif
 						break;
@@ -1319,7 +1319,7 @@ CHD_EXPORT void chd_close(chd_file *chd)
 					break;
 
 				case CHD_CODEC_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 					codec = &chd->zstd_codec_data;
 #endif
 					break;
@@ -1343,7 +1343,7 @@ CHD_EXPORT void chd_close(chd_file *chd)
 					break;
 
 				case CHD_CODEC_CD_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 					codec = &chd->cdzs_codec_data;
 #endif
 					break;
@@ -2110,7 +2110,7 @@ static chd_error hunk_read_into_memory(chd_file *chd, uint32_t hunknum, uint8_t
 						break;
 
 					case CHD_CODEC_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 						codec = &chd->zstd_codec_data;
 #endif
 						break;
@@ -2134,7 +2134,7 @@ static chd_error hunk_read_into_memory(chd_file *chd, uint32_t hunknum, uint8_t
 						break;
 
 					case CHD_CODEC_CD_ZSTD:
-#ifdef HAVE_ZSTD
+#if defined(HAVE_ZSTD) || defined(HAVE_RZSTD)
 						codec = &chd->cdzs_codec_data;
 #endif
 						break;

libretro-common/formats/libchdr/libchdr_zstd.c

@@ -48,7 +48,12 @@
 #include <libchdr/huffman.h>
 #include <libchdr/libchdr_zlib.h>
 #include <libchdr/libchdr_zstd.h>
+
+#ifdef HAVE_RZSTD
+#include <encodings/rzstd.h>
+#else
 #include <zstd.h>
+#endif
 
 #include <retro_inline.h>
 #include <streams/file_stream.h>
@@ -65,6 +70,12 @@
 
 chd_error zstd_codec_init(void* codec, uint32_t hunkbytes)
 {
+#ifdef HAVE_RZSTD
+	/* Nothing to construct: rzstd holds no state across calls. */
+	(void)codec;
+	(void)hunkbytes;
+	return CHDERR_NONE;
+#else
 	zstd_codec_data* zstd_codec = (zstd_codec_data*) codec;
 
 	zstd_codec->dstream = ZSTD_createDStream();
@@ -73,6 +84,7 @@ chd_error zstd_codec_init(void* codec, uint32_t hunkbytes)
 		return CHDERR_DECOMPRESSION_ERROR;
 	}
 	return CHDERR_NONE;
+#endif
 }
 
 /*-------------------------------------------------
@@ -82,9 +94,13 @@ chd_error zstd_codec_init(void* codec, uint32_t hunkbytes)
 
 void zstd_codec_free(void* codec)
 {
+#ifdef HAVE_RZSTD
+	(void)codec;
+#else
 	zstd_codec_data* zstd_codec = (zstd_codec_data*) codec;
 
 	ZSTD_freeDStream(zstd_codec->dstream);
+#endif
 }
 
 /*-------------------------------------------------
@@ -94,6 +110,23 @@ void zstd_codec_free(void* codec)
  */
 chd_error zstd_codec_decompress(void* codec, const uint8_t *src, uint32_t complen, uint8_t *dest, uint32_t destlen)
 {
+#ifdef HAVE_RZSTD
+	/* Every caller hands over one whole frame and already knows how
+	 * large it expands to, so this is a single call. complen is the
+	 * exact frame length in both callers - cdzs splits the blob on the
+	 * stored base length - which matters because rzstd treats bytes
+	 * left over after the frame as an error rather than ignoring them. */
+	size_t wrote = 0;
+
+	(void)codec;
+
+	if (rzstd_decode(dest, destlen, src, complen, &wrote)
+			!= RZSTD_PROCESS_END)
+		return CHDERR_DECOMPRESSION_ERROR;
+	if (wrote != destlen)
+		return CHDERR_DECOMPRESSION_ERROR;
+	return CHDERR_NONE;
+#else
 	ZSTD_inBuffer input;
 	ZSTD_outBuffer output;
 
@@ -132,7 +165,7 @@ chd_error zstd_codec_decompress(void* codec, const uint8_t *src, uint32_t comple
 		return CHDERR_DECOMPRESSION_ERROR;
 	}
 	return CHDERR_NONE;
-
+#endif
 }
 
 /* cdzs */
@@ -177,6 +210,7 @@ chd_error cdzs_codec_decompress(void *codec, const uint8_t *src, uint32_t comple
 {
 	chd_error ret;
 	uint32_t framenum;
+	uint32_t complen_base;
 	cdzs_codec_data* cdzs = (cdzs_codec_data*)codec;
 
 	/* determine header bytes */
@@ -185,7 +219,6 @@ chd_error cdzs_codec_decompress(void *codec, const uint8_t *src, uint32_t comple
 	uint32_t ecc_bytes = (frames + 7) / 8;
 	uint32_t header_bytes = ecc_bytes + complen_bytes;
 
-
 	/* header_bytes and complen_base are both taken from the hunk, so
 	 * neither can be trusted before it is checked. Without this, a
 	 * complen_base larger than the hunk makes
@@ -197,7 +230,7 @@ chd_error cdzs_codec_decompress(void *codec, const uint8_t *src, uint32_t comple
 	if (complen < header_bytes)
 		return CHDERR_DECOMPRESSION_ERROR;
 	/* extract compressed length of base */
-	uint32_t complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1];
+	complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1];
 	if (complen_bytes > 2)
 		complen_base = (complen_base << 8) | src[ecc_bytes + 2];
 

libretro-common/include/encodings/rzstd.h

@@ -62,11 +62,12 @@
  *                             recognise one.
  *   formats/chd/rchd.c        decompresses a hunk, output size known
  *                             in advance.
- *   formats/libchdr/          decompresses a hunk through a streaming
- *     libchdr_zstd.c          interface, re-initialising per hunk. It
- *                             is the only caller that streams and it
- *                             goes away with libchdr, which is why no
- *                             streaming interface is offered here.
+ *   formats/libchdr/          decompresses a hunk, output size known in
+ *     libchdr_zstd.c          advance, same as rchd. It reaches the
+ *                             reference library through a streaming
+ *                             interface when built against that, but a
+ *                             hunk is one whole frame either way, so
+ *                             nothing here has to stream.
  *
  * None of them uses a dictionary.
  */
@@ -178,12 +179,12 @@ int rzstd_encode(uint8_t *dst, size_t dst_len,
 
 /* No streaming interface is offered.
  *
- * One was declared here, both directions, for a caller that cannot
- * hold a whole frame. The only such caller in this tree is
- * formats/libchdr, which formats/chd/rchd.c replaces and which holds
- * a hunk entire. Eleven entry points were declared and none was ever
- * implemented: nothing would have noticed them breaking, because
- * nothing called them. Absent is honest where declared was not.
+ * One was declared here, both directions, for a caller that cannot hold
+ * a whole frame. No caller in this tree is one: both CHD readers hold a
+ * hunk entire, and every other user above starts from a complete frame.
+ * Eleven entry points were declared and none was ever implemented:
+ * nothing would have noticed them breaking, because nothing called
+ * them. Absent is honest where declared was not.
  */
 
 /**

libretro-common/include/libchdr/libchdr_zstd.h

@@ -15,14 +15,27 @@
 
 #include <stdint.h>
 
+/* Either backend serves. rzstd wins when both are configured: it is the
+ * one the rest of the tree is moving to, and it keeps <zstd.h> - which
+ * declares long long - out of this translation unit. */
+#ifndef HAVE_RZSTD
 #include <zstd.h>
+#endif
+
 #include "coretypes.h"
 #include "chd.h"
 
 typedef struct _zstd_codec_data zstd_codec_data;
 struct _zstd_codec_data
 {
+#ifdef HAVE_RZSTD
+	/* rzstd decodes a hunk in one call and carries nothing between
+	 * hunks, so there is no decoder to hold on to. The member exists
+	 * only because an empty struct is not C89. */
+	uint8_t unused;
+#else
 	ZSTD_DStream *dstream;
+#endif
 };
 
 typedef struct _cdzs_codec_data cdzs_codec_data;