Bug 1535766 - Handle unaligned buffers in WebrtcGmpVideoCodec::Encoded; r=bwc
authorDan Minor <dminor@mozilla.com>
Tue, 19 Mar 2019 17:15:39 +0000
changeset 465071 1239a10882e9609b39ff38a1adcb44e92339fc0a
parent 465070 3653c185546ba9b9e720cc135a28f87f09ba1ac7
child 465072 117ee5068079863807c27cdb6b7e63c412f7bdb7
push id80877
push userdminor@mozilla.com
push dateTue, 19 Mar 2019 17:21:52 +0000
treeherderautoland@1239a10882e9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1535766
milestone68.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 1535766 - Handle unaligned buffers in WebrtcGmpVideoCodec::Encoded; r=bwc This uses the Endian routines to ensure that reads will match the endianess of the current machine, which is the expected behaviour of the openh264 plugin. The calls to readUint16 and readUint32 memcpy to a properly aligned buffer avoiding any problems with alignment. The memcpy adds some overhead but it seems negligible compared to the amount of work done to packetize and send the encoded data. These changes were tested by adding code to create an unaligned buffer and memcopying the received buffer into it. This also adds a null check for the received buffer as we have seen a small volume of null pointer crashes. Differential Revision: https://phabricator.services.mozilla.com/D24030
media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
--- a/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
+++ b/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
@@ -2,16 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebrtcGmpVideoCodec.h"
 
 #include <vector>
 
 #include "mozilla/CheckedInt.h"
+#include "mozilla/EndianUtils.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/Move.h"
 #include "mozilla/SyncRunnable.h"
 #include "VideoConduit.h"
 #include "AudioConduit.h"
 #include "runnable_utils.h"
 
 #include "mozIGeckoMediaPluginService.h"
@@ -507,16 +508,22 @@ void WebrtcGmpVideoEncoder::Encoded(
           aEncodedFrame->TimeStamp(), aEncodedFrame->BufferType(),
           aEncodedFrame->Size()));
 
     // Right now makes one Encoded() callback per unit
     // XXX convert to FragmentationHeader format (array of offsets and sizes
     // plus a buffer) in combination with H264 packetization changes in
     // webrtc/trunk code
     uint8_t* buffer = aEncodedFrame->Buffer();
+
+    if (!buffer) {
+      LOG(LogLevel::Error, ("GMP plugin returned null buffer"));
+      return;
+    }
+
     uint8_t* end = aEncodedFrame->Buffer() + aEncodedFrame->Size();
     size_t size_bytes;
     switch (aEncodedFrame->BufferType()) {
       case GMP_BufferSingle:
         size_bytes = 0;
         break;
       case GMP_BufferLength8:
         size_bytes = 1;
@@ -550,31 +557,39 @@ void WebrtcGmpVideoEncoder::Encoded(
       switch (aEncodedFrame->BufferType()) {
         case GMP_BufferSingle:
           size = aEncodedFrame->Size();
           break;
         case GMP_BufferLength8:
           size = *buffer++;
           break;
         case GMP_BufferLength16:
-          // presumes we can do unaligned loads
-          size = *(reinterpret_cast<uint16_t*>(buffer));
+// The plugin is expected to encode data in native byte order
+#ifdef MOZ_LITTLE_ENDIAN
+          size = LittleEndian::readUint16(buffer);
+#else
+          size = BigEndian::readUint16(buffer);
+#endif
           buffer += 2;
           break;
         case GMP_BufferLength24:
           // 24-bits is a pain, since byte-order issues make things painful
           // I'm going to define 24-bit as little-endian always; big-endian must
           // convert
           size = ((uint32_t)*buffer) | (((uint32_t) * (buffer + 1)) << 8) |
                  (((uint32_t) * (buffer + 2)) << 16);
           buffer += 3;
           break;
         case GMP_BufferLength32:
-          // presumes we can do unaligned loads
-          size = *(reinterpret_cast<uint32_t*>(buffer));
+// The plugin is expected to encode data in native byte order
+#ifdef MOZ_LITTLE_ENDIAN
+          size = LittleEndian::readUint32(buffer);
+#else
+          size = BigEndian::readUint32(buffer);
+#endif
           buffer += 4;
           break;
         default:
           MOZ_CRASH("GMP_BufferType already handled in switch above");
       }
       MOZ_ASSERT(size != 0 &&
                  buffer + size <=
                      end);  // in non-debug code, don't crash in this case