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 269649 74e541ff80e178e3c6e6da1118fd2744206e8741
parent 269648 ef8381db1873176dc88ea4c5dc9e3000870347da
child 269650 faacf69f83adec2f2c0bbaf49a548b6e65db1ae9
push id2540
push userwcosta@mozilla.com
push dateWed, 03 Jun 2015 20:55:41 +0000
reviewersgiles
bugs1157991
milestone41.0a1
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;
   }