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 712834 - video-sink: add gl texture upload meta api support
video-sink: add gl texture upload meta api support
Status: RESOLVED FIXED
Product: clutter-gst
Classification: Other
Component: general
2.0.x
Other Linux
: Normal normal
: ---
Assigned To: clutter-gst-maint
clutter-gst-maint
Depends on: 712558 712832
Blocks:
 
 
Reported: 2013-11-21 17:31 UTC by Matthieu Bouron
Modified: 2013-12-23 11:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH 1/4] video-sink: support video meta api (10.96 KB, patch)
2013-11-22 18:40 UTC, Matthieu Bouron
reviewed Details | Review
[PATCH 2/2] add context field to GstClutterSinkRenderer (3.31 KB, patch)
2013-11-22 18:40 UTC, Matthieu Bouron
none Details | Review
[PATCH 3/4] video-sink: support gl texture upload meta api (6.99 KB, patch)
2013-11-22 18:41 UTC, Matthieu Bouron
none Details | Review
[PATCH 4/4] video-sink: support video crop meta api in GLTextureUploadRenderer (5.23 KB, patch)
2013-11-22 18:42 UTC, Matthieu Bouron
none Details | Review
[PATCH 3/4] video-sink: support gl texture upload meta api (6.99 KB, patch)
2013-11-25 23:54 UTC, Matthieu Bouron
none Details | Review
[PATCH 1/4] video-sink: support video meta api (6.78 KB, patch)
2013-11-27 15:29 UTC, Matthieu Bouron
none Details | Review
[PATCH 1/4] video-sink: support video meta api (6.82 KB, patch)
2013-11-27 18:10 UTC, Matthieu Bouron
none Details | Review
[PATCH 1/4] video-sink: support video meta api (6.51 KB, patch)
2013-12-17 15:18 UTC, Matthieu Bouron
committed Details | Review
[PATCH 2/4] add context field to GstClutterSinkRenderer (3.31 KB, patch)
2013-12-17 15:18 UTC, Matthieu Bouron
committed Details | Review
[PATCH 3/4] video-sink: support gl texture upload meta api (7.15 KB, patch)
2013-12-17 15:18 UTC, Matthieu Bouron
none Details | Review
[PATCH 4/4] video-sink: support video crop meta api in GLTextureUploadRenderer (4.76 KB, patch)
2013-12-17 15:19 UTC, Matthieu Bouron
committed Details | Review
video-sink: support gl texture upload meta api (7.13 KB, patch)
2013-12-18 15:29 UTC, Matthieu Bouron
committed Details | Review

Description Matthieu Bouron 2013-11-21 17:31:31 UTC
Adding gl texture upload meta api support to clutter-sink (2.0.x release) will allow the use the vaapidecode element in conjunction with clutter-video-sink.

For now it depends on the NV12 support of clutter-video-sink (https://bugzilla.gnome.org/show_bug.cgi?id=712832) since vaapidecode always set its caps to this format.

The developpement branch can be found here:
http://cgit.collabora.com/git/user/mateo/clutter-gst.git/log/?h=wip_vaapi_texture_upload
Comment 1 Matthieu Bouron 2013-11-22 18:40:26 UTC
Created attachment 261257 [details] [review]
[PATCH 1/4] video-sink: support video meta api
Comment 2 Matthieu Bouron 2013-11-22 18:40:55 UTC
Created attachment 261258 [details] [review]
[PATCH 2/2] add context field to GstClutterSinkRenderer
Comment 3 Matthieu Bouron 2013-11-22 18:41:29 UTC
Created attachment 261259 [details] [review]
[PATCH 3/4] video-sink: support gl texture upload meta api
Comment 4 Matthieu Bouron 2013-11-22 18:42:08 UTC
Created attachment 261260 [details] [review]
[PATCH 4/4] video-sink: support video crop meta api in GLTextureUploadRenderer
Comment 5 Matthieu Bouron 2013-11-22 18:45:37 UTC
In order to get good cpu usage results the patch attached to this issue https://bugzilla.gnome.org/show_bug.cgi?id=712558 should be applied.
Comment 6 Matthieu Bouron 2013-11-25 23:54:53 UTC
Created attachment 262799 [details] [review]
[PATCH 3/4] video-sink: support gl texture upload meta api

Fix GLTextureUploadMeta caps BGRA → RGBA and make it compatible with vaapidecode.
Comment 7 Lionel Landwerlin 2013-11-27 12:41:55 UTC
Review of attachment 261257 [details] [review]:

::: clutter-gst/clutter-gst-video-sink.c
@@ +817,3 @@
+        priv->info.height,
+        CLUTTER_GST_TEXTURE_FLAGS,
+        format, format, stride, data);

You're using 'priv->info' here but you loaded the data into 'info'.

@@ +882,3 @@
+        priv->info.height,
+        CLUTTER_GST_TEXTURE_FLAGS,
+    if (!meta->map (meta, 0, &info, &data, &stride, GST_MAP_READ))

Same here 'priv->info' instead of 'info'.

@@ +949,3 @@
+          cogl_texture_new_from_data (GST_VIDEO_INFO_COMP_WIDTH (&priv->info, i),
+          GST_VIDEO_INFO_COMP_HEIGHT (&priv->info, i), CLUTTER_GST_TEXTURE_FLAGS,
+          COGL_PIXEL_FORMAT_G_8, COGL_PIXEL_FORMAT_G_8, stride,

Same here 'priv->info' instead of 'info'.

@@ +1039,3 @@
+      COGL_PIXEL_FORMAT_U_V, COGL_PIXEL_FORMAT_U_V, stride,
+      data);
+    if (!meta->map (meta, 1, &info, &data, &stride, GST_MAP_READ))

Same here 'priv->info' instead of 'info'.

@@ +1210,3 @@
+        COGL_PIXEL_FORMAT_RGBA_8888,
+        COGL_PIXEL_FORMAT_RGBA_8888, stride, data);
+

Same here 'priv->info' instead of 'info'.
Comment 8 Matthieu Bouron 2013-11-27 13:21:57 UTC
(In reply to comment #7)
> Review of attachment 261257 [details] [review]:
> 
> ::: clutter-gst/clutter-gst-video-sink.c
> @@ +817,3 @@
> +        priv->info.height,
> +        CLUTTER_GST_TEXTURE_FLAGS,
> +        format, format, stride, data);
> 
> You're using 'priv->info' here but you loaded the data into 'info'.

info is of type GstMapInfo which does carry video information, so priv->info still needs to be used here unless i've missed something.

[...]
Comment 9 Lionel Landwerlin 2013-11-27 14:01:02 UTC
Comment on attachment 261257 [details] [review]
[PATCH 1/4] video-sink: support video meta api

Maybe use meta->width/height then?
Comment 10 Matthieu Bouron 2013-11-27 15:29:22 UTC
Created attachment 262953 [details] [review]
[PATCH 1/4] video-sink: support video meta api

Also support buffers alignments using the video frame API.
Comment 11 Matthieu Bouron 2013-11-27 15:32:00 UTC
(In reply to comment #9)
> (From update of attachment 261257 [details] [review])
> Maybe use meta->width/height then?

I've update the patch and used the video frame API which takes care of buffer alignement for us if video meta api is used.
Comment 12 Matthieu Bouron 2013-11-27 18:10:48 UTC
Created attachment 262968 [details] [review]
[PATCH 1/4] video-sink: support video meta api

Fixes COMP_STRIDE|DATA vs PLANE_STRIDE|DATA usage.
Comment 13 Matthieu Bouron 2013-12-17 15:18:05 UTC
Created attachment 264411 [details] [review]
[PATCH 1/4] video-sink: support video meta api
Comment 14 Matthieu Bouron 2013-12-17 15:18:34 UTC
Created attachment 264412 [details] [review]
[PATCH 2/4] add context field to GstClutterSinkRenderer
Comment 15 Matthieu Bouron 2013-12-17 15:18:59 UTC
Created attachment 264413 [details] [review]
[PATCH 3/4] video-sink: support gl texture upload meta api
Comment 16 Matthieu Bouron 2013-12-17 15:19:23 UTC
Created attachment 264414 [details] [review]
[PATCH 4/4] video-sink: support video crop meta api in GLTextureUploadRenderer
Comment 17 Matthieu Bouron 2013-12-18 15:29:43 UTC
Created attachment 264486 [details] [review]
video-sink: support gl texture upload meta api

Fix caps ordering so that gltextureupload is prefered over the other formats
Comment 18 Lionel Landwerlin 2013-12-20 11:37:49 UTC
Review of attachment 264411 [details] [review]:

Looks good, I'll push to the 2.0 branch.
Comment 19 Lionel Landwerlin 2013-12-20 11:38:04 UTC
Review of attachment 264412 [details] [review]:

Looks good, I'll push to the 2.0 branch.
Comment 20 Lionel Landwerlin 2013-12-20 11:38:16 UTC
Review of attachment 264486 [details] [review]:

Looks good, I'll push to the 2.0 branch.
Comment 21 Lionel Landwerlin 2013-12-20 11:38:29 UTC
Review of attachment 264414 [details] [review]:

Looks good, I'll push to the 2.0 branch.