Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Second look: add support for HFP and mSBC #37

Closed
wants to merge 13 commits into from

Conversation

kuikka
Copy link

@kuikka kuikka commented Feb 28, 2017

Let's try this again. I fixed all the points you commented about in the previous pull request.

I was also able to test the mSBC support on Marvell 8897 and it does work. There is slight underflow so the flow control may require some more tweaking.

Juha Kuikka added 13 commits February 28, 2017 11:32
Add a separate AT command parser that is capable or parsing all three command
types the AG is receiving.
Codec is used by forhcoming mSBC support, else it defaults to CVSD.
Flags are used to implement features in the rfcomm transport thread.
For HFP there are two codec selections, consider an SCO transport "not ready"
until the decision has been made on which one is being used.
Add AT command support to implement minimal support for HFP.
Currently only CVSD codec is supported.
For mSBC support, it is required to configure the SCO socket into transparent
mode. Add an option to configure acquired SCO socket into this mode.
These files contain the functionality required to implement
mSBC support for the SCO IO thread
Add support for mSBC codec for HFP into the SCO io thread.
It is conditional on the --enable-msbc configuration flag
HSP only supports CVSD so set HSP codec to that, for HFP the codec is decided
later in the rfcomm io thread.
@arkq
Copy link
Owner

arkq commented Mar 2, 2017

Looks, good. I haven't tested it by myself yet, though.

I'd like to pull some commits already, e.g. transparent mode for socket, SCO codecs. It will reduce a little bit this PR, so it will be easier to track changes. However, it might be an extra work for you, because this will reorder commit history, and I will make some minor code style fixes. What do you think?

@kuikka
Copy link
Author

kuikka commented Mar 2, 2017

Feel free to merge what you feel like, or even all of it. Since the mSBC is very self contained it shouldn't be a problem when I keep working on it.

@kuikka
Copy link
Author

kuikka commented Mar 2, 2017

That being said, I have identified some issues with the way I do the mSBC flow control. I took some assumptions that turn out not to be true on every controller. For example, on the Marvell 8897 the SCO MTU is 120 and the HCI bitrate is 8000 bytes/s, same as the air rate. This means that sending the 59 byte frames with one byte of padding is very convenient. However, on a Broadcom controller I have the SCO MTU is 48 but the HCI SCO bitrate is 16000 which means I cannot rely on the incoming data to easily synchronize. I need more sophisticated buffering and flow control mechanism here.

@arkq
Copy link
Owner

arkq commented Mar 2, 2017

At the end, I'd like to merge it all, of course. However, in order to maintain the code I have to understand how it works (maybe not thoroughly, but at least the logic behind it). Right now, the mSBC flow is not very clear for me, I must say :) When I finalize my struggle with the "PCM delay" (I've lost count how many approaches I've made... and it's still not right) I'll try to learn how the HFP works, and I'll proceed with merging this PR.

Once again, many tanks for the contribution!

@kuikka
Copy link
Author

kuikka commented Mar 2, 2017

I understand. I am currently refactoring the whole mSBC flow as the current one doesn't work in every case. I think I may end up using a similar mechanism to what you already have for the sync but the frames vs MTU makes it difficult.

Once I have it nailed down I'll be sure to add some comments on how it works so we don't forget it.

@arkq
Copy link
Owner

arkq commented Mar 6, 2017

I've pulled following commits (and squashed them, so the rebase won't be clean, sorry for that):

  • Add codec and flags fields into SCO transport
  • ctl: Ignore SCO transports that don't have a codec selected yet
  • Add HFP header file
  • Add option to configure SCO socket to transparent mode
  • transport: Set SCO socket into transperent mode if mSBC is used
  • transport: Set SCO codec type upon creation if known

Also, I've taken the liberty to make some changes (I hope you don't mind). I've removed the enum sco_codec in favour of hfp.h header file, which will be consistent with the a2dp-codecs.h. The codec is stored in the transport.codec field (not in the transport.sco.codec), because it was an unnecessary redundancy (imho).

I've changed a little bit the logic behind the initialisation of the transparent voice mode: t->sco.codec == HFP_CODEC_MSBC -> t->codec != HFP_CODEC_CVSD. I'm not sure if it's right, though. The reason for this was the fact that internally kernel supports CVSD only, so anything else has to be handled in the userspace - hence, transparent mode.

@arkq
Copy link
Owner

arkq commented Jul 4, 2017

Hi,

I've just pulled another piece of this PR into the master. Basically, everything except mSBC support is now merged :). I've tested HFP AG with Jabra MOVE v2.3.0 and it seems to be working just fine. @kuikka, many thanks for your contribution!

@arkq arkq mentioned this pull request Sep 7, 2017
arkq added a commit that referenced this pull request Sep 19, 2017
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
arkq added a commit that referenced this pull request Dec 28, 2017
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
arkq added a commit that referenced this pull request Feb 16, 2018
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
arkq added a commit that referenced this pull request Apr 10, 2018
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
arkq added a commit that referenced this pull request Apr 10, 2018
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
arkq added a commit that referenced this pull request May 14, 2018
Add support for the mSBC codec for HFP into the SCO IO thread.
This support is optional, and is controlled by the --enable-msbc
configuration flag.

The receiving part of this mSBC support has been tested with Jabra
MOVE v2.3.0 headset and seems to work flawlessly. However, playback
does not work... Maybe it will work with some other BT device.

Note:
This commit is a rework of a pull request submitted by Juha Kuikka.

Fixes #29 and closes #37
@arkq arkq closed this in a6c087c Mar 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants