From ccb469b9a714d04f76bd01645b616c6983bd8f19 Mon Sep 17 00:00:00 2001
From: m1macrophage <168948267+m1macrophage@users.noreply.github.com>
Date: Tue, 8 Apr 2025 15:36:19 -0700
Subject: [PATCH] sound/va_eg.cpp: Fixing subtle bugs. (#13570)

* Actually take a voltage snapshot when R or C change. This was being attempted, but didn't work because set_target_v would exit early if the target V was not changing. Made the snapshoting more explicit.
* Consider the EG done based on elapsed time, instead of proximity to target value. Some low volume DMX sounds were affected by this.
---
 src/devices/sound/va_eg.cpp | 46 +++++++++++++++++++++----------------
 src/devices/sound/va_eg.h   |  4 ++++
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/devices/sound/va_eg.cpp b/src/devices/sound/va_eg.cpp
index e2674c0e699..401d5c34197 100644
--- a/src/devices/sound/va_eg.cpp
+++ b/src/devices/sound/va_eg.cpp
@@ -5,6 +5,9 @@
 #include "va_eg.h"
 #include "machine/rescap.h"
 
+// The envelope is considered completed after this many time constants.
+static constexpr const float TIME_CONSTANTS_TO_END = 10;
+
 DEFINE_DEVICE_TYPE(VA_RC_EG, va_rc_eg_device, "va_rc_eg", "RC-based Envelope Generator")
 
 va_rc_eg_device::va_rc_eg_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock)
@@ -28,7 +31,7 @@ va_rc_eg_device &va_rc_eg_device::set_r(float r)
 	if (m_stream != nullptr)
 		m_stream->update();
 
-	set_target_v(m_v_end);  // Snapshots voltage, using the old `r` value.
+	snapshot();  // Snapshots voltage using the old `r` value.
 	m_r = r;
 	m_rc_inv = 1.0F / (m_r * m_c);
 	return *this;
@@ -42,7 +45,7 @@ va_rc_eg_device &va_rc_eg_device::set_c(float c)
 	if (m_stream != nullptr)
 		m_stream->update();
 
-	set_target_v(m_v_end);  // Snapshots voltage, using the old `c` value.
+	snapshot();  // Snapshots voltage using the old `c` value.
 	m_c = c;
 	m_rc_inv = 1.0F / (m_r * m_c);
 	return *this;
@@ -55,19 +58,8 @@ va_rc_eg_device &va_rc_eg_device::set_target_v(float v)
 	if (m_stream != nullptr)
 		m_stream->update();
 
-	if (has_running_machine())
-	{
-		const attotime now = machine().time();
-		m_v_start = get_v(now);
-		m_v_end = v;
-		m_t_start = now;
-	}
-	else
-	{
-		m_v_start = 0;
-		m_v_end = v;
-		m_t_start = attotime::zero;
-	}
+	snapshot();
+	m_v_end = v;
 	return *this;
 }
 
@@ -79,6 +71,7 @@ va_rc_eg_device &va_rc_eg_device::set_instant_v(float v)
 	m_v_start = v;
 	m_v_end = v;
 	m_t_start = has_running_machine() ? machine().time() : attotime::zero;
+	m_t_end_approx = m_t_start;
 	return *this;
 }
 
@@ -103,19 +96,16 @@ void va_rc_eg_device::device_start()
 	save_item(NAME(m_v_start));
 	save_item(NAME(m_v_end));
 	save_item(NAME(m_t_start));
+	save_item(NAME(m_t_end_approx));
 }
 
 void va_rc_eg_device::sound_stream_update(sound_stream &stream, const std::vector<read_stream_view> &inputs, std::vector<write_stream_view> &outputs)
 {
-	// The envelope stage will be considered done once the voltage reaches
-	// within MIN_DELTA of the target.
-	static constexpr const float MIN_DELTA = 0.0001F;
-
 	assert(inputs.size() == 0 && outputs.size() == 1);
 	write_stream_view &out = outputs[0];
 	attotime t = out.start_time();
 
-	if (fabsf(get_v(t) - m_v_end) < MIN_DELTA)
+	if (t >= m_t_end_approx)
 	{
 		// Avoid expensive get_v() calls if the envelope stage has completed.
 		out.fill(m_v_end);
@@ -127,3 +117,19 @@ void va_rc_eg_device::sound_stream_update(sound_stream &stream, const std::vecto
 	for (int i = 0; i < n; ++i, t += dt)
 		out.put(i, get_v(t));
 }
+
+void va_rc_eg_device::snapshot()
+{
+	if (has_running_machine())
+	{
+		const attotime now = machine().time();
+		m_v_start = get_v(now);
+		m_t_start = now;
+	}
+	else
+	{
+		m_v_start = 0;
+		m_t_start = attotime::zero;
+	}
+	m_t_end_approx = m_t_start + attotime::from_double(TIME_CONSTANTS_TO_END * m_r * m_c);
+}
diff --git a/src/devices/sound/va_eg.h b/src/devices/sound/va_eg.h
index 520487100b0..bd8c00cd722 100644
--- a/src/devices/sound/va_eg.h
+++ b/src/devices/sound/va_eg.h
@@ -42,6 +42,9 @@ protected:
 	void sound_stream_update(sound_stream &stream, const std::vector<read_stream_view> &inputs, std::vector<write_stream_view> &outputs) override;
 
 private:
+	// Takes a snapshot of the current voltage into m_v_start and m_t_start.
+	void snapshot();
+
 	sound_stream *m_stream;
 
 	float m_r;
@@ -50,6 +53,7 @@ private:
 	float m_v_start;
 	float m_v_end;
 	attotime m_t_start;
+	attotime m_t_end_approx;
 };
 
 DECLARE_DEVICE_TYPE(VA_RC_EG, va_rc_eg_device)