Re: [spice opus support 3/6 (take 3)] Add support for the Opus codec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+SPICE_GNUC_VISIBLE uint32_t spice_server_get_best_record_rate(SpiceRecordInstance *sin)
+{
+    int client_can_opus = TRUE;
+    if (sin && sin->st->worker.connection)
+    {
+        SndChannel *channel = sin->st->worker.connection;
+        RecordChannel *record_channel = SPICE_CONTAINEROF(channel, RecordChannel, base);
+        client_can_opus = red_channel_client_test_remote_cap(record_channel->base.channel_client,
+                                          SPICE_RECORD_CAP_OPUS);
+    }
+
+    if (client_can_opus && snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS, SND_CODEC_ANY_FREQUENCY))

Wouldn't it be better to use sin->st->frequency when sin is non-NULL?

No.  Frequency is always set server side, never from the client.

At this point in the code, sin->st->frequency will be set to 44100 for backwards compatibility reasons, so using it would be harmful.


+    {
+        return SND_CODEC_OPUS_PLAYBACK_FREQ;
+    }

spice_server_get_best_playback_rate does not have these {}, I'd make them
consistent

Yah.

In looking at this, though, I also notice that I failed to use K&R braces in a number of places.

That's disrespectful of me; I should fix it :-/.

@@ -1464,8 +1525,10 @@ void snd_attach_playback(SpicePlaybackInstance *sin)
      red_channel_register_client_cbs(channel, &client_cbs);
      red_channel_set_data(channel, playback_worker);

-    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1))
+    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY))
          red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_CELT_0_5_1);
+    if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS, SND_CODEC_ANY_FREQUENCY))
+        red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_OPUS);

I think I would have delayed this bit until
spice_server_set_playback_rate() is called, but this works this way too.

I think you're right. As written, we'll advertise Opus support, even when run with an old qemu. I think if I shift that logic into the spice_server_set_xxx_rate functions, it will be cleaner.

The K&R fixes will require a respin of common/snd_codec.c. I may also replace a certain true/false pair with a flag while I'm at it.

I'll try to put together another (hopefully final) round today or tomorrow.

Cheers,

Jeremy
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]