st/omx: straighten get/put_screen
authorEmil Velikov <emil.l.velikov@gmail.com>
Mon, 9 Nov 2015 11:03:01 +0000 (11:03 +0000)
committerEmil Velikov <emil.l.velikov@gmail.com>
Fri, 20 Nov 2015 10:56:10 +0000 (10:56 +0000)
The current code is busted in a number of ways.

 - initially checks for omx_display (rather than omx_screen), which may
or may not be around.
 - blindly feeds the empty env variable string to loader_open_device()
 - reads the env variable every time get_screen is called
 - the latter manifests into memory leaks, and other issues as one sets
the variable between two get_screen calls.

Additionally it cleans up a couple of extra bits
 - drops unneeded set/check of omx_display.
 - make the teardown (put_screen) order was not symmetrical to the setup
(get_screen)

v2: Drop the "is empty string" check (Leo)

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Leo Liu <leo.liu@amd.com>
src/gallium/state_trackers/omx/entrypoint.c

index 7df90b16a84e60c85894f9c32743fcc2f59e0e92..4716333015b2dc9b5c3856c31ba72e34744b99ed 100644 (file)
@@ -33,6 +33,7 @@
 
 #include <assert.h>
 #include <string.h>
+#include <stdbool.h>
 
 #include <X11/Xlib.h>
 
@@ -73,28 +74,29 @@ int omx_component_library_Setup(stLoaderComponentType **stComponents)
 
 struct vl_screen *omx_get_screen(void)
 {
+   static bool first_time = true;
    pipe_mutex_lock(omx_lock);
 
-   if (!omx_display) {
-      omx_render_node = debug_get_option("OMX_RENDER_NODE", NULL);
-      if (!omx_render_node) {
-         omx_display = XOpenDisplay(NULL);
-         if (!omx_display)
-            goto error;
-      }
-   }
-
    if (!omx_screen) {
+      if (first_time) {
+         omx_render_node = debug_get_option("OMX_RENDER_NODE", NULL);
+         first_time = false;
+      }
       if (omx_render_node) {
          drm_fd = loader_open_device(omx_render_node);
          if (drm_fd < 0)
             goto error;
+
          omx_screen = vl_drm_screen_create(drm_fd);
          if (!omx_screen) {
             close(drm_fd);
             goto error;
          }
       } else {
+         omx_display = XOpenDisplay(NULL);
+         if (!omx_display)
+            goto error;
+
          omx_screen = vl_screen_create(omx_display, 0);
          if (!omx_screen) {
             XCloseDisplay(omx_display);
@@ -117,16 +119,14 @@ void omx_put_screen(void)
 {
    pipe_mutex_lock(omx_lock);
    if ((--omx_usecount) == 0) {
-      if (!omx_render_node) {
-         vl_screen_destroy(omx_screen);
-         if (omx_display)
-            XCloseDisplay(omx_display);
-      } else {
-         close(drm_fd);
+      if (omx_render_node) {
          vl_drm_screen_destroy(omx_screen);
+         close(drm_fd);
+      } else {
+         vl_screen_destroy(omx_screen);
+         XCloseDisplay(omx_display);
       }
       omx_screen = NULL;
-      omx_display = NULL;
    }
    pipe_mutex_unlock(omx_lock);
 }