GNOME Bugzilla – Bug 537889
[xvimagesink] colorbalance is bad
Last modified: 2008-08-07 11:37:02 UTC
xvimagesink implements colorbalance, to adjust various Xv attributes. This has a number of problems: - internally, it (for no apparent reason!) scales the values. When scaling back, this introduces rounding errors. - it then sets the values on the xv port - but due to the rounding, they're different from what they were previously. - Some drivers (e.g. nvidia) treat these attributes as persistent - so the rounding errors accumulate over time, until eventually video is unwatchable (completely black). Attached patch doesn't set the values unless they were changed through the interface (fixes this problem for gst-launch, but not for totem) This still doesn't fix it entirely with totem, for two reasons: - totem sets the values every time. We could fix that by not scaling things (or by only scaling things for use via properties, not via the colourbalance interface), but... - totem reads these via the colorbalance interface, and does its own scaling, with rounding errors. Probably the best answer is to add this patch, and then remove the colorbalance stuff from totem (or at a minimum, stop setting values via that interface unless the user changes something).
Created attachment 112585 [details] [review] Don't set port attributes if they haven't been changed via interface or properties
2008-06-20 Michael Smith <msmith@songbirdnest.com> * sys/xvimage/xvimagesink.c: Don't set colour balance values on the Xv port if the user hasn't changed them (via properties or the interface). Avoids accumulating rounding errors for the common case. Partial fix for bug #537889. It'd be better to avoid the rounding errors entirely, but that would require a much larger, invasive, patch, so I'm skipping that for now. Keeping the bug open in case anyone wants to look at it.
2008-07-28 David Schleef <ds@schleef.org> * sys/xvimage/xvimagesink.c: Fix rounding errors when converting colorbalance values between hardware and object property ranges. Partial fix for #537889, however, there still seems to be a small drift problem that could be totem's fault.
Created attachment 115580 [details] [review] patch for totem Fixes rounding errors in totem.
I can confirm that with David's patch for Totem, the drift problem goes away completely. Michael, what do you mean by "avoiding the rounding errors entirely"? Use the hardware ranges also for the internal values? Then, the properties (range [-1000, 1000]) would have to be scaled on get and set. Or could we change the properties to have the same ranges as the hardware? I'd like to write a patch, but have to know what to implement.
Thanks for the patch! Committed: 2008-08-07 Tim-Philipp Müller <tim.muller at collabora co uk> Patch by: David Schleef <ds@schleef.org> * src/backend/bacon-video-widget-gst-0.10.c: (bacon_video_widget_get_video_property), (bacon_video_widget_set_video_property):: Fix rounding errors when converting colorbalance values to/from a different value range. Should fix drift in colour balance. (Closes: #545566, #537889)