[Spice-devel] [PATCH 2/2] audio: Avoid bad pipelines from gst_parse_launch

[Fabiano Fidêncio]

On Wed, Nov 26, 2014 at 3:30 PM, Victor Toso <victortoso at redhat.com> wrote:
> gst_parse_launch may return not NULL even when error is set.
> This can lead to data loss when playing or recording audio.
> ---
>  gtk/spice-gstaudio.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/gtk/spice-gstaudio.c b/gtk/spice-gstaudio.c
> index 5f9abb2..1b82f24 100644
> --- a/gtk/spice-gstaudio.c
> +++ b/gtk/spice-gstaudio.c
> @@ -245,9 +245,9 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
>                              "appsink caps=\"%s\" name=appsink", audio_caps);
>
>          p->record.pipe = gst_parse_launch(pipeline, &error);
> -        if (p->record.pipe == NULL) {
> +        if (error != NULL) {
>              g_warning("Failed to create pipeline: %s", error->message);
> -            goto lerr;
> +            goto cleanup;
>          }
>
>          bus = gst_pipeline_get_bus(GST_PIPELINE(p->record.pipe));
> @@ -268,7 +268,11 @@ static void record_start(SpiceRecordChannel *channel, gint format, gint channels
>                                        G_CALLBACK(record_new_buffer), gstaudio, 0);
>  #endif
>
> -lerr:
> +cleanup:
> +        if (error != NULL && p->record.pipe != NULL) {
> +            gst_object_unref(p->record.pipe);
> +            p->record.pipe = NULL;
> +        }
>          g_clear_error(&error);
>          g_free(audio_caps);
>          g_free(pipeline);
> @@ -345,16 +349,20 @@ static void playback_start(SpicePlaybackChannel *channel, gint format, gint chan
>                                         "audioconvert ! audioresample ! autoaudiosink name=\"audiosink\"", audio_caps);
>          SPICE_DEBUG("audio pipeline: %s", pipeline);
>          p->playback.pipe = gst_parse_launch(pipeline, &error);
> -        if (p->playback.pipe == NULL) {
> +        if (error != NULL) {
>              g_warning("Failed to create pipeline: %s", error->message);
> -            goto lerr;
> +            goto cleanup;
>          }
>          p->playback.src = gst_bin_get_by_name(GST_BIN(p->playback.pipe), "appsrc");
>          p->playback.sink = gst_bin_get_by_name(GST_BIN(p->playback.pipe), "audiosink");
>          p->playback.rate = frequency;
>          p->playback.channels = channels;
>
> -lerr:
> +cleanup:
> +        if (error != NULL && p->playback.pipe != NULL) {
> +            gst_object_unref(p->playback.pipe);
> +            p->playback.pipe = NULL;
> +        }
>          g_clear_error(&error);
>          g_free(audio_caps);
>          g_free(pipeline);
> --
> 2.1.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

ACK both patches!

-- 
Fabiano Fidêncio