GNOME Bugzilla – Bug 526214
Cheese should not use "xgamma" for flash effect - it unsets ICC calibration
Last modified: 2008-05-25 08:55:58 UTC
Steps to reproduce: Generate an ICC profile for your monitor, via some means. Run "xcalib yourprofile.icc" Run Cheese Take a picture Expected results: Flash! Actual results: Flash! Your display is now restored to its uncalibrated state! The issue lies where Cheese uses xgamma to determine the current state of the monitor. When, e.g. xcalib has been used to calibrate properly, xgamma has no chance of representing the state correctly, so it just returns 1.0, 1.0, 1.0. It is worth pointing out that "gamma curves" are simple response functions, rarely representative of a display's actual response. alex@fizz:~$ xcalib lcd.icc alex@fizz:~$ xgamma -> Red 1.000, Green 1.000, Blue 1.000 So we need to come up with another way of flashing the screen. Can we detect if a compositor is running, create a full screen RGBA window and use window manager hints to modify its opacity?
Confirming. Don't know how to solve it though
I confirm this (I use an ICC profile for color calibration)
Created attachment 111362 [details] proposed utility to do the "flashing" I'm experiencing the same issue here, the X server calibration is lost after taking a photo with Cheese. Attached is a quick hack that simulates the described effect and doesn't mess with the gamma values, which leaves the settings of xcalib intact. You probably should get the code directly into Cheese, and not in a seperate binary like this flasher.c is currently, but I did it as a standalone app so you can see the resulting effect and can play around with it a bit more. Compile with: gcc -o flasher flasher.c `pkg-config --libs --cflags gtk+-2.0` I'd be happy if you could get this into Cheese. You can still fallback to the "old" way of messing with the gamma in case the user does not have a compositing window manager running.
Hi Thomas Thanks for making this effort. I've made some changes. The flash actually serves a practical purpose here, too, in that the white light from your screen /does/ act as a white flash. Thus, you need to co-ordinate the white light with the image capture. I have simply done away with the ramp-up, instantly (or at least as quickly as possible) flooding the screen with white, with a 250ms or so pause and then ramp-down. This should ensure that the small amount of round trip lag between the screen and the camera is accounted for. Using the RGBA colourmap explicitly is unnecessary when you're dealing with window opacity. I dropped it. I added code to stop the window from taking any focus, and to make it pass input events (e.g. mouse clicks etc.). Outstanding issues: In Compiz, the "fade windows" and "animations" plugins get in the way and do annoying things themselves. Also, "blur" makes things look blurry under this window -- we don't want that effect. We should find a way to hint at the window manager not to do any kind of effect on these windows. Need to drop the realize() call and put the input shaping into a callback for the realize signal. I can take care of integrating this all into Cheese.
Created attachment 111372 [details] new version, with my edits
Thanks for your feedback and input. I tried your version, but it doesn't do the alpha fading, so I think the RGBA colormap-setting code is still needed (all my packages are from Ubuntu Hardy, using Compiz Fusion for compositing). Adding the colormap-setting code from my original version to your code makes things work again, so it does serve a purpose, after all, I assume.
Very nice improvement for the flashing. When you make the patch for cheese my suggestion is to remove the flash code from cheese-webcam.c and put it in a cheese-flash.c file which just exports one function "void cheese_flash ()"
Created attachment 111393 [details] [review] Patch Work in progress. Unfortunately I'm blocked from testing because of a segfault in the current HEAD.
Comment on attachment 111393 [details] [review] Patch Oh, and I still need to get to the bottom of the need for the RGBA colourmap... Anyone have any ideas? If not, I'll just add the code with a comment.
Created attachment 111408 [details] [review] Patch OK, I'm pretty happy with this. Please review.
thanks for your help! just two things: - the "take a photo" button does not get back into sensitive state, please check your return values - would it be possible to check if a compositing manager is running, and if not using the old xgamma way?
Strange that the take photo button doesn't work for you. In the cheese_webcam_flash function I have a g_signal_emit (webcam, webcam_signals[PHOTO_SAVED], 0); (with a comment), and indeed it does seem to work for me. Maybe it's a race condition? Also, I've added a non-composited alternative that simply avoids the fade. I am reluctant to restore the XGamma stuff because, like the original bug comment states, it unsets calibration LUTs. And it looks ugly, as it doesn't really make the screen white, and black/ My observations: Metacity, no compositor: Screen flashes, no fade, but panels appear on top still. Odd. Metacity, with compositor: Screen flashes, fade is a little bit uneven on my system (it's using XRender, remember), again, panels on top. Compiz: Screen flashes, fade is smooth, but a few of the effects get in the way. Their devs suggest that there is no standard way to request a window be exempt from effects, so we'll (they'll) just have to deal with it by special casing or something until they figure something out for this and other OSD-style windows.
Created attachment 111410 [details] [review] Patch Added the code path for non-composited cases. Simply avoid the fade-out.
Created attachment 111412 [details] [review] Patch Moved the signal emission from webcam_flash to webcam_photo_data_cb. Fixes the race condition for when you take photos without a countdown.
Created attachment 111413 [details] [review] Patch Added: gtk_window_set_keep_above (window, TRUE);
Created attachment 111414 [details] [review] Patch Manage the timeouts properly so that flashes that fire in rapid succession don't break.
Created attachment 111415 [details] [review] Patch Renamed cheese_flash_flash to cheese_flash_fire -- less of a silly name. Got rid of cheese_webcam_flash and replaced its use with a direct call to cheese_flash_fire.
Created attachment 111467 [details] [review] Updated patch which conforms to coding style of cheese I updated the patch so it conforms to our coding style. Please this as a reference for future patches. There are still problems with the patch. If I run cheese with patch the following happens Compiz disabled --------------- Segfault when taking a picture. BTW I don't have a camera connected but use the test image Compiz enabled ------------- Crash at startup cheese: ../../src/xcb_lock.c:33: _XCBUnlockDisplay: Assertion `xcb_get_request_sent(dpy->xcb->connection) == dpy->request' failed. Aborted
Forgot to mention that in the updated patch I also change configure.ac because we don't use xxf86vm anymore. Forgot to mention as well that the initial flasher.c program works fine for me Daniel I'm in favor of ripping out xxf86m because of the bug reports we're getting. More and more people will run composited desktops so most of them will get the nice flash.
Jaap I unloaded my camera module and started cheese up, it waited for a few seconds then gave me the test card with the yellow information box, just fine. WORKSFORME, so I can't debug it. You'll have to get a trace and look into it yourself. Note, though, that I saw that XCBUnlockDisplay crasher once, though it was a one-off and went away as soon as I tried it again. I assumed it had nothing to do with my changes.
Created attachment 111476 [details] [review] This patch works :-) Alex, you're calling gtk functions in the cheese_flash_init code. This gets called in cheese_webcam_new, but cheese_webcam_new is running in another thread so cheese_webcam_new has to be run in a critical section. I added that to the patch. Daniel, Do you agree to add the patch?
sure! awesome work of everybody! just one comment: when running with metacity (not composited) and i click on the "take a photo" button, the button looses its focus and the window too. i would have to click on the window once and then on the button to take a photo again.
If I run metacity in non composited mode the button does not loose focus. I committed the patch can you double check if it works. 2008-05-25 Jaap Haitsma <jaap@haitsma.org> * configure.ac, src/cheese-flash.c, src/cheese-flash.h, src/cheese-webcam.c, src/cheese-window.c, src/Makefile.am: Add new flash and remove changing gamma for simulating flash. The new flash works best if you run a composited desktop. Patch by Alex "weej" Jones and Thomas Perl. Fixes #526214
Did you forget to "svn add" the cheese-flash files? Trying to build from an up-to-date SVN trunk checkout gives me: cheese-webcam.c:37:26: error: cheese-flash.h: No such file or directory
(In reply to comment #24) > Did you forget to "svn add" the cheese-flash files? > Yes :-( I now added them