[Spice-devel] [spice-gtk sound rework 3/3 (take 4)] Use the new snd_codec interface to process encoded audio.

[Christophe Fergeau]

And a few more comments to that one as well :(

On Tue, Nov 05, 2013 at 04:25:23PM -0600, Jeremy White wrote:
> diff --git a/gtk/channel-playback.c b/gtk/channel-playback.c
> @@ -428,7 +410,6 @@ static void playback_handle_start(SpiceChannel *channel, SpiceMsgIn *in)
>  {
>      SpicePlaybackChannelPrivate *c = SPICE_PLAYBACK_CHANNEL(channel)->priv;
>      SpiceMsgPlaybackStart *start = spice_msg_in_parsed(in);
> -    int celt_mode_err;
>  
>      CHANNEL_DEBUG(channel, "%s: fmt %d channels %d freq %d time %d", __FUNCTION__,
>                    start->format, start->channels, start->frequency, start->time);
> @@ -437,6 +418,7 @@ static void playback_handle_start(SpiceChannel *channel, SpiceMsgIn *in)
>      c->last_time = start->time;
>      c->is_active = TRUE;
>      c->min_latency = SPICE_PLAYBACK_DEFAULT_LATENCY_MS;
> +    c->codec = NULL;
>  
>      switch (c->mode) {
>      case SPICE_AUDIO_DATA_MODE_RAW:
> @@ -444,19 +426,8 @@ static void playback_handle_start(SpiceChannel *channel, SpiceMsgIn *in)
>                            start->format, start->channels, start->frequency);
>          break;
>      case SPICE_AUDIO_DATA_MODE_CELT_0_5_1: {
> -        /* TODO: only support one setting now */
> -        int frame_size = 256;
> -        if (!c->celt_mode)
> -            c->celt_mode = celt051_mode_create(start->frequency, start->channels,
> -                                               frame_size, &celt_mode_err);
> -        if (!c->celt_mode)
> -            g_warning("create celt mode failed %d", celt_mode_err);
> -
> -        if (!c->celt_decoder)
> -            c->celt_decoder = celt051_decoder_create(c->celt_mode);
> -
> -        if (!c->celt_decoder)
> -            g_warning("create celt decoder failed");
> +        if (snd_codec_create(&c->codec, c->mode, start->frequency, FALSE, TRUE) != SND_CODEC_OK)
> +            g_warning("create decoder failed");
>  
>          emit_main_context(channel, SPICE_PLAYBACK_START,
>                            start->format, start->channels, start->frequency);

The issue is preexisting, but I don't think it's correct to emit
SPICE_PLAYBACK_START if we failed to create the decoder, so the code should
probably be:
if (snd_codec_create(&c->codec, c->mode, start->frequency, FALSE, TRUE) != SND_CODEC_OK) {
    g_warning("create decoder failed");
    return
}

> diff --git a/gtk/channel-record.c b/gtk/channel-record.c
> @@ -446,42 +428,25 @@ static void record_handle_start(SpiceChannel *channel, SpiceMsgIn *in)
>      CHANNEL_DEBUG(channel, "%s: fmt %d channels %d freq %d", __FUNCTION__,
>                    start->format, start->channels, start->frequency);
>  
> -    c->frame_bytes = FRAME_SIZE * 16 * start->channels / 8;
> +    g_return_if_fail(start->format == SPICE_AUDIO_FMT_S16);
>  
> -    g_free(c->last_frame);
> -    c->last_frame = g_malloc(c->frame_bytes);
> -    c->last_frame_current = 0;
> +    c->codec = NULL;
> +    c->frame_bytes = SND_CODEC_MAX_FRAME_SIZE * 16 * start->channels / 8;
>  
> -    switch (c->mode) {
> -    case SPICE_AUDIO_DATA_MODE_RAW:
> -        emit_main_context(channel, SPICE_RECORD_START,
> -                          start->format, start->channels, start->frequency);
> -        break;
> -    case SPICE_AUDIO_DATA_MODE_CELT_0_5_1: {
> -        int celt_mode_err;
> -
> -        g_return_if_fail(start->format == SPICE_AUDIO_FMT_S16);
> -
> -        if (!c->celt_mode)
> -            c->celt_mode = celt051_mode_create(start->frequency, start->channels, FRAME_SIZE,
> -                                               &celt_mode_err);
> -        if (!c->celt_mode)
> -            g_warning("Failed to create celt mode");
> +    if (c->mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1)
> +    {
> +        c->frame_bytes = SND_CODEC_CELT_FRAME_SIZE * 16 * start->channels / 8;
>  
> -        if (!c->celt_encoder)
> -            c->celt_encoder = celt051_encoder_create(c->celt_mode);
> +        if (snd_codec_create(&c->codec, c->mode, start->frequency, TRUE, FALSE) != SND_CODEC_OK)
> +            g_warning("Failed to create encoder");
> +    }
>  
> -        if (!c->celt_encoder)
> -            g_warning("Failed to create celt encoder");
> +    g_free(c->last_frame);
> +    c->last_frame = g_malloc(c->frame_bytes);
> +    c->last_frame_current = 0;
>  
> -        emit_main_context(channel, SPICE_RECORD_START,
> -                          start->format, start->channels, start->frequency);
> -        break;
> -    }
> -    default:
> -        g_warning("%s: unhandled mode %d", __FUNCTION__, c->mode);
> -        break;
> -    }
> +    emit_main_context(channel, SPICE_RECORD_START,
> +                      start->format, start->channels, start->frequency);

Same comment applies here.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20131108/1824bfdb/attachment-0001.pgp>