Bug 557982. Use Image objects in the video frame queue so we can avoid making an extra copy as we put frames into the queue. r=kinetik
authorRich Dougherty <rich@rd.gen.nz>
Wed, 19 May 2010 15:04:33 +1200
changeset 42438 e530c2b50c0a9588233dadb85278cad49cf1e100
parent 42437 a55765a1c2f576b5c73f8cd25b88d3a717419e76
child 42439 b500025971125c0c0126b672ddfd44196dfc1e34
push idunknown
push userunknown
push dateunknown
reviewerskinetik
bugs557982
milestone1.9.3a5pre
Bug 557982. Use Image objects in the video frame queue so we can avoid making an extra copy as we put frames into the queue. r=kinetik
content/media/nsBuiltinDecoderReader.cpp
content/media/nsBuiltinDecoderReader.h
content/media/nsBuiltinDecoderStateMachine.cpp
content/media/nsBuiltinDecoderStateMachine.h
content/media/ogg/nsOggCodecState.cpp
content/media/ogg/nsOggReader.cpp
content/media/ogg/nsOggReader.h
gfx/layers/ImageLayers.h
--- a/content/media/nsBuiltinDecoderReader.cpp
+++ b/content/media/nsBuiltinDecoderReader.cpp
@@ -42,16 +42,18 @@
 #include "nsTArray.h"
 #include "nsBuiltinDecoder.h"
 #include "nsBuiltinDecoderReader.h"
 #include "nsBuiltinDecoderStateMachine.h"
 #include "mozilla/mozalloc.h"
 #include "VideoUtils.h"
 
 using namespace mozilla;
+using mozilla::layers::ImageContainer;
+using mozilla::layers::PlanarYCbCrImage;
 
 // Un-comment to enable logging of seek bisections.
 //#define SEEK_LOGGING
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gBuiltinDecoderLog;
 #define LOG(type, msg) PR_LOG(gBuiltinDecoderLog, type, msg)
 #ifdef SEEK_LOGGING
@@ -59,57 +61,110 @@ extern PRLogModuleInfo* gBuiltinDecoderL
 #else
 #define SEEK_LOG(type, msg)
 #endif
 #else
 #define LOG(type, msg)
 #define SEEK_LOG(type, msg)
 #endif
 
-// 32 bit integer multiplication with overflow checking. Returns PR_TRUE
-// if the multiplication was successful, or PR_FALSE if the operation resulted
-// in an integer overflow.
-PRBool MulOverflow32(PRUint32 a, PRUint32 b, PRUint32& aResult) {
-  PRUint64 a64 = a;
-  PRUint64 b64 = b;
-  PRUint64 r64 = a64 * b64;
-  if (r64 > PR_UINT32_MAX)
+// Adds two 32bit unsigned numbers, retuns PR_TRUE if addition succeeded,
+// or PR_FALSE the if addition would result in an overflow.
+static PRBool AddOverflow32(PRUint32 a, PRUint32 b, PRUint32& aResult) {
+  PRUint64 rl = static_cast<PRUint64>(a) + static_cast<PRUint64>(b);
+  if (rl > PR_UINT32_MAX) {
     return PR_FALSE;
-  aResult = static_cast<PRUint32>(r64);
-  return PR_TRUE;
+  }
+  aResult = static_cast<PRUint32>(rl);
+  return true;
 }
 
-VideoData* VideoData::Create(PRInt64 aOffset,
+static PRBool
+ValidatePlane(const VideoData::YCbCrBuffer::Plane& aPlane)
+{
+  return aPlane.mWidth <= PlanarYCbCrImage::MAX_DIMENSION &&
+         aPlane.mHeight <= PlanarYCbCrImage::MAX_DIMENSION &&
+         aPlane.mStride > 0;
+}
+
+VideoData* VideoData::Create(nsVideoInfo& aInfo,
+                             ImageContainer* aContainer,
+                             PRInt64 aOffset,
                              PRInt64 aTime,
-                             const YCbCrBuffer &aBuffer,
+                             const YCbCrBuffer& aBuffer,
                              PRBool aKeyframe,
                              PRInt64 aTimecode)
 {
+  if (!aContainer) {
+    return nsnull;
+  }
+
+  // The following situation should never happen unless there is a bug
+  // in the decoder
+  if (aBuffer.mPlanes[1].mWidth != aBuffer.mPlanes[2].mWidth ||
+      aBuffer.mPlanes[1].mHeight != aBuffer.mPlanes[2].mHeight) {
+    NS_ERROR("C planes with different sizes");
+    return nsnull;
+  }
+
+  // The following situations could be triggered by invalid input
+  if (aInfo.mPicture.width <= 0 || aInfo.mPicture.height <= 0) {
+    NS_WARNING("Empty picture rect");
+    return nsnull;
+  }
+  if (aBuffer.mPlanes[0].mWidth != PRUint32(aInfo.mFrame.width) ||
+      aBuffer.mPlanes[0].mHeight != PRUint32(aInfo.mFrame.height)) {
+    NS_WARNING("Unexpected frame size");
+    return nsnull;
+  }
+  if (!ValidatePlane(aBuffer.mPlanes[0]) || !ValidatePlane(aBuffer.mPlanes[1]) ||
+      !ValidatePlane(aBuffer.mPlanes[2])) {
+    NS_WARNING("Invalid plane size");
+    return nsnull;
+  }
+  // Ensure the picture size specified in the headers can be extracted out of
+  // the frame we've been supplied without indexing out of bounds.
+  PRUint32 picXLimit;
+  PRUint32 picYLimit;
+  if (!AddOverflow32(aInfo.mPicture.x, aInfo.mPicture.width, picXLimit) ||
+      picXLimit > PRUint32(aBuffer.mPlanes[0].mStride) ||
+      !AddOverflow32(aInfo.mPicture.y, aInfo.mPicture.height, picYLimit) ||
+      picYLimit > PRUint32(aBuffer.mPlanes[0].mHeight))
+  {
+    // The specified picture dimensions can't be contained inside the video
+    // frame, we'll stomp memory if we try to copy it. Fail.
+    NS_WARNING("Overflowing picture rect");
+    return nsnull;
+  }
+
   nsAutoPtr<VideoData> v(new VideoData(aOffset, aTime, aKeyframe, aTimecode));
-  for (PRUint32 i=0; i < 3; ++i) {
-    PRUint32 size = 0;
-    if (!MulOverflow32(PR_ABS(aBuffer.mPlanes[i].mHeight),
-                       PR_ABS(aBuffer.mPlanes[i].mStride),
-                       size))
-    {
-      // Invalid frame size. Skip this plane. The plane will have 0
-      // dimensions, thanks to our constructor.
-      continue;
-    }
-    unsigned char* p = static_cast<unsigned char*>(moz_xmalloc(size));
-    if (!p) {
-      NS_WARNING("Failed to allocate memory for video frame");
-      return nsnull;
-    }
-    v->mBuffer.mPlanes[i].mData = p;
-    v->mBuffer.mPlanes[i].mWidth = aBuffer.mPlanes[i].mWidth;
-    v->mBuffer.mPlanes[i].mHeight = aBuffer.mPlanes[i].mHeight;
-    v->mBuffer.mPlanes[i].mStride = aBuffer.mPlanes[i].mStride;
-    memcpy(v->mBuffer.mPlanes[i].mData, aBuffer.mPlanes[i].mData, size);
+  // Currently our decoder only knows how to output to PLANAR_YCBCR
+  // format.
+  Image::Format format = Image::PLANAR_YCBCR;
+  v->mImage = aContainer->CreateImage(&format, 1);
+  if (!v->mImage) {
+    return nsnull;
   }
+  NS_ASSERTION(v->mImage->GetFormat() == Image::PLANAR_YCBCR,
+               "Wrong format?");
+  PlanarYCbCrImage* videoImage = static_cast<PlanarYCbCrImage*>(v->mImage.get());
+
+  PlanarYCbCrImage::Data data;
+  data.mYChannel = aBuffer.mPlanes[0].mData;
+  data.mYSize = gfxIntSize(aBuffer.mPlanes[0].mWidth, aBuffer.mPlanes[0].mHeight);
+  data.mYStride = aBuffer.mPlanes[0].mStride;
+  data.mCbChannel = aBuffer.mPlanes[1].mData;
+  data.mCrChannel = aBuffer.mPlanes[2].mData;
+  data.mCbCrSize = gfxIntSize(aBuffer.mPlanes[1].mWidth, aBuffer.mPlanes[1].mHeight);
+  data.mCbCrStride = aBuffer.mPlanes[1].mStride;
+  data.mPicX = aInfo.mPicture.x;
+  data.mPicY = aInfo.mPicture.y;
+  data.mPicSize = gfxIntSize(aInfo.mPicture.width, aInfo.mPicture.height);
+
+  videoImage->SetData(data); // Copies buffer
   return v.forget();
 }
 
 nsBuiltinDecoderReader::nsBuiltinDecoderReader(nsBuiltinDecoder* aDecoder)
   : mMonitor("media.decoderreader"),
     mDecoder(aDecoder),
     mDataOffset(0)
 {
--- a/content/media/nsBuiltinDecoderReader.h
+++ b/content/media/nsBuiltinDecoderReader.h
@@ -35,25 +35,74 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 #if !defined(nsBuiltinDecoderReader_h_)
 #define nsBuiltinDecoderReader_h_
 
 #include <nsDeque.h>
+#include "Layers.h"
+#include "ImageLayers.h"
 #include "nsAutoLock.h"
 #include "nsClassHashtable.h"
 #include "mozilla/TimeStamp.h"
 #include "nsSize.h"
 #include "nsRect.h"
 #include "mozilla/Monitor.h"
 
 class nsBuiltinDecoderStateMachine;
 
+// Stores info relevant to presenting media samples.
+class nsVideoInfo {
+public:
+  nsVideoInfo()
+    : mFramerate(0.0),
+      mAspectRatio(1.0),
+      mCallbackPeriod(1),
+      mAudioRate(0),
+      mAudioChannels(0),
+      mFrame(0,0),
+      mHasAudio(PR_FALSE),
+      mHasVideo(PR_FALSE)
+  {}
+
+  // Frames per second.
+  float mFramerate;
+
+  // Aspect ratio, as stored in the metadata.
+  float mAspectRatio;
+
+  // Length of a video frame in milliseconds, or the callback period if
+  // there's no audio.
+  PRUint32 mCallbackPeriod;
+
+  // Samples per second.
+  PRUint32 mAudioRate;
+
+  // Number of audio channels.
+  PRUint32 mAudioChannels;
+
+  // Dimensions of the video frame.
+  nsIntSize mFrame;
+
+  // The picture region inside the video frame to be displayed.
+  nsIntRect mPicture;
+
+  // The offset of the first non-header page in the file, in bytes.
+  // Used to seek to the start of the media.
+  PRInt64 mDataOffset;
+
+  // PR_TRUE if we have an active audio bitstream.
+  PRPackedBool mHasAudio;
+
+  // PR_TRUE if we have an active video bitstream.
+  PRPackedBool mHasVideo;
+};
+
 // Holds chunk a decoded sound samples.
 class SoundData {
 public:
   SoundData(PRInt64 aOffset,
             PRInt64 aTime,
             PRInt64 aDuration,
             PRUint32 aSamples,
             float* aData,
@@ -101,36 +150,43 @@ public:
   const PRUint32 mSamples;
   const PRUint32 mChannels;
   nsAutoArrayPtr<float> mAudioData;
 };
 
 // Holds a decoded video frame, in YCbCr format. These are queued in the reader.
 class VideoData {
 public:
+  typedef mozilla::layers::ImageContainer ImageContainer;
+  typedef mozilla::layers::Image Image;
+
   // YCbCr data obtained from decoding the video. The index's are:
   //   0 = Y
   //   1 = Cb
   //   2 = Cr
   struct YCbCrBuffer {
     struct Plane {
       PRUint8* mData;
       PRUint32 mWidth;
       PRUint32 mHeight;
       PRUint32 mStride;
     };
 
     Plane mPlanes[3];
   };
 
   // Constructs a VideoData object. Makes a copy of YCbCr data in aBuffer.
-  // This may return nsnull if we run out of memory when allocating buffers
-  // to store the frame. aTimecode is a codec specific number representing
-  // the timestamp of the frame of video data.
-  static VideoData* Create(PRInt64 aOffset,
+  // aTimecode is a codec specific number representing the timestamp of
+  // the frame of video data. Returns nsnull if an error occurs. This may
+  // indicate that memory couldn't be allocated to create the VideoData
+  // object, or it may indicate some problem with the input data (e.g.
+  // negative stride).
+  static VideoData* Create(nsVideoInfo& aInfo,
+                           ImageContainer* aContainer,
+                           PRInt64 aOffset,
                            PRInt64 aTime,
                            const YCbCrBuffer &aBuffer,
                            PRBool aKeyframe,
                            PRInt64 aTimecode);
 
   // Constructs a duplicate VideoData object. This intrinsically tells the
   // player that it does not need to update the displayed frame when this
   // frame is played; this frame is identical to the previous.
@@ -139,48 +195,45 @@ public:
                                     PRInt64 aTimecode)
   {
     return new VideoData(aOffset, aTime, aTimecode);
   }
 
   ~VideoData()
   {
     MOZ_COUNT_DTOR(VideoData);
-    for (PRUint32 i = 0; i < 3; ++i) {
-      moz_free(mBuffer.mPlanes[i].mData);
-    }
   }
 
   // Approximate byte offset of the end of the frame in the media.
   PRInt64 mOffset;
 
   // Start time of frame in milliseconds.
   PRInt64 mTime;
 
   // Codec specific internal time code. For Ogg based codecs this is the
   // granulepos.
   PRInt64 mTimecode;
 
-  YCbCrBuffer mBuffer;
+  // This frame's image.
+  nsRefPtr<Image> mImage;
 
   // When PR_TRUE, denotes that this frame is identical to the frame that
   // came before; it's a duplicate. mBuffer will be empty.
   PRPackedBool mDuplicate;
   PRPackedBool mKeyframe;
 
 public:
   VideoData(PRInt64 aOffset, PRInt64 aTime, PRInt64 aTimecode)
     : mOffset(aOffset),
       mTime(aTime),
       mTimecode(aTimecode),
       mDuplicate(PR_TRUE),
       mKeyframe(PR_FALSE)
   {
     MOZ_COUNT_CTOR(VideoData);
-    memset(&mBuffer, 0, sizeof(YCbCrBuffer));
   }
 
   VideoData(PRInt64 aOffset,
             PRInt64 aTime,
             PRBool aKeyframe,
             PRInt64 aTimecode)
     : mOffset(aOffset),
       mTime(aTime),
@@ -328,63 +381,16 @@ public:
            mTimeStart == 0 &&
            mTimeEnd == 0;
   }
 
   PRInt64 mOffsetStart, mOffsetEnd; // in bytes.
   PRInt64 mTimeStart, mTimeEnd; // in ms.
 };
 
-// Stores info relevant to presenting media samples.
-class nsVideoInfo {
-public:
-  nsVideoInfo()
-    : mFramerate(0.0),
-      mAspectRatio(1.0),
-      mCallbackPeriod(1),
-      mAudioRate(0),
-      mAudioChannels(0),
-      mFrame(0,0),
-      mHasAudio(PR_FALSE),
-      mHasVideo(PR_FALSE)
-  {}
-
-  // Frames per second.
-  float mFramerate;
-
-  // Aspect ratio, as stored in the metadata.
-  float mAspectRatio;
-
-  // Length of a video frame in milliseconds, or the callback period if
-  // there's no audio.
-  PRUint32 mCallbackPeriod;
-
-  // Samples per second.
-  PRUint32 mAudioRate;
-
-  // Number of audio channels.
-  PRUint32 mAudioChannels;
-
-  // Dimensions of the video frame.
-  nsIntSize mFrame;
-
-  // The picture region inside the video frame to be displayed.
-  nsIntRect mPicture;
-
-  // The offset of the first non-header page in the file, in bytes.
-  // Used to seek to the start of the media.
-  PRInt64 mDataOffset;
-
-  // PR_TRUE if we have an active audio bitstream.
-  PRPackedBool mHasAudio;
-
-  // PR_TRUE if we have an active video bitstream.
-  PRPackedBool mHasVideo;
-};
-
 // Encapsulates the decoding and reading of media data. Reading can be done
 // on either the state machine thread (when loading and seeking) or on
 // the reader thread (when it's reading and decoding). The reader encapsulates
 // the reading state and maintains it's own monitor to ensure thread safety
 // and correctness. Never hold the nsBuiltinDecoder's monitor when calling into
 // this class.
 class nsBuiltinDecoderReader : public nsRunnable {
 public:
@@ -410,35 +416,40 @@ public:
   // than aTimeThreshold will be decoded (unless they're not keyframes
   // and aKeyframeSkip is PR_TRUE), but will not be added to the queue.
   virtual PRBool DecodeVideoFrame(PRBool &aKeyframeSkip,
                                   PRInt64 aTimeThreshold) = 0;
 
   virtual PRBool HasAudio() = 0;
   virtual PRBool HasVideo() = 0;
 
-  // Read header data for all bitstreams in the file. Fills aInfo with
+  // Read header data for all bitstreams in the file. Fills mInfo with
   // the data required to present the media. Returns NS_OK on success,
   // or NS_ERROR_FAILURE on failure.
-  virtual nsresult ReadMetadata(nsVideoInfo& aInfo) = 0;
+  virtual nsresult ReadMetadata() = 0;
 
 
   // Stores the presentation time of the first sample in the stream in
   // aOutStartTime, and returns the first video sample, if we have video.
   VideoData* FindStartTime(PRInt64 aOffset,
                            PRInt64& aOutStartTime);
 
   // Returns the end time of the last page which occurs before aEndOffset.
   // This will not read past aEndOffset. Returns -1 on failure.
   virtual PRInt64 FindEndTime(PRInt64 aEndOffset) = 0;
 
   // Moves the decode head to aTime milliseconds. aStartTime and aEndTime
   // denote the start and end times of the media.
   virtual nsresult Seek(PRInt64 aTime, PRInt64 aStartTime, PRInt64 aEndTime) = 0;
 
+  // Gets presentation info required for playback.
+  const nsVideoInfo& GetInfo() {
+    return mInfo;
+  }
+
   // Queue of audio samples. This queue is threadsafe.
   MediaQueue<SoundData> mAudioQueue;
 
   // Queue of video samples. This queue is threadsafe.
   MediaQueue<VideoData> mVideoQueue;
 
 protected:
 
@@ -485,11 +496,14 @@ protected:
 
   // Reference to the owning decoder object. Do not hold the
   // reader's monitor when accessing this.
   nsBuiltinDecoder* mDecoder;
 
   // The offset of the start of the first non-header page in the file.
   // Used to seek to media start time.
   PRInt64 mDataOffset;
+
+  // Stores presentation info required for playback.
+  nsVideoInfo mInfo;
 };
 
 #endif
--- a/content/media/nsBuiltinDecoderStateMachine.cpp
+++ b/content/media/nsBuiltinDecoderStateMachine.cpp
@@ -44,20 +44,16 @@
 #include "nsBuiltinDecoderReader.h"
 #include "nsBuiltinDecoderStateMachine.h"
 #include "mozilla/mozalloc.h"
 #include "VideoUtils.h"
 
 using namespace mozilla;
 using namespace mozilla::layers;
 
-// Adds two 32bit unsigned numbers, retuns PR_TRUE if addition succeeded,
-// or PR_FALSE the if addition would result in an overflow.
-static PRBool AddOverflow(PRUint32 a, PRUint32 b, PRUint32& aResult);
-
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gBuiltinDecoderLog;
 #define LOG(type, msg) PR_LOG(gBuiltinDecoderLog, type, msg)
 #else
 #define LOG(type, msg)
 #endif
 
 // Wait this number of seconds when buffering, then leave and play
@@ -91,20 +87,16 @@ const unsigned AMPLE_AUDIO_MS = 2000;
 // which is at or after the current playback position.
 //
 // Also if the decode catches up with the end of the downloaded data,
 // we'll only go into BUFFERING state if we've got audio and have queued
 // less than LOW_AUDIO_MS of audio, or if we've got video and have queued
 // less than LOW_VIDEO_FRAMES frames.
 static const PRUint32 LOW_VIDEO_FRAMES = 1;
 
-// The frame rate to use if there is no video data in the resource to
-// be played.
-#define AUDIO_FRAME_RATE 25.0
-
 nsBuiltinDecoderStateMachine::nsBuiltinDecoderStateMachine(nsBuiltinDecoder* aDecoder,
                                                            nsBuiltinDecoderReader* aReader) :
   mDecoder(aDecoder),
   mState(DECODER_STATE_DECODING_METADATA),
   mAudioMonitor("media.audiostream"),
   mCbCrSize(0),
   mPlayDuration(0),
   mBufferingEndOffset(0),
@@ -495,19 +487,20 @@ void nsBuiltinDecoderStateMachine::Start
     MonitorAutoExit exitMon(mDecoder->GetMonitor());
     MonitorAutoEnter audioMon(mAudioMonitor);
     if (mAudioStream) {
       // We have an audiostream, so it must have been paused the last time
       // StopPlayback() was called.
       mAudioStream->Resume();
     } else {
       // No audiostream, create one.
+      const nsVideoInfo& info = mReader->GetInfo();
       mAudioStream = new nsAudioStream();
-      mAudioStream->Init(mInfo.mAudioChannels,
-                         mInfo.mAudioRate,
+      mAudioStream->Init(info.mAudioChannels,
+                         info.mAudioRate,
                          nsAudioStream::FORMAT_FLOAT32);
       mAudioStream->SetVolume(mVolume);
     }
   }
   mPlayStartTime = TimeStamp::Now();
   mDecoder->GetMonitor().NotifyAll();
 }
 
@@ -892,17 +885,17 @@ nsresult nsBuiltinDecoderStateMachine::R
               mPlayDuration = TimeDuration::FromMilliseconds(audio->mTime);
             }
             if (HasVideo()) {
               nsAutoPtr<VideoData> video(mReader->mVideoQueue.PeekFront());
               if (video) {
                 RenderVideoFrame(video);
                 if (!audio) {
                   NS_ASSERTION(video->mTime <= seekTime &&
-                               seekTime <= video->mTime + mInfo.mCallbackPeriod,
+                               seekTime <= video->mTime + mReader->GetInfo().mCallbackPeriod,
                                "Seek target should lie inside the first frame after seek");
                   mPlayDuration = TimeDuration::FromMilliseconds(seekTime);
                 }
               }
               mReader->mVideoQueue.PopFront();
             }
             UpdatePlaybackPosition(seekTime);
           }
@@ -1001,17 +994,17 @@ nsresult nsBuiltinDecoderStateMachine::R
 
         if (mState != DECODER_STATE_COMPLETED)
           continue;
 
         LOG(PR_LOG_DEBUG, ("Shutting down the state machine thread"));
         StopDecodeThreads();
 
         if (mDecoder->GetState() == nsBuiltinDecoder::PLAY_STATE_PLAYING) {
-          PRInt64 videoTime = HasVideo() ? (mVideoFrameTime + mInfo.mCallbackPeriod) : 0;
+          PRInt64 videoTime = HasVideo() ? (mVideoFrameTime + mReader->GetInfo().mCallbackPeriod) : 0;
           PRInt64 clockTime = NS_MAX(mEndTime, NS_MAX(videoTime, GetAudioClock()));
           UpdatePlaybackPosition(clockTime);
           {
             MonitorAutoExit exitMon(mDecoder->GetMonitor());
             nsCOMPtr<nsIRunnable> event =
               NS_NewRunnableMethod(mDecoder, &nsBuiltinDecoder::PlaybackEnded);
             NS_DispatchToMainThread(event, NS_DISPATCH_SYNC);
           }
@@ -1031,87 +1024,20 @@ nsresult nsBuiltinDecoderStateMachine::R
 void nsBuiltinDecoderStateMachine::RenderVideoFrame(VideoData* aData)
 {
   NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread), "Should be on state machine thread.");
 
   if (aData->mDuplicate) {
     return;
   }
 
-  NS_ASSERTION(mInfo.mPicture.width != 0 && mInfo.mPicture.height != 0,
-               "We can only render non-zero-sized video");
-  NS_ASSERTION(aData->mBuffer.mPlanes[0].mStride >= 0 && aData->mBuffer.mPlanes[0].mHeight >= 0 &&
-               aData->mBuffer.mPlanes[1].mStride >= 0 && aData->mBuffer.mPlanes[1].mHeight >= 0 &&
-               aData->mBuffer.mPlanes[2].mStride >= 0 && aData->mBuffer.mPlanes[2].mHeight >= 0,
-               "YCbCr stride and height must be non-negative");
-
-  // Ensure the picture size specified in the headers can be extracted out of
-  // the frame we've been supplied without indexing out of bounds.
-  PRUint32 picXLimit;
-  PRUint32 picYLimit;
-  if (!AddOverflow(mInfo.mPicture.x, mInfo.mPicture.width, picXLimit) ||
-      picXLimit > PRUint32(PR_ABS(aData->mBuffer.mPlanes[0].mStride)) ||
-      !AddOverflow(mInfo.mPicture.y, mInfo.mPicture.height, picYLimit) ||
-      picYLimit > PRUint32(PR_ABS(aData->mBuffer.mPlanes[0].mHeight)))
-  {
-    // The specified picture dimensions can't be contained inside the video
-    // frame, we'll stomp memory if we try to copy it. Fail.
-    return;
-  }
-
-  unsigned ySize = aData->mBuffer.mPlanes[0].mStride * aData->mBuffer.mPlanes[0].mHeight;
-  unsigned cbSize = aData->mBuffer.mPlanes[1].mStride * aData->mBuffer.mPlanes[1].mHeight;
-  unsigned crSize = aData->mBuffer.mPlanes[2].mStride * aData->mBuffer.mPlanes[2].mHeight;
-  unsigned cbCrSize = ySize + cbSize + crSize;
-
-  if (cbCrSize != mCbCrSize) {
-    mCbCrSize = cbCrSize;
-    mCbCrBuffer = static_cast<unsigned char*>(moz_xmalloc(cbCrSize));
-    if (!mCbCrBuffer) {
-      // Malloc failed...
-      NS_WARNING("Malloc failure allocating YCbCr->RGB buffer");
-      return;
-    }
-  }
-
-  unsigned char* data = mCbCrBuffer.get();
-
-  unsigned char* y = data;
-  unsigned char* cb = y + ySize;
-  unsigned char* cr = cb + cbSize;
-  
-  memcpy(y, aData->mBuffer.mPlanes[0].mData, ySize);
-  memcpy(cb, aData->mBuffer.mPlanes[1].mData, cbSize);
-  memcpy(cr, aData->mBuffer.mPlanes[2].mData, crSize);
- 
-  ImageContainer* container = mDecoder->GetImageContainer();
-  // Currently our decoder only knows how to output to PLANAR_YCBCR
-  // format.
-  Image::Format format = Image::PLANAR_YCBCR;
-  nsRefPtr<Image> image;
-  if (container) {
-    image = container->CreateImage(&format, 1);
-  }
+  nsRefPtr<Image> image = aData->mImage;
   if (image) {
-    NS_ASSERTION(image->GetFormat() == Image::PLANAR_YCBCR,
-                 "Wrong format?");
-    PlanarYCbCrImage* videoImage = static_cast<PlanarYCbCrImage*>(image.get());
-    PlanarYCbCrImage::Data data;
-    data.mYChannel = y;
-    data.mYSize = gfxIntSize(mInfo.mFrame.width, mInfo.mFrame.height);
-    data.mYStride = aData->mBuffer.mPlanes[0].mStride;
-    data.mCbChannel = cb;
-    data.mCrChannel = cr;
-    data.mCbCrSize = gfxIntSize(aData->mBuffer.mPlanes[1].mWidth, aData->mBuffer.mPlanes[1].mHeight);
-    data.mCbCrStride = aData->mBuffer.mPlanes[1].mStride;
-    data.mPicX = mInfo.mPicture.x;
-    data.mPicY = mInfo.mPicture.y;
-    data.mPicSize = gfxIntSize(mInfo.mPicture.width, mInfo.mPicture.height);
-    videoImage->SetData(data);
-    mDecoder->SetVideoData(data.mPicSize, mInfo.mAspectRatio, image);
+    const nsVideoInfo& info = mReader->GetInfo();
+    mDecoder->SetVideoData(gfxIntSize(info.mPicture.width, info.mPicture.height), info.mAspectRatio, image);
   }
 }
 
 PRInt64
 nsBuiltinDecoderStateMachine::GetAudioClock()
 {
   NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread), "Should be on state machine thread.");
   if (!mAudioStream || !HasAudio())
@@ -1132,17 +1058,17 @@ void nsBuiltinDecoderStateMachine::Advan
       mDecoder->GetMonitor().NotifyAll();
     }
 
     if (HasAudio() && mAudioStartTime == -1 && !mAudioCompleted) {
       // We've got audio (so we should sync off the audio clock), but we've
       // not played a sample on the audio thread, so we can't get a time
       // from the audio clock. Just wait and then return, to give the audio
       // clock time to tick.
-      Wait(mInfo.mCallbackPeriod);
+      Wait(mReader->GetInfo().mCallbackPeriod);
       return;
     }
 
     // Determine the clock time. If we've got audio, and we've not reached
     // the end of the audio, use the audio clock. However if we've finished
     // audio, or don't have audio, use the system clock.
     PRInt64 clock_time = -1;
     PRInt64 audio_time = GetAudioClock();
@@ -1204,17 +1130,17 @@ void nsBuiltinDecoderStateMachine::Advan
     }
 
     // If the number of audio/video samples queued has changed, either by
     // this function popping and playing a video sample, or by the audio
     // thread popping and playing an audio sample, we may need to update our
     // ready state. Post an update to do so.
     UpdateReadyState();
 
-    Wait(mInfo.mCallbackPeriod);
+    Wait(mReader->GetInfo().mCallbackPeriod);
   } else {
     if (IsPlaying()) {
       StopPlayback(AUDIO_PAUSE);
       mDecoder->GetMonitor().NotifyAll();
     }
 
     if (mState == DECODER_STATE_DECODING ||
         mState == DECODER_STATE_COMPLETED) {
@@ -1246,17 +1172,17 @@ VideoData* nsBuiltinDecoderStateMachine:
 {
   NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread), "Should be on state machine thread.");
   mDecoder->GetMonitor().AssertCurrentThreadIn();
   PRInt64 startTime = 0;
   mStartTime = 0;
   VideoData* v = nsnull;
   {
     MonitorAutoExit exitMon(mDecoder->GetMonitor());
-    v = mReader->FindStartTime(mInfo.mDataOffset, startTime);
+    v = mReader->FindStartTime(mReader->GetInfo().mDataOffset, startTime);
   }
   if (startTime != 0) {
     mStartTime = startTime;
     if (mGotDurationFromHeader) {
       NS_ASSERTION(mEndTime != -1,
                    "We should have mEndTime as supplied duration here");
       // We were specified a duration from a Content-Duration HTTP header.
       // Adjust mEndTime so that mEndTime-mStartTime matches the specified
@@ -1284,21 +1210,21 @@ void nsBuiltinDecoderStateMachine::FindE
   {
     MonitorAutoExit exitMon(mDecoder->GetMonitor());
     endTime = mReader->FindEndTime(length);
   }
   if (endTime != -1) {
     mEndTime = endTime;
   }
 
-  NS_ASSERTION(mInfo.mDataOffset > 0,
+  NS_ASSERTION(mReader->GetInfo().mDataOffset > 0,
                "Should have offset of first non-header page");
   {
     MonitorAutoExit exitMon(mDecoder->GetMonitor());
-    stream->Seek(nsISeekableStream::NS_SEEK_SET, mInfo.mDataOffset);
+    stream->Seek(nsISeekableStream::NS_SEEK_SET, mReader->GetInfo().mDataOffset);
   }
   LOG(PR_LOG_DEBUG, ("%p Media end time is %lldms", mDecoder, mEndTime));   
 }
 
 void nsBuiltinDecoderStateMachine::UpdateReadyState() {
   mDecoder->GetMonitor().AssertCurrentThreadIn();
 
   nsCOMPtr<nsIRunnable> event;
@@ -1314,56 +1240,42 @@ void nsBuiltinDecoderStateMachine::Updat
       break;
     default:
       PR_NOT_REACHED("unhandled frame state");
   }
 
   NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
 }
 
-
-static PRBool AddOverflow(PRUint32 a, PRUint32 b, PRUint32& aResult) {
-  PRUint64 rl = static_cast<PRUint64>(a) + static_cast<PRUint64>(b);
-  if (rl > PR_UINT32_MAX) {
-    return PR_FALSE;
-  }
-  aResult = static_cast<PRUint32>(rl);
-  return true;
-}
-
 void nsBuiltinDecoderStateMachine::LoadMetadata()
 {
   NS_ASSERTION(IsCurrentThread(mDecoder->mStateMachineThread),
                "Should be on state machine thread.");
   mDecoder->GetMonitor().AssertCurrentThreadIn();
 
   LOG(PR_LOG_DEBUG, ("Loading Media Headers"));
 
   nsMediaStream* stream = mDecoder->mStream;
 
-  nsVideoInfo info;
   {
     MonitorAutoExit exitMon(mDecoder->GetMonitor());
-    mReader->ReadMetadata(info);
+    mReader->ReadMetadata();
   }
-  mInfo = info;
   mDecoder->StartProgressUpdates();
+  const nsVideoInfo& info = mReader->GetInfo();
 
-  if (!mInfo.mHasVideo && !mInfo.mHasAudio) {
+  if (!info.mHasVideo && !info.mHasAudio) {
     mState = DECODER_STATE_SHUTDOWN;      
     nsCOMPtr<nsIRunnable> event =
       NS_NewRunnableMethod(mDecoder, &nsBuiltinDecoder::DecodeError);
     NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
     return;
   }
 
-  if (!mInfo.mHasVideo) {
-    mInfo.mCallbackPeriod = 1000 / AUDIO_FRAME_RATE;
-  }
-  LOG(PR_LOG_DEBUG, ("%p Callback Period: %u", mDecoder, mInfo.mCallbackPeriod));
+  LOG(PR_LOG_DEBUG, ("%p Callback Period: %u", mDecoder, info.mCallbackPeriod));
 
   // TODO: Get the duration from Skeleton index, if available.
 
   // Get the duration from the media file. We only do this if the
   // content length of the resource is known as we need to seek
   // to the end of the file to get the last time field. We also
   // only do this if the resource is seekable and if we haven't
   // already obtained the duration via an HTTP header.
--- a/content/media/nsBuiltinDecoderStateMachine.h
+++ b/content/media/nsBuiltinDecoderStateMachine.h
@@ -177,24 +177,24 @@ public:
   // State machine thread run function. Polls the state, sends frames to be
   // displayed at appropriate times, and generally manages the decode.
   NS_IMETHOD Run();
 
   // This is called on the state machine thread and audio thread.
   // The decoder monitor must be obtained before calling this.
   PRBool HasAudio() const {
     mDecoder->GetMonitor().AssertCurrentThreadIn();
-    return mInfo.mHasAudio;
+    return mReader->GetInfo().mHasAudio;
   }
 
   // This is called on the state machine thread and audio thread.
   // The decoder monitor must be obtained before calling this.
   PRBool HasVideo() const {
     mDecoder->GetMonitor().AssertCurrentThreadIn();
-    return mInfo.mHasVideo;
+    return mReader->GetInfo().mHasVideo;
   }
 
   // Should be called by main thread.
   PRBool HaveNextFrameData() const;
 
   // Must be called with the decode monitor held.
   PRBool IsBuffering() const {
     mDecoder->GetMonitor().AssertCurrentThreadIn();
@@ -370,19 +370,16 @@ protected:
   // this value. Accessed on main and state machine thread.
   PRInt64 mSeekTime;
 
   // The audio stream resource. Used on the state machine, audio, and main
   // threads. You must hold the mAudioMonitor, and must NOT hold the decoder
   // monitor when using the audio stream!
   nsAutoPtr<nsAudioStream> mAudioStream;
 
-  // Stores presentation info about required for playback of the media.
-  nsVideoInfo mInfo;
-
   // The reader, don't call its methods with the decoder monitor held.
   // This is created in the play state machine's constructor, and destroyed
   // in the play state machine's destructor.
   nsAutoPtr<nsBuiltinDecoderReader> mReader;
 
   // The time of the current frame in milliseconds. This is referenced from
   // 0 which is the initial playback position. Set by the state machine
   // thread, and read-only from the main thread to get the current
--- a/content/media/ogg/nsOggCodecState.cpp
+++ b/content/media/ogg/nsOggCodecState.cpp
@@ -56,19 +56,29 @@
 // if addition would result in an overflow.
 static PRBool AddOverflow(PRInt64 a, PRInt64 b, PRInt64& aResult);
 
 // 64 bit integer multiplication with overflow checking. Returns PR_TRUE
 // if the multiplication was successful, or PR_FALSE if the operation resulted
 // in an integer overflow.
 static PRBool MulOverflow(PRInt64 a, PRInt64 b, PRInt64& aResult);
 
-// Defined in nsOggReader.cpp.
-extern PRBool MulOverflow32(PRUint32 a, PRUint32 b, PRUint32& aResult);
-
+static PRBool MulOverflow32(PRUint32 a, PRUint32 b, PRUint32& aResult)
+{
+  // 32 bit integer multiplication with overflow checking. Returns PR_TRUE
+  // if the multiplication was successful, or PR_FALSE if the operation resulted
+  // in an integer overflow.
+  PRUint64 a64 = a;
+  PRUint64 b64 = b;
+  PRUint64 r64 = a64 * b64;
+  if (r64 > PR_UINT32_MAX)
+     return PR_FALSE;
+  aResult = static_cast<PRUint32>(r64);
+  return PR_TRUE;
+}
 
 nsOggCodecState*
 nsOggCodecState::Create(ogg_page* aPage)
 {
   nsAutoPtr<nsOggCodecState> codecState;
   if (aPage->body_len > 6 && memcmp(aPage->body+1, "theora", 6) == 0) {
     codecState = new nsTheoraState(aPage);
   } else if (aPage->body_len > 6 && memcmp(aPage->body+1, "vorbis", 6) == 0) {
@@ -173,17 +183,18 @@ PRBool nsTheoraState::Init() {
     return mActive = PR_FALSE;
   }
   f /= n;
   if (f > PR_UINT32_MAX) {
     return mActive = PR_FALSE;
   }
   mFrameDuration = static_cast<PRUint32>(f);
 
-  n = mInfo.aspect_numerator;
+  n = mInfo.aspect_numerator;
+
   d = mInfo.aspect_denominator;
   mAspectRatio = (n == 0 || d == 0) ?
     1.0f : static_cast<float>(n) / static_cast<float>(d);
 
   // Ensure the frame isn't larger than our prescribed maximum.
   PRUint32 pixels;
   if (!MulOverflow32(mInfo.pic_width, mInfo.pic_height, pixels) ||
       pixels > MAX_VIDEO_WIDTH * MAX_VIDEO_HEIGHT ||
--- a/content/media/ogg/nsOggReader.cpp
+++ b/content/media/ogg/nsOggReader.cpp
@@ -60,16 +60,20 @@ extern PRLogModuleInfo* gBuiltinDecoderL
 #define LOG(type, msg)
 #define SEEK_LOG(type, msg)
 #endif
 
 // Chunk size to read when reading Ogg files. Average Ogg page length
 // is about 4300 bytes, so we read the file in chunks larger than that.
 static const int PAGE_STEP = 8192;
 
+// The frame rate to use if there is no video data in the resource to
+// be played.
+#define AUDIO_FRAME_RATE 25.0
+
 nsOggReader::nsOggReader(nsBuiltinDecoder* aDecoder)
   : nsBuiltinDecoderReader(aDecoder),
     mTheoraState(nsnull),
     mVorbisState(nsnull),
     mPageOffset(0),
     mCallbackPeriod(0),
     mTheoraGranulepos(-1),
     mVorbisGranulepos(-1)
@@ -130,17 +134,17 @@ static PRBool DoneReadingHeaders(nsTArra
     if (!aBitstreams [i]->DoneReadingHeaders()) {
       return PR_FALSE;
     }
   }
   return PR_TRUE;
 }
 
 
-nsresult nsOggReader::ReadMetadata(nsVideoInfo& aInfo)
+nsresult nsOggReader::ReadMetadata()
 {
   NS_ASSERTION(mDecoder->OnStateMachineThread(), "Should be on play state machine thread.");
   MonitorAutoEnter mon(mMonitor);
 
   // We read packets until all bitstreams have read all their header packets.
   // We record the offset of the first non-header page so that we know
   // what page to seek to when seeking to the media start.
 
@@ -242,49 +246,51 @@ nsresult nsOggReader::ReadMetadata(nsVid
       s->Deactivate();
     }
   }
 
   // Initialize the first Theora and Vorbis bitstreams. According to the
   // Theora spec these can be considered the 'primary' bitstreams for playback.
   // Extract the metadata needed from these streams.
   float aspectRatio = 0;
+  // Set a default callback period for if we have no video data
+  mCallbackPeriod = 1000 / AUDIO_FRAME_RATE;
   if (mTheoraState) {
     if (mTheoraState->Init()) {
       mCallbackPeriod = mTheoraState->mFrameDuration;
       aspectRatio = mTheoraState->mAspectRatio;
       gfxIntSize sz(mTheoraState->mInfo.pic_width,
                     mTheoraState->mInfo.pic_height);
       mDecoder->SetVideoData(sz, mTheoraState->mAspectRatio, nsnull);
     } else {
       mTheoraState = nsnull;
     }
   }
   if (mVorbisState) {
     mVorbisState->Init();
   }
 
-  aInfo.mHasAudio = HasAudio();
-  aInfo.mHasVideo = HasVideo();
-  aInfo.mCallbackPeriod = mCallbackPeriod;
+  mInfo.mHasAudio = HasAudio();
+  mInfo.mHasVideo = HasVideo();
+  mInfo.mCallbackPeriod = mCallbackPeriod;
   if (HasAudio()) {
-    aInfo.mAudioRate = mVorbisState->mInfo.rate;
-    aInfo.mAudioChannels = mVorbisState->mInfo.channels;
+    mInfo.mAudioRate = mVorbisState->mInfo.rate;
+    mInfo.mAudioChannels = mVorbisState->mInfo.channels;
   }
   if (HasVideo()) {
-    aInfo.mFramerate = mTheoraState->mFrameRate;
-    aInfo.mAspectRatio = mTheoraState->mAspectRatio;
-    aInfo.mPicture.width = mTheoraState->mInfo.pic_width;
-    aInfo.mPicture.height = mTheoraState->mInfo.pic_height;
-    aInfo.mPicture.x = mTheoraState->mInfo.pic_x;
-    aInfo.mPicture.y = mTheoraState->mInfo.pic_y;
-    aInfo.mFrame.width = mTheoraState->mInfo.frame_width;
-    aInfo.mFrame.height = mTheoraState->mInfo.frame_height;
+    mInfo.mFramerate = mTheoraState->mFrameRate;
+    mInfo.mAspectRatio = mTheoraState->mAspectRatio;
+    mInfo.mPicture.width = mTheoraState->mInfo.pic_width;
+    mInfo.mPicture.height = mTheoraState->mInfo.pic_height;
+    mInfo.mPicture.x = mTheoraState->mInfo.pic_x;
+    mInfo.mPicture.y = mTheoraState->mInfo.pic_y;
+    mInfo.mFrame.width = mTheoraState->mInfo.frame_width;
+    mInfo.mFrame.height = mTheoraState->mInfo.frame_height;
   }
-  aInfo.mDataOffset = mDataOffset;
+  mInfo.mDataOffset = mDataOffset;
 
   LOG(PR_LOG_DEBUG, ("Done loading headers, data offset %lld", mDataOffset));
 
   return NS_OK;
 }
 
 nsresult nsOggReader::DecodeVorbis(nsTArray<SoundData*>& aChunks,
                                    ogg_packet* aPacket)
@@ -486,22 +492,26 @@ nsresult nsOggReader::DecodeTheora(nsTAr
     PRBool isKeyframe = th_packet_iskeyframe(aPacket) == 1;
     VideoData::YCbCrBuffer b;
     for (PRUint32 i=0; i < 3; ++i) {
       b.mPlanes[i].mData = buffer[i].data;
       b.mPlanes[i].mHeight = buffer[i].height;
       b.mPlanes[i].mWidth = buffer[i].width;
       b.mPlanes[i].mStride = buffer[i].stride;
     }
-    VideoData *v = VideoData::Create(mPageOffset,
+    VideoData *v = VideoData::Create(mInfo,
+                                     mDecoder->GetImageContainer(),
+                                     mPageOffset,
                                      time,
                                      b,
                                      isKeyframe,
                                      aPacket->granulepos);
     if (!v) {
+      // There may be other reasons for this error, but for
+      // simplicity just assume the worst case: out of memory.
       NS_WARNING("Failed to allocate memory for video frame");
       Clear(aFrames);
       return NS_ERROR_OUT_OF_MEMORY;
     }
     aFrames.AppendElement(v);
   }
   return NS_OK;
 }
--- a/content/media/ogg/nsOggReader.h
+++ b/content/media/ogg/nsOggReader.h
@@ -71,17 +71,17 @@ public:
   }
 
   virtual PRBool HasVideo()
   {
     mozilla::MonitorAutoEnter mon(mMonitor);
     return mTheoraState != 0 && mTheoraState->mActive;
   }
 
-  virtual nsresult ReadMetadata(nsVideoInfo& aInfo);
+  virtual nsresult ReadMetadata();
   virtual nsresult Seek(PRInt64 aTime, PRInt64 aStartTime, PRInt64 aEndTime);
 
 private:
   // Decodes one packet of Vorbis data, storing the resulting chunks of
   // PCM samples in aChunks.
   nsresult DecodeVorbis(nsTArray<SoundData*>& aChunks,
                         ogg_packet* aPacket);
 
--- a/gfx/layers/ImageLayers.h
+++ b/gfx/layers/ImageLayers.h
@@ -238,16 +238,20 @@ public:
     PRInt32 mCbCrStride;
     gfxIntSize mCbCrSize;
     // Picture region
     PRUint32 mPicX;
     PRUint32 mPicY;
     gfxIntSize mPicSize;
   };
 
+  enum {
+    MAX_DIMENSION = 16384
+  };
+
   /**
    * This makes a copy of the data buffers.
    * XXX Eventually we will change this to not make a copy of the data,
    * Right now it doesn't matter because the BasicLayer implementation
    * does YCbCr conversion here anyway.
    */
   virtual void SetData(const Data& aData) = 0;