Bug 1157991 - Remove buggy/unused multiple-frame-per-packet handling from WebM decoders. r=giles
authorMatthew Gregan <kinetik@flim.org>
Thu, 04 Jun 2015 00:19:17 +1200
changeset 246991 74e541ff80e178e3c6e6da1118fd2744206e8741
parent 246990 ef8381db1873176dc88ea4c5dc9e3000870347da
child 246992 faacf69f83adec2f2c0bbaf49a548b6e65db1ae9
push id28848
push userryanvm@gmail.com
push dateWed, 03 Jun 2015 20:00:13 +0000
treeherdermozilla-central@0920f2325a6d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgiles
bugs1157991
milestone41.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 1157991 - Remove buggy/unused multiple-frame-per-packet handling from WebM decoders. r=giles
dom/media/webm/IntelWebMVideoDecoder.cpp
dom/media/webm/SoftwareWebMVideoDecoder.cpp
dom/media/webm/WebMReader.cpp
--- a/dom/media/webm/IntelWebMVideoDecoder.cpp
+++ b/dom/media/webm/IntelWebMVideoDecoder.cpp
@@ -165,16 +165,21 @@ IntelWebMVideoDecoder::Demux(nsRefPtr<VP
   }
 
   unsigned int count = 0;
   r = nestegg_packet_count(packet, &count);
   if (r == -1) {
     return false;
   }
 
+  if (count > 1) {
+    NS_WARNING("Packet contains more than one video frame");
+    return false;
+  }
+
   int64_t tstamp = holder->Timestamp();
 
   // The end time of this frame is the start time of the next frame.  Fetch
   // the timestamp of the next packet for this track.  If we've reached the
   // end of the resource, use the file's duration as the end time of this
   // video frame.
   int64_t next_tstamp = 0;
   nsRefPtr<NesteggPacketHolder> next_holder(mReader->NextPacket(WebMReader::VIDEO));
@@ -182,44 +187,42 @@ IntelWebMVideoDecoder::Demux(nsRefPtr<VP
     next_tstamp = holder->Timestamp();
     mReader->PushVideoPacket(next_holder.forget());
   } else {
     next_tstamp = tstamp;
     next_tstamp += tstamp - mReader->GetLastVideoFrameTime();
   }
   mReader->SetLastVideoFrameTime(tstamp);
 
-  for (uint32_t i = 0; i < count; ++i) {
-    unsigned char* data;
-    size_t length;
-    r = nestegg_packet_data(packet, i, &data, &length);
-    if (r == -1) {
-      return false;
-    }
+  unsigned char* data;
+  size_t length;
+  r = nestegg_packet_data(packet, 0, &data, &length);
+  if (r == -1) {
+    return false;
+  }
 
-    vpx_codec_stream_info_t si;
-    memset(&si, 0, sizeof(si));
-    si.sz = sizeof(si);
-    if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP8) {
-      vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
-    } else if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP9) {
-      vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
-    }
+  vpx_codec_stream_info_t si;
+  memset(&si, 0, sizeof(si));
+  si.sz = sizeof(si);
+  if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP8) {
+    vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
+  } else if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP9) {
+    vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
+  }
 
-    MOZ_ASSERT(mPlatform && mMediaDataDecoder);
+  MOZ_ASSERT(mPlatform && mMediaDataDecoder);
 
-    aSample = new VP8Sample(tstamp,
-                            next_tstamp - tstamp,
-                            0,
-                            data,
-                            length,
-                            si.is_kf);
-    if (!aSample->mData) {
-      return false;
-    }
+  aSample = new VP8Sample(tstamp,
+                          next_tstamp - tstamp,
+                          0,
+                          data,
+                          length,
+                          si.is_kf);
+  if (!aSample->mData) {
+    return false;
   }
 
   return true;
 }
 
 bool
 IntelWebMVideoDecoder::Decode()
 {
--- a/dom/media/webm/SoftwareWebMVideoDecoder.cpp
+++ b/dom/media/webm/SoftwareWebMVideoDecoder.cpp
@@ -93,16 +93,21 @@ SoftwareWebMVideoDecoder::DecodeVideoFra
   }
 
   unsigned int count = 0;
   r = nestegg_packet_count(packet, &count);
   if (r == -1) {
     return false;
   }
 
+  if (count > 1) {
+    NS_WARNING("Packet contains more than one video frame");
+    return false;
+  }
+
   int64_t tstamp = holder->Timestamp();
 
   // The end time of this frame is the start time of the next frame.  Fetch
   // the timestamp of the next packet for this track.  If we've reached the
   // end of the resource, use the file's duration as the end time of this
   // video frame.
   int64_t next_tstamp = 0;
   nsRefPtr<NesteggPacketHolder> next_holder(mReader->NextPacket(WebMReader::VIDEO));
@@ -110,114 +115,112 @@ SoftwareWebMVideoDecoder::DecodeVideoFra
     next_tstamp = next_holder->Timestamp();
     mReader->PushVideoPacket(next_holder.forget());
   } else {
     next_tstamp = tstamp;
     next_tstamp += tstamp - mReader->GetLastVideoFrameTime();
   }
   mReader->SetLastVideoFrameTime(tstamp);
 
-  for (uint32_t i = 0; i < count; ++i) {
-    unsigned char* data;
-    size_t length;
-    r = nestegg_packet_data(packet, i, &data, &length);
-    if (r == -1) {
-      return false;
-    }
+  unsigned char* data;
+  size_t length;
+  r = nestegg_packet_data(packet, 0, &data, &length);
+  if (r == -1) {
+    return false;
+  }
+
+  vpx_codec_stream_info_t si;
+  memset(&si, 0, sizeof(si));
+  si.sz = sizeof(si);
+  if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP8) {
+    vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
+  } else if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP9) {
+    vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
+  }
+  if (aKeyframeSkip && (!si.is_kf || tstamp < aTimeThreshold)) {
+    // Skipping to next keyframe...
+    a.mParsed++;
+    a.mDropped++;
+    return true;
+  }
+
+  if (aKeyframeSkip && si.is_kf) {
+    aKeyframeSkip = false;
+  }
+
+  if (vpx_codec_decode(&mVPX, data, length, nullptr, 0)) {
+    return false;
+  }
+
+  // If the timestamp of the video frame is less than
+  // the time threshold required then it is not added
+  // to the video queue and won't be displayed.
+  if (tstamp < aTimeThreshold) {
+    a.mParsed++;
+    a.mDropped++;
+    return true;
+  }
 
-    vpx_codec_stream_info_t si;
-    memset(&si, 0, sizeof(si));
-    si.sz = sizeof(si);
-    if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP8) {
-      vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
-    } else if (mReader->GetVideoCodec() == NESTEGG_CODEC_VP9) {
-      vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
-    }
-    if (aKeyframeSkip && (!si.is_kf || tstamp < aTimeThreshold)) {
-      // Skipping to next keyframe...
-      a.mParsed++; // Assume 1 frame per chunk.
-      a.mDropped++;
-      continue;
-    }
+  vpx_codec_iter_t  iter = nullptr;
+  vpx_image_t      *img;
+
+  while ((img = vpx_codec_get_frame(&mVPX, &iter))) {
+    NS_ASSERTION(img->fmt == VPX_IMG_FMT_I420, "WebM image format not I420");
+
+    // Chroma shifts are rounded down as per the decoding examples in the SDK
+    VideoData::YCbCrBuffer b;
+    b.mPlanes[0].mData = img->planes[0];
+    b.mPlanes[0].mStride = img->stride[0];
+    b.mPlanes[0].mHeight = img->d_h;
+    b.mPlanes[0].mWidth = img->d_w;
+    b.mPlanes[0].mOffset = b.mPlanes[0].mSkip = 0;
+
+    b.mPlanes[1].mData = img->planes[1];
+    b.mPlanes[1].mStride = img->stride[1];
+    b.mPlanes[1].mHeight = (img->d_h + 1) >> img->y_chroma_shift;
+    b.mPlanes[1].mWidth = (img->d_w + 1) >> img->x_chroma_shift;
+    b.mPlanes[1].mOffset = b.mPlanes[1].mSkip = 0;
 
-    if (aKeyframeSkip && si.is_kf) {
-      aKeyframeSkip = false;
-    }
-
-    if (vpx_codec_decode(&mVPX, data, length, nullptr, 0)) {
-      return false;
-    }
+    b.mPlanes[2].mData = img->planes[2];
+    b.mPlanes[2].mStride = img->stride[2];
+    b.mPlanes[2].mHeight = (img->d_h + 1) >> img->y_chroma_shift;
+    b.mPlanes[2].mWidth = (img->d_w + 1) >> img->x_chroma_shift;
+    b.mPlanes[2].mOffset = b.mPlanes[2].mSkip = 0;
 
-    // If the timestamp of the video frame is less than
-    // the time threshold required then it is not added
-    // to the video queue and won't be displayed.
-    if (tstamp < aTimeThreshold) {
-      a.mParsed++; // Assume 1 frame per chunk.
-      a.mDropped++;
-      continue;
+    nsIntRect pictureRect = mReader->GetPicture();
+    IntRect picture = pictureRect;
+    nsIntSize initFrame = mReader->GetInitialFrame();
+    if (img->d_w != static_cast<uint32_t>(initFrame.width) ||
+        img->d_h != static_cast<uint32_t>(initFrame.height)) {
+      // Frame size is different from what the container reports. This is
+      // legal in WebM, and we will preserve the ratio of the crop rectangle
+      // as it was reported relative to the picture size reported by the
+      // container.
+      picture.x = (pictureRect.x * img->d_w) / initFrame.width;
+      picture.y = (pictureRect.y * img->d_h) / initFrame.height;
+      picture.width = (img->d_w * pictureRect.width) / initFrame.width;
+      picture.height = (img->d_h * pictureRect.height) / initFrame.height;
     }
 
-    vpx_codec_iter_t  iter = nullptr;
-    vpx_image_t      *img;
-
-    while ((img = vpx_codec_get_frame(&mVPX, &iter))) {
-      NS_ASSERTION(img->fmt == VPX_IMG_FMT_I420, "WebM image format not I420");
-
-      // Chroma shifts are rounded down as per the decoding examples in the SDK
-      VideoData::YCbCrBuffer b;
-      b.mPlanes[0].mData = img->planes[0];
-      b.mPlanes[0].mStride = img->stride[0];
-      b.mPlanes[0].mHeight = img->d_h;
-      b.mPlanes[0].mWidth = img->d_w;
-      b.mPlanes[0].mOffset = b.mPlanes[0].mSkip = 0;
-
-      b.mPlanes[1].mData = img->planes[1];
-      b.mPlanes[1].mStride = img->stride[1];
-      b.mPlanes[1].mHeight = (img->d_h + 1) >> img->y_chroma_shift;
-      b.mPlanes[1].mWidth = (img->d_w + 1) >> img->x_chroma_shift;
-      b.mPlanes[1].mOffset = b.mPlanes[1].mSkip = 0;
-
-      b.mPlanes[2].mData = img->planes[2];
-      b.mPlanes[2].mStride = img->stride[2];
-      b.mPlanes[2].mHeight = (img->d_h + 1) >> img->y_chroma_shift;
-      b.mPlanes[2].mWidth = (img->d_w + 1) >> img->x_chroma_shift;
-      b.mPlanes[2].mOffset = b.mPlanes[2].mSkip = 0;
-
-      nsIntRect pictureRect = mReader->GetPicture();
-      IntRect picture = pictureRect;
-      nsIntSize initFrame = mReader->GetInitialFrame();
-      if (img->d_w != static_cast<uint32_t>(initFrame.width) ||
-          img->d_h != static_cast<uint32_t>(initFrame.height)) {
-        // Frame size is different from what the container reports. This is
-        // legal in WebM, and we will preserve the ratio of the crop rectangle
-        // as it was reported relative to the picture size reported by the
-        // container.
-        picture.x = (pictureRect.x * img->d_w) / initFrame.width;
-        picture.y = (pictureRect.y * img->d_h) / initFrame.height;
-        picture.width = (img->d_w * pictureRect.width) / initFrame.width;
-        picture.height = (img->d_h * pictureRect.height) / initFrame.height;
-      }
-
-      VideoInfo videoInfo = mReader->GetMediaInfo().mVideo;
-      nsRefPtr<VideoData> v = VideoData::Create(videoInfo,
-                                                mReader->GetDecoder()->GetImageContainer(),
-                                                holder->Offset(),
-                                                tstamp,
-                                                next_tstamp - tstamp,
-                                                b,
-                                                si.is_kf,
-                                                -1,
-                                                picture);
-      if (!v) {
-        return false;
-      }
-      a.mParsed++;
-      a.mDecoded++;
-      NS_ASSERTION(a.mDecoded <= a.mParsed,
-        "Expect only 1 frame per chunk per packet in WebM...");
-      mReader->VideoQueue().Push(v);
+    VideoInfo videoInfo = mReader->GetMediaInfo().mVideo;
+    nsRefPtr<VideoData> v = VideoData::Create(videoInfo,
+                                              mReader->GetDecoder()->GetImageContainer(),
+                                              holder->Offset(),
+                                              tstamp,
+                                              next_tstamp - tstamp,
+                                              b,
+                                              si.is_kf,
+                                              -1,
+                                              picture);
+    if (!v) {
+      return false;
     }
+    a.mParsed++;
+    a.mDecoded++;
+    NS_ASSERTION(a.mDecoded <= a.mParsed,
+                 "Expect only 1 frame per chunk per packet in WebM...");
+    mReader->VideoQueue().Push(v);
   }
 
   return true;
 }
 
 } // namespace mozilla
--- a/dom/media/webm/WebMReader.cpp
+++ b/dom/media/webm/WebMReader.cpp
@@ -914,50 +914,35 @@ WebMReader::DemuxPacket()
 
   unsigned int track = 0;
   r = nestegg_packet_track(packet, &track);
   if (r == -1) {
     return nullptr;
   }
 
   // Figure out if this is a keyframe.
-  //
-  // Doing this at packet-granularity is kind of the wrong level of
-  // abstraction, but timestamps are on the packet, so the only time
-  // we have multiple video frames in a packet is when we have "alternate
-  // reference frames", which are a compression detail and never displayed.
-  // So for our purposes, we can just take the union of the is_kf values for
-  // all the frames in the packet.
   bool isKeyframe = false;
   if (track == mAudioTrack) {
     isKeyframe = true;
   } else if (track == mVideoTrack) {
-    unsigned int count = 0;
-    r = nestegg_packet_count(packet, &count);
+    unsigned char* data;
+    size_t length;
+    r = nestegg_packet_data(packet, 0, &data, &length);
     if (r == -1) {
       return nullptr;
     }
-
-    for (unsigned i = 0; i < count; ++i) {
-      unsigned char* data;
-      size_t length;
-      r = nestegg_packet_data(packet, i, &data, &length);
-      if (r == -1) {
-        return nullptr;
-      }
-      vpx_codec_stream_info_t si;
-      memset(&si, 0, sizeof(si));
-      si.sz = sizeof(si);
-      if (mVideoCodec == NESTEGG_CODEC_VP8) {
-        vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
-      } else if (mVideoCodec == NESTEGG_CODEC_VP9) {
-        vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
-      }
-      isKeyframe = isKeyframe || si.is_kf;
+    vpx_codec_stream_info_t si;
+    memset(&si, 0, sizeof(si));
+    si.sz = sizeof(si);
+    if (mVideoCodec == NESTEGG_CODEC_VP8) {
+      vpx_codec_peek_stream_info(vpx_codec_vp8_dx(), data, length, &si);
+    } else if (mVideoCodec == NESTEGG_CODEC_VP9) {
+      vpx_codec_peek_stream_info(vpx_codec_vp9_dx(), data, length, &si);
     }
+    isKeyframe = si.is_kf;
   }
 
   int64_t offset = mDecoder->GetResource()->Tell();
   nsRefPtr<NesteggPacketHolder> holder = new NesteggPacketHolder();
   if (!holder->Init(packet, offset, track, isKeyframe)) {
     return nullptr;
   }