A few comments: Remove "configure" changes from subsequent patches. What is MOZ_NO_CONFIG? -DIRS = base canvas media html xml xul xbl xslt +DIRS = base canvas html xml xul xbl xslt What state is your tree in? "media" is not present on trunk. We'll need to figure out a final destination for portaudio and the Ogg libraries. "content" isn't it. You should probably start making your printfs #ifdef DEBUG_doublec, or (better) convert them to PR_LOG. +class VideoInvalidateEvent : public nsRunnable { +public: + VideoInvalidateEvent(nsIFrame* f) : mFrame(f) {} There will be problems if the frame goes away while one of these events is pending. Similar issues apply to your other events. There are two ways to solve this: -- Keep a reference to pending events from the content element or (in this case) the frame, and if the element/frame is destroyed while an event is pending, call a method on the event to disconnect it from the data that's going away. nsRevocableEventPtr can help with that. -- Hold strong (nsCOMPtr/nsRefPtr) references to content elements from events so the element can't be deleted until the event has fired. You can't hold strong references to frames, since they're not refcounted, but you can hold a reference to the frame's element and simply re-lookup the frame when the event fires, via an nsIPresShell (which you can get from a frame, and also should hold a strong reference to). Of course you have to be aware there might no longer be a frame. These events need comments explaining what they're for and what thread they run on. In fact some documentation for the general plan here would be good. It seems that you have threads other than the main thread touching content elements and possibly even frames. This is not good. Any state that needs to be shared across threads should be broken out into its own object. This might be a good time to break out the ogg-playing code into its own object so there's less ogg-specific stuff in nsHTMLVideoElement. + virtual ~VideoInvalidateEvent() { } This is not needed. + PresentationEvent(nsHTMLVideoElement* element, nsCOMPtr& decodeThread, OggPlay* player, PaStream* audio_stream, double framerate, double lastVideoFrameTime) : You generally do not want to pass nsCOMPtrs as parameters. Just take an nsIThread* here. Also, naming conventions: aAudioStream, etc. + mDecoding = false; PR_FALSE (and PR_TRUE elsewhere, etc) + mAudioThread = 0; nsnull + if (mRGB) { + PR_Free(mRGB); + } Use nsAutoPtr/nsAutoArrayPtr to avoid manual freeing of buffers. You'll need to use operator new instead of PR_Malloc, which is also a good thing :-) + unsigned long mFrames; Use PRUint32 or PRUint64. + unsigned char* getVideoRGB(); GetVideoRGB() (similar for other names) + bool IsStopped(); + bool IsDecoding(); + bool IsPaused(); You should really use PRBool instead of bool, here and elsewhere. + NS_IMETHOD Stop(); Don't use NS_IMETHOD unless you're actually implementing an XPCOM interface. You probably just want "virtual void" here. +class AudioDevice Public classes like this should use the ns... prefix. Ditto for ChannelToPipeListener and OggPlayChannelReader. + gfxContext* ctx = (gfxContext*)aRenderingContext.GetNativeGraphicData(nsIRenderingContext::NATIVE_THEBES_CONTEXT); static_cast<> + nsIDeviceContext *idc; + aRenderingContext.GetDeviceContext(idc); + double p2a = idc->AppUnitsPerDevPixel(); Use GetPresContext()->AppUnitsToDevPixels instead. + if((unsigned int)mLastFrameSize.width != element->getVideoWidth() || + (unsigned int)mLastFrameSize.height != element->getVideoHeight()) { + printf("Frame Needs Reflow!\n"); + nsPresContext *presContext = PresContext(); + nsIPresShell *presShell = presContext ? presContext->GetPresShell() : 0; + if (presShell) { + presShell->FrameNeedsReflow(this, nsIPresShell::eStyleChange, + NS_FRAME_IS_DIRTY); Why are you doing this while painting? Is this the only time you can get the video height/width? Can the video height and width change during the stream or something? As you know, this needs to change to scale the video to the size of the frame. That will also take care of the situation where there's multiple device pixels per CSS pixel. I know you just copied stuff from nsImageFrame but GetInnerArea and mBorderPadding can go away; just call GetContentRect() instead of GetInnerArea() (but GetContentRect() returns a rect relative to the parent, so you'll want GetContentRect() - GetPosition(), or something like that)