* [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
* 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
* [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
* 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
* [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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.