patches base on the ones in: https://bugzilla.gnome.org/show_bug.cgi?id=720375

Created attachment 264122 [details] [review]
0005-vaapipostproc-add-colorbalance-filters-hue-saturatio

Created attachment 264123 [details] [review]
0006-vaapipostproc-add-GstColorBalance-interface

Review of attachment 264122 [details] [review]:

OK

Hmm, sorry for the long delay! I will push this for next release (0.5.11).

Review of attachment 264122 [details] [review]:

Pushed with minor changes for a while now. :)

Review of attachment 264123 [details] [review]:

This needs to be reworked in a way comparable to what is "now" implemented in vaapisink.

(In reply to Gwenole Beauchesne from comment #7)
> Review of attachment 264123 [details] [review] [review]:
> 
> This needs to be reworked in a way comparable to what is "now" implemented
> in vaapisink.

I can do that if that's OK for Zhao.

For sake of completion, I'm adding color balance support to gst-player: 

https://github.com/sdroege/gst-player/pull/57

Created attachment 305088 [details] [review]
vaapipostproc: add color balance interface

I have setup a branch for gst-player to play with vaapipostproc controls using gtk ui:

https://github.com/ceyusa/gst-player/tree/vaapipostproc

The interesting thing is, if I enable the STE, and then play with the color balance channels, I got a crash:

gen75_picture_process.c:169: VAStatus gen75_proc_picture(VADriverContextP, VAProfile, union codec_state *, struct hw_context *): Assertion `pipeline_param->num_filters <= 4' failed.
Aborted

Perpahs we need to handle this. Perhaps the fix belongs to libva-driver-intel.

By the way, the color balance support in gst-player has been merged \o/

(In reply to Víctor Manuel Jáquez Leal from comment #11)
> I have setup a branch for gst-player to play with vaapipostproc controls
> using gtk ui:
> 
> https://github.com/ceyusa/gst-player/tree/vaapipostproc
> 
> The interesting thing is, if I enable the STE, and then play with the color
> balance channels, I got a crash:
> 
> gen75_picture_process.c:169: VAStatus gen75_proc_picture(VADriverContextP,
> VAProfile, union codec_state *, struct hw_context *): Assertion
> `pipeline_param->num_filters <= 4' failed.
> Aborted
> 
> Perpahs we need to handle this. Perhaps the fix belongs to
> libva-driver-intel.

The fix indeed belongs to libva-intel-driver. Thanks.

Review of attachment 305088 [details] [review]:

::: gst/vaapi/gstvaapipostproc.c
@@ +1652,3 @@
+  postproc->saturation = DEFAULT_SATURATION;
+  postproc->brightness = DEFAULT_BRIGHTNESS;
+  postproc->contrast = DEFAULT_CONTRAST;

You can get the default values from the filter_ops pspec.

@@ +1731,3 @@
+
+  for (i = 0; i < G_N_ELEMENTS (cb_channels); i++) {
+    if (g_strcmp0 (channel->label, cb_channels[i].name) == 0)

I believe the common usage is to delegate that to g_ascii_strcasecmp(). Check e.g. vaapisink. :)

@@ +1737,3 @@
+  return 0;
+}
+

You can add a cb_get_value_ptr() that returns a pointer to the gfloat, or NULL if not found. Then use that in set/get to set the final value. Besides, if you follow the H/S/B/C order, then setting up, or clearing, the relevant GST_VAAPI_POSTPROC_FLAG_xxx can be easily based upon GST_VAAPI_POSTPROC_FLAG_HUE + i where i = (op - GST_VAAPI_FILTER_OP_HUE).

Created attachment 305170 [details] [review]
vaapipostproc: add color balance interface

Attachment 305170 [details] pushed as cbc2d15 - vaapipostproc: add color balance interface