[Scummvm-git-logs] scummvm master -> 23fc7f52e01af248f3cc1ae0bedb46c83eb8e2c9
sluicebox
22204938+sluicebox at users.noreply.github.com
Sun Apr 12 08:55:58 UTC 2020
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
23fc7f52e0 SCI: Fix SoundResource error handling, ctor, dtor
Commit: 23fc7f52e01af248f3cc1ae0bedb46c83eb8e2c9
https://github.com/scummvm/scummvm/commit/23fc7f52e01af248f3cc1ae0bedb46c83eb8e2c9
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2020-04-12T01:47:15-07:00
Commit Message:
SCI: Fix SoundResource error handling, ctor, dtor
Fixes several problems with the SoundResource class:
- Constructor doesn't fully initialize object if resource doesn't exist
- Destructor crashes if object not fully initialized
- Constructor has no mechanism to report failure
- Callers believe failure is reported by constructor returning null
- SoundCommandParser::initSoundResource attempts to pre-detect failure
insufficiently in the absence of a formal mechanism.
SoundResource now always fully initializes, the destructor no longer
accesses uninitialized memory, and an exists() method has been added
which callers now test.
SQ6 Mac can now progress past the main menu.
Changed paths:
engines/sci/console.cpp
engines/sci/engine/features.cpp
engines/sci/resource.h
engines/sci/resource_audio.cpp
engines/sci/sound/soundcmd.cpp
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 06241f5ad8..1ab65edb38 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -2705,24 +2705,22 @@ bool Console::cmdIsSample(int argc, const char **argv) {
return true;
}
- SoundResource *soundRes = new SoundResource(number, _engine->getResMan(), _engine->_features->detectDoSoundType());
+ SoundResource soundRes(number, _engine->getResMan(), _engine->_features->detectDoSoundType());
- if (!soundRes) {
+ if (!soundRes.exists()) {
debugPrintf("Not a sound resource!\n");
return true;
}
- SoundResource::Track *track = soundRes->getDigitalTrack();
+ SoundResource::Track *track = soundRes.getDigitalTrack();
if (!track || track->digitalChannelNr == -1) {
debugPrintf("Valid song, but not a sample.\n");
- delete soundRes;
return true;
}
debugPrintf("Sample size: %d, sample rate: %d, channels: %d, digital channel number: %d\n",
track->digitalSampleSize, track->digitalSampleRate, track->channelCount, track->digitalChannelNr);
- delete soundRes;
return true;
}
diff --git a/engines/sci/engine/features.cpp b/engines/sci/engine/features.cpp
index 980e417d7e..e76adeafa7 100644
--- a/engines/sci/engine/features.cpp
+++ b/engines/sci/engine/features.cpp
@@ -718,7 +718,7 @@ bool GameFeatures::generalMidiOnly() {
}
SoundResource sound(13, g_sci->getResMan(), detectDoSoundType());
- return (sound.getTrackByType(/* AdLib */ 0) == nullptr);
+ return (sound.exists() && sound.getTrackByType(/* AdLib */ 0) == nullptr);
}
default:
break;
diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index 783a574014..f76bdc36bd 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -684,12 +684,13 @@ public:
int getChannelFilterMask(int hardwareMask, bool wantsRhythm);
byte getInitialVoiceCount(byte channel);
byte getSoundPriority() const { return _soundPriority; }
+ bool exists() const { return _resource != nullptr; }
private:
SciVersion _soundVersion;
int _trackCount;
Track *_tracks;
- Resource *_innerResource;
+ Resource *_resource;
ResourceManager *_resMan;
byte _soundPriority;
};
diff --git a/engines/sci/resource_audio.cpp b/engines/sci/resource_audio.cpp
index 4bc417a97a..68d6ec5dd5 100644
--- a/engines/sci/resource_audio.cpp
+++ b/engines/sci/resource_audio.cpp
@@ -707,30 +707,25 @@ bool ResourceManager::isGMTrackIncluded() {
Common::List<ResourceId>::iterator itr = resources.begin();
int firstSongId = itr->getNumber();
- SoundResource *song1 = new SoundResource(firstSongId, this, soundVersion);
- if (!song1) {
+ SoundResource song1(firstSongId, this, soundVersion);
+ if (!song1.exists()) {
warning("ResourceManager::isGMTrackIncluded: track 1 not found");
return false;
}
- SoundResource::Track *gmTrack = song1->getTrackByType(0x07);
+ SoundResource::Track *gmTrack = song1.getTrackByType(0x07);
if (gmTrack)
result = true;
- delete song1;
-
return result;
}
-SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVersion soundVersion) : _resMan(resMan), _soundVersion(soundVersion) {
- Resource *resource = _resMan->findResource(ResourceId(kResourceTypeSound, resourceNr), true);
- int trackNr, channelNr;
- if (!resource)
+SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVersion soundVersion) :
+ _resMan(resMan), _soundVersion(soundVersion), _trackCount(0), _tracks(nullptr), _soundPriority(0xFF) {
+ _resource = _resMan->findResource(ResourceId(kResourceTypeSound, resourceNr), true);
+ if (!_resource)
return;
- _innerResource = resource;
- _soundPriority = 0xFF;
-
Channel *channel, *sampleChannel;
if (_soundVersion <= SCI_VERSION_0_LATE) {
@@ -741,17 +736,17 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
_tracks->type = 0; // Not used for SCI0
_tracks->channelCount = 1;
// Digital sample data included? -> Add an additional channel
- if (resource->getUint8At(0) == 2)
+ if (_resource->getUint8At(0) == 2)
_tracks->channelCount++;
// header information that can be passed to the SCI0 sound driver
- _tracks->header = resource->subspan(0, _soundVersion == SCI_VERSION_0_EARLY ? 0x11 : 0x21);
+ _tracks->header = _resource->subspan(0, _soundVersion == SCI_VERSION_0_EARLY ? 0x11 : 0x21);
_tracks->channels = new Channel[_tracks->channelCount];
channel = &_tracks->channels[0];
channel->flags |= 2; // don't remap (SCI0 doesn't have remapping)
if (_soundVersion == SCI_VERSION_0_EARLY) {
- channel->data = resource->subspan(0x11);
+ channel->data = _resource->subspan(0x11);
} else {
- channel->data = resource->subspan(0x21);
+ channel->data = _resource->subspan(0x21);
}
if (_tracks->channelCount == 2) {
// Digital sample data included
@@ -776,7 +771,7 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
sampleChannel->data += 44; // Skip over header
}
} else if (_soundVersion >= SCI_VERSION_1_EARLY && _soundVersion <= SCI_VERSION_2_1_MIDDLE) {
- SciSpan<const byte> data = *resource;
+ SciSpan<const byte> data = *_resource;
// Count # of tracks
_trackCount = 0;
while ((*data++) != 0xFF) {
@@ -786,11 +781,11 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
++data;
}
_tracks = new Track[_trackCount];
- data = *resource;
+ data = *_resource;
byte channelCount;
- for (trackNr = 0; trackNr < _trackCount; trackNr++) {
+ for (int trackNr = 0; trackNr < _trackCount; trackNr++) {
// Track info starts with track type:BYTE
// Then the channel information gets appended Unknown:WORD, ChannelOffset:WORD, ChannelSize:WORD
// 0xFF:BYTE as terminator to end that track and begin with another track type
@@ -813,12 +808,12 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
_tracks[trackNr].digitalSampleStart = 0;
_tracks[trackNr].digitalSampleEnd = 0;
if (_tracks[trackNr].type != 0xF0) { // Digital track marker - not supported currently
- channelNr = 0;
+ int channelNr = 0;
while (channelCount--) {
channel = &_tracks[trackNr].channels[channelNr];
const uint16 dataOffset = data.getUint16LEAt(2);
- if (dataOffset >= resource->size()) {
+ if (dataOffset >= _resource->size()) {
warning("Invalid offset inside sound resource %d: track %d, channel %d", resourceNr, trackNr, channelNr);
data += 6;
continue;
@@ -826,12 +821,12 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
uint16 size = data.getUint16LEAt(4);
- if ((uint32)dataOffset + size > resource->size()) {
+ if ((uint32)dataOffset + size > _resource->size()) {
warning("Invalid size inside sound resource %d: track %d, channel %d", resourceNr, trackNr, channelNr);
- size = resource->size() - dataOffset;
+ size = _resource->size() - dataOffset;
}
- channel->data = resource->subspan(dataOffset, size);
+ channel->data = _resource->subspan(dataOffset, size);
channel->curPos = 0;
channel->number = channel->data[0];
@@ -883,11 +878,15 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
}
SoundResource::~SoundResource() {
- for (int trackNr = 0; trackNr < _trackCount; trackNr++)
- delete[] _tracks[trackNr].channels;
- delete[] _tracks;
+ if (_tracks != nullptr) {
+ for (int trackNr = 0; trackNr < _trackCount; trackNr++)
+ delete[] _tracks[trackNr].channels;
+ delete[] _tracks;
+ }
- _resMan->unlockResource(_innerResource);
+ if (_resource != nullptr) {
+ _resMan->unlockResource(_resource);
+ }
}
#if 0
@@ -922,7 +921,7 @@ SoundResource::Track *SoundResource::getDigitalTrack() {
// Gets the filter mask for SCI0 sound resources
int SoundResource::getChannelFilterMask(int hardwareMask, bool wantsRhythm) {
- SciSpan<const byte> data = *_innerResource;
+ SciSpan<const byte> data = *_resource;
int channelMask = 0;
if (_soundVersion > SCI_VERSION_0_LATE)
@@ -990,7 +989,7 @@ byte SoundResource::getInitialVoiceCount(byte channel) {
return 0; // TODO
// Skip over digital sample flag
- SciSpan<const byte> data = _innerResource->subspan(1);
+ SciSpan<const byte> data = _resource->subspan(1);
if (_soundVersion == SCI_VERSION_0_EARLY)
return data[channel] >> 4;
diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index 2305b4ff93..8b5ed42c1c 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -91,10 +91,15 @@ int SoundCommandParser::getSoundResourceId(reg_t obj) {
}
void SoundCommandParser::initSoundResource(MusicEntry *newSound) {
- if (newSound->resourceId && _resMan->testResource(ResourceId(kResourceTypeSound, newSound->resourceId)))
+ if (newSound->resourceId) {
newSound->soundRes = new SoundResource(newSound->resourceId, _resMan, _soundVersion);
- else
- newSound->soundRes = 0;
+ if (!newSound->soundRes->exists()) {
+ delete newSound->soundRes;
+ newSound->soundRes = nullptr;
+ }
+ } else {
+ newSound->soundRes = nullptr;
+ }
// In SCI1.1 games, sound effects are started from here. If we can find
// a relevant audio resource, play it, otherwise switch to synthesized
More information about the Scummvm-git-logs
mailing list