simondotm / vgm-packer

LZ4 based compression tool for packing SN76489 PSG VGM music data

Hi there 👋

Welcome to my Github. I'm Simon and I'm a software engineer and technical director.

• 🔭 I’m currently tinkering with serverless projects using Google Cloud/Typescript/Node
• 🌱 I'm a a retro software fan, and you can follow me on Twitter if you like
• ⚡ Most if not all of my own projects are open sourced MIT License
• 💙 If you've found my projects useful, you can if you like!

vgm-packer's Issues

Data byte should not be ignored for latched noise channel 3 or latched volume (all channels)

in vgmpacker.py (def split_raw) :

					else:
						if verbose:
							print(" tone data on latched channel " + str(latched_channel))
						registers[latched_channel*2+1] = d # we no longer do any masking here # d & 63 # tone data only contains 6 bits of info anyway, so no need for mask
						if latched_channel == 3:
							print("ERROR CHANNEL")

This code does not handle when :

• data byte is written after a latch volume (all channels)
• data byte is written after a latch noise on channel 3

When reading this :
https://www.smspower.org/Development/SN76489
It appears that data byte should not be ignored.

Volume registers
(DDDDDD)dddd = (--vvvv)vvvv
dddd gives the 4-bit volume value.
If a data byte is written, the low 4 bits of DDDDDD update the 4-bit volume value. However, this is unnecessary.

Noise register
(DDDDDD)dddd = (---trr)-trr
The low 2 bits of dddd select the shift rate and the next highest bit (bit 2) selects the mode (white (1) or "periodic" (0)).
If a data byte is written, its low 3 bits update the shift rate and mode in the same way.

The data byte is NOT ignored. If it is, some games (e.g. Micro Machines) produce the wrong sound on their noise channel.

I suggest to use this code instead (verbose should be updated) :

		latched_type = 0

...
			if packet_size == 255:
				Packet = False
			else:
				for x in range(packet_size):
					d = rawData[n+x]
					if verbose:
					   print "  frame byte number=" +str(x)
					   print "    frame byte=" +str(d)
					if d & 128:
						# latch
						c = (d>>5)&3
						latched_channel = c
						if d & 16:
							# volume
							if verbose:
								print(" volume on channel " + str(c))
							registers[c+7] = d & register_mask
							latched_type = 1

						else:
							# tone
							if verbose:
								print(" tone on channel " + str(c))

							registers[c*2+0] = d & register_mask
							latched_type = 0                    

					else:
						if verbose:
							print(" tone data on latched channel " + str(latched_channel))

						if latched_type == 0:
							if latched_channel < 3:
								# tone upper bits
								registers[latched_channel*2+1] = d
							else:
								# update noise 
								registers[latched_channel*2] = d & 7
						else:
							# update volume
							registers[latched_channel+7] = d & 15

Wrong error message : Found invalid noise register setting of 8

When using vgmpacker, a wrong error message is printed on each run :

"Found invalid noise register setting of 8"

It's because the value 8 is used as an end marker :

		# Add EOF marker (0x08) to tone3 byte stream
		output_blocks[6].append(0x08)	# 0x08 is an invalid noise tone.

The code that checks invalid noise settings should take care of this EOF marker, or the check should be removed.

		# check there's no odd noise settings
		if True:
			invalid_noise_range = False
			for n in range(len(registers[6])):
				noise = registers[6][n]
				if noise > 7:
					print(" - Found invalid noise register setting of " + str(noise) + ", at offset " + str(n))
					invalid_noise_range = True

Encoding fails when blockSize is 1, because of stdout progress indicator

Encoding fails when blockSize is 1, because of stdout progress indicator :

lz4enc.py

        # show progress
        if (i & 511) == 0 or i == (blockSize - 1):
          sys.stdout.write("   Scanning block data " + str(int(i*100/(blockSize-1))) + "%...\r")
          sys.stdout.flush()

Adding a test will fix the issue :

        # show progress
        if (i & 511) == 0 or i == (blockSize - 1):
          if (blockSize > 1):
            sys.stdout.write("   Scanning block data " + str(int(i*100/(blockSize-1))) + "%...\r")
          sys.stdout.flush()

lz4enc should not have an auto detection to check if data should be compressed or not

lz4enc have an auto detection to check if data should be compressed or not based on output size vs original size.
But vgmpacker and the player are only handling compressed streams.

      # did compression do harm ?
      useCompression   = len(block) < uncompressedSize and not uncompressed

I recommend to force useCompression to True.

This defect will happen when small sized VGM are encoded, in this log we can see that testUnpackLZ4 does not handle the uncompressed flag and fails.

  Selecting best matches...
 Writing output block - uncompressed (12), compressed (13) ...
  Uncompressed data selected for this block.

new token, unpacked offset=0
literal_count=15, literal_length=15
literal_count=240, literal_length=255
copy literals - literal_length=255
literal byte copy n=0, to offset 0, with byte 0xf0
literal byte copy n=1, to offset 1, with byte 0xf0
literal byte copy n=2, to offset 2, with byte 0xf0
literal byte copy n=3, to offset 3, with byte 0xf0
literal byte copy n=4, to offset 4, with byte 0xf0
literal byte copy n=5, to offset 5, with byte 0xf0
literal byte copy n=6, to offset 6, with byte 0xf0
literal byte copy n=7, to offset 7, with byte 0xf0
literal byte copy n=8, to offset 8, with byte 0xf0
literal byte copy n=9, to offset 9, with byte 0xf0
Traceback (most recent call last):
  File "C:\Users\bhrou\git\6809-game-builder\toolbox\audio\vgm2vgc\src\main\resources\vgmpacker\vgmpacker.py", line 806, in <module>
    packer.process(src, dst, args.buffer, args.huffman)
  File "C:\Users\bhrou\git\6809-game-builder\toolbox\audio\vgm2vgc\src\main\resources\vgmpacker\vgmpacker.py", line 715, in process
    self.testUnpackLZ4(compressed_block, stream)
  File "C:\Users\bhrou\git\6809-game-builder\toolbox\audio\vgm2vgc\src\main\resources\vgmpacker\vgmpacker.py", line 485, in testUnpackLZ4
    byte = getByte()
  File "C:\Users\bhrou\git\6809-game-builder\toolbox\audio\vgm2vgc\src\main\resources\vgmpacker\vgmpacker.py", line 459, in getByte
    byte = compressed[self.index]
IndexError: bytearray index out of range

Two frames are added at the end of data stream that does not exists in the original VGM file

In vgmparser.py, the parser add two frames at the end of data stream that does not exists in the original VGM file.
Thoses two added frames breaks the music rythm when the stream is looped.

The first one is added when token x66 is found. As per VGM specification, x66 is end of file or goto marker when a loop exists. this token does not mean there is a frame wait.
I understand that the vgc player is based on frame "blocks" that can not be splitted between an end and a start of a file, so vgm files should be prepared in this regard. Well formed VGM files that plays frame based stream should end by x63 x66 or x62 x66.
Trackers like "Furnace" has options to convert to frame based loops when exporting vgm files, and will end the file correctly (this is not the case for Deflemask).

So here, it is recommended to not append data when command is x66 :

			# 0x62 - Wait 735 samples (60th of a second)
			# 0x63 - Wait 882 samples (50th of a second)
			# 0x66 - End of sound data
			elif command in [b'\x62', b'\x63', b'\x66']:
				self.command_list.append({'command': command, 'data': None})

				# Stop processing commands if we are at the end of the music
				# data
				if command == b'\x66':
					break

Just before the EOF, the last wait should also be removed :

		# eof
		data_block.append(0x00)	# append one last wait
		data_block.append(0xFF)	# signal EOF