Delay adjustment

[borine]

See discussion #650 for rationale. Extends the D-Bus API org.bluealsa.PCM1 to allow setting a persistent adjustment that is applied to the Delay property of the PCM. Each codec supported by the PCM has its own delay adjustment.

The choice of control name for the CTL plugin is not yet agreed, and in this initial draft the name "Sync" is used.

[codecov]

Codecov Report

Patch coverage: 41.10% and project coverage change: -0.81% ⚠️

Comparison is base (4c3c49d) 71.08% compared to head (75169e5) 70.27%.

❗ Current head 75169e5 differs from pull request most recent head 449fa24. Consider uploading reports for the commit 449fa24 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #653      +/-   ##
==========================================
- Coverage   71.08%   70.27%   -0.81%     
==========================================
  Files          47       48       +1     
  Lines        9010     9239     +229     
==========================================
+ Hits         6405     6493      +88     
- Misses       2605     2746     +141     

Files Changed	Coverage Δ
utils/cli/cmd-delay-adjustment.c	0.00% <0.00%> (ø)
src/shared/dbus-client.c	66.32% <12.50%> (-1.97%)	⬇️
src/bluealsa-dbus.c	65.06% <13.63%> (-3.57%)	⬇️
src/asound/bluealsa-ctl.c	75.31% <45.61%> (-1.84%)	⬇️
src/storage.c	83.95% <83.72%> (-0.09%)	⬇️
src/ba-transport.c	79.79% <100.00%> (+0.48%)	⬆️
utils/cli/cli.c	77.55% <100.00%> (+0.09%)	⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[borine]

I've had second thoughts about the non-blocking D-Bus calls, because amixer does not wait for the update to complete and therefore does not report the updated value. So I've now reverted that commit and instead invoke the D-Bus event handling code explicitly after codec and sync updates. This approach seems to keep both amixer and alsamixer happy.

I plan to rebase against master this weekend to pull in the revised EXT argument code, and at the same same squash some of these commits to make each logical change a single commit. I will then mark this PR as ready for review.

BTW I've been searching through dictionaries and thesauruses to find better names for the delay adjustment control, but given there is only room for 4 characters I still haven't found anything better than "Sync".

[arkq]

BTW I've been searching through dictionaries and thesauruses to find better names for the delay adjustment control, but given there is only room for 4 characters I still haven't found anything better than "Sync".

OK, I think that we can go with "Sync" then. For audio only usage it's a little bit cryptic name, but since this control element will not be visible by default it's OK. And for A/V usage, I guess that someone who will need to adjust sync issue the "Sync" word in the manual/wiki will help to find the answer :)

[arkq]

I've just reviewed commits for CTL plugin and for bluealsa-cli, and they look good. I've fixed some typos and reordered sync and battery (battery is the least related with audio, so I'd like to place it as the last item) and force pushed the changes (to avoid conflicts in the nearest future).

Now, I need to find some free time to review the core part of this PR...
However, already I've spotted one thing that I'd like to change. The ba_transport_pcm_delay_adjustment_set should get the codec_id not a string representation. It will be consistent with the rest of the internal API.

[arkq]

I think that the core change looks good as well!

[arkq]

I've just white-reviewed the code and it looks OK.

I've added InvalidArguments error to the SetDelayAdjustment, so now the call like this will return proper D-Bus error:

gdbus call --system --dest org.bluealsa \
   --object-path /org/bluealsa/hci0/dev_04_5D_4B_ED_D7_00/a2dpsrc/sink \
   --method org.bluealsa.PCM1.SetDelayAdjustment xxx 33

I will perform few runtime tests and I think that we are ready to go :)

Also, I like very much the idea of "mutable" from C++! I'll try to adopt that in the whole project. Maybe with a bit different semantic. I thought about adding define just for mutexes (I don't see any other ba-structs member that can be "mutable"), something like #define MUTABLE_MUTEX(mtx) ....