OSD700 Release 2

Before I get into my work over the past two weeks, Here's links to the code and bugs:

GitHub Branch for bug 715323

Bugzilla bug for 715323

Github Branch for bug 517363

Bugzilla bug for 517363

 

Bug 715323 - nsBuiltinDecoderReader::NotifyDataArrived() should have a 64bit offset parameter

This was an easy bug to work on. All that was needed was a change of type for a parameter in the NotifyDataArrived method.  It used to accept an unsigned 32 bit integer, which was sufficient, but limited the offset length to (2^32)-1 bytes. Sounds like a lot to me, but I imagine there are people out there who will hit that limit. I went ahead and changed the value to an unsigned 64 bit integer, which worked great.  After a review, Chris Pearce requested that I change it to a signed 64 bit integer because there could have been a case where a -1 could be used to indicate a failure. Easy change to make, now the change has been pulled into mozilla-central.

 

Bug 517363 - Preserve video poster aspect ratio when scaling

This bug proved itself to be exponentially more trivial than I had first expected. In a nutshell, if you create a video element that is 500x500 in size, and have a video that is 640x480, Firefox will preserve the aspect ratio of the video within the border of the video element. If that video has a poster frame image that's also 640x480, Firefox will stretch and skew it to fit within the video element. Here's what that looks like: The poster is clearly stretched out.  This bug requested a fix so that the poster image is scaled to maintain it's original aspect ratio when it is placed within video elements.

My first order of business figuring this one out was discovering where the poster was being painted to the element. A quick search in MXR for "poster frame" led me to nsVideoFrame.cpp. Aside from all the code in there I did not understand, one thing stood out, and that was this line. It looked like it was setting the height and width to draw the poster image. I commented it out and tested it, and found that it did indeed specify the size of the image.

I was now faced with a problem. I had no information about the image, or the size of the video element. I didn't even know how to scale this properly. What I did know was that every frame of the video was scaled in some way. quickly scanning through the file led me to a fairly obvious method called CorrectForAspectRatio, that from what I could tell, calculated the right width, height and position to draw the video frames within the element. Over the next several days, I hacked different things into the code to try and find the right solution but ended up with nothing solid.

Then, a couple days ago, I was driving home from Seneca, and The solution came to me. I needed to get references to the poster frame's anonymous image element and get a reference to the video element's dimensions, then do similar calculations to CorrectForAspectRatio. The key to getting the elements' information was do_QueryInterface, a magical method that gets me a reference to a type that isn't necessarily defined or in scope at runtime. (at least I *think* that's what it does). Once I had the code I beleived would scale the poster, I realized I needed code to ensure it was centered in the video element horizontally and vertically. Easy stuff, (N1 / 2) - (N2 / 2) did the trick easily.  Here's what things look like after my patch:

Both of these show unusual widths, so here is what it looked like with large heights and small widths:

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

 

So that works quite nicely. Only problem, is that my hack is ugly and needs a lot of work yet. But at least it works. I've submitted the patch for feedback here

Keep on Hacking!