Bug 1524890 - P4. Use Span<> with AudioBufferCursor. r=bryce
authorJean-Yves Avenard <jyavenard@mozilla.com>
Fri, 22 Feb 2019 09:18:05 +0000
changeset 460792 90c5339325a342c3bba190d08ce3dd4564c48319
parent 460791 22d4f90342efbe0893e22f4f32b725b95fcd731a
child 460793 f5daf84b3b7e9a02cf62f3235a56e217daf6c930
push id35603
push userdluca@mozilla.com
push dateMon, 25 Feb 2019 01:46:40 +0000
treeherdermozilla-central@3a2ced4fbd98 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1524890
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1524890 - P4. Use Span<> with AudioBufferCursor. r=bryce And we add some strong assertions that we never read passed the end of the buffer. Differential Revision: https://phabricator.services.mozilla.com/D20162
dom/media/AudioStream.cpp
dom/media/AudioStream.h
dom/media/mediasink/AudioSink.cpp
--- a/dom/media/AudioStream.cpp
+++ b/dom/media/AudioStream.cpp
@@ -622,18 +622,20 @@ void AudioStream::GetTimeStretched(Audio
       },
       aWriter.Available());
 }
 
 long AudioStream::DataCallback(void* aBuffer, long aFrames) {
   MonitorAutoLock mon(mMonitor);
   MOZ_ASSERT(mState != SHUTDOWN, "No data callback after shutdown");
 
-  auto writer = AudioBufferWriter(reinterpret_cast<AudioDataValue*>(aBuffer),
-                                  mOutChannels, aFrames);
+  auto writer = AudioBufferWriter(
+      MakeSpan<AudioDataValue>(reinterpret_cast<AudioDataValue*>(aBuffer),
+                               mOutChannels * aFrames),
+      mOutChannels, aFrames);
 
   if (mPrefillQuirk) {
     // Don't consume audio data until Start() is called.
     // Expected only with cubeb winmm backend.
     if (mState == INITIALIZED) {
       NS_WARNING("data callback fires before cubeb_stream_start() is called");
       mAudioClock.UpdateFrameHistory(0, aFrames);
       return writer.WriteZeros(aFrames);
--- a/dom/media/AudioStream.h
+++ b/dom/media/AudioStream.h
@@ -87,66 +87,81 @@ class AudioClock {
   const nsAutoPtr<FrameHistory> mFrameHistory;
 };
 
 /*
  * A bookkeeping class to track the read/write position of an audio buffer.
  */
 class AudioBufferCursor {
  public:
-  AudioBufferCursor(AudioDataValue* aPtr, uint32_t aChannels, uint32_t aFrames)
-      : mPtr(aPtr), mChannels(aChannels), mFrames(aFrames) {}
+  AudioBufferCursor(Span<AudioDataValue> aSpan, uint32_t aChannels,
+                    uint32_t aFrames)
+      : mChannels(aChannels), mSpan(aSpan), mFrames(aFrames) {}
 
   // Advance the cursor to account for frames that are consumed.
   uint32_t Advance(uint32_t aFrames) {
+    MOZ_DIAGNOSTIC_ASSERT(Contains(aFrames));
     MOZ_ASSERT(mFrames >= aFrames);
     mFrames -= aFrames;
-    mPtr += mChannels * aFrames;
+    mOffset += mChannels * aFrames;
     return aFrames;
   }
 
   // The number of frames available for read/write in this buffer.
   uint32_t Available() const { return mFrames; }
 
   // Return a pointer where read/write should begin.
-  AudioDataValue* Ptr() const { return mPtr; }
+  AudioDataValue* Ptr() const {
+    MOZ_DIAGNOSTIC_ASSERT(mOffset <= mSpan.Length());
+    return mSpan.Elements() + mOffset;
+  }
 
  protected:
-  AudioDataValue* mPtr;
+  bool Contains(uint32_t aFrames) const {
+    return mSpan.Length() >= mOffset + mChannels * aFrames;
+  }
   const uint32_t mChannels;
+
+ private:
+  const Span<AudioDataValue> mSpan;
+  size_t mOffset = 0;
   uint32_t mFrames;
 };
 
 /*
  * A helper class to encapsulate pointer arithmetic and provide means to modify
  * the underlying audio buffer.
  */
 class AudioBufferWriter : private AudioBufferCursor {
  public:
-  AudioBufferWriter(AudioDataValue* aPtr, uint32_t aChannels, uint32_t aFrames)
-      : AudioBufferCursor(aPtr, aChannels, aFrames) {}
+  AudioBufferWriter(Span<AudioDataValue> aSpan, uint32_t aChannels,
+                    uint32_t aFrames)
+      : AudioBufferCursor(aSpan, aChannels, aFrames) {}
 
   uint32_t WriteZeros(uint32_t aFrames) {
-    memset(mPtr, 0, sizeof(AudioDataValue) * mChannels * aFrames);
+    MOZ_DIAGNOSTIC_ASSERT(Contains(aFrames));
+    memset(Ptr(), 0, sizeof(AudioDataValue) * mChannels * aFrames);
     return Advance(aFrames);
   }
 
   uint32_t Write(const AudioDataValue* aPtr, uint32_t aFrames) {
-    memcpy(mPtr, aPtr, sizeof(AudioDataValue) * mChannels * aFrames);
+    MOZ_DIAGNOSTIC_ASSERT(Contains(aFrames));
+    memcpy(Ptr(), aPtr, sizeof(AudioDataValue) * mChannels * aFrames);
     return Advance(aFrames);
   }
 
   // Provide a write fuction to update the audio buffer with the following
   // signature: uint32_t(const AudioDataValue* aPtr, uint32_t aFrames)
   // aPtr: Pointer to the audio buffer.
   // aFrames: The number of frames available in the buffer.
   // return: The number of frames actually written by the function.
   template <typename Function>
   uint32_t Write(const Function& aFunction, uint32_t aFrames) {
-    return Advance(aFunction(mPtr, aFrames));
+    MOZ_DIAGNOSTIC_ASSERT(Contains(aFrames));
+    return Advance(aFunction(Ptr(), aFrames));
   }
 
   using AudioBufferCursor::Available;
 };
 
 // Access to a single instance of this class must be synchronized by
 // callers, or made from a single thread.  One exception is that access to
 // GetPosition, GetPositionInFrames, SetVolume, and Get{Rate,Channels},
--- a/dom/media/mediasink/AudioSink.cpp
+++ b/dom/media/mediasink/AudioSink.cpp
@@ -240,19 +240,18 @@ UniquePtr<AudioStream::Chunk> AudioSink:
     // order to prevent the pop event to fire too early (prior
     // mProcessedQueueLength being updated) or prevent HasUnplayedFrames
     // to incorrectly return true during the time interval betweeen the
     // when mProcessedQueue is read and mWritten is updated.
     needPopping = true;
     mCurrentData = mProcessedQueue.PeekFront();
     {
       MonitorAutoLock mon(mMonitor);
-      mCursor = MakeUnique<AudioBufferCursor>(mCurrentData->Data().Elements(),
-                                              mCurrentData->mChannels,
-                                              mCurrentData->mFrames);
+      mCursor = MakeUnique<AudioBufferCursor>(
+          mCurrentData->Data(), mCurrentData->mChannels, mCurrentData->mFrames);
     }
     MOZ_ASSERT(mCurrentData->mFrames > 0);
     mProcessedQueueLength -=
         FramesToUsecs(mCurrentData->mFrames, mOutputRate).value();
   }
 
   auto framesToPop = std::min(aFrames, mCursor->Available());