From 4aa33110214f9d1c3c2886eaedd1b38ecc4aa6c1 Mon Sep 17 00:00:00 2001 From: krcroft Date: Wed, 5 Feb 2020 23:07:25 -0800 Subject: [PATCH] Refine types and sizes to more accurately reflect the values they hold This allows for fewer casts, simpler math, and more readable code in subsequent commits. --- src/dos/cdrom.cpp | 4 +-- src/dos/cdrom.h | 62 +++++++++++++++++++-------------------- src/dos/cdrom_image.cpp | 64 +++++++++++++++++++++-------------------- src/dos/dos_mscdex.cpp | 7 +++-- 4 files changed, 70 insertions(+), 67 deletions(-) diff --git a/src/dos/cdrom.cpp b/src/dos/cdrom.cpp index 9a5acaf9..20ea44df 100644 --- a/src/dos/cdrom.cpp +++ b/src/dos/cdrom.cpp @@ -18,14 +18,14 @@ #include "cdrom.h" -bool CDROM_Interface_Fake :: GetAudioTracks(int& stTrack, int& end, TMSF& leadOut) { +bool CDROM_Interface_Fake::GetAudioTracks(uint8_t& stTrack, uint8_t& end, TMSF& leadOut) { stTrack = end = 1; leadOut.min = 60; leadOut.sec = leadOut.fr = 0; return true; } -bool CDROM_Interface_Fake :: GetAudioTrackInfo(int track, TMSF& start, unsigned char& attr) { +bool CDROM_Interface_Fake::GetAudioTrackInfo(uint8_t track, TMSF& start, unsigned char& attr) { if (track>1) return false; start.min = start.fr = 0; start.sec = 2; diff --git a/src/dos/cdrom.h b/src/dos/cdrom.h index 38fe3ad2..ed733925 100644 --- a/src/dos/cdrom.h +++ b/src/dos/cdrom.h @@ -85,12 +85,12 @@ public: virtual ~CDROM_Interface (void) {} virtual bool SetDevice (char *path) = 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 GetAudioTracks (uint8_t& stTrack, uint8_t& end, TMSF& leadOut) = 0; + virtual bool GetAudioTrackInfo (uint8_t 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 PlayAudioSector (uint64_t start, uint64_t len) = 0; virtual bool PauseAudio (bool resume) = 0; virtual bool StopAudio (void) = 0; virtual void ChannelControl (TCtrl ctrl) = 0; @@ -104,12 +104,12 @@ class CDROM_Interface_Fake : public CDROM_Interface public: bool SetDevice (char *) { 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 GetAudioTracks (uint8_t& stTrack, uint8_t& end, TMSF& leadOut); + bool GetAudioTrackInfo (uint8_t 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 PlayAudioSector (uint64_t start, uint64_t len) { (void)start; (void)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 @@ -126,15 +126,15 @@ private: protected: TrackFile(Bit16u _chunkSize) : chunkSize(_chunkSize) {} public: - virtual ~TrackFile() = default; - virtual bool read(Bit8u *buffer, int seek, int count) = 0; - virtual bool seek(Bit32u offset) = 0; - virtual Bit32u decode(Bit16s *buffer, Bit32u desired_track_frames) = 0; - virtual Bit16u getEndian() = 0; - virtual Bit32u getRate() = 0; - virtual Bit8u getChannels() = 0; - virtual int getLength() = 0; - const Bit16u chunkSize; + virtual ~TrackFile() = default; + virtual bool read(Bit8u *buffer, int seek, int count) = 0; + virtual bool seek(Bit32u offset) = 0; + virtual uint64_t decode(Bit16s *buffer, uint32_t desired_track_frames) = 0; + virtual Bit16u getEndian() = 0; + virtual Bit32u getRate() = 0; + virtual Bit8u getChannels() = 0; + virtual int getLength() = 0; + const Bit16u chunkSize; }; class BinaryFile : public TrackFile { @@ -148,7 +148,7 @@ private: bool read(Bit8u *buffer, int seek, int count); bool seek(Bit32u offset); - Bit32u decode(Bit16s *buffer, Bit32u desired_track_frames); + uint64_t decode(Bit16s *buffer, Bit32u desired_track_frames); Bit16u getEndian(); Bit32u getRate() { return 44100; } Bit8u getChannels() { return 2; } @@ -172,7 +172,7 @@ private: (void)count; // ... return false; } bool seek(Bit32u offset); - Bit32u decode(Bit16s *buffer, Bit32u desired_track_frames); + uint64_t decode(Bit16s *buffer, Bit32u desired_track_frames); Bit16u getEndian(); Bit32u getRate(); Bit8u getChannels(); @@ -185,12 +185,12 @@ public: // Nested struct definition struct Track { std::shared_ptr file = nullptr; - int number = 0; - int attr = 0; - int start = 0; - int length = 0; - int skip = 0; - int sectorSize = 0; + uint32_t start = 0; + uint32_t length = 0; + uint32_t skip = 0; + uint16_t number = 0; + uint16_t sectorSize = 0; + uint8_t attr = 0; bool mode2 = false; }; CDROM_Interface_Image (Bit8u _subUnit); @@ -198,12 +198,12 @@ public: void InitNewMedia (void); bool SetDevice (char *path); bool GetUPC (unsigned char& attr, char* upc); - bool GetAudioTracks (int& stTrack, int& end, TMSF& leadOut); - bool GetAudioTrackInfo (int track, TMSF& start, unsigned char& attr); + bool GetAudioTracks (uint8_t& stTrack, uint8_t& end, TMSF& leadOut); + bool GetAudioTrackInfo (uint8_t 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); + bool PlayAudioSector (uint64_t start, uint64_t len); bool PauseAudio (bool resume); bool StopAudio (void); void ChannelControl (TCtrl ctrl); @@ -221,10 +221,10 @@ private: MixerChannel *channel; CDROM_Interface_Image *cd; void (MixerChannel::*addFrames) (Bitu, const Bit16s*); - Bit32u startSector; - Bit32u totalRedbookFrames; - Bit32u playedTrackFrames; - Bit32u totalTrackFrames; + uint32_t startSector; + uint32_t totalRedbookFrames; + uint64_t playedTrackFrames; + uint64_t totalTrackFrames; bool isPlaying; bool isPaused; } player; @@ -232,7 +232,7 @@ private: // Private utility functions bool LoadIsoFile(char *filename); bool CanReadPVD(TrackFile *file, int sectorSize, bool mode2); - std::vector::iterator GetTrack(int sector); + std::vector::iterator GetTrack(const uint32_t sector); static void CDAudioCallBack (Bitu desired_frames); // Private functions for cue sheet processing diff --git a/src/dos/cdrom_image.cpp b/src/dos/cdrom_image.cpp index 4a5467b8..8a14b533 100644 --- a/src/dos/cdrom_image.cpp +++ b/src/dos/cdrom_image.cpp @@ -108,14 +108,13 @@ bool CDROM_Interface_Image::BinaryFile::seek(Bit32u offset) return !file->fail(); } -Bit32u CDROM_Interface_Image::BinaryFile::decode(Bit16s *buffer, Bit32u desired_track_frames) +uint64_t CDROM_Interface_Image::BinaryFile::decode(Bit16s *buffer, Bit32u desired_track_frames) { // 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)); + return (file->gcount() + BYTES_PER_REDBOOK_PCM_FRAME - 1) / BYTES_PER_REDBOOK_PCM_FRAME; } CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) @@ -172,7 +171,7 @@ bool CDROM_Interface_Image::AudioFile::seek(Bit32u offset) return result; } -Bit32u CDROM_Interface_Image::AudioFile::decode(Bit16s *buffer, Bit32u desired_track_frames) +uint64_t CDROM_Interface_Image::AudioFile::decode(Bit16s *buffer, Bit32u desired_track_frames) { return Sound_Decode_Direct(sample, (void*)buffer, desired_track_frames); } @@ -291,7 +290,9 @@ bool CDROM_Interface_Image::GetUPC(unsigned char& attr, char* upc) return true; } -bool CDROM_Interface_Image::GetAudioTracks(int& start_track_num, int& lead_out_num, TMSF& lead_out_msf) +bool CDROM_Interface_Image::GetAudioTracks(uint8_t& start_track_num, + uint8_t& end_track_num, + TMSF& lead_out_msf) { // Guard: A valid CD has atleast two tracks: the first plus the lead-out, so bail // out if we have fewer than 2 tracks @@ -303,10 +304,9 @@ bool CDROM_Interface_Image::GetAudioTracks(int& start_track_num, int& lead_out_n #endif return false; } - start_track_num = tracks.begin()->number; - track_const_iter lead_out(prev(tracks.end())); - lead_out_num = lead_out->number; - lead_out_msf = frames_to_msf(lead_out->start + 150); + start_track_num = tracks.front().number; + end_track_num = next(tracks.crbegin())->number; // next(crbegin) == [vec.size - 2] + lead_out_msf = frames_to_msf(tracks.back().start + 150); #ifdef DEBUG LOG_MSG("CDROM: GetAudioTracks => start track is %2d, lead out track is %2d, " "and lead out MSF is %02d:%02d:%02d", @@ -432,7 +432,7 @@ bool CDROM_Interface_Image::GetMediaTrayStatus(bool& mediaPresent, bool& mediaCh return true; } -bool CDROM_Interface_Image::PlayAudioSector(unsigned long start, unsigned long len) +bool CDROM_Interface_Image::PlayAudioSector(uint64_t start, uint64_t len) { track_const_iter track(GetTrack(start)); @@ -443,6 +443,9 @@ bool CDROM_Interface_Image::PlayAudioSector(unsigned long start, unsigned long l track->file == nullptr || player.channel == nullptr) { StopAudio(); +#ifdef DEBUG + LOG_MSG("CDROM: PlayAudioSector => sanity check failed"); +#endif return false; } @@ -457,12 +460,14 @@ bool CDROM_Interface_Image::PlayAudioSector(unsigned long start, unsigned long l 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) + const int offset = (track->skip + + clamp(relative_start, 0, static_cast(track->length - 1)) * track->sectorSize); TrackFile* trackFile = track->file.get(); + // 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", @@ -474,7 +479,7 @@ bool CDROM_Interface_Image::PlayAudioSector(unsigned long start, unsigned long l // Get properties about the current track const Bit8u track_channels = trackFile->getChannels(); - const Bit32u track_rate = trackFile->getRate(); + const uint64_t track_rate = trackFile->getRate(); // Guard: // Before we update our player object with new track details, we lock access @@ -642,17 +647,14 @@ bool CDROM_Interface_Image::LoadUnloadMedia(bool unload) return true; } -track_iter CDROM_Interface_Image::GetTrack(const int sector) +track_iter CDROM_Interface_Image::GetTrack(const uint32_t sector) { - // Guard if we have no tracks or the sector is out of bounds - if (sector < 0 || - sector > MAX_REDBOOK_SECTOR || + // Guard if we have no tracks or the sector is beyond the lead-out + if (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())); + sector >= tracks.back().start) { + LOG_MSG("CDROM: GetTrack at sector %u is outside the" + " playable range", sector); return tracks.end(); } @@ -660,10 +662,10 @@ track_iter CDROM_Interface_Image::GetTrack(const int sector) // 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; + track_iter track = tracks.begin(); + uint32_t lower_bound = track->start; while (track != tracks.end()) { - const int upper_bound = track->start + track->length; + const uint32_t upper_bound = track->start + track->length; if (lower_bound <= sector && sector < upper_bound) { break; } @@ -754,7 +756,7 @@ void CDROM_Interface_Image::CDAudioCallBack(Bitu desired_track_frames) return; } - const Bit32u decoded_track_frames = + const uint64_t decoded_track_frames = player.trackFile->decode(player.buffer, desired_track_frames); player.playedTrackFrames += decoded_track_frames; @@ -1015,17 +1017,17 @@ bool CDROM_Interface_Image::AddTrack(Track &curr, int &shift, const int prestart // frames between index 0(prestart) and 1(curr.start) must be skipped if (prestart >= 0) { - if (prestart > curr.start) { + if (prestart > static_cast(curr.start)) { + LOG_MSG("CDROM: AddTrack => prestart %d cannot be > curr.start %u", + prestart, curr.start); return false; } skip = curr.start - prestart; } - // first track (track number must be 1) + // Add the first track, if our vector is empty if (tracks.empty()) { - if (curr.number != 1) { - return false; - } + assertm(curr.number == 1, "The first track must be labelled number 1 [BUG!]"); curr.skip = skip * curr.sectorSize; curr.start += currPregap; totalPregap = currPregap; diff --git a/src/dos/dos_mscdex.cpp b/src/dos/dos_mscdex.cpp index f18f2319..49685a50 100644 --- a/src/dos/dos_mscdex.cpp +++ b/src/dos/dos_mscdex.cpp @@ -406,7 +406,8 @@ void CMscdex::GetDriverInfo (PhysPt data) { bool CMscdex::GetCDInfo(Bit8u subUnit, Bit8u& tr1, Bit8u& tr2, TMSF& leadOut) { if (subUnit>=numDrives) return false; - int tr1i,tr2i; + uint8_t tr1i; + uint8_t tr2i; // Assume Media change cdrom[subUnit]->InitNewMedia(); dinfo[subUnit].lastResult = cdrom[subUnit]->GetAudioTracks(tr1i,tr2i,leadOut); @@ -414,8 +415,8 @@ bool CMscdex::GetCDInfo(Bit8u subUnit, Bit8u& tr1, Bit8u& tr2, TMSF& leadOut) { tr1 = tr2 = 0; memset(&leadOut,0,sizeof(leadOut)); } else { - tr1 = (Bit8u) tr1i; - tr2 = (Bit8u) tr2i; + tr1 = tr1i; + tr2 = tr2i; } return dinfo[subUnit].lastResult; }