Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds SMAF/MMF parsing and MIDI conversion, Yamaha MAFM FM and ADPCM rendering, custom voice handling, runtime playback routing, error reporting, documentation, and MAFM-enabled builds across supported platforms. ChangesSMAF and MAFM support
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant wildmidi_lib
participant f_smaf
participant smaf2mid
participant MAFM
participant OutputBuffer
Client->>wildmidi_lib: Load MMMD data
wildmidi_lib->>f_smaf: _WM_ParseNewSmaf
f_smaf->>smaf2mid: _WM_smaf2midi
f_smaf->>MAFM: Detect custom voices and create synth
wildmidi_lib->>MAFM: Feed events and render PCM
MAFM->>OutputBuffer: Accumulate stereo samples
OutputBuffer-->>Client: Return audio output
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
Still WIP but wanted the coderabbit output :) |
psi29a
marked this pull request as ready for review
July 23, 2026 08:36
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
build.zig (1)
72-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exposing MAFM as an opt-out build option.
WILDMIDI_MAFMis hardcoded to1for every zig-package consumer with no way to disable the (still WIP) FM engine. Ab.option(bool, "mafm", "Enable Yamaha MA FM synthesis", true)would let consumers opt out while keeping the current default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build.zig` at line 72, Replace the hardcoded WILDMIDI_MAFM value in the build configuration with a boolean b.option named "mafm", described as enabling Yamaha MA FM synthesis and defaulting to true. Use that option’s value for WILDMIDI_MAFM so package consumers can disable the WIP FM engine while preserving the current default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/formats/SmafFileFormat.txt`:
- Around line 393-399: Update decode_all_handyphone and decode_handyphone so
Rhythm-typed channels are not added to perc_mask or rerouted to MIDI channel 9;
preserve their original channel and distinct programs, matching the documented
sample-corpus behavior.
- Line 21: Correct the spelling of “decyphered” to “deciphered” in the
descriptive sentence.
In `@src/mafm.c`:
- Around line 340-346: Update _WM_MAFM_HasCustomVoices to allocate the temporary
mafm_synth probe on the heap instead of placing it on the stack, preserving the
existing input validation and bank-count check. Handle allocation failure by
returning no custom voices, pass the allocated instance to mafm_build_bank, read
bank_count, free the instance before returning, and ensure every return path
releases allocated memory.
- Line 206: Update the PCM allocation in the surrounding decode flow before
_WM_MAFM_AdpcmDecodeAll to use calloc for 2*alen int16_t samples, allowing the
allocation to validate the total product and prevent overflow-based
under-allocation. Preserve the existing w->pcm ownership and failure handling.
- Around line 350-365: Thread the drum note through patch selection instead of
using the channel program: update mafm_select_patch to accept a note parameter
and pass that note to _WM_MAFM_DrumApprox when is_drum is true, while retaining
program-based selection for non-drum patches. Update the mafm_note_on call site
to provide the current note and adjust any affected declarations or callers.
In `@src/root.zig`:
- Around line 163-165: Replace the removed std.process.getEnvVarOwned call in
the FREEPATS_PATH setup with a Zig 0.16-compatible environment lookup, such as
std.c.getenv followed by an allocator-owned copy, while preserving the existing
error.SkipZigTest behavior when the variable is unavailable. Ensure the
resulting path remains allocator-managed for subsequent use.
In `@src/smaf2mid.c`:
- Around line 696-712: Remove the Rhythm-channel percussion reroute introduced
through perc_mask in decode_all_handyphone and decode_handyphone, preserving
each channel’s normal MIDI channel, program, pan, and volume handling. Keep the
documented behavior that Rhythm-typed channels may contain melodic voices; do
not route them to MIDI channel 9 or reinterpret program values as drum pitches.
- Around line 178-200: Guard the allocation-size multiplication in both
schedule_off and hp_push before calling realloc: validate that the computed
element count fits in size_t when multiplied by sizeof(struct pending_off) or
the corresponding HandyPhone event type, returning the existing allocation
failure path on overflow. Apply the same checked-growth behavior to ctx->offs
and e->ev, including their doubling logic, while preserving normal growth and
insertion/push behavior.
In `@src/wildmidi_lib.c`:
- Around line 2398-2409: Update the mix-buffer growth logic around
mdi->mix_buffer_size to validate new_size before multiplying by sizeof(int32_t);
reject values that would overflow the allocation size, report WM_ERR_MEM, unlock
mdi->lock, and return -1 before calling realloc. Preserve the existing growth
calculation and successful reallocation behavior for safe sizes.
---
Nitpick comments:
In `@build.zig`:
- Line 72: Replace the hardcoded WILDMIDI_MAFM value in the build configuration
with a boolean b.option named "mafm", described as enabling Yamaha MA FM
synthesis and defaulting to true. Use that option’s value for WILDMIDI_MAFM so
package consumers can disable the WIP FM engine while preserving the current
default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8423380-5f73-47f6-bd12-c40b5758faf2
📒 Files selected for processing (33)
CMakeLists.txtNOTICEamiga/Makefileamiga/Makefile.vbccandroid/jni/Android.mkbuild.zigdjgpp/Makefiledocs/formats/SmafFileFormat.txtinclude/config.h.cmakeinclude/f_smaf.hinclude/internal_midi.hinclude/mafm.hinclude/smaf2mid.hinclude/wm_error.hmacosx/Makefilemingw/Makefilemsvc/common.makos2/makefile.emxos2/makefile.watsrc/CMakeLists.txtsrc/f_smaf.csrc/internal_midi.csrc/mafm.csrc/mafm/ma_fm_core.csrc/mafm/ma_fm_core.hsrc/mafm/smaf_voice.csrc/mafm/smaf_voice.hsrc/mafm/yamaha_adpcm.csrc/mafm/yamaha_adpcm.hsrc/root.zigsrc/smaf2mid.csrc/wildmidi_lib.csrc/wm_error.c
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/SMAF_FM.md`:
- Line 3: Update the document status declaration in SMAF_FM.md to reflect that
implementation work exists, removing the “no implementation yet” wording while
preserving the documented M1–M4 completion state.
In `@win32wat/makefile`:
- Line 43: Complete the Win32 Watcom MAFM integration by adding ma_fm_core.obj,
smaf_voice.obj, and yamaha_adpcm.obj to the OBJ list alongside mafm.obj, and
define WILDMIDI_MAFM in CFLAGS_LIB so the guarded runtime paths in f_smaf.c and
wildmidi_lib.c are enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76da31c0-00c6-48e6-b541-b59b5db72b84
📒 Files selected for processing (5)
android/jni/Android.mkdocs/SMAF_FM.mdos2/makefile.watsrc/root.zigwin32wat/makefile
🚧 Files skipped from review as they are similar to previous changes (1)
- os2/makefile.wat
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/smaf2mid.c (1)
183-186: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCheck capacity before doubling it.
alloc * 2can wrap before the new product checks run, producing a smaller allocation followed by an out-of-bounds write. Guard the existing capacity before doubling, and compare usingsize_twithout narrowingSIZE_MAX / sizeof(...)touint32_t.Proposed fix
- uint32_t na = ctx->offs_alloc ? ctx->offs_alloc * 2 : 64; + uint32_t na; struct pending_off *n; - if (na > (uint32_t)(SIZE_MAX / sizeof(struct pending_off))) return -1; + if (ctx->offs_alloc > UINT32_MAX / 2u) return -1; + na = ctx->offs_alloc ? ctx->offs_alloc * 2 : 64; + if ((size_t)na > SIZE_MAX / sizeof(*n)) return -1;Apply the equivalent change in
hp_push.Also applies to: 468-471
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/smaf2mid.c` around lines 183 - 186, Update the capacity-growth logic in the pending-offset allocation path and the equivalent hp_push path: validate the existing capacity before doubling it, reject growth that would overflow, and perform the SIZE_MAX comparison in size_t without narrowing it to uint32_t. Preserve the realloc flow only after the doubled capacity is proven safe.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/smaf2mid.c`:
- Around line 183-186: Update the capacity-growth logic in the pending-offset
allocation path and the equivalent hp_push path: validate the existing capacity
before doubling it, reject growth that would overflow, and perform the SIZE_MAX
comparison in size_t without narrowing it to uint32_t. Preserve the realloc flow
only after the doubled capacity is proven safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e07af82-9f18-4922-ba8a-d6e37d0a0129
📒 Files selected for processing (6)
docs/SMAF_FM.mddocs/formats/SmafFileFormat.txtsrc/mafm.csrc/smaf2mid.csrc/wildmidi_lib.cwin32wat/makefile
🚧 Files skipped from review as they are similar to previous changes (4)
- win32wat/makefile
- docs/formats/SmafFileFormat.txt
- docs/SMAF_FM.md
- src/mafm.c
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
sezero
reviewed
Jul 24, 2026
Member
Author
|
@sezero I updated with your review, thanks :) |
Member
Author
|
I was very amazed by the quality. |
Closed
Member
Author
|
Is ready for review and eval :) I treated it like SF2 so that if people don't need it, then they compile without support and keep the binary small. |
Contributor
|
Can I push to your branch? |
Contributor
OK here is a clean-up patch: smaflib.patch (Not pushing, just in case.) (Also inlined below.) Detailsbuild clean-ups. guard mafm/*.c with config. fix build in c++ mode.
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4e6d794..6788b67 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -211,7 +211,7 @@ check_symbol_exists(sqrtf math.h HAVE_SQRTF)
cmake_pop_check_state()
# ######### General setup ##########
-INCLUDE_DIRECTORIES(BEFORE "${PROJECT_SOURCE_DIR}/include" "${PROJECT_BINARY_DIR}/include" "${PROJECT_SOURCE_DIR}/src/mafm")
+INCLUDE_DIRECTORIES(BEFORE "${PROJECT_SOURCE_DIR}/include" "${PROJECT_BINARY_DIR}/include")
IF (NOT HAVE_STDINT_H) # AND NOT HAVE_INTTYPES_H
INCLUDE_DIRECTORIES(BEFORE "${PROJECT_SOURCE_DIR}/include/stdint")
ENDIF()
diff --git a/amiga/Makefile b/amiga/Makefile
index e65f59f..380924a 100644
--- a/amiga/Makefile
+++ b/amiga/Makefile
@@ -21,7 +21,7 @@ AR=$(CROSS)-ar
RANLIB=$(CROSS)-ranlib
endif
-INCLUDE = -I. -I../include -I../src/mafm
+INCLUDE = -I. -I../include
CFLAGS = $(INCLUDE) -Wall -W -fno-common
ARFLAGS = cr
LD = $(CC)
diff --git a/amiga/Makefile.vbcc b/amiga/Makefile.vbcc
index ec5869d..912cf68 100644
--- a/amiga/Makefile.vbcc
+++ b/amiga/Makefile.vbcc
@@ -10,7 +10,7 @@ CC=vc
#MKLIB=join $(LIB_OBJ) as WildMidi.lib
MKLIB=cat $(LIB_OBJ) > WildMidi.lib
-INCLUDE = -I. -I../include -I../src/mafm
+INCLUDE = -I. -I../include
CFLAGS = -O1 -speed -c99
CFLAGS += $(INCLUDE)
LDFLAGS = -L. -lWildMidi
diff --git a/android/jni/Android.mk b/android/jni/Android.mk
index b78e31a..9784045 100644
--- a/android/jni/Android.mk
+++ b/android/jni/Android.mk
@@ -2,7 +2,7 @@ LOCAL_PATH := $(call my-dir)/../..
include $(CLEAR_VARS)
LOCAL_MODULE := WildMidi
-LOCAL_C_INCLUDES := $(LOCAL_PATH)/android/jni $(LOCAL_PATH)/include $(LOCAL_PATH)/src/mafm
+LOCAL_C_INCLUDES := $(LOCAL_PATH)/android/jni $(LOCAL_PATH)/include
LOCAL_ARM_MODE := arm
LOCAL_CFLAGS += -DWILDMIDI_BUILD
LOCAL_CFLAGS += -fvisibility=hidden -DSYM_VISIBILITY
diff --git a/build.zig b/build.zig
index a58c796..28d29e1 100644
--- a/build.zig
+++ b/build.zig
@@ -11,7 +11,6 @@ pub fn build(b: *std.Build) void {
});
lib_mod.addIncludePath(b.path("include"));
- lib_mod.addIncludePath(b.path("src/mafm"));
const lib = b.addLibrary(.{
.name = "wildmidi",
diff --git a/djgpp/Makefile b/djgpp/Makefile
index 09237ca..4bf7324 100644
--- a/djgpp/Makefile
+++ b/djgpp/Makefile
@@ -25,7 +25,7 @@ RANLIB=$(CROSS)-ranlib
endif
DXE3GEN=dxe3gen
-INCLUDE = -I. -I../include/stdint -I../include -I../src/mafm
+INCLUDE = -I. -I../include/stdint -I../include
CFLAGS = $(INCLUDE) -Wall -W -fno-common
ARFLAGS = cr
diff --git a/macosx/Makefile b/macosx/Makefile
index 6707f88..7a342c3 100644
--- a/macosx/Makefile
+++ b/macosx/Makefile
@@ -32,7 +32,7 @@ LIB_INSTALLNAME=$(LIBSHARED_MAJ)
PLAYER=wildmidi
PLAYER_STATIC=wildmidi-static
-INCLUDE=-I. -I../include -I../src/mafm
+INCLUDE=-I. -I../include
ARFLAGS=cr
CFLAGS=$(INCLUDE) -Wall -W -fno-common
LDFLAGS=
diff --git a/mingw/Makefile b/mingw/Makefile
index e38a38c..b6b1669 100644
--- a/mingw/Makefile
+++ b/mingw/Makefile
@@ -25,7 +25,7 @@ IMPLIB=lib$(LIBNAME).dll.a
PLAYER=wildmidi.exe
PLAYER_STATIC=wildmidi-static.exe
-INCLUDE=-I. -I../include -I../src/mafm
+INCLUDE=-I. -I../include
ARFLAGS=cr
CFLAGS=$(INCLUDE) -Wall -W -fno-common
LDFLAGS=-static-libgcc
diff --git a/msvc/Makefile.vs b/msvc/Makefile.vs
index f0d9ef5..751bc79 100644
--- a/msvc/Makefile.vs
+++ b/msvc/Makefile.vs
@@ -1,7 +1,7 @@
# Makefile for Win32 using Visual Studio 2010 / newer:
# nmake -f Makefile.vs
-INCLUDES = -I. -I..\include -I..\src\mafm
+INCLUDES = -I. -I..\include
CPPFLAGS = -DNDEBUG -D_CRT_SECURE_NO_WARNINGS
CFLAGS = -nologo /O2 /MD /W3
diff --git a/msvc/Makefile.vs6 b/msvc/Makefile.vs6
index 42f7ed3..478b321 100644
--- a/msvc/Makefile.vs6
+++ b/msvc/Makefile.vs6
@@ -1,7 +1,7 @@
# Makefile for Win32 using Visual Studio 6:
# nmake -f Makefile.vs6
-INCLUDES = -I. -I..\include -I..\include\stdint -I..\src\mafm
+INCLUDES = -I. -I..\include -I..\include\stdint
CPPFLAGS = -DNDEBUG
CFLAGS = -nologo /O2 /MD /W3 /G6
diff --git a/msvc/Makefile.vs8 b/msvc/Makefile.vs8
index bd40d36..d3f5869 100644
--- a/msvc/Makefile.vs8
+++ b/msvc/Makefile.vs8
@@ -1,7 +1,7 @@
# Makefile for Win32 using Visual Studio 2005/2008:
# nmake -f Makefile.vs8
-INCLUDES = -I. -I..\include -I..\include\stdint -I..\src\mafm
+INCLUDES = -I. -I..\include -I..\include\stdint
CPPFLAGS = -DNDEBUG -D_CRT_SECURE_NO_WARNINGS
CFLAGS = -nologo /O2 /MD /W3
diff --git a/os2/makefile.emx b/os2/makefile.emx
index 4888caa..e6462ea 100644
--- a/os2/makefile.emx
+++ b/os2/makefile.emx
@@ -17,7 +17,7 @@ EMXOMF=emxomf
endif
LD=$(CC)
-INCLUDES=-I. -I../include -I../src/mafm
+INCLUDES=-I. -I../include
CFLAGS = $(INCLUDES) -Wall -W -Zmt
LDFLAGS = -Zomf -Zmt
LDFLAGS_DLL = -Zdll -Zomf -Zmt
diff --git a/os2/makefile.wat b/os2/makefile.wat
index 5c8ef87..64dbcd1 100644
--- a/os2/makefile.wat
+++ b/os2/makefile.wat
@@ -36,7 +36,7 @@ CFLAGS_LIB+= $(DLLFLAGS)
BLD_TARGET=$(DLLNAME) $(PLAYER)
!endif
INCPATH=-I"$(%WATCOM)/h/os2" -I"$(%WATCOM)/h"
-INCLUDES=$(INCPATH) -I. -I"../include" -I"../src/mafm"
+INCLUDES=$(INCPATH) -I. -I"../include"
OBJ=wm_error.obj file_io.obj lock.obj wildmidi_lib.obj reverb.obj gus_pat.obj f_xmidi.obj f_mus.obj f_hmp.obj f_midi.obj f_hmi.obj f_smaf.obj mus2mid.obj xmi2mid.obj hmp2mid.obj hmi2mid.obj smaf2mid.obj internal_midi.obj patches.obj sample.obj sf2.obj mafm.obj ma_fm_core.obj smaf_voice.obj yamaha_adpcm.obj synth.obj opl3.obj
PLAYER_OBJ=wm_tty.obj msleep.obj getopt_long.obj out_none.obj out_wave.obj out_dart.obj wildmidi.obj
diff --git a/src/mafm.c b/src/mafm.c
index 2ba2972..cfd35f9 100644
--- a/src/mafm.c
+++ b/src/mafm.c
@@ -43,9 +43,9 @@ typedef char mafm_char20[20]; /* no empty source. */
#include "wildmidi_lib.h"
#include "internal_midi.h"
#include "mafm.h"
-#include "ma_fm_core.h"
-#include "smaf_voice.h"
-#include "yamaha_adpcm.h"
+#include "mafm/ma_fm_core.h"
+#include "mafm/smaf_voice.h"
+#include "mafm/yamaha_adpcm.h"
/* big-endian 32-bit read */
#define MAFM_BE32(p) (((uint32_t)(p)[0] << 24) | ((uint32_t)(p)[1] << 16) | \
diff --git a/src/mafm/ma_fm_core.c b/src/mafm/ma_fm_core.c
index d55eb95..789447a 100644
--- a/src/mafm/ma_fm_core.c
+++ b/src/mafm/ma_fm_core.c
@@ -15,6 +15,14 @@
* curve, not in the mantissa.
*/
+#include "config.h"
+
+#ifndef WILDMIDI_MAFM
+
+typedef char mafm_core20[20]; /* no empty src. */
+
+#else
+
#include "ma_fm_core.h"
#include <math.h>
@@ -424,3 +432,4 @@ float _WM_MAFM_VoiceTick(struct mafm_voice *v) {
return (float)(out * v->velocity * v->volume * 0.7);
}
+#endif /* WILDMIDI_MAFM */
diff --git a/src/mafm/smaf_voice.c b/src/mafm/smaf_voice.c
index aeb5622..636f8e5 100644
--- a/src/mafm/smaf_voice.c
+++ b/src/mafm/smaf_voice.c
@@ -15,6 +15,14 @@
* into the same struct.
*/
+#include "config.h"
+
+#ifndef WILDMIDI_MAFM
+
+typedef char smaf_voice20[20]; /* no empty src. */
+
+#else
+
#include "smaf_voice.h"
#include <string.h>
@@ -255,3 +263,4 @@ void _WM_MAFM_ParseVoiceExclusive(const uint8_t *p, uint32_t n,
/* unknown Yamaha sub-form: leave out->valid = 0 */
}
+#endif /* WILDMIDI_MAFM */
diff --git a/src/mafm/yamaha_adpcm.c b/src/mafm/yamaha_adpcm.c
index ffead6d..2589b85 100644
--- a/src/mafm/yamaha_adpcm.c
+++ b/src/mafm/yamaha_adpcm.c
@@ -15,6 +15,14 @@
* (high-nibble-first is the Yamaha-standard default).
*/
+#include "config.h"
+
+#ifndef WILDMIDI_MAFM
+
+typedef char yamaha_adpcm20[20]; /* no empty src. */
+
+#else
+
#include "yamaha_adpcm.h"
static int adpcm_clamp16(int v) {
@@ -69,3 +77,5 @@ uint32_t _WM_MAFM_AdpcmDecodeAll(const uint8_t *data, uint32_t n,
}
return w;
}
+
+#endif /* WILDMIDI_MAFM */
diff --git a/src/smaf2mid.c b/src/smaf2mid.c
index 644f23b..7d90280 100644
--- a/src/smaf2mid.c
+++ b/src/smaf2mid.c
@@ -28,6 +28,7 @@
* See docs/formats/SmafFileFormat.txt for the format description.
*/
+#define __STDC_LIMIT_MACROS
#include "config.h"
#include <stddef.h>
diff --git a/src/wildmidi_lib.c b/src/wildmidi_lib.c
index faebcd4..cb6204a 100644
--- a/src/wildmidi_lib.c
+++ b/src/wildmidi_lib.c
@@ -22,6 +22,7 @@
*/
#define _WILDMIDI_LIB_C
+#define __STDC_LIMIT_MACROS
#include "config.h"
diff --git a/win32wat/makefile b/win32wat/makefile
index a892ee1..2a33c63 100644
--- a/win32wat/makefile
+++ b/win32wat/makefile
@@ -38,7 +38,7 @@ CFLAGS_LIB+= $(DLLFLAGS)
BLD_TARGET=$(DLLNAME) $(PLAYER)
!endif
INCPATH=-I"$(%WATCOM)/h/nt" -I"$(%WATCOM)/h"
-INCLUDES=$(INCPATH) -I. -I"../include" -I"../src/mafm"
+INCLUDES=$(INCPATH) -I. -I"../include"
OBJ=wm_error.obj file_io.obj lock.obj wildmidi_lib.obj reverb.obj gus_pat.obj f_xmidi.obj f_mus.obj f_hmp.obj f_midi.obj f_hmi.obj f_smaf.obj mus2mid.obj xmi2mid.obj hmp2mid.obj hmi2mid.obj smaf2mid.obj internal_midi.obj patches.obj sample.obj sf2.obj mafm.obj ma_fm_core.obj smaf_voice.obj yamaha_adpcm.obj synth.obj opl3.obj
PLAYER_OBJ=wm_tty.obj msleep.obj getopt_long.obj out_none.obj out_wave.obj out_openal.obj out_win32mm.obj wildmidi.obj |
Contributor
|
@psi29a : If you allow me, I'll do some butchering: (i) Will push the |
Member
Author
Yeah, I'm "done" for now. :) |
Contributor
|
Done: all squashed and the |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/smaf2mid.c (1)
836-848: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
ntracks == 0produces a valid-but-empty MIDI reported as success.If no HandyPhone
Mtsqis found (e.g. every MTR* was malformed), the output is just tempo + EOT, so playback is silent with no diagnostic. Declining withWM_ERR_NOT_SMAF"(no score track)" would matchfind_sequence's behaviour for the Mobile path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/smaf2mid.c` around lines 836 - 848, The HandyPhone conversion path around decode_all_handyphone must reject ntracks == 0 instead of emitting an empty MIDI and succeeding. Before hp_emit, report WM_ERR_NOT_SMAF with the “no score track” diagnostic and jump to _end, while preserving the existing memory-error handling for negative results and normal emission for positive track counts.docs/SMAF_FM.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTitle still says “(Design)” though the status is now IMPLEMENTED. Consider dropping the suffix for consistency with line 3.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/SMAF_FM.md` at line 1, Update the document title heading to remove the “(Design)” suffix so it matches the IMPLEMENTED status shown on line 3.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.zig`:
- Around line 39-42: Update the Zig build configuration to add a MAFM
enable/disable option, conditionally include the MAFM source files and set
WILDMIDI_MAFM only when enabled, and preserve builds without MAFM for
conversion-only use. Use the existing build-option/configuration setup in
build.zig rather than unconditionally compiling these sources or defining the
macro.
In `@docs/SMAF_FM.md`:
- Around line 142-147: Update the byte-layout literal “[formatByte][fmt2]” in
the M1 ADPCM codec documentation to be enclosed in backticks, and apply the same
Markdown escaping to the matching pattern near the later referenced section.
Preserve the surrounding wording and formatting.
In `@src/smaf2mid.c`:
- Around line 88-92: Add a sticky out-of-memory flag to struct smaf_ctx, set it
whenever resize_dst() fails in write1 and the other write* helpers, and make
_WM_smaf2mid return WM_ERR_MEM before transferring the output buffer when the
flag is set. Ensure subsequent writes remain harmless while the conversion
reports failure instead of success.
---
Nitpick comments:
In `@docs/SMAF_FM.md`:
- Line 1: Update the document title heading to remove the “(Design)” suffix so
it matches the IMPLEMENTED status shown on line 3.
In `@src/smaf2mid.c`:
- Around line 836-848: The HandyPhone conversion path around
decode_all_handyphone must reject ntracks == 0 instead of emitting an empty MIDI
and succeeding. Before hp_emit, report WM_ERR_NOT_SMAF with the “no score track”
diagnostic and jump to _end, while preserving the existing memory-error handling
for negative results and normal emission for positive track counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36a60c9e-9cba-4a76-bb66-b1b59ff17fbb
📒 Files selected for processing (43)
CMakeLists.txtNOTICEamiga/Makefileamiga/Makefile.vbccamiga/config.handroid/jni/Android.mkandroid/jni/config.hbuild.zigdjgpp/Makefiledjgpp/config.hdocs/SMAF_FM.mddocs/formats/SmafFileFormat.txtinclude/config.h.cmakeinclude/f_smaf.hinclude/internal_midi.hinclude/mafm.hinclude/smaf2mid.hinclude/wm_error.hmacosx/Makefilemacosx/config.hmingw/Makefilemingw/config.hmsvc/common.makmsvc/config.hos2/config.hos2/makefile.emxos2/makefile.watsrc/CMakeLists.txtsrc/f_smaf.csrc/internal_midi.csrc/mafm.csrc/mafm/ma_fm_core.csrc/mafm/ma_fm_core.hsrc/mafm/smaf_voice.csrc/mafm/smaf_voice.hsrc/mafm/yamaha_adpcm.csrc/mafm/yamaha_adpcm.hsrc/root.zigsrc/smaf2mid.csrc/wildmidi_lib.csrc/wm_error.cwin32wat/config.hwin32wat/makefile
🚧 Files skipped from review as they are similar to previous changes (31)
- include/f_smaf.h
- mingw/config.h
- android/jni/config.h
- include/smaf2mid.h
- macosx/config.h
- djgpp/config.h
- NOTICE
- os2/makefile.wat
- os2/config.h
- amiga/config.h
- win32wat/config.h
- include/wm_error.h
- src/root.zig
- msvc/config.h
- include/mafm.h
- src/mafm/smaf_voice.h
- include/config.h.cmake
- android/jni/Android.mk
- src/f_smaf.c
- msvc/common.mak
- src/internal_midi.c
- src/CMakeLists.txt
- src/mafm/yamaha_adpcm.h
- src/wm_error.c
- include/internal_midi.h
- src/mafm/ma_fm_core.h
- src/mafm/yamaha_adpcm.c
- docs/formats/SmafFileFormat.txt
- src/mafm/smaf_voice.c
- src/mafm.c
- src/mafm/ma_fm_core.c
Contributor
|
Among the test files you provided abıove, three are rejected: Errors as printed by player: |
Contributor
|
The coderabbit reviews are annoying, even though some are legit. And, no, I haven't read or evaluated the latest ones here... |
Contributor
|
P.S.: Is the smaf branch still relevant? |
Member
Author
Not really, it was my attempt to write a parser from scratch that purely did midi conversion. But the MMF/SMAF have samples in them like trackers do. Sooooo.... I mean, if we just want to dump to midi it was nice, but this new one also does that. |
Member
Author
Yes, those are not supported because they are MA-7 files. Compressed / proprietary yamaha formart. I would really need to reverse engineer it. So I'll leave it for another PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/smaf2mid.c (1)
22-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHeader comment is stale. HandyPhone (
0x00) is now decoded too, and Mtsu/ATR data is consumed by the MAFM path rather than universally ignored.📝 Suggested wording
- * Converts Yamaha SMAF ("MMF") Mobile Standard score tracks into a Standard - * MIDI File. Only the score track SEQUENCE (Mtsq) is converted; embedded - * PCM/ADPCM audio (Mtsp/ATR) and custom FM voice banks (Mtsu) are ignored, so - * playback falls back to the General MIDI patch set. + * Converts Yamaha SMAF ("MMF") HandyPhone Standard (format 0x00) and Mobile + * Standard (0x01/0x02) score tracks into a Standard MIDI File. Only the score + * track SEQUENCE (Mtsq) is converted here; embedded PCM/ADPCM audio (Mtsp/ATR) + * and custom FM voice banks (Mtsu) are handled separately by the MAFM path + * (src/mafm.c) and otherwise fall back to the General MIDI patch set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/smaf2mid.c` around lines 22 - 29, Update the file header comment to reflect current behavior: mention that HandyPhone (0x00) is decoded, and remove the claim that Mtsu/ATR data is universally ignored, noting its consumption by the MAFM path instead. Keep the existing description of score-track conversion and the format documentation reference.src/wildmidi_lib.c (1)
2405-2534: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
WM_GetOutput_MAFMis structurally sound but is now the 4th near-duplicate of the mix-buffer growth/overflow-guard logic in this file.The event loop, tail-ring-out (
_WM_MAFM_ActiveVoices), render, clamp, and pack stages all correctly mirrorWM_GetOutput_SF2— no functional issues found. However, the buffer-growth block (lines 2425-2442) is now byte-for-byte identical to the same block inWM_GetOutput_Linear(963-969),WM_GetOutput_Gauss(1303-1309), andWM_GetOutput_SF2(2286-2292). Extracting a smallgrow_mix_buffer(struct _mdi *mdi, uint32_t needed)helper would collapse all 4 copies into one, so any future fix to this logic (like the overflow guard itself) only needs to land once.♻️ Suggested helper
static int grow_mix_buffer(struct _mdi *mdi, uint32_t size) { if ((size / 2) > mdi->mix_buffer_size) { uint32_t new_size = ((size / 2) <= (mdi->mix_buffer_size * 2)) ? mdi->mix_buffer_size + MEM_CHUNK : size / 2; int32_t *new_buf; if (new_size > (UINT32_MAX / sizeof(int32_t))) { _WM_GLOBAL_ERROR(WM_ERR_MEM, NULL, 0); return -1; } new_buf = (int32_t *) realloc(mdi->mix_buffer, new_size * sizeof(int32_t)); if (new_buf == NULL) { _WM_GLOBAL_ERROR(WM_ERR_MEM, NULL, errno); return -1; } mdi->mix_buffer = new_buf; mdi->mix_buffer_size = new_size; } return 0; }Each call site then becomes
if (grow_mix_buffer(mdi, size) < 0) { _WM_Unlock(&mdi->lock); return -1; }.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/wildmidi_lib.c` around lines 2405 - 2534, Extract the duplicated mix-buffer growth and overflow-check block into a shared grow_mix_buffer helper near the output routines. Update WM_GetOutput_Linear, WM_GetOutput_Gauss, WM_GetOutput_SF2, and WM_GetOutput_MAFM to call it, preserving their existing lock-unlock and error-return behavior when growth fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/smaf2mid.c`:
- Around line 174-182: Update the shared write_event helper to prevent unsigned
underflow when at_ms precedes ctx->last_event_ms: clamp the event time or delta
so backward timestamps produce no large delay. Preserve normal delta calculation
and last_event_ms updates for non-wrapped timestamps, covering both decoder
paths that call write_event.
- Around line 661-682: Update the MIDI channel mapping for melodic channels in
the note handling branch and the long-control path near line 613 so it skips
reserved MIDI channel 9, while keeping percussion explicitly routed to channel
9. Reuse one consistent mapping approach for both paths, preserving the existing
base-channel behavior and ensuring melodic channels never collide with the
percussion destination.
---
Nitpick comments:
In `@src/smaf2mid.c`:
- Around line 22-29: Update the file header comment to reflect current behavior:
mention that HandyPhone (0x00) is decoded, and remove the claim that Mtsu/ATR
data is universally ignored, noting its consumption by the MAFM path instead.
Keep the existing description of score-track conversion and the format
documentation reference.
In `@src/wildmidi_lib.c`:
- Around line 2405-2534: Extract the duplicated mix-buffer growth and
overflow-check block into a shared grow_mix_buffer helper near the output
routines. Update WM_GetOutput_Linear, WM_GetOutput_Gauss, WM_GetOutput_SF2, and
WM_GetOutput_MAFM to call it, preserving their existing lock-unlock and
error-return behavior when growth fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bfa80aa-e33c-4c41-ab06-a2a86cf80c2a
📒 Files selected for processing (43)
CMakeLists.txtNOTICEamiga/Makefileamiga/Makefile.vbccamiga/config.handroid/jni/Android.mkandroid/jni/config.hbuild.zigdjgpp/Makefiledjgpp/config.hdocs/SMAF_FM.mddocs/formats/SmafFileFormat.txtinclude/config.h.cmakeinclude/f_smaf.hinclude/internal_midi.hinclude/mafm.hinclude/smaf2mid.hinclude/wm_error.hmacosx/Makefilemacosx/config.hmingw/Makefilemingw/config.hmsvc/common.makmsvc/config.hos2/config.hos2/makefile.emxos2/makefile.watsrc/CMakeLists.txtsrc/f_smaf.csrc/internal_midi.csrc/mafm.csrc/mafm/ma_fm_core.csrc/mafm/ma_fm_core.hsrc/mafm/smaf_voice.csrc/mafm/smaf_voice.hsrc/mafm/yamaha_adpcm.csrc/mafm/yamaha_adpcm.hsrc/root.zigsrc/smaf2mid.csrc/wildmidi_lib.csrc/wm_error.cwin32wat/config.hwin32wat/makefile
🚧 Files skipped from review as they are similar to previous changes (35)
- djgpp/config.h
- macosx/config.h
- msvc/config.h
- include/f_smaf.h
- amiga/config.h
- os2/config.h
- android/jni/config.h
- win32wat/config.h
- mingw/config.h
- src/mafm/smaf_voice.h
- src/mafm/yamaha_adpcm.h
- os2/makefile.emx
- os2/makefile.wat
- NOTICE
- include/smaf2mid.h
- CMakeLists.txt
- src/root.zig
- src/CMakeLists.txt
- build.zig
- include/config.h.cmake
- msvc/common.mak
- include/mafm.h
- android/jni/Android.mk
- src/f_smaf.c
- src/wm_error.c
- amiga/Makefile.vbcc
- src/mafm/yamaha_adpcm.c
- src/internal_midi.c
- win32wat/makefile
- docs/SMAF_FM.md
- src/mafm/ma_fm_core.h
- src/mafm.c
- docs/formats/SmafFileFormat.txt
- src/mafm/smaf_voice.c
- src/mafm/ma_fm_core.c
sezero
approved these changes
Jul 25, 2026
sezero
left a comment
sezero
left a comment
Contributor
There was a problem hiding this comment.
This should be good. Should we squash?
Member
Author
I want the shuffle bits to land first. Then we can merge this if there are no conflicts. |
Member
Author
|
I'm going to rebase this on master, fix conflicts, tidy up changelog |
Decode the HandyPhone Standard score format (format_type 0x00, MA-1/MA-2) in smaf2mid.c, merging its multi-track streams to MIDI. Add an optional FM synthesis engine (WANT_MAFM) so SMAF files play with their real Yamaha voices instead of the GM approximation: port the FM core, voice-exclusive decoder, and ADPCM codec to C from the (Apache-2.0) akustikrausch/yamaha-smaf-player, wrapped in src/mafm.c and wired into the render loop like the SF2 path (per-mdi mafm_synth). Embedded ATR/Awa ADPCM samples play as scheduled drums alongside the FM voices. Also document the MFi (.mld) format and update SmafFileFormat.txt.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/smaf2mid.c (1)
133-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
seekdst()setsdst_ptrbefore guaranteeing capacity.
ctx->dst_ptr = ctx->dst + posis computed against the old buffer, thenresize_dst()reallocs (movingctx->dst) and recomputesposfrom the stale pointer difference — it happens to work today only because every call site passes aposalready withindstsize. Reordering (grow first, then set the pointer) makes it robust.♻️ Suggested reorder
static void seekdst(struct smaf_ctx *ctx, uint32_t pos) { - ctx->dst_ptr = ctx->dst + pos; while (ctx->dstsize < pos) if (resize_dst(ctx)) { ctx->oom = 1; return; } + ctx->dst_ptr = ctx->dst + pos; ctx->dstrem = ctx->dstsize - pos; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/smaf2mid.c` around lines 133 - 138, Update seekdst() to ensure the destination buffer has capacity for pos before assigning ctx->dst_ptr. Perform the resize loop first, preserve the OOM return behavior, then compute ctx->dst_ptr from the potentially reallocated ctx->dst and set ctx->dstrem.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/man/man3/WildMidi_OpenBuffer.3`:
- Line 17: Add MUS to the documented input-format list for WildMidi_OpenBuffer
in docs/man/man3/WildMidi_OpenBuffer.3 at lines 17-17, and make the same
documentation update for WildMidi_Open in docs/man/man3/WildMidi_Open.3 at lines
13-13.
---
Nitpick comments:
In `@src/smaf2mid.c`:
- Around line 133-138: Update seekdst() to ensure the destination buffer has
capacity for pos before assigning ctx->dst_ptr. Perform the resize loop first,
preserve the OOM return behavior, then compute ctx->dst_ptr from the potentially
reallocated ctx->dst and set ctx->dstrem.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 807da926-41fe-44a7-8638-d4323869b0f9
📒 Files selected for processing (49)
CMakeLists.txtNOTICEREADME.mdamiga/Makefileamiga/Makefile.vbccamiga/config.handroid/jni/Android.mkandroid/jni/config.hbuild.zigdjgpp/Makefiledjgpp/config.hdocs/SMAF_FM.mddocs/formats/SmafFileFormat.txtdocs/man/man1/wildmidi.1docs/man/man3/WildMidi_ConvertToMidi.3docs/man/man3/WildMidi_Open.3docs/man/man3/WildMidi_OpenBuffer.3include/config.h.cmakeinclude/f_smaf.hinclude/internal_midi.hinclude/mafm.hinclude/smaf2mid.hinclude/wm_error.hmacosx/Makefilemacosx/config.hmingw/Makefilemingw/config.hmsvc/common.makmsvc/config.hos2/config.hos2/makefile.emxos2/makefile.watsrc/CMakeLists.txtsrc/f_smaf.csrc/internal_midi.csrc/mafm.csrc/mafm/ma_fm_core.csrc/mafm/ma_fm_core.hsrc/mafm/smaf_voice.csrc/mafm/smaf_voice.hsrc/mafm/yamaha_adpcm.csrc/mafm/yamaha_adpcm.hsrc/player/playlist.csrc/root.zigsrc/smaf2mid.csrc/wildmidi_lib.csrc/wm_error.cwin32wat/config.hwin32wat/makefile
🚧 Files skipped from review as they are similar to previous changes (34)
- android/jni/config.h
- win32wat/config.h
- macosx/config.h
- src/root.zig
- os2/makefile.emx
- os2/config.h
- include/config.h.cmake
- msvc/config.h
- include/f_smaf.h
- include/smaf2mid.h
- djgpp/config.h
- CMakeLists.txt
- amiga/config.h
- os2/makefile.wat
- src/mafm/yamaha_adpcm.h
- NOTICE
- include/internal_midi.h
- src/mafm/smaf_voice.h
- mingw/config.h
- amiga/Makefile.vbcc
- src/wm_error.c
- build.zig
- msvc/common.mak
- src/CMakeLists.txt
- docs/formats/SmafFileFormat.txt
- src/mafm/ma_fm_core.h
- win32wat/makefile
- android/jni/Android.mk
- include/mafm.h
- src/mafm/smaf_voice.c
- docs/SMAF_FM.md
- src/mafm/ma_fm_core.c
- src/mafm.c
- src/mafm/yamaha_adpcm.c
Closed
Contributor
|
Do we need the old smaf branch or can we delete it? |
Contributor
Diffed smaf2mid between old smaf branch and master, they are practically the same but latter being improved. Deleted the old smaf branch. |
Member
Author
|
Looking into ma-7 now... going to be awhile, very little documentation online. The only player that supports it is Yamaha's official app. |
This was referenced Jul 27, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decode the HandyPhone Standard score format (format_type 0x00, MA-1/MA-2) in smaf2mid.c, merging its multi-track streams to MIDI.
Add an optional FM synthesis engine (WANT_MAFM) so SMAF files play with their real Yamaha voices instead of the GM approximation: port the FM core, voice-exclusive decoder, and ADPCM codec to C from the (Apache-2.0) akustikrausch/yamaha-smaf-player, wrapped in src/mafm.c and wired into the render loop like the SF2 path (per-mdi mafm_synth). Embedded ATR/Awa ADPCM samples play as scheduled drums alongside the FM voices.
Also document the MFi (.mld) format and update SmafFileFormat.txt.
SMAF_handyphone_test files.zip
SolidSoda.mp3 (older version)
SolidSoda.mp3 (latest version)