While looking at the video in the URL (which is loaded directly, no embedded in HTML), I noticed a couple of odd things: * The left side has a column of 4 green pixels. I thought these might just be encoded in the video, but I don't see them in VLC. (Although I sometimes see a green 1-pixel column on the right in VLC?!) * The controls extend 1 pixel to the right of the visible video region. If I use DOM Inspector to set a background color on the <video>, that color is visible. So, it's like the <video> is larger than the video playback region, which doesn't make sense because the video doesn't have an explicit height/width set. I don't think the latter is specifically a problem with the controls, but I suppose it's possible. Not sure what's happening here... Maybe the actual and reported video dimensions are different?

The red on the right is after using DOMi to set video.style.backgroundColor="red".

The kind folks in #theora point out: <mtc> Total image: 864 by 480, crop offset (4, 0) <MikeS--tp> ah, so the mozilla problem on the right might just be that it's not handling the odd width properly at all - so it's padding by an extra pixel (to be an even width), and rendering that extra pixel with the bg colour. <MikeS--tp> the left-side problem is that it's ignoring the crop offset <MikeS--tp> 864x480 is the encoded size. The 853x480 is the subset of that that's meant to be actually displayed <gmaxwell_> 864x480 is the size after being rounded up to multiples of 16. The player should be cropping it back to 853x480, and sliding the crop window right by 4 pixels. DOMi indicates the <video> is 853 pixels wide (and that matches the width of the control bar in the attached screencap). There was also a suggestion that there might be a bug somewhere with odd-width videos, resulting in the transparent pixel column on the right.

It's worth noting that a few other applications do not handle offsets (mplayer; for example), so simply verifying that mozilla is working the same as 'foo' may only result in validating that both are equally broken. In my experience Gstreamer/totem and VLC handle offsets. Almost no player handles odd width 4:2:0 video cleanly (not at all a Theora specific issue).

It's a liboggplay bug btw, not the nsOggDecoder code.

This is a result of the current YUV conversion code not handling odd widths. We ran into a similar problem with the SSE and MMX optimized code when the video width wasn't a multiple of 16 and 8, so there's a workaround in place to fall back to the vanilla code in those cases. But the vanilla code currently only handles widths in multiples of 2. Viktor has a fix for this as part of fixing a related issue with the SSE and MMX code not handling widths that aren't multiples of 16 and 8, respectively. So once that's committed to the liboggplay repository, updating to that should fix this bug.

I've attached the patch fixing this to 485291 comment 39 and committed it to the master branch of liboggplay's git repository: 683f232749894a29d8ebdce668b932d4d4d140f3

Matthew, could you cherrypick this?

I rolled up the following revisions from upstream: dabde8d5d362f7491365ec0cafc7195ea3d5671e, 683f232749894a29d8ebdce668b932d4d4d140f3, and 4d7581b807c2aabc7afc961e7020e6a531909c35. Mainly the YUV conversion fixes for odd sized files, but also a couple of other minor fixes in the same code. Mochitests pass on OS X, and I've visually confirmed that Justin's testcase works correctly. The reftests fail due to two greenish pixels appearing in the bottom right of the frame--investigating this now...

Viktor pointed out the problem, but there might be a related problem showing up with the MozThankYoudemo.ogg, Viktor is investigating. To get some intermediate test coverage while working on this, I've pushed the current state of this patch (as attached above, but with the reftests marked random now marked as non-random since the problem seen in bug 486646 has now been fixed, and with Viktor's current patch applied) to the try server.

The builds went through fine, but there were a bunch of related-looking test failures on the try server. I'm pretty sure (based on inspection of the failures, and running the tests locally on Windows) these were all caused by other video breakage in the base revision I was using, so I rebased my tree against the current trunk to pick up the fixes and repushed to the try server. As a reminder to myself for the morning: need to create a new reftest for the odd sized video case so that the bug this liboggplay refresh is trying to fix is tested somewhere.

The remaining issue with MozThankYoudemo.ogg was that the video is 853 pixels wide and Y'CbCr 4:2:0, so the chroma planes are 426 pixels wide. The logic in oggplay_yuv2rgb_template.h for incrementing the pu and pv pointers doesn't deal with this and tries to use chroma samples from the 427th pixel (which doesn't exist) when converting the last pixel in this case. It needs to detect this and reuse the previous chroma sample.

Builds and tests worked fine on all platforms, except the following reftests failed on Linux: REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-linux-unittest/mozilla/layout/reftests/ogg-video/object-aspect-ratio-1a.xhtml | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-linux-unittest/mozilla/layout/reftests/ogg-video/object-aspect-ratio-2a.xhtml | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-linux-unittest/mozilla/layout/reftests/ogg-video/object-aspect-ratio-2b.xhtml | These were previously marked random, and I unmarked them. But I bet they need to be skipped on Linux because of zooming/EXTEND_PAD issues. Some of the non-object aspect ratio tests are already skipped for the same reason. Looking at the tests, object-aspect-ratio-2a.xhtml and object-aspect-ratio-2a.xhtml are the same as the non-object versions and show be skipped on Linux. object-aspect-ratio-1a.xhtml shouldn't be skipped, and it looks like the test failed because the video didn't load at all (the reftest is all white).

(In reply to comment #12) > object-aspect-ratio-1a.xhtml shouldn't be skipped, and it looks like the test > failed because the video didn't load at all (the reftest is all white). Could be bug 493100?

Includes fixes for the issues discussed in comments 8, 9, and 11. Bug 493100 looks like the same issue as the failure I saw with object-aspect-ratio-1a.xhtml on the Linux tryserver, so I think this patch is ready to land.

Fix another YUV conversion issue when dealing with odd height videos. This also includes a reftest that tests odd sized videos with an offset (it's a 61x57 green box (which Theora pads to 64x64 internally) with a black 29x29 box at offset 28x4, the displayed frames should be a black 29x29 rectangle), which should suffice to test the fix for this bug and bug 493140.

Checked in: http://hg.mozilla.org/mozilla-central/rev/14d05e1b55cd http://hg.mozilla.org/mozilla-central/rev/057648c9f7e1 I re-marked the object-aspect-ratio tests as random because they failed, e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242808933.1242814503.7174.gz http://hg.mozilla.org/mozilla-central/rev/13a61d034785

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/02d3b944151b http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d3c7125698e0 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0014703f63cb

One of the tests is causing a random orange Bug 494295 - offset-1.xhtml | timed out waiting for onload to fire