valter/dosbox-staging
1
0
Fork 0
Code Releases Activity

Migrate some members of the Player class to smart pointers

Browse source 
1. Moves the mutex and mixer channel members to unique pointers
2. Moves the trackFile to a weak pointer
3. Move member initialization to the class definition

This class still retains a raw *cd member, however fixing this
ripples up to the array of [26] CD images, and across more
code that generically deals with mount points; so this work
remains to do.
This commit is contained in:
krcroft 2020-02-23 20:23:22 -08:00 • committed by Patryk Obara
parent 2d76132a8b
commit 91cd907c4e
4 changed files with 70 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
include/mixer.h
View file

@ -106,6 +106,18 @@ MixerChannel * MIXER_FindChannel(const char * name);
/* Find the device you want to delete with findchannel "delchan gets deleted" */
void MIXER_DelChannel(MixerChannel* delchan);
// A smart pointer deleter for MixerChannel objects
// Use-case:
// std::unique_ptr<MixerChannel, MixerChannelDeleter> myChannel;
// myChannel.reset(MIXER_AddChannel(...));
//
struct MixerChannelDeleter {
void operator()(MixerChannel *channel) {
if (channel)
MIXER_DelChannel(channel);
}
};
/* Object to maintain a mixerchannel; As all objects it registers itself with create
* and removes itself when destroyed. */
class MixerObject{

16
include/support.h
View file

@ -27,11 +27,27 @@
#include <string.h>
#include <string>
#include <SDL.h>
#ifdef _MSC_VER
#define strcasecmp(a, b) _stricmp(a, b)
#define strncasecmp(a, b, n) _strnicmp(a, b, n)
#endif
// A smart-pointer deleter for SDL-Mutex objects.
// Use-case example:
// std::unique_ptr<SDL_mutex, SdlMutexDeleter> myMutex;
// myMutex.reset(SDL_CreateMutex());
//
struct SdlMutexDeleter {
void operator()(SDL_mutex* mutex) {
if (mutex)
SDL_DestroyMutex(mutex);
}
};
// Returns the filename with the prior path stripped.
// Works with both \ and / directory delimeters.
std::string get_basename(const std::string& filename);
// Include a message in assert, similar to static_assert:

25
src/dos/cdrom.h
View file

@ -32,6 +32,7 @@
#include <SDL_thread.h>
#include "dosbox.h"
#include "support.h"
#include "mem.h"
#include "mixer.h"
#include "../libs/decoders/SDL_sound.h"
@ -220,18 +221,18 @@ public:
private:
static struct imagePlayer {
Bit16s buffer[MIXER_BUFSIZE * 2]; // 2 channels (max)
SDL_mutex *mutex;
TrackFile *trackFile;
MixerChannel *channel;
CDROM_Interface_Image *cd;
void (MixerChannel::*addFrames) (Bitu, const Bit16s*);
uint32_t startSector;
uint32_t totalRedbookFrames;
uint64_t playedTrackFrames;
uint64_t totalTrackFrames;
bool isPlaying;
bool isPaused;
Bit16s buffer[MIXER_BUFSIZE * 2] = {0};
std::unique_ptr<SDL_mutex, SdlMutexDeleter> mutex = nullptr;
std::unique_ptr<MixerChannel, MixerChannelDeleter> channel = nullptr;
std::weak_ptr<TrackFile> trackFile;
CDROM_Interface_Image *cd = nullptr;
void (MixerChannel::*addFrames) (Bitu, const Bit16s*) = nullptr;
uint64_t playedTrackFrames = 0;
uint64_t totalTrackFrames = 0;
uint32_t startSector = 0;
uint32_t totalRedbookFrames = 0;
bool isPlaying = false;
bool isPaused = false;
} player;
// Private utility functions

93
src/dos/cdrom_image.cpp
View file

@ -271,20 +271,7 @@ int CDROM_Interface_Image::AudioFile::getLength()
// initialize static members
int CDROM_Interface_Image::refCount = 0;
CDROM_Interface_Image* CDROM_Interface_Image::images[26] = {};
CDROM_Interface_Image::imagePlayer CDROM_Interface_Image::player = {
{0}, // buffer[]
nullptr, // SDL_Mutex*
nullptr, // TrackFile*
nullptr, // MixerChannel*
nullptr, // CDROM_Interface_Image*
nullptr, // addFrames
0, // startSector
0, // totalRedbookFrames
0, // playedTrackFrames
0, // totalTrackFrames
false, // isPlaying
false // isPaused
};
CDROM_Interface_Image::imagePlayer CDROM_Interface_Image::player;
CDROM_Interface_Image::CDROM_Interface_Image(Bit8u _subUnit)
: tracks({}),
@ -293,17 +280,13 @@ CDROM_Interface_Image::CDROM_Interface_Image(Bit8u _subUnit)
{
images[subUnit] = this;
if (refCount == 0) {
if (player.mutex == nullptr) {
player.mutex = SDL_CreateMutex();
}
if (player.channel == nullptr) {
// channel is kept dormant except during cdrom playback periods
player.channel = MIXER_AddChannel(&CDAudioCallBack, 0, "CDAUDIO");
player.channel->Enable(false);
player.mutex.reset(SDL_CreateMutex());
// channel is kept dormant except during cdrom playback periods
player.channel.reset(MIXER_AddChannel(&CDAudioCallBack, 0, "CDAUDIO"));
player.channel->Enable(false);
#ifdef DEBUG
LOG_MSG("CDROM: Initialized the CDAUDIO mixer channel and mutex");
#endif
}
}
refCount++;
}
@ -311,34 +294,18 @@ CDROM_Interface_Image::CDROM_Interface_Image(Bit8u _subUnit)
CDROM_Interface_Image::~CDROM_Interface_Image()
{
refCount--;
// Ensure the global player and mixer states are stopped
// before we drop our last CD drive.
if (refCount == 0 && player.cd) {
StopAudio();
}
if (player.cd == this) {
player.cd = nullptr;
}
// Empty our tracks and the now-invalid player pointer
tracks.clear();
player.trackFile = nullptr;
// Note: the player's channel and mutex objects are
// useable for all CDROM drives, therefore we leave
// them until we have no more CDROM drives left (when
// refCount falls to zero), at which point we finally
// purge them.
//
if (refCount == 0) {
StopAudio();
if (player.channel != nullptr) {
MIXER_DelChannel(player.channel);
player.channel = nullptr;
}
if (player.mutex != nullptr) {
SDL_DestroyMutex(player.mutex);
player.mutex = nullptr;
}
#ifdef DEBUG
LOG_MSG("CDROM: Released the CDAUDIO mixer channel and mutex");
LOG_MSG("CDROM: Released and cleared resources");
#endif
}
}
void CDROM_Interface_Image::InitNewMedia()
@ -445,8 +412,8 @@ bool CDROM_Interface_Image::GetAudioSub(unsigned char& attr,
if (!tracks.empty()) { // We have a useable CD; get a valid play-position
track_iter track = tracks.begin();
// the CD's current track is valid
if (player.trackFile) {
const uint32_t sample_rate = player.trackFile->getRate();
if (player.trackFile.lock()) {
const uint32_t sample_rate = player.trackFile.lock()->getRate();
const uint32_t played_frames = (player.playedTrackFrames
* REDBOOK_FRAMES_PER_SECOND
+ sample_rate - 1) / sample_rate;
@ -552,7 +519,7 @@ bool CDROM_Interface_Image::PlayAudioSector(uint64_t start, uint64_t len)
+ clamp(relative_start, 0, static_cast<int>(track->length - 1))
* track->sectorSize);
TrackFile* trackFile = track->file.get();
std::shared_ptr<TrackFile> trackFile(track->file);
// Guard: Bail if our track could not be seeked
if (!trackFile->seek(offset)) {
@ -571,7 +538,7 @@ bool CDROM_Interface_Image::PlayAudioSector(uint64_t start, uint64_t len)
// Before we update our player object with new track details, we lock access
// to it to prevent the Callback (which runs in a separate thread) from
// getting inconsistent or partial values.
if (SDL_LockMutex(player.mutex) < 0) {
if (SDL_LockMutex(player.mutex.get()) < 0) {
LOG_MSG("CDROM: PlayAudioSector couldn't lock our player for exclusive access");
StopAudio();
return false;
@ -632,7 +599,7 @@ bool CDROM_Interface_Image::PlayAudioSector(uint64_t start, uint64_t len)
player.channel->Enable(true);
// Guard: release the lock in this data
if (SDL_UnlockMutex(player.mutex) < 0) {
if (SDL_UnlockMutex(player.mutex.get()) < 0) {
LOG_MSG("CDROM: PlayAudioSector couldn't unlock this thread");
StopAudio();
return false;
@ -667,7 +634,7 @@ bool CDROM_Interface_Image::StopAudio(void)
void CDROM_Interface_Image::ChannelControl(TCtrl ctrl)
{
// Guard: Bail if our mixer channel hasn't been allocated
if (player.channel == nullptr) {
if (!player.channel) {
#ifdef DEBUG
LOG_MSG("CDROM: ChannelControl => game tried applying channel controls "
"before playing audio");
@ -811,34 +778,36 @@ void CDROM_Interface_Image::CDAudioCallBack(Bitu desired_track_frames)
if (desired_track_frames == 0
|| !player.cd
|| !player.mutex
|| !player.trackFile) {
|| player.trackFile.expired()) {
#ifdef DEBUG
LOG_MSG("CDROM: CDAudioCallBack called with one more empty dependencies:\n"
"\t - frames to play (%" PRIuPTR ")\n"
"\t - pointer to the CD object (%p)\n"
"\t - pointer to the mutex object (%p)\n"
"\t - pointer to the track's file (%p)\n",
"\t - pointer to the track's file (%s)\n",
desired_track_frames,
player.cd,
player.mutex,
player.trackFile);
player.mutex.get(),
player.trackFile.expired() ? "nullptr" : "valid" );
#endif
if (player.cd)
player.cd->StopAudio();
return;
}
// Ensure we have exclusive access to update our player members
if (SDL_LockMutex(player.mutex) < 0) {
if (SDL_LockMutex(player.mutex.get()) < 0) {
LOG_MSG("CDROM: CDAudioCallBack couldn't lock this thread");
return;
}
const uint64_t decoded_track_frames =
player.trackFile->decode(player.buffer, desired_track_frames);
player.trackFile.lock()->decode(player.buffer, desired_track_frames);
player.playedTrackFrames += decoded_track_frames;
// uses either the stereo or mono and native or
// nonnative AddSamples call assigned during construction
(player.channel->*player.addFrames)(decoded_track_frames, player.buffer);
(player.channel.get()->*player.addFrames)(decoded_track_frames, player.buffer);
if (player.playedTrackFrames >= player.totalTrackFrames) {
#ifdef DEBUG
@ -860,23 +829,21 @@ void CDROM_Interface_Image::CDAudioCallBack(Bitu desired_track_frames)
+ played_redbook_frames;
const Bit32u remaining_redbook_frames = player.totalRedbookFrames
- played_redbook_frames;
if (SDL_UnlockMutex(player.mutex) < 0) {
if (SDL_UnlockMutex(player.mutex.get()) < 0) {
LOG_MSG("CDROM: CDAudioCallBack couldn't unlock to move to next track");
return;
}
player.cd->PlayAudioSector(new_redbook_start_frame, remaining_redbook_frames);
return;
}
if (SDL_UnlockMutex(player.mutex) < 0) {
if (SDL_UnlockMutex(player.mutex.get()) < 0) {
LOG_MSG("CDROM: CDAudioCallBack couldn't unlock our player before returning");
}
}
bool CDROM_Interface_Image::LoadIsoFile(char* filename)
{
// Empty our tracks and the now-invalid player pointer
tracks.clear();
player.trackFile = nullptr;
// data track (track 1)
Track track;
@ -959,9 +926,7 @@ static string dirname(char * file) {
bool CDROM_Interface_Image::LoadCueSheet(char *cuefile)
{
// Empty our tracks and the now-invalid player pointer
tracks.clear();
player.trackFile = nullptr;
Track track;
int shift = 0;