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

[Victor Toso]

Hi,

On Sun, Sep 08, 2019 at 04:40:46PM +0300, Snir Sheriber wrote:
> HI,
> 
> On 9/2/19 7:04 PM, Victor Toso wrote:
> > From: Victor Toso <me at 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 at 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."

Well, I was wondering if for testing purposes that could be fine.

> > +        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)

Sure, I'll always take advice in renaming functions :)

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

Thanks!

> 
> 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();
> >   }
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190909/340ba3bd/attachment-0001.sig>