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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: gmaxwell, Assigned: cajbir)

References

Details

Attachments

(1 file)

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.
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
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.
Are there test cases for this so we can confirm that the version of liboggplay we're using works?
Actually, this was fixed by the liboggplay update.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → DUPLICATE
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.
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 → ---
The issue is SIMD specific; it looks file if I toss a #define DISABLE_CPU_FEATURES
 into liboggplay.
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.
blocking2.0: --- → ?
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
I think Viktor is working on a fix for this.
finally (!!!) restructured the x86 SIMD conversion functions in commit 2249a3e that finally fixes the YUV444 conversion bug.
(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...
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
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.
What's the current status of this?  There doesn't seem to be a fix committed to upstream liboggplay yet.
It looks like the 4:2:2 SIMD is generating incorrect output as well.
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.
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 ago14 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: