Factor out bounds-checking for GetAudio to a single place

It was being done in inconsistent ways in several places, which did not
include all of the places it needed to be done.

Closes #1509.

Originally committed to SVN as r6960.

aegisub/src/audio_provider.cpp

@@ -88,6 +88,44 @@ void AudioProvider::GetAudioWithVolume(void *buf, int64_t start, int64_t count,
 	}
 }
 
+void AudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const {
+	if (start+count > num_samples) {
+		int64_t oldcount = count;
+		count = num_samples-start;
+		if (count < 0) count = 0;
+
+		// Fill beyond with zero
+		if (bytes_per_sample == 1) {
+			char *temp = (char *) buf;
+			for (int i=count;i<oldcount;i++) {
+				temp[i] = 0;
+			}
+		}
+		if (bytes_per_sample == 2) {
+			short *temp = (short *) buf;
+			for (int i=count;i<oldcount;i++) {
+				temp[i] = 0;
+			}
+		}
+	}
+
+	if (start + count > num_samples) {
+		int64_t zero_count = std::min(count, start + count - num_samples);
+		count -= zero_count;
+		char *zero_buf = static_cast<char *>(buf) + count * bytes_per_sample * channels;
+
+		if (bytes_per_sample == 1)
+			// 8 bit formats are usually unsigned with bias 127
+			memset(zero_buf, 127, zero_count * channels);
+		else
+			// While everything else is signed
+			memset(zero_buf, 0, zero_count * bytes_per_sample * channels);
+	}
+
+	if (count > 0)
+		FillBuffer(buf, start, count);
+}
+
 AudioProvider *AudioProviderFactory::GetProvider(wxString const& filename, int cache) {
 	AudioProvider *provider = 0;
 	bool found_file = false;

aegisub/src/audio_provider_avs.cpp

@@ -151,35 +151,7 @@ void AvisynthAudioProvider::LoadFromClip(AVSValue _clip) {
 	clip = tempclip;
 }
 
-/// @brief Get audio
+void AvisynthAudioProvider::FillBuffer(void *buf, int64_t start, int64_t count) const {
-/// @param buf
+	clip->GetAudio(buf,start,count,avs_wrapper.GetEnv());
-/// @param start
-/// @param count
-///
-void AvisynthAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const {
-	// Requested beyond the length of audio
-	if (start+count > num_samples) {
-		int64_t oldcount = count;
-		count = num_samples-start;
-		if (count < 0) count = 0;
-
-		// Fill beyond with zero
-		if (bytes_per_sample == 1) {
-			char *temp = (char *) buf;
-			for (int i=count;i<oldcount;i++) {
-				temp[i] = 0;
-			}
-		}
-		if (bytes_per_sample == 2) {
-			short *temp = (short *) buf;
-			for (int i=count;i<oldcount;i++) {
-				temp[i] = 0;
-			}
-		}
-	}
-
-	if (count) {
-		clip->GetAudio(buf,start,count,avs_wrapper.GetEnv());
-	}
 }
 #endif

aegisub/src/audio_provider_avs.h

@@ -56,6 +56,7 @@ class AvisynthAudioProvider : public AudioProvider {
 	PClip clip;
 
 	void LoadFromClip(AVSValue clip);
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
 
 public:
 	AvisynthAudioProvider(wxString _filename);
@@ -64,7 +65,5 @@ public:
 
 	bool AreSamplesNativeEndian() const { return true; }
 	bool NeedsCache() const { return true; }
-
-	void GetAudio(void *buf, int64_t start, int64_t count) const;
 };
 #endif

aegisub/src/audio_provider_convert.cpp

@@ -67,7 +67,7 @@ public:
 		bytes_per_sample = sizeof(Target);
 	}
 
-	void GetAudio(void *buf, int64_t start, int64_t count) const {
+	void FillBuffer(void *buf, int64_t start, int64_t count) const {
 		std::vector<char> src_buf(count * src_bytes_per_sample * channels);
 		source->GetAudio(&src_buf[0], start, count);
 
@@ -120,7 +120,7 @@ public:
 		float_samples = false;
 	}
 
-	void GetAudio(void *buf, int64_t start, int64_t count) const {
+	void FillBuffer(void *buf, int64_t start, int64_t count) const {
 		std::vector<Source> src_buf(count * channels);
 		source->GetAudio(&src_buf[0], start, count);
 
@@ -156,7 +156,7 @@ public:
 		channels = 1;
 	}
 
-	void GetAudio(void *buf, int64_t start, int64_t count) const {
+	void FillBuffer(void *buf, int64_t start, int64_t count) const {
 		if (count == 0) return;
 
 		std::vector<int16_t> src_buf(count * src_channels);
@@ -187,7 +187,7 @@ public:
 		num_samples *= 2;
 	}
 
-	void GetAudio(void *buf, int64_t start, int64_t count) const {
+	void FillBuffer(void *buf, int64_t start, int64_t count) const {
 		if (count == 0) return;
 
 		int not_end = start + count < num_samples;

aegisub/src/audio_provider_dummy.cpp

@@ -48,7 +48,7 @@ DummyAudioProvider::DummyAudioProvider(unsigned long dur_ms, bool _noise) {
 	num_samples = (int64_t)dur_ms * sample_rate / 1000;
 }
 
-void DummyAudioProvider::GetAudio(void *buf, int64_t, int64_t count) const {
+void DummyAudioProvider::FillBuffer(void *buf, int64_t, int64_t count) const {
 	short *workbuf = (short*)buf;
 
 	if (noise) {

aegisub/src/audio_provider_dummy.h

@@ -44,10 +44,10 @@
 class DummyAudioProvider : public AudioProvider {
 	/// DOCME
 	bool noise;
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
 
 public:
 	DummyAudioProvider(unsigned long dur_ms, bool _noise);
 
 	bool AreSamplesNativeEndian() const { return true; }
-	void GetAudio(void *buf, int64_t start, int64_t count) const;
 };

aegisub/src/audio_provider_ffmpegsource.cpp

@@ -175,7 +175,7 @@ void FFmpegSourceAudioProvider::LoadAudio(wxString filename) {
 	}
 }
 
-void FFmpegSourceAudioProvider::GetAudio(void *Buf, int64_t Start, int64_t Count) const {
+void FFmpegSourceAudioProvider::FillBuffer(void *Buf, int64_t Start, int64_t Count) const {
 	if (FFMS_GetAudio(AudioSource, Buf, Start, Count, &ErrInfo)) {
 		throw AudioDecodeError(std::string("Failed to get audio samples: ") + ErrInfo.Buffer);
 	}

aegisub/src/audio_provider_ffmpegsource.h

@@ -49,6 +49,7 @@ class FFmpegSourceAudioProvider : public AudioProvider, FFmpegSourceProvider {
 	mutable FFMS_ErrorInfo ErrInfo;			///< FFMS error codes/messages
 
 	void LoadAudio(wxString filename);
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
 
 public:
 	FFmpegSourceAudioProvider(wxString filename);
@@ -58,7 +59,5 @@ public:
 	/// FFMS always delivers native endian samples.
 	bool AreSamplesNativeEndian() const { return true; }
 	bool NeedsCache() const { return true; }
-
-	virtual void GetAudio(void *buf, int64_t start, int64_t count) const;
 };
 #endif

aegisub/src/audio_provider_hd.cpp

@@ -140,7 +140,7 @@ HDAudioProvider::~HDAudioProvider() {
 	wxRemoveFile(diskCacheFilename);
 }
 
-void HDAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const {
+void HDAudioProvider::FillBuffer(void *buf, int64_t start, int64_t count) const {
 	cache_provider->GetAudio(buf, start, count);
 }
 

aegisub/src/audio_provider_hd.h

@@ -64,11 +64,11 @@ class HDAudioProvider : public AudioProvider {
 	/// @param ps Sink for progress reporting
 	void FillCache(AudioProvider *src, std::ofstream *file, agi::ProgressSink *ps);
 
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
+
 public:
 	HDAudioProvider(AudioProvider *source, agi::BackgroundRunner *br);
 	~HDAudioProvider();
 
 	bool AreSamplesNativeEndian() const { return true; }
-
-	void GetAudio(void *buf, int64_t start, int64_t count) const;
 };

aegisub/src/audio_provider_lock.cpp

@@ -30,7 +30,7 @@ LockAudioProvider::LockAudioProvider(AudioProvider *source) : source(source) {
 	float_samples = source->AreSamplesFloat();
 }
 
-void LockAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const {
+void LockAudioProvider::FillBuffer(void *buf, int64_t start, int64_t count) const {
 	wxMutexLocker lock(mutex);
 	source->GetAudio(buf, start, count);
 }

aegisub/src/audio_provider_lock.h

@@ -30,8 +30,8 @@ class LockAudioProvider : public AudioProvider {
 	agi::scoped_ptr<const AudioProvider> source;
 	mutable wxMutex mutex;
 
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
 public:
 	LockAudioProvider(AudioProvider *source);
-	void GetAudio(void *buf, int64_t start, int64_t count) const;
 	bool AreSamplesNativeEndian() const { return source->AreSamplesNativeEndian(); }
 };

aegisub/src/audio_provider_pcm.cpp

@@ -59,10 +59,6 @@
 #include "compat.h"
 #include "utils.h"
 
-
-/// @brief DOCME
-/// @param filename
-///
 PCMAudioProvider::PCMAudioProvider(const wxString &filename)
 #ifdef _WIN32
 : file_handle(0, CloseHandle)
@@ -117,8 +113,6 @@ PCMAudioProvider::PCMAudioProvider(const wxString &filename)
 	float_samples = false;
 }
 
-/// @brief DOCME
-///
 PCMAudioProvider::~PCMAudioProvider()
 {
 #ifdef _WIN32
@@ -130,11 +124,6 @@ PCMAudioProvider::~PCMAudioProvider()
 #endif
 }
 
-/// @brief DOCME
-/// @param range_start
-/// @param range_length
-/// @return
-///
 char * PCMAudioProvider::EnsureRangeAccessible(int64_t range_start, int64_t range_length) const
 {
 	if (range_start + range_length > file_size) {
@@ -203,12 +192,7 @@ char * PCMAudioProvider::EnsureRangeAccessible(int64_t range_start, int64_t rang
 	return ((char*)current_mapping) + rel_ofs;
 }
 
-/// @brief DOCME
+void PCMAudioProvider::FillBuffer(void *buf, int64_t start, int64_t count) const
-/// @param buf
-/// @param start
-/// @param count
-///
-void PCMAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const
 {
 	// Read blocks from the file
 	size_t index = 0;
@@ -234,16 +218,6 @@ void PCMAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const
 		}
 		index++;
 	}
-
-	// If we exhausted all sample sections zerofill the rest
-	if (count > 0) {
-		if (bytes_per_sample == 1)
-			// 8 bit formats are usually unsigned with bias 127
-			memset(buf, 127, count*channels);
-		else
-			// While everything else is signed
-			memset(buf, 0, count*bytes_per_sample*channels);
-	}
 }
 
 /// DOCME
@@ -308,11 +282,8 @@ class  RiffWavPCMAudioProvider : public PCMAudioProvider {
 
 public:
 
-	/// @brief DOCME
-	/// @param _filename
-	///
 	RiffWavPCMAudioProvider(const wxString &_filename)
-		: PCMAudioProvider(_filename)
+	: PCMAudioProvider(_filename)
 	{
 		filename = _filename;
 
@@ -388,9 +359,6 @@ public:
 		}
 	}
 
-	/// @brief DOCME
-	/// @return
-	///
 	bool AreSamplesNativeEndian() const
 	{
 		// 8 bit samples don't consider endianness
@@ -401,34 +369,26 @@ public:
 	}
 };
 
-/// DOCME
 static const uint8_t w64GuidRIFF[16] = {
 	// {66666972-912E-11CF-A5D6-28DB04C10000}
 	0x72, 0x69, 0x66, 0x66, 0x2E, 0x91, 0xCF, 0x11, 0xA5, 0xD6, 0x28, 0xDB, 0x04, 0xC1, 0x00, 0x00
 };
 
-
-/// DOCME
 static const uint8_t w64GuidWAVE[16] = {
 	// {65766177-ACF3-11D3-8CD1-00C04F8EDB8A}
 	0x77, 0x61, 0x76, 0x65, 0xF3, 0xAC, 0xD3, 0x11, 0x8C, 0xD1, 0x00, 0xC0, 0x4F, 0x8E, 0xDB, 0x8A
 };
 
-
-/// DOCME
 static const uint8_t w64Guidfmt[16] = {
 	// {20746D66-ACF3-11D3-8CD1-00C04F8EDB8A}
 	0x66, 0x6D, 0x74, 0x20, 0xF3, 0xAC, 0xD3, 0x11, 0x8C, 0xD1, 0x00, 0xC0, 0x4F, 0x8E, 0xDB, 0x8A
 };
 
-
-/// DOCME
 static const uint8_t w64Guiddata[16] = {
 	// {61746164-ACF3-11D3-8CD1-00C04F8EDB8A}
 	0x64, 0x61, 0x74, 0x61, 0xF3, 0xAC, 0xD3, 0x11, 0x8C, 0xD1, 0x00, 0xC0, 0x4F, 0x8E, 0xDB, 0x8A
 };
 
-
 /// DOCME
 /// @class Wave64AudioProvider
 /// @brief Sony Wave64 audio provider
@@ -448,49 +408,24 @@ class Wave64AudioProvider : public PCMAudioProvider {
 		uint16_t cbSize;
 	};
 
-	/// DOCME
 	struct RiffChunk {
-		/// DOCME
 		uint8_t riff_guid[16];
-
-		/// DOCME
 		uint64_t file_size;
-
-		/// DOCME
 		uint8_t format_guid[16];
 	};
 
-
-	/// DOCME
 	struct FormatChunk {
-		/// DOCME
 		uint8_t chunk_guid[16];
-
-		/// DOCME
 		uint64_t chunk_size;
-
-		/// DOCME
 		WaveFormatEx format;
-
-		/// DOCME
 		uint8_t padding[6];
 	};
 
-
-	/// DOCME
 	struct DataChunk {
-		/// DOCME
 		uint8_t chunk_guid[16];
-
-		/// DOCME
 		uint64_t chunk_size;
 	};
 
-	/// @brief DOCME
-	/// @param guid1
-	/// @param guid2
-	/// @return
-	///
 	inline bool CheckGuid(const uint8_t *guid1, const uint8_t *guid2)
 	{
 		return memcmp(guid1, guid2, 16) == 0;
@@ -498,9 +433,6 @@ class Wave64AudioProvider : public PCMAudioProvider {
 
 public:
 
-	/// @brief DOCME
-	/// @param _filename
-	///
 	Wave64AudioProvider(const wxString &_filename)
 	: PCMAudioProvider(_filename)
 	{
@@ -579,9 +511,6 @@ public:
 		}
 	}
 
-	/// @brief DOCME
-	/// @return
-	///
 	bool AreSamplesNativeEndian() const
 	{
 		// 8 bit samples don't consider endianness

aegisub/src/audio_provider_pcm.h

@@ -57,19 +57,12 @@
 /// DOCME
 class PCMAudioProvider : public AudioProvider {
 #ifdef _WIN32
-	/// DOCME
 	agi::scoped_holder<HANDLE, BOOL (__stdcall *)(HANDLE)> file_handle;
-
-	/// DOCME
 	agi::scoped_holder<HANDLE, BOOL (__stdcall *)(HANDLE)> file_mapping;
 
-	/// DOCME
 	mutable void *current_mapping;
 
-	/// DOCME
 	mutable int64_t mapping_start;
-
-	/// DOCME
 	mutable size_t mapping_length;
 #else
 	agi::scoped_holder<int, int(*)(int)> file_handle;
@@ -83,32 +76,20 @@ protected:
 	virtual ~PCMAudioProvider(); // Closes the file mapping
 	char * EnsureRangeAccessible(int64_t range_start, int64_t range_length) const; // Ensure that the given range of bytes are accessible in the file mapping and return a pointer to the first byte of the requested range
 
+	/// Size of the opened file
+	int64_t file_size;
 
-	/// DOCME
-	int64_t file_size; // Size of the opened file
-
-
-	/// DOCME
 	struct IndexPoint {
-
-		/// DOCME
 		int64_t start_byte;
-
-		/// DOCME
 		int64_t start_sample;
-
-		/// DOCME
 		int64_t num_samples;
 	};
 
-	/// DOCME
 	typedef std::vector<IndexPoint> IndexVector;
 
-	/// DOCME
 	IndexVector index_points;
 
-public:
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
-	virtual void GetAudio(void *buf, int64_t start, int64_t count) const;
 };
 
 // Construct the right PCM audio provider (if any) for the file

aegisub/src/audio_provider_ram.cpp

@@ -112,45 +112,22 @@ void RAMAudioProvider::Clear() {
 	}
 }
 
-void RAMAudioProvider::GetAudio(void *buf, int64_t start, int64_t count) const {
+void RAMAudioProvider::FillBuffer(void *buf, int64_t start, int64_t count) const {
-	// Requested beyond the length of audio
+	// Prepare copy
-	if (start+count > num_samples) {
+	char *charbuf = (char *)buf;
-		int64_t oldcount = count;
+	int i = (start*bytes_per_sample) >> CacheBits;
-		count = num_samples-start;
+	int start_offset = (start*bytes_per_sample) & (CacheBlockSize-1);
-		if (count < 0) count = 0;
+	int64_t bytesremaining = count*bytes_per_sample;
 
-		// Fill beyond with zero
+	// Copy
-		if (bytes_per_sample == 1) {
+	while (bytesremaining) {
-			char *temp = (char *) buf;
+		int readsize = std::min<int>(bytesremaining, CacheBlockSize - start_offset);
-			for (int i=count;i<oldcount;i++) {
-				temp[i] = 0;
-			}
-		}
-		if (bytes_per_sample == 2) {
-			short *temp = (short *) buf;
-			for (int i=count;i<oldcount;i++) {
-				temp[i] = 0;
-			}
-		}
-	}
 
-	if (count) {
+		memcpy(charbuf,(char *)(blockcache[i++]+start_offset),readsize);
-		// Prepare copy
-		char *charbuf = (char *)buf;
-		int i = (start*bytes_per_sample) >> CacheBits;
-		int start_offset = (start*bytes_per_sample) & (CacheBlockSize-1);
-		int64_t bytesremaining = count*bytes_per_sample;
 
-		// Copy
+		charbuf+=readsize;
-		while (bytesremaining) {
-			int readsize = std::min<int>(bytesremaining, CacheBlockSize - start_offset);
 
-			memcpy(charbuf,(char *)(blockcache[i++]+start_offset),readsize);
+		start_offset=0;
-
+		bytesremaining-=readsize;
-			charbuf+=readsize;
-
-			start_offset=0;
-			bytesremaining-=readsize;
-		}
 	}
 }

aegisub/src/audio_provider_ram.h

@@ -58,11 +58,11 @@ class RAMAudioProvider : public AudioProvider {
 
 	void Clear();
 	void FillCache(AudioProvider *source, agi::ProgressSink *ps);
+	void FillBuffer(void *buf, int64_t start, int64_t count) const;
 
 public:
 	RAMAudioProvider(AudioProvider *source, agi::BackgroundRunner *br);
 	~RAMAudioProvider();
 
 	bool AreSamplesNativeEndian() const { return samples_native_endian; }
-	void GetAudio(void *buf, int64_t start, int64_t count) const;
 };

aegisub/src/include/aegisub/audio_provider.h

@@ -67,11 +67,13 @@ protected:
 	/// DOCME
 	wxString filename;
 
+	virtual void FillBuffer(void *buf, int64_t start, int64_t count) const = 0;
+
 public:
 	virtual ~AudioProvider() { }
 
 	virtual wxString GetFilename() const { return filename; };
-	virtual void GetAudio(void *buf, int64_t start, int64_t count) const = 0;
+	void GetAudio(void *buf, int64_t start, int64_t count) const;
 	void GetAudioWithVolume(void *buf, int64_t start, int64_t count, double volume) const;
 
 	int64_t GetNumSamples() const { return num_samples; }