Improve z_jpeg documentation (#825)

* Improve z_jpeg documentation

* JpegWork::data is probably better than address

* Rename Jpeg_SendTask to Jpeg_ScheduleDecoderTask

* Rename njpegUCode to gJpegUCode

* Rename GetU16 to GetUnalignedU16 and improve docs

asm/non_matchings/code/z_jpeg/Jpeg_Decode.s

@@ -402,7 +402,7 @@ glabel Jpeg_Decode
 /* AE5ABC 8006E91C 10000016 */  b     .L8006E978
 /* AE5AC0 8006E920 26B50004 */   addiu $s5, $s5, 4
 .L8006E924:
-/* AE5AC4 8006E924 0C01B78C */  jal   Jpeg_SendTask
+/* AE5AC4 8006E924 0C01B78C */  jal   Jpeg_ScheduleDecoderTask
 /* AE5AC8 8006E928 27A40208 */   addiu $a0, $sp, 0x208
 /* AE5ACC 8006E92C 8FA40040 */  lw    $a0, 0x40($sp)
 /* AE5AD0 8006E930 0C001880 */  jal   osInvalDCache

data/rsp.rodata.s

@@ -33,5 +33,5 @@ glabel D_80157580
 glabel D_801579A0
     .incbin "baserom.z64", 0xBCEB40, 0x390
 
-glabel D_80157D30
+glabel gJpegUCodeData
     .incbin "baserom.z64", 0xBCEED0, 0x60

data/rsp.text.s

@@ -15,5 +15,5 @@ glabel D_801120C0
 glabel D_80113070
     .incbin "baserom.z64", 0xB8A210, 0x18C0
 
-glabel D_80114930
+glabel gJpegUCode
     .incbin "baserom.z64", 0xB8BAD0, 0xAF0

include/functions.h

@@ -857,7 +857,7 @@ void func_8006D0EC(GlobalContext* globalCtx, Player* player);
 void func_8006D684(GlobalContext* globalCtx, Player* player);
 void func_8006DC68(GlobalContext* globalCtx, Player* player);
 void func_8006DD9C(Actor* actor, Vec3f* arg1, s16 arg2);
-u32 Jpeg_SendTask(JpegContext* ctx);
+u32 Jpeg_ScheduleDecoderTask(JpegContext* ctx);
 void Jpeg_CopyToZbuffer(u16* src, u16* zbuffer, s32 x, s32 y);
 u16 Jpeg_GetU16(u8* ptr);
 void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx);

include/variables.h

@@ -106,7 +106,7 @@ extern DmaEntry gDmaDataTable[0x60C];
 //extern ? D_800D7288;
 extern u8 D_801120C0[];
 extern u8 D_80113070[];
-extern u64 D_80114930[];
+extern u64 gJpegUCode[];
 //extern ? D_80115440;
 //extern ? D_80115760;
 //extern ? D_80115770;
@@ -3034,7 +3034,7 @@ extern s16 D_80153960[];
 extern u8 D_80155F50[];
 extern u8 D_80157580[];
 extern u8 D_801579A0[];
-extern u64 D_80157D30[];
+extern u64 gJpegUCodeData[];
 //extern ? D_8015BC30;
 //extern ? D_8015BC3C;
 //extern ? D_8015BC48;

include/z64.h

@@ -1654,7 +1654,7 @@ typedef struct {
     /* 0x320 */ JpegQuantizationTable qTableV;
     /* 0x3A0 */ u8 codesLengths[0x110];
     /* 0x4B0 */ u16 codes[0x108];
-    /* 0x6C0 */ u16 unk_6C0[4][0x180];
+    /* 0x6C0 */ u16 data[4][0x180];
 } JpegWork; // size = 0x12C0
 
 typedef struct {

src/code/z_jpeg.c

@@ -1,6 +1,7 @@
 #include "global.h"
 #include "vt.h"
 
+#define MARKER_ESCAPE 0x00
 #define MARKER_SOI 0xD8
 #define MARKER_SOF 0xC0
 #define MARKER_DHT 0xC4
@@ -13,32 +14,33 @@
 #define MARKER_COM 0xFE
 #define MARKER_EOI 0xD9
 
-u32 Jpeg_SendTask(JpegContext* ctx) {
+/**
-    // clang-format off
+ * Configures and schedules a JPEG decoder task and waits for it to finish.
+ */
+u32 Jpeg_ScheduleDecoderTask(JpegContext* ctx) {
     static OSTask_t sJpegTask = {
         M_NJPEGTASK,          // type
         0,                    // flags
         NULL,                 // ucode_boot
         0,                    // ucode_boot_size
-        D_80114930, // ucode
+        gJpegUCode,           // ucode
         0x1000,               // ucode_size
-        D_80157D30, // ucode_data
+        gJpegUCodeData,       // ucode_data
         0x800,                // ucode_data_size
         NULL,                 // dram_stack
         0,                    // dram_stack_size
         NULL,                 // output_buff
-        0, // output_buff_size
+        NULL,                 // output_buff_size
         NULL,                 // data_ptr
         sizeof(JpegTaskData), // data_size
         NULL,                 // yield_data_ptr
         0x200,                // yield_data_size
     };
-    // clang-format on
 
     JpegWork* workBuf = ctx->workBuf;
     s32 pad[2];
 
-    workBuf->taskData.address = PHYSICAL_TO_VIRTUAL(&workBuf->unk_6C0);
+    workBuf->taskData.address = PHYSICAL_TO_VIRTUAL(&workBuf->data);
     workBuf->taskData.mode = ctx->mode;
     workBuf->taskData.mbCount = 4;
     workBuf->taskData.qTableYPtr = PHYSICAL_TO_VIRTUAL(&workBuf->qTableY);
@@ -63,6 +65,9 @@ u32 Jpeg_SendTask(JpegContext* ctx) {
     osRecvMesg(&ctx->mq, NULL, OS_MESG_BLOCK);
 }
 
+/**
+ * Copies a 16x16 block of decoded image data to the Z-buffer.
+ */
 void Jpeg_CopyToZbuffer(u16* src, u16* zbuffer, s32 x, s32 y) {
     u16* dst = zbuffer + (((y * SCREEN_WIDTH) + x) * 16);
     s32 i;
@@ -90,14 +95,26 @@ void Jpeg_CopyToZbuffer(u16* src, u16* zbuffer, s32 x, s32 y) {
     }
 }
 
-u16 Jpeg_GetU16(u8* ptr) {
+/**
-    if (((u32)ptr & 1) == 0) { // if the address is aligned to 2
+ * Reads an u16 from a possibly unaligned address in memory.
+ *
+ * Replaces unaligned 16-bit reads with a pair of aligned reads, allowing for reading the possibly
+ * unaligned values in JPEG header files.
+ */
+u16 Jpeg_GetUnalignedU16(u8* ptr) {
+    if (((u32)ptr & 1) == 0) {
+        // Read the value normally if it's aligned to a 16-bit address.
         return *(u16*)ptr;
     } else {
-        return *(u16*)(ptr - 1) << 8 | (*(u16*)(ptr + 1) >> 8); // lhu crashes with unaligned addresses
+        // Read unaligned values using two separate aligned memory accesses when it's not.
+        return *(u16*)(ptr - 1) << 8 | (*(u16*)(ptr + 1) >> 8);
     }
 }
 
+/**
+ * Parses the markers in the JPEG file, storing information such as the pointer to the image data
+ * in `ctx` for later processing.
+ */
 void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx) {
     u32 exit = false;
 
@@ -109,47 +126,58 @@ void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx) {
             break;
         }
 
+        // 0xFF indicates the start of a JPEG marker, so look for the next.
         if (*ptr++ == 0xFF) {
             switch (*ptr++) {
-                case 0:
+                case MARKER_ESCAPE: {
+                    // Compressed value 0xFF is stored as 0xFF00 to escape it, so ignore it.
                     break;
+                }
                 case MARKER_SOI: {
+                    // Start of Image
                     osSyncPrintf("MARKER_SOI\n");
                     break;
                 }
                 case MARKER_APP0: {
-                    osSyncPrintf("MARKER_APP0 %d\n", Jpeg_GetU16(ptr));
+                    // Application marker for JFIF
-                    ptr += Jpeg_GetU16(ptr);
+                    osSyncPrintf("MARKER_APP0 %d\n", Jpeg_GetUnalignedU16(ptr));
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_APP1: {
-                    osSyncPrintf("MARKER_APP1 %d\n", Jpeg_GetU16(ptr));
+                    // Application marker for EXIF
-                    ptr += Jpeg_GetU16(ptr);
+                    osSyncPrintf("MARKER_APP1 %d\n", Jpeg_GetUnalignedU16(ptr));
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_APP2: {
-                    osSyncPrintf("MARKER_APP2 %d\n", Jpeg_GetU16(ptr));
+                    osSyncPrintf("MARKER_APP2 %d\n", Jpeg_GetUnalignedU16(ptr));
-                    ptr += Jpeg_GetU16(ptr);
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_DQT: {
-                    osSyncPrintf("MARKER_DQT %d %d %02x\n", ctx->dqtCount, Jpeg_GetU16(ptr), ptr[2]);
+                    // Define Quantization Table, stored for later processing
+                    osSyncPrintf("MARKER_DQT %d %d %02x\n", ctx->dqtCount, Jpeg_GetUnalignedU16(ptr), ptr[2]);
                     ctx->dqtPtr[ctx->dqtCount++] = ptr + 2;
-                    ptr += Jpeg_GetU16(ptr);
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_DHT: {
-                    osSyncPrintf("MARKER_DHT %d %d %02x\n", ctx->dhtCount, Jpeg_GetU16(ptr), ptr[2]);
+                    // Define Huffman Table, stored for later processing
+                    osSyncPrintf("MARKER_DHT %d %d %02x\n", ctx->dhtCount, Jpeg_GetUnalignedU16(ptr), ptr[2]);
                     ctx->dhtPtr[ctx->dhtCount++] = ptr + 2;
-                    ptr += Jpeg_GetU16(ptr);
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_DRI: {
-                    osSyncPrintf("MARKER_DRI %d\n", Jpeg_GetU16(ptr));
+                    // Define Restart Interval
-                    ptr += Jpeg_GetU16(ptr);
+                    osSyncPrintf("MARKER_DRI %d\n", Jpeg_GetUnalignedU16(ptr));
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_SOF: {
+                    // Start of Frame, stores important metadata of the image.
+                    // Only used for extracting the sampling factors (ctx->mode).
                     osSyncPrintf("MARKER_SOF   %d "
                                  "精度%02x " // accuracy
                                  "垂直%d "   // vertical
@@ -158,33 +186,35 @@ void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx) {
                                  "(1:Y)%d (H0=2,V0=1(422) or 2(420))%02x (量子化テーブル)%02x "
                                  "(2:Cb)%d (H1=1,V1=1)%02x (量子化テーブル)%02x "
                                  "(3:Cr)%d (H2=1,V2=1)%02x (量子化テーブル)%02x\n",
-                                 Jpeg_GetU16(ptr),
+                                 Jpeg_GetUnalignedU16(ptr),
                                  ptr[2],                        // precision
-                                 Jpeg_GetU16(ptr + 3),      // height
+                                 Jpeg_GetUnalignedU16(ptr + 3), // height
-                                 Jpeg_GetU16(ptr + 5),      // width
+                                 Jpeg_GetUnalignedU16(ptr + 5), // width
-                                 ptr[7],                    // component count
+                                 ptr[7],                        // component count (assumed to be 3)
-                                 ptr[8], ptr[9], ptr[10],   // Y
+                                 ptr[8], ptr[9], ptr[10],       // Y component
-                                 ptr[11], ptr[12], ptr[13], // Cb
+                                 ptr[11], ptr[12], ptr[13],     // Cb component
-                                 ptr[14], ptr[15], ptr[16]  // Cr
+                                 ptr[14], ptr[15], ptr[16]      // Cr component
                     );
 
-                    if (ptr[9] == 0x21) // component Y : V0 == 1
+                    if (ptr[9] == 0x21) {
-                    {
+                        // component Y : V0 == 1
                         ctx->mode = 0;
-                    } else if (ptr[9] == 0x22) // component Y : V0 == 2
+                    } else if (ptr[9] == 0x22) {
-                    {
+                        // component Y : V0 == 2
                         ctx->mode = 2;
                     }
-                    ptr += Jpeg_GetU16(ptr);
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
                 case MARKER_SOS: {
-                    osSyncPrintf("MARKER_SOS %d\n", Jpeg_GetU16(ptr));
+                    // Start of Scan marker, indicates the start of the image data.
-                    ptr += Jpeg_GetU16(ptr);
+                    osSyncPrintf("MARKER_SOS %d\n", Jpeg_GetUnalignedU16(ptr));
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     ctx->imageData = ptr;
                     break;
                 }
                 case MARKER_EOI: {
+                    // End of Image
                     osSyncPrintf("MARKER_EOI\n");
                     exit = true;
                     break;
@@ -192,7 +222,7 @@ void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx) {
                 default: {
                     // Unknown marker
                     osSyncPrintf("マーカー不明 %02x\n", ptr[-1]);
-                    ptr += Jpeg_GetU16(ptr);
+                    ptr += Jpeg_GetUnalignedU16(ptr);
                     break;
                 }
             }
@@ -201,7 +231,7 @@ void Jpeg_ParseMarkers(u8* ptr, JpegContext* ctx) {
 }
 
 #ifdef NON_MATCHING
-// the time diff isn't correct, workBuff->unk_6C0 is kept in a temp register instead of being stored in the stack and
+// the time diff isn't correct, workBuff->data is kept in a temp register instead of being stored in the stack and
 // regalloc differences
 s32 Jpeg_Decode(void* data, u16* zbuffer, JpegWork* workBuff, u32 workSize) {
     s32 y;
@@ -218,7 +248,7 @@ s32 Jpeg_Decode(void* data, u16* zbuffer, JpegWork* workBuff, u32 workSize) {
     OSTime time2;
 
     time = osGetTime();
-    // (?) I guess MB_SIZE=0x180, PROC_OF_MBS=5 which means unk_6C0 is not a part of JpegWork
+    // (?) I guess MB_SIZE=0x180, PROC_OF_MBS=5 which means data is not a part of JpegWork
     ASSERT(workSize >= sizeof(JpegWork), "worksize >= sizeof(JPEGWork) + MB_SIZE * (PROC_OF_MBS - 1)", "../z_jpeg.c",
            527);
 
@@ -312,16 +342,16 @@ s32 Jpeg_Decode(void* data, u16* zbuffer, JpegWork* workBuff, u32 workSize) {
     y = 0;
     x = 0;
     for (i = 0; i < 300; i += 4) {
-        if (JpegDecoder_Decode(&decoder, (u16*)workBuff->unk_6C0, 4, i != 0, &state)) {
+        if (JpegDecoder_Decode(&decoder, (u16*)workBuff->data, 4, i != 0, &state)) {
             osSyncPrintf(VT_FGCOL(RED));
             osSyncPrintf("Error : Can't decode jpeg\n");
             osSyncPrintf(VT_RST);
         } else {
-            Jpeg_SendTask(&ctx);
+            Jpeg_ScheduleDecoderTask(&ctx);
-            osInvalDCache(&workBuff->unk_6C0, sizeof(workBuff->unk_6C0[0]));
+            osInvalDCache(&workBuff->data, sizeof(workBuff->data[0]));
 
-            src = workBuff->unk_6C0;
+            src = workBuff->data;
-            for (j = 0; j < ARRAY_COUNT(workBuff->unk_6C0); j++) {
+            for (j = 0; j < ARRAY_COUNT(workBuff->data); j++) {
                 Jpeg_CopyToZbuffer(src[j], zbuffer, x, y);
                 x++;