Comment 8 for bug 412647

(In reply to comment #6)
> What is MOZ_NO_CONFIG?

I have no idea. Where does this appear?

>
> What state is your tree in? "media" is not present on trunk.

See the bit in the my comment about the patch requiring the 'third party library' patch to be applied first:

http://www.double.co.nz/video_test/third_party_modules.patch.gz

> We'll need to figure out a final destination for portaudio and the Ogg
> libraries. "content" isn't it.

How about in the root 'modules' directory, where libbz2, etc are? If that is the preferred place I'd need some advice on how to have 'content/media/video' have that as a dependancy.

> -- Keep a reference to pending events from the content element...

To do this I'll need to keep a collection of events. What's the best way of storing an array/list of events? Is there a collection class that can safely hold nsCOMPtr's?

If the events are posted to the queue of a thread which is not the main thread, is it sufficient to stop those threads - therefore the remaining events won't be processed? That's they way I'm currently handling it.

> It seems that you have threads other than the main thread touching content
> elements and possibly even frames.

All frame stuff is done on the main thread. The only thread code that affects the frame is via an event dispatched to the main thread. I do call GetPrimaryFrame in that non-main thread though, to pass to the invalidate event. I'll move to passing the element to the event and have that call GetPrimaryFrame so it occurs in the main thread. Is that ok?

No element code is touched on other threads except for specific members I added, so I'm pretty sure nothing in the core code is being called off the main thread. I do plan to move that code out into another object though to seperate it from the element.

> You generally do not want to pass nsCOMPtrs as parameters. Just take an
> nsIThread* here.

They are references to nsCOMPtr's and being assigned in the constructor initialisation to another nsCOMPtr. If I make it a raw nsIThread*, but my original pointer is an nsCOMPtr<nsIThread>, is it safe to do:

  void var(nsIThread* baz) { ...store baz in an nsCOMPtr for later use... }
  nsCOMPtr<nsIThread> foo = ...;
  call_bar(foo);

Can I pass that foo directly or do I need to call a method on nsCOMPtr to get at the pointer?

> You'll need to
> use operator new instead of PR_Malloc, which is also a good thing :-)

I used operator new in patch1 but another commenter via email suggested using PR_Malloc :-) What's a good strategy for protecting against integer overflow here? Given that the video size is from the ogg file, a manipulated ogg could craft an overflow.

> 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?

Yes, the height and width can change during the stream. I get the video height and width when decoding the video frame data. So I'd be doing it at the same time as I send the event that does the invalidate on the frame. Would it be better to do it there (in the nsHTMLVideoElement code, just before I call Invalidate)?

Thanks for the comments, I'll have them done in the next patch!