Bug 1535766 - Handle unaligned buffers in WebrtcGmpVideoCodec::Encoded; r=bwc a=pascalc
☠☠ backed out by 1dabaf68f8fe ☠ ☠
authorDan Minor <dminor@mozilla.com>
Tue, 19 Mar 2019 17:15:39 +0000
changeset 525659 475f82f18288433b2a972c4d28a784bf323fe70b
parent 525658 5aa8eacca7b5501cd8230c607b684c73683c41b3
child 525660 327cadd9820c9f7ef6580d229ac4c8ce1ff5f324
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc, pascalc
bugs1535766
milestone67.0
Bug 1535766 - Handle unaligned buffers in WebrtcGmpVideoCodec::Encoded; r=bwc a=pascalc 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