Bug 784329 - Part 7: Add sanity checking of video format metadata. r=doublec

media/omx-plugin/OmxPlugin.cpp

@@ -12,6 +12,8 @@
 #else
 #include <stagefright/OMXClient.h>
 #endif
+
+#include "mozilla/Assertions.h"
 #include "mozilla/Types.h"
 #include "MPAPI.h"
 
@@ -288,10 +290,6 @@ bool OmxDecoder::Init() {
   for (size_t i = 0; i < extractor->countTracks(); ++i) {
     sp<MetaData> meta = extractor->getTrackMetaData(i);
 
-    int32_t bitRate;
-    if (!meta->findInt32(kKeyBitRate, &bitRate))
-      bitRate = 0;
-
     const char *mime;
     if (!meta->findCString(kKeyMIMEType, &mime)) {
       continue;
@@ -349,6 +347,8 @@ bool OmxDecoder::Init() {
 
     int64_t durationUs;
     if (videoTrack->getFormat()->findInt64(kKeyDuration, &durationUs)) {
+      if (durationUs < 0)
+        LOG("video duration %lld should be nonnegative", durationUs);
       if (durationUs > totalDurationUs)
         totalDurationUs = durationUs;
     }
@@ -380,6 +380,8 @@ bool OmxDecoder::Init() {
 
     int64_t durationUs;
     if (audioTrack->getFormat()->findInt64(kKeyDuration, &durationUs)) {
+      if (durationUs < 0)
+        LOG("audio duration %lld should be nonnegative", durationUs);
       if (durationUs > totalDurationUs)
         totalDurationUs = durationUs;
     }
@@ -405,6 +407,16 @@ bool OmxDecoder::Init() {
         return false;
       }
       mAudioMetadataRead = true;
+
+      if (mAudioChannels < 0) {
+        LOG("audio channel count %d must be nonnegative", mAudioChannels);
+        return false;
+      }
+
+      if (mAudioSampleRate < 0) {
+        LOG("audio sample rate %d must be nonnegative", mAudioSampleRate);
+        return false;
+      }
     }
     else if (!SetAudioFormat()) {
         return false;
@@ -437,6 +449,16 @@ bool OmxDecoder::SetVideoFormat() {
     return false;
   }
 
+  if (mVideoStride <= 0) {
+    LOG("stride %d must be positive", mVideoStride);
+    return false;
+  }
+
+  if (mVideoSliceHeight <= 0) {
+    LOG("slice height %d must be positive", mVideoSliceHeight);
+    return false;
+  }
+
   int32_t cropRight, cropBottom;
   if (!format->findRect(kKeyCropRect, &mVideoCropLeft, &mVideoCropTop,
                                       &cropRight, &cropBottom)) {
@@ -447,14 +469,27 @@ bool OmxDecoder::SetVideoFormat() {
     LOG("crop rect not available, assuming no cropping");
   }
 
+  if (mVideoCropLeft < 0 || mVideoCropLeft >= cropRight || cropRight >= mVideoStride ||
+      mVideoCropTop < 0 || mVideoCropTop >= cropBottom || cropBottom >= mVideoSliceHeight) {
+    LOG("invalid crop rect %d,%d-%d,%d", mVideoCropLeft, mVideoCropTop, cropRight, cropBottom);
+    return false;
+  }
+
   mVideoWidth = cropRight - mVideoCropLeft + 1;
   mVideoHeight = cropBottom - mVideoCropTop + 1;
+  MOZ_ASSERT(mVideoWidth > 0 && mVideoWidth <= mVideoStride);
+  MOZ_ASSERT(mVideoHeight > 0 && mVideoHeight <= mVideoSliceHeight);
 
   if (!format->findInt32(kKeyRotation, &mVideoRotation)) {
     mVideoRotation = 0;
     LOG("rotation not available, assuming 0");
   }
 
+  if (mVideoRotation != 0 && mVideoRotation != 90 &&
+      mVideoRotation != 180 && mVideoRotation != 270) {
+    LOG("invalid rotation %d, assuming 0", mVideoRotation);
+  }
+
   LOG("width: %d height: %d component: %s format: %#x stride: %d sliceHeight: %d rotation: %d crop: %d,%d-%d,%d",
       mVideoWidth, mVideoHeight, componentName, mVideoColorFormat,
       mVideoStride, mVideoSliceHeight, mVideoRotation,
@@ -470,8 +505,17 @@ bool OmxDecoder::SetAudioFormat() {
     return false;
   }
 
-  LOG("channelCount: %d sampleRate: %d",
-      mAudioChannels, mAudioSampleRate);
+  LOG("channelCount: %d sampleRate: %d", mAudioChannels, mAudioSampleRate);
+
+  if (mAudioChannels < 0) {
+    LOG("audio channel count %d must be nonnegative", mAudioChannels);
+    return false;
+  }
+
+  if (mAudioSampleRate < 0) {
+    LOG("audio sample rate %d must be nonnegative", mAudioSampleRate);
+    return false;
+  }
 
   return true;
 }
@@ -582,6 +626,8 @@ bool OmxDecoder::ToAudioFrame(AudioFrame *aFrame, int64_t aTimeUs, void *aData,
 
 bool OmxDecoder::ReadVideo(VideoFrame *aFrame, int64_t aSeekTimeUs)
 {
+  MOZ_ASSERT(aSeekTimeUs >= -1);
+
   if (!mVideoSource.get())
     return false;
 
@@ -599,11 +645,15 @@ bool OmxDecoder::ReadVideo(VideoFrame *aFrame, int64_t aSeekTimeUs)
 
   if (err == OK && mVideoBuffer->range_length() > 0) {
     int64_t timeUs;
-    int32_t unreadable;
     int32_t keyFrame;
 
     if (!mVideoBuffer->meta_data()->findInt64(kKeyTime, &timeUs) ) {
-      LOG("no key time");
+      LOG("no frame time");
+      return false;
+    }
+
+    if (timeUs < 0) {
+      LOG("frame time %lld must be nonnegative", timeUs);
       return false;
     }
 
@@ -611,10 +661,6 @@ bool OmxDecoder::ReadVideo(VideoFrame *aFrame, int64_t aSeekTimeUs)
        keyFrame = 0;
     }
 
-    if (!mVideoBuffer->meta_data()->findInt32(kKeyIsUnreadable, &unreadable)) {
-      unreadable = 0;
-    }
-
     char *data = reinterpret_cast<char *>(mVideoBuffer->data()) + mVideoBuffer->range_offset();
     size_t length = mVideoBuffer->range_length();
 
@@ -642,6 +688,8 @@ bool OmxDecoder::ReadVideo(VideoFrame *aFrame, int64_t aSeekTimeUs)
 
 bool OmxDecoder::ReadAudio(AudioFrame *aFrame, int64_t aSeekTimeUs)
 {
+  MOZ_ASSERT(aSeekTimeUs >= -1);
+
   status_t err;
   if (mAudioMetadataRead && aSeekTimeUs == -1) {
     // Use the data read into the buffer during metadata time
@@ -663,8 +711,15 @@ bool OmxDecoder::ReadAudio(AudioFrame *aFrame, int64_t aSeekTimeUs)
 
   if (err == OK && mAudioBuffer->range_length() != 0) {
     int64_t timeUs;
-    if (!mAudioBuffer->meta_data()->findInt64(kKeyTime, &timeUs))
+    if (!mAudioBuffer->meta_data()->findInt64(kKeyTime, &timeUs)) {
+      LOG("no frame time");
       return false;
+    }
+
+    if (timeUs < 0) {
+      LOG("frame time %lld must be nonnegative", timeUs);
+      return false;
+    }
 
     return ToAudioFrame(aFrame, timeUs,
                         mAudioBuffer->data(),