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

[Snir Sheriber]

Hi,

On 9/9/19 12:01 PM, Victor Toso wrote:
> 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.


Yes, i get it , just mentioned it as the reason for adding
the comment so that if someone will look at gstreamer's
code he will know he wouldn't find the initialization there
but in the session code :)

Snir.


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