Bug 1007223 - Follow-up: Fix the media decoding test, and also ensure that we're freeing the right buffer. r=Waldo, a=bustage
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 29 May 2014 15:42:49 -0400
changeset 193442 8260d1fb1c34a489223e18de42cdab6b3fa19e28
parent 193441 f24c36fc3a763764f3507168495bcc8d1bb0fed3
child 193443 971b045d6755a7f424c605546cbce8bfe7888d53
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo, bustage
bugs1007223
milestone30.0
Bug 1007223 - Follow-up: Fix the media decoding test, and also ensure that we're freeing the right buffer. r=Waldo, a=bustage Note that I am not fixing the SyncDecodeMedia case to not leak, because that code is disabled, and has been removed from later versions.
content/media/webaudio/AudioContext.cpp
content/media/webaudio/MediaBufferDecoder.cpp
content/media/webaudio/MediaBufferDecoder.h
content/media/webaudio/test/test_mediaDecoding.html
--- a/content/media/webaudio/AudioContext.cpp
+++ b/content/media/webaudio/AudioContext.cpp
@@ -464,17 +464,17 @@ AudioContext::DecodeAudioData(const Arra
 
   nsCOMPtr<DecodeErrorCallback> failureCallback;
   if (aFailureCallback.WasPassed()) {
     failureCallback = &aFailureCallback.Value();
   }
   nsRefPtr<WebAudioDecodeJob> job(
     new WebAudioDecodeJob(contentType, this,
                           &aSuccessCallback, failureCallback));
-  mDecoder.AsyncDecodeMedia(contentType.get(), data, length, *job);
+  mDecoder.AsyncDecodeMedia(contentType.get(), data, dummy, length, *job);
   // Transfer the ownership to mDecodeJobs
   mDecodeJobs.AppendElement(job);
 }
 
 void
 AudioContext::RemoveFromDecodeQueue(WebAudioDecodeJob* aDecodeJob)
 {
   mDecodeJobs.RemoveElement(aDecodeJob);
--- a/content/media/webaudio/MediaBufferDecoder.cpp
+++ b/content/media/webaudio/MediaBufferDecoder.cpp
@@ -84,21 +84,22 @@ MOZ_BEGIN_ENUM_CLASS(PhaseEnum, int)
   AllocateBuffer,
   Done
 MOZ_END_ENUM_CLASS(PhaseEnum)
 
 class MediaDecodeTask : public nsRunnable
 {
 public:
   MediaDecodeTask(const char* aContentType, uint8_t* aBuffer,
-                  uint32_t aLength,
+                  void* aRawBuffer, uint32_t aLength,
                   WebAudioDecodeJob& aDecodeJob,
                   nsIThreadPool* aThreadPool)
     : mContentType(aContentType)
     , mBuffer(aBuffer)
+    , mRawBuffer(aRawBuffer)
     , mLength(aLength)
     , mDecodeJob(aDecodeJob)
     , mPhase(PhaseEnum::Decode)
     , mThreadPool(aThreadPool)
   {
     MOZ_ASSERT(aBuffer);
     MOZ_ASSERT(NS_IsMainThread());
 
@@ -137,22 +138,23 @@ private:
 
   void Cleanup()
   {
     MOZ_ASSERT(NS_IsMainThread());
     // MediaDecoderReader expects that BufferDecoder is alive.
     // Destruct MediaDecoderReader first.
     mDecoderReader = nullptr;
     mBufferDecoder = nullptr;
-    JS_free(nullptr, mBuffer);
+    JS_free(nullptr, mRawBuffer);
   }
 
 private:
   nsCString mContentType;
   uint8_t* mBuffer;
+  void* mRawBuffer;
   uint32_t mLength;
   WebAudioDecodeJob& mDecodeJob;
   PhaseEnum mPhase;
   nsCOMPtr<nsIThreadPool> mThreadPool;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsRefPtr<BufferDecoder> mBufferDecoder;
   nsAutoPtr<MediaDecoderReader> mDecoderReader;
 };
@@ -445,17 +447,17 @@ WebAudioDecodeJob::AllocateBuffer()
     mOutput->SetRawChannelContents(cx, i, mChannelBuffers[i]);
   }
 
   return true;
 }
 
 void
 MediaBufferDecoder::AsyncDecodeMedia(const char* aContentType, uint8_t* aBuffer,
-                                     uint32_t aLength,
+                                     void* aRawBuffer, uint32_t aLength,
                                      WebAudioDecodeJob& aDecodeJob)
 {
   // Do not attempt to decode the media if we were not successful at sniffing
   // the content type.
   if (!*aContentType ||
       strcmp(aContentType, APPLICATION_OCTET_STREAM) == 0) {
     nsCOMPtr<nsIRunnable> event =
       new ReportResultTask(aDecodeJob,
@@ -472,17 +474,17 @@ MediaBufferDecoder::AsyncDecodeMedia(con
                            WebAudioDecodeJob::UnknownError);
     NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
     return;
   }
 
   MOZ_ASSERT(mThreadPool);
 
   nsRefPtr<MediaDecodeTask> task =
-    new MediaDecodeTask(aContentType, aBuffer, aLength, aDecodeJob, mThreadPool);
+    new MediaDecodeTask(aContentType, aBuffer, aRawBuffer, aLength, aDecodeJob, mThreadPool);
   if (!task->CreateReader()) {
     nsCOMPtr<nsIRunnable> event =
       new ReportResultTask(aDecodeJob,
                            &WebAudioDecodeJob::OnFailure,
                            WebAudioDecodeJob::UnknownError);
     NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
   } else {
     mThreadPool->Dispatch(task, nsIThreadPool::DISPATCH_NORMAL);
@@ -497,17 +499,17 @@ MediaBufferDecoder::SyncDecodeMedia(cons
   // Do not attempt to decode the media if we were not successful at sniffing
   // the content type.
   if (!*aContentType ||
       strcmp(aContentType, APPLICATION_OCTET_STREAM) == 0) {
     return false;
   }
 
   nsRefPtr<MediaDecodeTask> task =
-    new MediaDecodeTask(aContentType, aBuffer, aLength, aDecodeJob, nullptr);
+    new MediaDecodeTask(aContentType, aBuffer, nullptr, aLength, aDecodeJob, nullptr);
   if (!task->CreateReader()) {
     return false;
   }
 
   task->Run();
   return true;
 }
 
--- a/content/media/webaudio/MediaBufferDecoder.h
+++ b/content/media/webaudio/MediaBufferDecoder.h
@@ -71,17 +71,17 @@ struct WebAudioDecodeJob MOZ_FINAL
  *
  * This class manages the resources that it uses internally (such as the
  * thread-pool) and provides a clean external interface.
  */
 class MediaBufferDecoder
 {
 public:
   void AsyncDecodeMedia(const char* aContentType, uint8_t* aBuffer,
-                        uint32_t aLength, WebAudioDecodeJob& aDecodeJob);
+                        void* aRawBuffer, uint32_t aLength, WebAudioDecodeJob& aDecodeJob);
 
   bool SyncDecodeMedia(const char* aContentType, uint8_t* aBuffer,
                        uint32_t aLength, WebAudioDecodeJob& aDecodeJob);
 
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
     return 0;
   }
--- a/content/media/webaudio/test/test_mediaDecoding.html
+++ b/content/media/webaudio/test/test_mediaDecoding.html
@@ -293,45 +293,47 @@ function checkResampledBuffer(buffer, te
                    cx.createBuffer(test.numberOfChannels,
                                    test.frames, test.sampleRate));
     callback();
   }
   cx.startRendering();
 }
 
 function runResampling(test, response, callback) {
+  var compressedAudio = response.slice(0);
   var sampleRate = test.sampleRate == 44100 ? 48000 : 44100;
   var cx = new OfflineAudioContext(1, 1, sampleRate);
   cx.decodeAudioData(response, function onSuccess(asyncResult) {
     is(asyncResult.sampleRate, sampleRate, "Correct sample rate");
-    syncResult = cx.createBuffer(response, false);
+    syncResult = cx.createBuffer(compressedAudio, false);
     compareBuffers(syncResult, asyncResult);
 
     checkResampledBuffer(asyncResult, test, callback);
   }, function onFailure() {
     ok(false, "Expected successful decode with resample");
     callback();
   });
 }
 
 function runTest(test, response, callback) {
+  var compressedAudio = response.slice(0);
   var expectCallback = false;
   var cx = new OfflineAudioContext(test.numberOfChannels || 1,
                                    test.frames || 1, test.sampleRate);
   cx.decodeAudioData(response, function onSuccess(asyncResult) {
     ok(expectCallback, "Success callback should fire asynchronously");
     ok(test.valid, "Did expect success for test " + test.url);
 
-    syncResult = cx.createBuffer(response, false);
+    syncResult = cx.createBuffer(compressedAudio, false);
     compareBuffers(syncResult, asyncResult);
     checkAudioBuffer(asyncResult, test);
 
     test.expectedBuffer = asyncResult;
     test.nativeContext = cx;
-    runResampling(test, response, callback);
+    runResampling(test, compressedAudio, callback);
   }, function onFailure() {
     ok(expectCallback, "Failure callback should fire asynchronously");
     ok(!test.valid, "Did expect failure for test " + test.url);
     callback();
   });
   expectCallback = true;
 }