[Spice-devel] [PATCH 07/14] spice-gstaudio: Use GTask instead of GSimpleAsyncResult

[Victor Toso]

Hi,

On Mon, Jan 18, 2016 at 02:51:05PM +0100, Christophe Fergeau wrote:
> On Mon, Jan 18, 2016 at 10:05:43AM +0100, Fabiano Fidêncio wrote:
> > Instead of using GSimpleAsyncResult, use the new GTask API, which is
> > much more straightforward.
> > ---
> >  src/spice-gstaudio.c | 48 ++++++++++--------------------------------------
> >  1 file changed, 10 insertions(+), 38 deletions(-)
> > 
> > diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> > index 096fea4..65fc173 100644
> > --- a/src/spice-gstaudio.c
> > +++ b/src/spice-gstaudio.c
> > @@ -570,16 +570,9 @@ static void spice_gstaudio_get_playback_volume_info_async(SpiceAudio *audio,
> >                                                            GAsyncReadyCallback callback,
> >                                                            gpointer user_data)
> >  {
> > -    GSimpleAsyncResult *simple;
> > +    GTask *task = g_task_new(audio, cancellable, callback, user_data);
> >  
> > -    simple = g_simple_async_result_new(G_OBJECT(audio),
> > -                                       callback,
> > -                                       user_data,
> > -                                       spice_gstaudio_get_playback_volume_info_async);
> > -    g_simple_async_result_set_check_cancellable (simple, cancellable);
> > -
> > -    g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> > -    g_simple_async_result_complete_in_idle(simple);
> > +    g_task_return_boolean(task, TRUE);
> >  }
> >  
> >  static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio,
> > @@ -594,14 +587,9 @@ static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
> >      gboolean lmute;
> >      gdouble vol;
> >      gboolean fake_channel = FALSE;
> > -    GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> > -
> > -    g_return_val_if_fail(g_simple_async_result_is_valid(res,
> > -        G_OBJECT(audio), spice_gstaudio_get_playback_volume_info_async), FALSE);
> > +    GTask *task = G_TASK(res);
> >  
> > -    if (g_simple_async_result_propagate_error(simple, error)) {
> > -        return FALSE;
> > -    }
> > +    g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);
> 
> 
> I would keep a
> if (g_task_had_error(task)) {
>     return g_task_propage_error(task, error);
> }
> 
> to avoid trying to do a lot of things when there had been an error.
> 
> >      if (p->playback.sink == NULL || p->playback.channels == 0) {
> >          SPICE_DEBUG("PlaybackChannel not created yet, force start");
> > @@ -647,7 +635,7 @@ static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
> >          }
> >      }
> >  
> > -    return g_simple_async_result_get_op_res_gboolean(simple);
> > +    return g_task_propagate_boolean(task, error);
> >  }
> >  
> >  static void spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> > @@ -656,16 +644,9 @@ static void spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> >                                                          GAsyncReadyCallback callback,
> >                                                          gpointer user_data)
> >  {
> > -    GSimpleAsyncResult *simple;
> > -
> > -    simple = g_simple_async_result_new(G_OBJECT(audio),
> > -                                       callback,
> > -                                       user_data,
> > -                                       spice_gstaudio_get_record_volume_info_async);
> > -    g_simple_async_result_set_check_cancellable (simple, cancellable);
> > +    GTask *task = g_task_new(audio, cancellable, callback, user_data);
> >  
> > -    g_simple_async_result_set_op_res_gboolean(simple, TRUE);
> > -    g_simple_async_result_complete_in_idle(simple);
> > +    g_task_return_boolean(task, TRUE);
> >  }
> >  
> >  static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
> > @@ -680,18 +661,9 @@ static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
> >      gboolean lmute;
> >      gdouble vol;
> >      gboolean fake_channel = FALSE;
> > -    GSimpleAsyncResult *simple = (GSimpleAsyncResult *) res;
> > +    GTask *task = G_TASK(res);
> >
> > -    g_return_val_if_fail(g_simple_async_result_is_valid(res,
> > -        G_OBJECT(audio), spice_gstaudio_get_record_volume_info_async), FALSE);
> > -
> > -    if (g_simple_async_result_propagate_error(simple, error)) {
> > -        /* set out args that should have new alloc'ed memory to NULL */
> > -        if (volume != NULL) {
> > -            *volume = NULL;
> > -        }
>
> I would keep this too, I'm a bit surprised it's only done in the record
> case and not in the playback case.

Indeed, seems that it was forgotten in the playback case.
Both calls from channel-main don't mess with the output arg if the
function return FALSE but it is recommended to set to NULL.

>
> Christophe

> 
> > -        return FALSE;
> > -    }
> > +    g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);
> >  
> >      if (p->record.src == NULL || p->record.channels == 0) {
> >          SPICE_DEBUG("RecordChannel not created yet, force start");
> > @@ -737,5 +709,5 @@ static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
> >          }
> >      }
> >  
> > -    return g_simple_async_result_get_op_res_gboolean(simple);
> > +    return g_task_propagate_boolean(task, error);
> >  }
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel



> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel