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

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

Details

Message ID 20190902160449.19589-3-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>

This means we can drop gstvideo_init() function and replace its calls
with gst_is_initialized().

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 src/channel-display-gst.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

Patch hide | download patch | download mbox

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()) {
         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 |

Comments

Hi,


On 9/2/19 7:04 PM, Victor Toso wrote:
> From: Victor Toso <me@victortoso.com>
>
> This means we can drop gstvideo_init() function and replace its calls
> with gst_is_initialized().
>
> Signed-off-by: Victor Toso <victortoso@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 i would add a comment it's now initialized
by the session (if not externally)

(also on gstaudio)

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 |

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@victortoso.com>
>>>
>>> This means we can drop gstvideo_init() function and replace its calls
>>> with gst_is_initialized().
>>>
>>> Signed-off-by: Victor Toso <victortoso@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel