Re: [Spice-devel] [PATCH] make celt to be optional
On Thu, Jun 14, 2012 at 08:55:59AM +0200, Gerd Hoffmann wrote: Hi, I suspect there is no way around resampling support though. We'll need it for compatibility reasons, so sound keeps working if only one side is able to operate at 48 kHz. The ability to resample OTOH probably isn't a silly thing to have, and can be fairly easily set up as an inline filter that is switched out if you really don't need it. Just to be clear about this though, there is actually no reason that both sides of the link need to use the *same* sample rate, even without a resampler. For instance you can perfectly well push an 8kHz stream into opus, and decode that as a 48kHz stream at the other end, or vice versa, without resampling, for any integer fraction of 48k down to 8. Or just because one end needs to resample from 44.1 to 48, it doesn't prevent the other side from still natively using 48k etc. Ah, ok, so opus is different from mp3 here. So you could reample using opus? i.e. have one side feed 44.1 into opus encoder, then send opus over the wire, have the other side decode 48 kHz from opus? From 44.1 you'd still need to externally resample to one of 8, 12, 16, 24, or 48k - but you can put any of those rates in, and take any of those out again without them needing to be the same. So only endpoints actually needing an 'oddball' rate would need a resampler to get them, they wouldn't force the other end to also support and use that rate as well. Is that true for celt 0.5.1 too btw? Unfortunately no. For celt you compose your own 'mode' and that mode essentially defines the bitstream. Different modes make different bitstreams so you need to decode with the same mode you encoded with. For opus, the emphasis on everything should be interoperable came to the fore, so from the user visible side there is only one mode, that supports multiple rates/frame sizes/channel counts. Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Ron píše v Pá 15. 06. 2012 v 16:29 +0930: On Thu, Jun 14, 2012 at 01:58:55PM +0200, David Jaša wrote: Christophe Fergeau píše v Čt 14. 06. 2012 v 12:31 +0200: On Thu, Jun 14, 2012 at 10:14:36AM +0200, Alexander Larsson wrote: However, that is imho a different issue than the celt051 support. A new release of spice client and server supporting opus does not magically make old servers and client disappear, so it would still be the case that e.g. debian spice client would get lame audio performance if connecting to say a RHEV spice client, or if some old client connects to a server running on debian. In time, it would perhaps make sense to drop celt051 support, but its seems pretty bad to release a client binary that doesn't do audio well with all currently existing deployed servers. It all depends if we consider remote SPICE access with limited bandwidth and with audio needed will be an important use case that must run as good as possible. In my opinion, sound is most of the time not the most important thing if what you want is a remote desktop. It also won't be really noticeable on LAN, or in GNOME Boxes use case, ... What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? I don't know if no audio at all is more acceptable than not doing audio well in some cases. There was an interest in lossless audio for some medical application some time ago so support for explicit codec choice (and incoproration of FLAC or some better lossless codec, if any) would be a big improvement for some spice users. I guess this kind of depends on what they mean by lossless. At 64kb/s, in listening tests of opus, there were a notable number of samples where the listeners could not pick opus from the reference. At 128kb/s, even codec developers and practiced listeners have trouble hearing any difference from the reference samples with high quality headphones, on all but the most pathologically difficult examples to code. And we expect that will improve even further in the months to come. The decoder and bitstream are frozen, but there is still potential for a compatible encoder to make notable improvements to quality. So if what they want is 'transparency', then maybe opus can do this for them all by itself. Celt is already good enough in my experience in this regard and I don't expect Opus to be any worse. That's not the same as true lossless if they need to analyse the samples received - but that would be something I'd expect to have an application specific data stream all of its own if that was needed, rather than it using the generic audio support of spice. IIRC this was the case, they wanted to use it to feed outputs from some tools to a software run in the guest and they liked that the spice audio support was ready to use for them, apart from not allowing codec choice. David Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- David Jaša, RHCE SPICE QE based in Brno GPG Key: 22C33E24 Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi, From 44.1 you'd still need to externally resample to one of 8, 12, 16, 24, or 48k - but you can put any of those rates in, and take any of those out again without them needing to be the same. So only endpoints actually needing an 'oddball' rate would need a resampler to get them, they wouldn't force the other end to also support and use that rate as well. Ok, so we must resample 44.1 anyway before encoding. Is that true for celt 0.5.1 too btw? Unfortunately no. For celt you compose your own 'mode' and that mode essentially defines the bitstream. Different modes make different bitstreams so you need to decode with the same mode you encoded with. Ok, so it's easiest to simply not touch celt. So, how to go forward? spice-server gets a 44.1 stream from qemu today. I think we should switch that to 48k. Needs changes in the sound interface and in qemu. spice-server needs to be able to handle both old (44.1) and new (48k) qemu. spice-server also needs to be able to handle clients with different capabilities (possibly even at the same time if multiclient is enabled, not sure multiclient support covers the sound channel though). So we'll need resampling support for both 44.1 - 48k and 48k - 44.1. In the end we'll have the following cases, at the server side: old qemu, no caps [- 44.1 pcm] pass through pcm audio data as-is new qemu, no caps [- 44.1 pcm] resample 48k - 44.1 old qemu, cap_celt encode 44.1 celt new qemu, cap_celt resample 48k - 44.1, encode 44.1 celt old qemu, cap_opus resample 44.1 - 48k, encode opus new qemu, cap_opus encode opus We might want to add: new qemu, cap_pcm48k pass through pcm audio data as-is Long-term (new spice-server, new qemu, clients supporting opus) we'll operate at 48k on the whole chain without resampling needed in the spice-server. Client side is easy, we just decode stuff and pass it on to pulseaudio to deal with it. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Fri, Jun 15, 2012 at 11:17:34AM +0200, Gerd Hoffmann wrote: Hi, From 44.1 you'd still need to externally resample to one of 8, 12, 16, 24, or 48k - but you can put any of those rates in, and take any of those out again without them needing to be the same. So only endpoints actually needing an 'oddball' rate would need a resampler to get them, they wouldn't force the other end to also support and use that rate as well. Ok, so we must resample 44.1 anyway before encoding. Is that true for celt 0.5.1 too btw? Unfortunately no. For celt you compose your own 'mode' and that mode essentially defines the bitstream. Different modes make different bitstreams so you need to decode with the same mode you encoded with. Ok, so it's easiest to simply not touch celt. So, how to go forward? spice-server gets a 44.1 stream from qemu today. I think we should switch that to 48k. Needs changes in the sound interface and in qemu. spice-server needs to be able to handle both old (44.1) and new (48k) qemu. spice-server also needs to be able to handle clients with different capabilities (possibly even at the same time if multiclient is enabled, not sure multiclient support covers the sound channel though). No sound channel for multiclient. So we'll need resampling support for both 44.1 - 48k and 48k - 44.1. In the end we'll have the following cases, at the server side: old qemu, no caps [- 44.1 pcm] pass through pcm audio data as-is new qemu, no caps [- 44.1 pcm] resample 48k - 44.1 old qemu, cap_celt encode 44.1 celt new qemu, cap_celt resample 48k - 44.1, encode 44.1 celt old qemu, cap_opus resample 44.1 - 48k, encode opus new qemu, cap_opus encode opus We might want to add: new qemu, cap_pcm48k pass through pcm audio data as-is Long-term (new spice-server, new qemu, clients supporting opus) we'll operate at 48k on the whole chain without resampling needed in the spice-server. Client side is easy, we just decode stuff and pass it on to pulseaudio to deal with it. Sounds like a good plan. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Thu, Jun 14, 2012 at 06:49:28PM +0400, Michael Tokarev wrote: And I'm about to give up, too, just disabling spice if spice guys don't want it to be shipped in popular distributions. Well, at worst you have the option of shipping spice with a distro patch even if upstream disagrees with it while proper Opus support gets implemented. Christophe pgpgw64XvRi9p.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, 2012-06-12 at 16:33 +0200, Christophe Fergeau wrote: On Tue, Jun 12, 2012 at 09:59:39AM -0400, Marc-André Lureau wrote: - Mensaje original - On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. As I understand it, while the bitstream is not officially frozen yet, it's very unlikely to change before the real freeze (unless big last minute issues are fine), so an Opus mode (marked as no compat guarantees, use at your own risk, ...) will probably not cause significant compatibilities issues. That's just guesses. It's not about library API, but about bitstream. There is no guarantee. A guess supported by the slides at http://www.opus-codec.org/presentations/ , by various mailing posts from opus developers, ... Sticking to celt051 is still a safer alternative. Not suggesting dropping celt051 support upstream... Otherwise, how would you identify almost-frozen bitstream to frozen bitstream? You would have to identify by library version (erk!) and be compatible with the old and new bitstram (which might be complicated depending on library design), or be incompatible with the intermediate version, situation which we better avoid! We would make no guarantee with interoperability between binaries using different opus versions until the format is officially frozen. I agree there's a bit of uncertainty in this move, but I think that at this point it's a reasonable assumption that things will work, even with different opus versions. It seems like Opus is at a stage where we want to at least start adding support for it so we can switch to it by default as early as possible. Its not like this is a new idea, the plan was always to jump to a stable bitstream format when one appeared. However, that is imho a different issue than the celt051 support. A new release of spice client and server supporting opus does not magically make old servers and client disappear, so it would still be the case that e.g. debian spice client would get lame audio performance if connecting to say a RHEV spice client, or if some old client connects to a server running on debian. In time, it would perhaps make sense to drop celt051 support, but its seems pretty bad to release a client binary that doesn't do audio well with all currently existing deployed servers. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Thu, Jun 14, 2012 at 10:14:36AM +0200, Alexander Larsson wrote: However, that is imho a different issue than the celt051 support. A new release of spice client and server supporting opus does not magically make old servers and client disappear, so it would still be the case that e.g. debian spice client would get lame audio performance if connecting to say a RHEV spice client, or if some old client connects to a server running on debian. In time, it would perhaps make sense to drop celt051 support, but its seems pretty bad to release a client binary that doesn't do audio well with all currently existing deployed servers. It all depends if we consider remote SPICE access with limited bandwidth and with audio needed will be an important use case that must run as good as possible. In my opinion, sound is most of the time not the most important thing if what you want is a remote desktop. It also won't be really noticeable on LAN, or in GNOME Boxes use case, ... What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? I don't know if no audio at all is more acceptable than not doing audio well in some cases. Christophe pgpyCBTY4WApX.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Thu, 2012-06-14 at 12:31 +0200, Christophe Fergeau wrote: On Thu, Jun 14, 2012 at 10:14:36AM +0200, Alexander Larsson wrote: However, that is imho a different issue than the celt051 support. A new release of spice client and server supporting opus does not magically make old servers and client disappear, so it would still be the case that e.g. debian spice client would get lame audio performance if connecting to say a RHEV spice client, or if some old client connects to a server running on debian. In time, it would perhaps make sense to drop celt051 support, but its seems pretty bad to release a client binary that doesn't do audio well with all currently existing deployed servers. It all depends if we consider remote SPICE access with limited bandwidth and with audio needed will be an important use case that must run as good as possible. In my opinion, sound is most of the time not the most important thing if what you want is a remote desktop. It also won't be really noticeable on LAN, or in GNOME Boxes use case, ... What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? I don't know if no audio at all is more acceptable than not doing audio well in some cases. I don't know if that is true. If nothing else works then in some cases PCM is a good approach. However, maybe the client should warn about this and allow disabling audio? ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Thu, Jun 14, 2012 at 12:50:39PM +0200, Alexander Larsson wrote: On Thu, 2012-06-14 at 12:31 +0200, Christophe Fergeau wrote: What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? I don't know if no audio at all is more acceptable than not doing audio well in some cases. I don't know if that is true. If nothing else works then in some cases PCM is a good approach. However, maybe the client should warn about this and allow disabling audio? If this is good enough for you to avoid bad audio experience when celt isn't there, then yes this would good too. Christophe pgpkUpcc31Ykl.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi, What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? You can't, PCM is the baseline which isn't negotiated. You can only disconnect the sound channel. I don't know if no audio at all is more acceptable than not doing audio well in some cases. Of course you can warn if no codec could be negotiated and offer the option to disable sound. Not sure it is worth the trouble though, as far I know there is no data traveling the wire if the guest doesn't play audio. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Christophe Fergeau píše v Čt 14. 06. 2012 v 12:31 +0200: On Thu, Jun 14, 2012 at 10:14:36AM +0200, Alexander Larsson wrote: However, that is imho a different issue than the celt051 support. A new release of spice client and server supporting opus does not magically make old servers and client disappear, so it would still be the case that e.g. debian spice client would get lame audio performance if connecting to say a RHEV spice client, or if some old client connects to a server running on debian. In time, it would perhaps make sense to drop celt051 support, but its seems pretty bad to release a client binary that doesn't do audio well with all currently existing deployed servers. It all depends if we consider remote SPICE access with limited bandwidth and with audio needed will be an important use case that must run as good as possible. In my opinion, sound is most of the time not the most important thing if what you want is a remote desktop. It also won't be really noticeable on LAN, or in GNOME Boxes use case, ... What I gather from this thread is that we don't want anyone to use the fallback PCM code, which means we should deprecate it if that's really what we want... Maybe the clients could be patched to stop advertising raw PCM support? I don't know if no audio at all is more acceptable than not doing audio well in some cases. There was an interest in lossless audio for some medical application some time ago so support for explicit codec choice (and incoproration of FLAC or some better lossless codec, if any) would be a big improvement for some spice users. David Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- David Jaša, RHCE SPICE QE based in Brno GPG Key: 22C33E24 Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi, More official than the codec working group declaring it to be in its finally frozen form, or the codec specification being submitted to and approved by the IESG for publication? You keep refering to standards, while I am talking about what we can actually rely on, the implementation. I'd be very surprised if the most recent opus release doesn't conform to the submitted specification. Guess the 1.0 release just waits for the formal approval publication This is not helping, we have implementations not really frozen. ??? Can't see what your problem is ... Since no releases happened recently, I assume no security issues are known. That's an interesting assumption ... Isn't it how security works? It's safe until it's proven it's no longer, isn't it? For a piece of software not being actively maintained any more by anyone I wouldn't bet on it. Spice depends on celt 0.5.1, people do care. What kind of care, beyond packaging using it? It's your decision what you do with spice. But saying Opus isn't ready yet is simply not true. You may have other reasons not to use it, but that isn't one of them that stands up to genuine scrutiny. Personally, I would be glad to have opus support once there is an implementation the spice server can rely on. IMHO we should undust Alons opus patches *now* and get stuff ready. Given the sample rate issue a bit more work that just switching the codec, so it will need some time anyway. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Wed, Jun 13, 2012 at 09:25:59AM +0200, Gerd Hoffmann wrote: Hi, More official than the codec working group declaring it to be in its finally frozen form, or the codec specification being submitted to and approved by the IESG for publication? You keep refering to standards, while I am talking about what we can actually rely on, the implementation. I'd be very surprised if the most recent opus release doesn't conform to the submitted specification. Guess the 1.0 release just waits for the formal approval publication This is not helping, we have implementations not really frozen. ??? Can't see what your problem is ... Since no releases happened recently, I assume no security issues are known. That's an interesting assumption ... Isn't it how security works? It's safe until it's proven it's no longer, isn't it? For a piece of software not being actively maintained any more by anyone I wouldn't bet on it. Spice depends on celt 0.5.1, people do care. What kind of care, beyond packaging using it? It's your decision what you do with spice. But saying Opus isn't ready yet is simply not true. You may have other reasons not to use it, but that isn't one of them that stands up to genuine scrutiny. Personally, I would be glad to have opus support once there is an implementation the spice server can rely on. IMHO we should undust Alons opus patches *now* and get stuff ready. Given the sample rate issue a bit more work that just switching the codec, so it will need some time anyway. I replied to Ron privately but let me repeat my problems last time around. They are certainly not unsurmountable, I just didn't have the right motivation, that Ron provided right now (i.e. saying that we might be carrying a breakable package): We have a fixed sample rate of 44100 in spiceaudio, as defined by spice server. I guess I could try to either resample (didn't want to go down that path last time since it seemed a waste of cpu for something I could just adjust upfront in the guest/client for playback/recording) or change it. I can start by seeing if changing the rate breaks anything using celt, if not I can change it for both celt and opus. If it does break I still have the options of either having a single codec enabled, or doing resampling and providing the older sampling rate (and then some time in the future when we drop celt we can drop the resampling). Alon cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Wed, Jun 13, 2012 at 09:51:24AM +0400, Michael Tokarev wrote: On 13.06.2012 04:57, Brian Vetter wrote: As an outside observer with nothing really at stake here, it would seem that rather than Debian providing a hobbled version of the spice client that uses raw audio (disables Celt), they offer up a patch for both the server and the client that implements a negotiation for either Opus or Celt (for backwards compatibility). A spice client without Opus could utilize Celt and one that only supports Opus could negotiate with the server for Opus. The fall back for either is the raw PPM data. 1. Celt051 will not be in debian, for the reasons outlined several times before. 2. I stepped in trying to write a patch in question when the alternative was to drop spice from debian due to its dependency on celt. I don't know neither spice nor celt nor opus to be able to write something more serious. 3. We've about a week or two before debian release freeze, after which time no new software will be accepted into debian. Care to explain how realistic your proposal is? OPUS is already in debian, correct? if spice is accepted with raw audio, and later (don't want to put a date) a version with OPUS support exists, will it be accepted as an update? Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi, IMHO we should undust Alons opus patches *now* and get stuff ready. Given the sample rate issue a bit more work that just switching the codec, so it will need some time anyway. I replied to Ron privately but let me repeat my problems last time around. They are certainly not unsurmountable, I just didn't have the right motivation, that Ron provided right now (i.e. saying that we might be carrying a breakable package): We have a fixed sample rate of 44100 in spiceaudio, as defined by spice server. I guess I could try to either resample (didn't want to go down that path last time since it seemed a waste of cpu for something I could just adjust upfront in the guest/client for playback/recording) or change it. It's a waste indeed, and I think the best long-term option is to go for 48 kHz. Real hardware does this, virtual qemu hardware does this, and using it on the complete path from guest to spice client makes perfect sense. I suspect there is no way around resampling support though. We'll need it for compatibility reasons, so sound keeps working if only one side is able to operate at 48 kHz. cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Wed, Jun 13, 2012 at 01:49:59PM +0200, Gerd Hoffmann wrote: Hi, IMHO we should undust Alons opus patches *now* and get stuff ready. Given the sample rate issue a bit more work that just switching the codec, so it will need some time anyway. I replied to Ron privately Yeah, I only took that part off the list to avoid distracting an otherwise busy thread with what seemed like a 'side issue' at the time, that Alon and I could clarify and bring back to it again later. You're welcome to quote or forward any of that if you like. but let me repeat my problems last time around. They are certainly not unsurmountable, I just didn't have the right motivation, that Ron provided right now (i.e. saying that we might be carrying a breakable package): We have a fixed sample rate of 44100 in spiceaudio, as defined by spice server. I guess I could try to either resample (didn't want to go down that path last time since it seemed a waste of cpu for something I could just adjust upfront in the guest/client for playback/recording) or change it. It's a waste indeed, and I think the best long-term option is to go for 48 kHz. Real hardware does this, virtual qemu hardware does this, and using it on the complete path from guest to spice client makes perfect sense. The 44.1 rate is what CD audio uses (for amusing reasons related to the frame rate of video cassettes originally used for storing digital audio) but it's becoming increasingly isolated in that for modern digital audio. Lots of hardware these days doesn't handle that rate natively, and needs it to be resampled to/from 48k anyway - so if you don't need 44.1 for other reasons of your own, then roundtripping through it is a good thing to avoid. I suspect there is no way around resampling support though. We'll need it for compatibility reasons, so sound keeps working if only one side is able to operate at 48 kHz. The ability to resample OTOH probably isn't a silly thing to have, and can be fairly easily set up as an inline filter that is switched out if you really don't need it. Just to be clear about this though, there is actually no reason that both sides of the link need to use the *same* sample rate, even without a resampler. For instance you can perfectly well push an 8kHz stream into opus, and decode that as a 48kHz stream at the other end, or vice versa, without resampling, for any integer fraction of 48k down to 8. Or just because one end needs to resample from 44.1 to 48, it doesn't prevent the other side from still natively using 48k etc. So this isn't something you strictly need to coordinate between endpoints unless they have their own requirements to do that. The common thing they negotiate is we both speak opus, while the sample rate they see at their own ends can be entirely up to them to decide, without needing to agree on a common rate, or to share information about the rate that they chose. Once encoded in opus, there is no sample rate anymore, except what you choose to decode it as, at the time you choose to decode it. Does that make sense, or did I just make this sound even more confusing than it really is ;? Cheers, Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] Make celt to be optional
Like patch [1] for spice, This patch will make celt to be
optional for spice-gtk.
[1] http://lists.freedesktop.org/archives/spice-devel/2012-June/009410.html
Signed-off-by: Liang Guo guoli...@debian.org
---
configure.ac | 24 +++-
gtk/channel-playback.c | 23 ++-
gtk/channel-record.c | 37 -
3 个文件被修改，插入 61 行(+)，删除 23 行(-)
diff --git a/configure.ac b/configure.ac
index 09129b7..e218f2d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -95,11 +95,24 @@ AC_SUBST(PIXMAN_CFLAGS)
AC_SUBST(PIXMAN_LIBS)
SPICE_GLIB_REQUIRES+= pixman-1 = 0.17.7
-PKG_CHECK_MODULES(CELT051, celt051 = 0.5.1.1)
-AC_SUBST(CELT051_CFLAGS)
-AC_SUBST(CELT051_LIBS)
-AC_SUBST(CELT051_LIBDIR)
-SPICE_GLIB_REQUIRES+= celt051 = 0.5.1.1
+AC_ARG_ENABLE([celt],
+ AS_HELP_STRING([--enable-celt=@:@yes/no@:@],
+ [Enable celt support @:@default=yes@:@]),
+ [],
+ [enable_celt=yes])
+
+if test x$enable_celt = xno; then
+ AM_CONDITIONAL(WITH_CELT051, false)
+else
+ PKG_CHECK_MODULES(CELT051, celt051 = 0.5.1.1)
+ AC_SUBST(CELT051_CFLAGS)
+ AC_SUBST(CELT051_LIBS)
+ AC_SUBST(CELT051_LIBDIR)
+ SPICE_GLIB_REQUIRES+= celt051 = 0.5.1.1
+ AC_DEFINE(HAVE_CELT051, [1], [Define if celt codec])
+ AM_CONDITIONAL(WITH_CELT051, true)
+fi
+
PKG_CHECK_MODULES(SSL, openssl)
AC_SUBST(SSL_CFLAGS)
@@ -618,6 +631,7 @@ AC_MSG_NOTICE([
SASL support: ${enable_sasl}
Smartcard support:${enable_smartcard}
USB redirection support: ${have_usbredir}
+CELT support: ${enable_celt}
Gtk: $GTK_API_VERSION
Now type 'make' to build $PACKAGE
diff --git a/gtk/channel-playback.c b/gtk/channel-playback.c
index 61501c8..d4e50cc 100644
--- a/gtk/channel-playback.c
+++ b/gtk/channel-playback.c
@@ -15,7 +15,6 @@
You should have received a copy of the GNU Lesser General Public
License along with this library; if not, see http://www.gnu.org/licenses/.
*/
-#include celt051/celt.h
#include spice-client.h
#include spice-common.h
@@ -24,6 +23,10 @@
#include spice-marshal.h
+#if HAVE_CELT051
+#include celt051/celt.h
+#endif
+
/**
* SECTION:channel-playback
* @short_description: audio stream for playback
@@ -48,8 +51,10 @@
struct _SpicePlaybackChannelPrivate {
int mode;
+#if HAVE_CELT051
CELTMode*celt_mode;
CELTDecoder *celt_decoder;
+#endif
guint32 frame_count;
guint32 last_time;
guint8 nchannels;
@@ -85,8 +90,10 @@ static void spice_playback_handle_msg(SpiceChannel *channel,
SpiceMsgIn *msg);
static void spice_playback_channel_reset_capabilities(SpiceChannel *channel)
{
+#ifdef HAVE_CELT051
if (!g_getenv(SPICE_DISABLE_CELT))
spice_channel_set_capability(SPICE_CHANNEL(channel),
SPICE_PLAYBACK_CAP_CELT_0_5_1);
+#endif
spice_channel_set_capability(SPICE_CHANNEL(channel),
SPICE_PLAYBACK_CAP_VOLUME);
}
@@ -99,6 +106,7 @@ static void spice_playback_channel_init(SpicePlaybackChannel
*channel)
static void spice_playback_channel_finalize(GObject *obj)
{
+#if HAVE_CELT051
SpicePlaybackChannelPrivate *c = SPICE_PLAYBACK_CHANNEL(obj)-priv;
if (c-celt_decoder) {
@@ -110,9 +118,9 @@ static void spice_playback_channel_finalize(GObject *obj)
celt051_mode_destroy(c-celt_mode);
c-celt_mode = NULL;
}
-
g_free(c-volume);
c-volume = NULL;
+#endif
if (G_OBJECT_CLASS(spice_playback_channel_parent_class)-finalize)
G_OBJECT_CLASS(spice_playback_channel_parent_class)-finalize(obj);
@@ -163,8 +171,8 @@ static void spice_playback_channel_set_property(GObject
*gobject,
/* main or coroutine context */
static void spice_playback_channel_reset(SpiceChannel *channel, gboolean
migrating)
{
+#ifdef HAVE_CELT051
SpicePlaybackChannelPrivate *c = SPICE_PLAYBACK_CHANNEL(channel)-priv;
-
if (c-celt_decoder) {
celt051_decoder_destroy(c-celt_decoder);
c-celt_decoder = NULL;
@@ -174,7 +182,7 @@ static void spice_playback_channel_reset(SpiceChannel
*channel, gboolean migrati
celt051_mode_destroy(c-celt_mode);
c-celt_mode = NULL;
}
-
+#endif
SPICE_CHANNEL_CLASS(spice_playback_channel_parent_class)-channel_reset(channel,
migrating);
}
@@ -359,6 +367,7 @@ static void playback_handle_data(SpiceChannel *channel,
SpiceMsgIn *in)
emit_main_context(channel, SPICE_PLAYBACK_DATA,
packet-data, packet-data_size);
break;
+#ifdef HAVE_CELT051
case SPICE_AUDIO_DATA_MODE_CELT_0_5_1: {
celt_int16_t pcm[256 * 2];
@@ -374,6 +383,7 @@ static void playback_handle_data(SpiceChannel *channel,
SpiceMsgIn *in)
(uint8_t *)pcm, sizeof(pcm));
break;
}
+#endif
default:
g_warning(%s:
Re: [Spice-devel] [PATCH] make celt to be optional
On Sat, 2012-06-02 at 15:46 +0400, Michael Tokarev wrote: I plan to use this patch in the upcoming Debian release, codename wheezy, to get rid of celt codec library there, since we decided celt051 is not going to be included, but it is obviously not a good idea to drop spice entirely. Isn't it better to drop spice completely rather than shipping a version thats essentially protocol incompatible? (Well, it will fall back to no audio or non-compressed audio, but that is untested and pretty bad for actual use of spice.) Then users that want to use spice can get a working version somewhere else instead of just thinking that spice sucks because of this change. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 12.06.2012 12:48, Alexander Larsson wrote: On Sat, 2012-06-02 at 15:46 +0400, Michael Tokarev wrote: I plan to use this patch in the upcoming Debian release, codename wheezy, to get rid of celt codec library there, since we decided celt051 is not going to be included, but it is obviously not a good idea to drop spice entirely. Isn't it better to drop spice completely rather than shipping a version thats essentially protocol incompatible? (Well, it will fall back to no audio or non-compressed audio, but that is untested and pretty bad for actual use of spice.) It isn't incompatible. It might be incompatible with older releases of spice where audio codec negotiation/fallback were not properly implemented (read: was buggy), but it is okay now. Did you read the whole my email where I mention testing I performed? Then users that want to use spice can get a working version somewhere else instead of just thinking that spice sucks because of this change. Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, 2012-06-12 at 12:54 +0400, Michael Tokarev wrote: On 12.06.2012 12:48, Alexander Larsson wrote: On Sat, 2012-06-02 at 15:46 +0400, Michael Tokarev wrote: I plan to use this patch in the upcoming Debian release, codename wheezy, to get rid of celt codec library there, since we decided celt051 is not going to be included, but it is obviously not a good idea to drop spice entirely. Isn't it better to drop spice completely rather than shipping a version thats essentially protocol incompatible? (Well, it will fall back to no audio or non-compressed audio, but that is untested and pretty bad for actual use of spice.) It isn't incompatible. It might be incompatible with older releases of spice where audio codec negotiation/fallback were not properly implemented (read: was buggy), but it is okay now. Did you read the whole my email where I mention testing I performed? Its not incompatible, like I i said in my (Well, .. part above. But what it means is that it will send audio as PCM instead of compressing it. This is much larger, and will cause anyone using the debian spice client to connect to a spice server (even one that supports celt) to use much more bandwidth due to this. Its not very obvious that the reason for this is that Debian decided to package Spice that way, but rather the likely conclusion that any user would draw is that Spice just works like that. Thats not exactly a good impression for Spice. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Mon, Jun 4, 2012 at 11:33 AM, Christophe Fergeau cferg...@redhat.com wrote: Makes sense to me though I've only looked quickly through it. Since it works for you, I'm in favour of committing it if noone disagrees. I also don't like these changes to end up upstream, for the same reasons Alex mentioned. It makes me sad that Spice will end up with debian without Celt. They should disable audio completely if it's the case. I would be more in favour of a patch --disable-audio instead. -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 12.06.2012 13:15, Alexander Larsson wrote: [] Its not incompatible, like I i said in my (Well, .. part above. But what it means is that it will send audio as PCM instead of compressing it. This is much larger, and will cause anyone using the debian spice client to connect to a spice server (even one that supports celt) to use much more bandwidth due to this. Its not very obvious that the reason Why do you think it is really that bad? When used in a LAN, bandwidth doesn't matter, and it will even work a tiny bit better due to less cpu usage for encoding/decoding. It may matter for slow/long Internet links, but these are much less important for audio. Also, audio does not come alone usually (except of a few windows sounds), it usually comes together with video (ie, movies etc), which has their own requiriments which are much stronger than even raw audio. I'm not saying celt or other codec is bad or not needed, something is definitely worth to have to be able to transmit audio cheaply, but it is not the first priority thing for sure. Much more important is to have good visual expirence, and this is what spice is all about, and this is something what is not changed by this patch. Do you not agree? for this is that Debian decided to package Spice that way, but rather the likely conclusion that any user would draw is that Spice just works like that. Thats not exactly a good impression for Spice. Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 12.06.2012 14:27, Marc-André Lureau wrote: On Mon, Jun 4, 2012 at 11:33 AM, Christophe Fergeau cferg...@redhat.com wrote: Makes sense to me though I've only looked quickly through it. Since it works for you, I'm in favour of committing it if noone disagrees. I also don't like these changes to end up upstream, for the same reasons Alex mentioned. It makes me sad that Spice will end up with debian without Celt. So let's decide. Without spice, which means that qemu/kvm, which is a primary user of spice, does not support spice in debian, and which means there's no spice clients in debian (which is less of a problem). Or without celt. (Mind you, it wasn't me who decided that celt is somehow bad to have in Debian, and I myself does not understand the issues 100%. One of the issues, iirc, is that celt does not compile/work on anything but x86). I already replied to email by Christophe, asking why he thinks raw audio is that bad. The same question pops again. For the upstream, and I already mentioned this, the patch might help to identify all places which should be touched if a need to add a new codec emerges. They should disable audio completely if it's the case. I would be more in favour of a patch --disable-audio instead. It was one of alternatives. For the reasons I already mentioned (raw pcm isn't bad), making celt optional has been choosen, as it appears to be less restrictive for the user. Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, 2012-06-12 at 14:31 +0400, Michael Tokarev wrote: On 12.06.2012 13:15, Alexander Larsson wrote: [] Its not incompatible, like I i said in my (Well, .. part above. But what it means is that it will send audio as PCM instead of compressing it. This is much larger, and will cause anyone using the debian spice client to connect to a spice server (even one that supports celt) to use much more bandwidth due to this. Its not very obvious that the reason Why do you think it is really that bad? When used in a LAN, bandwidth doesn't matter, and it will even work a tiny bit better due to less cpu usage for encoding/decoding. It may matter for slow/long Internet links, but these are much less important for audio. Also, audio does not come alone usually (except of a few windows sounds), it usually comes together with video (ie, movies etc), which has their own requiriments which are much stronger than even raw audio. I'm not saying celt or other codec is bad or not needed, something is definitely worth to have to be able to transmit audio cheaply, but it is not the first priority thing for sure. Much more important is to have good visual expirence, and this is what spice is all about, and this is something what is not changed by this patch. Do you not agree? I don't really agree. Its true that if you have unlimited bandwidth, then bandwidth doesn't matter. But I don't think in practice this is true, as bandwidth always has some limits in reality. Its possible that it works fine in some situations, but its also quite possible that its a deal-breaker in some others, and we have no data to substantiate the claim that it doesn't matter (in fact, it seems unlikely since the developers spent time and energy to implement audio compression). Spice is very much about more than just pixels, that it does more than graphics (usb, sound, audio-video sync, clipboard, etc) is an important part of why Spice is better than other solutions. In fact, our support of movies by using compressed video and audio is a pretty important selling point of spice. The main thing I disagree with this change is that you change how spice works, making act differently than its creators intended, due to a packaging technicallity. You decide that spice users are not interested in low-bandwidh audio, without asking and without telling them. (I'm sure it'll be mentioned somewhere, but its unlikely that most users will see it.) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 12.06.2012 15:35, Alexander Larsson wrote: On Tue, 2012-06-12 at 14:31 +0400, Michael Tokarev wrote: On 12.06.2012 13:15, Alexander Larsson wrote: [] Its not incompatible, like I i said in my (Well, .. part above. But what it means is that it will send audio as PCM instead of compressing it. This is much larger, and will cause anyone using the debian spice client to connect to a spice server (even one that supports celt) to use much more bandwidth due to this. Its not very obvious that the reason Why do you think it is really that bad? When used in a LAN, bandwidth doesn't matter, and it will even work a tiny bit better due to less cpu usage for encoding/decoding. It may matter for slow/long Internet links, but these are much less important for audio. Also, audio does not come alone usually (except of a few windows sounds), it usually comes together with video (ie, movies etc), which has their own requiriments which are much stronger than even raw audio. I'm not saying celt or other codec is bad or not needed, something is definitely worth to have to be able to transmit audio cheaply, but it is not the first priority thing for sure. Much more important is to have good visual expirence, and this is what spice is all about, and this is something what is not changed by this patch. Do you not agree? I don't really agree. Its true that if you have unlimited bandwidth, then bandwidth doesn't matter. But I don't think in practice this is true, as bandwidth always has some limits in reality. Its possible that Yes, in practice there's always some limits. For standard CD-quality audio on a LAN, -- which I mentioned above -- this limit is more in unlimited category (since PCM/RAW stream is small even for 10Mbps network, but such networks don't exists on LANs anymore). And yes, when you connect to it over 'net, esp. over long-distance links, that limit becomes much more limiting. it works fine in some situations, but its also quite possible that its a deal-breaker in some others, and we have no data to substantiate the claim that it doesn't matter (in fact, it seems unlikely since the developers spent time and energy to implement audio compression). Yet again, I didn't say it does not matter, I especially mentioned this in previous email. Spice is very much about more than just pixels, that it does more than graphics (usb, sound, audio-video sync, clipboard, etc) is an important part of why Spice is better than other solutions. In fact, our support of movies by using compressed video and audio is a pretty important selling point of spice. The main thing I disagree with this change is that you change how spice works, making act differently than its creators intended, due to a packaging technicallity. You decide that spice users are not interested in low-bandwidh audio, without asking and without telling them. (I'm sure it'll be mentioned somewhere, but its unlikely that most users will see it.) Well. Alternative I have is to not provide spice in Debian at all. This includes qemu/kvm packages shipping without spice support, so even if you add spice client built from sources, it wont help. This includes such things like xspice. Does this work better for you? For users? Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 03:47:09PM +0400, Michael Tokarev wrote: On 12.06.2012 15:35, Alexander Larsson wrote: Spice is very much about more than just pixels, that it does more than graphics (usb, sound, audio-video sync, clipboard, etc) is an important part of why Spice is better than other solutions. In fact, our support of movies by using compressed video and audio is a pretty important selling point of spice. The main thing I disagree with this change is that you change how spice works, making act differently than its creators intended, due to a packaging technicallity. You decide that spice users are not interested in low-bandwidh audio, without asking and without telling them. (I'm sure it'll be mentioned somewhere, but its unlikely that most users will see it.) Well. Alternative I have is to not provide spice in Debian at all. This includes qemu/kvm packages shipping without spice support, so even if you add spice client built from sources, it wont help. This includes such things like xspice. Does this work better for you? For users? Neither scenario is good for users. We need to focus on addressing why Celt051 can't be included in Debian, not arguing over which terrible alternative is least-bad for users. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 02:44:56PM +0400, Michael Tokarev wrote: (Mind you, it wasn't me who decided that celt is somehow bad to have in Debian, and I myself does not understand the issues 100%. One of the issues, iirc, is that celt does not compile/work on anything but x86). I think the main issue debian has with our spice use is that we require an old version for compatibility reasons, and debian does not want to ship this older version just for spice. They should disable audio completely if it's the case. I would be more in favour of a patch --disable-audio instead. It was one of alternatives. For the reasons I already mentioned (raw pcm isn't bad), making celt optional has been choosen, as it appears to be less restrictive for the user. While it's sad that spice won't be able to use celt on debian, it's probably better than no spice support at all, at least this means things will work locally/on LAN. This indeed also means people evaluating SPICE on Debian for WAN use will have worse results than what they could have. The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( Christophe pgpfOWjDTUnqs.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. Sadly, I think debian folks aren't looking at what is really celt051. We could just have renamed it to spicelt and they wouldn't even have said a word I bet. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. As I understand it, while the bitstream is not officially frozen yet, it's very unlikely to change before the real freeze (unless big last minute issues are fine), so an Opus mode (marked as no compat guarantees, use at your own risk, ...) will probably not cause significant compatibilities issues. Sadly, I think debian folks aren't looking at what is really celt051. It's not really an old and unmaintained release of celt ? Christophe pgpuEr4l6M3O1.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. I know both Celt Opus are better codecs, but is it feasible to perhaps add a Vorbis codec as a further option. It is a widely available codec which has been bitstream compatible for ages, and I'd hope it would be better than falling back to raw PCM ? This would mean we'd not have to rush into Opus or newer Celt while they are still not declared bitstram stable. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
- Mensaje original - On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. As I understand it, while the bitstream is not officially frozen yet, it's very unlikely to change before the real freeze (unless big last minute issues are fine), so an Opus mode (marked as no compat guarantees, use at your own risk, ...) will probably not cause significant compatibilities issues. That's just guesses. It's not about library API, but about bitstream. There is no guarantee. Sticking to celt051 is still a safer alternative. Otherwise, how would you identify almost-frozen bitstream to frozen bitstream? You would have to identify by library version (erk!) and be compatible with the old and new bitstram (which might be complicated depending on library design), or be incompatible with the intermediate version, situation which we better avoid! Sadly, I think debian folks aren't looking at what is really celt051. It's not really an old and unmaintained release of celt ? If something would show up, the spice team would certainly update it. So I wouldn't say unmaintained. That's also what I would call a stable release. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 06/12/2012 09:11 AM, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. Sadly, I think debian folks aren't looking at what is really celt051. We could just have renamed it to spicelt and they wouldn't even have said a word I bet. What about a client popup no audio codecs available. audio will be disabled if/when the detected bandwidth is below some margin, or just completely assuming nothing other than raw PCM is available. Or a warning that the low bandwidth will impact performance and give the user the option to disable audio. A user trying to fix their problem would google debian spice no audio codec and presumably come across this thread ;-) and curse at the politics of software development. Thanks, David ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:59:39AM -0400, Marc-André Lureau wrote: - Mensaje original - On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. As I understand it, while the bitstream is not officially frozen yet, it's very unlikely to change before the real freeze (unless big last minute issues are fine), so an Opus mode (marked as no compat guarantees, use at your own risk, ...) will probably not cause significant compatibilities issues. That's just guesses. It's not about library API, but about bitstream. There is no guarantee. A guess supported by the slides at http://www.opus-codec.org/presentations/ , by various mailing posts from opus developers, ... Sticking to celt051 is still a safer alternative. Not suggesting dropping celt051 support upstream... Otherwise, how would you identify almost-frozen bitstream to frozen bitstream? You would have to identify by library version (erk!) and be compatible with the old and new bitstram (which might be complicated depending on library design), or be incompatible with the intermediate version, situation which we better avoid! We would make no guarantee with interoperability between binaries using different opus versions until the format is officially frozen. I agree there's a bit of uncertainty in this move, but I think that at this point it's a reasonable assumption that things will work, even with different opus versions. Sadly, I think debian folks aren't looking at what is really celt051. It's not really an old and unmaintained release of celt ? If something would show up, the spice team would certainly update it. So I wouldn't say unmaintained. Is anyone from the team monitoring celt security issues and whether they apply to celt 0.5.1? Christophe pgp5dfD2tO545.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi - Mensaje original - A guess supported by the slides at http://www.opus-codec.org/presentations/ , by various mailing posts from opus developers, ... Can you quote the relevant part of the slide? According to http://www.opus-codec.org/downloads/ The bit-stream was tentatively frozen several times, and broke from time to time (in 0.9.8 at least). Sticking to celt051 is still a safer alternative. Not suggesting dropping celt051 support upstream... Why do you suggest that? Otherwise, how would you identify almost-frozen bitstream to frozen bitstream? You would have to identify by library version (erk!) and be compatible with the old and new bitstram (which might be complicated depending on library design), or be incompatible with the intermediate version, situation which we better avoid! We would make no guarantee with interoperability between binaries using different opus versions until the format is officially frozen. I agree there's a bit of uncertainty in this move, but I think that at this point it's a reasonable assumption that things will work, even with different opus versions. We don't want to introduce changes that will break in the future, even knowingly, as we want to maintain compatibility. That's the reason why the update celt codec never went upstream. But we can't force people from not forking and do what pleases them. Regarding current Spice upstream, we better stay off those experimental and unstable path. Sadly, I think debian folks aren't looking at what is really celt051. It's not really an old and unmaintained release of celt ? If something would show up, the spice team would certainly update it. So I wouldn't say unmaintained. Is anyone from the team monitoring celt security issues and whether they apply to celt 0.5.1? I would be really surprised if nobody is fixing spice celt security issues for RHEL. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. OPUS is frozen. The problem I had when trying to use it was that it uses a sampling rate of 48000 and we have 44110. That required changes to qemu/spice interface and I stopped there. Sadly, I think debian folks aren't looking at what is really celt051. We could just have renamed it to spicelt and they wouldn't even have said a word I bet. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. Ignore my previous message, it is a draft yet, I remembered wrongly, sorry for the noise. Sadly, I think debian folks aren't looking at what is really celt051. We could just have renamed it to spicelt and they wouldn't even have said a word I bet. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 10:55:48AM -0400, Marc-André Lureau wrote: Can you quote the relevant part of the slide? See page 20 Freeze bitstream format with a check mark to its side Page 3 too WGLC (Working Group Last Call) last minor bitstream changes There are probably more details about this in the video. Sticking to celt051 is still a safer alternative. Not suggesting dropping celt051 support upstream... Why do you suggest that? I probably misinterpreted your Sticking to celt051... Of course it's the best alternative compatibility wise, but the reason for this thread is that Debian does not want it. We don't want to introduce changes that will break in the future, even knowingly, as we want to maintain compatibility. That's the reason why the update celt codec never went upstream. As long as we don't drop celt051 support, that Opus can be disabled (it can be be disabled by default), that our position about compatibility/interoperability is clear, having support for it would be a good thing, especially since it would mean a good SPICE in Debian. Maybe I'm putting too much trust on the Opus people not making regular breaks to the Opus bitstream format, but I think it's a chance we should take. But we can't force people from not forking and do what pleases them. Regarding current Spice upstream, we better stay off those experimental and unstable path. Experimental and unstable as in being standardized by IETF? Opus is not final yet, but a this point I hope it's out of the experimental zone and is stable enough. If something would show up, the spice team would certainly update it. So I wouldn't say unmaintained. Is anyone from the team monitoring celt security issues and whether they apply to celt 0.5.1? I would be really surprised if nobody is fixing spice celt security issues for RHEL. And this would magically trigger a celt 0.5.1 update from the spice team? I assume you were talking about making new celt .tar.gz available. Christophe pgpRdi2pn9hVH.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi Marc-André, On Tue, Jun 12, 2012 at 09:59:39AM -0400, Marc-André Lureau wrote: - Mensaje original - On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. As I understand it, while the bitstream is not officially frozen yet, it's very unlikely to change before the real freeze (unless big last minute issues are fine), so an Opus mode (marked as no compat guarantees, use at your own risk, ...) will probably not cause significant compatibilities issues. That's just guesses. It's not about library API, but about bitstream. There is no guarantee. Sticking to celt051 is still a safer alternative. Otherwise, how would you identify almost-frozen bitstream to frozen bitstream? You would have to identify by library version (erk!) and be compatible with the old and new bitstram (which might be complicated depending on library design), or be incompatible with the intermediate version, situation which we better avoid! The bitstream has been frozen for just a tad under a year now. The API is frozen. The working group and IETF last calls are over. The IESG telecon has been held and has approved passing this. The only thing we're effectively waiting on before the RFC is officially published now is for this to pass through -editors. And they aren't going to edit anything that changes the API or bitstream. Fedora is shipping Opus packages now, as is Debian. Are there any other guesses that you would like informed answers to ;? Sadly, I think debian folks aren't looking at what is really celt051. It's not really an old and unmaintained release of celt ? If something would show up, the spice team would certainly update it. So I wouldn't say unmaintained. That's also what I would call a stable release. So if say, a later version of celt was found to have issues, which were fixed in that later version, and which led normally very reliable upstream developers to suggest that at least some earlier versions may be at risk of being crashed remotely ... then what would you do? Given that nobody is working on celt *at all* anymore, and there are no simple patches being produced for this that you can Just Apply to it, how exactly would you plan to certainly update it? Since the only certain update is not bitstream compatible with the celt you currently use. hint: this isn't a hypothetical question Stable releases have maintainers. Celt no longer has a maintainer of any description. There is an important distinction there. I'm not suggesting you need to panic and do anything silly. But the time to start addressing the real future of this _is_ now. Cheers, Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi Alon, On Tue, Jun 12, 2012 at 06:03:58PM +0300, Alon Levy wrote: On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. OPUS is frozen. The problem I had when trying to use it was that it uses a sampling rate of 48000 and we have 44110. That required changes to qemu/spice interface and I stopped there. Opus operates in the frequency domain, so it doesn't actually have a sampling rate at all so far as the codec is concerned. The library API supports converting that to and from the time domain at 48k or any integer fraction of that rate down to 8k. For 'oddball' rates you simply need to pass the input or output through a resampler, of which code for many is widely available. The opusenc and opusdec reference tools support files at any sampling rate you might dream up or encounter. Why exactly do you need 44.1? Many (most?) sound cards don't support that rate anyway and will require it to be resampled to 48k before being played on them in any case ... Cheers, Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Wed, Jun 13, 2012 at 01:22:43AM +0930, Ron wrote: Hi Alon, On Tue, Jun 12, 2012 at 06:03:58PM +0300, Alon Levy wrote: On Tue, Jun 12, 2012 at 09:11:24AM -0400, Marc-André Lureau wrote: Hi - Mensaje original - The ideal solution would be to have support for Opus, I think Marc André and Alon had started to look into that, but that's not done yet :( No, I didn't. Long time ago I proposed we have celt unstable flag, which would be completely broken in most cases but would also have made debian folks happy. As long as the bitstream is not frozen, we can't use opus, or we will have the same problems as with celt today. OPUS is frozen. The problem I had when trying to use it was that it uses a sampling rate of 48000 and we have 44110. That required changes to qemu/spice interface and I stopped there. Opus operates in the frequency domain, so it doesn't actually have a sampling rate at all so far as the codec is concerned. The library API supports converting that to and from the time domain at 48k or any integer fraction of that rate down to 8k. For 'oddball' rates you simply need to pass the input or output through a resampler, of which code for many is widely available. The opusenc and opusdec reference tools support files at any sampling rate you might dream up or encounter. Why exactly do you need 44.1? Many (most?) sound cards don't support that rate anyway and will require it to be resampled to 48k before being played on them in any case ... I wasn't talking about opus internals at all, since I don't understand them. I was just commenting on where I bumped into a wall when I tried to update spice to use opus. I'll need to go back and look at my tree, but I am just speaking of api problems, not about the codec itself, which is perfectly opaque to me (like celt is right now - spice doesn't care about the codec). Cheers, Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
Hi On Tue, Jun 12, 2012 at 5:35 PM, Ron r...@debian.org wrote: The bitstream has been frozen for just a tad under a year now. I don't know how you can claim that: http://www.opus-codec.org/downloads/ The bit-stream has not changed, except for some corner cases that are unlikely to happen in the real world. [ ] opus-0.9.9.tar.gz 18-Feb-2012 16:20 710K If it is, can you point to an official release note stating it clearly bitstream is frozen or compatible with any further release? The API is frozen. The working group and IETF last calls are over. The IESG telecon has been held and has approved passing this. The only thing we're effectively waiting on before the RFC is officially published now is for this to pass through -editors. And they aren't going to edit anything that changes the API or bitstream. Fedora is shipping Opus packages now, as is Debian. This is not helping, we have implementations not really frozen. So if say, a later version of celt was found to have issues, which were fixed in that later version, and which led normally very reliable upstream developers to suggest that at least some earlier versions may be at risk of being crashed remotely ... then what would you do? Given that nobody is working on celt *at all* anymore, and there are no simple patches being produced for this that you can Just Apply to it, how exactly would you plan to certainly update it? Since the only certain update is not bitstream compatible with the celt you currently use. We would make a release, just like there has been several 0.5.1 releases: http://downloads.us.xiph.org/releases/celt/celt-0.5.1.tar.gz vs http://downloads.us.xiph.org/releases/celt/celt-0.5.1.3.tar.gz Since no releases happened recently, I assume no security issues are known. As I said, it would be quite critical for the Spice project to rely on a celt release with security issues. As long as spice support 0.5.1 (which is likely for a long time still) there will be new releases or fix when necessary. I'm not suggesting you need to panic and do anything silly. But the time to start addressing the real future of this _is_ now. It will be when there is an official and clear announcement that the opus library implementation has frozen bit-stream (or any further release will be bit-stream compatible) -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 06:30:36PM +0200, Marc-André Lureau wrote: Hi On Tue, Jun 12, 2012 at 5:35 PM, Ron r...@debian.org wrote: The bitstream has been frozen for just a tad under a year now. I don't know how you can claim that: http://www.opus-codec.org/downloads/ The bit-stream has not changed, except for some corner cases that are unlikely to happen in the real world. [ ] opus-0.9.9.tar.gz 18-Feb-2012 16:20 710K Which part of The bit-stream has not changed is unclear? The qualifier there I believe is a reference to this commit: Fixes the code for optional self-delimited packing to make it fit the draft. This has no impact on opus_demo, test vectors, or normal codec operation. Since support for that packing mode is declared to be OPTIONAL in the standard (their caps not mine :), anything which might use it cannot expect to actually be interoperable with a compliant decoder anyhow. If unlikely to happen in the real world was not an extremely conservative estimate of the real impact, then strictly preserving the bitstream would have won hands down at that point in time, without any doubt at all. If it is, can you point to an official release note stating it clearly bitstream is frozen or compatible with any further release? More official than the codec working group declaring it to be in its finally frozen form, or the codec specification being submitted to and approved by the IESG for publication? Archives of all those things are public if you don't want to take my word for it. The API is frozen. The working group and IETF last calls are over. The IESG telecon has been held and has approved passing this. The only thing we're effectively waiting on before the RFC is officially published now is for this to pass through -editors. And they aren't going to edit anything that changes the API or bitstream. Fedora is shipping Opus packages now, as is Debian. This is not helping, we have implementations not really frozen. Who is we here? I *absolutely* would not have pushed a copy of this to Debian for people to use if there was anything short of an ASTRONOMICALLY SMALL chance of there still being a bitstream change. (my caps this time, I'm serious about that. It's not going to happen while the upstream developers draw breath to stab anybody who might try). Anything which did change it would no longer be Opus, since the IETF now has change control over it, and the Last Call for changes is over. So if say, a later version of celt was found to have issues, which were fixed in that later version, and which led normally very reliable upstream developers to suggest that at least some earlier versions may be at risk of being crashed remotely ... then what would you do? Given that nobody is working on celt *at all* anymore, and there are no simple patches being produced for this that you can Just Apply to it, how exactly would you plan to certainly update it? Since the only certain update is not bitstream compatible with the celt you currently use. We would make a release, just like there has been several 0.5.1 releases: http://downloads.us.xiph.org/releases/celt/celt-0.5.1.tar.gz vs http://downloads.us.xiph.org/releases/celt/celt-0.5.1.3.tar.gz At risk of sounding repetitive, Who is we here? That looks like the download site of the people who are no longer maintaining celt to me, not evidence of people existing who actually are. 0.5.1.3 was May 2009. Made just 2 weeks after 0.5.1 to fix some initial obvious screwups. It hasn't been touched or looked at by anyone since. Since no releases happened recently, I assume no security issues are known. That's an interesting assumption ... Here's another, that is actually a little more grounded in what the people who might actually make those releases have actually been focussed on: No releases happened because *nobody* is looking at celt 0.5.1 at all, let alone backporting fixes to it for things found in much later releases. Issues found in later releases were fixed in later releases, and nobody spent even a minute looking at backporting those things to 0.5.1. And now that Opus is finalised, nobody is looking at *any version* of celt at all outside of what is in Opus. So if you aren't actively doing that, well, I guess that just leaves the black hats with motivation to do so ... As I said, it would be quite critical for the Spice project to rely on a celt release with security issues. As long as spice support 0.5.1 (which is likely for a long time still) there will be new releases or fix when necessary. Well, I just told you there is a reasonable suspicion (from the person who single-handedly has found more bugs in this code than anyone else in the world, all put together) ... What is your plan to confirm or deny 0.5.1 is affected by the problems that he found and fixed in later versions? The only thing I know for sure is that nobody
Re: [Spice-devel] [PATCH] make celt to be optional
Hi On Tue, Jun 12, 2012 at 8:19 PM, Ron r...@debian.org wrote: On Tue, Jun 12, 2012 at 06:30:36PM +0200, Marc-André Lureau wrote: Hi On Tue, Jun 12, 2012 at 5:35 PM, Ron r...@debian.org wrote: The bitstream has been frozen for just a tad under a year now. I don't know how you can claim that: http://www.opus-codec.org/downloads/ The bit-stream has not changed, except for some corner cases that are unlikely to happen in the real world. [ ] opus-0.9.9.tar.gz 18-Feb-2012 16:20 710K Which part of The bit-stream has not changed is unclear? The qualifier there I believe is a reference to this commit: Fixes the code for optional self-delimited packing to make it fit the draft. This has no impact on opus_demo, test vectors, or normal codec operation. Since support for that packing mode is declared to be OPTIONAL in the standard (their caps not mine :), anything which might use it cannot expect to actually be interoperable with a compliant decoder anyhow. If unlikely to happen in the real world was not an extremely conservative estimate of the real impact, then strictly preserving the bitstream would have won hands down at that point in time, without any doubt at all. If it is, can you point to an official release note stating it clearly bitstream is frozen or compatible with any further release? More official than the codec working group declaring it to be in its finally frozen form, or the codec specification being submitted to and approved by the IESG for publication? You keep refering to standards, while I am talking about what we can actually rely on, the implementation. This is not helping, we have implementations not really frozen. Who is we here? The community, at large. The people who actually use and distribute this code. I *absolutely* would not have pushed a copy of this to Debian for people to use if there was anything short of an ASTRONOMICALLY SMALL chance of there still being a bitstream change. (my caps this time, I'm serious about that. It's not going to happen while the upstream developers draw breath to stab anybody who might try). We just need an opus implementation to say clearly that it will be bit-stream compatible with future releases. Not play with chances. Since no releases happened recently, I assume no security issues are known. That's an interesting assumption ... Isn't it how security works? It's safe until it's proven it's no longer, isn't it? Here's another, that is actually a little more grounded in what the people who might actually make those releases have actually been focussed on: No releases happened because *nobody* is looking at celt 0.5.1 at all, let Spice depends on celt 0.5.1, people do care. What is your plan to confirm or deny 0.5.1 is affected by the problems that he found and fixed in later versions? I am not saying it's not without problems. But if something is serious, the Spice project will certainly drop celt 0.5.1 support or fix it. If Spice should drop celt 0.5.1 support altogether for security reasons, a bunch of people would be interested to know. You could just wait for the news of a live exploit actually doing the rounds, but I personally wouldn't want to trust my own systems to that plan. I am no security expert, but even if we would make a security audit, there would still be chances an exploit be found. Rushing into using new libraries isn't going to magically make security issues disappear. I'm not particularly interested in arguing with someone who has already made up their mind and for whom facts are just annoying details to handwave away. But if you're actually interested in the facts, I'm happy to help answer any questions that you or the rest of the spice team might have. I have not made up my mind, but my points are: - spice depends on 0.5.1 of celt, and for quite some time (several years), so security issues should be addressed - xiph opus implementation doesn't guarantee bit-stream compatibility yet, according to release log It's your decision what you do with spice. But saying Opus isn't ready yet is simply not true. You may have other reasons not to use it, but that isn't one of them that stands up to genuine scrutiny. Personally, I would be glad to have opus support once there is an implementation the spice server can rely on. Even when opus support will be added, celt 0.5.1 will continue to be maintained unless we drop audio support for older releases. regards -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Tue, Jun 12, 2012 at 09:38:37PM +0200, Marc-André Lureau wrote: Hi On Tue, Jun 12, 2012 at 8:19 PM, Ron r...@debian.org wrote: More official than the codec working group declaring it to be in its finally frozen form, or the codec specification being submitted to and approved by the IESG for publication? You keep refering to standards, while I am talking about what we can actually rely on, the implementation. Then you have my apologies for omitting a fact from my original list that I'd assumed was common knowledge: - The implementation you are referring to _is_ the normative part of the now finalised standard. That code defines the bitstream. Officially. Any other text is purely informative. So any deviation from compatibility with that in later releases, or any other implementation, will be a very serious bug, and cause it to no longer be an implementation of Opus. ie. This simply isn't going to happen now. We just need an opus implementation to say clearly that it will be bit-stream compatible with future releases. Not play with chances. It said that by being published as the normative standard :) You have more chance of your office being struck by a meterorite shaped like a rhinoceros, with your name written on it in 40 foot high gold leaf, whistling ride of the valkyries as it descends into the atmosphere, than of opus breaking bitstream compatibility now. Really. It's somewhere on that sort of order of risk. You're already playing a game of chance. Think of Opus as your insurance policy if your luck doesn't hold out as long as you'd like. Ron ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
As an outside observer with nothing really at stake here, it would seem that rather than Debian providing a hobbled version of the spice client that uses raw audio (disables Celt), they offer up a patch for both the server and the client that implements a negotiation for either Opus or Celt (for backwards compatibility). A spice client without Opus could utilize Celt and one that only supports Opus could negotiate with the server for Opus. The fall back for either is the raw PPM data. From all accounts, it would seem that Opus has reached a frozen status, incorporates many of the same techniques and offers the same or better specs than Celt, and has the backing of an important standards organization. Kind of seems obvious to me where things should go. Not sure why there is so much arguing here. Brian ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 13.06.2012 04:57, Brian Vetter wrote: As an outside observer with nothing really at stake here, it would seem that rather than Debian providing a hobbled version of the spice client that uses raw audio (disables Celt), they offer up a patch for both the server and the client that implements a negotiation for either Opus or Celt (for backwards compatibility). A spice client without Opus could utilize Celt and one that only supports Opus could negotiate with the server for Opus. The fall back for either is the raw PPM data. 1. Celt051 will not be in debian, for the reasons outlined several times before. 2. I stepped in trying to write a patch in question when the alternative was to drop spice from debian due to its dependency on celt. I don't know neither spice nor celt nor opus to be able to write something more serious. 3. We've about a week or two before debian release freeze, after which time no new software will be accepted into debian. Care to explain how realistic your proposal is? Thanks, /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On 04.06.2012 16:28, Alon Levy wrote: On Mon, Jun 04, 2012 at 11:33:22AM +0200, Christophe Fergeau wrote: Makes sense to me though I've only looked quickly through it. Since it works for you, I'm in favour of committing it if noone disagrees. I agree. Would you do a review and ack? Please don't apply it without at least minimal testing. I know right to nothing about spice and celt, the changes I made are purely mechanical, without understanding what's going on in a big picture (I ofcourse tried to understand the small picture, the code I touched). And I since I don't know how it works, I'm not sure I can perform any reasonable testing. For one, I'm not even sure I actually used spice way to transmit sound, -- it looked that way, but I'm not sure. Just a minimal testing is very welcome. Thank you! /mjt ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] make celt to be optional
On Mon, Jun 04, 2012 at 11:33:22AM +0200, Christophe Fergeau wrote:
Makes sense to me though I've only looked quickly through it. Since it
works for you, I'm in favour of committing it if noone disagrees.
I agree. Would you do a review and ack?
Christophe
On Sat, Jun 02, 2012 at 03:46:55PM +0400, Michael Tokarev wrote:
With this patch applied, celt051 library isn't required
anymore. It is still required by default, but there's
a new configure option, --disable-celt051, which makes
the configure code to omit checking/finding the celt
library and makes resulting spice library to not use
celt codec at all.
The changes in the code - there are relatively many -
are located in 3 source files (see diffstat):
client/playback_channel.cpp
client/record_channel.cpp
server/snd_worker.c
and are local/private to the library (client and server),
so no external ABI/API is done.
I found and marked hopefully all places where celt
codec is being touched/referenced. The patch may
help future development too, indicating all places
where codec-specific code is used (just grep source
for HAVE_CELT051 to see these).
I plan to use this patch in the upcoming Debian
release, codename wheezy, to get rid of celt
codec library there, since we decided celt051 is
not going to be included, but it is obviously not
a good idea to drop spice entirely.
I did some interoperability tests and the thing
appears to work -- unpatched client to patched
server, patched client to unpatched server and
patched client to patched server. I didn't check
old clients and servers, however. In all cases,
raw audio stream is being choosen and used. But
since I don't really know how spice works internally,
maybe I didn't perform correct testing.
Please consider applying.
Signed-off-By: Michael Tokarev m...@tls.msk.ru
Cc: Ron Lee r...@debian.org
Cc: Liang Guo bluestonech...@gmail.com
---
client/audio_channels.h |8 +
client/playback_channel.cpp | 25 ++---
client/record_channel.cpp | 21 +--
configure.ac| 16 ++---
server/snd_worker.c | 82
++-
5 files changed, 122 insertions(+), 30 deletions(-)
diff --git a/client/audio_channels.h b/client/audio_channels.h
index d38a79e..8f8a186 100644
--- a/client/audio_channels.h
+++ b/client/audio_channels.h
@@ -18,7 +18,9 @@
#ifndef _H_AUDIO_CHANNELS
#define _H_AUDIO_CHANNELS
+#if HAVE_CELT051
#include celt051/celt.h
+#endif
#include red_channel.h
#include debug.h
@@ -45,7 +47,9 @@ private:
void handle_start(RedPeer::InMessage* message);
void handle_stop(RedPeer::InMessage* message);
void handle_raw_data(RedPeer::InMessage* message);
+#if HAVE_CELT051
void handle_celt_data(RedPeer::InMessage* message);
+#endif
void null_handler(RedPeer::InMessage* message);
void disable();
@@ -57,8 +61,10 @@ private:
WavePlaybackAbstract* _wave_player;
uint32_t _mode;
uint32_t _frame_bytes;
+#if HAVE_CELT051
CELTMode *_celt_mode;
CELTDecoder *_celt_decoder;
+#endif
bool _playing;
uint32_t _frame_count;
};
@@ -96,8 +102,10 @@ private:
Mutex _messages_lock;
std::listRecordSamplesMessage * _messages;
int _mode;
+#if HAVE_CELT051
CELTMode *_celt_mode;
CELTEncoder *_celt_encoder;
+#endif
uint32_t _frame_bytes;
static int data_mode;
diff --git a/client/playback_channel.cpp b/client/playback_channel.cpp
index 802a4d3..caf6424 100644
--- a/client/playback_channel.cpp
+++ b/client/playback_channel.cpp
@@ -151,8 +151,10 @@ PlaybackChannel::PlaybackChannel(RedClient client,
uint32_t id)
Platform::PRIORITY_HIGH)
, _wave_player (NULL)
, _mode (SPICE_AUDIO_DATA_MODE_INVALID)
+#if HAVE_CELT051
, _celt_mode (NULL)
, _celt_decoder (NULL)
+#endif
, _playing (false)
{
#ifdef WAVE_CAPTURE
@@ -169,7 +171,9 @@ PlaybackChannel::PlaybackChannel(RedClient client,
uint32_t id)
handler-set_handler(SPICE_MSG_PLAYBACK_MODE,
PlaybackChannel::handle_mode);
+#if HAVE_CELT051
set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
+#endif
}
void PlaybackChannel::clear()
@@ -182,6 +186,7 @@ void PlaybackChannel::clear()
}
_mode = SPICE_AUDIO_DATA_MODE_INVALID;
+#if HAVE_CELT051
if (_celt_decoder) {
celt051_decoder_destroy(_celt_decoder);
_celt_decoder = NULL;
@@ -191,6 +196,7 @@ void PlaybackChannel::clear()
celt051_mode_destroy(_celt_mode);
_celt_mode = NULL;
}
+#endif
}
void PlaybackChannel::on_disconnect()
@@ -214,8 +220,10 @@ void PlaybackChannel::set_data_handler()
if (_mode == SPICE_AUDIO_DATA_MODE_RAW) {
[Spice-devel] [PATCH] make celt to be optional
With this patch applied, celt051 library isn't required
anymore. It is still required by default, but there's
a new configure option, --disable-celt051, which makes
the configure code to omit checking/finding the celt
library and makes resulting spice library to not use
celt codec at all.
The changes in the code - there are relatively many -
are located in 3 source files (see diffstat):
client/playback_channel.cpp
client/record_channel.cpp
server/snd_worker.c
and are local/private to the library (client and server),
so no external ABI/API is done.
I found and marked hopefully all places where celt
codec is being touched/referenced. The patch may
help future development too, indicating all places
where codec-specific code is used (just grep source
for HAVE_CELT051 to see these).
I plan to use this patch in the upcoming Debian
release, codename wheezy, to get rid of celt
codec library there, since we decided celt051 is
not going to be included, but it is obviously not
a good idea to drop spice entirely.
I did some interoperability tests and the thing
appears to work -- unpatched client to patched
server, patched client to unpatched server and
patched client to patched server. I didn't check
old clients and servers, however. In all cases,
raw audio stream is being choosen and used. But
since I don't really know how spice works internally,
maybe I didn't perform correct testing.
Please consider applying.
Signed-off-By: Michael Tokarev m...@tls.msk.ru
Cc: Ron Lee r...@debian.org
Cc: Liang Guo bluestonech...@gmail.com
---
client/audio_channels.h |8 +
client/playback_channel.cpp | 25 ++---
client/record_channel.cpp | 21 +--
configure.ac| 16 ++---
server/snd_worker.c | 82 ++-
5 files changed, 122 insertions(+), 30 deletions(-)
diff --git a/client/audio_channels.h b/client/audio_channels.h
index d38a79e..8f8a186 100644
--- a/client/audio_channels.h
+++ b/client/audio_channels.h
@@ -18,7 +18,9 @@
#ifndef _H_AUDIO_CHANNELS
#define _H_AUDIO_CHANNELS
+#if HAVE_CELT051
#include celt051/celt.h
+#endif
#include red_channel.h
#include debug.h
@@ -45,7 +47,9 @@ private:
void handle_start(RedPeer::InMessage* message);
void handle_stop(RedPeer::InMessage* message);
void handle_raw_data(RedPeer::InMessage* message);
+#if HAVE_CELT051
void handle_celt_data(RedPeer::InMessage* message);
+#endif
void null_handler(RedPeer::InMessage* message);
void disable();
@@ -57,8 +61,10 @@ private:
WavePlaybackAbstract* _wave_player;
uint32_t _mode;
uint32_t _frame_bytes;
+#if HAVE_CELT051
CELTMode *_celt_mode;
CELTDecoder *_celt_decoder;
+#endif
bool _playing;
uint32_t _frame_count;
};
@@ -96,8 +102,10 @@ private:
Mutex _messages_lock;
std::listRecordSamplesMessage * _messages;
int _mode;
+#if HAVE_CELT051
CELTMode *_celt_mode;
CELTEncoder *_celt_encoder;
+#endif
uint32_t _frame_bytes;
static int data_mode;
diff --git a/client/playback_channel.cpp b/client/playback_channel.cpp
index 802a4d3..caf6424 100644
--- a/client/playback_channel.cpp
+++ b/client/playback_channel.cpp
@@ -151,8 +151,10 @@ PlaybackChannel::PlaybackChannel(RedClient client,
uint32_t id)
Platform::PRIORITY_HIGH)
, _wave_player (NULL)
, _mode (SPICE_AUDIO_DATA_MODE_INVALID)
+#if HAVE_CELT051
, _celt_mode (NULL)
, _celt_decoder (NULL)
+#endif
, _playing (false)
{
#ifdef WAVE_CAPTURE
@@ -169,7 +171,9 @@ PlaybackChannel::PlaybackChannel(RedClient client,
uint32_t id)
handler-set_handler(SPICE_MSG_PLAYBACK_MODE,
PlaybackChannel::handle_mode);
+#if HAVE_CELT051
set_capability(SPICE_PLAYBACK_CAP_CELT_0_5_1);
+#endif
}
void PlaybackChannel::clear()
@@ -182,6 +186,7 @@ void PlaybackChannel::clear()
}
_mode = SPICE_AUDIO_DATA_MODE_INVALID;
+#if HAVE_CELT051
if (_celt_decoder) {
celt051_decoder_destroy(_celt_decoder);
_celt_decoder = NULL;
@@ -191,6 +196,7 @@ void PlaybackChannel::clear()
celt051_mode_destroy(_celt_mode);
_celt_mode = NULL;
}
+#endif
}
void PlaybackChannel::on_disconnect()
@@ -214,8 +220,10 @@ void PlaybackChannel::set_data_handler()
if (_mode == SPICE_AUDIO_DATA_MODE_RAW) {
handler-set_handler(SPICE_MSG_PLAYBACK_DATA,
PlaybackChannel::handle_raw_data);
+#if HAVE_CELT051
} else if (_mode == SPICE_AUDIO_DATA_MODE_CELT_0_5_1) {
handler-set_handler(SPICE_MSG_PLAYBACK_DATA,
PlaybackChannel::handle_celt_data);
+#endif
} else {
THROW(invalid mode);
}
@@ -224,8 +232,11 @@ void PlaybackChannel::set_data_handler()
void PlaybackChannel::handle_mode(RedPeer::InMessage* message)
{
SpiceMsgPlaybackMode* playbacke_mode =
(SpiceMsgPlaybackMode*)message-data();
-if (playbacke_mode-mode != SPICE_AUDIO_DATA_MODE_RAW
-playbacke_mode-mode !=
