Bug 1190238: P3. Do not loop calling MediaResource::Read or ReadAt, let MediaResourceIndex do it for us. r=cpearce

This allows to remove a fair amount of duplicated logic.
Most of it is in obsoleted code though.

dom/media/apple/AppleMP3Reader.cpp

@@ -91,25 +91,15 @@ static void _AudioSampleCallback(void *aThis,
 nsresult
 AppleMP3Reader::Read(uint32_t *aNumBytes, char *aData)
 {
-  // Loop until we have all the data asked for, or we've reached EOS
-  uint32_t totalBytes = 0;
-  uint32_t numBytes;
-  do {
-    uint32_t bytesWanted = *aNumBytes - totalBytes;
-    nsresult rv = mResource.Read(aData + totalBytes, bytesWanted, &numBytes);
-    totalBytes += numBytes;
+  nsresult rv = mResource.Read(aData, *aNumBytes, aNumBytes);
 
-    if (NS_FAILED(rv)) {
-      *aNumBytes = 0;
-      return NS_ERROR_FAILURE;
-    }
-  } while(totalBytes < *aNumBytes && numBytes);
+  if (NS_FAILED(rv)) {
+    *aNumBytes = 0;
+    return NS_ERROR_FAILURE;
+  }
 
-  *aNumBytes = totalBytes;
-
-  // We will have read some data in the last iteration iff we filled the buffer.
   // XXX Maybe return a better value than NS_ERROR_FAILURE?
-  return numBytes ? NS_OK : NS_ERROR_FAILURE;
+  return *aNumBytes ? NS_OK : NS_ERROR_FAILURE;
 }
 
 nsresult

dom/media/directshow/SourceFilter.cpp

@@ -76,7 +76,7 @@ public:
   {}
 
   int64_t GetLength() {
-    int64_t len = mResource->GetLength();
+    int64_t len = mResource.GetLength();
     if (len == -1) {
       return len;
     }
@@ -85,19 +85,20 @@ public:
   nsresult ReadAt(int64_t aOffset, char* aBuffer,
                   uint32_t aCount, uint32_t* aBytes)
   {
-    return mResource->ReadAt(aOffset + mDataOffset,
-                             aBuffer,
-                             aCount,
-                             aBytes);
+    return mResource.ReadAt(aOffset + mDataOffset,
+                            aBuffer,
+                            aCount,
+                            aBytes);
   }
   int64_t GetCachedDataEnd() {
-    int64_t tell = mResource->Tell();
-    int64_t dataEnd = mResource->GetCachedDataEnd(tell) - mDataOffset;
+    int64_t tell = mResource.GetResource()->Tell();
+    int64_t dataEnd =
+      mResource.GetResource()->GetCachedDataEnd(tell) - mDataOffset;
     return dataEnd;
   }
 private:
   // MediaResource from which we read data.
-  RefPtr<MediaResource> mResource;
+  MediaResourceIndex mResource;
   int64_t mDataOffset;
 };
 
@@ -559,23 +560,13 @@ OutputPin::SyncRead(LONGLONG aPosition,
     }
   }
 
-  // Read in a loop to ensure we fill the buffer, when possible.
-  LONG totalBytesRead = 0;
-  while (totalBytesRead < aLength) {
-    BYTE* readBuffer = aBuffer + totalBytesRead;
-    uint32_t bytesRead = 0;
-    LONG length = aLength - totalBytesRead;
-    nsresult rv = mResource.ReadAt(aPosition + totalBytesRead,
-                                   reinterpret_cast<char*>(readBuffer),
-                                   length,
-                                   &bytesRead);
-    if (NS_FAILED(rv)) {
-      return E_FAIL;
-    }
-    totalBytesRead += bytesRead;
-    if (bytesRead == 0) {
-      break;
-    }
+  uint32_t totalBytesRead = 0;
+  nsresult rv = mResource.ReadAt(aPosition,
+                                 reinterpret_cast<char*>(aBuffer),
+                                 aLength,
+                                 &totalBytesRead);
+  if (NS_FAILED(rv)) {
+    return E_FAIL;
   }
   if (totalBytesRead > 0) {
     CriticalSectionAutoEnter lock(*mLock);

dom/media/fmp4/MP4Stream.cpp

@@ -32,22 +32,17 @@ MP4Stream::BlockingReadIntoCache(int64_t aOffset, size_t aCount, Monitor* aToUnl
     return false;
   }
 
-  uint32_t sum = 0;
   uint32_t bytesRead = 0;
-  do {
-    uint64_t offset = aOffset + sum;
-    char* buffer = block.Buffer() + sum;
-    uint32_t toRead = aCount - sum;
+  {
     MonitorAutoUnlock unlock(*aToUnlock);
-    nsresult rv = mResource->ReadAt(offset, buffer, toRead, &bytesRead);
+    nsresult rv = mResource.ReadAt(aOffset, block.Buffer(), aCount, &bytesRead);
     if (NS_FAILED(rv)) {
       return false;
     }
-    sum += bytesRead;
-  } while (sum < aCount && bytesRead > 0);
+  }
 
-  MOZ_ASSERT(block.mCount >= sum);
-  block.mCount = sum;
+  MOZ_ASSERT(block.mCount >= bytesRead);
+  block.mCount = bytesRead;
 
   mCache.AppendElement(block);
   return true;
@@ -85,8 +80,9 @@ MP4Stream::CachedReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
     }
   }
 
-  nsresult rv = mResource->ReadFromCache(reinterpret_cast<char*>(aBuffer),
-                                         aOffset, aCount);
+  nsresult rv =
+    mResource.GetResource()->ReadFromCache(reinterpret_cast<char*>(aBuffer),
+                                           aOffset, aCount);
   if (NS_FAILED(rv)) {
     *aBytesRead = 0;
     return false;
@@ -98,9 +94,9 @@ MP4Stream::CachedReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
 bool
 MP4Stream::Length(int64_t* aSize)
 {
-  if (mResource->GetLength() < 0)
+  if (mResource.GetLength() < 0)
     return false;
-  *aSize = mResource->GetLength();
+  *aSize = mResource.GetLength();
   return true;
 }
 

dom/media/fmp4/MP4Stream.h

@@ -49,13 +49,13 @@ public:
 
   void Pin()
   {
-    mResource->Pin();
+    mResource.GetResource()->Pin();
     ++mPinCount;
   }
 
   void Unpin()
   {
-    mResource->Unpin();
+    mResource.GetResource()->Unpin();
     MOZ_ASSERT(mPinCount);
     --mPinCount;
     if (mPinCount == 0) {
@@ -64,7 +64,7 @@ public:
   }
 
 private:
-  nsRefPtr<MediaResource> mResource;
+  MediaResourceIndex mResource;
   Maybe<ReadRecord> mFailedRead;
   uint32_t mPinCount;
 

dom/media/ogg/OggReader.cpp

@@ -950,8 +950,8 @@ bool OggReader::ReadOggPage(ogg_page* aPage)
     uint32_t bytesRead = 0;
 
     nsresult rv = mResource.Read(buffer, 4096, &bytesRead);
-    if (NS_FAILED(rv) || (bytesRead == 0 && ret == 0)) {
-      // End of file.
+    if (NS_FAILED(rv) || !bytesRead) {
+      // End of file or error.
       return false;
     }
 

dom/media/raw/RawReader.cpp

@@ -123,19 +123,13 @@ RawReader::IsMediaSeekable()
 // or returns false
 bool RawReader::ReadFromResource(uint8_t* aBuf, uint32_t aLength)
 {
-  while (aLength > 0) {
-    uint32_t bytesRead = 0;
-    nsresult rv;
+  uint32_t bytesRead = 0;
+  nsresult rv;
 
-    rv = mResource.Read(reinterpret_cast<char*>(aBuf), aLength, &bytesRead);
-    NS_ENSURE_SUCCESS(rv, false);
-
-    if (bytesRead == 0) {
-      return false;
-    }
-
-    aLength -= bytesRead;
-    aBuf += bytesRead;
+  rv = mResource.Read(reinterpret_cast<char*>(aBuf), aLength, &bytesRead);
+  NS_ENSURE_SUCCESS(rv, false);
+  if (bytesRead == 0) {
+    return false;
   }
 
   return true;

dom/media/wave/WaveReader.cpp

@@ -302,24 +302,20 @@ media::TimeIntervals WaveReader::GetBuffered()
 bool
 WaveReader::ReadAll(char* aBuf, int64_t aSize, int64_t* aBytesRead)
 {
-  uint32_t got = 0;
   if (aBytesRead) {
     *aBytesRead = 0;
   }
-  do {
-    uint32_t read = 0;
-    if (NS_FAILED(mResource.Read(aBuf + got, uint32_t(aSize - got), &read))) {
-      NS_WARNING("Resource read failed");
-      return false;
-    }
-    if (read == 0) {
-      return false;
-    }
-    got += read;
-    if (aBytesRead) {
-      *aBytesRead = got;
-    }
-  } while (got != aSize);
+  uint32_t read = 0;
+  if (NS_FAILED(mResource.Read(aBuf, uint32_t(aSize), &read))) {
+    NS_WARNING("Resource read failed");
+    return false;
+  }
+  if (!read) {
+    return false;
+  }
+  if (aBytesRead) {
+    *aBytesRead = read;
+  }
   return true;
 }
 

dom/media/webm/WebMReader.cpp

@@ -57,19 +57,11 @@ static int webm_read(void *aBuffer, size_t aLength, void *aUserData)
     reinterpret_cast<MediaResourceIndex*>(aUserData);
 
   nsresult rv = NS_OK;
-  bool eof = false;
+  uint32_t bytes = 0;
 
-  char *p = static_cast<char *>(aBuffer);
-  while (NS_SUCCEEDED(rv) && aLength > 0) {
-    uint32_t bytes = 0;
-    rv = resource->Read(p, aLength, &bytes);
-    if (bytes == 0) {
-      eof = true;
-      break;
-    }
-    aLength -= bytes;
-    p += bytes;
-  }
+  rv = resource->Read(static_cast<char *>(aBuffer), aLength, &bytes);
+
+  bool eof = !bytes;
 
   return NS_FAILED(rv) ? -1 : eof ? 0 : 1;
 }