diff --git a/src/dos/cdrom.h b/src/dos/cdrom.h index 34b78c2e..b186bc38 100644 --- a/src/dos/cdrom.h +++ b/src/dos/cdrom.h @@ -53,6 +53,7 @@ #define MAX_REDBOOK_TRACKS 99u // a CD can contain 99 playable tracks plus the remaining leadout #define MIN_REDBOOK_TRACKS 2u // One track plus the lead-out track #define REDBOOK_PCM_BYTES_PER_MS 176.4f // 44.1 frames/ms * 4 bytes/frame +#define REDBOOK_PCM_BYTES_PER_MIN 10584000u // 44.1 frames/ms * 4 bytes/frame * 1000 ms/s * 60 s/min #define BYTES_PER_REDBOOK_PCM_FRAME 4u // 2 bytes/sample * 2 samples/frame #define MAX_REDBOOK_BYTES (MAX_REDBOOK_FRAMES * BYTES_PER_RAW_REDBOOK_FRAME) // length of a CDROM in bytes #define MAX_REDBOOK_DURATION_MS (99 * 60 * 1000) // 99 minute CDROM in milliseconds @@ -138,6 +139,11 @@ private: class TrackFile { protected: TrackFile(Bit16u _chunkSize) : chunkSize(_chunkSize) {} + bool offsetInsideTrack(const uint32_t offset); + uint32_t adjustOverRead(const uint32_t offset, + const uint32_t requested_bytes); + int length_redbook_bytes = -1; + public: virtual ~TrackFile() = default; virtual bool read(uint8_t *buffer, @@ -149,7 +155,7 @@ private: virtual Bit32u getRate() = 0; virtual Bit8u getChannels() = 0; virtual int getLength() = 0; - const Bit16u chunkSize; + const Bit16u chunkSize = 0; }; class BinaryFile : public TrackFile { diff --git a/src/dos/cdrom_image.cpp b/src/dos/cdrom_image.cpp index 20027722..4a36b9af 100644 --- a/src/dos/cdrom_image.cpp +++ b/src/dos/cdrom_image.cpp @@ -53,9 +53,46 @@ using track_iter = vector::iterator; using track_const_iter = vector::const_iterator; using tracks_size_t = vector::size_type; +// Report bad seeks that would go beyond the end of the track +bool CDROM_Interface_Image::TrackFile::offsetInsideTrack(const uint32_t offset) +{ + if (static_cast(offset) >= getLength()) { + LOG_MSG("CDROM: attempted to seek to byte %u, beyond the " + "track's %d byte-length", + offset, length_redbook_bytes); + return false; + } + return true; +} + +// Trim requested read sizes that will spill beyond the track ending +uint32_t CDROM_Interface_Image::TrackFile::adjustOverRead(const uint32_t offset, + const uint32_t requested_bytes) +{ + // The most likely scenario is read is valid and no adjustment is needed + uint32_t adjusted_bytes = requested_bytes; + + // If we seek beyond the end of the track, then we can't read any bytes... + if (static_cast(offset) >= getLength()) { + adjusted_bytes = 0; + LOG_MSG("CDROM: can't read audio because requested offset %u " + "is beyond the track length, %u", + offset, getLength()); + } + // Otherwise if our seek + requested bytes goes beyond, then prune back + // the request + else if (static_cast(offset + requested_bytes) > getLength()) { + adjusted_bytes = static_cast(getLength()) - offset; + LOG_MSG("CDROM: reducing read-length by %u bytes to avoid " + "reading beyond track.", + requested_bytes - adjusted_bytes); + } + return adjusted_bytes; +} + CDROM_Interface_Image::BinaryFile::BinaryFile(const char *filename, bool &error) - : TrackFile(BYTES_PER_RAW_REDBOOK_FRAME), - file(nullptr) + : TrackFile(BYTES_PER_RAW_REDBOOK_FRAME), + file(nullptr) { file = new ifstream(filename, ios::in | ios::binary); // If new fails, an exception is generated and scope leaves this constructor @@ -81,31 +118,38 @@ bool CDROM_Interface_Image::BinaryFile::read(uint8_t *buffer, assertm(offset <= MAX_REDBOOK_BYTES, "Requested offset exceeds CDROM size"); assertm(requested_bytes <= MAX_REDBOOK_BYTES, "Requested bytes exceeds CDROM size"); - file->seekg(offset, ios::beg); - file->read((char*)buffer, requested_bytes); + if (!seek(offset)) + return false; + + const uint32_t adjusted_bytes = adjustOverRead(offset, requested_bytes); + if (adjusted_bytes == 0) // no work to do! + return true; + + file->read((char *)buffer, adjusted_bytes); return !file->fail(); } int CDROM_Interface_Image::BinaryFile::getLength() { - // Check for logic bugs and illegal values - assertm(file, "The file pointer is invalid"); + // Return our cached result if we've already been asked before + if (length_redbook_bytes < 0 && file) { + file->seekg(0, ios::end); + /** + * All read(..) operations involve an absolute position and + * this function isn't called in other threads, therefore + * we don't need to restore the original file position. + */ + length_redbook_bytes = static_cast(file->tellg()); - /** - * All read operations involve an absolute position and - * this function isn't called in other threads, therefore - * we don't need to retain the original read position. - */ - file->seekg(0, ios::end); - const int length = static_cast(file->tellg()); - assertm(length == -1 // allow the length to fail, which is valid - || static_cast(length) <= MAX_REDBOOK_BYTES, - "Length exceeds the maximum CDROM size"); + assertm(length_redbook_bytes >= 0, + "Track length could not be determined"); + assertm(static_cast(length_redbook_bytes) <= MAX_REDBOOK_BYTES, + "Track length exceeds the maximum CDROM size"); #ifdef DEBUG - LOG_MSG("CDROM: Length of image is %d bytes", length); + LOG_MSG("CDROM: Length of image is %d bytes", length_redbook_bytes); #endif - return length; - + } + return length_redbook_bytes; } Bit16u CDROM_Interface_Image::BinaryFile::getEndian() @@ -114,19 +158,21 @@ Bit16u CDROM_Interface_Image::BinaryFile::getEndian() return AUDIO_S16LSB; } - bool CDROM_Interface_Image::BinaryFile::seek(const uint32_t offset) { // Check for logic bugs and illegal values assertm(file, "The file pointer needs to be valid, but is the nullptr"); assertm(offset <= MAX_REDBOOK_BYTES, "Requested offset exceeds CDROM size"); + if (!offsetInsideTrack(offset)) + return false; + file->seekg(offset, ios::beg); - // If the seek failed, then let's try harder + // If the first seek attempt failed, then try harder if (file->fail()) { file->clear(); // clear fail and eof bits - file->seekg(0, std::ios::beg); // "I have returned." + file->seekg(0, std::ios::beg); // "I have returned." file->seekg(offset, ios::beg); // "It will be done." } return !file->fail(); @@ -162,10 +208,8 @@ CDROM_Interface_Image::AudioFile::AudioFile(const char *filename, bool &error) if (sample) { error = false; LOG_MSG("CDROM: Loaded %s [%d Hz, %d-channel, %2.1f minutes]", - filename_only.c_str(), - getRate(), - getChannels(), - getLength()/static_cast(REDBOOK_PCM_BYTES_PER_MS * 1000 * 60)); + filename_only.c_str(), getRate(), getChannels(), + getLength() / static_cast(REDBOOK_PCM_BYTES_PER_MIN)); } else { LOG_MSG("CDROM: Failed adding '%s' as CDDA track!", filename_only.c_str()); error = true; @@ -199,6 +243,10 @@ bool CDROM_Interface_Image::AudioFile::seek(const uint32_t requested_pos) // Check for logic bugs and if the track is already positioned as requested assertm(sample, "Audio sample needs to be valid, but is the nullptr"); assertm(requested_pos <= MAX_REDBOOK_BYTES, "Requested offset exceeds CDROM size"); + + if (!offsetInsideTrack(requested_pos)) + return false; + if (track_pos == requested_pos) { #ifdef DEBUG LOG_MSG("CDROM: seek to %u avoided with position-tracking", requested_pos); @@ -260,9 +308,8 @@ bool CDROM_Interface_Image::AudioFile::read(uint8_t *buffer, assertm(buffer != nullptr, "buffer needs to be allocated but is the nullptr"); assertm(sample != nullptr, "Audio sample needs to be valid, but is the nullptr"); assertm(requested_pos <= MAX_REDBOOK_BYTES, "Requested offset exceeds CDROM size"); - assertm(requested_bytes <= MAX_REDBOOK_BYTES, "Requested bytes exceeds CDROM size"); - if (requested_bytes == 0) - return true; + assertm(requested_bytes <= MAX_REDBOOK_BYTES, + "Requested bytes exceeds CDROM size"); /** * Support DAE for stereo and mono 44.1 kHz tracks, otherwise inform the user. @@ -285,10 +332,15 @@ bool CDROM_Interface_Image::AudioFile::read(uint8_t *buffer, if (!seek(requested_pos)) return false; - // Setup characteristics about our track and the request + const uint32_t adjusted_bytes = adjustOverRead(requested_pos, requested_bytes); + if (adjusted_bytes == 0) // no work to do! + return true; + + // Setup characteristics about our track and the request const uint8_t channels = getChannels(); const uint8_t bytes_per_frame = channels * REDBOOK_BPS; - const uint32_t requested_frames = ceil_udivide(requested_bytes, BYTES_PER_REDBOOK_PCM_FRAME); + const uint32_t requested_frames = ceil_udivide(adjusted_bytes, + BYTES_PER_REDBOOK_PCM_FRAME); uint32_t decoded_bytes = 0; uint32_t decoded_frames = 0; @@ -303,7 +355,7 @@ bool CDROM_Interface_Image::AudioFile::read(uint8_t *buffer, } // Zero out any remainining frames that we didn't fill if (decoded_frames < requested_frames) - memset(buffer + decoded_bytes, 0, requested_bytes - decoded_bytes); + memset(buffer + decoded_bytes, 0, adjusted_bytes - decoded_bytes); // If the track is mono, convert to stereo if (channels == 1 && decoded_frames) { @@ -386,11 +438,20 @@ Bit8u CDROM_Interface_Image::AudioFile::getChannels() int CDROM_Interface_Image::AudioFile::getLength() { - // Sound_GetDuration returns milliseconds but getLength() - // needs to return bytes, so we covert using PCM bytes/s - return sample ? - static_cast - (Sound_GetDuration(sample) * REDBOOK_PCM_BYTES_PER_MS) : -1; + if (length_redbook_bytes < 0 && sample) { + /* + * Sound_GetDuration returns milliseconds but getLength() + * needs to return bytes, so we covert using PCM bytes/s + */ + length_redbook_bytes = static_cast( + static_cast(Sound_GetDuration(sample)) * + REDBOOK_PCM_BYTES_PER_MS); + } + assertm(length_redbook_bytes >= 0, + "Track length could not be determined"); + assertm(static_cast(length_redbook_bytes) <= MAX_REDBOOK_BYTES, + "Track length exceeds the maximum CDROM size"); + return length_redbook_bytes; } // initialize static members