From 7b65f3748a14ad289733ddb893c9cf19b810316f Mon Sep 17 00:00:00 2001 From: David Reid Date: Sun, 18 Jul 2021 10:35:34 +1000 Subject: [PATCH] Add some safety checks to data sources. With this change, an error is now returned when the requested frame count is zero. Two reasons for this: 1) It usually means there's an error in the higher level logic if something is trying to read zero frames. 2) When no frames are read, MA_AT_END should be returned. However, if the input frame count is also zero, it creates ambiguity as to whether or not the data source is truly at the end. --- extras/miniaudio_libopus.h | 12 +++ extras/miniaudio_libvorbis.h | 12 +++ miniaudio.h | 140 +++++++++++++++++++++++++++-------- 3 files changed, 134 insertions(+), 30 deletions(-) diff --git a/extras/miniaudio_libopus.h b/extras/miniaudio_libopus.h index 7b1f8752..da059660 100644 --- a/extras/miniaudio_libopus.h +++ b/extras/miniaudio_libopus.h @@ -268,6 +268,14 @@ MA_API void ma_libopus_uninit(ma_libopus* pOpus, const ma_allocation_callbacks* MA_API ma_result ma_libopus_read_pcm_frames(ma_libopus* pOpus, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pOpus == NULL) { return MA_INVALID_ARGS; } @@ -317,6 +325,10 @@ MA_API ma_result ma_libopus_read_pcm_frames(ma_libopus* pOpus, void* pFramesOut, *pFramesRead = totalFramesRead; } + if (result == MA_SUCCESS && totalFramesRead == 0) { + result = MA_AT_END; + } + return result; } #else diff --git a/extras/miniaudio_libvorbis.h b/extras/miniaudio_libvorbis.h index 50433e4c..80af083c 100644 --- a/extras/miniaudio_libvorbis.h +++ b/extras/miniaudio_libvorbis.h @@ -263,6 +263,14 @@ MA_API void ma_libvorbis_uninit(ma_libvorbis* pVorbis, const ma_allocation_callb MA_API ma_result ma_libvorbis_read_pcm_frames(ma_libvorbis* pVorbis, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pVorbis == NULL) { return MA_INVALID_ARGS; } @@ -327,6 +335,10 @@ MA_API ma_result ma_libvorbis_read_pcm_frames(ma_libvorbis* pVorbis, void* pFram *pFramesRead = totalFramesRead; } + if (result == MA_SUCCESS && totalFramesRead == 0) { + result = MA_AT_END; + } + return result; } #else diff --git a/miniaudio.h b/miniaudio.h index 8b9a2b1b..bbaca193 100644 --- a/miniaudio.h +++ b/miniaudio.h @@ -46029,54 +46029,59 @@ static ma_result ma_data_source_resolve_current(ma_data_source* pDataSource, ma_ static ma_result ma_data_source_read_pcm_frames_within_range(ma_data_source* pDataSource, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead, ma_bool32 loop) { ma_data_source_base* pDataSourceBase = (ma_data_source_base*)pDataSource; - + ma_result result; + ma_uint64 framesRead = 0; + if (pDataSourceBase == NULL) { return MA_AT_END; } + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pDataSourceBase->rangeEndInFrames == ~((ma_uint64)0) && (pDataSourceBase->loopEndInFrames == ~((ma_uint64)0) || loop == MA_FALSE)) { /* No range is set - just read like normal. The data source itself will tell us when the end is reached. */ - return pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, pFramesRead); + result = pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, &framesRead); } else { /* Need to clamp to within the range. */ - ma_result result; ma_uint64 cursor; - ma_uint64 framesRead = 0; - ma_uint64 rangeEnd; result = ma_data_source_get_cursor_in_pcm_frames(pDataSourceBase, &cursor); if (result != MA_SUCCESS) { /* Failed to retrieve the cursor. Cannot read within a range or loop points. Just read like normal - this may happen for things like noise data sources where it doesn't really matter. */ - return pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, pFramesRead); - } + result = pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, &framesRead); + } else { + ma_uint64 rangeEnd; - /* We have the cursor. We need to make sure we don't read beyond our range. */ - rangeEnd = pDataSourceBase->rangeEndInFrames; + /* We have the cursor. We need to make sure we don't read beyond our range. */ + rangeEnd = pDataSourceBase->rangeEndInFrames; - /* If looping, make sure we're within range. */ - if (loop) { - if (pDataSourceBase->loopEndInFrames != ~((ma_uint64)0)) { - rangeEnd = ma_min(rangeEnd, pDataSourceBase->rangeBegInFrames + pDataSourceBase->loopEndInFrames); + /* If looping, make sure we're within range. */ + if (loop) { + if (pDataSourceBase->loopEndInFrames != ~((ma_uint64)0)) { + rangeEnd = ma_min(rangeEnd, pDataSourceBase->rangeBegInFrames + pDataSourceBase->loopEndInFrames); + } } + + if (frameCount > (rangeEnd - cursor) && rangeEnd != ~((ma_uint64)0)) { + frameCount = (rangeEnd - cursor); + } + + result = pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, &framesRead); } - - if (frameCount > (rangeEnd - cursor) && rangeEnd != ~((ma_uint64)0)) { - frameCount = (rangeEnd - cursor); - } - - result = pDataSourceBase->vtable->onRead(pDataSourceBase, pFramesOut, frameCount, &framesRead); - - if (pFramesRead != NULL) { - *pFramesRead = framesRead; - } - - /* We need to make sure MA_AT_END is returned if we hit the end of the range. */ - if (result != MA_AT_END && framesRead == 0) { - result = MA_AT_END; - } - - return result; } + + if (pFramesRead != NULL) { + *pFramesRead = framesRead; + } + + /* We need to make sure MA_AT_END is returned if we hit the end of the range. */ + if (result == MA_SUCCESS && framesRead == 0) { + result = MA_AT_END; + } + + return result; } MA_API ma_result ma_data_source_read_pcm_frames(ma_data_source* pDataSource, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead, ma_bool32 loop) @@ -46094,6 +46099,10 @@ MA_API ma_result ma_data_source_read_pcm_frames(ma_data_source* pDataSource, voi *pFramesRead = 0; } + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pDataSourceBase == NULL) { return MA_INVALID_ARGS; } @@ -46206,6 +46215,10 @@ MA_API ma_result ma_data_source_read_pcm_frames(ma_data_source* pDataSource, voi *pFramesRead = totalFramesProcessed; } + if (result == MA_SUCCESS && totalFramesProcessed == 0) { + result = MA_AT_END; + } + return result; } @@ -50111,6 +50124,14 @@ MA_API void ma_wav_uninit(ma_wav* pWav, const ma_allocation_callbacks* pAllocati MA_API ma_result ma_wav_read_pcm_frames(ma_wav* pWav, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pWav == NULL) { return MA_INVALID_ARGS; } @@ -50158,6 +50179,10 @@ MA_API ma_result ma_wav_read_pcm_frames(ma_wav* pWav, void* pFramesOut, ma_uint6 *pFramesRead = totalFramesRead; } + if (result == MA_SUCCESS && totalFramesRead == 0) { + result = MA_AT_END; + } + return result; } #else @@ -50736,6 +50761,14 @@ MA_API void ma_flac_uninit(ma_flac* pFlac, const ma_allocation_callbacks* pAlloc MA_API ma_result ma_flac_read_pcm_frames(ma_flac* pFlac, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pFlac == NULL) { return MA_INVALID_ARGS; } @@ -50784,6 +50817,10 @@ MA_API ma_result ma_flac_read_pcm_frames(ma_flac* pFlac, void* pFramesOut, ma_ui *pFramesRead = totalFramesRead; } + if (result == MA_SUCCESS && totalFramesRead == 0) { + result = MA_AT_END; + } + return result; } #else @@ -51360,6 +51397,14 @@ MA_API void ma_mp3_uninit(ma_mp3* pMP3, const ma_allocation_callbacks* pAllocati MA_API ma_result ma_mp3_read_pcm_frames(ma_mp3* pMP3, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pMP3 == NULL) { return MA_INVALID_ARGS; } @@ -52013,6 +52058,14 @@ MA_API void ma_stbvorbis_uninit(ma_stbvorbis* pVorbis, const ma_allocation_callb MA_API ma_result ma_stbvorbis_read_pcm_frames(ma_stbvorbis* pVorbis, void* pFramesOut, ma_uint64 frameCount, ma_uint64* pFramesRead) { + if (pFramesRead != NULL) { + *pFramesRead = 0; + } + + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pVorbis == NULL) { return MA_INVALID_ARGS; } @@ -52148,6 +52201,10 @@ MA_API ma_result ma_stbvorbis_read_pcm_frames(ma_stbvorbis* pVorbis, void* pFram *pFramesRead = totalFramesRead; } + if (result == MA_SUCCESS && totalFramesRead == 0) { + result = MA_AT_END; + } + return result; } #else @@ -53310,6 +53367,10 @@ MA_API ma_result ma_decoder_read_pcm_frames(ma_decoder* pDecoder, void* pFramesO *pFramesRead = 0; /* Safety. */ } + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + if (pDecoder == NULL) { return MA_INVALID_ARGS; } @@ -57145,6 +57206,10 @@ MA_API ma_result ma_resource_manager_data_buffer_read_pcm_frames(ma_resource_man *pFramesRead = 0; } + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + /* We cannot be using the data buffer after it's been uninitialized. If you trigger this assert it means you're trying to read from the data buffer after it's been uninitialized or is in the process of uninitializing. @@ -57223,6 +57288,10 @@ MA_API ma_result ma_resource_manager_data_buffer_read_pcm_frames(ma_resource_man *pFramesRead = framesRead; } + if (result == MA_SUCCESS && framesRead == 0) { + result = MA_AT_END; + } + return result; } @@ -57940,6 +58009,10 @@ MA_API ma_result ma_resource_manager_data_stream_read_pcm_frames(ma_resource_man *pFramesRead = 0; } + if (frameCount == 0) { + return MA_INVALID_ARGS; + } + /* We cannot be using the data source after it's been uninitialized. */ MA_ASSERT(ma_resource_manager_data_stream_result(pDataStream) != MA_UNAVAILABLE); @@ -57987,6 +58060,10 @@ MA_API ma_result ma_resource_manager_data_stream_read_pcm_frames(ma_resource_man *pFramesRead = totalFramesProcessed; } + if (result == MA_SUCCESS && totalFramesProcessed == 0) { + result = MA_AT_END; + } + return result; } @@ -61090,6 +61167,9 @@ static void ma_data_source_node_process_pcm_frames(ma_node* pNode, const float** frameCount = *pFrameCountOut; + /* miniaudio should never be calling this with a frame count of zero. */ + MA_ASSERT(frameCount > 0); + if (ma_data_source_get_data_format(pDataSourceNode->pDataSource, &format, &channels, NULL, NULL, 0) == MA_SUCCESS) { /* <-- Don't care about sample rate here. */ /* The node graph system requires samples be in floating point format. This is checked in ma_data_source_node_init(). */ MA_ASSERT(format == ma_format_f32);