[Spice-devel] [codec 2/spice] Revise the spice server to use the new snd_codec functions in spice-common.

[Jeremy White]

>> I'm willing to be persuaded otherwise.
> 
> As said on IRC, I'm not looking for something fully dynamic, but if
> more of the number of channel/sample size could be hidden in
> SndCodec (while still being hardcoded), this would look nicer imo.
> But I agree it only makes sense if it's straightforward to change
> this.

k.

> 
> 
>> 
>>>> +AM_CONDITIONAL([HAVE_CELT051], [test "x$have_celt051" =
>>>> "xyes"]) +AM_COND_IF([HAVE_CELT051],
>>>> AC_DEFINE([HAVE_CELT051], 1, [Define if
>> we have celt051 codec]))
>>> 
>>> Is it needed at all now that you moved the celt logic to
>>> spice-common?
>> 
>> It is needed in the current code (the size of SndCodec depends on
>> the config.h matching at each level).  But perhaps that suggests
>> that SndCodec should be allocated and freed by spice-common so
>> it's entirely opaque.
> 
> Ugh, yeah, needing to have config.h to match everywhere seems very
> fragile, I'd favour dynamic allocation of SndCodec (iirc I
> refrained from saying something about this during the initial
> review)

Yeah, I theoretically fixed that in the last patch set.

>> 
>> While we're on the subject, though - is there any good way to
>> test this code path?
> 
> Hmm, I suspect this is dead code. If you look at the old 
> Spice_user_manual.odt from the RHEL5 era or so (spice-space.org
> should have it but is down for me at the moment), in 4.2 Qemu
> monitor commands, you'll see there used to be a command to
> dynamically change playback compression at runtime. I guess this is
> when this codepath run. These days, I think the command is gone.

The entry is currently used by the Xspice code, but just for the 'turn
it off' case.  But I now think that use is wrong and have a future
patch to remove that.

Perhaps I'll flag the entry point as deprecated, and leave the perhaps
working code in place?

> 
> Christophe

tl;dr:  My latest patch of 3 is still a viable proposal, and worthy of
review; given KVM Forum, it'll likely be a while on that.

Cheers,

Jeremy