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 663204 - Video Sink should support color balance
Video Sink should support color balance
Status: RESOLVED FIXED
Product: clutter-gst
Classification: Other
Component: general
2.0.x
Other Linux
: Normal enhancement
: ---
Assigned To: clutter-gst-maint
clutter-gst-maint
Depends on: 656156
Blocks:
 
 
Reported: 2011-11-01 22:42 UTC by Nicolas Dufresne (ndufresne)
Modified: 2014-05-28 14:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
video-sink: implement colorbalance interface (11.64 KB, patch)
2014-02-14 19:45 UTC, Matthieu Bouron
needs-work Details | Review
video-sink: implement colorbalance interface (11.64 KB, patch)
2014-02-18 19:40 UTC, Matthieu Bouron
none Details | Review
video-sink: implement colorbalance interface (23.90 KB, patch)
2014-02-18 19:44 UTC, Matthieu Bouron
none Details | Review
video-sink: implement colorbalance interface (23.87 KB, patch)
2014-02-22 09:39 UTC, Matthieu Bouron
none Details | Review

Description Nicolas Dufresne (ndufresne) 2011-11-01 22:42:15 UTC
While digging Totem code I found that they where using videobalance element to handle saturation, hue, brightness and contrast. I think we could improve performance by implementing GstColorBalance interface using shaders. Some upstream clutter work to add such effects seems inline. Also, this is one of the current blocker for enabling HW accelerated buffers being rendered in Totem.

Add lightness, brightness, and contrast effects
https://bugzilla.gnome.org/show_bug.cgi?id=656156
Comment 1 Lionel Landwerlin 2014-01-07 10:38:26 UTC
I'm currently looking at adding that feature into cogl-gst : 

https://github.com/djdeath/cogl/commit/8f1091f26b6274722f044b09bc0fc7623d89b90b

Should be available in ClutterGst 3.0.
Comment 2 Matthieu Bouron 2014-02-14 19:45:11 UTC
Created attachment 269138 [details] [review]
video-sink: implement colorbalance interface

The following patch implement the colorbalance interface for the clutter-gst-2.0 video-sink.

The current limitation on this implementation is relative to the clutter desaturate effect which only supports values from 0.0 (fully saturated, ie: original image) to 1.0 (fully desaturated).
Comment 3 Lionel Landwerlin 2014-02-17 09:26:09 UTC
Thanks a lot for looking into that for ClutterGst 2.0, but this implementation is unlikely to perform well.
Here you're using 3 clutter effects derived from ClutterOffscreenEffect. That means that when you're using the 3 of them at the same time, you're copying the video texture 3 times before it finally ends on the stage. It's also potentially consuming a fair amount of memory.

I would recommend that you look at the stuff I'm trying to get into Cogl 1.18 :

https://github.com/djdeath/cogl/commit/5904aac57b4e8733cefd84a7f78e1ec0d2b82c31

It pretty similar to what the videobalance gst element does, except it's done on the gpu : we compute 3 lookup tables for y, u and v, which are then uploaded on the gpu memory, what's left to do by the gpu, is the actual lookup for each pixel.
Comment 4 Matthieu Bouron 2014-02-17 14:37:02 UTC
(In reply to comment #3)
> Thanks a lot for looking into that for ClutterGst 2.0, but this implementation
> is unlikely to perform well.
> Here you're using 3 clutter effects derived from ClutterOffscreenEffect. That
> means that when you're using the 3 of them at the same time, you're copying the
> video texture 3 times before it finally ends on the stage. It's also
> potentially consuming a fair amount of memory.

Thanks for the review. I agree that it is not the most efficient way to implement it and that it can be done within a single pass.
Do you know some kind of tool that can be used to mesure the GPU performance and mesure the impact of this code ?

> 
> I would recommend that you look at the stuff I'm trying to get into Cogl 1.18 :
> 
> https://github.com/djdeath/cogl/commit/5904aac57b4e8733cefd84a7f78e1ec0d2b82c31
> 
> It pretty similar to what the videobalance gst element does, except it's done
> on the gpu : we compute 3 lookup tables for y, u and v, which are then uploaded
> on the gpu memory, what's left to do by the gpu, is the actual lookup for each
> pixel.

Thanks, i will try to implement a shader effect using this method.
Comment 5 Matthieu Bouron 2014-02-18 19:40:22 UTC
Created attachment 269606 [details] [review]
video-sink: implement colorbalance interface

Patch updated. It now uses a single shader to perform the colorbalance operation.
Comment 6 Matthieu Bouron 2014-02-18 19:44:56 UTC
Created attachment 269607 [details] [review]
video-sink: implement colorbalance interface

Right patch attached, sorry for the noise :(
Comment 7 Matthieu Bouron 2014-02-22 09:39:10 UTC
Created attachment 269985 [details] [review]
video-sink: implement colorbalance interface

Fixes comment about Y/U/V ranges for the ycbcr2rgb shader function and fixes CLUTTER_GST_*_DEFAULT declarations (BRIGHTNESS and CONTRAST was inverted).
Comment 8 Matthieu Bouron 2014-04-02 16:11:15 UTC
Here is some benchmark. intel_gpu_top is used to mesure how busy the gpu is. The following pipeline has been used:

LIBGL_SHOW_FPS=1 gst-launch-1.0 videotestsrc ! video/x-raw,format=RGBA,width=1920,height=1080,framerate=60/1 ! cluttersink

CPU: Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz
Graphics: Intell HD4000.

cluttersink window (on screen) is of size 2300x1276.

* 1920x1080p @60fps, colorbalance disabled

average FPS = 60fps
average render busy: 29%

* 1920x1080p @60fps, colorbalance enabled

average FPS = 60fps
average render busy: 45%

* 3800x1080 @60fps, colorbalance disabled

average FPS = 60fps
average render busy: 29%

* 3800x1080 @60fps, colorbalance enabled

average FPS = 60fps
average render busy: not stable ~83% jumps down to 50% for short period of times.
Comment 9 Lionel Landwerlin 2014-04-02 16:47:43 UTC
In the last patch you attached, there is still an additional step of going through a ClutterOffscreenEffect.
This basically means the application keeps a buffer of the size of the video and goes through 

video texture -> offscreen buffer -> application buffer (+ composition of the window manager)

The patch I've mentioned on Cogl doesn't use an intermediate buffer to do this.
It obviously requires more invasive changes to ClutterGst which is why I'm dragging my feet on this. This is basically rewriting something already done somewhere else.

Though, since the CoglGst transition didn't happen for 3.12, I'll try to find some time to do it.
Comment 10 Lionel Landwerlin 2014-05-28 14:21:54 UTC
I reimported the video sink into ClutterGst.
It uses the color correction implementation I pointed to.
It's on master.
Comment 11 Nicolas Dufresne (ndufresne) 2014-05-28 14:38:29 UTC
Great ! thanks.
Comment 12 Matthieu Bouron 2014-05-28 14:39:47 UTC
Thanks :)