After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 614466 - videomixer makes some formats grayscale
videomixer makes some formats grayscale
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Linux
: Normal blocker
: 0.10.22
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-03-31 13:50 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2010-04-19 14:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
example image (11.20 KB, image/png)
2010-04-19 07:06 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
  Details
example image (13.48 KB, image/png)
2010-04-19 08:44 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
  Details
videomixer: Add i387 floating point registers to the clobbered registers list (3.20 KB, patch)
2010-04-19 14:44 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2010-03-31 13:50:16 UTC
this is color:
gst-launch-0.10 videotestsrc ! video/x-raw-yuv,format =\(fourcc\)I420 ! xvimagesink

this makes it gray:
gst-launch-0.10 videotestsrc ! video/x-raw-yuv,format =\(fourcc\)I420 ! videomixer ! xvimagesink

this is color again:
gst-launch-0.10 videotestsrc ! videomixer ! ffmpegcolorspace ! xvimagesink

this might be a regression in recent videomixer patches
Comment 1 Sebastian Dröge (slomo) 2010-04-02 17:31:06 UTC
I can't confirm this here... all three pipelines produce the same output.

Could you try to bisect this?
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-03 18:44:43 UTC
It happens on suse too. WIll not try with older releases (if those are easily downgradeable).
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 07:06:20 UTC
Created attachment 159050 [details]
example image

gst-launch-0.10 videotestsrc num-buffers=1 ! video/x-raw-yuv,format =\(fourcc\)I420 ! videomixer ! tee name=t ! queue ! xvimagesink t. ! queue ! ffmpegcolorspace ! pngenc ! filesink location=test.png

These are the caps that the plugins negotiate to:
caps = video/x-raw-yuv, format=(fourcc)I420, color-matrix=(string)sdtv, chroma-site=(string)mpeg2, width=(int)320, height=(int)240, framerate=(fraction)30/1

$ file test.png 
test.png: PNG image, 320 x 240, 8-bit/color RGB, non-interlaced

Also the output is in color, there are just no colors left. If I do

gst-launch-0.10 -v videotestsrc -v num-buffers=1 ! videomixer ! tee name=t ! queue ! ffmpegcolorspace ! xvimagesink t. ! queue ! ffmpegcolorspace ! pngenc ! filesink location=test.png

the caps are almost the same:
caps = video/x-raw-yuv, format=(fourcc)I420, width=(int)320, height=(int)240, framerate=(fraction)30/1, color-matrix=(string)sdtv

but there is not chroma-site=(string)mpeg2 parameter. The image is then in color.
Comment 4 Sebastian Dröge (slomo) 2010-04-19 08:31:14 UTC
The attached image is quite colorful here... did you attach the wrong one? And both pipelines are producing the same, colorful output.

Which architecture are you using? Does it make a difference if you use OIL_CPU_FLAGS=0? Do you know which commit introduced this?
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 08:44:29 UTC
Created attachment 159052 [details]
example image

dumb me was running the second pipeline and thus replacing the image. I was expecting the brower to load the file after it has been selected.

Good catch, by using
OIL_CPU_FLAGS=0 it works. I've also tried the latest liboil from git and bug is still there. Will file a upstream bug in a sec.
Comment 6 Sebastian Dröge (slomo) 2010-04-19 08:54:18 UTC
(In reply to comment #5)

> Good catch, by using
> OIL_CPU_FLAGS=0 it works. I've also tried the latest liboil from git and bug is
> still there. Will file a upstream bug in a sec.

videomixer uses liboil only for CPU feature detection... it must be a bug in the MMX assembly then. Which architecture are you using? 32 bit x86?
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 08:58:21 UTC
It is 32bit on two of those (Centrino Core Duo)
$ cat /proc/cpuinfo 
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 14
model name	: Genuine Intel(R) CPU           T2400  @ 1.83GHz
stepping	: 8
cpu MHz		: 1000.000
cache size	: 2048 KB
physical id	: 0
siblings	: 2
core id		: 0
cpu cores	: 2
apicid		: 0
initial apicid	: 0
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 10
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe constant_tsc arch_perfmon bts pni monitor vmx est tm2 xtpr pdcm
bogomips	: 3657.81
clflush size	: 64
power management:
Comment 8 Sebastian Dröge (slomo) 2010-04-19 09:01:22 UTC
Ok, on 64 bit your two pipelines work as expected. Maybe others can confirm this?
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 11:02:39 UTC
I don't think i is the asm. As pad_alpha is 1.0 dy default it should do the memcpy

GST_DEBUG="videom*:3,default:3" gst-launch-0.10 -v videotestsrc ! video/x-raw-yuv,format="(fourcc)I420" ! videomixer sink_0::alpha=1.0 ! xvimagesink

0:00:01.767935936 17707  0x9170ab0 INFO                 default blend.c:558:_blend_i420_mmx: Fast copy (alpha == 1.0)

...

and the MEMCPY is a plain memcpy according to blend.c:377.

I thing we should make this as a blocker, as it was working previously (I420 was not supported and it worked via ffmpegcolorspace).
Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 11:46:31 UTC
It seem to be a bug in _memcpy_u8_mmx(). Its trashing the the value for src_alpha. Which for the U/V loops becomes NAN. This does not happen if I call the ordinary memcpy.

0:00:01.985169347 26735  0x8f17f30 INFO                 default blend.c:562:blend_i420_mmx: src 0x8f2a300, dst 0xb67d3000
0:00:01.985213067 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx:   src 0x8f2a300, dst 0xb67d3000, strides 320/320, width=320/320, heigth=240
0:00:01.985250433 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx: Fast copy (alpha == 1.000000)
0:00:01.985342693 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx:   src 0x8f3cf00, dst 0xb67e5c00, strides 160/160, width=160/160, heigth=120
0:00:01.985377125 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx: Slow blend (alpha == nan)
0:00:01.985434255 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx:   src 0x8f41a00, dst 0xb67ea700, strides 160/160, width=160/160, heigth=120
0:00:01.985467779 26735  0x8f17f30 INFO                 default blend.c:562:_blend_i420_mmx: Slow blend (alpha == nan)

I am checking the assembly now.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 12:02:58 UTC
While I have no luck with the finding the issue of the asm code, I also compared the speed

time gst-launch-0.10 -q videotestsrc num-buffers=10000 ! video/x-raw-yuv,format="(fourcc)I420" ! videomixer sink_0::alpha=1.0 ! fakesink sync=false

mmx-version:
real	0m8.810s
user	0m8.781s
sys	0m0.020s

real	0m9.035s
user	0m8.737s
sys	0m0.224s

ordinary memcpy:
real	0m7.027s
user	0m6.928s
sys	0m0.072s

real	0m7.321s
user	0m6.928s
sys	0m0.364s
Comment 12 Stefan Sauer (gstreamer, gtkdoc dev) 2010-04-19 12:50:50 UTC
the bug is gone when I remove then inline keyword from blend.c::_blend_i420_##name

compiler bug?
$ gcc --version
gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1
Comment 13 Sebastian Dröge (slomo) 2010-04-19 13:58:33 UTC
Ok, so let's use plain memcpy/memset in the u8 cases then. With newer glibc this should be faster anyway (because it uses SSE assembly and stuff).

Patch is coming soon...
Comment 14 Sebastian Dröge (slomo) 2010-04-19 14:44:46 UTC
Created attachment 159085 [details] [review]
videomixer: Add i387 floating point registers to the clobbered registers list

They are the same as the mm0-mm7 MMX registers and will be overwritten
by the assembly code if gcc doesn't know about the MMX registers.

Fixes bug #614466.
Comment 15 Sebastian Dröge (slomo) 2010-04-19 14:58:59 UTC
commit 3f88dce350afa87f68fcacecef585885d4b90aa2
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Mon Apr 19 16:43:28 2010 +0200

    videomixer: Add i387 floating point registers to the clobbered registers lis
    
    They are the same as the mm0-mm7 MMX registers and will be overwritten
    by the assembly code if gcc doesn't know about the MMX registers.
    
    Note: They're all added to the list of clobbered registers in all cases
    and not only when __MMX__ is not defined just to make sure that no other
    bugs happen with this code just because some compiler version gets things
    wrong.
    
    Fixes bug #614466.