* [PATCH v3 0/3] cec: One Touch Record tests
@ 2021-06-22 4:35 Deborah Brouwer
2021-06-22 4:35 ` [PATCH v3 1/3] cec: expand One Touch Record TV Screen test Deborah Brouwer
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Deborah Brouwer @ 2021-06-22 4:35 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, jaffe1, Deborah Brouwer
This is part of an Outreachy project to expand the testing of
One Touch Record messages as handled by CEC adapters.
Changes since v2:
Patch 1/3 cec: expand One Touch Record TV Screen test:
- replace numbers with corresponding defines
Patch 2/3 cec: expand One Touch Record On/Off tests
- rename commit to reflect expanded scope of tests
- increase msg timeout for reply to 10s
- rename helper function and invert return value
- use primary device type to identify remote follower
- use logical address type to identify remote initiator
- limit range of accepted external plug numbers to 6
- disallow external physical address source
- keep track of whether the device is recording
- add additional invalid tests
Patch 3/3 cec: add One Touch Record Standby tests
- new patch
Changes since v1:
Patch 1/2 cec: expand One Touch Record TV Screen test:
- add space after 'switch'
- add "return" before fail
- check analog broadcast type and broadcast system operand
- add a comment when follower ignores message
Patch 2/2 cec: expand One Touch Record On test
- new patch
Deborah Brouwer (3):
cec: expand One Touch Record TV Screen test
cec: expand One Touch Record On/Off tests
cec: add One Touch Record Standby tests
utils/cec-compliance/cec-compliance.h | 5 +
utils/cec-compliance/cec-test-power.cpp | 63 +++++++
utils/cec-compliance/cec-test.cpp | 211 ++++++++++++++++++++++--
utils/cec-follower/cec-follower.cpp | 2 +
utils/cec-follower/cec-follower.h | 5 +-
utils/cec-follower/cec-processing.cpp | 14 +-
utils/cec-follower/cec-tuner.cpp | 119 ++++++++++++-
7 files changed, 388 insertions(+), 31 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] cec: expand One Touch Record TV Screen test
2021-06-22 4:35 [PATCH v3 0/3] cec: One Touch Record tests Deborah Brouwer
@ 2021-06-22 4:35 ` Deborah Brouwer
2021-06-22 15:06 ` Hans Verkuil
2021-06-22 4:36 ` [PATCH v3 2/3] cec: expand One Touch Record On/Off tests Deborah Brouwer
2021-06-22 4:36 ` [PATCH v3 3/3] cec: add One Touch Record Standby tests Deborah Brouwer
2 siblings, 1 reply; 6+ messages in thread
From: Deborah Brouwer @ 2021-06-22 4:35 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, jaffe1, Deborah Brouwer
Check that the follower ignores the Record TV Screen message if the
initiator has a logical address other than Record or Backup (aka Reserved
in CEC Version < 2.0). If the follower replies correctly, check that the
source operand is valid.
Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
utils/cec-compliance/cec-test.cpp | 54 ++++++++++++++++++++++++++-----
utils/cec-follower/cec-tuner.cpp | 9 ++++--
2 files changed, 52 insertions(+), 11 deletions(-)
diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
index 40d8369d..2ea1b712 100644
--- a/utils/cec-compliance/cec-test.cpp
+++ b/utils/cec-compliance/cec-test.cpp
@@ -1150,13 +1150,6 @@ static const vec_remote_subtests tuner_ctl_subtests{
static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la, bool interactive)
{
- /*
- TODO:
- - Page 36 in HDMI CEC 1.4b spec lists additional behaviors that should be
- checked for.
- - The TV should ignore this message when received from other LA than Recording or
- Reserved.
- */
struct cec_msg msg;
cec_msg_init(&msg, me, la);
@@ -1172,8 +1165,53 @@ static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la,
return OK_REFUSED;
if (cec_msg_status_is_abort(&msg))
return OK_PRESUMED;
+ /*
+ * Follower should ignore this message if initiator has a logical
+ * address other than Record or Backup (aka "Reserved" in CEC Version < 2.0).
+ */
+ if (!cec_has_record(1 << me) && !cec_has_backup(1 << me)) {
+ fail_on_test(!timed_out(&msg));
+ return OK;
+ }
+ fail_on_test(timed_out(&msg));
- return 0;
+ struct cec_op_record_src rec_src = {};
+
+ cec_ops_record_on(&msg, &rec_src);
+
+ if (rec_src.type < CEC_OP_RECORD_SRC_OWN || rec_src.type > CEC_OP_RECORD_SRC_EXT_PHYS_ADDR)
+ return fail("Invalid source.\n");
+
+ if (rec_src.type == CEC_OP_RECORD_SRC_DIGITAL &&
+ rec_src.digital.service_id_method == CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID) {
+
+ switch (rec_src.digital.dig_bcast_system) {
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_CS:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_CABLE:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
+ break;
+ default:
+ return fail("Invalid digital service broadcast system operand.\n");
+ }
+ }
+
+ if (rec_src.type == CEC_OP_RECORD_SRC_ANALOG) {
+ fail_on_test(rec_src.analog.ana_bcast_type > CEC_OP_ANA_BCAST_TYPE_TERRESTRIAL);
+ fail_on_test(rec_src.analog.bcast_system > CEC_OP_BCAST_SYSTEM_PAL_DK &&
+ rec_src.analog.bcast_system != CEC_OP_BCAST_SYSTEM_OTHER);
+ }
+
+ return OK;
}
static int one_touch_rec_on(struct node *node, unsigned me, unsigned la, bool interactive)
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index b9c21684..fd11bd10 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -583,9 +583,6 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
This is only a basic implementation.
TODO:
- - If we are a TV, we should only send Record On if the
- remote end is a Recording device or Reserved. Otherwise ignore.
-
- Device state should reflect whether we are recording, etc. In
recording mode we should ignore Standby messages.
*/
@@ -594,6 +591,12 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
if (!node->has_rec_tv)
break;
+ __u8 la = cec_msg_initiator(&msg);
+
+ /* Ignore if initiator is not Record or Backup (aka "Reserved" in CEC Version < 2.0) */
+ if (!cec_has_record(1 << la) && !cec_has_backup(1 << la))
+ return;
+
struct cec_op_record_src rec_src = {};
rec_src.type = CEC_OP_RECORD_SRC_OWN;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] cec: expand One Touch Record On/Off tests
2021-06-22 4:35 [PATCH v3 0/3] cec: One Touch Record tests Deborah Brouwer
2021-06-22 4:35 ` [PATCH v3 1/3] cec: expand One Touch Record TV Screen test Deborah Brouwer
@ 2021-06-22 4:36 ` Deborah Brouwer
2021-06-22 15:34 ` Hans Verkuil
2021-06-22 4:36 ` [PATCH v3 3/3] cec: add One Touch Record Standby tests Deborah Brouwer
2 siblings, 1 reply; 6+ messages in thread
From: Deborah Brouwer @ 2021-06-22 4:36 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, jaffe1, Deborah Brouwer
Send all Record On source operands and check that the follower responds
with a corresponding Record Status. Send invalid Record On operands and
check that the follower returns Feature Abort with Invalid Operand or
Record Status indicating why the recording has failed.
Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
utils/cec-compliance/cec-compliance.h | 5 +
utils/cec-compliance/cec-test.cpp | 157 ++++++++++++++++++++++++--
utils/cec-follower/cec-follower.cpp | 1 +
utils/cec-follower/cec-follower.h | 3 +-
utils/cec-follower/cec-processing.cpp | 7 +-
utils/cec-follower/cec-tuner.cpp | 101 ++++++++++++++++-
6 files changed, 255 insertions(+), 19 deletions(-)
diff --git a/utils/cec-compliance/cec-compliance.h b/utils/cec-compliance/cec-compliance.h
index 818181ab..d6c11d70 100644
--- a/utils/cec-compliance/cec-compliance.h
+++ b/utils/cec-compliance/cec-compliance.h
@@ -331,6 +331,11 @@ static inline bool is_playback_or_rec(unsigned la)
return cec_has_playback(1 << la) || cec_has_record(1 << la);
}
+static inline bool is_record(unsigned la, unsigned prim_type)
+{
+ return cec_has_record(1 << la) || (cec_has_backup(1 << la) && prim_type == CEC_OP_PRIM_DEVTYPE_RECORD);
+}
+
static inline bool cec_msg_status_is_abort(const struct cec_msg *msg)
{
return msg->rx_status & CEC_RX_STATUS_FEATURE_ABORT;
diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
index 2ea1b712..4712f993 100644
--- a/utils/cec-compliance/cec-test.cpp
+++ b/utils/cec-compliance/cec-test.cpp
@@ -48,6 +48,45 @@ static int test_play_mode(struct node *node, unsigned me, unsigned la, __u8 play
return OK;
}
+static int one_touch_rec_on_send(struct node *node, unsigned me, unsigned la, const struct cec_op_record_src &rec_src, __u8 &rec_status)
+{
+ struct cec_msg msg;
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_off(&msg, false);
+ fail_on_test(!transmit_timeout(node, &msg));
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_on(&msg, true, &rec_src);
+ fail_on_test(!transmit_timeout(node, &msg, 10000));
+ fail_on_test(timed_out_or_abort(&msg));
+ cec_ops_record_status(&msg, &rec_status);
+
+ return OK;
+}
+
+static bool one_touch_rec_status_not_shared(__u8 rec_status)
+{
+ switch (rec_status) {
+ case CEC_OP_RECORD_STATUS_UNSUP_CA:
+ case CEC_OP_RECORD_STATUS_NO_CA_ENTITLEMENTS:
+ case CEC_OP_RECORD_STATUS_CANT_COPY_SRC:
+ case CEC_OP_RECORD_STATUS_NO_MORE_COPIES:
+ case CEC_OP_RECORD_STATUS_NO_MEDIA:
+ case CEC_OP_RECORD_STATUS_PLAYING:
+ case CEC_OP_RECORD_STATUS_ALREADY_RECORDING:
+ case CEC_OP_RECORD_STATUS_MEDIA_PROT:
+ case CEC_OP_RECORD_STATUS_NO_SIGNAL:
+ case CEC_OP_RECORD_STATUS_MEDIA_PROBLEM:
+ case CEC_OP_RECORD_STATUS_NO_SPACE:
+ case CEC_OP_RECORD_STATUS_PARENTAL_LOCK:
+ case CEC_OP_RECORD_STATUS_OTHER:
+ return false;
+ default:
+ return true;
+ }
+}
+
/* System Information */
int system_info_polling(struct node *node, unsigned me, unsigned la, bool interactive)
@@ -1216,27 +1255,116 @@ static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la,
static int one_touch_rec_on(struct node *node, unsigned me, unsigned la, bool interactive)
{
- /*
- TODO: Page 36 in HDMI CEC 1.4b spec lists additional behaviors that should be
- checked for.
- */
struct cec_msg msg;
struct cec_op_record_src rec_src = {};
rec_src.type = CEC_OP_RECORD_SRC_OWN;
cec_msg_init(&msg, me, la);
cec_msg_record_on(&msg, true, &rec_src);
- fail_on_test(!transmit_timeout(node, &msg));
+ /* Allow 10s for reply because specs say it may take several seconds to accurately respond. */
+ fail_on_test(!transmit_timeout(node, &msg, 10000));
fail_on_test(timed_out(&msg));
- fail_on_test(cec_has_record(1 << la) && unrecognized_op(&msg));
- if (unrecognized_op(&msg))
+ if (unrecognized_op(&msg)) {
+ fail_on_test(is_record(la, node->remote[la].prim_type));
return OK_NOT_SUPPORTED;
+ }
if (refused(&msg))
return OK_REFUSED;
if (cec_msg_status_is_abort(&msg))
return OK_PRESUMED;
- return 0;
+ __u8 rec_status;
+
+ cec_ops_record_status(&msg, &rec_status);
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_CUR_SRC &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ rec_src.type = CEC_OP_RECORD_SRC_DIGITAL;
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_DIG_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_DIG_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ rec_src.type = CEC_OP_RECORD_SRC_ANALOG;
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_ANA_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ rec_src.type = CEC_OP_RECORD_SRC_EXT_PLUG;
+ rec_src.ext_plug.plug = 1;
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_EXT_INPUT &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ memset(&rec_src, 0, sizeof(rec_src));
+ rec_src.type = CEC_OP_RECORD_SRC_EXT_PHYS_ADDR;
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_EXT_INPUT &&
+ rec_status != CEC_OP_RECORD_STATUS_INVALID_EXT_PHYS_ADDR &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ return OK;
+}
+
+static int one_touch_rec_on_invalid(struct node *node, unsigned me, unsigned la, bool interactive)
+{
+ struct cec_msg msg;
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_on_own(&msg);
+ msg.msg[2] = 0; /* Invalid source operand */
+ fail_on_test(!transmit_timeout(node, &msg, 10000));
+ if (unrecognized_op(&msg))
+ return OK_NOT_SUPPORTED;
+ fail_on_test(!cec_msg_status_is_abort(&msg));
+ fail_on_test(abort_reason(&msg) != CEC_OP_ABORT_INVALID_OP);
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_on_own(&msg);
+ msg.msg[2] = 6; /* Invalid source operand */
+ fail_on_test(!transmit_timeout(node, &msg, 10000));
+ if (unrecognized_op(&msg))
+ return OK_NOT_SUPPORTED;
+ fail_on_test(!cec_msg_status_is_abort(&msg));
+ fail_on_test(abort_reason(&msg) != CEC_OP_ABORT_INVALID_OP);
+
+ struct cec_op_record_src rec_src = {};
+ __u8 rec_status;
+
+ rec_src.type = CEC_OP_RECORD_SRC_DIGITAL;
+ rec_src.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID;
+ rec_src.digital.dig_bcast_system = 3; /* Invalid digital broadcast system */
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_DIG_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ memset(&rec_src, 0, sizeof(rec_src));
+ rec_src.type = CEC_OP_RECORD_SRC_ANALOG;
+ rec_src.analog.ana_bcast_type = 3; /* Invalid analog broadcast type */
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ rec_src.analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
+ rec_src.analog.bcast_system = 9; /* Invalid analog broadcast system */
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
+ rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ memset(&rec_src, 0, sizeof(rec_src));
+ rec_src.type = CEC_OP_RECORD_SRC_EXT_PLUG;
+ rec_src.ext_plug.plug = 0; /* Invalid plug */
+ fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_INVALID_EXT_PLUG &&
+ one_touch_rec_status_not_shared(rec_status));
+
+ return OK;
}
static int one_touch_rec_off(struct node *node, unsigned me, unsigned la, bool interactive)
@@ -1261,8 +1389,17 @@ static int one_touch_rec_off(struct node *node, unsigned me, unsigned la, bool i
static const vec_remote_subtests one_touch_rec_subtests{
{ "Record TV Screen", CEC_LOG_ADDR_MASK_TV, one_touch_rec_tv_screen },
- { "Record On", CEC_LOG_ADDR_MASK_RECORD, one_touch_rec_on },
- { "Record Off", CEC_LOG_ADDR_MASK_RECORD, one_touch_rec_off },
+ {
+ "Record On",
+ CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP,
+ one_touch_rec_on,
+ },
+ {
+ "Record On Invalid Operand",
+ CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP,
+ one_touch_rec_on_invalid,
+ },
+ { "Record Off", CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP, one_touch_rec_off },
};
/* Timer Programming */
diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index ff47d698..482192e7 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -317,6 +317,7 @@ void state_init(struct node &node)
node.state.deck_report_changes_to = 0;
node.state.deck_state = CEC_OP_DECK_INFO_STOP;
node.state.deck_skip_start = 0;
+ node.state.one_touch_record_on = false;
tuner_dev_info_init(&node.state);
node.state.last_aud_rate_rx_ts = 0;
}
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 3fa95417..8e8877d7 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -53,6 +53,7 @@ struct state {
__u8 deck_report_changes_to;
__u8 deck_state;
__u64 deck_skip_start;
+ bool one_touch_record_on;
time_t toggle_power_status;
__u64 last_aud_rate_rx_ts;
};
@@ -222,7 +223,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
// cec-tuner.cpp
void tuner_dev_info_init(struct state *state);
-void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
+void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me, __u8 type);
// CEC processing
void reply_feature_abort(struct node *node, struct cec_msg *msg,
diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
index 41bb990c..b1c8f3d9 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -271,7 +271,7 @@ static void update_deck_state(struct node *node, unsigned me, __u8 deck_state_ne
}
}
-static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
+static void processMsg(struct node *node, struct cec_msg &msg, unsigned me, __u8 type)
{
__u8 to = cec_msg_destination(&msg);
__u8 from = cec_msg_initiator(&msg);
@@ -672,7 +672,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
case CEC_MSG_SET_TIMER_PROGRAM_TITLE:
case CEC_MSG_TIMER_CLEARED_STATUS:
case CEC_MSG_TIMER_STATUS:
- process_tuner_record_timer_msgs(node, msg, me);
+ process_tuner_record_timer_msgs(node, msg, me, type);
return;
/* Dynamic Auto Lipsync */
@@ -1009,6 +1009,7 @@ void testProcessing(struct node *node, bool wallclock)
doioctl(node, CEC_S_MODE, &mode);
doioctl(node, CEC_ADAP_G_LOG_ADDRS, &laddrs);
me = laddrs.log_addr[0];
+ __u8 type = laddrs.log_addr_type[0];
poll_remote_devs(node, me);
@@ -1088,7 +1089,7 @@ void testProcessing(struct node *node, bool wallclock)
msg.sequence, ts2s(msg.rx_ts, wallclock).c_str());
}
if (node->adap_la_mask)
- processMsg(node, msg, me);
+ processMsg(node, msg, me, type);
}
__u8 pwr_state = current_power_state(node);
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index fd11bd10..c6dd47f0 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -482,7 +482,47 @@ static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
return false;
}
-void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
+bool digital_service_info_valid(const struct cec_op_record_src &rec_src)
+{
+ if (rec_src.digital.service_id_method == CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID) {
+
+ switch (rec_src.digital.dig_bcast_system) {
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_GEN:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_CS:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_CABLE:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
+ case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
+ break;
+ default:
+ return false;
+ }
+ }
+ //To do: check digital_arib_data, digital_atsc_data and digital_dvb_data
+ return true;
+}
+
+bool analog_service_info_valid(const cec_op_record_src &rec_src)
+{
+ if (rec_src.analog.ana_bcast_type > CEC_OP_ANA_BCAST_TYPE_TERRESTRIAL)
+ return false;
+
+ if(rec_src.analog.bcast_system > CEC_OP_BCAST_SYSTEM_PAL_DK &&
+ rec_src.analog.bcast_system != CEC_OP_BCAST_SYSTEM_OTHER)
+ return false;
+
+ //To do: check analog valid channels analog_freqs_khz
+ return true;
+}
+
+void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me, __u8 type)
{
bool is_bcast = cec_msg_is_broadcast(&msg);
@@ -605,19 +645,70 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
transmit(node, &msg);
return;
}
- case CEC_MSG_RECORD_ON:
- if (!cec_has_record(1 << me))
+ case CEC_MSG_RECORD_ON: {
+ if (type != CEC_LOG_ADDR_TYPE_RECORD)
+ break;
+
+ __u8 rec_status;
+ bool start_recording = false;
+ struct cec_op_record_src rec_src = {};
+
+ cec_ops_record_on(&msg, &rec_src);
+
+ switch (rec_src.type) {
+ case CEC_OP_RECORD_SRC_OWN:
+ rec_status = CEC_OP_RECORD_STATUS_CUR_SRC;
+ start_recording = true;
+ break;
+ case CEC_OP_RECORD_SRC_DIGITAL:
+ if (digital_service_info_valid(rec_src)) {
+ rec_status = CEC_OP_RECORD_STATUS_DIG_SERVICE;
+ start_recording = true;
+ } else {
+ rec_status = CEC_OP_RECORD_STATUS_NO_DIG_SERVICE;
+ }
+ break;
+ case CEC_OP_RECORD_SRC_ANALOG:
+ if (analog_service_info_valid(rec_src)) {
+ rec_status = CEC_OP_RECORD_STATUS_ANA_SERVICE;
+ start_recording = true;
+ } else {
+ rec_status = CEC_OP_RECORD_STATUS_NO_ANA_SERVICE;
+ }
+ break;
+ case CEC_OP_RECORD_SRC_EXT_PLUG:
+ /* Plug number range is 1-255 in specs, but a realistic range of connectors is 6. */
+ if (rec_src.ext_plug.plug < 1 || rec_src.ext_plug.plug > 6) {
+ rec_status = CEC_OP_RECORD_STATUS_INVALID_EXT_PLUG;
+ } else {
+ rec_status = CEC_OP_RECORD_STATUS_EXT_INPUT;
+ start_recording = true;
+ }
+ break;
+ case CEC_OP_RECORD_SRC_EXT_PHYS_ADDR:
+ rec_status = CEC_OP_RECORD_STATUS_INVALID_EXT_PHYS_ADDR;
break;
+ default:
+ reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
+ return;
+ }
+ if (node->state.one_touch_record_on)
+ rec_status = CEC_OP_RECORD_STATUS_ALREADY_RECORDING;
cec_msg_set_reply_to(&msg, &msg);
- cec_msg_record_status(&msg, CEC_OP_RECORD_STATUS_CUR_SRC);
+ cec_msg_record_status(&msg, rec_status);
transmit(node, &msg);
+ if (start_recording)
+ node->state.one_touch_record_on = true;
return;
+ }
case CEC_MSG_RECORD_OFF:
- if (!cec_has_record(1 << me))
+ if (type != CEC_LOG_ADDR_TYPE_RECORD)
break;
+
cec_msg_set_reply_to(&msg, &msg);
cec_msg_record_status(&msg, CEC_OP_RECORD_STATUS_TERMINATED_OK);
transmit(node, &msg);
+ node->state.one_touch_record_on = false;
return;
case CEC_MSG_RECORD_STATUS:
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] cec: add One Touch Record Standby tests
2021-06-22 4:35 [PATCH v3 0/3] cec: One Touch Record tests Deborah Brouwer
2021-06-22 4:35 ` [PATCH v3 1/3] cec: expand One Touch Record TV Screen test Deborah Brouwer
2021-06-22 4:36 ` [PATCH v3 2/3] cec: expand One Touch Record On/Off tests Deborah Brouwer
@ 2021-06-22 4:36 ` Deborah Brouwer
2 siblings, 0 replies; 6+ messages in thread
From: Deborah Brouwer @ 2021-06-22 4:36 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, jaffe1, Deborah Brouwer
Check that the recording device ignores a standby message while it is
recording. When the recording is finished, check that the recording device
enters standby unless the recording device is the active source.
Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
---
utils/cec-compliance/cec-test-power.cpp | 63 +++++++++++++++++++++++++
utils/cec-follower/cec-follower.cpp | 1 +
utils/cec-follower/cec-follower.h | 2 +
utils/cec-follower/cec-processing.cpp | 7 ++-
utils/cec-follower/cec-tuner.cpp | 9 ++++
5 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/utils/cec-compliance/cec-test-power.cpp b/utils/cec-compliance/cec-test-power.cpp
index b675bfc4..c6bf7093 100644
--- a/utils/cec-compliance/cec-test-power.cpp
+++ b/utils/cec-compliance/cec-test-power.cpp
@@ -677,6 +677,67 @@ static int standby_resume_wakeup_deck_play(struct node *node, unsigned me, unsig
return standby_resume_wakeup_deck(node, me, la, interactive, CEC_OP_PLAY_MODE_PLAY_FWD);
}
+static int standby_record(struct node *node, unsigned me, unsigned la, bool interactive, __u8 opcode)
+{
+ struct cec_msg msg;
+ __u8 rec_status;
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_on_own(&msg);
+ msg.reply = CEC_MSG_RECORD_STATUS;
+ fail_on_test(!transmit_timeout(node, &msg, 10000));
+ if (timed_out_or_abort(&msg))
+ return OK_NOT_SUPPORTED;
+ cec_ops_record_status(&msg, &rec_status);
+ fail_on_test(rec_status != CEC_OP_RECORD_STATUS_CUR_SRC &&
+ rec_status != CEC_OP_RECORD_STATUS_ALREADY_RECORDING);
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_standby(&msg);
+ fail_on_test(!transmit_timeout(node, &msg));
+ /* Standby should not interrupt the recording. */
+ fail_on_test(!cec_msg_status_is_abort(&msg));
+
+ /* When the recording stops, the recorder should standby unless the recorder is the active source */
+ cec_msg_init(&msg, me, la);
+ if (opcode == CEC_MSG_ACTIVE_SOURCE)
+ cec_msg_active_source(&msg, node->remote[la].phys_addr);
+ else
+ cec_msg_active_source(&msg, node->remote[la].phys_addr + 1);
+ fail_on_test(!transmit_timeout(node, &msg));
+
+ cec_msg_init(&msg, me, la);
+ cec_msg_record_off(&msg, false);
+ fail_on_test(!transmit_timeout(node, &msg));
+
+ unsigned unresponsive_time = 0;
+
+ if (opcode == CEC_MSG_ACTIVE_SOURCE) {
+ fail_on_test(!poll_stable_power_status(node, me, la, CEC_OP_POWER_STATUS_ON, unresponsive_time));
+ } else {
+ fail_on_test(!poll_stable_power_status(node, me, la, CEC_OP_POWER_STATUS_STANDBY, unresponsive_time));
+ fail_on_test(interactive && !question("Is the device in standby?"));
+ node->remote[la].in_standby = true;
+
+ int ret = standby_resume_wakeup(node, me, la, interactive);
+ if (ret)
+ return ret;
+ node->remote[la].in_standby = false;
+ }
+
+ return OK;
+}
+
+static int standby_record_active_source(struct node *node, unsigned me, unsigned la, bool interactive)
+{
+ return standby_record(node, me, la, interactive, CEC_MSG_ACTIVE_SOURCE);
+}
+
+static int standby_record_inactive_source(struct node *node, unsigned me, unsigned la, bool interactive)
+{
+ return standby_record(node, me, la, interactive, CEC_MSG_INACTIVE_SOURCE);
+}
+
const vec_remote_subtests standby_resume_subtests{
{ "Standby", CEC_LOG_ADDR_MASK_ALL, standby_resume_standby },
{ "Repeated Standby message does not wake up", CEC_LOG_ADDR_MASK_ALL, standby_resume_standby_toggle },
@@ -697,4 +758,6 @@ const vec_remote_subtests standby_resume_subtests{
{ "Power State Transitions", CEC_LOG_ADDR_MASK_TV, power_state_transitions, false, true },
{ "Deck Eject Standby Resume", CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD, standby_resume_wakeup_deck_eject },
{ "Deck Play Standby Resume", CEC_LOG_ADDR_MASK_PLAYBACK | CEC_LOG_ADDR_MASK_RECORD, standby_resume_wakeup_deck_play },
+ { "Record Standby Active Source", CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP, standby_record_active_source },
+ { "Record Standby Inactive Source", CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP, standby_record_inactive_source },
};
diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
index 482192e7..ff8bdff7 100644
--- a/utils/cec-follower/cec-follower.cpp
+++ b/utils/cec-follower/cec-follower.cpp
@@ -317,6 +317,7 @@ void state_init(struct node &node)
node.state.deck_report_changes_to = 0;
node.state.deck_state = CEC_OP_DECK_INFO_STOP;
node.state.deck_skip_start = 0;
+ node.state.one_touch_record_standby = false;
node.state.one_touch_record_on = false;
tuner_dev_info_init(&node.state);
node.state.last_aud_rate_rx_ts = 0;
diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
index 8e8877d7..4fdc4aec 100644
--- a/utils/cec-follower/cec-follower.h
+++ b/utils/cec-follower/cec-follower.h
@@ -53,6 +53,7 @@ struct state {
__u8 deck_report_changes_to;
__u8 deck_state;
__u64 deck_skip_start;
+ bool one_touch_record_standby;
bool one_touch_record_on;
time_t toggle_power_status;
__u64 last_aud_rate_rx_ts;
@@ -229,5 +230,6 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
void reply_feature_abort(struct node *node, struct cec_msg *msg,
__u8 reason = CEC_OP_ABORT_UNRECOGNIZED_OP);
void testProcessing(struct node *node, bool wallclock);
+bool enter_standby(struct node *node);
#endif
diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
index b1c8f3d9..b4ddb8b4 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -157,7 +157,7 @@ static bool exit_standby(struct node *node)
return false;
}
-static bool enter_standby(struct node *node)
+bool enter_standby(struct node *node)
{
if (node->state.power_status == CEC_OP_POWER_STATUS_ON ||
node->state.power_status == CEC_OP_POWER_STATUS_TO_ON) {
@@ -320,6 +320,11 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me, __u8
/* Standby */
case CEC_MSG_STANDBY:
+ /* Standby should not interrupt a recording in progress. */
+ if (node->state.one_touch_record_on) {
+ node->state.one_touch_record_standby = true;
+ break;
+ }
enter_standby(node);
return;
diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
index c6dd47f0..225fa934 100644
--- a/utils/cec-follower/cec-tuner.cpp
+++ b/utils/cec-follower/cec-tuner.cpp
@@ -708,6 +708,15 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
cec_msg_set_reply_to(&msg, &msg);
cec_msg_record_status(&msg, CEC_OP_RECORD_STATUS_TERMINATED_OK);
transmit(node, &msg);
+ /*
+ * If standby was received during recording, enter standby when the recording is finished
+ * unless the recording device is the currently the active source.
+ */
+ if (node->state.one_touch_record_standby) {
+ if (node->phys_addr != node->state.active_source_pa)
+ enter_standby(node);
+ node->state.one_touch_record_standby = false;
+ }
node->state.one_touch_record_on = false;
return;
case CEC_MSG_RECORD_STATUS:
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] cec: expand One Touch Record TV Screen test
2021-06-22 4:35 ` [PATCH v3 1/3] cec: expand One Touch Record TV Screen test Deborah Brouwer
@ 2021-06-22 15:06 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2021-06-22 15:06 UTC (permalink / raw)
To: Deborah Brouwer, linux-media; +Cc: jaffe1
On 22/06/2021 06:35, Deborah Brouwer wrote:
> Check that the follower ignores the Record TV Screen message if the
> initiator has a logical address other than Record or Backup (aka Reserved
> in CEC Version < 2.0). If the follower replies correctly, check that the
> source operand is valid.
>
> Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
> ---
> utils/cec-compliance/cec-test.cpp | 54 ++++++++++++++++++++++++++-----
> utils/cec-follower/cec-tuner.cpp | 9 ++++--
> 2 files changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
> index 40d8369d..2ea1b712 100644
> --- a/utils/cec-compliance/cec-test.cpp
> +++ b/utils/cec-compliance/cec-test.cpp
> @@ -1150,13 +1150,6 @@ static const vec_remote_subtests tuner_ctl_subtests{
>
> static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la, bool interactive)
> {
> - /*
> - TODO:
> - - Page 36 in HDMI CEC 1.4b spec lists additional behaviors that should be
> - checked for.
> - - The TV should ignore this message when received from other LA than Recording or
> - Reserved.
> - */
> struct cec_msg msg;
>
> cec_msg_init(&msg, me, la);
> @@ -1172,8 +1165,53 @@ static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la,
> return OK_REFUSED;
> if (cec_msg_status_is_abort(&msg))
> return OK_PRESUMED;
> + /*
> + * Follower should ignore this message if initiator has a logical
> + * address other than Record or Backup (aka "Reserved" in CEC Version < 2.0).
> + */
> + if (!cec_has_record(1 << me) && !cec_has_backup(1 << me)) {
> + fail_on_test(!timed_out(&msg));
> + return OK;
> + }
> + fail_on_test(timed_out(&msg));
>
> - return 0;
> + struct cec_op_record_src rec_src = {};
> +
> + cec_ops_record_on(&msg, &rec_src);
> +
> + if (rec_src.type < CEC_OP_RECORD_SRC_OWN || rec_src.type > CEC_OP_RECORD_SRC_EXT_PHYS_ADDR)
> + return fail("Invalid source.\n");
> +
> + if (rec_src.type == CEC_OP_RECORD_SRC_DIGITAL &&
> + rec_src.digital.service_id_method == CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID) {
> +
No need for this empty line.
> + switch (rec_src.digital.dig_bcast_system) {
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_CS:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_CABLE:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
> + break;
> + default:
> + return fail("Invalid digital service broadcast system operand.\n");
> + }
> + }
> +
> + if (rec_src.type == CEC_OP_RECORD_SRC_ANALOG) {
> + fail_on_test(rec_src.analog.ana_bcast_type > CEC_OP_ANA_BCAST_TYPE_TERRESTRIAL);
> + fail_on_test(rec_src.analog.bcast_system > CEC_OP_BCAST_SYSTEM_PAL_DK &&
> + rec_src.analog.bcast_system != CEC_OP_BCAST_SYSTEM_OTHER);
You can also check the analog frequency here: corner cases 0x0000 and 0xffff are invalid.
> + }
For CEC_OP_RECORD_SRC_EXT_PLUG you can check against the invalid value 0x00.
Regards,
Hans
> +
> + return OK;
> }
>
> static int one_touch_rec_on(struct node *node, unsigned me, unsigned la, bool interactive)
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index b9c21684..fd11bd10 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -583,9 +583,6 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
> This is only a basic implementation.
>
> TODO:
> - - If we are a TV, we should only send Record On if the
> - remote end is a Recording device or Reserved. Otherwise ignore.
> -
> - Device state should reflect whether we are recording, etc. In
> recording mode we should ignore Standby messages.
> */
> @@ -594,6 +591,12 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
> if (!node->has_rec_tv)
> break;
>
> + __u8 la = cec_msg_initiator(&msg);
> +
> + /* Ignore if initiator is not Record or Backup (aka "Reserved" in CEC Version < 2.0) */
> + if (!cec_has_record(1 << la) && !cec_has_backup(1 << la))
> + return;
> +
> struct cec_op_record_src rec_src = {};
>
> rec_src.type = CEC_OP_RECORD_SRC_OWN;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/3] cec: expand One Touch Record On/Off tests
2021-06-22 4:36 ` [PATCH v3 2/3] cec: expand One Touch Record On/Off tests Deborah Brouwer
@ 2021-06-22 15:34 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2021-06-22 15:34 UTC (permalink / raw)
To: Deborah Brouwer, linux-media; +Cc: jaffe1
On 22/06/2021 06:36, Deborah Brouwer wrote:
> Send all Record On source operands and check that the follower responds
> with a corresponding Record Status. Send invalid Record On operands and
> check that the follower returns Feature Abort with Invalid Operand or
> Record Status indicating why the recording has failed.
>
> Signed-off-by: Deborah Brouwer <deborahbrouwer3563@gmail.com>
> ---
> utils/cec-compliance/cec-compliance.h | 5 +
> utils/cec-compliance/cec-test.cpp | 157 ++++++++++++++++++++++++--
> utils/cec-follower/cec-follower.cpp | 1 +
> utils/cec-follower/cec-follower.h | 3 +-
> utils/cec-follower/cec-processing.cpp | 7 +-
> utils/cec-follower/cec-tuner.cpp | 101 ++++++++++++++++-
> 6 files changed, 255 insertions(+), 19 deletions(-)
>
> diff --git a/utils/cec-compliance/cec-compliance.h b/utils/cec-compliance/cec-compliance.h
> index 818181ab..d6c11d70 100644
> --- a/utils/cec-compliance/cec-compliance.h
> +++ b/utils/cec-compliance/cec-compliance.h
> @@ -331,6 +331,11 @@ static inline bool is_playback_or_rec(unsigned la)
> return cec_has_playback(1 << la) || cec_has_record(1 << la);
> }
>
> +static inline bool is_record(unsigned la, unsigned prim_type)
> +{
> + return cec_has_record(1 << la) || (cec_has_backup(1 << la) && prim_type == CEC_OP_PRIM_DEVTYPE_RECORD);
> +}
> +
> static inline bool cec_msg_status_is_abort(const struct cec_msg *msg)
> {
> return msg->rx_status & CEC_RX_STATUS_FEATURE_ABORT;
> diff --git a/utils/cec-compliance/cec-test.cpp b/utils/cec-compliance/cec-test.cpp
> index 2ea1b712..4712f993 100644
> --- a/utils/cec-compliance/cec-test.cpp
> +++ b/utils/cec-compliance/cec-test.cpp
> @@ -48,6 +48,45 @@ static int test_play_mode(struct node *node, unsigned me, unsigned la, __u8 play
> return OK;
> }
>
> +static int one_touch_rec_on_send(struct node *node, unsigned me, unsigned la, const struct cec_op_record_src &rec_src, __u8 &rec_status)
This line is too long, add a line break after the la argument.
> +{
> + struct cec_msg msg;
> +
> + cec_msg_init(&msg, me, la);
> + cec_msg_record_off(&msg, false);
> + fail_on_test(!transmit_timeout(node, &msg));
> +
> + cec_msg_init(&msg, me, la);
> + cec_msg_record_on(&msg, true, &rec_src);
> + fail_on_test(!transmit_timeout(node, &msg, 10000));
> + fail_on_test(timed_out_or_abort(&msg));
> + cec_ops_record_status(&msg, &rec_status);
> +
> + return OK;
> +}
> +
> +static bool one_touch_rec_status_not_shared(__u8 rec_status)
> +{
> + switch (rec_status) {
> + case CEC_OP_RECORD_STATUS_UNSUP_CA:
> + case CEC_OP_RECORD_STATUS_NO_CA_ENTITLEMENTS:
> + case CEC_OP_RECORD_STATUS_CANT_COPY_SRC:
> + case CEC_OP_RECORD_STATUS_NO_MORE_COPIES:
> + case CEC_OP_RECORD_STATUS_NO_MEDIA:
> + case CEC_OP_RECORD_STATUS_PLAYING:
> + case CEC_OP_RECORD_STATUS_ALREADY_RECORDING:
> + case CEC_OP_RECORD_STATUS_MEDIA_PROT:
> + case CEC_OP_RECORD_STATUS_NO_SIGNAL:
> + case CEC_OP_RECORD_STATUS_MEDIA_PROBLEM:
> + case CEC_OP_RECORD_STATUS_NO_SPACE:
> + case CEC_OP_RECORD_STATUS_PARENTAL_LOCK:
> + case CEC_OP_RECORD_STATUS_OTHER:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> /* System Information */
>
> int system_info_polling(struct node *node, unsigned me, unsigned la, bool interactive)
> @@ -1216,27 +1255,116 @@ static int one_touch_rec_tv_screen(struct node *node, unsigned me, unsigned la,
>
> static int one_touch_rec_on(struct node *node, unsigned me, unsigned la, bool interactive)
> {
> - /*
> - TODO: Page 36 in HDMI CEC 1.4b spec lists additional behaviors that should be
> - checked for.
> - */
> struct cec_msg msg;
> struct cec_op_record_src rec_src = {};
>
> rec_src.type = CEC_OP_RECORD_SRC_OWN;
> cec_msg_init(&msg, me, la);
> cec_msg_record_on(&msg, true, &rec_src);
> - fail_on_test(!transmit_timeout(node, &msg));
> + /* Allow 10s for reply because specs say it may take several seconds to accurately respond. */
specs -> the spec
There is only on spec, so singular.
> + fail_on_test(!transmit_timeout(node, &msg, 10000));
> fail_on_test(timed_out(&msg));
> - fail_on_test(cec_has_record(1 << la) && unrecognized_op(&msg));
> - if (unrecognized_op(&msg))
> + if (unrecognized_op(&msg)) {
> + fail_on_test(is_record(la, node->remote[la].prim_type));
> return OK_NOT_SUPPORTED;
> + }
> if (refused(&msg))
> return OK_REFUSED;
> if (cec_msg_status_is_abort(&msg))
> return OK_PRESUMED;
>
> - return 0;
> + __u8 rec_status;
> +
> + cec_ops_record_status(&msg, &rec_status);
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_CUR_SRC &&
> + one_touch_rec_status_not_shared(rec_status));
As discussed in irc I suggest something like this:
if (rec_status != CEC_OP_RECORD_STATUS_CUR_SRC)
fail_on_test(!rec_status_is_a_valid_error_status(rec_status));
I.e., if it did not start recording with the current source, then check if
the status is a valid error, otherwise something strange happened and that should
be marked as a failure.
> +
> + rec_src.type = CEC_OP_RECORD_SRC_DIGITAL;
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_DIG_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_DIG_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
I wouldn't test for these error cases, just add that to rec_status_is_a_valid_error_status().
> + one_touch_rec_status_not_shared(rec_status));
> +
> + rec_src.type = CEC_OP_RECORD_SRC_ANALOG;
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_ANA_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
Ditto.
> + one_touch_rec_status_not_shared(rec_status));
> +
> + rec_src.type = CEC_OP_RECORD_SRC_EXT_PLUG;
> + rec_src.ext_plug.plug = 1;
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_EXT_INPUT &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + memset(&rec_src, 0, sizeof(rec_src));
> + rec_src.type = CEC_OP_RECORD_SRC_EXT_PHYS_ADDR;
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_EXT_INPUT &&
> + rec_status != CEC_OP_RECORD_STATUS_INVALID_EXT_PHYS_ADDR &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + return OK;
> +}
> +
> +static int one_touch_rec_on_invalid(struct node *node, unsigned me, unsigned la, bool interactive)
> +{
> + struct cec_msg msg;
> +
> + cec_msg_init(&msg, me, la);
> + cec_msg_record_on_own(&msg);
> + msg.msg[2] = 0; /* Invalid source operand */
> + fail_on_test(!transmit_timeout(node, &msg, 10000));
Reacting to invalid operands shouldn't take 10 seconds, just use the default timeout.
Same below.
> + if (unrecognized_op(&msg))
> + return OK_NOT_SUPPORTED;
> + fail_on_test(!cec_msg_status_is_abort(&msg));
> + fail_on_test(abort_reason(&msg) != CEC_OP_ABORT_INVALID_OP);
> +
> + cec_msg_init(&msg, me, la);
> + cec_msg_record_on_own(&msg);
> + msg.msg[2] = 6; /* Invalid source operand */
> + fail_on_test(!transmit_timeout(node, &msg, 10000));
> + if (unrecognized_op(&msg))
> + return OK_NOT_SUPPORTED;
> + fail_on_test(!cec_msg_status_is_abort(&msg));
> + fail_on_test(abort_reason(&msg) != CEC_OP_ABORT_INVALID_OP);
> +
> + struct cec_op_record_src rec_src = {};
> + __u8 rec_status;
> +
> + rec_src.type = CEC_OP_RECORD_SRC_DIGITAL;
> + rec_src.digital.service_id_method = CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID;
> + rec_src.digital.dig_bcast_system = 3; /* Invalid digital broadcast system */
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
Shouldn't this feature abort with invalid op? This looks weird. Same below.
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_DIG_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + memset(&rec_src, 0, sizeof(rec_src));
> + rec_src.type = CEC_OP_RECORD_SRC_ANALOG;
> + rec_src.analog.ana_bcast_type = 3; /* Invalid analog broadcast type */
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + rec_src.analog.ana_bcast_type = CEC_OP_ANA_BCAST_TYPE_CABLE;
> + rec_src.analog.bcast_system = 9; /* Invalid analog broadcast system */
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_NO_ANA_SERVICE &&
> + rec_status != CEC_OP_RECORD_STATUS_NO_SERVICE &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + memset(&rec_src, 0, sizeof(rec_src));
> + rec_src.type = CEC_OP_RECORD_SRC_EXT_PLUG;
> + rec_src.ext_plug.plug = 0; /* Invalid plug */
> + fail_on_test(one_touch_rec_on_send(node, me, la, rec_src, rec_status));
> + fail_on_test(rec_status != CEC_OP_RECORD_STATUS_INVALID_EXT_PLUG &&
> + one_touch_rec_status_not_shared(rec_status));
> +
> + return OK;
> }
>
> static int one_touch_rec_off(struct node *node, unsigned me, unsigned la, bool interactive)
> @@ -1261,8 +1389,17 @@ static int one_touch_rec_off(struct node *node, unsigned me, unsigned la, bool i
>
> static const vec_remote_subtests one_touch_rec_subtests{
> { "Record TV Screen", CEC_LOG_ADDR_MASK_TV, one_touch_rec_tv_screen },
> - { "Record On", CEC_LOG_ADDR_MASK_RECORD, one_touch_rec_on },
> - { "Record Off", CEC_LOG_ADDR_MASK_RECORD, one_touch_rec_off },
> + {
> + "Record On",
> + CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP,
> + one_touch_rec_on,
> + },
> + {
> + "Record On Invalid Operand",
> + CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP,
> + one_touch_rec_on_invalid,
> + },
> + { "Record Off", CEC_LOG_ADDR_MASK_RECORD | CEC_LOG_ADDR_MASK_BACKUP, one_touch_rec_off },
> };
>
> /* Timer Programming */
> diff --git a/utils/cec-follower/cec-follower.cpp b/utils/cec-follower/cec-follower.cpp
> index ff47d698..482192e7 100644
> --- a/utils/cec-follower/cec-follower.cpp
> +++ b/utils/cec-follower/cec-follower.cpp
> @@ -317,6 +317,7 @@ void state_init(struct node &node)
> node.state.deck_report_changes_to = 0;
> node.state.deck_state = CEC_OP_DECK_INFO_STOP;
> node.state.deck_skip_start = 0;
> + node.state.one_touch_record_on = false;
> tuner_dev_info_init(&node.state);
> node.state.last_aud_rate_rx_ts = 0;
> }
> diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h
> index 3fa95417..8e8877d7 100644
> --- a/utils/cec-follower/cec-follower.h
> +++ b/utils/cec-follower/cec-follower.h
> @@ -53,6 +53,7 @@ struct state {
> __u8 deck_report_changes_to;
> __u8 deck_state;
> __u64 deck_skip_start;
> + bool one_touch_record_on;
> time_t toggle_power_status;
> __u64 last_aud_rate_rx_ts;
> };
> @@ -222,7 +223,7 @@ void sad_encode(const struct short_audio_desc *sad, __u32 *descriptor);
>
> // cec-tuner.cpp
> void tuner_dev_info_init(struct state *state);
> -void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me);
> +void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me, __u8 type);
>
> // CEC processing
> void reply_feature_abort(struct node *node, struct cec_msg *msg,
> diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp
> index 41bb990c..b1c8f3d9 100644
> --- a/utils/cec-follower/cec-processing.cpp
> +++ b/utils/cec-follower/cec-processing.cpp
> @@ -271,7 +271,7 @@ static void update_deck_state(struct node *node, unsigned me, __u8 deck_state_ne
> }
> }
>
> -static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
> +static void processMsg(struct node *node, struct cec_msg &msg, unsigned me, __u8 type)
Adding the type argument and wiring it up to process_tuner_record_timer_msgs() should
be done in a separate patch. It's not related to the record tests as such.
> {
> __u8 to = cec_msg_destination(&msg);
> __u8 from = cec_msg_initiator(&msg);
> @@ -672,7 +672,7 @@ static void processMsg(struct node *node, struct cec_msg &msg, unsigned me)
> case CEC_MSG_SET_TIMER_PROGRAM_TITLE:
> case CEC_MSG_TIMER_CLEARED_STATUS:
> case CEC_MSG_TIMER_STATUS:
> - process_tuner_record_timer_msgs(node, msg, me);
> + process_tuner_record_timer_msgs(node, msg, me, type);
> return;
>
> /* Dynamic Auto Lipsync */
> @@ -1009,6 +1009,7 @@ void testProcessing(struct node *node, bool wallclock)
> doioctl(node, CEC_S_MODE, &mode);
> doioctl(node, CEC_ADAP_G_LOG_ADDRS, &laddrs);
> me = laddrs.log_addr[0];
> + __u8 type = laddrs.log_addr_type[0];
>
> poll_remote_devs(node, me);
>
> @@ -1088,7 +1089,7 @@ void testProcessing(struct node *node, bool wallclock)
> msg.sequence, ts2s(msg.rx_ts, wallclock).c_str());
> }
> if (node->adap_la_mask)
> - processMsg(node, msg, me);
> + processMsg(node, msg, me, type);
> }
>
> __u8 pwr_state = current_power_state(node);
> diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp
> index fd11bd10..c6dd47f0 100644
> --- a/utils/cec-follower/cec-tuner.cpp
> +++ b/utils/cec-follower/cec-tuner.cpp
> @@ -482,7 +482,47 @@ static bool analog_set_tuner_dev_info(struct node *node, struct cec_msg *msg)
> return false;
> }
>
> -void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me)
> +bool digital_service_info_valid(const struct cec_op_record_src &rec_src)
Can this be a static function? It's only used in this source, so I think t should be.
Ditto for the similar bool functions below.
> +{
> + if (rec_src.digital.service_id_method == CEC_OP_SERVICE_ID_METHOD_BY_DIG_ID) {
> +
Drop this newline.
> + switch (rec_src.digital.dig_bcast_system) {
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_GEN:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_BS:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_CS:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ARIB_T:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_CABLE:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_SAT:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_ATSC_T:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_C:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_S2:
> + case CEC_OP_DIG_SERVICE_BCAST_SYSTEM_DVB_T:
> + break;
> + default:
> + return false;
> + }
> + }
> + //To do: check digital_arib_data, digital_atsc_data and digital_dvb_data
> + return true;
> +}
> +
> +bool analog_service_info_valid(const cec_op_record_src &rec_src)
> +{
> + if (rec_src.analog.ana_bcast_type > CEC_OP_ANA_BCAST_TYPE_TERRESTRIAL)
> + return false;
> +
> + if(rec_src.analog.bcast_system > CEC_OP_BCAST_SYSTEM_PAL_DK &&
> + rec_src.analog.bcast_system != CEC_OP_BCAST_SYSTEM_OTHER)
> + return false;
> +
> + //To do: check analog valid channels analog_freqs_khz
> + return true;
> +}
> +
> +void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, unsigned me, __u8 type)
> {
> bool is_bcast = cec_msg_is_broadcast(&msg);
>
> @@ -605,19 +645,70 @@ void process_tuner_record_timer_msgs(struct node *node, struct cec_msg &msg, uns
> transmit(node, &msg);
> return;
> }
> - case CEC_MSG_RECORD_ON:
> - if (!cec_has_record(1 << me))
> + case CEC_MSG_RECORD_ON: {
> + if (type != CEC_LOG_ADDR_TYPE_RECORD)
> + break;
> +
> + __u8 rec_status;
> + bool start_recording = false;
> + struct cec_op_record_src rec_src = {};
> +
> + cec_ops_record_on(&msg, &rec_src);
> +
> + switch (rec_src.type) {
> + case CEC_OP_RECORD_SRC_OWN:
> + rec_status = CEC_OP_RECORD_STATUS_CUR_SRC;
> + start_recording = true;
> + break;
> + case CEC_OP_RECORD_SRC_DIGITAL:
> + if (digital_service_info_valid(rec_src)) {
> + rec_status = CEC_OP_RECORD_STATUS_DIG_SERVICE;
> + start_recording = true;
> + } else {
> + rec_status = CEC_OP_RECORD_STATUS_NO_DIG_SERVICE;
Huh? Shouldn't this feature abort with invalid op? Since you provide invalid data here.
> + }
> + break;
> + case CEC_OP_RECORD_SRC_ANALOG:
> + if (analog_service_info_valid(rec_src)) {
> + rec_status = CEC_OP_RECORD_STATUS_ANA_SERVICE;
> + start_recording = true;
> + } else {
> + rec_status = CEC_OP_RECORD_STATUS_NO_ANA_SERVICE;
> + }
> + break;
> + case CEC_OP_RECORD_SRC_EXT_PLUG:
> + /* Plug number range is 1-255 in specs, but a realistic range of connectors is 6. */
> + if (rec_src.ext_plug.plug < 1 || rec_src.ext_plug.plug > 6) {
> + rec_status = CEC_OP_RECORD_STATUS_INVALID_EXT_PLUG;
> + } else {
> + rec_status = CEC_OP_RECORD_STATUS_EXT_INPUT;
> + start_recording = true;
> + }
> + break;
> + case CEC_OP_RECORD_SRC_EXT_PHYS_ADDR:
> + rec_status = CEC_OP_RECORD_STATUS_INVALID_EXT_PHYS_ADDR;
> break;
> + default:
> + reply_feature_abort(node, &msg, CEC_OP_ABORT_INVALID_OP);
> + return;
> + }
> + if (node->state.one_touch_record_on)
> + rec_status = CEC_OP_RECORD_STATUS_ALREADY_RECORDING;
> cec_msg_set_reply_to(&msg, &msg);
> - cec_msg_record_status(&msg, CEC_OP_RECORD_STATUS_CUR_SRC);
> + cec_msg_record_status(&msg, rec_status);
> transmit(node, &msg);
> + if (start_recording)
> + node->state.one_touch_record_on = true;
> return;
> + }
> case CEC_MSG_RECORD_OFF:
> - if (!cec_has_record(1 << me))
> + if (type != CEC_LOG_ADDR_TYPE_RECORD)
> break;
> +
> cec_msg_set_reply_to(&msg, &msg);
> cec_msg_record_status(&msg, CEC_OP_RECORD_STATUS_TERMINATED_OK);
> transmit(node, &msg);
> + node->state.one_touch_record_on = false;
> return;
> case CEC_MSG_RECORD_STATUS:
> return;
>
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-22 15:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-22 4:35 [PATCH v3 0/3] cec: One Touch Record tests Deborah Brouwer
2021-06-22 4:35 ` [PATCH v3 1/3] cec: expand One Touch Record TV Screen test Deborah Brouwer
2021-06-22 15:06 ` Hans Verkuil
2021-06-22 4:36 ` [PATCH v3 2/3] cec: expand One Touch Record On/Off tests Deborah Brouwer
2021-06-22 15:34 ` Hans Verkuil
2021-06-22 4:36 ` [PATCH v3 3/3] cec: add One Touch Record Standby tests Deborah Brouwer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox