From eaeb001b1709ea88d51179a53423fffdfdc81c37 Mon Sep 17 00:00:00 2001 From: krcroft Date: Wed, 20 Nov 2019 00:04:14 -0800 Subject: [PATCH] Play into subsequent track(s) if playback length exceeds the the current track Issue reported by Dagar and Pr3tty F1y, and confirmed as a bug by ripsaw8080. Thank you! This fixes the GoG release of Betrayal at Krondor which (either due to CD mastering issues or a faulty rip), requests playback of a given track at the tail end of the prior track. In debugging and performing this fix, many debug messages were improved as well as making some small small code adjustments, such as using iterators to point to individual tracks (track->attribute) instead of using the tracks array (tracks[track -1].attribute). --- configure.ac | 1 + include/setup.h | 2 +- include/support.h | 9 + src/dos/cdrom.h | 177 ++++---- src/dos/cdrom_image.cpp | 798 ++++++++++++++++++++-------------- src/dos/cdrom_ioctl_win32.cpp | 14 +- src/dos/drive_iso.cpp | 2 +- src/hardware/mixer.cpp | 7 - src/misc/support.cpp | 9 +- 9 files changed, 590 insertions(+), 429 deletions(-) diff --git a/configure.ac b/configure.ac index a2937d51..986743d4 100644 --- a/configure.ac +++ b/configure.ac @@ -574,6 +574,7 @@ case "$host" in *-*-cygwin* | *-*-mingw* | *-*-msys*) LIBS="$LIBS -lwinmm" AC_DEFINE(WIN32, 1, [Compiling on Windows]) + AC_DEFINE(NOMINMAX, 1, [Prevent from clobbering std::min and std::max]) AC_DEFINE(C_DIRECTSERIAL, 1, [ Define to 1 if you want serial passthrough support (Win32, Posix and OS/2 only).]) if test x$have_sdl_net_lib = xyes -a x$have_sdl_net_h = xyes ; then LIBS="$LIBS -lws2_32" diff --git a/include/setup.h b/include/setup.h index 566c08c4..417fc112 100644 --- a/include/setup.h +++ b/include/setup.h @@ -170,7 +170,7 @@ public: } int getMin() { return min;} int getMax() { return max;} - void SetMinMax(Value const& min,Value const& max) {this->min = min; this->max=max;} + void SetMinMax(Value const& _min,Value const& _max) {this->min = _min; this->max=_max;} bool SetValue(std::string const& in); ~Prop_int(){ } virtual bool CheckValue(Value const& in, bool warn); diff --git a/include/support.h b/include/support.h index bd21e654..b0d8f2d4 100644 --- a/include/support.h +++ b/include/support.h @@ -20,6 +20,7 @@ #ifndef DOSBOX_SUPPORT_H #define DOSBOX_SUPPORT_H +#include #include #include #include @@ -38,6 +39,14 @@ #include #endif +// Clamp: given a value that can be compared with the given minimum and maximum +// values, this function will: +// * return the value if it's in-between or equal to either bounds, or +// * return either bound depending on which bound the value is beyond +template T clamp(const T& n, const T& lower, const T& upper) { + return std::max(lower, std::min(n, upper)); +} + void strreplace(char * str,char o,char n); char *ltrim(char *str); char *rtrim(char *str); diff --git a/src/dos/cdrom.h b/src/dos/cdrom.h index 9f16c5e4..d2cddac2 100644 --- a/src/dos/cdrom.h +++ b/src/dos/cdrom.h @@ -37,10 +37,15 @@ #include "../libs/decoders/SDL_sound.h" // CDROM data and audio format constants -#define RAW_SECTOR_SIZE 2352 -#define COOKED_SECTOR_SIZE 2048 -#define BYTES_PER_TRACK_FRAME 4 -#define REDBOOK_FRAMES_PER_SECOND 75 +#define BYTES_PER_RAW_REDBOOK_FRAME 2352 +#define BYTES_PER_COOKED_REDBOOK_FRAME 2048 +#define REDBOOK_FRAMES_PER_SECOND 75 +#define MAX_REDBOOK_FRAMES 400000 // frames are Redbook's data unit +#define MAX_REDBOOK_SECTOR 399999 // a sector is the index to a frame +#define MAX_REDBOOK_TRACKS 99 +#define MIN_REDBOOK_TRACKS 2 // One track plus the lead-out track +#define REDBOOK_PCM_BYTES_PER_MS 176.4f // 44.1 frames/ms * 4 bytes/frame +#define BYTES_PER_REDBOOK_PCM_FRAME 4 // 2 bytes/sample * 2 samples/frame enum { CDROM_USE_SDL, CDROM_USE_ASPI, CDROM_USE_IOCTL_DIO, CDROM_USE_IOCTL_DX, CDROM_USE_IOCTL_MCI }; @@ -77,83 +82,78 @@ extern int CDROM_GetMountType(char* path, int force); class CDROM_Interface { public: - virtual ~CDROM_Interface (void) {}; - - virtual bool SetDevice (char* path, int forceCD) = 0; - - virtual bool GetUPC (unsigned char& attr, char* upc) = 0; - - virtual bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut) = 0; - virtual bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr) = 0; - virtual bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos) = 0; - virtual bool GetAudioStatus (bool& playing, bool& pause) = 0; - virtual bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen) = 0; - - virtual bool PlayAudioSector (unsigned long start,unsigned long len) = 0; - virtual bool PauseAudio (bool resume) = 0; - virtual bool StopAudio (void) = 0; - virtual void ChannelControl (TCtrl ctrl) = 0; - - virtual bool ReadSectors (PhysPt buffer, bool raw, unsigned long sector, unsigned long num) = 0; - - virtual bool LoadUnloadMedia (bool unload) = 0; - - virtual void InitNewMedia (void) {}; -}; + virtual ~CDROM_Interface (void) {}; + virtual bool SetDevice (char* path, int forceCD) = 0; + virtual bool GetUPC (unsigned char& attr, char* upc) = 0; + virtual bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut) = 0; + virtual bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr) = 0; + virtual bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos) = 0; + virtual bool GetAudioStatus (bool& playing, bool& pause) = 0; + virtual bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen) = 0; + virtual bool PlayAudioSector (unsigned long start,unsigned long len) = 0; + virtual bool PauseAudio (bool resume) = 0; + virtual bool StopAudio (void) = 0; + virtual void ChannelControl (TCtrl ctrl) = 0; + virtual bool ReadSectors (PhysPt buffer, bool raw, unsigned long sector, unsigned long num) = 0; + virtual bool LoadUnloadMedia (bool unload) = 0; + virtual void InitNewMedia (void) {}; +}; class CDROM_Interface_SDL : public CDROM_Interface { public: - CDROM_Interface_SDL (void); - virtual ~CDROM_Interface_SDL(void); - - virtual bool SetDevice (char* path, int forceCD); - virtual bool GetUPC (unsigned char& attr, char* upc) { attr = 0; strcpy(upc,"UPC"); return true; }; - virtual bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut); - virtual bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr); - virtual bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos); - virtual bool GetAudioStatus (bool& playing, bool& pause); - virtual bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen); - virtual bool PlayAudioSector (unsigned long start,unsigned long len); - virtual bool PauseAudio (bool resume); - virtual bool StopAudio (void); - virtual void ChannelControl (TCtrl ctrl) { return; }; - virtual bool ReadSectors (PhysPt /*buffer*/, bool /*raw*/, unsigned long /*sector*/, unsigned long /*num*/) { return false; }; - virtual bool LoadUnloadMedia (bool unload); + CDROM_Interface_SDL (void); + virtual ~CDROM_Interface_SDL (void); + virtual bool SetDevice (char* path, int forceCD); + virtual bool GetUPC (unsigned char& attr, char* upc) { attr = 0; strcpy(upc,"UPC"); return true; }; + virtual bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut); + virtual bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr); + virtual bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos); + virtual bool GetAudioStatus (bool& playing, bool& pause); + virtual bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen); + virtual bool PlayAudioSector (unsigned long start,unsigned long len); + virtual bool PauseAudio (bool resume); + virtual bool StopAudio (void); + virtual void ChannelControl (TCtrl ctrl) { (void)ctrl; // unused but part of the API + return; }; + virtual bool ReadSectors (PhysPt /*buffer*/, bool /*raw*/, unsigned long /*sector*/, unsigned long /*num*/) { return false; }; + virtual bool LoadUnloadMedia (bool unload); private: - bool Open (void); - void Close (void); + bool Open (void); + void Close (void); - SDL_CD* cd; - int driveID; - Uint32 oldLeadOut; + SDL_CD* cd; + int driveID; + Uint32 oldLeadOut; }; class CDROM_Interface_Fake : public CDROM_Interface { public: - bool SetDevice (char* /*path*/, int /*forceCD*/) { return true; }; - bool GetUPC (unsigned char& attr, char* upc) { attr = 0; strcpy(upc,"UPC"); return true; }; - bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut); - bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr); - bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos); - bool GetAudioStatus (bool& playing, bool& pause); - bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen); - bool PlayAudioSector (unsigned long /*start*/,unsigned long /*len*/) { return true; }; - bool PauseAudio (bool /*resume*/) { return true; }; - bool StopAudio (void) { return true; }; - void ChannelControl (TCtrl ctrl) { return; }; - bool ReadSectors (PhysPt /*buffer*/, bool /*raw*/, unsigned long /*sector*/, unsigned long /*num*/) { return true; }; - bool LoadUnloadMedia (bool /*unload*/) { return true; }; + bool SetDevice (char* /*path*/, int /*forceCD*/) { return true; }; + bool GetUPC (unsigned char& attr, char* upc) { attr = 0; strcpy(upc,"UPC"); return true; }; + bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut); + bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr); + bool GetAudioSub (unsigned char& attr, unsigned char& track, unsigned char& index, TMSF& relPos, TMSF& absPos); + bool GetAudioStatus (bool& playing, bool& pause); + bool GetMediaTrayStatus (bool& mediaPresent, bool& mediaChanged, bool& trayOpen); + bool PlayAudioSector (unsigned long /*start*/,unsigned long /*len*/) { return true; }; + bool PauseAudio (bool /*resume*/) { return true; }; + bool StopAudio (void) { return true; }; + void ChannelControl (TCtrl ctrl) { (void)ctrl; // unused by part of the API + return; }; + bool ReadSectors (PhysPt /*buffer*/, bool /*raw*/, unsigned long /*sector*/, unsigned long /*num*/) { return true; }; + bool LoadUnloadMedia (bool /*unload*/) { return true; }; }; class CDROM_Interface_Image : public CDROM_Interface { private: + // Nested Class Definitions class TrackFile { protected: - TrackFile(Bit16u chunkSize) : chunkSize(chunkSize) {} + TrackFile(Bit16u _chunkSize) : chunkSize(_chunkSize) {} public: virtual bool read(Bit8u *buffer, int seek, int count) = 0; virtual bool seek(Bit32u offset) = 0; @@ -162,10 +162,9 @@ private: virtual Bit32u getRate() = 0; virtual Bit8u getChannels() = 0; virtual int getLength() = 0; - const Bit16u chunkSize; virtual ~TrackFile() {} + const Bit16u chunkSize; }; - class BinaryFile : public TrackFile { public: BinaryFile (const char *filename, bool &error); @@ -181,11 +180,15 @@ private: std::ifstream *file; BinaryFile(); }; - + class AudioFile : public TrackFile { public: AudioFile (const char *filename, bool &error); - bool read(Bit8u *buffer, int seek, int count) { return false; } + bool read(Bit8u *buffer, int seek, int count) { + (void)buffer; // unused but part of the API + (void)seek; // ... + (void)count; // ... + return false; } bool seek(Bit32u offset); Bit32u decode(Bit16s *buffer, Bit32u desired_track_frames); Bit16u getEndian(); @@ -197,7 +200,9 @@ private: Sound_Sample *sample; AudioFile(); }; - + +public: + // Nested struct definition struct Track { TrackFile *file; int number; @@ -208,9 +213,7 @@ private: int sectorSize; bool mode2; }; - -public: - CDROM_Interface_Image (Bit8u subUnit); + CDROM_Interface_Image (Bit8u _subUnit); virtual ~CDROM_Interface_Image (void); void InitNewMedia (void); bool SetDevice (char* path, int forceCD); @@ -228,33 +231,32 @@ public: bool LoadUnloadMedia (bool unload); bool ReadSector (Bit8u *buffer, bool raw, unsigned long sector); bool HasDataTrack (void); - -static CDROM_Interface_Image* images[26]; + static CDROM_Interface_Image* images[26]; private: - // player -static void CDAudioCallBack(Bitu desired_frames); - int GetTrack(int sector); - -static struct imagePlayer { + 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*); - Bit32u startRedbookFrame; + Bit32u startSector; Bit32u totalRedbookFrames; Bit32u playedTrackFrames; Bit32u totalTrackFrames; bool isPlaying; bool isPaused; } player; - + + // Private utility functions void ClearTracks(); bool LoadIsoFile(char *filename); bool CanReadPVD(TrackFile *file, int sectorSize, bool mode2); + std::vector::iterator GetTrack(int sector); + static void CDAudioCallBack (Bitu desired_frames); - // cue sheet processing + // Private functions for cue sheet processing bool LoadCueSheet(char *cuefile); bool GetRealFileName(std::string& filename, std::string& pathname); bool GetCueKeyword(std::string &keyword, std::istream &in); @@ -263,19 +265,22 @@ static struct imagePlayer { bool AddTrack(Track &curr, int &shift, int prestart, int &totalPregap, int currPregap); // member variables - std::vector tracks; - typedef std::vector::iterator track_it; - std::string mcn; - static int refCount; - Bit8u subUnit; + std::vector tracks; + std::string mcn; + static int refCount; + Bit8u subUnit; }; #if defined (WIN32) /* Win 32 */ #define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers +#ifndef NOMINMAX +#define NOMINMAX // Don't clobber std::max and std::min +#endif + #include -#include "wnaspi32.h" // Aspi stuff +#include "wnaspi32.h" // Aspi stuff class CDROM_Interface_Aspi : public CDROM_Interface { diff --git a/src/dos/cdrom_image.cpp b/src/dos/cdrom_image.cpp index c3d4ba73..9dc97a9d 100644 --- a/src/dos/cdrom_image.cpp +++ b/src/dos/cdrom_image.cpp @@ -16,18 +16,15 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -#include "cdrom.h" - // #define DEBUG 1 -#ifdef DEBUG -#include // time_t, tm, time(), and localtime() -#endif +#include "cdrom.h" #include #include #include #include #include +#include #include #include #include @@ -36,7 +33,7 @@ #if !defined(WIN32) #include #else -#include +#include #endif #include "drives.h" @@ -45,22 +42,18 @@ using namespace std; +// String maximums, local to this file #define MAX_LINE_LENGTH 512 #define MAX_FILENAME_LENGTH 256 -#ifdef DEBUG -char* get_time() { - static char time_str[] = "00:00:00"; - static time_t rawtime; - time(&rawtime); - const struct tm* ptime = localtime(&rawtime); - snprintf(time_str, sizeof(time_str), "%02d:%02d:%02d", ptime->tm_hour, ptime->tm_min, ptime->tm_sec); - return time_str; -} -#endif +// STL type shorteners, local to this file +using track_iter = vector::iterator; +using track_const_iter = vector::const_iterator; +using tracks_size_t = vector::size_type; CDROM_Interface_Image::BinaryFile::BinaryFile(const char *filename, bool &error) - :TrackFile(RAW_SECTOR_SIZE) + :TrackFile(BYTES_PER_RAW_REDBOOK_FRAME), + file(nullptr) { file = new ifstream(filename, ios::in | ios::binary); error = (file == nullptr) || (file->fail()); @@ -68,30 +61,32 @@ CDROM_Interface_Image::BinaryFile::BinaryFile(const char *filename, bool &error) CDROM_Interface_Image::BinaryFile::~BinaryFile() { - if (file != nullptr) { - delete file; - file = nullptr; - } + // Guard: only cleanup if needed + if (file == nullptr) return; + + delete file; + file = nullptr; } bool CDROM_Interface_Image::BinaryFile::read(Bit8u *buffer, int seek, int count) { + // Guard: only proceed with a valid file + if (file == nullptr) return false; + file->seekg(seek, ios::beg); file->read((char*)buffer, count); - return !(file->fail()); + return !file->fail(); } int CDROM_Interface_Image::BinaryFile::getLength() { - int length = -1; // The maximum value held by a signed int is 2,147,483,647, - // which is larger than the maximum size of a CDROM, which - // is known to be 99 minutes or roughly 870 MB in size. - if (file) { - std::streampos original_pos = file->tellg(); - file->seekg(0, ios::end); - length = static_cast(file->tellg()); - file->seekg(original_pos, ios::end); - } + // Guard: only proceed with a valid file + if (file == nullptr) return -1; + + std::streampos original_pos = file->tellg(); + file->seekg(0, ios::end); + const int length = static_cast(file->tellg()); + file->seekg(original_pos, ios::end); return length; } @@ -104,18 +99,26 @@ Bit16u CDROM_Interface_Image::BinaryFile::getEndian() bool CDROM_Interface_Image::BinaryFile::seek(Bit32u offset) { + // Guard: only proceed with a valid file + if (file == nullptr) return false; + file->seekg(offset, ios::beg); return !file->fail(); } Bit32u CDROM_Interface_Image::BinaryFile::decode(Bit16s *buffer, Bit32u desired_track_frames) { - file->read((char*)buffer, desired_track_frames * BYTES_PER_TRACK_FRAME); - return (Bit32u) file->gcount() / BYTES_PER_TRACK_FRAME; + // Guard: only proceed with a valid file + if (file == nullptr) return 0; + + file->read((char*)buffer, desired_track_frames * BYTES_PER_REDBOOK_PCM_FRAME); + return static_cast(ceil( + static_cast(file->gcount()) / BYTES_PER_REDBOOK_PCM_FRAME)); } CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) - :TrackFile(4096) + :TrackFile(4096), + sample(nullptr) { // Use the audio file's actual sample rate and number of channels as opposed to overriding Sound_AudioInfo desired = {AUDIO_S16, 0, 0}; @@ -124,10 +127,11 @@ CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) error = false; std::string filename_only(filename); filename_only = filename_only.substr(filename_only.find_last_of("\\/") + 1); - LOG_MSG("CDROM: Loaded %s [%d Hz %d-channel]", + LOG_MSG("CDROM: Loaded %s [%d Hz, %d-channel, %2.1f minutes]", filename_only.c_str(), - this->getRate(), - this->getChannels()); + getRate(), + getChannels(), + getLength()/static_cast(REDBOOK_PCM_BYTES_PER_MS * 1000 * 60)); } else { error = true; } @@ -135,7 +139,11 @@ CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) CDROM_Interface_Image::AudioFile::~AudioFile() { + // Guard to prevent double-free or nullptr free + if (sample != nullptr) return; + Sound_FreeSample(sample); + sample = nullptr; } bool CDROM_Interface_Image::AudioFile::seek(Bit32u offset) @@ -149,14 +157,12 @@ bool CDROM_Interface_Image::AudioFile::seek(Bit32u offset) // Leave this in place (but #ifdef'ed away) for development and regression testing purposes. const auto begin = std::chrono::steady_clock::now(); #endif - // Convert the byte-offset to a time offset (milliseconds) - const bool result = Sound_Seek(sample, lround(offset/176.4f)); + const bool result = Sound_Seek(sample, lround(offset / REDBOOK_PCM_BYTES_PER_MS)); #ifdef BENCHMARK const auto end = std::chrono::steady_clock::now(); - LOG_MSG("%s CDROM: seek(%u) took %f ms", - get_time(), + LOG_MSG("CDROM: seek to sector %u => took %f ms", offset, chrono::duration (end - begin).count()); #endif @@ -170,7 +176,7 @@ Bit32u CDROM_Interface_Image::AudioFile::decode(Bit16s *buffer, Bit32u desired_t Bit16u CDROM_Interface_Image::AudioFile::getEndian() { - return sample->actual.format; + return sample ? sample->actual.format : AUDIO_S16SYS; } Bit32u CDROM_Interface_Image::AudioFile::getRate() @@ -185,23 +191,9 @@ Bit8u CDROM_Interface_Image::AudioFile::getChannels() int CDROM_Interface_Image::AudioFile::getLength() { - int length(-1); - - // GetDuration returns milliseconds ... but getLength needs Red Book bytes. - const int duration_ms = Sound_GetDuration(sample); - if (duration_ms > 0) { - // ... so convert ms to "Red Book bytes" by multiplying with 176.4f, - // which is 44,100 samples/second * 2-channels * 2 bytes/sample - // / 1000 milliseconds/second - length = (int) round(duration_ms * 176.4f); - } -#ifdef DEBUG - LOG_MSG("%s CDROM: AudioFile::getLength is %d bytes", - get_time(), - length); -#endif - - return length; + // Sound_GetDuration returns milliseconds so we covert to bytes + return sample ? static_cast(round( + Sound_GetDuration(sample) * REDBOOK_PCM_BYTES_PER_MS)) : -1; } // initialize static members @@ -209,29 +201,33 @@ 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, // startRedbookFrame + 0, // startSector 0, // totalRedbookFrames 0, // playedTrackFrames 0, // totalTrackFrames false, // isPlaying false // isPaused }; - -CDROM_Interface_Image::CDROM_Interface_Image(Bit8u subUnit) - :subUnit(subUnit) + +CDROM_Interface_Image::CDROM_Interface_Image(Bit8u _subUnit) + :subUnit(_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); #ifdef DEBUG - LOG_MSG("%s CDROM: Initialized the CDAUDIO mixer channel", get_time()); + LOG_MSG("CDROM: Initialized the CDAUDIO mixer channel and mutex"); #endif } } @@ -247,10 +243,16 @@ CDROM_Interface_Image::~CDROM_Interface_Image() ClearTracks(); if (refCount == 0) { StopAudio(); - MIXER_DelChannel(player.channel); - player.channel = nullptr; + 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("%s CDROM: Audio channel freed", get_time()); + LOG_MSG("CDROM: Released the CDAUDIO mixer channel and mutex"); #endif } } @@ -261,6 +263,7 @@ void CDROM_Interface_Image::InitNewMedia() bool CDROM_Interface_Image::SetDevice(char* path, int forceCD) { + (void)forceCD; // unused by part of the API if (LoadCueSheet(path) || LoadIsoFile(path)) { return true; @@ -279,116 +282,144 @@ bool CDROM_Interface_Image::GetUPC(unsigned char& attr, char* upc) attr = 0; strcpy(upc, this->mcn.c_str()); #ifdef DEBUG - LOG_MSG("%s CDROM: GetUPC=%s", - get_time(), - upc); + LOG_MSG("CDROM: GetUPC => returned %s", upc); #endif - return true; } -bool CDROM_Interface_Image::GetAudioTracks(int& stTrack, int& end, TMSF& leadOut) +bool CDROM_Interface_Image::GetAudioTracks(int& start_track_num, int& lead_out_num, TMSF& lead_out_msf) { - stTrack = 1; - end = (int)(tracks.size() - 1); - frames_to_msf(tracks[tracks.size() - 1].start + 150, - &leadOut.min, &leadOut.sec, &leadOut.fr); + // Guard: A valid CD has atleast two tracks: the first plus the lead-out, so bail + // out if we have fewer than 2 tracks + if (tracks.size() < MIN_REDBOOK_TRACKS) { #ifdef DEBUG - LOG_MSG("%s CDROM: GetAudioTracks, stTrack=%d, end=%d, " - "leadOut.min=%d, leadOut.sec=%d, leadOut.fr=%d", - get_time(), - stTrack, - end, - leadOut.min, - leadOut.sec, - leadOut.fr); + LOG_MSG("CDROM: GetAudioTracks: game wanted to dump track " + "metadata but our CD image has too few tracks: %u", + static_cast(tracks.size())); #endif - - return true; -} - -bool CDROM_Interface_Image::GetAudioTrackInfo(int track, TMSF& start, unsigned char& attr) -{ - if (track < 1 || track > (int)tracks.size()) { return false; } - frames_to_msf(tracks[track - 1].start + 150, - &start.min, &start.sec, &start.fr); - attr = tracks[track - 1].attr; - + start_track_num = tracks.begin()->number; + track_const_iter lead_out(prev(tracks.end())); + lead_out_num = lead_out->number; + frames_to_msf(lead_out->start + 150, + &lead_out_msf.min, + &lead_out_msf.sec, + &lead_out_msf.fr); #ifdef DEBUG - LOG_MSG("%s CDROM: GetAudioTrackInfo track=%d MSF %02d:%02d:%02d, attr=%u", - get_time(), - track, - start.min, - start.sec, - start.fr, - attr); + LOG_MSG("CDROM: GetAudioTracks => start track is %2d, lead out track is %2d, " + "and lead out MSF is %02d:%02d:%02d", + start_track_num, + lead_out_num, + lead_out_msf.min, + lead_out_msf.sec, + lead_out_msf.fr); #endif return true; } -bool CDROM_Interface_Image::GetAudioSub(unsigned char& attr, unsigned char& track, +bool CDROM_Interface_Image::GetAudioTrackInfo(int requested_track_num, + TMSF& start_msf, + unsigned char& attr) +{ + if (tracks.size() < MIN_REDBOOK_TRACKS || + (requested_track_num < 1 && + static_cast(requested_track_num) > tracks.size())) { +#ifdef DEBUG + LOG_MSG("CDROM: GetAudioTrackInfo for track %d => " + "outside our valid track numbers: 1 to %u", + requested_track_num, + static_cast(tracks.size())); +#endif + return false; + } + + const tracks_size_t requested_track_index(requested_track_num - 1); + track_const_iter track(tracks.begin() + requested_track_index); + frames_to_msf(track->start + 150, &start_msf.min, &start_msf.sec, &start_msf.fr); + attr = track->attr; +#ifdef DEBUG + LOG_MSG("CDROM: GetAudioTrackInfo for track %d => " + "MSF %02d:%02d:%02d, which is sector %d", + requested_track_num, + start_msf.min, + start_msf.sec, + start_msf.fr, + msf_to_frames(start_msf.min, start_msf.sec, start_msf.fr)); +#endif + return true; +} + +bool CDROM_Interface_Image::GetAudioSub(unsigned char& attr, unsigned char& track_num, unsigned char& index, TMSF& relPos, TMSF& absPos) { - bool rcode = false; + // Guard 1: bail if the game requests playback status before actually playing + if (player.trackFile == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: GetAudioSub => game asked for playback position " + "before playing audio (ignoring)"); +#endif + return false; + } - // Convert our running tally of track-frames played to Redbook-frames played. - // We round up because if our track-frame tally lands in the middle of a (fractional) + // Convert our running tally of played *track* frames to *Redbook* frames. + // We 'ceil' because if our track-frame tally lands in the middle of a (fractional) // Redbook frame, then that Redbook frame would have to be considered played to produce // even the smallest amount of track-frames played. This also accurately represents // the very end of a sequence where the last Redbook frame might only contain a couple // PCM samples - but the entire last 2352-byte Redbook frame is needed to cover those samples. - const Bit32u playedRedbookFrames = static_cast(ceil( - static_cast(REDBOOK_FRAMES_PER_SECOND * player.playedTrackFrames) / player.trackFile->getRate() )); + const Bit32u playedRedbookFrames = static_cast( + ceil(static_cast(REDBOOK_FRAMES_PER_SECOND * player.playedTrackFrames) + / player.trackFile->getRate()) ); - // Add that to the track's starting Redbook frame to determine our absolute current Redbook frame - const Bit32u currentRedbookFrame = player.startRedbookFrame + playedRedbookFrames; - - const char cur_track = GetTrack(currentRedbookFrame); - if (cur_track >= 1) { - track = static_cast(cur_track); - attr = tracks[track - 1].attr; - index = 1; - frames_to_msf(currentRedbookFrame + 150, &absPos.min, &absPos.sec, &absPos.fr); - frames_to_msf(currentRedbookFrame - tracks[track - 1].start, &relPos.min, &relPos.sec, &relPos.fr); - rcode = true; + // Add that to the track's starting sector to determine our absolute current sector + const Bit32u currentSector = player.startSector + playedRedbookFrames; + track_const_iter track(GetTrack(currentSector)); + // Guard 2: bail if the track is invalid + if (track == tracks.end()) { #ifdef DEBUG - LOG_MSG("%s CDROM: GetAudioSub attr=%u, track=%u, index=%u", - get_time(), - attr, - track, - index); - LOG_MSG("%s CDROM: GetAudioSub absoute offset (%d), MSF=%d:%d:%d", - get_time(), - currentRedbookFrame + 150, - absPos.min, - absPos.sec, - absPos.fr); - LOG_MSG("%s CDROM: GetAudioSub relative offset (%d), MSF=%d:%d:%d", - get_time(), - currentRedbookFrame - tracks[track - 1].start + 150, - relPos.min, - relPos.sec, - relPos.fr); + LOG_MSG("CDROM: GetAudioSub => playback position lands outside of our tracks"); #endif + return false; } - return rcode; + + track_num = track->number; + attr = track->attr; + index = 1; + frames_to_msf(currentSector + 150, + &absPos.min, + &absPos.sec, + &absPos.fr); + frames_to_msf(currentSector - track->start, + &relPos.min, + &relPos.sec, + &relPos.fr); +#ifdef DEBUG + LOG_MSG("CDROM: GetAudioSub => playing at %02d:%02d:%02d (on sector %u) " + "in track %u at its %02d:%02d:%02d (at its sector %d)", + absPos.min, + absPos.sec, + absPos.fr, + currentSector + 150, + track->number, + relPos.min, + relPos.sec, + relPos.fr, + static_cast(currentSector) - track->start); +#endif + return true; } bool CDROM_Interface_Image::GetAudioStatus(bool& playing, bool& pause) { playing = player.isPlaying; pause = player.isPaused; - #ifdef DEBUG - LOG_MSG("%s CDROM: GetAudioStatus playing=%d, paused=%d", - get_time(), - playing, - pause); + LOG_MSG("CDROM: GetAudioStatus => %s and %s", + playing ? "is playing" : "stopped", + pause ? "paused" : "not paused"); #endif - return true; } @@ -397,134 +428,186 @@ bool CDROM_Interface_Image::GetMediaTrayStatus(bool& mediaPresent, bool& mediaCh mediaPresent = true; mediaChanged = false; trayOpen = false; - #ifdef DEBUG - LOG_MSG("%s CDROM: GetMediaTrayStatus present=%d, changed=%d, open=%d", - get_time(), - mediaPresent, - mediaChanged, - trayOpen); + LOG_MSG("CDROM: GetMediaTrayStatus => media is %s, %s, and the tray is %s", + mediaPresent ? "present" : "not present", + mediaChanged ? "was changed" : "hasn't been changed", + trayOpen ? "open" : "closed"); #endif - return true; } bool CDROM_Interface_Image::PlayAudioSector(unsigned long start, unsigned long len) { - bool is_playable(false); - const int track = GetTrack(start) - 1; + track_const_iter track(GetTrack(start)); - // The CDROM Red Book standard allows up to 99 tracks, which includes the data track - if ( track < 0 || track > 99 ) - LOG(LOG_MISC, LOG_WARN)("Game tried to load track #%d, which is invalid", track); + // Guard: sanity check the request beyond what GetTrack already checks + if (len == 0 || + track == tracks.end() || + track->attr == 0x40 || + track->file == nullptr || + player.channel == nullptr) { + LOG_MSG("CDROM: PlayAudioSector at start sector %lu for %lu " + " number of frames => bad request, skipping", + start, + len); + StopAudio(); + return false; + } - // Attempting to play zero sectors is a no-op - else if (len == 0) - LOG(LOG_MISC, LOG_WARN)("Game tried to play zero sectors, skipping"); + // Convert the requested absolute start sector to a byte offset relative to the track's start + // Note: even though 'GetTrack() has determined the requested sector falls within our given + // track, it's still possible that the sector is outside of the "physical" bounds of the + // file itself - such as in the pre-gap region. Therefore, we clamp the offset within the + // bounds of the actual track. - // The maximum storage achieved on a CDROM was ~900MB or just under 100 minutes - // with overburning, so use this threshold to sanity-check the start sector. - else if (start > 450000) - LOG(LOG_MISC, LOG_WARN)("Game tried to read sector %lu, " - "which is beyond the 100-minute maximum of a CDROM", - start); + const int relative_start = start - track->start; + // If the request falls in the pregap we deduct the difference from the playback duration. + if (relative_start < 0) { + len -= relative_start; + } + // Seek to the calculated byte offset, bounded to the valid byte offsets + const int offset = (track->skip + + + clamp(relative_start, 0, track->length - 1) + * track->sectorSize); - // We can't play audio from a data track (as it would result in garbage/static) - else if (track >= 0 && tracks[track].attr == 0x40) - LOG(LOG_MISC,LOG_WARN)("Game tried to play the data track. Not doing this"); + TrackFile* trackFile = track->file; + // Guard: Bail if our track could not be seeked + if (!trackFile->seek(offset)) { + LOG_MSG("CDROM: Track %d failed to seek to byte %u, so cancelling playback", + track->number, + offset); + StopAudio(); + return false; + } - // Checks passed, setup the audio stream - else { - TrackFile* trackFile = tracks[track].file; + // Get properties about the current track + const Bit8u track_channels = trackFile->getChannels(); + const Bit32u track_rate = trackFile->getRate(); - // Convert the playback start sector to a time offset (milliseconds) relative to the track - const Bit32u offset = tracks[track].skip + (start - tracks[track].start) * tracks[track].sectorSize; - is_playable = trackFile->seek(offset); + // Guard: + // 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) { + LOG_MSG("CDROM: PlayAudioSector couldn't lock our player for exclusive access"); + StopAudio(); + return false; + } - // only initialize the player elements if our track is playable - if (is_playable) { - const Bit8u track_channels = trackFile->getChannels(); - const Bit32u track_rate = trackFile->getRate(); + // Update our player with properties about this playback sequence + player.cd = this; + player.trackFile = trackFile; + player.startSector = start; + player.totalRedbookFrames = len; + player.isPlaying = true; + player.isPaused = false; - player.cd = this; - player.trackFile = trackFile; - player.startRedbookFrame = start; - player.totalRedbookFrames = len; - player.isPlaying = true; - player.isPaused = false; + // Assign the mixer function associated with this track's content type + if (trackFile->getEndian() == AUDIO_S16SYS) { + player.addFrames = track_channels == 2 ? &MixerChannel::AddSamples_s16 \ + : &MixerChannel::AddSamples_m16; + } else { + player.addFrames = track_channels == 2 ? &MixerChannel::AddSamples_s16_nonnative \ + : &MixerChannel::AddSamples_m16_nonnative; + } - if (trackFile->getEndian() == AUDIO_S16SYS) { - player.addFrames = track_channels == 2 ? &MixerChannel::AddSamples_s16 \ - : &MixerChannel::AddSamples_m16; - } else { - player.addFrames = track_channels == 2 ? &MixerChannel::AddSamples_s16_nonnative \ - : &MixerChannel::AddSamples_m16_nonnative; - } - // Convert Redbook frames to Track frames, rounding up to whole integer frames. - // Round up to whole track frames because the content originated from whole redbook frames, - // which will require the last fractional frames to be represented by a whole PCM frame. - player.totalTrackFrames = static_cast(ceil( - static_cast(track_rate * player.totalRedbookFrames) / REDBOOK_FRAMES_PER_SECOND)); - player.playedTrackFrames = 0; + // Convert Redbook frames to Track frames, rounding up to whole integer frames. + // Round up to whole track frames because the content originated from whole redbook frames, + // which will require the last fractional frames to be represented by a whole PCM frame. + player.playedTrackFrames = 0; + player.totalTrackFrames = static_cast(ceil(static_cast + (track_rate * player.totalRedbookFrames) + / REDBOOK_FRAMES_PER_SECOND)); #ifdef DEBUG - LOG_MSG("%s CDROM: Playing track %d (%d Hz " - "%d-channel) at starting sector %lu (%.1f minute-mark) " - "for %u Redbook frames (%.1f seconds)", - get_time(), - track, - track_rate, - track_channels, - start, - static_cast(start) / (REDBOOK_FRAMES_PER_SECOND * 60), - player.totalRedbookFrames, - static_cast(player.totalRedbookFrames) / REDBOOK_FRAMES_PER_SECOND); + if (static_cast(start) < track->start) { + LOG_MSG("CDROM: PlayAudioSector from sector %lu to %lu => " + "starting in the pregap of track %d [pregap %d, start %u, end %u]", + start, + start + len, + track->number, + prev(track)->start - prev(track)->length, + track->start, + track->start + track->length); + } else { + LOG_MSG("CDROM: PlayAudioSector from sector %lu to %lu => " + "in track %d [start %u, end %u]", + start, + start + len, + track->number, + track->start, + track->start + track->length); + } #endif - // start the channel! - player.channel->SetFreq(track_rate); - player.channel->Enable(true); - } - } - if (!is_playable) StopAudio(); - return is_playable; + // start the channel! + player.channel->SetFreq(track_rate); + player.channel->Enable(true); + + // Guard: release the lock in this data + if (SDL_UnlockMutex(player.mutex) < 0) { + LOG_MSG("CDROM: PlayAudioSector couldn't unlock this thread"); + StopAudio(); + return false; + } + return true; } bool CDROM_Interface_Image::PauseAudio(bool resume) { + // Guard: Bail if our mixer channel hasn't been allocated + if (player.channel == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: PauseAudio => game toggled before playing audio"); +#endif + return false; + } + // Only switch states if needed if (player.isPaused == resume) { player.channel->Enable(resume); player.isPaused = !resume; - } - #ifdef DEBUG - LOG_MSG("%s CDROM: PauseAudio, state=%s", - get_time(), resume ? "resumed" : "paused"); + LOG_MSG("CDROM: PauseAudio => audio is now %s", + resume ? "unpaused" : "paused"); #endif - + } return true; } bool CDROM_Interface_Image::StopAudio(void) { + // Guard: Bail if our mixer channel hasn't been allocated + if (player.channel == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: StopAudio => game tried stopping the CD before playing audio"); +#endif + return false; + } + // Only switch states if needed if (player.isPlaying) { player.channel->Enable(false); player.isPlaying = false; player.isPaused = false; - } - #ifdef DEBUG - LOG_MSG("%s CDROM: StopAudio", get_time()); + LOG_MSG("CDROM: StopAudio => stopped playback and halted the mixer"); #endif - + } return true; } void CDROM_Interface_Image::ChannelControl(TCtrl ctrl) { - if (player.channel == NULL) return; + // Guard: Bail if our mixer channel hasn't been allocated + if (player.channel == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: ChannelControl => game tried applying channel controls " + "before playing audio"); +#endif + return; + } // Adjust the volume of our mixer channel as defined by the application player.channel->SetScale(static_cast(ctrl.vol[0]/255.0), // left vol @@ -533,20 +616,30 @@ void CDROM_Interface_Image::ChannelControl(TCtrl ctrl) // Map the audio channels in our mixer channel as defined by the application player.channel->MapChannels(ctrl.out[0], // left map ctrl.out[1]); // right map + +#ifdef DEBUG + LOG_MSG("CDROM: ChannelControl => volumes %d/255 and %d/255, " + "and left-right map %d, %d", + ctrl.vol[0], + ctrl.vol[1], + ctrl.out[0], + ctrl.out[1]); +#endif } bool CDROM_Interface_Image::ReadSectors(PhysPt buffer, bool raw, unsigned long sector, unsigned long num) { - int sectorSize = raw ? RAW_SECTOR_SIZE : COOKED_SECTOR_SIZE; + int sectorSize = (raw ? BYTES_PER_RAW_REDBOOK_FRAME : BYTES_PER_COOKED_REDBOOK_FRAME); Bitu buflen = num * sectorSize; Bit8u* buf = new Bit8u[buflen]; bool success = true; //Gobliiins reads 0 sectors for(unsigned long i = 0; i < num; i++) { success = ReadSector(&buf[i * sectorSize], raw, sector + i); - if (!success) break; + if (!success) { + break; + } } - MEM_BlockWrite(buffer, buf, buflen); delete[] buf; return success; @@ -554,81 +647,153 @@ bool CDROM_Interface_Image::ReadSectors(PhysPt buffer, bool raw, unsigned long s bool CDROM_Interface_Image::LoadUnloadMedia(bool unload) { + (void)unload; // unused by part of the API return true; } -int CDROM_Interface_Image::GetTrack(int sector) +track_iter CDROM_Interface_Image::GetTrack(const int sector) { - vector::iterator i = tracks.begin(); - vector::iterator end = tracks.end() - 1; - - while(i != end) { - Track &curr = *i; - Track &next = *(i + 1); - if (curr.start <= sector && sector < next.start) { - return curr.number; - } - i++; + // Guard if we have no tracks or the sector is out of bounds + if (sector < 0 || + sector > MAX_REDBOOK_SECTOR || + tracks.size() < MIN_REDBOOK_TRACKS || + sector >= tracks.back().start + tracks.back().length) { + LOG_MSG("CDROM: GetTrack at sector %d => " + "is outside the bounds of our CD having %u tracks", + sector, + static_cast(tracks.size())); + return tracks.end(); } - return -1; + + // Walk the tracks checking if the desired sector falls + // inside of a given track's range, which starts at the + // end of the prior track and goes to the current track's + // (start + length). + track_iter track(tracks.begin()); + int lower_bound = track->start; + while(track != tracks.end()) { + const int upper_bound = track->start + track->length; + if (lower_bound <= sector && sector < upper_bound) { + break; + } + track++; + lower_bound = upper_bound; + } // If we made it here without breaking, then the track + // wasn't found and the iterator is now the end() item. + +#ifdef DEBUG + // some games excessively query for track 1 (which isn't + // audio anyway, so avoid flooding the console with these + // calls + if (track != tracks.begin() && track != tracks.end()) { + if (sector < track->start) { + LOG_MSG("CDROM: GetTrack at sector %d => in the pregap of " + "track %d [pregap %d, start %d, end %d]", + sector, + track->number, + prev(track)->start - prev(track)->length, + track->start, + track->start + track->length); + } else { + LOG_MSG("CDROM: GetTrack at sector %d => track %d [start %d, end %d]", + sector, + track->number, + track->start, + track->start + track->length); + } + } else if (track == tracks.end()) { + LOG_MSG("CDROM: GetTrack at sector %d => fell outside " + "the bounds of our %u tracks", + sector, + static_cast(tracks.size())); + } +#endif + return track; } bool CDROM_Interface_Image::ReadSector(Bit8u *buffer, bool raw, unsigned long sector) { - int track = GetTrack(sector) - 1; - if (track < 0) { - return false; - } + track_const_iter track = GetTrack(sector); - int seek = tracks[track].skip + (sector - tracks[track].start) * tracks[track].sectorSize; - int length = (raw ? RAW_SECTOR_SIZE : COOKED_SECTOR_SIZE); - if (tracks[track].sectorSize != RAW_SECTOR_SIZE && raw) { + // Guard: Bail if the requested sector fell outside our tracks + if (track == tracks.end() || track->file == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: ReadSector at %lu => resulted " + "in an invalid track or track->file", + sector); +#endif return false; } - if (tracks[track].sectorSize == RAW_SECTOR_SIZE && !tracks[track].mode2 && !raw) seek += 16; - if (tracks[track].mode2 && !raw) seek += 24; + int seek = track->skip + (sector - track->start) * track->sectorSize; + int length = (raw ? BYTES_PER_RAW_REDBOOK_FRAME : BYTES_PER_COOKED_REDBOOK_FRAME); + if (track->sectorSize != BYTES_PER_RAW_REDBOOK_FRAME && raw) { + return false; + } + if (track->sectorSize == BYTES_PER_RAW_REDBOOK_FRAME && !track->mode2 && !raw) seek += 16; + if (track->mode2 && !raw) seek += 24; #if 0 // Excessively verbose.. only enable if needed #ifdef DEBUG - LOG_MSG("%s CDROM: ReadSector track=%d, desired raw=%s, sector=%ld, length=%d", - get_time(), - track, + LOG_MSG("CDROM: ReadSector track %2d, desired raw %s, sector %ld, length=%d", + track->number, raw ? "true":"false", sector, length); #endif #endif - - return tracks[track].file->read(buffer, seek, length); + return track->file->read(buffer, seek, length); } void CDROM_Interface_Image::CDAudioCallBack(Bitu desired_track_frames) { - if (desired_track_frames > 0) { + // Guards: Bail if the request or our player is invalid + if (desired_track_frames == 0 || player.trackFile == nullptr || player.cd == nullptr) { +#ifdef DEBUG + LOG_MSG("CDROM: CDAudioCallBack for %u frames => empty request, " + "file pointer, or CD pointer (skipping for now)", + static_cast(desired_track_frames)); +#endif + return; + } - const Bit32u decoded_track_frames = player.trackFile->decode(player.buffer, desired_track_frames); + // Ensure we have exclusive access to update our player members + if (SDL_LockMutex(player.mutex) < 0) { + LOG_MSG("CDROM: CDAudioCallBack couldn't lock this thread"); + return; + } - // uses either the stereo or mono and native or nonnative AddSamples call assigned during construction - (player.channel->*player.addFrames)(decoded_track_frames, player.buffer); + const Bit32u decoded_track_frames = + player.trackFile->decode(player.buffer, desired_track_frames); + player.playedTrackFrames += decoded_track_frames; - // Stop the audio if our decode stream has run dry or when we've played at least the - // total number of frames. We consider "played" to mean the running tally so far plus - // the current requested frames, which takes into account that the number of currently - // decoded frames might be less than desired if we're at the end of the track. - // (The mixer will request frames forever until we stop it, so at some point we /will/ - // decode fewer than requested; in this "tail scenario", we push those remaining decoded - // frames into the mixer and then stop the audio.) - if (decoded_track_frames == 0 - || player.playedTrackFrames + desired_track_frames >= player.totalTrackFrames) { - player.cd->StopAudio(); + // uses either the stereo or mono and native or + // nonnative AddSamples call assigned during construction + (player.channel->*player.addFrames)(decoded_track_frames, player.buffer); + + if (player.playedTrackFrames >= player.totalTrackFrames) { + player.cd->StopAudio(); + } else if (decoded_track_frames == 0) { + // Our track has run dry but we still have more music left to play! + const double percent_played = static_cast( + player.playedTrackFrames) + / player.totalTrackFrames; + const Bit32u played_redbook_frames = static_cast(ceil( + percent_played + * player.totalRedbookFrames)); + const Bit32u new_redbook_start_frame = player.startSector + + played_redbook_frames; + const Bit32u remaining_redbook_frames = player.totalRedbookFrames + - played_redbook_frames; + if (SDL_UnlockMutex(player.mutex) < 0) { + LOG_MSG("CDROM: CDAudioCallBack couldn't unlock to move to next track"); + return; } - - // Increment our tally of played frames by those we just decoded (and fed to the mixer). - // Even if we've hit the end of the track and we've stopped the audio, we still want to - // increment our tally so subsequent calls to GetAudioSub() return the full number of - // frames played. - player.playedTrackFrames += decoded_track_frames; + player.cd->PlayAudioSector(new_redbook_start_frame, remaining_redbook_frames); + return; + } + if (SDL_UnlockMutex(player.mutex) < 0) { + LOG_MSG("CDROM: CDAudioCallBack couldn't unlock our player before returning"); } } @@ -659,17 +824,17 @@ bool CDROM_Interface_Image::LoadIsoFile(char* filename) track.attr = 0x40;//data // try to detect iso type - if (CanReadPVD(track.file, COOKED_SECTOR_SIZE, false)) { - track.sectorSize = COOKED_SECTOR_SIZE; + if (CanReadPVD(track.file, BYTES_PER_COOKED_REDBOOK_FRAME, false)) { + track.sectorSize = BYTES_PER_COOKED_REDBOOK_FRAME; track.mode2 = false; - } else if (CanReadPVD(track.file, RAW_SECTOR_SIZE, false)) { - track.sectorSize = RAW_SECTOR_SIZE; + } else if (CanReadPVD(track.file, BYTES_PER_RAW_REDBOOK_FRAME, false)) { + track.sectorSize = BYTES_PER_RAW_REDBOOK_FRAME; track.mode2 = false; } else if (CanReadPVD(track.file, 2336, true)) { track.sectorSize = 2336; track.mode2 = true; - } else if (CanReadPVD(track.file, RAW_SECTOR_SIZE, true)) { - track.sectorSize = RAW_SECTOR_SIZE; + } else if (CanReadPVD(track.file, BYTES_PER_RAW_REDBOOK_FRAME, true)) { + track.sectorSize = BYTES_PER_RAW_REDBOOK_FRAME; track.mode2 = true; } else { if (track.file != nullptr) { @@ -680,7 +845,7 @@ bool CDROM_Interface_Image::LoadIsoFile(char* filename) } track.length = track.file->getLength() / track.sectorSize; #ifdef DEBUG - LOG_MSG("LoadIsoFile: %s, track 1, 0x40, sectorSize=%d, mode2=%s", + LOG_MSG("LoadIsoFile parsed %s => track 1, 0x40, sectorSize %d, mode2 is %s", filename, track.sectorSize, track.mode2 ? "true":"false"); @@ -700,14 +865,20 @@ bool CDROM_Interface_Image::LoadIsoFile(char* filename) bool CDROM_Interface_Image::CanReadPVD(TrackFile *file, int sectorSize, bool mode2) { - Bit8u pvd[COOKED_SECTOR_SIZE]; + // Guard: Bail if our file pointer is empty + if (file == nullptr) return false; + + // Initialize our array in the event file->read() doesn't fully write it + Bit8u pvd[BYTES_PER_COOKED_REDBOOK_FRAME] = {0}; + int seek = 16 * sectorSize; // first vd is located at sector 16 - if (sectorSize == RAW_SECTOR_SIZE && !mode2) seek += 16; + if (sectorSize == BYTES_PER_RAW_REDBOOK_FRAME && !mode2) seek += 16; if (mode2) seek += 24; - file->read(pvd, seek, COOKED_SECTOR_SIZE); - // pvd[0] = descriptor type, pvd[1..5] = standard identifier, pvd[6] = iso version (+8 for High Sierra) - return ((pvd[0] == 1 && !strncmp((char*)(&pvd[1]), "CD001", 5) && pvd[6] == 1) || - (pvd[8] == 1 && !strncmp((char*)(&pvd[9]), "CDROM", 5) && pvd[14] == 1)); + file->read(pvd, seek, BYTES_PER_COOKED_REDBOOK_FRAME); + // pvd[0] = descriptor type, pvd[1..5] = standard identifier, + // pvd[6] = iso version (+8 for High Sierra) + return ((pvd[0] == 1 && !strncmp((char*)(&pvd[1]), "CD001", 5) && pvd[6] == 1) || + (pvd[8] == 1 && !strncmp((char*)(&pvd[9]), "CDROM", 5) && pvd[14] == 1)); } #if defined(WIN32) @@ -778,15 +949,15 @@ bool CDROM_Interface_Image::LoadCueSheet(char *cuefile) GetCueKeyword(type, line); if (type == "AUDIO") { - track.sectorSize = RAW_SECTOR_SIZE; + track.sectorSize = BYTES_PER_RAW_REDBOOK_FRAME; track.attr = 0; track.mode2 = false; } else if (type == "MODE1/2048") { - track.sectorSize = COOKED_SECTOR_SIZE; + track.sectorSize = BYTES_PER_COOKED_REDBOOK_FRAME; track.attr = 0x40; track.mode2 = false; } else if (type == "MODE1/2352") { - track.sectorSize = RAW_SECTOR_SIZE; + track.sectorSize = BYTES_PER_RAW_REDBOOK_FRAME; track.attr = 0x40; track.mode2 = false; } else if (type == "MODE2/2336") { @@ -794,7 +965,7 @@ bool CDROM_Interface_Image::LoadCueSheet(char *cuefile) track.attr = 0x40; track.mode2 = true; } else if (type == "MODE2/2352") { - track.sectorSize = RAW_SECTOR_SIZE; + track.sectorSize = BYTES_PER_RAW_REDBOOK_FRAME; track.attr = 0x40; track.mode2 = true; } else success = false; @@ -884,7 +1055,8 @@ bool CDROM_Interface_Image::LoadCueSheet(char *cuefile) -bool CDROM_Interface_Image::AddTrack(Track &curr, int &shift, int prestart, int &totalPregap, int currPregap) +bool CDROM_Interface_Image::AddTrack(Track &curr, int &shift, const int prestart, + int &totalPregap, const int currPregap) { int skip = 0; @@ -921,29 +1093,15 @@ bool CDROM_Interface_Image::AddTrack(Track &curr, int &shift, int prestart, int curr.start += totalPregap; // current track uses a different file as the previous track } else { - if (!prev.length) { - int tmp = prev.file->getLength() - prev.skip; - prev.length = tmp / prev.sectorSize; - if (tmp % prev.sectorSize != 0) prev.length++; // padding - } + const int tmp = prev.file->getLength() - prev.skip; + prev.length = tmp / prev.sectorSize; + if (tmp % prev.sectorSize != 0) prev.length++; // padding + curr.start += prev.start + prev.length + currPregap; curr.skip = skip * curr.sectorSize; shift += prev.start + prev.length; totalPregap = currPregap; } - -#ifdef DEBUG - LOG_MSG("%s CDROM: AddTrack cur.start=%d cur.len=%d cur.start+len=%d " - "| prev.start=%d prev.len=%d prev.start+len=%d", - get_time(), - curr.start, - curr.length, - curr.start + curr.length, - prev.start, - prev.length, - prev.start + prev.length); -#endif - // error checks if (curr.number <= 1 || prev.number + 1 != curr.number || @@ -959,8 +1117,8 @@ bool CDROM_Interface_Image::AddTrack(Track &curr, int &shift, int prestart, int bool CDROM_Interface_Image::HasDataTrack(void) { //Data track has attribute 0x40 - for(track_it it = tracks.begin(); it != tracks.end(); it++) { - if ((*it).attr == 0x40) { + for(const auto &track : tracks) { + if (track.attr == 0x40) { return true; } } @@ -1064,17 +1222,11 @@ bool CDROM_Interface_Image::GetCueString(string &str, istream &in) void CDROM_Interface_Image::ClearTracks() { - vector::iterator i = tracks.begin(); - vector::iterator end = tracks.end(); - - TrackFile* last = nullptr; - while(i != end) { - Track &curr = *i; - if (curr.file != last) { - delete curr.file; - last = curr.file; + for (auto &track : tracks) { + if (track.file) { + delete track.file; + track.file = nullptr; } - i++; } tracks.clear(); } @@ -1084,6 +1236,8 @@ void CDROM_Image_Destroy(Section*) { } void CDROM_Image_Init(Section* sec) { - sec->AddDestroyFunction(CDROM_Image_Destroy, false); + if (sec != nullptr) { + sec->AddDestroyFunction(CDROM_Image_Destroy, false); + } Sound_Init(); } diff --git a/src/dos/cdrom_ioctl_win32.cpp b/src/dos/cdrom_ioctl_win32.cpp index 87a8b576..86ec36a3 100644 --- a/src/dos/cdrom_ioctl_win32.cpp +++ b/src/dos/cdrom_ioctl_win32.cpp @@ -495,18 +495,18 @@ bool CDROM_Interface_Ioctl::ReadSector(Bit8u *buffer, bool raw, unsigned long se BOOL bStat; DWORD byteCount = 0; - Bitu buflen = raw ? RAW_SECTOR_SIZE : COOKED_SECTOR_SIZE; + Bitu buflen = raw ? BYTES_PER_RAW_REDBOOK_FRAME : BYTES_PER_COOKED_REDBOOK_FRAME; if (!raw) { // Cooked int success = 0; - DWORD newPos = SetFilePointer(hIOCTL, sector*COOKED_SECTOR_SIZE, 0, FILE_BEGIN); + DWORD newPos = SetFilePointer(hIOCTL, sector*BYTES_PER_COOKED_REDBOOK_FRAME, 0, FILE_BEGIN); if (newPos != 0xFFFFFFFF) success = ReadFile(hIOCTL, buffer, buflen, &byteCount, NULL); bStat = (success!=0); } else { // Raw RAW_READ_INFO in; - in.DiskOffset.LowPart = sector*COOKED_SECTOR_SIZE; + in.DiskOffset.LowPart = sector*BYTES_PER_COOKED_REDBOOK_FRAME; in.DiskOffset.HighPart = 0; in.SectorCount = 1; in.TrackMode = CDDA; @@ -521,19 +521,19 @@ bool CDROM_Interface_Ioctl::ReadSectors(PhysPt buffer, bool raw, unsigned long s BOOL bStat; DWORD byteCount = 0; - Bitu buflen = raw ? num*RAW_SECTOR_SIZE : num*COOKED_SECTOR_SIZE; + Bitu buflen = raw ? num*BYTES_PER_RAW_REDBOOK_FRAME : num*BYTES_PER_COOKED_REDBOOK_FRAME; Bit8u* bufdata = new Bit8u[buflen]; if (!raw) { // Cooked int success = 0; - DWORD newPos = SetFilePointer(hIOCTL, sector*COOKED_SECTOR_SIZE, 0, FILE_BEGIN); + DWORD newPos = SetFilePointer(hIOCTL, sector*BYTES_PER_COOKED_REDBOOK_FRAME, 0, FILE_BEGIN); if (newPos != 0xFFFFFFFF) success = ReadFile(hIOCTL, bufdata, buflen, &byteCount, NULL); bStat = (success!=0); } else { // Raw RAW_READ_INFO in; - in.DiskOffset.LowPart = sector*COOKED_SECTOR_SIZE; + in.DiskOffset.LowPart = sector*BYTES_PER_COOKED_REDBOOK_FRAME; in.DiskOffset.HighPart = 0; in.SectorCount = num; in.TrackMode = CDDA; @@ -563,7 +563,7 @@ void CDROM_Interface_Ioctl::dx_CDAudioCallBack(Bitu len) { if (success) { player.currFrame++; - player.bufLen += RAW_SECTOR_SIZE; + player.bufLen += BYTES_PER_RAW_REDBOOK_FRAME; } else { memset(&player.buffer[player.bufLen], 0, len - player.bufLen); player.bufLen = len; diff --git a/src/dos/drive_iso.cpp b/src/dos/drive_iso.cpp index 8de74162..bbc7b82a 100644 --- a/src/dos/drive_iso.cpp +++ b/src/dos/drive_iso.cpp @@ -511,7 +511,7 @@ int isoDrive :: readDirEntry(isoDirEntry *de, Bit8u *data) { } bool isoDrive :: loadImage() { - Bit8u pvd[COOKED_SECTOR_SIZE]; + Bit8u pvd[BYTES_PER_COOKED_REDBOOK_FRAME]; dataCD = false; readSector(pvd, ISO_FIRST_VD); if (pvd[0] == 1 && !strncmp((char*)(&pvd[1]), "CD001", 5) && pvd[6] == 1) iso = true; diff --git a/src/hardware/mixer.cpp b/src/hardware/mixer.cpp index 32414776..f51a2722 100644 --- a/src/hardware/mixer.cpp +++ b/src/hardware/mixer.cpp @@ -34,8 +34,6 @@ #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #endif -// prevent the Windows header from clobbering std::min and max -#define NOMINMAX #include #include #endif @@ -156,11 +154,6 @@ void MixerChannel::UpdateVolume(void) { volmul[1]=(Bits)((1 << MIXER_VOLSHIFT)*scale[1]*volmain[1]*mixer.mastervol[1]); } -template -T clamp(const T& n, const T& lower, const T& upper) { - return std::max(lower, std::min(n, upper)); -} - void MixerChannel::SetVolume(float _left,float _right) { // Allow unconstrained user-defined values volmain[0] = _left; diff --git a/src/misc/support.cpp b/src/misc/support.cpp index 89f7802a..e90b07ef 100644 --- a/src/misc/support.cpp +++ b/src/misc/support.cpp @@ -17,16 +17,15 @@ */ -#include -#include +#include #include +#include #include #include #include -#include -#include -#include +#include #include +#include #include "dosbox.h" #include "debug.h"