From 914fb50397d4026d0dda216d0d6ee4c6de7089f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 15 Jun 2009 15:15:32 +0000 Subject: [PATCH] * Added a couple TODOs after reading a bit in the source. * Introduced a lock in GetDefaultDevice() and ReleaseDevice() as a quick solution to the race condition in those functions. It could also use proper atomic ref counting. Untested. * Automatic white space cleanup. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@31060 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/kits/game/FileGameSound.cpp | 35 ++++---- src/kits/game/GameSound.cpp | 49 ++++++----- src/kits/game/GameSoundDevice.cpp | 139 ++++++++++++++++-------------- 3 files changed, 122 insertions(+), 101 deletions(-) diff --git a/src/kits/game/FileGameSound.cpp b/src/kits/game/FileGameSound.cpp index 337230171a..1c0ba42c49 100644 --- a/src/kits/game/FileGameSound.cpp +++ b/src/kits/game/FileGameSound.cpp @@ -151,8 +151,13 @@ BFileGameSound::BFileGameSound(const char *file, BFileGameSound::~BFileGameSound() { - if (fReadThread >= 0) + if (fReadThread >= 0) { + // TODO: kill_thread() is very bad, since it will leak any resources + // that the thread had allocated. It will also keep locks locked that + // the thread holds! Set a flag to make the thread quit and use + // wait_for_thread() here! kill_thread(fReadThread); + } if (fAudioStream) { if (fAudioStream->stream) @@ -223,7 +228,7 @@ BFileGameSound::FillBuffer(void *inBuffer, // fPlayPosition is where we got up to in the input buffer after last call size_t out_offset = 0; - + while (inByteCount > 0 && !fPaused) { if (!fPaused || fPausing) { printf("mixout %ld, inByteCount %ld, decin %ld, BufferSize %ld\n",out_offset, inByteCount, fPlayPosition, fBufferSize); @@ -233,14 +238,14 @@ BFileGameSound::FillBuffer(void *inBuffer, if (fPausing) { Lock(); - + bool rampDone = false; size_t bytes = fBufferSize - fPlayPosition; - + if (bytes > inByteCount) { bytes = inByteCount; } - + // Fill the requested buffer, stopping if the paused flag is set char * buffer = (char*)inBuffer; @@ -248,24 +253,24 @@ BFileGameSound::FillBuffer(void *inBuffer, case gs_audio_format::B_GS_U8: rampDone = ::FillBuffer(fPausing, (uint8*)&buffer[out_offset], (uint8*)&fBuffer[fPlayPosition], &bytes); break; - + case gs_audio_format::B_GS_S16: rampDone = ::FillBuffer(fPausing, (int16*)&buffer[out_offset], (int16*)&fBuffer[fPlayPosition], &bytes); break; - + case gs_audio_format::B_GS_S32: rampDone = ::FillBuffer(fPausing, (int32*)&buffer[out_offset], (int32*)&fBuffer[fPlayPosition], &bytes); break; - + case gs_audio_format::B_GS_F: rampDone = ::FillBuffer(fPausing, (float*)&buffer[out_offset], (float*)&fBuffer[fPlayPosition], &bytes); break; } - + inByteCount -= bytes; out_offset += bytes; fPlayPosition += bytes; - + // We finished ramping if (rampDone) { @@ -274,14 +279,14 @@ BFileGameSound::FillBuffer(void *inBuffer, buffer[out_offset++] = fBuffer[fPlayPosition++]; inByteCount--; } - + delete fPausing; fPausing = NULL; } - + Unlock(); } else { - + char * buffer = (char*)inBuffer; // We need to be able to stop asap when the pause flag is flipped. @@ -410,7 +415,7 @@ BFileGameSound::Load() fAudioStream->stream->ReadFrames(fBuffer, &frames); fBufferSize = frames * fFrameSize; fPlayPosition = 0; - + if (fBufferSize <= 0) { // EOF if (fLooping) { @@ -421,7 +426,7 @@ BFileGameSound::Load() StopPlaying(); } } - + return true; } diff --git a/src/kits/game/GameSound.cpp b/src/kits/game/GameSound.cpp index b81b0d1c9a..b87ef3d0a6 100644 --- a/src/kits/game/GameSound.cpp +++ b/src/kits/game/GameSound.cpp @@ -1,8 +1,8 @@ -/* +/* * Copyright 2001-2002, Haiku Inc. * Authors: * Christopher ML Zumwalt May (zummy@users.sf.net) - * + * * Distributed under the terms of the MIT License. */ @@ -23,6 +23,9 @@ using std::nothrow; BGameSound::BGameSound(BGameSoundDevice *device) : fSound(-1) { + // TODO: device is ignored! + // NOTE: BeBook documents that BGameSoundDevice must currently always + // be NULL... fDevice = GetDefaultDevice(); fInitError = fDevice->InitCheck(); } @@ -32,9 +35,10 @@ BGameSound::BGameSound(const BGameSound &other) : fSound(-1) { memcpy(&fFormat, &other.fFormat, sizeof(gs_audio_format)); + // TODO: device from other is ignored! fDevice = GetDefaultDevice(); - - fInitError = fDevice->InitCheck(); + + fInitError = fDevice->InitCheck(); } @@ -42,7 +46,7 @@ BGameSound::~BGameSound() { if (fSound >= 0) fDevice->ReleaseBuffer(fSound); - + ReleaseDevice(); } @@ -57,6 +61,7 @@ BGameSound::InitCheck() const BGameSoundDevice * BGameSound::Device() const { + // TODO: Must return NULL if default device is being used! return fDevice; } @@ -64,6 +69,8 @@ BGameSound::Device() const gs_id BGameSound::ID() const { + // TODO: Should be 0 if no sound has been selected! But fSound + // is initialized with -1 in the constructors. return fSound; } @@ -103,13 +110,13 @@ BGameSound::SetGain(float gain, bigtime_t duration) { gs_attribute attribute; - + attribute.attribute = B_GS_GAIN; attribute.value = gain; attribute.duration = duration; attribute.flags = 0; - - return fDevice->SetAttributes(fSound, &attribute, 1); + + return fDevice->SetAttributes(fSound, &attribute, 1); } @@ -118,12 +125,12 @@ BGameSound::SetPan(float pan, bigtime_t duration) { gs_attribute attribute; - + attribute.attribute = B_GS_PAN; attribute.value = pan; attribute.duration = duration; attribute.flags = 0; - + return fDevice->SetAttributes(fSound, &attribute, 1); } @@ -132,13 +139,13 @@ float BGameSound::Gain() { gs_attribute attribute; - + attribute.attribute = B_GS_GAIN; attribute.flags = 0; - + if (fDevice->GetAttributes(fSound, &attribute, 1) != B_OK) return 0.0; - + return attribute.value; } @@ -147,13 +154,13 @@ float BGameSound::Pan() { gs_attribute attribute; - + attribute.attribute = B_GS_PAN; attribute.flags = 0; - + if (fDevice->GetAttributes(fSound, &attribute, 1) != B_OK) return 0.0; - + return attribute.value; } @@ -247,7 +254,7 @@ BGameSound::Init(gs_id handle) { if (fSound < 0) fSound = handle; - + return B_OK; } @@ -257,10 +264,12 @@ BGameSound::operator=(const BGameSound &other) { if (fSound) fDevice->ReleaseBuffer(fSound); - + fSound = other.fSound; fInitError = other.fInitError; - + + // TODO: This would need to acquire the sound another time! + return this; } */ @@ -269,7 +278,7 @@ BGameSound::operator=(const BGameSound &other) * * BGameSound::BGameSound() */ - + status_t BGameSound::_Reserved_BGameSound_0(int32 arg, ...) diff --git a/src/kits/game/GameSoundDevice.cpp b/src/kits/game/GameSoundDevice.cpp index 99a6be4c06..717d224440 100644 --- a/src/kits/game/GameSoundDevice.cpp +++ b/src/kits/game/GameSoundDevice.cpp @@ -22,7 +22,7 @@ // File Name: BGameSoundDevice.cpp // Author: Christopher ML Zumwalt May (zummy@users.sf.net) // Description: Manages the game producer. The class may change with out -// notice and was only inteneded for use by the GameKit at +// notice and was only inteneded for use by the GameKit at // this time. Use at your own risk. //------------------------------------------------------------------------------ @@ -30,7 +30,9 @@ #include #include +#include #include +#include #include #include #include @@ -47,14 +49,17 @@ const int32 kGrowth = 16; static int32 sDeviceCount = 0; static BGameSoundDevice* sDevice = NULL; +static BLocker sDeviceRefCountLock = BLocker("GameSound device lock"); BGameSoundDevice * GetDefaultDevice() { + BAutolock _(sDeviceRefCountLock); + if (!sDevice) sDevice = new BGameSoundDevice(); - + sDeviceCount++; return sDevice; } @@ -63,8 +68,10 @@ GetDefaultDevice() void ReleaseDevice() { + BAutolock _(sDeviceRefCountLock); + sDeviceCount--; - + if (sDeviceCount <= 0) { delete sDevice; sDevice = NULL; @@ -79,12 +86,12 @@ BGameSoundDevice::BGameSoundDevice() { fConnection = new Connection; memset(&fFormat, 0, sizeof(gs_audio_format)); - + fInitError = Connect(); - + fSounds = new GameSoundBuffer*[kInitSoundCount]; for (int32 i = 0; i < kInitSoundCount; i++) - fSounds[i] = NULL; + fSounds[i] = NULL; } @@ -98,23 +105,23 @@ BGameSoundDevice::~BGameSoundDevice() fSounds[i]->StopPlaying(); delete fSounds[i]; } - + if (fIsConnected) { // stop the nodes if they are running roster->StopNode(fConnection->producer, 0, true); // synchronous stop - + // Ordinarily we'd stop *all* of the nodes in the chain at this point. However, // one of the nodes is the System Mixer, and stopping the Mixer is a Bad Idea (tm). // So, we just disconnect from it, and release our references to the nodes that // we're using. We *are* supposed to do that even for global nodes like the Mixer. roster->Disconnect(fConnection->producer.node, fConnection->source, fConnection->consumer.node, fConnection->destination); - + roster->ReleaseNode(fConnection->producer); roster->ReleaseNode(fConnection->consumer); } - + delete[] fSounds; delete fConnection; } @@ -154,20 +161,20 @@ BGameSoundDevice::CreateBuffer(gs_id * sound, const void * data, int64 frames) { - if (frames <= 0 || !sound) + if (frames <= 0 || !sound) return B_BAD_VALUE; - - status_t err = B_MEDIA_TOO_MANY_BUFFERS; + + status_t err = B_MEDIA_TOO_MANY_BUFFERS; int32 position = AllocateSound(); - + if (position >= 0) { fSounds[position] = new SimpleSoundBuffer(format, data, frames); err = fSounds[position]->Connect(&fConnection->producer); - } + } - if (err == B_OK) + if (err == B_OK) *sound = gs_id(position + 1); - return err; + return err; } @@ -176,20 +183,20 @@ BGameSoundDevice::CreateBuffer(gs_id * sound, const void * object, const gs_audio_format * format) { - if (!object || !sound) + if (!object || !sound) return B_BAD_VALUE; - - status_t err = B_MEDIA_TOO_MANY_BUFFERS; + + status_t err = B_MEDIA_TOO_MANY_BUFFERS; int32 position = AllocateSound(); - + if (position >= 0) { fSounds[position] = new StreamingSoundBuffer(format, object); err = fSounds[position]->Connect(&fConnection->producer); - } + } - if (err == B_OK) + if (err == B_OK) *sound = gs_id(position+1); - return err; + return err; } @@ -203,12 +210,12 @@ BGameSoundDevice::ReleaseBuffer(gs_id sound) // We must stop playback befor destroying the sound or else // we may recieve fatel errors. fSounds[sound - 1]->StopPlaying(); - + delete fSounds[sound - 1]; fSounds[sound - 1] = NULL; - } + } } - + status_t BGameSoundDevice::Buffer(gs_id sound, gs_audio_format* format, void *& data) @@ -222,8 +229,8 @@ BGameSoundDevice::Buffer(gs_id sound, gs_audio_format* format, void *& data) memcpy(data, fSounds[sound-1]->Data(), format->buffer_size); } else data = NULL; - - return B_OK; + + return B_OK; } @@ -232,12 +239,12 @@ BGameSoundDevice::StartPlaying(gs_id sound) { if (sound <= 0) return B_BAD_VALUE; - + if (!fSounds[sound - 1]->IsPlaying()) { // tell the producer to start playing the sound return fSounds[sound - 1]->StartPlaying(); } - + fSounds[sound - 1]->Reset(); return EALREADY; } @@ -248,13 +255,13 @@ BGameSoundDevice::StopPlaying(gs_id sound) { if (sound <= 0) return B_BAD_VALUE; - + if (fSounds[sound - 1]->IsPlaying()) { // Tell the producer to stop play this sound - fSounds[sound - 1]->Reset(); + fSounds[sound - 1]->Reset(); return fSounds[sound - 1]->StopPlaying(); } - + return EALREADY; } @@ -265,7 +272,7 @@ BGameSoundDevice::IsPlaying(gs_id sound) if (sound <= 0) return false; return fSounds[sound - 1]->IsPlaying(); -} +} status_t @@ -273,76 +280,76 @@ BGameSoundDevice::GetAttributes(gs_id sound, gs_attribute * attributes, size_t attributeCount) { - if (!fSounds[sound - 1]) + if (!fSounds[sound - 1]) return B_ERROR; - - return fSounds[sound - 1]->GetAttributes(attributes, attributeCount); + + return fSounds[sound - 1]->GetAttributes(attributes, attributeCount); } - + status_t BGameSoundDevice::SetAttributes(gs_id sound, gs_attribute * attributes, size_t attributeCount) { - if (!fSounds[sound - 1]) + if (!fSounds[sound - 1]) return B_ERROR; - + return fSounds[sound - 1]->SetAttributes(attributes, attributeCount); -} +} status_t BGameSoundDevice::Connect() { BMediaRoster* roster = BMediaRoster::Roster(); - + // create your own audio mixer // TODO: Don't do this!!! See bug #575 dormant_node_info mixer_dormant_info; int32 mixer_count = 1; // for now, we only care about the first we find. status_t err = roster->GetDormantNodes(&mixer_dormant_info, &mixer_count, 0, 0, 0, B_SYSTEM_MIXER, 0); - if (err != B_OK) + if (err != B_OK) return err; - + //fMixer = new media_node; err = roster->InstantiateDormantNode(mixer_dormant_info, &fConnection->producer); - if (err != B_OK) + if (err != B_OK) return err; - + // retieve the system's audio mixer err = roster->GetAudioMixer(&fConnection->consumer); - if (err != B_OK) + if (err != B_OK) return err; - + int32 count = 1; media_input mixerInput; err = roster->GetFreeInputsFor(fConnection->consumer, &mixerInput, 1, &count); - if (err != B_OK) + if (err != B_OK) return err; - + count = 1; media_output mixerOutput; err = roster->GetFreeOutputsFor(fConnection->producer, &mixerOutput, 1, &count); - if (err != B_OK) + if (err != B_OK) return err; - + media_format format(mixerOutput.format); err = roster->Connect(mixerOutput.source, mixerInput.destination, &format, &mixerOutput, &mixerInput); - if (err != B_OK) - return err; - + if (err != B_OK) + return err; + // set the producer's time source to be the "default" time source, which // the Mixer uses too. roster->GetTimeSource(&fConnection->timeSource); roster->SetTimeSourceFor(fConnection->producer.node, fConnection->timeSource.node); - - // Start our mixer's time source if need be. Chances are, it won't need to be, + + // Start our mixer's time source if need be. Chances are, it won't need to be, // but if we forget to do this, our mixer might not do anything at all. BTimeSource* mixerTimeSource = roster->MakeTimeSourceFor(fConnection->producer); - if (!mixerTimeSource) + if (!mixerTimeSource) return B_ERROR; if (!mixerTimeSource->IsRunning()) { @@ -357,9 +364,9 @@ BGameSoundDevice::Connect() bigtime_t tpNow = mixerTimeSource->Now(); err = roster->StartNode(fConnection->producer, tpNow + 10000); mixerTimeSource->Release(); - if (err != B_OK) + if (err != B_OK) return err; - + // the inputs and outputs might have been reassigned during the // nodes' negotiation of the Connect(). That's why we wait until // after Connect() finishes to save their contents. @@ -382,20 +389,20 @@ BGameSoundDevice::AllocateSound() for (int32 i = 0; i < fSoundCount; i++) if (!fSounds[i]) return i; - + // we need to allocate new space for the sound GameSoundBuffer ** sounds = new GameSoundBuffer*[fSoundCount + kGrowth]; for (int32 i = 0; i < fSoundCount; i++) sounds[i] = fSounds[i]; - + for (int32 i = fSoundCount; i < fSoundCount + kGrowth; i++) sounds[i] = NULL; - - // replace the old list + + // replace the old list delete [] fSounds; fSounds = sounds; fSoundCount += kGrowth; - + return fSoundCount - kGrowth; }