[Spice-devel] [spice-gtk v1 2/6] display-gst: rely on SpiceSession init of GStreamer

[Victor Toso]

Hi,

On Sun, Sep 08, 2019 at 04:25:32PM +0300, Snir Sheriber wrote:
> Hi,
> 
> 
> On 9/2/19 7:04 PM, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> > 
> > This means we can drop gstvideo_init() function and replace its calls
> > with gst_is_initialized().
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >   src/channel-display-gst.c | 23 +++++------------------
> >   1 file changed, 5 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 6fccf62..a34b5d0 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -748,22 +748,6 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
> >       return TRUE;
> >   }
> > -static gboolean gstvideo_init(void)
> > -{
> > -    static int success = 0;
> > -    if (!success) {
> > -        GError *err = NULL;
> > -        if (gst_init_check(NULL, NULL, &err)) {
> > -            success = 1;
> > -        } else {
> > -            spice_warning("Disabling GStreamer video support: %s", err->message);
> > -            g_clear_error(&err);
> > -            success = -1;
> > -        }
> > -    }
> > -    return success > 0;
> > -}
> > -
> >   G_GNUC_INTERNAL
> >   VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream)
> >   {
> > @@ -771,7 +755,7 @@ VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream)
> >       g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), NULL);
> > -    if (gstvideo_init()) {
> > +    if (gst_is_initialized()) {
> 
> Since the logical thing to do is usually to initialize
> gstreamer once you start using it

Why? GStreamer is a library and I'd say the init is to be called
early on because that would deal with command line options,
environment variables etc. No extra delay on initializing this
kind of thing when we want to create a pipeline for video/audio
processing for instance is also valid argument.

> i would add a comment it's now initialized by the session (if
> not externally)
> 
> (also on gstaudio)

Ok, no problem. Thanks!

> 
> Snir.
> 
> >           decoder = g_new0(SpiceGstDecoder, 1);
> >           decoder->base.destroy = spice_gst_decoder_destroy;
> >           decoder->base.reschedule = spice_gst_decoder_reschedule;
> > @@ -820,7 +804,10 @@ gboolean gstvideo_has_codec(int codec_type)
> >       GstCaps *caps;
> >       GstElementFactoryListType type;
> > -    g_return_val_if_fail(gstvideo_init(), FALSE);
> > +    if (!gst_is_initialized()) {
> > +        return FALSE;
> > +    }
> > +
> >       g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> >       type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> _______________________________________________
> 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/15fe6bfc/attachment.sig>