Hmm. I can do that; my instinct is to avoid mixing changes in a patch;
I would tend to do a patch just for the rename, so that the git history has a
fairly clear record.
Yeah, I did not want to burden you with an additional patch, especially as
it does not have to do with what you are trying to achieve. Feel free to
ignore that remark.
Ah, it's an easy patch, and the typos bug me as well.
But looking at the code, I think I should change it so that if the frequency
is invalid (which we'll know because codec creation will fail), we
disconnect the channel. That's an easy change.
Yup, would be nice.
Looking at the code in qemu/audio/spiceaudio.c:line_out_init() and
spice/server/snd_worker.c:snd_attach_playback() , I'm wondering if we could
delay
if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_OPUS))
red_channel_set_cap(channel, SPICE_PLAYBACK_CAP_OPUS);
until spice_server_set_playback_rate(&out->sin, settings.freq); is called.
This way we only advertise OPUS support if we have a rate we can handle.
Yeah, that's what I've been working on. The case of old qemu + new spice
server and client is a good test case, and my code didn't handle it well.
I'm hoping to have a set of patches today that better address those cases.
Why are there some caps checks in here? It's called from qemu
line_out_init() which was called once at startup (when no client are
connected) in my testing.
Checking the caps of a channel, if we have it, makes it a slightly
more useful implementation.
In the case of Xspice, for example, we can easily delay this creation
until we have a channel.
It also would, in theory, allow us to dynamically change the rate. I have
tested that usage, and it does seem to work.
Yeah, reason I mentioned this is that at the moment it's mostly unused
code, which can be surprising to someone reading the code. Maybe it's worth
a comment saying that the way qemu uses this, the client caps checks won't
be done? Though this comment is bound to bitrot :(
I'll see about adding a comment as to what the null parameter means.
This may be useful for the case of an old client connecting to a new qemu image;
the current implementation will end up with a recording frequency mismatch (44.1 vs 48),
so recording doesn't work quite right.
Ah, that sounds bad (I've only tested playback until now), and I'm afraid
we'll need to fix it sooner or later :-/
Yeah, I'll see if I can articulate the exact corner case; it's one of the clients
that had made a hard coded frequency assumption on the record channel, afair.
I'm hoping to have take 3 ready to send today.
Cheers,
Jeremy
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel