[PATCH v4 0/3] cec: add tests to Audio Rate Control

[PATCH v4 0/3] cec: add tests to Audio Rate Control

[Deborah Brouwer]

Update the Set Audio Rate test to include the Audio System source mask.
Add an active sensing test.
Add an invalid operand test.

Changes since v3:
* Patch 1: cec-compliance: add Audio System mask to Set Audio Rate
	* New patch

* Patch 2: cec: add active sensing test for Audio Rate Control
	* Remove comments indicating basic and rudimentary status of test.
	* Add Audio System source mask to active sensing test.

* Patch 3: cec: add invalid operand test for Audio Rate Control messages
	* Replace "parameter" with "operand" in name, commit msg and comment.
	* Add Audio System source mask to invalid operand test.

Changes since v2:
* Patch 1: cec: add active sensing test for Audio Rate Control
	* No changes.

* Patch 2: cec: add invalid parameter test for Audio Rate Control
	* Add indent to correct formatting error.
	* Change invalid parameter from -1 to 0xa.
	* Add check that message has not timed_out.
	* Add check that cec_msg_status_is_abort.
	* Add break at end of default case statement.

Changes since v1:
* Patch 1: cec: add active sensing test for Audio Rate Control
	* remove or add spaces to conform to kernel coding style.
	* add empty line to improve readability.
	* add and clarify comments; remove TODO comment.
	* change commit subject to refer to "active sensing".
	* rename function to audio_rate_ctl_active_sensing.
	* rename node state to last_aud_rate_rx_ts.
	* make follower warning more specific.
	* change control flow in cec-processing to avoid extra indent.

Deborah Brouwer (3):
  cec-compliance: add Audio System mask to Set Audio Rate
  cec: add active sensing test for Audio Rate Control messages
  cec: add invalid operand test for Audio Rate Control messages

 utils/cec-compliance/cec-test-audio.cpp | 62 +++++++++++++++++++++++--
 utils/cec-follower/cec-follower.cpp     |  1 +
 utils/cec-follower/cec-follower.h       |  1 +
 utils/cec-follower/cec-processing.cpp   | 50 ++++++++++++++++----
 4 files changed, 100 insertions(+), 14 deletions(-)

--
2.17.1

[PATCH v4 1/3] cec-compliance: add Audio System mask to Set Audio Rate

[Deborah Brouwer]

An Audio System can support Audio Rate Control, so add the
Audio System as a possible source in the Set Audio Rate test.

Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
 utils/cec-compliance/cec-test-audio.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/utils/cec-compliance/cec-test-audio.cpp b/utils/cec-compliance/cec-test-audio.cpp
index 2a541d55..4674295b 100644
--- a/utils/cec-compliance/cec-test-audio.cpp
+++ b/utils/cec-compliance/cec-test-audio.cpp
@@ -878,7 +878,8 @@ static int audio_rate_ctl_set_audio_rate(struct node *node, unsigned me, unsigne
 const vec_remote_subtests audio_rate_ctl_subtests{
 	{
 		"Set Audio Rate",
-		CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_TUNER,
+		CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD |
+		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
 		audio_rate_ctl_set_audio_rate,
 	},
 };
-- 
2.17.1

[PATCH v4 2/3] cec: add active sensing test for Audio Rate Control messages

[Deborah Brouwer]

The controlling device should send an Audio Rate Control message at least
once every 2 seconds for active sensing. Add a test in cec-compliance to
delay the message by more than 2 seconds. Check the interval between
messages in cec-follower and warn if the delay is greater than 2 seconds.

Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
 utils/cec-compliance/cec-test-audio.cpp | 34 +++++++++++++++---
 utils/cec-follower/cec-follower.cpp     |  1 +
 utils/cec-follower/cec-follower.h       |  1 +
 utils/cec-follower/cec-processing.cpp   | 46 ++++++++++++++++++++-----
 4 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/utils/cec-compliance/cec-test-audio.cpp b/utils/cec-compliance/cec-test-audio.cpp
index 4674295b..e1386dbb 100644
--- a/utils/cec-compliance/cec-test-audio.cpp
+++ b/utils/cec-compliance/cec-test-audio.cpp
@@ -851,10 +851,6 @@ const vec_remote_subtests sac_subtests{
 
 /* Audio Rate Control */
 
-/*
-  TODO: These are very rudimentary tests which should be expanded.
- */
-
 static int audio_rate_ctl_set_audio_rate(struct node *node, unsigned me, unsigned la, bool interactive)
 {
 	struct cec_msg msg = {};
@@ -875,6 +871,30 @@ static int audio_rate_ctl_set_audio_rate(struct node *node, unsigned me, unsigne
 	return OK_PRESUMED;
 }
 
+static int audio_rate_ctl_active_sensing(struct node *node, unsigned me, unsigned la, bool interactive)
+{
+	/*
+	 * The source shall go back to Rate Control Off if no Set Audio Rate message is
+	 * received for more than 2 seconds.
+	 */
+	if (!node->remote[la].has_aud_rate)
+		return NOTAPPLICABLE;
+
+	struct cec_msg msg = {};
+
+	cec_msg_init(&msg, me, la);
+
+	/*
+	 * Since this subtest runs immediately after Set Audio Rate, delaying the interval
+	 * between the two tests is sufficient to test that the Source turns off rate control.
+	 */
+	sleep(3);
+	cec_msg_set_audio_rate(&msg, CEC_OP_AUD_RATE_OFF);
+	fail_on_test(!transmit_timeout(node, &msg));
+	fail_on_test_v2(node->remote[la].cec_version, unrecognized_op(&msg));
+	return OK_PRESUMED;
+}
+
 const vec_remote_subtests audio_rate_ctl_subtests{
 	{
 		"Set Audio Rate",
@@ -882,4 +902,10 @@ const vec_remote_subtests audio_rate_ctl_subtests{
 		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
 		audio_rate_ctl_set_audio_rate,
 	},
+	{
+		"Audio Rate Active Sensing",
+		CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD |
+		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
+		audio_rate_ctl_active_sensing,
+	},
 };
diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index bb63f90d..184cf16a 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -314,6 +314,7 @@ void state_init(struct node &node)
 	node.state.volume = 50;
 	node.state.mute = false;
 	tuner_dev_info_init(&node.state);
+	node.state.last_aud_rate_rx_ts = 0;
 }
 
 int main(int argc, char **argv)
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 7806a4b6..391b9ab4 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -51,6 +51,7 @@ struct state {
 	bool tuner_report_changes;
 	bool deck_report_changes;
 	unsigned toggle_power_status;
+	__u64 last_aud_rate_rx_ts;
 };
 
 struct node {
diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
index 02440747..fc0d5df0 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -29,6 +29,9 @@
 /* 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
+
 struct cec_enum_values {
 	const char *type_name;
 	__u8 value;
@@ -230,6 +233,22 @@ static __u8 current_power_state(struct node *node)
 	return CEC_OP_POWER_STATUS_TO_STANDBY;
 }
 
+static void aud_rate_msg_interval_check(__u64 ts_new, __u64 ts_old)
+{
+	/*
+	 * The interval between messages is not relevant if this is the
+	 * first audio rate control message or if the previous message
+	 * turned off the audio rate control.
+	 */
+	if (ts_old) {
+		unsigned interval = (ts_new - ts_old) / 1000000000;
+		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");
+		}
+	}
+}
+
 static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
 {
 	__u8 to = cec_msg_destination(&msg);
@@ -775,18 +794,27 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
 		return;
 
 
-		/*
-		  Audio Rate Control
-
-		  This is only a basic implementation.
-
-		  TODO: Set Audio Rate shall be sent at least every 2 seconds by
-		  the controlling device. This should be checked and kept track of.
-		*/
+		/* Audio Rate Control */
 
 	case CEC_MSG_SET_AUDIO_RATE:
-		if (node->has_aud_rate)
+		if (!node->has_aud_rate)
+			break;
+
+		switch (msg.msg[2]) {
+		case CEC_OP_AUD_RATE_OFF:
+			aud_rate_msg_interval_check(msg.rx_ts, node->state.last_aud_rate_rx_ts);
+			node->state.last_aud_rate_rx_ts = 0;
+			return;
+		case CEC_OP_AUD_RATE_WIDE_STD:
+		case CEC_OP_AUD_RATE_WIDE_FAST:
+		case CEC_OP_AUD_RATE_WIDE_SLOW:
+		case CEC_OP_AUD_RATE_NARROW_STD:
+		case CEC_OP_AUD_RATE_NARROW_FAST:
+		case CEC_OP_AUD_RATE_NARROW_SLOW:
+			aud_rate_msg_interval_check(msg.rx_ts, node->state.last_aud_rate_rx_ts);
+			node->state.last_aud_rate_rx_ts = msg.rx_ts;
 			return;
+		}
 		break;
 
 
-- 
2.17.1

Re: [PATCH v4 2/3] cec: add active sensing test for Audio Rate Control messages

[Hans Verkuil]

On 22/04/2021 05:27, Deborah Brouwer wrote:
> The controlling device should send an Audio Rate Control message at least
> once every 2 seconds for active sensing. Add a test in cec-compliance to
> delay the message by more than 2 seconds. Check the interval between
> messages in cec-follower and warn if the delay is greater than 2 seconds.
> 
> Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
> ---
>  utils/cec-compliance/cec-test-audio.cpp | 34 +++++++++++++++---
>  utils/cec-follower/cec-follower.cpp     |  1 +
>  utils/cec-follower/cec-follower.h       |  1 +
>  utils/cec-follower/cec-processing.cpp   | 46 ++++++++++++++++++++-----
>  4 files changed, 69 insertions(+), 13 deletions(-)
> 
> diff --git a/utils/cec-compliance/cec-test-audio.cpp b/utils/cec-compliance/cec-test-audio.cpp
> index 4674295b..e1386dbb 100644
> --- a/utils/cec-compliance/cec-test-audio.cpp
> +++ b/utils/cec-compliance/cec-test-audio.cpp
> @@ -851,10 +851,6 @@ const vec_remote_subtests sac_subtests{
>  
>  /* Audio Rate Control */
>  
> -/*
> -  TODO: These are very rudimentary tests which should be expanded.
> - */
> -
>  static int audio_rate_ctl_set_audio_rate(struct node *node, unsigned me, unsigned la, bool interactive)
>  {
>  	struct cec_msg msg = {};
> @@ -875,6 +871,30 @@ static int audio_rate_ctl_set_audio_rate(struct node *node, unsigned me, unsigne
>  	return OK_PRESUMED;
>  }
>  
> +static int audio_rate_ctl_active_sensing(struct node *node, unsigned me, unsigned la, bool interactive)
> +{
> +	/*
> +	 * The source shall go back to Rate Control Off if no Set Audio Rate message is
> +	 * received for more than 2 seconds.
> +	 */
> +	if (!node->remote[la].has_aud_rate)
> +		return NOTAPPLICABLE;
> +
> +	struct cec_msg msg = {};
> +
> +	cec_msg_init(&msg, me, la);
> +
> +	/*
> +	 * Since this subtest runs immediately after Set Audio Rate, delaying the interval
> +	 * between the two tests is sufficient to test that the Source turns off rate control.
> +	 */
> +	sleep(3);
> +	cec_msg_set_audio_rate(&msg, CEC_OP_AUD_RATE_OFF);
> +	fail_on_test(!transmit_timeout(node, &msg));
> +	fail_on_test_v2(node->remote[la].cec_version, unrecognized_op(&msg));
> +	return OK_PRESUMED;
> +}
> +
>  const vec_remote_subtests audio_rate_ctl_subtests{
>  	{
>  		"Set Audio Rate",
> @@ -882,4 +902,10 @@ const vec_remote_subtests audio_rate_ctl_subtests{
>  		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
>  		audio_rate_ctl_set_audio_rate,
>  	},
> +	{
> +		"Audio Rate Active Sensing",
> +		CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD |
> +		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
> +		audio_rate_ctl_active_sensing,
> +	},
>  };
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index bb63f90d..184cf16a 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -314,6 +314,7 @@ void state_init(struct node &node)
>  	node.state.volume = 50;
>  	node.state.mute = false;
>  	tuner_dev_info_init(&node.state);
> +	node.state.last_aud_rate_rx_ts = 0;
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 7806a4b6..391b9ab4 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -51,6 +51,7 @@ struct state {
>  	bool tuner_report_changes;
>  	bool deck_report_changes;
>  	unsigned toggle_power_status;
> +	__u64 last_aud_rate_rx_ts;
>  };
>  
>  struct node {
> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
> index 02440747..fc0d5df0 100644
> --- a/utils/cec-follower/cec-processing.cpp
> +++ b/utils/cec-follower/cec-processing.cpp
> @@ -29,6 +29,9 @@
>  /* 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
> +
>  struct cec_enum_values {
>  	const char *type_name;
>  	__u8 value;
> @@ -230,6 +233,22 @@ static __u8 current_power_state(struct node *node)
>  	return CEC_OP_POWER_STATUS_TO_STANDBY;
>  }
>  
> +static void aud_rate_msg_interval_check(__u64 ts_new, __u64 ts_old)
> +{
> +	/*
> +	 * The interval between messages is not relevant if this is the
> +	 * first audio rate control message or if the previous message
> +	 * turned off the audio rate control.
> +	 */
> +	if (ts_old) {
> +		unsigned interval = (ts_new - ts_old) / 1000000000;
> +		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");
> +		}
> +	}
> +}
> +
>  static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>  {
>  	__u8 to = cec_msg_destination(&msg);
> @@ -775,18 +794,27 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
>  		return;
>  
>  
> -		/*
> -		  Audio Rate Control
> -
> -		  This is only a basic implementation.
> -
> -		  TODO: Set Audio Rate shall be sent at least every 2 seconds by
> -		  the controlling device. This should be checked and kept track of.
> -		*/
> +		/* Audio Rate Control */
>  
>  	case CEC_MSG_SET_AUDIO_RATE:
> -		if (node->has_aud_rate)
> +		if (!node->has_aud_rate)
> +			break;
> +
> +		switch (msg.msg[2]) {
> +		case CEC_OP_AUD_RATE_OFF:
> +			aud_rate_msg_interval_check(msg.rx_ts, node->state.last_aud_rate_rx_ts);
> +			node->state.last_aud_rate_rx_ts = 0;
> +			return;
> +		case CEC_OP_AUD_RATE_WIDE_STD:
> +		case CEC_OP_AUD_RATE_WIDE_FAST:
> +		case CEC_OP_AUD_RATE_WIDE_SLOW:
> +		case CEC_OP_AUD_RATE_NARROW_STD:
> +		case CEC_OP_AUD_RATE_NARROW_FAST:
> +		case CEC_OP_AUD_RATE_NARROW_SLOW:
> +			aud_rate_msg_interval_check(msg.rx_ts, node->state.last_aud_rate_rx_ts);
> +			node->state.last_aud_rate_rx_ts = msg.rx_ts;

Note that this only checks the interval if the initiator transmits two
CEC_MSG_SET_AUDIO_RATE messages with more than 2 seconds between them.

This doesn't address the case where the initiator just stops sending them
altogether.

So you need to add another check in testProcessing where you verify this
as well (at the end of the 'while (true)' loop).

Regards,

	Hans

>  			return;
> +		}
>  		break;
>  
>  
> 

[PATCH v4 3/3] cec: add invalid operand test for Audio Rate Control messages

[Deborah Brouwer]

Add a test in cec-compliance that sends an Audio Rate Control message
with an invalid operand. Check that it receives a Feature Abort reply
due to the invalid operand. Add a response in cec-follower to Feature
Abort due to an invalid operand.

Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
 utils/cec-compliance/cec-test-audio.cpp | 25 +++++++++++++++++++++++++
 utils/cec-follower/cec-processing.cpp   |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/utils/cec-compliance/cec-test-audio.cpp b/utils/cec-compliance/cec-test-audio.cpp
index e1386dbb..bdbcd178 100644
--- a/utils/cec-compliance/cec-test-audio.cpp
+++ b/utils/cec-compliance/cec-test-audio.cpp
@@ -895,6 +895,25 @@ static int audio_rate_ctl_active_sensing(struct node *node, unsigned me, unsigne
 	return OK_PRESUMED;
 }
 
+static int audio_rate_ctl_invalid(struct node *node, unsigned me, unsigned la, bool interactive)
+{
+	if (!node->remote[la].has_aud_rate)
+		return NOTAPPLICABLE;
+
+	struct cec_msg msg = {};
+
+	cec_msg_init(&msg, me, la);
+	cec_msg_set_audio_rate(&msg, 0xa); /* Invalid Audio Rate Control message operand */
+	fail_on_test(!transmit_timeout(node, &msg));
+	fail_on_test(timed_out(&msg));
+	fail_on_test(!cec_msg_status_is_abort(&msg));
+	if (abort_reason(&msg) != CEC_OP_ABORT_INVALID_OP) {
+		warn("Expected Feature Abort [Invalid operand]\n");
+		return FAIL;
+	}
+	return OK;
+}
+
 const vec_remote_subtests audio_rate_ctl_subtests{
 	{
 		"Set Audio Rate",
@@ -908,4 +927,10 @@ const vec_remote_subtests audio_rate_ctl_subtests{
 		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
 		audio_rate_ctl_active_sensing,
 	},
+	{
+		"Audio Rate Invalid Operand",
+		CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD |
+		CEC_LOG_ADDR_MASK_TUNER | CEC_LOG_ADDR_MASK_AUDIOSYSTEM,
+		audio_rate_ctl_invalid,
+	},
 };
diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
index fc0d5df0..93db4059 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -814,6 +814,10 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
 			aud_rate_msg_interval_check(msg.rx_ts, node->state.last_aud_rate_rx_ts);
 			node->state.last_aud_rate_rx_ts = msg.rx_ts;
 			return;
+		default:
+			cec_msg_reply_feature_abort(&msg, CEC_OP_ABORT_INVALID_OP);
+			transmit(node, &msg);
+			break;
 		}
 		break;
 
-- 
2.17.1

Re: [PATCH v4 0/3] cec: add tests to Audio Rate Control

[Hans Verkuil]

Hi Deb,

On 22/04/2021 05:27, Deborah Brouwer wrote:
> Update the Set Audio Rate test to include the Audio System source mask.
> Add an active sensing test.
> Add an invalid operand test.

Thank you for this patch series, it's been applied so you can mark this
Outreachy contribution as Accepted!

I have some follow-up comments for refinements (see my upcoming replies
to the corresponding patches), but those can be addressed in new patches.

Regards,

	Hans

> 
> Changes since v3:
> * Patch 1: cec-compliance: add Audio System mask to Set Audio Rate
> 	* New patch
> 
> * Patch 2: cec: add active sensing test for Audio Rate Control
> 	* Remove comments indicating basic and rudimentary status of test.
> 	* Add Audio System source mask to active sensing test.
> 
> * Patch 3: cec: add invalid operand test for Audio Rate Control messages
> 	* Replace "parameter" with "operand" in name, commit msg and comment.
> 	* Add Audio System source mask to invalid operand test.
> 
> Changes since v2:
> * Patch 1: cec: add active sensing test for Audio Rate Control
> 	* No changes.
> 
> * Patch 2: cec: add invalid parameter test for Audio Rate Control
> 	* Add indent to correct formatting error.
> 	* Change invalid parameter from -1 to 0xa.
> 	* Add check that message has not timed_out.
> 	* Add check that cec_msg_status_is_abort.
> 	* Add break at end of default case statement.
> 
> Changes since v1:
> * Patch 1: cec: add active sensing test for Audio Rate Control
> 	* remove or add spaces to conform to kernel coding style.
> 	* add empty line to improve readability.
> 	* add and clarify comments; remove TODO comment.
> 	* change commit subject to refer to "active sensing".
> 	* rename function to audio_rate_ctl_active_sensing.
> 	* rename node state to last_aud_rate_rx_ts.
> 	* make follower warning more specific.
> 	* change control flow in cec-processing to avoid extra indent.
> 
> Deborah Brouwer (3):
>   cec-compliance: add Audio System mask to Set Audio Rate
>   cec: add active sensing test for Audio Rate Control messages
>   cec: add invalid operand test for Audio Rate Control messages
> 
>  utils/cec-compliance/cec-test-audio.cpp | 62 +++++++++++++++++++++++--
>  utils/cec-follower/cec-follower.cpp     |  1 +
>  utils/cec-follower/cec-follower.h       |  1 +
>  utils/cec-follower/cec-processing.cpp   | 50 ++++++++++++++++----
>  4 files changed, 100 insertions(+), 14 deletions(-)
> 
> --
> 2.17.1
> 

Re: [PATCH v4 0/3] cec: add tests to Audio Rate Control

[Hans Verkuil]

On 22/04/2021 09:02, Hans Verkuil wrote:
> Hi Deb,
> 
> On 22/04/2021 05:27, Deborah Brouwer wrote:
>> Update the Set Audio Rate test to include the Audio System source mask.
>> Add an active sensing test.
>> Add an invalid operand test.
> 
> Thank you for this patch series, it's been applied so you can mark this
> Outreachy contribution as Accepted!
> 
> I have some follow-up comments for refinements (see my upcoming replies
> to the corresponding patches), but those can be addressed in new patches.

Just one follow-up comment, to be precise. See my reply to patch 2/3.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>>
>> Changes since v3:
>> * Patch 1: cec-compliance: add Audio System mask to Set Audio Rate
>> 	* New patch
>>
>> * Patch 2: cec: add active sensing test for Audio Rate Control
>> 	* Remove comments indicating basic and rudimentary status of test.
>> 	* Add Audio System source mask to active sensing test.
>>
>> * Patch 3: cec: add invalid operand test for Audio Rate Control messages
>> 	* Replace "parameter" with "operand" in name, commit msg and comment.
>> 	* Add Audio System source mask to invalid operand test.
>>
>> Changes since v2:
>> * Patch 1: cec: add active sensing test for Audio Rate Control
>> 	* No changes.
>>
>> * Patch 2: cec: add invalid parameter test for Audio Rate Control
>> 	* Add indent to correct formatting error.
>> 	* Change invalid parameter from -1 to 0xa.
>> 	* Add check that message has not timed_out.
>> 	* Add check that cec_msg_status_is_abort.
>> 	* Add break at end of default case statement.
>>
>> Changes since v1:
>> * Patch 1: cec: add active sensing test for Audio Rate Control
>> 	* remove or add spaces to conform to kernel coding style.
>> 	* add empty line to improve readability.
>> 	* add and clarify comments; remove TODO comment.
>> 	* change commit subject to refer to "active sensing".
>> 	* rename function to audio_rate_ctl_active_sensing.
>> 	* rename node state to last_aud_rate_rx_ts.
>> 	* make follower warning more specific.
>> 	* change control flow in cec-processing to avoid extra indent.
>>
>> Deborah Brouwer (3):
>>   cec-compliance: add Audio System mask to Set Audio Rate
>>   cec: add active sensing test for Audio Rate Control messages
>>   cec: add invalid operand test for Audio Rate Control messages
>>
>>  utils/cec-compliance/cec-test-audio.cpp | 62 +++++++++++++++++++++++--
>>  utils/cec-follower/cec-follower.cpp     |  1 +
>>  utils/cec-follower/cec-follower.h       |  1 +
>>  utils/cec-follower/cec-processing.cpp   | 50 ++++++++++++++++----
>>  4 files changed, 100 insertions(+), 14 deletions(-)
>>
>> --
>> 2.17.1
>>
>