From 93c8044f690306e23b5b3eb621207fbf57682c40 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Fri, 6 Oct 2017 11:24:08 -0500 Subject: SCI32: Clean up Audio32 * Rewrap comments to 80 columns * Remove resolved TODOs * Use containers and smart pointers where appropriate --- engines/sci/engine/ksound.cpp | 24 +--- engines/sci/sound/audio32.cpp | 281 ++++++++++++++++++----------------------- engines/sci/sound/audio32.h | 231 ++++++++++++++++----------------- engines/sci/sound/soundcmd.cpp | 8 +- 4 files changed, 242 insertions(+), 302 deletions(-) diff --git a/engines/sci/engine/ksound.cpp b/engines/sci/engine/ksound.cpp index bbfadf1c53..4d171bdd1c 100644 --- a/engines/sci/engine/ksound.cpp +++ b/engines/sci/engine/ksound.cpp @@ -384,10 +384,6 @@ reg_t kDoAudioPosition(EngineState *s, int argc, reg_t *argv) { } reg_t kDoAudioRate(EngineState *s, int argc, reg_t *argv) { - // NOTE: In the original engine this would set the hardware - // DSP sampling rate; ScummVM mixer does not need this, so - // we only store the value to satisfy engine compatibility. - if (argc > 0) { const uint16 sampleRate = argv[0].toUint16(); if (sampleRate != 0) { @@ -407,10 +403,6 @@ reg_t kDoAudioGetCapability(EngineState *s, int argc, reg_t *argv) { } reg_t kDoAudioBitDepth(EngineState *s, int argc, reg_t *argv) { - // NOTE: In the original engine this would set the hardware - // DSP bit depth; ScummVM mixer does not need this, so - // we only store the value to satisfy engine compatibility. - if (argc > 0) { const uint16 bitDepth = argv[0].toUint16(); if (bitDepth != 0) { @@ -426,10 +418,6 @@ reg_t kDoAudioMixing(EngineState *s, int argc, reg_t *argv) { } reg_t kDoAudioChannels(EngineState *s, int argc, reg_t *argv) { - // NOTE: In the original engine this would set the hardware - // DSP stereo output; ScummVM mixer does not need this, so - // we only store the value to satisfy engine compatibility. - if (argc > 0) { const int16 numChannels = argv[0].toSint16(); if (numChannels != 0) { @@ -441,11 +429,6 @@ reg_t kDoAudioChannels(EngineState *s, int argc, reg_t *argv) { } reg_t kDoAudioPreload(EngineState *s, int argc, reg_t *argv) { - // NOTE: In the original engine this would cause audio - // data for new channels to be preloaded to memory when - // the channel was initialized; we do not need this, so - // we only store the value to satisfy engine compatibility. - if (argc > 0) { g_sci->_audio32->setPreload(argv[0].toUint16()); } @@ -477,11 +460,8 @@ reg_t kDoAudioPanOff(EngineState *s, int argc, reg_t *argv) { } reg_t kSetLanguage(EngineState *s, int argc, reg_t *argv) { - // This is used by script 90 of MUMG Deluxe from the main menu to toggle - // the audio language between English and Spanish. - // Basically, it instructs the interpreter to switch the audio resources - // (resource.aud and associated map files) and load them from the "Spanish" - // subdirectory instead. + // Used by script 90 of MUMG Deluxe from the main menu to toggle between + // English and Spanish. const Common::String audioDirectory = s->_segMan->getString(argv[0]); g_sci->getResMan()->changeAudioDirectory(audioDirectory); return s->r_acc; diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp index 59ebc86bf3..f9a0140323 100644 --- a/engines/sci/sound/audio32.cpp +++ b/engines/sci/sound/audio32.cpp @@ -153,6 +153,7 @@ Audio32::Audio32(ResourceManager *resMan) : _handle(), _mutex(), + _channels(getSciVersion() < SCI_VERSION_2_1_EARLY ? 10 : getSciVersion() < SCI_VERSION_3 ? 5 : 8), _numActiveChannels(0), _inAudioThread(false), @@ -170,22 +171,10 @@ Audio32::Audio32(ResourceManager *resMan) : _startedAtTick(0), _attenuatedMixing(true), + _useModifiedAttenuation(g_sci->_features->usesModifiedAudioAttenuation()), _monitoredChannelIndex(-1), - _monitoredBuffer(nullptr), - _monitoredBufferSize(0), _numMonitoredSamples(0) { - - if (getSciVersion() < SCI_VERSION_2_1_EARLY) { - _channels.resize(10); - } else if (getSciVersion() < SCI_VERSION_3) { - _channels.resize(5); - } else { - _channels.resize(8); - } - - _useModifiedAttenuation = g_sci->_features->usesModifiedAudioAttenuation(); - // In games where scripts premultiply master audio volumes into the volumes // of the individual audio channels sent to the mixer, Audio32 needs to use // the kPlainSoundType so that the master SFX volume is not applied twice. @@ -202,15 +191,14 @@ Audio32::Audio32(ResourceManager *resMan) : Audio32::~Audio32() { stop(kAllChannels); _mixer->stopHandle(_handle); - free(_monitoredBuffer); } #pragma mark - #pragma mark AudioStream implementation -int Audio32::writeAudioInternal(Audio::AudioStream *const sourceStream, Audio::RateConverter *const converter, Audio::st_sample_t *targetBuffer, const int numSamples, const Audio::st_volume_t leftVolume, const Audio::st_volume_t rightVolume) { +int Audio32::writeAudioInternal(Audio::AudioStream &sourceStream, Audio::RateConverter &converter, Audio::st_sample_t *targetBuffer, const int numSamples, const Audio::st_volume_t leftVolume, const Audio::st_volume_t rightVolume) { const int samplePairsToRead = numSamples >> 1; - const int samplePairsWritten = converter->flow(*sourceStream, targetBuffer, samplePairsToRead, leftVolume, rightVolume); + const int samplePairsWritten = converter.flow(sourceStream, targetBuffer, samplePairsToRead, leftVolume, rightVolume); return samplePairsWritten << 1; } @@ -243,34 +231,28 @@ bool Audio32::channelShouldMix(const AudioChannel &channel) const { return true; } -// In earlier versions of SCI32 engine, audio mixing is -// split into three different functions. +// In earlier versions of SCI32 engine, audio mixing is split into three +// different functions. // -// The first function is called from the main game thread in -// AsyncEventCheck; later versions of SSCI also call it when -// getting the playback position. This function is -// responsible for cleaning up finished channels and -// filling active channel buffers with decompressed audio -// matching the hardware output audio format so they can -// just be copied into the main DAC buffer directly later. +// The first function is called from the main game thread in AsyncEventCheck; +// later versions of SSCI also call it when getting the playback position. This +// function is responsible for cleaning up finished channels and filling active +// channel buffers with decompressed audio matching the hardware output audio +// format so they can just be copied into the main DAC buffer directly later. // -// The second function is called by the audio hardware when -// the DAC buffer needs to be filled, and by `play` when -// there is only one active sample (so it can just blow away -// whatever was already in the DAC buffer). It merges all -// active channels into the DAC buffer and then updates the -// offset into the DAC buffer. +// The second function is called by the audio hardware when the DAC buffer needs +// to be filled, and by `play` when there is only one active sample (so it can +// just blow away whatever was already in the DAC buffer). It merges all active +// channels into the DAC buffer and then updates the offset into the DAC buffer. // -// Finally, a third function is called by the second -// function, and it actually puts data into the DAC buffer, -// performing volume, distortion, and balance adjustments. +// Finally, a third function is called by the second function, and it actually +// puts data into the DAC buffer, performing volume, distortion, and balance +// adjustments. // -// Since we only have one callback from the audio thread, -// and should be able to do all audio processing in -// real time, and we have streams, and we do not need to -// completely fill the audio buffer, the functionality of -// all these original functions is combined here and -// simplified. +// Since we only have one callback from the audio thread, and should be able to +// do all audio processing in real time, and we have streams, and we do not need +// to completely fill the audio buffer, the functionality of all these original +// functions is combined here and simplified. int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) { Common::StackLock lock(_mutex); @@ -278,20 +260,17 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) return 0; } - // ResourceManager is not thread-safe so we need to - // avoid calling into it from the audio thread, but at - // the same time we need to be able to clear out any - // finished channels on a regular basis + // ResourceManager is not thread-safe so we need to avoid calling into it + // from the audio thread, but at the same time we need to be able to clear + // out any finished channels on a regular basis _inAudioThread = true; freeUnusedChannels(); const bool playOnlyMonitoredChannel = getSciVersion() != SCI_VERSION_3 && _monitoredChannelIndex != -1; - // The caller of `readBuffer` is a rate converter, - // which reuses (without clearing) an intermediate - // buffer, so we need to zero the intermediate buffer - // to prevent mixing into audio data from the last - // callback. + // The caller of `readBuffer` is a rate converter, which reuses (without + // clearing) an intermediate buffer, so we need to zero the intermediate + // buffer to prevent mixing into audio data from the last callback. memset(buffer, 0, numSamples * sizeof(Audio::st_sample_t)); // This emulates the attenuated mixing mode of SSCI engine, which reduces @@ -340,8 +319,8 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) continue; } - // Channel finished fading and had the - // stopChannelOnFade flag set, so no longer exists + // Channel finished fading and had the stopChannelOnFade flag set, so no + // longer exists if (channel.fadeStartTick && processFade(channelIndex)) { --channelIndex; continue; @@ -352,10 +331,10 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) if (channel.pan == -1 || !isStereo()) { int volume = channel.volume; if (getSciVersion() == SCI_VERSION_2) { - // NOTE: In SSCI, audio is decompressed into a temporary - // buffer, then the samples in that buffer are looped over, - // shifting each sample right 3, 2, or 1 bits to reduce the - // volume. + // In SSCI, audio is decompressed into a temporary buffer, then + // the samples in that buffer are looped over, shifting each + // sample right 3, 2, or 1 bits to reduce the volume within the + // ranges given here if (volume > 0 && volume <= 42) { volume = 15; } else if (volume > 42 && volume <= 84) { @@ -367,20 +346,18 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) // In SCI3, granularity of the non-maximum volumes is 1/32 volume &= ~4; - // NOTE: In the SSCI DOS interpreter, non-maximum volumes are - // divided by 8 which puts them in a range of [0, 16). That - // reduced volume range gets passed into a volume function which - // expects values [0, 32). So, effectively, all non-maximum - // volumes are half-volume in DOS in SCI3. In Windows, volumes - // [120, 124) are the same as 127 due to a programming bug. - // We do not emulate either of these incorrect behaviors. + // In the SSCI DOS interpreter, non-maximum volumes are divided + // by 8 which puts them in a range of [0, 16). That reduced + // volume range gets passed into a volume function which expects + // values [0, 32). So, effectively, all non-maximum volumes are + // half-volume in DOS in SCI3 due to a programming bug. In + // Windows, volumes [120, 124) are the same as 127 due to + // another programming bug. We do not emulate either of these + // incorrect behaviors. } leftVolume = rightVolume = volume * Audio::Mixer::kMaxChannelVolume / kMaxVolume; } else { - // TODO: This should match the SCI3 algorithm, - // which seems to halve the volume of each - // channel when centered; is this intended? leftVolume = channel.volume * (100 - channel.pan) / 100 * Audio::Mixer::kMaxChannelVolume / kMaxVolume; rightVolume = channel.volume * channel.pan / 100 * Audio::Mixer::kMaxChannelVolume / kMaxVolume; } @@ -397,19 +374,15 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) } if (channelIndex == _monitoredChannelIndex) { - const size_t bufferSize = numSamples * sizeof(Audio::st_sample_t); - if (_monitoredBufferSize < bufferSize) { - _monitoredBuffer = (Audio::st_sample_t *)realloc(_monitoredBuffer, bufferSize); - _monitoredBufferSize = bufferSize; + if (numSamples > (int)_monitoredBuffer.size()) { + _monitoredBuffer.resize(numSamples); } + memset(_monitoredBuffer.data(), 0, _monitoredBuffer.size() * sizeof(Audio::st_sample_t)); + _numMonitoredSamples = writeAudioInternal(*channel.stream, *channel.converter, _monitoredBuffer.data(), numSamples, leftVolume, rightVolume); - memset(_monitoredBuffer, 0, _monitoredBufferSize); - - _numMonitoredSamples = writeAudioInternal(channel.stream, channel.converter, _monitoredBuffer, numSamples, leftVolume, rightVolume); - - Audio::st_sample_t *sourceBuffer = _monitoredBuffer; + Audio::st_sample_t *sourceBuffer = _monitoredBuffer.data(); Audio::st_sample_t *targetBuffer = buffer; - const Audio::st_sample_t *const end = _monitoredBuffer + _numMonitoredSamples; + const Audio::st_sample_t *const end = _monitoredBuffer.data() + _numMonitoredSamples; while (sourceBuffer != end) { Audio::clampedAdd(*targetBuffer++, *sourceBuffer++); } @@ -428,7 +401,7 @@ int Audio32::readBuffer(Audio::st_sample_t *const buffer, const int numSamples) leftVolume = rightVolume = 0; } - const int channelSamplesWritten = writeAudioInternal(channel.stream, channel.converter, buffer, numSamples, leftVolume, rightVolume); + const int channelSamplesWritten = writeAudioInternal(*channel.stream, *channel.converter, buffer, numSamples, leftVolume, rightVolume); if (channelSamplesWritten > maxSamplesWritten) { maxSamplesWritten = channelSamplesWritten; } @@ -458,10 +431,9 @@ uint8 Audio32::getNumUnlockedChannels() const { } int16 Audio32::findChannelByArgs(int argc, const reg_t *argv, const int startIndex, const reg_t soundNode) const { - // NOTE: argc/argv are already reduced by one in our engine because - // this call is always made from a subop, so no reduction for the - // subop is made in this function. SSCI takes extra steps to skip - // the subop argument. + // SSCI takes extra steps to skip the subop argument here, but argc/argv are + // already reduced by one in our engine by the kernel since these calls are + // always subops so we do not need to do anything extra argc -= startIndex; if (argc <= 0) { @@ -501,10 +473,10 @@ int16 Audio32::findChannelById(const ResourceId resourceId, const reg_t soundNod if (resourceId.getType() == kResourceTypeAudio) { for (int16 i = 0; i < _numActiveChannels; ++i) { - const AudioChannel channel = _channels[i]; + const AudioChannel &candidate = _channels[i]; if ( - channel.id == resourceId && - (soundNode.isNull() || soundNode == channel.soundNode) + candidate.id == resourceId && + (soundNode.isNull() || soundNode == candidate.soundNode) ) { return i; } @@ -553,7 +525,7 @@ void Audio32::freeUnusedChannels() { } void Audio32::freeChannel(const int16 channelIndex) { - // The original engine did this: + // SSCI did this: // 1. Unlock memory-cached resource, if one existed // 2. Close patched audio file descriptor, if one existed // 3. Free decompression memory buffer, if one existed @@ -563,14 +535,13 @@ void Audio32::freeChannel(const int16 channelIndex) { // Robots have no corresponding resource to free if (channel.robot) { - delete channel.stream; - channel.stream = nullptr; + channel.stream.reset(); channel.robot = false; } else { - // We cannot unlock resources from the audio thread - // because ResourceManager is not thread-safe; instead, - // we just record that the resource needs unlocking and - // unlock it whenever we are on the main thread again + // We cannot unlock resources from the audio thread because + // ResourceManager is not thread-safe; instead, we just record that the + // resource needs unlocking and unlock it whenever we are on the main + // thread again if (_inAudioThread) { _resourcesToUnlock.push_back(channel.resource); } else { @@ -578,12 +549,10 @@ void Audio32::freeChannel(const int16 channelIndex) { } channel.resource = nullptr; - delete channel.stream; - channel.stream = nullptr; + channel.stream.reset(); } - delete channel.converter; - channel.converter = nullptr; + channel.converter.reset(); if (_monitoredChannelIndex == channelIndex) { _monitoredChannelIndex = -1; @@ -677,13 +646,12 @@ bool Audio32::playRobotAudio(const RobotAudioStream::RobotAudioPacket &packet) { channel.pausedAtTick = 0; channel.soundNode = NULL_REG; channel.volume = kMaxVolume; - // TODO: SCI3 introduces stereo audio channel.pan = -1; - channel.converter = Audio::makeRateConverter(RobotAudioStream::kRobotSampleRate, getRate(), false); + channel.converter.reset(Audio::makeRateConverter(RobotAudioStream::kRobotSampleRate, getRate(), false)); // The RobotAudioStream buffer size is // ((bytesPerSample * channels * sampleRate * 2000ms) / 1000ms) & ~3 // where bytesPerSample = 2, channels = 1, and sampleRate = 22050 - channel.stream = new RobotAudioStream(88200); + channel.stream.reset(new RobotAudioStream(88200)); _robotAudioPaused = false; if (_numActiveChannels == 1) { @@ -691,7 +659,7 @@ bool Audio32::playRobotAudio(const RobotAudioStream::RobotAudioPacket &packet) { } } - return static_cast(channel.stream)->addPacket(packet); + return static_cast(channel.stream.get())->addPacket(packet); } bool Audio32::queryRobotAudio(RobotAudioStream::StreamState &status) const { @@ -703,7 +671,7 @@ bool Audio32::queryRobotAudio(RobotAudioStream::StreamState &status) const { return false; } - status = static_cast(getChannel(channelIndex).stream)->getStatus(); + status = static_cast(getChannel(channelIndex).stream.get())->getStatus(); return true; } @@ -715,7 +683,7 @@ bool Audio32::finishRobotAudio() { return false; } - static_cast(getChannel(channelIndex).stream)->finish(); + static_cast(getChannel(channelIndex).stream.get())->finish(); return true; } @@ -741,7 +709,7 @@ uint16 Audio32::play(int16 channelIndex, const ResourceId resourceId, const bool if (channelIndex != kNoExistingChannel) { AudioChannel &channel = getChannel(channelIndex); - MutableLoopAudioStream *stream = dynamic_cast(channel.stream); + MutableLoopAudioStream *stream = dynamic_cast(channel.stream.get()); if (stream == nullptr) { error("[Audio32::play]: Unable to cast stream for resource %s", resourceId.toString().c_str()); } @@ -760,43 +728,42 @@ uint16 Audio32::play(int16 channelIndex, const ResourceId resourceId, const bool return 0; } - // NOTE: SCI engine itself normally searches in this order: + // SSCI normally searches in this order: // // For Audio36: // - // 1. First, request a FD using Audio36 name and use it as the - // source FD for reading the audio resource data. - // 2a. If the returned FD is -1, or equals the audio map, or - // equals the audio bundle, try to get the offset of the - // data from the audio map, using the Audio36 name. + // 1. First, request a FD using Audio36 name and use it as the source FD for + // reading the audio resource data. + // 2a. If the returned FD is -1, or equals the audio map, or equals the + // audio bundle, try to get the offset of the data from the audio map, + // using the Audio36 name. // - // If the returned offset is -1, this is not a valid resource; - // return 0. Otherwise, set the read offset for the FD to the - // returned offset. - // 2b. Otherwise, use the FD as-is (it is a patch file), with zero - // offset, and record it separately so it can be closed later. + // If the returned offset is -1, this is not a valid resource; return 0. + // Otherwise, set the read offset for the FD to the returned offset. + // 2b. Otherwise, use the FD as-is (it is a patch file), with zero offset, + // and record it separately so it can be closed later. // // For plain audio: // - // 1. First, request an Audio resource from the resource cache. If - // one does not exist, make the same request for a Wave resource. - // 2a. If an audio resource was discovered, record its memory ID - // and clear the streaming FD - // 2b. Otherwise, request an Audio FD. If one does not exist, make - // the same request for a Wave FD. If neither exist, this is not - // a valid resource; return 0. Otherwise, use the returned FD as - // the streaming ID and set the memory ID to null. + // 1. First, request an Audio resource from the resource cache. If one does + // not exist, make the same request for a Wave resource. + // 2a. If an audio resource was discovered, record its memory ID and clear + // the streaming FD + // 2b. Otherwise, request an Audio FD. If one does not exist, make the same + // request for a Wave FD. If neither exist, this is not a valid + // resource; return 0. Otherwise, use the returned FD as the streaming + // ID and set the memory ID to null. // // Once these steps are complete, the audio engine either has a file - // descriptor + offset that it can use to read streamed audio, or it - // has a memory ID that it can use to read cached audio. + // descriptor + offset that it can use to read streamed audio, or it has a + // memory ID that it can use to read cached audio. // - // Here in ScummVM we just ask the resource manager to give us the - // resource and we get a seekable stream. + // Here in ScummVM we just ask the resource manager to give us the resource + // and we get a seekable stream. - // TODO: This should be fixed to use streaming, which means - // fixing the resource manager to allow streaming, which means - // probably rewriting a bunch of the resource manager. + // TODO: This should be fixed to use streaming, which means fixing the + // resource manager to allow streaming, which means probably rewriting a + // bunch of the resource manager. Resource *resource = _resMan->findResource(resourceId, true); if (resource == nullptr) { warning("[Audio32::play]: %s could not be found", resourceId.toString().c_str()); @@ -812,7 +779,6 @@ uint16 Audio32::play(int16 channelIndex, const ResourceId resourceId, const bool channel.fadeStartTick = 0; channel.soundNode = soundNode; channel.volume = volume < 0 || volume > kMaxVolume ? (int)kMaxVolume : volume; - // TODO: SCI3 introduces stereo audio channel.pan = -1; if (monitor) { @@ -842,18 +808,17 @@ uint16 Audio32::play(int16 channelIndex, const ResourceId resourceId, const bool audioStream = Audio::makeRawStream(dataStream, _globalSampleRate, flags, DisposeAfterUse::YES); } - channel.stream = new MutableLoopAudioStream(audioStream, loop); - channel.converter = Audio::makeRateConverter(channel.stream->getRate(), getRate(), channel.stream->isStereo(), false); + channel.stream.reset(new MutableLoopAudioStream(audioStream, loop)); + channel.converter.reset(Audio::makeRateConverter(channel.stream->getRate(), getRate(), channel.stream->isStereo(), false)); - // NOTE: SCI engine sets up a decompression buffer here for the audio - // stream, plus writes information about the sample to the channel to - // convert to the correct hardware output format, and allocates the - // monitoring buffer to match the bitrate/samplerate/channels of the - // original stream. We do not need to do any of these things since we - // use audio streams, and allocate and fill the monitoring buffer - // when reading audio data from the stream. + // SSCI sets up a decompression buffer here for the audio stream, plus + // writes information about the sample to the channel to convert to the + // correct hardware output format, and allocates the monitoring buffer to + // match the bitrate/samplerate/channels of the original stream. We do not + // need to do any of these things since we use audio streams, and allocate + // and fill the monitoring buffer when reading audio data from the stream. - MutableLoopAudioStream *stream = dynamic_cast(channel.stream); + MutableLoopAudioStream *stream = dynamic_cast(channel.stream.get()); if (stream == nullptr) { error("[Audio32::play]: Unable to cast stream for resource %s", resourceId.toString().c_str()); } @@ -958,8 +923,8 @@ bool Audio32::pause(const int16 channelIndex) { } } - // NOTE: The actual engine returns false here regardless of whether - // or not channels were paused + // SSCI returns false here regardless of whether or not channels were + // paused, so we emulate this behaviour } else { AudioChannel &channel = getChannel(channelIndex); @@ -996,9 +961,10 @@ int16 Audio32::stop(const int16 channelIndex) { } } - // NOTE: SSCI stops the DSP interrupt and frees the - // global decompression buffer here if there are no - // more active channels + // SSCI stops the DSP interrupt and frees the global decompression buffer + // here if there are no more active channels, which we do not need to do + // since the system manages audio callbacks and we have no static + // decompression buffer return oldNumChannels; } @@ -1015,13 +981,14 @@ int16 Audio32::getPosition(const int16 channelIndex) const { return -1; } - // NOTE: SSCI treats this as an unsigned short except for - // when the value is 65535, then it treats it as signed + // SSCI treats this as an unsigned short except for when the value is 65535, + // then it treats it as signed int position = -1; const uint32 now = g_sci->getTickCount(); - // NOTE: The original engine also queried the audio driver to see whether - // it thought that there was audio playback occurring via driver opcode 9 + // SSCI also queried the audio driver to see whether it thought that there + // was audio playback occurring via driver opcode 9, but we have no analogue + // to query if (channelIndex == kAllChannels) { if (_pausedAtTick) { position = _pausedAtTick - _startedAtTick; @@ -1052,7 +1019,7 @@ void Audio32::setLoop(const int16 channelIndex, const bool loop) { AudioChannel &channel = getChannel(channelIndex); - MutableLoopAudioStream *stream = dynamic_cast(channel.stream); + MutableLoopAudioStream *stream = dynamic_cast(channel.stream.get()); assert(stream); stream->loop() = loop; } @@ -1161,8 +1128,8 @@ bool Audio32::hasSignal() const { return false; } - const Audio::st_sample_t *buffer = _monitoredBuffer; - const Audio::st_sample_t *const end = _monitoredBuffer + _numMonitoredSamples; + const Audio::st_sample_t *buffer = _monitoredBuffer.data(); + const Audio::st_sample_t *const end = _monitoredBuffer.data() + _numMonitoredSamples; while (buffer != end) { const Audio::st_sample_t sample = *buffer++; @@ -1193,9 +1160,9 @@ reg_t Audio32::kernelPlay(const bool autoPlay, const int argc, const reg_t *cons if (argc < 6 || argv[5].toSint16() == 1) { loop = false; } else { - // NOTE: Uses -1 for infinite loop. Presumably the - // engine was supposed to allow counter loops at one - // point, but ended up only using loop as a boolean. + // SSCI uses -1 for infinite loop. Presumably the engine was + // supposed to allow counter loops at one point, but ended up only + // using loop as a boolean. loop = (bool)argv[5].toSint16(); } @@ -1311,8 +1278,10 @@ reg_t Audio32::kernelFade(const int argc, const reg_t *const argv) { Common::StackLock lock(_mutex); - // NOTE: In SSCI, this call to find the channel is hacked up; argc is - // set to 2 before the call, and then restored after the call. + // In SSCI, this call to find the channel is hacked up; argc is set to 2 + // before the call, and then restored after the call. We just implemented + // findChannelByArgs in a manner that allows us to pass this information + // without messing with argc/argv instead const int16 channelIndex = findChannelByArgs(2, argv, 0, argc > 5 ? argv[5] : NULL_REG); const int16 volume = argv[1].toSint16(); const int16 speed = argv[2].toSint16(); @@ -1359,7 +1328,7 @@ void Audio32::printAudioList(Console *con) const { Common::StackLock lock(_mutex); for (int i = 0; i < _numActiveChannels; ++i) { const AudioChannel &channel = _channels[i]; - const MutableLoopAudioStream *stream = dynamic_cast(channel.stream); + const MutableLoopAudioStream *stream = dynamic_cast(channel.stream.get()); con->debugPrintf(" %d[%04x:%04x]: %s, started at %d, pos %d/%d, vol %d, pan %d%s%s\n", i, PRINT_REG(channel.soundNode), diff --git a/engines/sci/sound/audio32.h b/engines/sci/sound/audio32.h index 71f5883541..a0167f3d2d 100644 --- a/engines/sci/sound/audio32.h +++ b/engines/sci/sound/audio32.h @@ -50,22 +50,23 @@ struct AudioChannel { ResourceId id; /** - * The resource loaded into this channel. + * The resource loaded into this channel. The resource is owned by + * ResourceManager. */ Resource *resource; /** - * The audio stream loaded into this channel. Can cast - * to `SeekableAudioStream` for normal channels and - * `RobotAudioStream` for robot channels. + * The audio stream loaded into this channel. Can cast to + * `SeekableAudioStream` for normal channels and `RobotAudioStream` for + * robot channels. */ - Audio::AudioStream *stream; + Common::ScopedPtr stream; /** - * The converter used to transform and merge the input - * stream into the mixer's output buffer. + * The converter used to transform and merge the input stream into the + * mixer's output buffer. */ - Audio::RateConverter *converter; + Common::ScopedPtr converter; /** * Duration of the channel, in ticks. @@ -83,8 +84,8 @@ struct AudioChannel { uint32 pausedAtTick; /** - * The time, in ticks, that the channel fade began. - * If 0, the channel is not being faded. + * The time, in ticks, that the channel fade began. If 0, the channel is not + * being faded. */ uint32 fadeStartTick; @@ -104,20 +105,19 @@ struct AudioChannel { int fadeTargetVolume; /** - * Whether or not the channel should be stopped and - * freed when the fade is complete. + * Whether or not the channel should be stopped and freed when the fade is + * complete. */ bool stopChannelOnFade; /** - * Whether or not this channel contains a Robot - * audio block. + * Whether or not this channel contains a Robot audio block. */ bool robot; /** - * For digital sound effects, the related VM - * Sound::nodePtr object for the sound. + * For digital sound effects, the related VM Sound::nodePtr object for the + * sound. */ reg_t soundNode; @@ -127,17 +127,37 @@ struct AudioChannel { int volume; /** - * The amount to pan to the right, from 0 to 100. - * 50 is centered, -1 is not panned. + * The amount to pan to the right, from 0 to 100. 50 is centered, -1 is not + * panned. */ int pan; + + AudioChannel &operator=(AudioChannel &other) { + id = other.id; + resource = other.resource; + stream.reset(other.stream.release()); + converter.reset(other.converter.release()); + duration = other.duration; + startedAtTick = other.startedAtTick; + pausedAtTick = other.pausedAtTick; + fadeStartTick = other.fadeStartTick; + fadeStartVolume = other.fadeStartVolume; + fadeDuration = other.fadeDuration; + fadeTargetVolume = other.fadeTargetVolume; + stopChannelOnFade = other.stopChannelOnFade; + robot = other.robot; + soundNode = other.soundNode; + volume = other.volume; + pan = other.pan; + return *this; + } }; #pragma mark - /** - * Special audio channel indexes used to select a channel - * for digital audio playback. + * Special audio channel indexes used to select a channel for digital audio + * playback. */ enum AudioChannelIndex { kRobotChannel = -3, @@ -146,10 +166,9 @@ enum AudioChannelIndex { }; /** - * Audio32 acts as a permanent audio stream into the system - * mixer and provides digital audio services for the SCI32 - * engine, since the system mixer does not support all the - * features of SCI. + * Audio32 acts as a permanent audio stream into the system mixer and provides + * digital audio services for the SCI32 engine, since the system mixer does not + * support all the features of SCI. */ class Audio32 : public Audio::AudioStream, public Common::Serializable { public: @@ -196,10 +215,10 @@ private: bool channelShouldMix(const AudioChannel &channel) const; /** - * Mixes audio from the given source stream into the - * target buffer using the given rate converter. + * Mixes audio from the given source stream into the target buffer using the + * given rate converter. */ - int writeAudioInternal(Audio::AudioStream *const sourceStream, Audio::RateConverter *const converter, Audio::st_sample_t *targetBuffer, const int numSamples, const Audio::st_volume_t leftVolume, const Audio::st_volume_t rightVolume); + int writeAudioInternal(Audio::AudioStream &sourceStream, Audio::RateConverter &converter, Audio::st_sample_t *targetBuffer, const int numSamples, const Audio::st_volume_t leftVolume, const Audio::st_volume_t rightVolume); #pragma mark - #pragma mark Channel management @@ -226,17 +245,15 @@ public: uint8 getNumUnlockedChannels() const; /** - * Finds a channel that is already configured for the - * given audio sample. + * Finds a channel that is already configured for the given audio sample. * - * @param startIndex The location of the audio resource - * information in the arguments list. + * @param startIndex The location of the audio resource information in the + * arguments list. */ int16 findChannelByArgs(int argc, const reg_t *argv, const int startIndex, const reg_t soundNode) const; /** - * Finds a channel that is already configured for the - * given audio sample. + * Finds a channel that is already configured for the given audio sample. */ int16 findChannelById(const ResourceId resourceId, const reg_t soundNode = NULL_REG) const; @@ -255,26 +272,24 @@ private: Common::Array _channels; /** - * The number of active audio channels in the mixer. - * Being active is not the same as playing; active - * channels may be paused. + * The number of active audio channels in the mixer. Being active is not the + * same as playing; active channels may be paused. */ uint8 _numActiveChannels; /** * Whether or not we are in the audio thread. * - * This flag is used instead of passing a parameter to - * `freeUnusedChannels` because a parameter would - * require forwarding through the public method `stop`, - * and there is not currently any reason for this - * implementation detail to be exposed. + * This flag is used instead of passing a parameter to `freeUnusedChannels` + * because a parameter would require forwarding through the public method + * `stop`, and there is not currently any reason for this implementation + * detail to be exposed. */ bool _inAudioThread; /** - * The list of resources from freed channels that need - * to be unlocked from the main thread. + * The list of resources from freed channels that need to be unlocked from + * the main thread. */ UnlockList _resourcesToUnlock; @@ -302,8 +317,7 @@ private: } /** - * Frees all non-looping channels that have reached the - * end of their stream. + * Frees all non-looping channels that have reached the end of their stream. */ void freeUnusedChannels(); @@ -313,8 +327,7 @@ private: void freeChannel(const int16 channelIndex); /** - * Unlocks all resources that were freed by the audio - * thread. + * Unlocks all resources that were freed by the audio thread. */ void unlockResources(); @@ -322,58 +335,58 @@ private: #pragma mark Script compatibility public: /** - * Gets the (fake) sample rate of the hardware DAC. - * For script compatibility only. + * Gets the (fake) sample rate of the hardware DAC. For script compatibility + * only. */ inline uint16 getSampleRate() const { return _globalSampleRate; } /** - * Sets the (fake) sample rate of the hardware DAC. - * For script compatibility only. + * Sets the (fake) sample rate of the hardware DAC. For script compatibility + * only. */ void setSampleRate(uint16 rate); /** - * Gets the (fake) bit depth of the hardware DAC. - * For script compatibility only. + * Gets the (fake) bit depth of the hardware DAC. For script compatibility + * only. */ inline uint8 getBitDepth() const { return _globalBitDepth; } /** - * Sets the (fake) sample rate of the hardware DAC. - * For script compatibility only. + * Sets the (fake) sample rate of the hardware DAC. For script compatibility + * only. */ void setBitDepth(uint8 depth); /** - * Gets the (fake) number of output (speaker) channels - * of the hardware DAC. For script compatibility only. + * Gets the (fake) number of output (speaker) channels of the hardware DAC. + * For script compatibility only. */ inline uint8 getNumOutputChannels() const { return _globalNumOutputChannels; } /** - * Sets the (fake) number of output (speaker) channels - * of the hardware DAC. For script compatibility only. + * Sets the (fake) number of output (speaker) channels of the hardware DAC. + * For script compatibility only. */ void setNumOutputChannels(int16 numChannels); /** - * Gets the (fake) number of preloaded channels. - * For script compatibility only. + * Gets the (fake) number of preloaded channels. For script compatibility + * only. */ inline uint8 getPreload() const { return _preload; } /** - * Sets the (fake) number of preloaded channels. - * For script compatibility only. + * Sets the (fake) number of preloaded channels. For script compatibility + * only. */ inline void setPreload(uint8 preload) { _preload = preload; @@ -381,49 +394,43 @@ public: private: /** - * The hardware DAC sample rate. Stored only for script - * compatibility. + * The hardware DAC sample rate. Stored only for script compatibility. */ uint16 _globalSampleRate; /** - * The maximum allowed sample rate of the system mixer. - * Stored only for script compatibility. + * The maximum allowed sample rate of the system mixer. Stored only for + * script compatibility. */ uint16 _maxAllowedSampleRate; /** - * The hardware DAC bit depth. Stored only for script - * compatibility. + * The hardware DAC bit depth. Stored only for script compatibility. */ uint8 _globalBitDepth; /** - * The maximum allowed bit depth of the system mixer. - * Stored only for script compatibility. + * The maximum allowed bit depth of the system mixer. Stored only for script + * compatibility. */ uint8 _maxAllowedBitDepth; /** - * The hardware DAC output (speaker) channel - * configuration. Stored only for script compatibility. + * The hardware DAC output (speaker) channel configuration. Stored only for + * script compatibility. */ uint8 _globalNumOutputChannels; /** - * The maximum allowed number of output (speaker) - * channels of the system mixer. Stored only for script - * compatibility. + * The maximum allowed number of output (speaker) channels of the system + * mixer. Stored only for script compatibility. */ uint8 _maxAllowedOutputChannels; /** - * The number of audio channels that should have their - * data preloaded into memory instead of streaming from - * disk. - * 1 = all channels, 2 = 2nd active channel and above, - * etc. - * Stored only for script compatibility. + * The number of audio channels that should have their data preloaded into + * memory instead of streaming from disk. 1 = all channels, 2 = 2nd active + * channel and above, etc. Stored only for script compatibility. */ uint8 _preload; @@ -442,8 +449,7 @@ private: int16 findRobotChannel() const; /** - * When true, channels marked as robot audio will not be - * played. + * When true, channels marked as robot audio will not be played. */ bool _robotAudioPaused; @@ -456,8 +462,8 @@ public: uint16 play(int16 channelIndex, const ResourceId resourceId, const bool autoPlay, const bool loop, const int16 volume, const reg_t soundNode, const bool monitor); /** - * Resumes playback of a paused audio channel, or of - * the entire audio player. + * Resumes playback of a paused audio channel, or of the entire audio + * player. */ bool resume(const int16 channelIndex); bool resume(const ResourceId resourceId, const reg_t soundNode = NULL_REG) { @@ -475,8 +481,7 @@ public: } /** - * Stops and unloads an audio channel, or the entire - * audio player. + * Stops and unloads an audio channel, or the entire audio player. */ int16 stop(const int16 channelIndex); int16 stop(const ResourceId resourceId, const reg_t soundNode = NULL_REG) { @@ -490,8 +495,7 @@ public: uint16 restart(const ResourceId resourceId, const bool autoPlay, const bool loop, const int16 volume, const reg_t soundNode, const bool monitor); /** - * Returns the playback position for the given channel - * number, in ticks. + * Returns the playback position for the given channel number, in ticks. */ int16 getPosition(const int16 channelIndex) const; int16 getPosition(const ResourceId resourceId, const reg_t soundNode = NULL_REG) { @@ -531,8 +535,8 @@ private: #pragma mark Effects public: /** - * Gets the volume for a given channel. Passing - * `kAllChannels` will get the global volume. + * Gets the volume for a given channel. Passing `kAllChannels` will get the + * global volume. */ int16 getVolume(const int16 channelIndex) const; int16 getVolume(const ResourceId resourceId, const reg_t soundNode) const { @@ -541,8 +545,8 @@ public: } /** - * Sets the volume of an audio channel. Passing - * `kAllChannels` will set the global volume. + * Sets the volume of an audio channel. Passing `kAllChannels` will set the + * global volume. */ void setVolume(const int16 channelIndex, int16 volume); void setVolume(const ResourceId resourceId, const reg_t soundNode, const int16 volume) { @@ -583,24 +587,21 @@ public: private: /** - * If true, audio will be mixed by reducing the target - * buffer by half every time a new channel is mixed in. - * The final channel is not attenuated. + * If true, audio will be mixed by reducing the target buffer by half every + * time a new channel is mixed in. The final channel is not attenuated. */ bool _attenuatedMixing; /** - * When true, a modified attenuation algorithm is used - * (`A/4 + B`) instead of standard linear attenuation - * (`A/2 + B/2`). + * When true, a modified attenuation algorithm is used (`A/4 + B`) instead + * of standard linear attenuation (`A/2 + B/2`). */ bool _useModifiedAttenuation; /** * Processes an audio fade for the given channel. * - * @returns true if the fade was completed and the - * channel was stopped. + * @returns true if the fade was completed and the channel was stopped. */ bool processFade(const int16 channelIndex); @@ -608,35 +609,27 @@ private: #pragma mark Signal monitoring public: /** - * Returns whether the currently monitored audio channel - * contains any signal within the next audio frame. + * Returns whether the currently monitored audio channel contains any signal + * within the next audio frame. */ bool hasSignal() const; private: /** - * The index of the channel being monitored for signal, - * or -1 if no channel is monitored. When a channel is - * monitored, it also causes the engine to play only the - * monitored channel. + * The index of the channel being monitored for signal, or -1 if no channel + * is monitored. When a channel is monitored, it also causes the engine to + * play only the monitored channel. */ int16 _monitoredChannelIndex; /** - * The data buffer holding decompressed audio data for - * the channel that will be monitored for an audio - * signal. - */ - Audio::st_sample_t *_monitoredBuffer; - - /** - * The size of the buffer, in bytes. + * The data buffer holding decompressed audio data for the channel that will + * be monitored for an audio signal. */ - size_t _monitoredBufferSize; + Common::Array _monitoredBuffer; /** - * The number of valid audio samples in the signal - * monitoring buffer. + * The number of valid audio samples in the signal monitoring buffer. */ int _numMonitoredSamples; diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp index 0bb295d41e..5a1c2c0825 100644 --- a/engines/sci/sound/soundcmd.cpp +++ b/engines/sci/sound/soundcmd.cpp @@ -350,11 +350,9 @@ reg_t SoundCommandParser::kDoSoundPause(EngineState *s, int argc, reg_t *argv) { } #ifdef ENABLE_SCI32 - // NOTE: The original engine also expected a global - // "kernel call" flag to be true in order to perform - // this action, but the architecture of the ScummVM - // implementation is so different that it doesn't - // matter here + // SSCI also expected a global "kernel call" flag to be true in order to + // perform this action, but the architecture of the ScummVM + // implementation is so different that it doesn't matter here if (_soundVersion >= SCI_VERSION_2_1_EARLY && musicSlot->isSample) { if (shouldPause) { g_sci->_audio32->pause(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj); -- cgit v1.2.3