Ensure DiscardPadding is parsed as a signed integer.

Per the spec DiscardPadding is a signed integer.  Sadly the parser
only supports UInt values at the moment.  To avoid refactoring the
world, we parse DiscardPadding as binary and have a one off parser.

A re-encoded bear-opus-end-trimming.webm will be landed separately
ahead of this change.

This patch also adds a check to ensure we never parse uint64 values
which will overflow an int64 container.

BUG=366750
TEST=Existing tests pass w/ DCHECK.

Review URL: https://codereview.chromium.org/251583003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266478 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/media/media_browsertest.cc

@@ -109,10 +109,9 @@ IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearWebm) {
   PlayVideo("bear.webm", GetParam());
 }
 
-// TODO(dalecurtis): Disabled while the test data file is updated.
-// IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearOpusWebm) {
-//   PlayVideo("bear-opus.webm", GetParam());
-// }
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearOpusWebm) {
+  PlayVideo("bear-opus.webm", GetParam());
+}
 
 IN_PROC_BROWSER_TEST_P(MediaTest, VideoBearOpusOgg) {
   PlayVideo("bear-opus.ogg", GetParam());

content/browser/media/media_source_browsertest.cc

@@ -12,8 +12,7 @@
 // Common media types.
 const char kWebMAudioOnly[] = "audio/webm; codecs=\"vorbis\"";
 #if !defined(OS_ANDROID)
-// TODO(dalecurtis): Disabled while the test data file is updated.
-// const char kWebMOpusAudioOnly[] = "audio/webm; codecs=\"opus\"";
+const char kWebMOpusAudioOnly[] = "audio/webm; codecs=\"opus\"";
 #endif
 const char kWebMVideoOnly[] = "video/webm; codecs=\"vp8\"";
 const char kWebMAudioVideo[] = "video/webm; codecs=\"vorbis, vp8\"";
@@ -65,10 +64,9 @@ IN_PROC_BROWSER_TEST_F(MediaSourceTest, Playback_VideoOnly_WebM) {
 
 // Opus is not supported in Android as of now.
 #if !defined(OS_ANDROID)
-// TODO(dalecurtis): Disabled while the test data file is updated.
-// IN_PROC_BROWSER_TEST_F(MediaSourceTest, Playback_AudioOnly_Opus_WebM) {
-//   TestSimplePlayback("bear-opus.webm", kWebMOpusAudioOnly, kEnded);
-// }
+IN_PROC_BROWSER_TEST_F(MediaSourceTest, Playback_AudioOnly_Opus_WebM) {
+  TestSimplePlayback("bear-opus.webm", kWebMOpusAudioOnly, kEnded);
+}
 #endif
 
 IN_PROC_BROWSER_TEST_F(MediaSourceTest, Playback_AudioOnly_WebM) {

media/filters/opus_audio_decoder.cc

@@ -522,22 +522,25 @@ bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
       }
       output_buffer->get()->TrimEnd(discard_padding);
       frames_to_output -= discard_padding;
+    } else {
+      DCHECK_EQ(input->discard_padding().InMicroseconds(), 0);
     }
   } else {
     frames_to_discard_ -= frames_to_output;
     frames_to_output = 0;
   }
 
+  // Discard the buffer to indicate we need more data.
+  if (!frames_to_output) {
+    *output_buffer = NULL;
+    return true;
+  }
+
   // Assign timestamp and duration to the buffer.
   output_buffer->get()->set_timestamp(output_timestamp_helper_->GetTimestamp());
   output_buffer->get()->set_duration(
       output_timestamp_helper_->GetFrameDuration(frames_to_output));
-  output_timestamp_helper_->AddFrames(frames_decoded);
-
-  // Discard the buffer to indicate we need more data.
-  if (!frames_to_output)
-    *output_buffer = NULL;
-
+  output_timestamp_helper_->AddFrames(frames_to_output);
   return true;
 }
 

media/filters/pipeline_integration_test.cc

@@ -63,7 +63,7 @@ const int kAppendTimeSec = 1;
 const int kAppendTimeMs = kAppendTimeSec * 1000;
 const int k320WebMFileDurationMs = 2736;
 const int k640WebMFileDurationMs = 2749;
-const int kOpusEndTrimmingWebMFileDurationMs = 2771;
+const int kOpusEndTrimmingWebMFileDurationMs = 2741;
 const int kVP9WebMFileDurationMs = 2736;
 const int kVP8AWebMFileDurationMs = 2733;
 
@@ -655,8 +655,7 @@ TEST_P(PipelineIntegrationTest, BasicPlayback_MediaSource_VP8A_WebM) {
   Stop();
 }
 
-// Disabled until the fix for http://crbug.com/366750 is landed.
-TEST_P(PipelineIntegrationTest, DISABLED_BasicPlayback_MediaSource_Opus_WebM) {
+TEST_P(PipelineIntegrationTest, BasicPlayback_MediaSource_Opus_WebM) {
   MockMediaSource source("bear-opus-end-trimming.webm", kOpusAudioOnlyWebM,
                          kAppendWholeFile, GetParam());
   StartPipelineWithMediaSource(&source);

media/formats/webm/webm_cluster_parser.cc

@@ -191,12 +191,6 @@ bool WebMClusterParser::OnUInt(int id, int64 val) {
     case kWebMIdBlockAddID:
       dst = &block_add_id_;
       break;
-    case kWebMIdDiscardPadding:
-      if (discard_padding_set_)
-        return false;
-      discard_padding_set_ = true;
-      discard_padding_ = val;
-      return true;
     default:
       return true;
   }
@@ -278,7 +272,18 @@ bool WebMClusterParser::OnBinary(int id, const uint8* data, int size) {
       memcpy(block_additional_data_.get() + 8, data, size);
       return true;
     }
+    case kWebMIdDiscardPadding: {
+      if (discard_padding_set_ || size <= 0 || size > 8)
+        return false;
+      discard_padding_set_ = true;
+
+      // Read in the big-endian integer.
+      discard_padding_ = static_cast<int8>(data[0]);
+      for (int i = 1; i < size; ++i)
+        discard_padding_ = (discard_padding_ << 8) | data[i];
 
+      return true;
+    }
     default:
       return true;
   }

media/formats/webm/webm_parser.cc

@@ -14,6 +14,7 @@
 #include <iomanip>
 
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
 #include "media/formats/webm/webm_constants.h"
 
 namespace media {
@@ -118,7 +119,7 @@ static const ElementIdInfo kBlockGroupIds[] = {
   {UINT, kWebMIdReferencePriority},
   {BINARY, kWebMIdReferenceBlock},
   {BINARY, kWebMIdCodecState},
-  {UINT, kWebMIdDiscardPadding},
+  {BINARY, kWebMIdDiscardPadding},
   {LIST, kWebMIdSlices},
 };
 
@@ -548,10 +549,15 @@ static int ParseUInt(const uint8* buf, int size, int id,
     return -1;
 
   // Read in the big-endian integer.
-  int64 value = 0;
+  uint64 value = 0;
   for (int i = 0; i < size; ++i)
     value = (value << 8) | buf[i];
 
+  // We use int64 in place of uint64 everywhere for convenience.  See this bug
+  // for more details: http://crbug.com/366750#c3
+  if (!base::IsValueInRangeForNumericType<int64>(value))
+    return -1;
+
   if (!client->OnUInt(id, value))
     return -1;