Hi, On Wed, Mar 13, 2013 at 9:20 AM, Tanu Kaskinen <tanuk at iki.fi> wrote: > On Tue, 2013-03-12 at 14:02 +0100, Mikel Astiz wrote: >> On Tue, Mar 12, 2013 at 1:42 PM, Tanu Kaskinen <tanuk at iki.fi> wrote: >> > If the MTU is initially set to 128, it sounds logical that it may be >> > reconfigured to 128 too. This wouldn't work with the proposed code: 128 >> > -> 96 -> 128. >> >> The proposed code specifically handles only two possible transitions: >> either 48->96 or 96->48. >> >> If the MTU is set to something else other than 48 or 96, the code >> won't change the previous behavior. That's what I meant when I said it >> was 'conservative', since it handles only the cases that have been >> experimentally observed. >> >> We can try to generalize this for any random MTU value, as I was >> trying to do in my first proposal, but I see very little benefit >> unless someone proves that similar policies are useful in other >> working conditions (e.g. using a non-USB Bluetooth adapter). In this >> case I would agree that something like MAX_SCO_READ_MTU would be >> required. > > I forgot to say that I didn't see much point in being "conservative" > here, so that's why I was ignoring the 48/96 special casing. While > writing this reply, however, I realized that there is a good reason for > being conservative. We recently added handling for packets with > non-aligned size, and receiving such packets means that there are > temporary glitches that can cause any random packet size. We shouldn't > reconfigure the MTU in those cases. > > What would you think about this: replace the unaligned size check with > checking that the received size is the same as the read MTU. If it's > different, check if it's 48 or 96, in which case reconfigure the MTU, > and otherwise ignore the packet? > >> > So what do you think? I don't want to rely on undocumented behavior. If >> > BlueZ guarantees that the MTUs will always be symmetric (also in the >> > future), it's fine to update the write MTU in PulseAudio when the read >> > MTU changes. But if there's no such guarantee, I wouldn't change the >> > write MTU, unless there's a really good reason to do so. So, what >> > horrible things happen if we don't update the write MTU? >> >> If you don't update the MTU as proposed by this patch, the Bluetooth >> adapter typically powers down. > > OK, that's horrible enough :) > >> Concerning the 'undocumented behavior', you're completely right, >> starting from the fact that I found no reference in the spec about all >> this. >> >> Regarding what BlueZ's API guarantees, we shouldn't assume the MTUs >> are symmetric. If you're concerned about this, I could extend the >> patch in such a way that the policy is triggered if and only if this >> condition holds true (input MTU == output MTU). > > I prefer adding that check, but that's not a strong opinion. > > -- > Tanu > For the record, this v1 patch ([1]) is still needed if anybody is interested in having two simultaneous SCO streams (two heaDsets/phones doing HSP/HFP). A later v2 "bluetooth: Read SCO MTU from socket options" was also submitted ([2]) but this approach doesn't seem as stable as the v1 approach. As mentioned several times, the patch has been tested and seems to work, but we found no documentation supporting this behavior. I would suggest it gets merged nevertheless and have an eye on it in case it introduces regressions. Cheers, Mikel [1] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/16128 [2] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/16273
