[spice-gtk,v1,6/6] RFC: test: session: gstreamer init by us or client

Submitted by Victor Toso on Sept. 2, 2019, 4:04 p.m.

Details

Message ID 20190902160449.19589-7-victortoso@redhat.com
State New
Headers show
Series "Initialize GStreamer on SpiceSession" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso Sept. 2, 2019, 4:04 p.m.
From: Victor Toso <me@victortoso.com>

Does not work properly just because gst_is_initialized() checks if
gst_init() or gst_check_init() was ever called but it does not
consider if gst_deinit() was called too. I'm sending this RFC to check
if should be added or wait till feedback on upstream about the API,
that is, I have a downstream patch that gst_is_initialized() would
return FALSE if gst_deinit() was called ...

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 tests/session.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Patch hide | download patch | download mbox

diff --git a/tests/session.c b/tests/session.c
index 8208016..8e1814e 100644
--- a/tests/session.c
+++ b/tests/session.c
@@ -1,3 +1,6 @@ 
+#include <stdbool.h>
+#include <gst/gst.h>
+
 #include <spice-client.h>
 
 typedef struct {
@@ -333,6 +336,50 @@  static void test_session_uri_unix_good(void)
     test_session_uri_good(tests, G_N_ELEMENTS(tests));
 }
 
+static void session_init_test_on_gst(bool should_init)
+{
+
+    if (should_init) {
+        GError *err = NULL;
+        gst_init_check(NULL, NULL, &err);
+        g_assert_no_error(err);
+        g_assert_true(gst_is_initialized());
+    } else if (gst_is_initialized()) {
+#if 0
+        /* Not working for now, gst_is_initialized() return TRUE also if
+         * gst_deinit() was called */
+        gst_deinit();
+        g_assert_false(gst_is_initialized());
+#endif
+    }
+
+    SpiceSession *session = spice_session_new();
+    g_assert_true(gst_is_initialized());
+    g_object_unref(session);
+    if (should_init) {
+        g_assert_true(gst_is_initialized());
+        gst_deinit();
+#if 0
+        g_assert_false(gst_is_initialized());
+#endif
+    } else {
+#if 0
+        /* Even if gst_deinit() is called, returns TRUE below */
+        g_assert_false(gst_is_initialized());
+#endif
+    }
+}
+
+static void test_session_gst_init_by_us(void)
+{
+    session_init_test_on_gst(true);
+}
+
+static void test_session_gst_init_by_client(void)
+{
+    session_init_test_on_gst(false);
+}
+
 int main(int argc, char* argv[])
 {
     g_test_init(&argc, &argv, NULL);
@@ -341,6 +388,8 @@  int main(int argc, char* argv[])
     g_test_add_func("/session/good-ipv4-uri", test_session_uri_ipv4_good);
     g_test_add_func("/session/good-ipv6-uri", test_session_uri_ipv6_good);
     g_test_add_func("/session/good-unix", test_session_uri_unix_good);
+    g_test_add_func("/session/gstreamer/init-by-us", test_session_gst_init_by_us);
+    g_test_add_func("/session/gstreamer/init-by-client", test_session_gst_init_by_client);
 
     return g_test_run();
 }

Comments

HI,

On 9/2/19 7:04 PM, Victor Toso wrote:
> From: Victor Toso <me@victortoso.com>
>
> Does not work properly just because gst_is_initialized() checks if
> gst_init() or gst_check_init() was ever called but it does not
> consider if gst_deinit() was called too. I'm sending this RFC to check
> if should be added or wait till feedback on upstream about the API,
> that is, I have a downstream patch that gst_is_initialized() would
> return FALSE if gst_deinit() was called ...


Maybe should also be mentioned as comment in the code


>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>   tests/session.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 49 insertions(+)
>
> diff --git a/tests/session.c b/tests/session.c
> index 8208016..8e1814e 100644
> --- a/tests/session.c
> +++ b/tests/session.c
> @@ -1,3 +1,6 @@
> +#include <stdbool.h>
> +#include <gst/gst.h>
> +
>   #include <spice-client.h>
>   
>   typedef struct {
> @@ -333,6 +336,50 @@ static void test_session_uri_unix_good(void)
>       test_session_uri_good(tests, G_N_ELEMENTS(tests));
>   }
>   
> +static void session_init_test_on_gst(bool should_init)
> +{
> +
> +    if (should_init) {
> +        GError *err = NULL;
> +        gst_init_check(NULL, NULL, &err);
> +        g_assert_no_error(err);
> +        g_assert_true(gst_is_initialized());
> +    } else if (gst_is_initialized()) {


Is this likely to happen?


> +#if 0
> +        /* Not working for now, gst_is_initialized() return TRUE also if
> +         * gst_deinit() was called */
> +        gst_deinit();

also documentation of gst_deinit mention:

"After this call GStreamer (including this method) should not be used 
anymore."


> +        g_assert_false(gst_is_initialized());
> +#endif
> +    }
> +
> +    SpiceSession *session = spice_session_new();
> +    g_assert_true(gst_is_initialized());
> +    g_object_unref(session);
> +    if (should_init) {
> +        g_assert_true(gst_is_initialized());
> +        gst_deinit();
> +#if 0
> +        g_assert_false(gst_is_initialized());
> +#endif
> +    } else {
> +#if 0
> +        /* Even if gst_deinit() is called, returns TRUE below */
> +        g_assert_false(gst_is_initialized());
> +#endif
> +    }
> +}
> +
> +static void test_session_gst_init_by_us(void)
> +{
> +    session_init_test_on_gst(true);
> +}
> +
> +static void test_session_gst_init_by_client(void)
> +{
> +    session_init_test_on_gst(false);
> +}
> +

I find it a bit confusing, what about init_by_session and init_by_test
(or maybe init_externally and internally)

Oh, and the rest of the patches i didn't comment on looks good to me :)

Snir.


>   int main(int argc, char* argv[])
>   {
>       g_test_init(&argc, &argv, NULL);
> @@ -341,6 +388,8 @@ int main(int argc, char* argv[])
>       g_test_add_func("/session/good-ipv4-uri", test_session_uri_ipv4_good);
>       g_test_add_func("/session/good-ipv6-uri", test_session_uri_ipv6_good);
>       g_test_add_func("/session/good-unix", test_session_uri_unix_good);
> +    g_test_add_func("/session/gstreamer/init-by-us", test_session_gst_init_by_us);
> +    g_test_add_func("/session/gstreamer/init-by-client", test_session_gst_init_by_client);
>   
>       return g_test_run();
>   }