-
Notifications
You must be signed in to change notification settings - Fork 189
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
Enable dbus delay signals for internal processing delay. #734
base: master
Are you sure you want to change the base?
Conversation
The `bluealsad` daemon currently keeps track of its own internal processing delay, but only presents that to a client when the 'Delay' property is explicitly queried by the client. This means that any client which relies on D-Bus signals to obtain Delay updates may never see this internal processing delay (for example the `bluealsa` ALSA plugin). The purpose of this commit is to have `bluealsad` emit a D-Bus PropertiesChanged signal whenever a "significant" change to the internal processing delay occurs. Not every change is reported because that might generate thousands of signals per second.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #734 +/- ##
==========================================
+ Coverage 70.30% 70.44% +0.14%
==========================================
Files 96 96
Lines 15799 15967 +168
Branches 2502 2504 +2
==========================================
+ Hits 11107 11248 +141
- Misses 4575 4602 +27
Partials 117 117 ☔ View full report in Codecov by Sentry. |
It was omitted due to lack of time to implement that :D Many thanks for that!
Yes, it should be safe. Internally signal is just a D-Bus message, and messages are sent by
I'm going to use such delay for the client-delay signaling as well and it's also arbitrary decision (based on commersial headset delay report updates). However, during testing, I've also added time-rate limiting logic (https://github.com/arkq/bluez-alsa/pull/516/files#diff-8bfb2124e7db1e5681c384f69b807d2a22821e0db287cab13d9ea0eb000cff81R827) because during playback start I could unnecessarily flood other device with lots of delay reports (the 1s rate limit is also based on some commercial headset). It's implemented on the client side, but maybe it should be moved to the bluealsad... I'll have to check how this delay report signaling will work with your PR. I hope that the client-delay will be merged soon (after support on BlueZ side will fully land in upstream). |
@@ -419,7 +419,7 @@ void *a2dp_aac_enc_thread(struct ba_transport_pcm *t_pcm) { | |||
rtp_state_update(&rtp, pcm_frames); | |||
|
|||
/* update busy delay (encoding overhead) */ | |||
t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100; | |||
ba_transport_pcm_update_processing_delay(t_pcm, asrsync_get_busy_usec(&io.asrs) / 100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client delay support landed in master (if you are using BlueALSA as a speaker system for watching videos, maybe you can test it as well; I've tested it to some extend and it seems to work quite nice with BlueALSA<->BlueALSA setup, but I wonder how it will work with smartphones or other devices, unfortunately my Android does not support delay reporting... :/), so here instead of ba_transport_pcm_update_processing_delay
I would use
t_pcm->processing_delay_dms = asrsync_get_busy_usec(&io.asrs) / 100;
ba_transport_pcm_delay_sync(t_pcm, BA_DBUS_PCM_UPDATE_DELAY);
so in case of decoder thread, the remote Bluetooth device will be notified (if it supports delay reporting). And of course move threshold logic to ba_transport_pcm_delay_sync()
. I hope that it will work that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebuilt bluez and bluez-alsa on my test pi zero w, both from their master branches, and am testing with bluez-alsa master on my laptop with firefox, and the audio sync is really excellent - no bluealsa delay adjustment applied on either device.
I will build bluez-alsa again from this branch and try again, probably at the weekend, although I think this branch is only useful when the sink does not support delay reporting, because otherwise Delay change signals from the sink will cause the source client to pick up the internal processing delay anyway.
I've raised this as a draft PR for discussion because: