Re: [PATCH] Breaks in A2DP playback during device search

[Marcel Holtmann <marcel@holtmann.org>]

Hi Amir,

> >> We encountered the same dilemma and suggested a solution using Flow
> >> specification.
> >>
> >> The Flow Specification HCI relates to an ACL handle and not a specific
> >> L2CAP channel. If we intend to configure the Flow Spec using socket
> >> option then different profiles might overrun each other settings.
> >>
> >> The other option of using the mgmt is not too good either. Since this
> >> command is "Link" related and not "Adapter/Device" related, the daemon
> >> dbus API would have to receive the ACL handle as parameter or introduce
> >> a new property on Device node which might not exist. Note that one can
> >> create an L2CAP socket without interfacing the daemon.
> >>
> >> My humble opinion is to set the Flow Spec per socket (l2cap/rfcomm)
> >> using socket options. Store it per channel and send accumulated flow
> >> spec on every change per ACL.
> >>
> >> - channel changed its flow sepc
> >> - channel with flow spec was closed
> >>
> >> All done in the kernel, no changes to the daemon.
> >
> > Thanks for your valuable inputs; as mentioned earlier please note we are
> > currently using Android Froyo (2.2.2) which has bluez version 4.47 &
> > kernel version 2.6.32.
> >
> > Using Flow specification is seems to be good. I will change it to flow spec
> > instead of qos setup command.
> >
> > I want to know one thing if we want to use setsockopt to send flow spec
> > (add new socket option SO_L2CAP_OQOS) just handle it l2cap.c to send hci
> > flow spec command,
> > that's what you are suggesting?
> >
> 
> First as said before you must submit your patch based on latest
> bluetooth-next kernel (not on 2.6.32).
> 
> Second,  You should get acceptance on adding new socket option. As I
> remember there was an objection on adding new socket options though in
> this case I think it is the correct solution.

I am not against adding new socket options if they make sense. Of course
we are not just adding options until we have figured them out. Reason
here is that socket option are part of the stable ABI promise that the
Linux kernel makes. So we are cautious here.

That said, a proposal need to include how this applies to older
controller with limited set of HCI commands. And of course newer
upcoming specification that might change this should also be taken into
account.

> Here is a short RFC
> I would suggest to store the flow spec parameters in the l2cap_chan struct.
> Store the current active flow spec (also add pending_flow_spec) in the
> hci_conn struct. In hci_connect after
> the acl is connected call a new function hci_calc_flow_spec which goes
> over all l2cap channel (hci_chan->list) and accumulate flow spec params.
> Set the hci_conn pending flow spec and send the HCI cmd. In hci_event add
> case for the command complete and a "cs" function for handling the response.
> In this function set the active flow spec.
> When an l2cap channel disconnect and the ACL is still active you should call
> the hci_calc_flow_spec again to accumulate the params for the rest of
> l2cap channels connected. (You should work out the details).

As a high-level approach this sounds fine. The devil is always in the
details, but you only figure that out once you start writing the code
for it.

> BTW,
> The flow spec params in the hci_conn and l2cap_chan can help us in the
> future in building trafic shaping on the hci_sched_acl.

That would be nice indeed. Though maybe a bit overkill in the end. It
seems that our SO_PRIORITY handling worked out just fine.

Regards

Marcel