mbox series

[v2,0/2] cec-follower: increase accuracy Audio Rate Control

Message ID cover.1619389023.git.deborahbrouwer3563@gmail.com
Headers show
Series cec-follower: increase accuracy Audio Rate Control | expand

Message

Deborah Brouwer April 25, 2021, 10:54 p.m. UTC 
Changes since v1:
* Patch 1:
  cec-follower: increase precision of Audio Rate Control active sensing
	* new patch

* Patch 2:
  cec-follower: detect the cessation of Audio Rate Control messages
	* Keep the aud_rate_msg_interval_check in processMsg to avoid missing
	  late messages that arrive between periodic interval checks.
	* Swap arguments so that struct node appears first.
	* Add blank line for readability.

Deborah Brouwer (2):
  cec-follower: increase precision of Audio Rate Control active sensing
  cec-follower: detect the cessation of Audio Rate Control messages

 utils/cec-follower/cec-processing.cpp | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

--
2.17.1

Comments

Hans Verkuil April 26, 2021, 6:33 a.m. UTC | #1 
On 26/04/2021 00:54, Deborah Brouwer wrote:
> Measure the interval since the last audio rate control message in

> nanoseconds instead of seconds. Increasing the precision catches audio

> rate messages that are late by less than a second.

> 

> Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>

> ---

>  utils/cec-follower/cec-processing.cpp | 7 ++++---

>  1 file changed, 4 insertions(+), 3 deletions(-)

> 

> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp

> index 93db4059..243c9d09 100644

> --- a/utils/cec-follower/cec-processing.cpp

> +++ b/utils/cec-follower/cec-processing.cpp

> @@ -29,8 +29,8 @@

>  /* Time between each polling message sent to a device */

>  #define POLL_PERIOD 15000

>  

> -/* The maximum interval in seconds between audio rate messages as defined in the spec */

> -#define MAX_AUD_RATE_MSG_INTERVAL 2

> +/* The maximum interval in nanoseconds between audio rate messages as defined in the spec */

> +#define MAX_AUD_RATE_MSG_INTERVAL 2000000000


It's a bit easier to read if you write this as:

#define MAX_AUD_RATE_MSG_INTERVAL_NS (2 * 1000000000ULL)

It's helpful to add the unit as a suffix to the define name and to
write it as a multiplication of seconds times nsecs_per_sec.

Also ULL helps cast the expression to an unsigned long long (64 bit).

Not strictly necessary, but since we check intervals using __u64 it
doesn't hurt either.

Regards,

	Hans

>  

>  struct cec_enum_values {

>  	const char *type_name;

> @@ -241,7 +241,8 @@ static void aud_rate_msg_interval_check(__u64 ts_new, __u64 ts_old)

>  	 * turned off the audio rate control.

>  	 */

>  	if (ts_old) {

> -		unsigned interval = (ts_new - ts_old) / 1000000000;

> +		__u64 interval = ts_new - ts_old;

> +

>  		if (interval > MAX_AUD_RATE_MSG_INTERVAL) {

>  			warn("The interval between Audio Rate Control messages was greater\n");

>  			warn("than the Maxiumum Audio Rate Message Interval (2s).\n");

>