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 768761 - wayland: Error check before using cached wl_display
wayland: Error check before using cached wl_display
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-13 09:11 UTC by Jagyum Koo
Modified: 2016-10-31 14:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: Error check before using cached wl_display (1.41 KB, patch)
2016-07-13 09:11 UTC, Jagyum Koo
none Details | Review
wayland: Error check before using cached wl_display (1.71 KB, patch)
2016-07-14 00:40 UTC, Jagyum Koo
committed Details | Review

Description Jagyum Koo 2016-07-13 09:11:03 UTC
Created attachment 331387 [details] [review]
wayland: Error check before using cached wl_display

Hello, I'm a newbie at this place.

I'm currently working for IVI system with Intel chipset. Recently I
dealt with an issue which 'sometimes' failed to create a planar
buffer, then wayland was disconnected. I put an error detection code
then it works well.
Comment 1 Víctor Manuel Jáquez Leal 2016-07-13 10:22:23 UTC
Review of attachment 331387 [details] [review]:

Overall it looks good, just a couple comments about code-style.

::: gst-libs/gst/vaapi/gstvaapidisplay_wayland.c
@@ +216,3 @@
       priv->display_name, g_display_types);
   if (info) {
+    wl_display_roundtrip(info->native_display);

GStreamer code-style keeps a space before parentheses:
wl_display_roundtrip (info->native_display);

@@ +217,3 @@
   if (info) {
+    wl_display_roundtrip(info->native_display);
+    if (dsp_error = wl_display_get_error(info->native_display))

ditto.

@@ +218,3 @@
+    wl_display_roundtrip(info->native_display);
+    if (dsp_error = wl_display_get_error(info->native_display))
+      GST_ERROR("wayland display error detected: %d\n", dsp_error);

There is no need of \n in log messages such as GST_ERROR
Comment 2 Víctor Manuel Jáquez Leal 2016-07-13 10:23:08 UTC
thanks for the patch! :D
Comment 3 Víctor Manuel Jáquez Leal 2016-07-13 10:27:08 UTC
Ah, another comment: I'd be great if you add in the commit description, what you said in the bug description, explaining why this patch is needed.
Comment 4 Jagyum Koo 2016-07-14 00:40:33 UTC
Created attachment 331452 [details] [review]
wayland: Error check before using cached wl_display

Thanks for the review, Victor.
Please take a look.
Comment 5 Víctor Manuel Jáquez Leal 2016-07-14 09:35:16 UTC
Review of attachment 331452 [details] [review]:

it looks good to me
Comment 6 Víctor Manuel Jáquez Leal 2016-07-14 10:29:40 UTC
Pushed slightly modified to avoid compiler's complain.

Attachment 331452 [details] pushed as 0d1d097 - wayland: Error check before using cached wl_display