Refactor the ReelMagic audio FIFO class

include/mixer.h

@@ -65,7 +65,11 @@ struct AudioFrame {
 	float left  = 0.0f;
 	float right = 0.0f;
 
-	float &operator[](std::size_t i)
+	constexpr AudioFrame() = default;
+	constexpr AudioFrame(const float l, const float r) : left(l), right(r) {}
+	constexpr AudioFrame(const int16_t l, const int16_t r) : left(l), right(r) {}
+
+	constexpr float& operator[](const size_t i) noexcept
 	{
 		assert(i < 2);
 		return i == 0 ? left : right;
@@ -170,16 +174,19 @@ class MixerChannel {
 	bool HasFeature(ChannelFeature feature) const;
 	const std::string& GetName() const;
 	int GetSampleRate() const;
-	using apply_level_callback_f = std::function<void(const AudioFrame &level)>;
-	void RegisterLevelCallBack(apply_level_callback_f cb);
-	void SetVolume(const float left, const float right);
-	void SetVolumeScale(const float f);
-	void SetVolumeScale(const float left, const float right);
-	const AudioFrame& GetVolumeScale() const;
+
+	void SetUserVolume(const float left, const float right);
+	void SetAppVolume(const float f);
+	void SetAppVolume(const float left, const float right);
+	void Set0dbScalar(const float f);
+	void RecalcCombinedVolume();
+
+	const AudioFrame& GetUserVolume() const;
+	const AudioFrame& GetAppVolume() const;
+
 	void ChangeChannelMap(const LINE_INDEX left, const LINE_INDEX right);
 	bool ChangeLineoutMap(std::string choice);
 	std::string DescribeLineout() const;
-	void UpdateVolume();
 	void ReactivateEnvelope();
 	void SetSampleRate(const int _freq);
 	void SetPeakAmplitude(const int peak);
@@ -233,7 +240,6 @@ class MixerChannel {
 	bool WakeUp(); // pass-through to the sleeper
 	void FlushSamples();
 
-	AudioFrame volume = {1.0f, 1.0f};
 	std::atomic<int> frames_done = 0; // Timing on how many sample frames
 	                                  // have been done by the mixer
 
@@ -272,12 +278,41 @@ class MixerChannel {
 	AudioFrame next_frame = {};
 
 	int sample_rate = 0u;
-	AudioFrame volume_gain = {1.0f, 1.0f};
-	AudioFrame volume_scale = {1.0f, 1.0f};
+
+	// Volume scalars
+	// ~~~~~~~~~~~~~~
+	// The user sets this via MIXER.COM, which lets them magnify or diminish
+	// the channel's volume relative to other adjustments, such as any
+	// adjustments done by the application at runtime.
+	AudioFrame user_volume_scalar = {1.0f, 1.0f};
+
+	// The application (might) adjust a channel's volume programmatically at
+	// runtime via the Sound Blaster or ReelMagic control interfaces.
+	AudioFrame app_volume_scalar = {1.0f, 1.0f};
+
+	// The 0 dB volume scalar is used to bring a channel to 0 dB in the
+	// signed 16-bit [-32k, +32k] range.
+	//
+	// Two examples:
+	//
+	//  1. The ReelMagic's MP2 samples are decoded to as floats between
+	//     [-1.0f, +1.0f], so for we we set this to 32767.0f.
+	//
+	//  2. The GUS's simultaneous voices can accumulate to ~100%+RMS
+	//     above 0 dB, so for that channel we set this to RMS (sqrt of half).
+	//
+	float db0_volume_scalar = 1.0f;
+
+	// All three of these are multiplied together to form the combined
+	// volume scalar. This means we can apply one float-multiply per sample
+	// and perform all three adjustments at once.
+	//
+	AudioFrame combined_volume_scalar = {1.0f, 1.0f};
 
 	// Defines the peak sample amplitude we can expect in this channel.
 	// Default to signed 16bit max, however channel's that know their own
 	// peak, like the PCSpeaker, should update it with: SetPeakAmplitude()
+	//
 	int peak_amplitude = MAX_AUDIO;
 
 	struct StereoLine {
@@ -297,11 +332,6 @@ class MixerChannel {
 	// "right" as themselves or vice-versa.
 	StereoLine channel_map = STEREO;
 
-	// The RegisterLevelCallBack() assigns this callback that can be used by
-	// the channel's source to manage the stream's level prior to mixing,
-	// in-place of scaling by volume.
-	apply_level_callback_f apply_level = nullptr;
-
 	bool last_samples_were_stereo = false;
 	bool last_samples_were_silence = true;
 

src/dos/cdrom_image.cpp

@@ -833,10 +833,8 @@ void CDROM_Interface_Image::ChannelControl(TCtrl ctrl)
 #endif
 		return;
 	}
-
 	// Adjust the volume of our mixer channel as defined by the application
-	player.channel->SetVolumeScale(static_cast<float>(ctrl.vol[0] / 255.0), // left vol
-	                               static_cast<float>(ctrl.vol[1] / 255.0)); // right vol
+	player.channel->SetAppVolume(ctrl.vol[0] / 255.0f, ctrl.vol[1] / 255.0f);
 
 	// Map the audio channels in our mixer channel as defined by the application
 	const auto left_mapped = static_cast<LINE_INDEX>(ctrl.out[0]);

src/hardware/gus.cpp

@@ -248,7 +248,6 @@ class Gus {
 	void Reset(uint8_t state);
 	AudioFrame RenderFrame();
 	void RenderUpToNow();
-	void SetLevelCallback(const AudioFrame &levels);
 	void StopPlayback();
 	void UpdateDmaAddress(uint8_t new_address);
 	void UpdateWaveMsw(int32_t &addr) const noexcept;
@@ -276,7 +275,6 @@ class Gus {
 	Voice *target_voice = nullptr;
 	DmaChannel *dma_channel = nullptr;
 	mixer_channel_t audio_channel = nullptr;
-	AudioFrame accumulator_scalar = {};
 
 	// Playback related
 	double last_rendered_ms = 0.0;
@@ -611,6 +609,11 @@ Gus::Gus(uint16_t port, uint8_t dma, uint8_t irq, const std::string &ultradir,
 
 	assert(audio_channel);
 
+	// GUS is prone to accumulating beyond the 16-bit range so we scale back
+	// by RMS.
+	constexpr auto rms_squared = static_cast<float>(M_SQRT1_2);
+	audio_channel->Set0dbScalar(rms_squared);
+
 	if (!audio_channel->TryParseAndSetCustomFilter(filter_prefs)) {
 		if (filter_prefs != "off")
 			LOG_WARNING("GUS: Invalid 'gus_filter' value: '%s', using 'off'",
@@ -622,10 +625,6 @@ Gus::Gus(uint16_t port, uint8_t dma, uint8_t irq, const std::string &ultradir,
 
 	ms_per_render = millis_in_second / audio_channel->GetSampleRate();
 
-	// Let the mixer command adjust the GUS's internal amplitude level's
-	const auto set_level_callback = std::bind(&Gus::SetLevelCallback, this, _1);
-	audio_channel->RegisterLevelCallBack(set_level_callback);
-
 	UpdateDmaAddress(dma);
 
 	// Populate the volume, pan, and auto-exec arrays
@@ -653,16 +652,6 @@ void Gus::ActivateVoices(uint8_t requested_voices)
 	}
 }
 
-void Gus::SetLevelCallback(const AudioFrame &levels)
-{
-	// GUS is prone to accumulating beyond the 16-bit range. Until we have
-	// an auto-volume-dampen-on-clip at the mixer-level, this function will
-	// scale the user-provided channel level by RMS.
-	constexpr auto rms_squared = static_cast<float>(M_SQRT1_2);
-
-	accumulator_scalar = {levels.left * rms_squared, levels.right * rms_squared};
-}
-
 AudioFrame Gus::RenderFrame()
 {
 	AudioFrame accumulator = {};
@@ -681,8 +670,6 @@ AudioFrame Gus::RenderFrame()
 
 			++voice;
 		}
-		accumulator.left *= accumulator_scalar.left;
-		accumulator.right *= accumulator_scalar.right;
 	}
 	CheckVoiceIrq();
 	return accumulator;

src/hardware/mixer.cpp

@@ -487,8 +487,8 @@ mixer_channel_t MIXER_AddChannel(MIXER_Handler handler, const int freq,
 {
 	auto chan = std::make_shared<MixerChannel>(handler, name, features);
 	chan->SetSampleRate(freq);
-	chan->SetVolumeScale(1.0f);
-	chan->SetVolume(1.0f, 1.0f);
+	chan->SetAppVolume(1.0f);
+	chan->SetUserVolume(1.0f, 1.0f);
 	chan->ChangeChannelMap(LEFT, RIGHT);
 	chan->Enable(false);
 
@@ -537,70 +537,79 @@ mixer_channel_t MIXER_FindChannel(const char *name)
 	return chan;
 }
 
-void MixerChannel::RegisterLevelCallBack(apply_level_callback_f cb)
+void MixerChannel::RecalcCombinedVolume()
 {
-	apply_level = cb;
-	apply_level(volume);
-}
-
-void MixerChannel::UpdateVolume()
-{
-	// Don't scale by volume if the level is being managed by the source
-	const float gain_left  = apply_level ? 1.0f : volume.left;
-	const float gain_right = apply_level ? 1.0f : volume.right;
+	combined_volume_scalar.left = user_volume_scalar.left *
+	                              app_volume_scalar.left *
+	                              mixer.master_volume.left * db0_volume_scalar;
 
-	volume_gain.left  = volume_scale.left  * gain_left  * mixer.master_volume.left;
-	volume_gain.right = volume_scale.right * gain_right * mixer.master_volume.right;
+	combined_volume_scalar.right = user_volume_scalar.right *
+	                               app_volume_scalar.right *
+	                               mixer.master_volume.right * db0_volume_scalar;
 }
 
-void MixerChannel::SetVolume(const float left, const float right)
+void MixerChannel::SetUserVolume(const float left, const float right)
 {
 	// Allow unconstrained user-defined values
-	volume = {left, right};
-
-	if (apply_level)
-		apply_level(volume);
-
-	UpdateVolume();
+	user_volume_scalar = {left, right};
+	RecalcCombinedVolume();
 }
 
-void MixerChannel::SetVolumeScale(const float f) {
-	SetVolumeScale(f, f);
+void MixerChannel::SetAppVolume(const float v)
+{
+	SetAppVolume(v, v);
 }
 
-void MixerChannel::SetVolumeScale(const float left, const float right)
+void MixerChannel::SetAppVolume(const float left, const float right)
 {
 	// Constrain application-defined volume between 0% and 100%
-	constexpr auto min_volume = 0.0f;
-	constexpr auto max_volume = 1.0f;
-
-	auto new_left  = clamp(left, min_volume, max_volume);
-	auto new_right = clamp(right, min_volume, max_volume);
+	auto clamp_to_unity = [](const float vol) {
+		constexpr auto min_unity_volume = 0.0f;
+		constexpr auto max_unity_volume = 1.0f;
+		return clamp(vol, min_unity_volume, max_unity_volume);
+	};
+	app_volume_scalar = {clamp_to_unity(left), clamp_to_unity(right)};
+	RecalcCombinedVolume();
 
-	if (volume_scale.left != new_left || volume_scale.right != new_right) {
-		volume_scale.left  = new_left;
-		volume_scale.right = new_right;
-		UpdateVolume();
 #ifdef DEBUG
-		LOG_MSG("MIXER %-7s channel: application changed left and right volumes to %3.0f%% and %3.0f%%, respectively",
-		        name,
-		        volume_scale.left * 100.0f,
-		        volume_scale.right * 100.0f);
+	LOG_MSG("MIXER %-7s channel: application requested volume "
+	        "{%3.0f%%, %3.0f%%}, and was set to {%3.0f%%, %3.0f%%}",
+	        name,
+	        static_cast<double>(left),
+	        static_cast < double(right),
+	        static_cast<double>(app_volume_scalar.left * 100.0f),
+	        static_cast<double>(app_volume_scalar.right * 100.0f));
 #endif
-	}
 }
 
-const AudioFrame& MixerChannel::GetVolumeScale() const
+void MixerChannel::Set0dbScalar(const float scalar)
 {
-	return volume_scale;
+	// Realistically we expect some channels might need a fixed boost
+	// to get to 0dB, but others might need a range mapping, like from
+	// a unity float [-1.0f, +1.0f] to  16-bit int [-32k,+32k] range.
+	assert(scalar >= 0.0f && scalar <= static_cast<int16_t>(INT16_MAX));
+
+	db0_volume_scalar = scalar;
+
+	RecalcCombinedVolume();
+}
+
+const AudioFrame& MixerChannel::GetUserVolume() const
+{
+	return user_volume_scalar;
+}
+
+const AudioFrame& MixerChannel::GetAppVolume() const
+{
+	return app_volume_scalar;
 }
 
 static void MIXER_UpdateAllChannelVolumes()
 {
 	MIXER_LockAudioDevice();
 
 	for (auto &it : mixer.channels)
-		it.second->UpdateVolume();
+		it.second->RecalcCombinedVolume();
 
 	MIXER_UnlockAudioDevice();
 }
@@ -873,11 +882,11 @@ void MixerChannel::AddSilence()
 				mixpos &= MIXER_BUFMASK;
 
 				mixer.work[mixpos][mapped_output_left] +=
-				        prev_frame.left * volume_gain.left;
+				        prev_frame.left * combined_volume_scalar.left;
 
 				mixer.work[mixpos][mapped_output_right] +=
 				        (stereo ? prev_frame.right : prev_frame.left) *
-				        volume_gain.right;
+				        combined_volume_scalar.right;
 
 				prev_frame = next_frame;
 				mixpos++;
@@ -1376,10 +1385,11 @@ void MixerChannel::ConvertSamples(const Type *data, const uint16_t frames,
 		// prevent severe clicks and pops. Becomes a no-op when done.
 		envelope.Process(stereo, prev_frame);
 
-		const auto left = prev_frame[mapped_channel_left] * volume_gain.left;
+		const auto left = prev_frame[mapped_channel_left] *
+		                  combined_volume_scalar.left;
 		const auto right = (stereo ? prev_frame[mapped_channel_right]
 		                           : prev_frame[mapped_channel_left]) *
-		                   volume_gain.right;
+		                   combined_volume_scalar.right;
 
 		out_frame = {0.0f, 0.0f};
 		out_frame[mapped_output_left] += left;
@@ -1663,10 +1673,12 @@ void MixerChannel::AddStretched(const uint16_t len, int16_t *data)
 		const auto sample = prev_frame.left +
 		                    ((diff * diff_mul) >> FREQ_SHIFT);
 
-		const AudioFrame frame_with_gain = {sample * volume_gain.left,
-		                                    sample * volume_gain.right};
-		if (do_sleep)
+		const AudioFrame frame_with_gain = {
+		        sample * combined_volume_scalar.left,
+		        sample * combined_volume_scalar.right};
+		if (do_sleep) {
 			sleeper.Listen(frame_with_gain);
+		}
 
 		mixer.work[mixpos][mapped_output_left] += frame_with_gain.left;
 		mixer.work[mixpos][mapped_output_right] += frame_with_gain.right;
@@ -2188,17 +2200,15 @@ static std::optional<AudioFrame> parse_volume(const std::string &s)
 	auto parts = split(s, ':');
 	if (parts.size() == 1) {
 		if (const auto v = to_volume(parts[0]); v) {
-			AudioFrame volume = {*v, *v};
-			return volume;
+			return AudioFrame(*v, *v);
 		}
 	}
 	// stereo volume value
 	else if (parts.size() == 2) {
 		const auto l = to_volume(parts[0]);
 		const auto r = to_volume(parts[1]);
 		if (l && r) {
-			AudioFrame volume = {*l, *r};
-			return volume;
+			return AudioFrame(*l, *r);
 		}
 	}
 	return {};
@@ -2329,7 +2339,7 @@ class MIXER final : public Program {
 					continue;
 
 				if (const auto v = parse_volume(arg); v) {
-					channel->SetVolume(v->left, v->right);
+					channel->SetUserVolume(v->left, v->right);
 				}
 			}
 		}
@@ -2462,7 +2472,7 @@ class MIXER final : public Program {
 			auto mode = chan->DescribeLineout();
 
 			show_channel(convert_ansi_markup(channel_name),
-			             chan->volume,
+			             chan->GetUserVolume(),
 			             mode,
 			             xfeed,
 			             reverb,