Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is CHANNEL_VOLUME_CURVE_POWER = 30? #102

Open
leo-hydraulic opened this issue Dec 31, 2023 · 9 comments
Open

Why is CHANNEL_VOLUME_CURVE_POWER = 30? #102

leo-hydraulic opened this issue Dec 31, 2023 · 9 comments

Comments

@leo-hydraulic
Copy link

This is more just a question than a bug, but there was no "Discussion" section in the project. Maybe add comments as a fix?

I was wondering where did the value of 30 come from in modeling the volume curve for the PSG:
https://github.com/ppeccin/WebMSX/blob/master/src/main/msx/audio/PSGAudio.js#L292

I notice it's a simple exponential formula, was just wondering why choose 30 as the base. I tried tracking previous commits, and the value used to be 16, and changed to 30 without comments. I wonder, is this just an aesthetic change because it "sounds good", or is that taken from a datasheet, or oscilloscope measurements, etc?

I've been doing a few experiments with MSX audio myself, and I've been taking WebMSX as a source of information and inspiration. Great project, and thanks!

@inchl
Copy link

inchl commented Jan 22, 2024

It is a bug!

Webmsx plays terribly psg samples, until now I though it was caused by the fact that it's a javascript-based emulator. However its because of a incorrect volume-table being used.

function createVolumeCurve() {
for (var v = 0; v < 16; v++)
volumeCurve[v] = (Math.pow(CHANNEL_VOLUME_CURVE_POWER, v / 15) - 1) / (CHANNEL_VOLUME_CURVE_POWER - 1) * CHANNEL_MAX_VOLUME;
}

When the code is changed into (below).... the psg samples are being played correctly!

function createVolumeCurve() {
for (var v = 1; v < 16; v++)
volumeCurve[v] = Math.pow(2, -(15-v)/2) * CHANNEL_MAX_VOLUME;
}

more info:
https://map.grauw.nl/articles/psg_sample.php

example video (error and fixed):
https://github.com/ppeccin/WebMSX/assets/19953254/95d99242-d132-465b-bd0a-8f58e4ecd897

@ppeccin
Copy link
Owner

ppeccin commented Jan 23, 2024 via email

@inchl
Copy link

inchl commented Jan 24, 2024

@NullPointerReference
Copy link

Hi Stephan,

I applied your changes to the createVolumeCurve() and tried https://nopmsx.nl/wp-content/uploads/2023/10/KingsValleyEnhanced9811ASCII16.zip, but that doesn't seem to sound fully correct still (compared to OpenMSX) ?

Am I doing something wrong perhaps or is there still some difference in emulation that possibly needs to be addressed ?

@inchl
Copy link

inchl commented Mar 13, 2024

Will check later this day. I fixed the sound by correcting the volumes (using the browser debugger) in the appropriate variables used by the replay routine of webmsx. Not sure if the function mentioned is the correct place to calculate the correct volumes. I tried to fix the filehunter website by providing them with a fixed function, but that also did not solve the problem. Anyway... I will look into this later this day and get back to you!

@inchl
Copy link

inchl commented Mar 13, 2024

Might be this:
In javascript arrays are not initliazed... So manually setting volumeCurve[0] = 0 is required (for loops starts at 1). Index 0 cannot be calculated with the formula. Must be 0!

@NullPointerReference
Copy link

I tested with index 0 explicitly set to 0 too, as I noticed it to ('empty'); it didn't help (much) :)

@inchl
Copy link

inchl commented Mar 13, 2024

Haha... Try changing the closure variables during playback / during a running webmsx.... Setting all 16 as they should be... Then it works!

@NullPointerReference
Copy link

NullPointerReference commented Apr 22, 2024

@ppeccin: @inchl, @TFHFony and I tested the (by @inchl) proposed curve - with the additional 'volumeCurve[0] = 0' - extensively and also put it on file-hunter.com for 'public' testing for about a month. We think it solves this audio-emulation issue and has no further negative impact. The complete snipplet is as follows, no additional changes outside mentioned code required.

function createVolumeCurve() { volumeCurve[0] = 0; for (let v = 1; v < 16; v++) volumeCurve[v] = Math.pow(2, -(15-v)/2) * CHANNEL_MAX_VOLUME; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants