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 <[email protected]> > > > > 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 <[email protected]> > > --- > > 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
