[Scummvm-git-logs] scummvm master -> bcbd4433594539cb0208809af12c6278632f8301

csnover csnover at users.noreply.github.com
Tue Sep 12 18:36:36 CEST 2017


This automated email contains information about 4 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
eb4e9fe1d4 GUI: Remove mostly-broken audio output sample rate control
4a75fbab1b SDL: Reduce audio playback latency
fa52df018e SDL: Fix DoubleBufferSDLMixerManager doubling audio latency
bcbd443359 SDL: Stop using double buffering mixer on macOS


Commit: eb4e9fe1d48bee1c9f641b019a29642780604b30
    https://github.com/scummvm/scummvm/commit/eb4e9fe1d48bee1c9f641b019a29642780604b30
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-12T11:27:45-05:00

Commit Message:
GUI: Remove mostly-broken audio output sample rate control

Removing this GUI control was suggested as far back as 2011 at
<http://lists.scummvm.org/pipermail/scummvm-devel/2011-November/010416.html>.
There were no objections, but it was never removed. When working
on audio latency bugs, I independently rediscovered that the GUI
option was broken: the per-game options would *never* work, and the
option would not take effect until ScummVM was restarted because
there is no API for interacting with the backend audio mixer. So
now, it is finally gone.

Primarily for the sake of future troubleshooting, configurability
of the audio sample frequency within SdlMixerManager is maintained
for the moment, but now users will need to edit their ScummVM
configuration file manually to change it.

Changed paths:
    backends/mixer/sdl/sdl-mixer.cpp
    gui/options.cpp
    gui/options.h
    gui/themes/default.inc
    gui/themes/scummclassic.zip
    gui/themes/scummclassic/classic_layout.stx
    gui/themes/scummclassic/classic_layout_lowres.stx
    gui/themes/scummmodern.zip
    gui/themes/scummmodern/scummmodern_layout.stx
    gui/themes/scummmodern/scummmodern_layout_lowres.stx


diff --git a/backends/mixer/sdl/sdl-mixer.cpp b/backends/mixer/sdl/sdl-mixer.cpp
index 3e65fba..5a31feb 100644
--- a/backends/mixer/sdl/sdl-mixer.cpp
+++ b/backends/mixer/sdl/sdl-mixer.cpp
@@ -128,23 +128,27 @@ void SdlMixerManager::init() {
 SDL_AudioSpec SdlMixerManager::getAudioSpec(uint32 outputRate) {
 	SDL_AudioSpec desired;
 
-	// Determine the desired output sampling frequency.
-	uint32 samplesPerSec = 0;
-	if (ConfMan.hasKey("output_rate"))
-		samplesPerSec = ConfMan.getInt("output_rate");
-	if (samplesPerSec <= 0)
-		samplesPerSec = outputRate;
+	const char *const appDomain = Common::ConfigManager::kApplicationDomain;
+
+	// There was once a GUI option for this, but it was never used;
+	// configurability is retained for advanced users only who wish to modify
+	// their ScummVM config file directly
+	uint32 freq = 0;
+	if (ConfMan.hasKey("output_rate", appDomain))
+		freq = ConfMan.getInt("output_rate", appDomain);
+	if (freq <= 0)
+		freq = outputRate;
 
 	// Determine the sample buffer size. We want it to store enough data for
 	// at least 1/16th of a second (though at most 8192 samples). Note
 	// that it must be a power of two. So e.g. at 22050 Hz, we request a
 	// sample buffer size of 2048.
 	uint32 samples = 8192;
-	while (samples * 16 > samplesPerSec * 2)
+	while (samples * 16 > freq * 2)
 		samples >>= 1;
 
 	memset(&desired, 0, sizeof(desired));
-	desired.freq = samplesPerSec;
+	desired.freq = freq;
 	desired.format = AUDIO_S16SYS;
 	desired.channels = 2;
 	desired.samples = (uint16)samples;
diff --git a/gui/options.cpp b/gui/options.cpp
index 3ff0276..82eb252 100644
--- a/gui/options.cpp
+++ b/gui/options.cpp
@@ -120,8 +120,6 @@ enum {
 
 static const char *savePeriodLabels[] = { _s("Never"), _s("every 5 mins"), _s("every 10 mins"), _s("every 15 mins"), _s("every 30 mins"), 0 };
 static const int savePeriodValues[] = { 0, 5 * 60, 10 * 60, 15 * 60, 30 * 60, -1 };
-static const char *outputRateLabels[] = { _s("<default>"), _s("8 kHz"), _s("11 kHz"), _s("22 kHz"), _s("44 kHz"), _s("48 kHz"), 0 };
-static const int outputRateValues[] = { 0, 8000, 11025, 22050, 44100, 48000, -1 };
 // The keyboard mouse speed values range from 0 to 7 and correspond to speeds shown in the label
 // "10" (value 3) is the default speed corresponding to the speed before introduction of this control
 static const char *kbdMouseSpeedLabels[] = { "3", "5", "8", "10", "13", "15", "18", "20", 0 };
@@ -167,8 +165,6 @@ void OptionsDialog::init() {
 	_midiPopUpDesc = 0;
 	_oplPopUp = 0;
 	_oplPopUpDesc = 0;
-	_outputRatePopUp = 0;
-	_outputRatePopUpDesc = 0;
 	_enableMIDISettings = false;
 	_gmDevicePopUp = 0;
 	_gmDevicePopUpDesc = 0;
@@ -335,15 +331,6 @@ void OptionsDialog::build() {
 		_oplPopUp->setSelectedTag(id);
 	}
 
-	if (_outputRatePopUp) {
-		_outputRatePopUp->setSelected(1);
-		int value = ConfMan.getInt("output_rate", _domain);
-		for	(int i = 0; outputRateLabels[i]; i++) {
-			if (value == outputRateValues[i])
-				_outputRatePopUp->setSelected(i);
-		}
-	}
-
 	if (_multiMidiCheckbox) {
 		if (!loadMusicDeviceSetting(_gmDevicePopUp, "gm_device"))
 			_gmDevicePopUp->setSelected(0);
@@ -641,17 +628,6 @@ void OptionsDialog::apply() {
 		}
 	}
 
-	if (_outputRatePopUp) {
-		if (_enableAudioSettings) {
-			if (_outputRatePopUp->getSelectedTag() != 0)
-				ConfMan.setInt("output_rate", _outputRatePopUp->getSelectedTag(), _domain);
-			else
-				ConfMan.removeKey("output_rate", _domain);
-		} else {
-			ConfMan.removeKey("output_rate", _domain);
-		}
-	}
-
 	// MIDI options
 	if (_multiMidiCheckbox) {
 		if (_enableMIDISettings) {
@@ -860,8 +836,6 @@ void OptionsDialog::setAudioSettingsState(bool enabled) {
 		_oplPopUpDesc->setEnabled(enabled);
 		_oplPopUp->setEnabled(enabled);
 	}
-	_outputRatePopUpDesc->setEnabled(enabled);
-	_outputRatePopUp->setEnabled(enabled);
 }
 
 void OptionsDialog::setMIDISettingsState(bool enabled) {
@@ -1096,14 +1070,6 @@ void OptionsDialog::addAudioControls(GuiObject *boss, const Common::String &pref
 		++ed;
 	}
 
-	// Sample rate settings
-	_outputRatePopUpDesc = new StaticTextWidget(boss, prefix + "auSampleRatePopupDesc", _("Output rate:"), _("Higher value specifies better sound quality but may be not supported by your soundcard"));
-	_outputRatePopUp = new PopUpWidget(boss, prefix + "auSampleRatePopup", _("Higher value specifies better sound quality but may be not supported by your soundcard"));
-
-	for (int i = 0; outputRateLabels[i]; i++) {
-		_outputRatePopUp->appendEntry(_(outputRateLabels[i]), outputRateValues[i]);
-	}
-
 	_enableAudioSettings = true;
 }
 
diff --git a/gui/options.h b/gui/options.h
index ed07307..b1666c2 100644
--- a/gui/options.h
+++ b/gui/options.h
@@ -159,8 +159,6 @@ private:
 	PopUpWidget *_midiPopUp;
 	StaticTextWidget *_oplPopUpDesc;
 	PopUpWidget *_oplPopUp;
-	StaticTextWidget *_outputRatePopUpDesc;
-	PopUpWidget *_outputRatePopUp;
 
 	StaticTextWidget *_mt32DevicePopUpDesc;
 	PopUpWidget *_mt32DevicePopUp;
diff --git a/gui/themes/default.inc b/gui/themes/default.inc
index 23488a8..fe48acd 100644
--- a/gui/themes/default.inc
+++ b/gui/themes/default.inc
@@ -907,14 +907,6 @@ const char *defaultXML1 = "<?xml version = '1.0'?>"
 "type='PopUp' "
 "/>"
 "</layout>"
-"<layout type='horizontal' padding='0,0,0,0' spacing='10' center='true'>"
-"<widget name='auSampleRatePopupDesc' "
-"type='OptionsLabel' "
-"/>"
-"<widget name='auSampleRatePopup' "
-"type='PopUp' "
-"/>"
-"</layout>"
 "<layout type='horizontal' padding='0,0,0,0' spacing='10'>"
 "<widget name='subToggleDesc' "
 "type='OptionsLabel' "
@@ -2496,14 +2488,6 @@ const char *defaultXML1 = "<?xml version = '1.0'?>"
 "type='PopUp' "
 "/>"
 "</layout>"
-"<layout type='horizontal' padding='0,0,0,0' spacing='6' center='true'>"
-"<widget name='auSampleRatePopupDesc' "
-"type='OptionsLabel' "
-"/>"
-"<widget name='auSampleRatePopup' "
-"type='PopUp' "
-"/>"
-"</layout>"
 "<layout type='horizontal' padding='0,0,0,0' spacing='3' center='true'>"
 "<widget name='subToggleDesc' "
 "type='OptionsLabel' "
diff --git a/gui/themes/scummclassic.zip b/gui/themes/scummclassic.zip
index acb6d20..4f7e3ce 100644
Binary files a/gui/themes/scummclassic.zip and b/gui/themes/scummclassic.zip differ
diff --git a/gui/themes/scummclassic/classic_layout.stx b/gui/themes/scummclassic/classic_layout.stx
index 2bb07d9..e3411dc 100644
--- a/gui/themes/scummclassic/classic_layout.stx
+++ b/gui/themes/scummclassic/classic_layout.stx
@@ -335,14 +335,6 @@
 						type = 'PopUp'
 				/>
 			</layout>
-			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '10' center = 'true'>
-				<widget name = 'auSampleRatePopupDesc'
-						type = 'OptionsLabel'
-				/>
-				<widget name = 'auSampleRatePopup'
-						type = 'PopUp'
-				/>
-			</layout>
 			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '10'>
 				<widget name = 'subToggleDesc'
 						type = 'OptionsLabel'
diff --git a/gui/themes/scummclassic/classic_layout_lowres.stx b/gui/themes/scummclassic/classic_layout_lowres.stx
index d0c3170..4b710b8 100644
--- a/gui/themes/scummclassic/classic_layout_lowres.stx
+++ b/gui/themes/scummclassic/classic_layout_lowres.stx
@@ -332,14 +332,6 @@
 						type = 'PopUp'
 				/>
 			</layout>
-			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '6' center = 'true'>
-				<widget name = 'auSampleRatePopupDesc'
-						type = 'OptionsLabel'
-				/>
-				<widget name = 'auSampleRatePopup'
-						type = 'PopUp'
-				/>
-			</layout>
 			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '3' center = 'true'>
 				<widget name = 'subToggleDesc'
 						type = 'OptionsLabel'
diff --git a/gui/themes/scummmodern.zip b/gui/themes/scummmodern.zip
index 055c82a..fae746b 100644
Binary files a/gui/themes/scummmodern.zip and b/gui/themes/scummmodern.zip differ
diff --git a/gui/themes/scummmodern/scummmodern_layout.stx b/gui/themes/scummmodern/scummmodern_layout.stx
index 0a1c377..34b66e2 100644
--- a/gui/themes/scummmodern/scummmodern_layout.stx
+++ b/gui/themes/scummmodern/scummmodern_layout.stx
@@ -349,14 +349,6 @@
 						type = 'PopUp'
 				/>
 			</layout>
-			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '10' center = 'true'>
-				<widget name = 'auSampleRatePopupDesc'
-						type = 'OptionsLabel'
-				/>
-				<widget name = 'auSampleRatePopup'
-						type = 'PopUp'
-				/>
-			</layout>
 			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '10'>
 				<widget name = 'subToggleDesc'
 						type = 'OptionsLabel'
diff --git a/gui/themes/scummmodern/scummmodern_layout_lowres.stx b/gui/themes/scummmodern/scummmodern_layout_lowres.stx
index 3da8a6c..506a975 100644
--- a/gui/themes/scummmodern/scummmodern_layout_lowres.stx
+++ b/gui/themes/scummmodern/scummmodern_layout_lowres.stx
@@ -330,14 +330,6 @@
 						type = 'PopUp'
 				/>
 			</layout>
-			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '6' center = 'true'>
-				<widget name = 'auSampleRatePopupDesc'
-						type = 'OptionsLabel'
-				/>
-				<widget name = 'auSampleRatePopup'
-						type = 'PopUp'
-				/>
-			</layout>
 			<layout type = 'horizontal' padding = '0, 0, 0, 0' spacing = '3' center = 'true'>
 				<widget name = 'subToggleDesc'
 						type = 'OptionsLabel'


Commit: 4a75fbab1b508f4240615c62811e2c8828534177
    https://github.com/scummvm/scummvm/commit/4a75fbab1b508f4240615c62811e2c8828534177
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-12T11:30:01-05:00

Commit Message:
SDL: Reduce audio playback latency

The previous default buffer size of 4096 samples for 44kHz mixer
would add up to 93ms of audio latency, which is fine for early
adventure games, but this is significantly more latency than is
acceptable for games with full motion video. For these games,
the latency needs to be kept within roughly +15ms and -45ms of
video frame presentation to avoid lip sync problems. With this
change, the default audio buffer size is calculated to be 1024
samples at 44kHz (which happens to match what DOSBox uses).

There is a possibility that the reduced latency may cause issues
that did not previously exist with things like the MT-32 emulator,
where a larger buffer size allowed for a larger window where
high-complexity synthesis that could not be generated in realtime
could be balanced out by low-complexity synthesis that could be
generated faster than realtime. In this case, rather than
increasing the system mixer buffer size again, please move the
MT-32 emulator into its own thread and give it its own larger ring
buffer into which it can generate more sample data in advance,
independently from the rest of the audio system.

For other systems where this buffer size reduction might cause a
problem with audio drop-outs, a new audio_buffer_size
configuration option has been added to allow users to tweak the
audio buffer size to match their machine's ability to generate
audio samples.

Fixes Trac#10033. Also improves playback of samples in SCI that
were programmed to restart across several consecutive frames,
relying on lower audio latency in the original engine for this to
not sound bad, like the hopping sound at the start of chapter 1
of KQ7, and the sound of turning on the power in the digger train
in the Lighthouse volcano.

Changed paths:
    backends/mixer/sdl/sdl-mixer.cpp


diff --git a/backends/mixer/sdl/sdl-mixer.cpp b/backends/mixer/sdl/sdl-mixer.cpp
index 5a31feb..11a45eb 100644
--- a/backends/mixer/sdl/sdl-mixer.cpp
+++ b/backends/mixer/sdl/sdl-mixer.cpp
@@ -125,6 +125,24 @@ void SdlMixerManager::init() {
 	startAudio();
 }
 
+static uint32 roundDownPowerOfTwo(uint32 samples) {
+	// Public domain code from Sean Eron Anderson
+	// http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2
+	uint32 rounded = samples;
+	--rounded;
+	rounded |= rounded >> 1;
+	rounded |= rounded >> 2;
+	rounded |= rounded >> 4;
+	rounded |= rounded >> 8;
+	rounded |= rounded >> 16;
+	++rounded;
+
+	if (rounded != samples)
+		rounded >>= 1;
+
+	return rounded;
+}
+
 SDL_AudioSpec SdlMixerManager::getAudioSpec(uint32 outputRate) {
 	SDL_AudioSpec desired;
 
@@ -139,19 +157,31 @@ SDL_AudioSpec SdlMixerManager::getAudioSpec(uint32 outputRate) {
 	if (freq <= 0)
 		freq = outputRate;
 
-	// Determine the sample buffer size. We want it to store enough data for
-	// at least 1/16th of a second (though at most 8192 samples). Note
-	// that it must be a power of two. So e.g. at 22050 Hz, we request a
-	// sample buffer size of 2048.
-	uint32 samples = 8192;
-	while (samples * 16 > freq * 2)
-		samples >>= 1;
+	// One SDL "sample" is a complete audio frame (i.e. all channels = 1 sample)
+	uint32 samples = 0;
+
+	// Different games and host systems have different performance
+	// characteristics which are not easily measured, so allow advanced users to
+	// tweak their audio buffer size if they are experience excess latency or
+	// drop-outs by setting this value in their ScummVM config file directly
+	if (ConfMan.hasKey("audio_buffer_size", appDomain))
+		samples = ConfMan.getInt("audio_buffer_size", appDomain);
+
+	// 256 is an arbitrary minimum; 32768 is the largest power-of-two value
+	// representable with uint16
+	if (samples < 256 || samples > 32768)
+		// By default, hold no more than 45ms worth of samples to avoid
+		// perceptable audio lag (ATSC IS-191). For reference, DOSBox (as of Sep
+		// 2017) uses a buffer size of 1024 samples by default for a 16-bit
+		// stereo 44kHz mixer, which happens to be the next lowest power of two
+		// below 45ms.
+		samples = freq / (1000.0 / 45);
 
 	memset(&desired, 0, sizeof(desired));
 	desired.freq = freq;
 	desired.format = AUDIO_S16SYS;
 	desired.channels = 2;
-	desired.samples = (uint16)samples;
+	desired.samples = roundDownPowerOfTwo(samples);
 	desired.callback = sdlCallback;
 	desired.userdata = this;
 


Commit: fa52df018ef44a30b45ceed31dffc25de3393e84
    https://github.com/scummvm/scummvm/commit/fa52df018ef44a30b45ceed31dffc25de3393e84
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-12T11:35:50-05:00

Commit Message:
SDL: Fix DoubleBufferSDLMixerManager doubling audio latency

If it turns out that everything that had previously been fixed by
this manager is broken by this change, everything that had been
fixed probably could have been fixed by just increasing the audio
buffer size in SdlMixerManager. :\

Changed paths:
    backends/mixer/doublebuffersdl/doublebuffersdl-mixer.cpp
    backends/mixer/doublebuffersdl/doublebuffersdl-mixer.h


diff --git a/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.cpp b/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.cpp
index e5f63dc..ea0ba02 100644
--- a/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.cpp
+++ b/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.cpp
@@ -62,6 +62,13 @@ void DoubleBufferSDLMixerManager::startAudio() {
 	SdlMixerManager::startAudio();
 }
 
+SDL_AudioSpec DoubleBufferSDLMixerManager::getAudioSpec(uint32 rate) {
+	SDL_AudioSpec desired = SdlMixerManager::getAudioSpec(rate);
+	// Don't double audio latency when double buffering
+	desired.samples /= 2;
+	return desired;
+}
+
 void DoubleBufferSDLMixerManager::mixerProducerThread() {
 	byte nextSoundBuffer;
 
diff --git a/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.h b/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.h
index e3019fe..e952cd2 100644
--- a/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.h
+++ b/backends/mixer/doublebuffersdl/doublebuffersdl-mixer.h
@@ -44,6 +44,8 @@ protected:
 	uint _soundBufSize;
 	byte *_soundBuffers[2];
 
+	virtual SDL_AudioSpec getAudioSpec(uint32 rate) override;
+
 	/**
 	 * Handles and swap the sound buffers
 	 */


Commit: bcbd4433594539cb0208809af12c6278632f8301
    https://github.com/scummvm/scummvm/commit/bcbd4433594539cb0208809af12c6278632f8301
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-12T11:35:51-05:00

Commit Message:
SDL: Stop using double buffering mixer on macOS

This mixer type was added in
943b4c2036002454b276e0190dfc2c8919fb0cbf because "anything which
produces sampled data with high latency (like the MT-32 emulator)
will sound terribly", but as far as I can see (or reproduce), this
mixer doesn't do anything that would solve that problem, except
that it effectively doubles the size of the audio buffer so there's
less chance of an underflow due to slower-than-realtime synthesis
by the softsynth. But you don't need the overhead of a separate
thread to do that, you just need to increase the buffer size.

Changed paths:
    backends/platform/sdl/macosx/macosx.cpp


diff --git a/backends/platform/sdl/macosx/macosx.cpp b/backends/platform/sdl/macosx/macosx.cpp
index 7e6ef95..457746c 100644
--- a/backends/platform/sdl/macosx/macosx.cpp
+++ b/backends/platform/sdl/macosx/macosx.cpp
@@ -64,7 +64,7 @@ void OSystem_MacOSX::init() {
 void OSystem_MacOSX::initBackend() {
 	// Create the mixer manager
 	if (_mixer == 0) {
-		_mixerManager = new DoubleBufferSDLMixerManager();
+		_mixerManager = new SdlMixerManager();
 
 		// Setup and start mixer
 		_mixerManager->init();





More information about the Scummvm-git-logs mailing list