SPC Import functionality #23

ChrisBlueStone · 2022-02-17T06:17:32Z

Added limited SPC import functionality.
Cleaned up blank template SPC.

* Added limited SPC import functionality. * Cleaned up blank template SPC.

* Refactored some BRR decoding logic. * Added code to verify the loop position aligns with a BRR block, which fixed a bug where the `decoding_start` position would be larger than it should be and corrupt memory in the `decode_brr_block` call.

* Added an out of bounds check when counting BRR blocks. * Added back a line that was accidentally removed.

Updated SPC loading to be able to load SPC files more generically and not be limited to SPCs from EarthBound.

Added a different music address searching algorithm that scans the patterns pointed to by $F0. If that fails it falls back to trying to use the music table if it found one.

Updated SPC exporting to scape out a portion of the program block and cram as many samples into the SPC as it can. This required moving the note lengths table as well and updating the program to point to the new location. Also note, exported SPCs can play but can't be reimported for some reason.

- Fixed exporting of the sample pointer table to include the loop addresses. Oops. - Changed the internal reference to the sample pointer table to use an offset instead of an actual address.

charasyn · 2022-02-23T17:27:26Z

src/main.c

+		const unsigned int NUM_INSTRUMENTS = 128;
+		const WORD dstSamplePointers = dstMusic + 0x100 + (size & 0xFF00);
+		const WORD dstInstruments = dstSamplePointers + 0x2*NUM_INSTRUMENTS;
+		const WORD inst_size = NUM_INSTRUMENTS*6;
+		const WORD dstSamples = dstInstruments + inst_size;


I think NUM_INSTRUMENTS should be 64. This matches with Earthbound's memory layout:

6C00-6DFF: Sample table: struct { WORD start; WORD loop; } [64] // Okay actually this doesn't match, only takes up 0x100 bytes - so you could maybe move it closer and get more space! 6E00-6F7F: Instrument table: byte [6*64] // 6*64=0x180 bytes, the exact size normally allocated 6F80-6F97: Misc data tables, 0x18 bytes

Notably, having 128 instruments takes up 0x300 bytes for the instrument table, which is 2x the normal space, which is what leads me to say that NUM_INSTRUMENTS should be 64. Have you seen games with more than 64 instruments?

Space Armada from StarFox uses 74 apparently unless something else is wrong somewhere. I've bumped it down to 80 and that seems to work.

Perhaps we should bump this back up just to be safe of other SPCs?

- Fixed a bug where the sample pointers weren't not being indexed into correctly. (`brr_copy[i]` instead of `brr_copy[2*i]`) - Fixed BRR block counting to return 0 if invalid. - Updated number of instruments to export to 80. - Updating logging information. - Added a 16 byte buffer between exported data.

Fixed crash when an edit is made to a loaded SPC but no rom pack is loaded.

Fixed crashes that occur when playing an SPC while opening a ROM.

Fixed a crash caused by the last commit where closing and opening a rom would crash the app

Fixed for a potential memory leak by freeing samples before decoding them when importing an SPC

Loading a ROM used to clear the playback state, but it stopped working with one of the previous commits. This change changes the functionality to assume `close_rom` means to close the the current ROM and/or any currently loaded SPCs and will correctly reset the playback state in either case to prevent crashes.

PhoenixBound

Kinda tired, but hopefully that's all the important bits to consider.

Pointer casts always make me nervous, but since the rest of ebmused uses them they can't be too awful I guess?

src/main.c

src/brr.c

src/main.c

- Added file read validation when importing SPCs. - Added safer parsing when searching for a music address. - Put constants in an enum. - Changed memset calls for single-byte changes. - Changed addressing while exporting an SPC file to use array indexing.

Fixed potential bug when using memcpy to copy to/from different locations of the same array.

- Fixed a value that changed since casting a pointer to a WORD* - Updated `maxPatterns` to prevent misaligned pointer arithmetic and round down.

SPC Import functionality

f4b2d72

* Added limited SPC import functionality. * Cleaned up blank template SPC.

ChrisBlueStone self-assigned this Feb 17, 2022

ChrisBlueStone added 6 commits February 17, 2022 22:19

Fixed BRR decode crash

cad1d1b

* Refactored some BRR decoding logic. * Added code to verify the loop position aligns with a BRR block, which fixed a bug where the `decoding_start` position would be larger than it should be and corrupt memory in the `decode_brr_block` call.

Small bugfix

ab47224

* Added an out of bounds check when counting BRR blocks. * Added back a line that was accidentally removed.

Updated SPC loading

50c3f5b

Updated SPC loading to be able to load SPC files more generically and not be limited to SPCs from EarthBound.

Added different music search algorithm

afbfa7b

Added a different music address searching algorithm that scans the patterns pointed to by $F0. If that fails it falls back to trying to use the music table if it found one.

Fixed sample pointer table exporting

99d2e7e

- Fixed exporting of the sample pointer table to include the loop addresses. Oops. - Changed the internal reference to the sample pointer table to use an offset instead of an actual address.

charasyn reviewed Feb 23, 2022

View reviewed changes

ChrisBlueStone added 6 commits February 23, 2022 19:54

Fix crash

5904102

Fixed crash when an edit is made to a loaded SPC but no rom pack is loaded.

Fixed crashes

5cb3ac3

Fixed crashes that occur when playing an SPC while opening a ROM.

Fixed crash when closing/opening rom

0dddca5

Fixed a crash caused by the last commit where closing and opening a rom would crash the app

Memory leak fix

33eacf5

Fixed for a potential memory leak by freeing samples before decoding them when importing an SPC

PhoenixBound reviewed Feb 26, 2022

View reviewed changes

src/main.c Outdated Show resolved Hide resolved

src/main.c Outdated Show resolved Hide resolved

src/main.c Show resolved Hide resolved

src/main.c Outdated Show resolved Hide resolved

src/brr.c Outdated Show resolved Hide resolved

src/main.c Outdated Show resolved Hide resolved

Code review changes

6213af9

- Added file read validation when importing SPCs. - Added safer parsing when searching for a music address. - Put constants in an enum. - Changed memset calls for single-byte changes. - Changed addressing while exporting an SPC file to use array indexing.

ChrisBlueStone requested a review from PhoenixBound February 26, 2022 22:30

ChrisBlueStone added 2 commits February 26, 2022 16:47

Fixed potential bug

508afc4

Fixed potential bug when using memcpy to copy to/from different locations of the same array.

Fixed pointer arithmetic

50b94d8

- Fixed a value that changed since casting a pointer to a WORD* - Updated `maxPatterns` to prevent misaligned pointer arithmetic and round down.

PhoenixBound approved these changes Feb 26, 2022

View reviewed changes

ChrisBlueStone merged commit 0b916c1 into PKHackers:master Mar 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPC Import functionality #23

SPC Import functionality #23

ChrisBlueStone commented Feb 17, 2022

charasyn Feb 23, 2022

ChrisBlueStone Feb 24, 2022

ChrisBlueStone Feb 26, 2022

PhoenixBound left a comment

SPC Import functionality #23

SPC Import functionality #23

Conversation

ChrisBlueStone commented Feb 17, 2022

charasyn Feb 23, 2022

Choose a reason for hiding this comment

ChrisBlueStone Feb 24, 2022

Choose a reason for hiding this comment

ChrisBlueStone Feb 26, 2022

Choose a reason for hiding this comment

PhoenixBound left a comment

Choose a reason for hiding this comment