Closed
Bug 497023
Opened 15 years ago
Closed 14 years ago
Optimized 4:4:4 Y'CbCr to RGB routines are broken
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta1+ |
People
(Reporter: gmaxwell, Assigned: cajbir)
References
Details
Attachments
(1 file)
2.51 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre Theora has support for 4:4:4 and 4:2:0 visuals, but Oggplay is not currently handling them. Examples: http://people.xiph.org/~tterribe/tmp/ducks_take_off_444_720p.ogg http://people.xiph.org/~tterribe/tmp/mobile_422_ntsc.ogg Reproducible: Always Steps to Reproduce: 1. Obtain url for a 4:4:4 or a 4:2:2 video 2. Play in firefox 3. Observe results Actual Results: Mildly psychedelic colors. Expected Results: Correct, boring, colors. VLC and Gstreamer (only with the latest patches) are about the only widely distributed playback tools that get 4:2:2 and 4:4:4 support right. Support has existed in the libtheora forever (as well as the specification), but to create these files you previously needed to use some experimental encoder. That is about to change. Fortunately this is a fairly simple fix. I created patches against oggplay for this a while back, but they aren't currently merged and may have issues.
Comment 1•15 years ago
|
||
Fixing this is mostly dependent on changes to liboggplay. I think our current code would work fine if oggplay's API stayed the same, but I had previously discussed making some changes to that API to correctly deal with offsets and odd-sized picture regions, as well as reducing memory copies. CCing Viktor for the liboggplay bits.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•15 years ago
|
||
I had the code for a while back, but couldn't produce a proper 4:4:4 format... I've committed the patch, see f209224 in liboggplay's repository. It works fine with both of the content provided in this bug. Just as in case of 4:2:0 conversion, for both 4:2:0, and 4:4:4 SIMD instructions are used if available.
Assignee | ||
Comment 3•15 years ago
|
||
Are there test cases for this so we can confirm that the version of liboggplay we're using works?
Comment 4•15 years ago
|
||
http://v2v.cc/~j/theora_testsuite/mobile_itu601_i_422.ogg http://v2v.cc/~j/theora_testsuite/ducks_take_off_444_720p25.ogg
Comment 5•15 years ago
|
||
Actually, this was fixed by the liboggplay update.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → DUPLICATE
Reporter | ||
Comment 6•15 years ago
|
||
You might want to encode some of those simple colored square videos in 4:4:4 and 4:2:2 for your test cases; examples/png2theora from libtheora (1.1) will happily generate theora files in 4:4:4 and 4:2:2 mode.
Reporter | ||
Comment 7•15 years ago
|
||
4:4:4 is actually broken in trunk. It's not totally obvious with ducks take off but this file shows it pretty well: https://trac.xiph.org/attachment/ticket/1599/queens-444.ogv
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reporter | ||
Comment 8•15 years ago
|
||
The issue is SIMD specific; it looks file if I toss a #define DISABLE_CPU_FEATURES into liboggplay.
Reporter | ||
Comment 9•15 years ago
|
||
This patch disables SIMD for 4:4:4; it's not the correct fix, but is unconditionally safe (it is only forcing the same code paths that get used on a non-SIMD system). It would be far better to ship 4:4:4 with poor performance than with 4:4:4 broken. I'll look at fixing it correctly; but I can't test VC, and I think my original 4:4:4 support patch didn't work on VC.
Reporter | ||
Updated•15 years ago
|
blocking2.0: --- → ?
Updated•15 years ago
|
Summary: video playback doesn't support 4:4:4 and 4:2:2 visuals → Optimized 4:4:4 Y'CbCr to RGB routines are broken
Comment 10•15 years ago
|
||
I think Viktor is working on a fix for this.
Comment 11•15 years ago
|
||
finally (!!!) restructured the x86 SIMD conversion functions in commit 2249a3e that finally fixes the YUV444 conversion bug.
Comment 12•15 years ago
|
||
(In reply to comment #11) > finally (!!!) restructured the x86 SIMD conversion functions in commit 2249a3e > that finally fixes the YUV444 conversion bug. btw: could anybody please check the output on a windows machine? i've triple checked the MSVC asm code, but still...
Comment 13•15 years ago
|
||
I get the following compile error when I try to build with your changes from 2249a3e applied on Windows: cpearce@TINKAP /d/work/src/purple/media/liboggplay $ patch -p1 < liboggplay.git-2249a3e31a634e37accbc965888c7b148551efef.patch patching file src/liboggplay/oggplay_yuv2rgb.c patching file src/liboggplay/x86/oggplay_yuv2rgb_x86.c patching file src/liboggplay/x86/yuv2rgb_x86.h patching file src/liboggplay/x86/yuv2rgb_x86_vs.h cpearce@TINKAP /d/work/src/purple/media/liboggplay $ build-modules purple media/liboggplay layout/build oggplay_yuv2rgb.c Building deps for /d/work/src/purple/media/liboggplay/src/liboggplay/oggplay_yuv2rgb.c oggplay_yuv2rgb.c d:\work\src\purple\media\liboggplay\src\liboggplay\oggplay_private.h(325) : warning C4308: negative integral c onstant converted to unsigned type d:\work\src\purple\media\liboggplay\src\liboggplay\oggplay_private.h(326) : warning C4308: negative integral c onstant converted to unsigned type d:\work\src\purple\media\liboggplay\src\liboggplay\oggplay_private.h(336) : warning C4308: negative integral c onstant converted to unsigned type d:\work\src\purple\media\liboggplay\src\liboggplay\oggplay_private.h(337) : warning C4308: negative integral c onstant converted to unsigned type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(191) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(191) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(192) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(192) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(193) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(193) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(196) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(196) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(197) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(197) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(198) : error C2415: improper oper and type d:\work\src\purple\media\liboggplay\src\liboggplay\x86/oggplay_yuv2rgb_x86.c(198) : error C2415: improper oper and type make[3]: *** [oggplay_yuv2rgb.obj] Error 2 make[2]: *** [libs] Error 2 make[1]: *** [libs] Error 2 make: *** [all] Error 2
Comment 14•15 years ago
|
||
thanks to Chris Pearce we've managed to find out that the problem is with the new LOAD_YUV macro, when it tries to load data only into the lower half of the xmm/mm registers in case of U and V channels.
Comment 15•14 years ago
|
||
What's the current status of this? There doesn't seem to be a fix committed to upstream liboggplay yet.
Comment 16•14 years ago
|
||
It looks like the 4:2:2 SIMD is generating incorrect output as well.
Comment 17•14 years ago
|
||
Here's some manual tests: http://flim.org/~kinetik/theora_formats/ Load them and look at the results with a magnifying glass. In Firefox trunk, tests 3 and 4 look incorrect for 4:2:2. Test 5, which is similar to test 4 but using a larger input, works correctly with 4:2:2 but is broken with 4:4:4.
Comment 18•14 years ago
|
||
Disabling 4:2:2 SIMD fixes tests 3 and 4.
We're going to fix this by importing some alternative non-liboggplay conversion code.
Assignee: nobody → chris.double
blocking2.0: ? → beta1
Fixed by importing Chromium conversion code and adding 4:4:4 support to it in bug 551378.
Status: REOPENED → RESOLVED
Closed: 15 years ago → 14 years ago
Resolution: --- → FIXED
Comment 22•9 years ago
|
||
I am seeing this issue again in Firefox 41.0.2 (the current version). I don't know how to reopen this bug, so I hope someone sees this comment.
You need to log in
before you can comment on or make changes to this bug.
Description
•