From 2899603fb0a40b1331d38a7c4e3b886d2cbb15d0 Mon Sep 17 00:00:00 2001 From: Sam Stenvall Date: Wed, 17 May 2023 13:16:10 +0300 Subject: [PATCH 1/5] Add a test for channels that can be decrypted by multiple DDCI devices --- tests/test_ddci.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/tests/test_ddci.c b/tests/test_ddci.c index 16f543757b..68d58b3b9f 100644 --- a/tests/test_ddci.c +++ b/tests/test_ddci.c @@ -85,22 +85,37 @@ void create_adapter(adapter *ad, int id) { int test_channels() { SHashTable h; int i; - Sddci_channel c, *t; + Sddci_channel c1, c2, *t; memset(&h, 0, sizeof(h)); create_hash_table(&h, 10); - memset(&c, 0, sizeof(c)); - c.sid = 200; - c.ddci[1].ddci = 1; - c.ddcis = 1; - setItem(&h, c.sid, &c, sizeof(c)); + + // Add one channel (SID 200) that can be decoded by DDCI 1 + memset(&c1, 0, sizeof(c1)); + c1.sid = 200; + c1.ddci[0].ddci = 1; + c1.ddcis = 1; + setItem(&h, c1.sid, &c1, sizeof(c1)); + + // Add a second channel (SID 300) that can be decoded by both DDCI 1 and 2 + memset(&c2, 0, sizeof(c2)); + c2.sid = 300; + c2.ddci[0].ddci = 1; + c2.ddci[1].ddci = 2; + c2.ddcis = 2; + setItem(&h, c2.sid, &c2, sizeof(c2)); + + // Save and reload the table save_channels(&h); free_hash(&h); create_hash_table(&h, 10); load_channels(&h); - ASSERT(getItem(&h, 200) != NULL, "Saved SID not found in table"); + + // Check the contents + ASSERT(getItem(&h, 200) != NULL, "Saved SID 200 not found in table"); + ASSERT(getItem(&h, 300) != NULL, "Saved SID 300 not found in table"); int ch = 0; FOREACH_ITEM(&h, t) { ch++; } - ASSERT(ch == 1, "Expected one channel after loading"); + ASSERT(ch == 2, "Expected two channels after loading"); free_hash(&h); return 0; } From e7b7407eb4891a34a4c65c32a41a30cf2128382b Mon Sep 17 00:00:00 2001 From: Sam Stenvall Date: Wed, 17 May 2023 13:24:18 +0300 Subject: [PATCH 2/5] Reset buffer between iterations, fixes misleading log entries The buffer from the previous line was reused if no DDCI adapters are specified, which is quite misleading --- src/ddci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ddci.c b/src/ddci.c index 5e851dfcfa..67b65a0be4 100644 --- a/src/ddci.c +++ b/src/ddci.c @@ -1278,14 +1278,15 @@ void load_channels(SHashTable *ch) { opts.cache_dir, CONFIG_FILE_NAME); FILE *f = fopen(ddci_conf_path, "rt"); - Sddci_channel c; char line[1000]; int channels = 0; - int pos = 0; if (!f) return; while (fgets(line, sizeof(line), f)) { + int pos = 0; char buf[1000]; + memset(buf, 0, sizeof(buf)); + Sddci_channel c; memset(&c, 0, sizeof(c)); char *x = strchr(line, '#'); if (x) @@ -1304,7 +1305,6 @@ void load_channels(SHashTable *ch) { int la = split(arg, cc, ARRAY_SIZE(arg), ','); int i = 0; channels++; - pos = 0; for (i = 0; i < la; i++) { int v = map_intd(arg[i], NULL, -1); if (v != -1) { From 24a5b27629d0059d948017ede89abbbed6c203f4 Mon Sep 17 00:00:00 2001 From: Sam Stenvall Date: Wed, 17 May 2023 13:52:49 +0300 Subject: [PATCH 3/5] Remove "pos" from Sddci_channel, only used in a loop --- src/ddci.c | 8 +++----- src/ddci.h | 1 - 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/ddci.c b/src/ddci.c index 67b65a0be4..a684d17899 100644 --- a/src/ddci.c +++ b/src/ddci.c @@ -292,11 +292,9 @@ int find_ddci_for_pmt(Sddci_channel *c, SPMT *pmt) { int ddid = -100; int retry = 0; - // search always from the beginning when the PMT is started - - // continue where we left off - for (c->pos = 0; c->pos < c->ddcis; c->pos++) { - ddid = c->ddci[c->pos].ddci; + int i = 0; + for (i = 0; i < c->ddcis; i++) { + ddid = c->ddci[i].ddci; ddci_device_t *d = get_ddci(ddid); if (!d) { LOG("DDID %d not enabled", ddid); diff --git a/src/ddci.h b/src/ddci.h index 47f5313f66..a740dccab8 100644 --- a/src/ddci.h +++ b/src/ddci.h @@ -62,7 +62,6 @@ typedef struct ddci_channel { uint8_t ddci; } ddci[MAX_ADAPTERS]; int ddcis; - int pos; int sid; char name[50]; char locked; From 4cabaeac5cec3b395c1cf0d6e3905d0c007a5f35 Mon Sep 17 00:00:00 2001 From: Sam Stenvall Date: Wed, 17 May 2023 14:18:46 +0300 Subject: [PATCH 4/5] Add a test for channels that are not assigned to any DDCI adapter on purpose --- tests/test_ddci.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_ddci.c b/tests/test_ddci.c index 68d58b3b9f..6caca1badf 100644 --- a/tests/test_ddci.c +++ b/tests/test_ddci.c @@ -85,7 +85,7 @@ void create_adapter(adapter *ad, int id) { int test_channels() { SHashTable h; int i; - Sddci_channel c1, c2, *t; + Sddci_channel c1, c2, c3, *t; memset(&h, 0, sizeof(h)); create_hash_table(&h, 10); @@ -104,6 +104,12 @@ int test_channels() { c2.ddcis = 2; setItem(&h, c2.sid, &c2, sizeof(c2)); + // Add a third channel (SID 400) that cannot be decoded by any adapter + memset(&c3, 0, sizeof(c3)); + c3.sid = 400; + c3.ddcis = 0; + setItem(&h, c3.sid, &c3, sizeof(c3)); + // Save and reload the table save_channels(&h); free_hash(&h); @@ -113,9 +119,10 @@ int test_channels() { // Check the contents ASSERT(getItem(&h, 200) != NULL, "Saved SID 200 not found in table"); ASSERT(getItem(&h, 300) != NULL, "Saved SID 300 not found in table"); + ASSERT(getItem(&h, 400) != NULL, "Saved SID 400 not found in table"); int ch = 0; FOREACH_ITEM(&h, t) { ch++; } - ASSERT(ch == 2, "Expected two channels after loading"); + ASSERT(ch == 3, "Expected two channels after loading"); free_hash(&h); return 0; } From 0591deabe951f4148f309454829f61dd169257e5 Mon Sep 17 00:00:00 2001 From: Sam Stenvall Date: Wed, 17 May 2023 14:33:43 +0300 Subject: [PATCH 5/5] Refactor find_ddci_for_pmt() somewhat so it's more clear what's going on No changes to the logic, just better logging and --- src/ddci.c | 67 ++++++++++++++++++++++++++++++++--------------- tests/test_ddci.c | 14 +++++++++- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/src/ddci.c b/src/ddci.c index a684d17899..443049114e 100644 --- a/src/ddci.c +++ b/src/ddci.c @@ -288,36 +288,57 @@ int create_channel_for_pmt(Sddci_channel *c, SPMT *pmt) { return 0; } +/** + * Find the DDCI device that should be used to handle the specified PMT. The + * first suitable device is returned. If a suitable device is found but the + * device is still initializing, -1 is returned. If no suitable device is found, + * -2 is returned. + */ int find_ddci_for_pmt(Sddci_channel *c, SPMT *pmt) { - int ddid = -100; - int retry = 0; + int ddid = -100; // -100 means we didn't find a suitable device int i = 0; for (i = 0; i < c->ddcis; i++) { - ddid = c->ddci[i].ddci; - ddci_device_t *d = get_ddci(ddid); + int candidate = c->ddci[i].ddci; + ddci_device_t *d = get_ddci(candidate); if (!d) { - LOG("DDID %d not enabled", ddid); + LOG("%s: DDCI %d not enabled, skipping", __FUNCTION__, candidate); + continue; } - if (is_ca_initializing(ddid)) { - LOG("DD %d is initializing", ddid); - retry = 1; - } else { - if (d && d->channels < d->max_channels) - break; - else - LOG("DDCI %d cannot be used for PMT %d, pid %d (used " - "channels " - "%d max %d)", - ddid, pmt->id, pmt->pid, d ? d->channels : -1, - d ? d->max_channels : -1); + if (d->channels >= d->max_channels) { + LOG("%s: DDCI %d cannot be used for PMT %d, pid %d, sid, %d (used " + "channels " + "%d max %d), skipping", + __FUNCTION__, candidate, pmt->id, pmt->pid, pmt->sid, + d ? d->channels : -1, d ? d->max_channels : -1); + continue; } - ddid = -100; + + ddid = candidate; + break; } - if (retry) + + if (ddid == -100) { + // Distinguish between otherwise not found and deliberately not assigned + if (c->ddcis == 0) { + LOG("%s: no suitable DDCI found for PMT %d (sid %d): not mapped to " + "any DDCI " + "device in ddci.conf", + __FUNCTION__, pmt->id, pmt->sid); + } else { + LOG("%s: no suitable DDCI found for PMT %d (sid %d)", __FUNCTION__, + pmt->id, pmt->sid); + } + + return -TABLES_RESULT_ERROR_NORETRY; + } else if (is_ca_initializing(ddid)) { + LOG("%s: DDCI %d is initializing, retrying", __FUNCTION__, ddid); return -TABLES_RESULT_ERROR_RETRY; - return ddid; + } else { + LOG("%s: Using DDCI %d", __FUNCTION__, ddid); + return ddid; + } } int is_pmt_running(SPMT *pmt) { @@ -380,10 +401,14 @@ int ddci_process_pmt(adapter *ad, SPMT *pmt) { "Could not allocate channel"); } + // Determine which DDCI should handle this PMT if (ddid == -1) ddid = find_ddci_for_pmt(channel, pmt); + // Negative return values are used to distinguish from valid return values (>= 0) if (ddid == -TABLES_RESULT_ERROR_RETRY) - return -ddid; + return TABLES_RESULT_ERROR_RETRY; + else if (ddid == -TABLES_RESULT_ERROR_NORETRY) + return TABLES_RESULT_ERROR_NORETRY; d = get_ddci(ddid); if (!d) { diff --git a/tests/test_ddci.c b/tests/test_ddci.c index 6caca1badf..05d133ef15 100644 --- a/tests/test_ddci.c +++ b/tests/test_ddci.c @@ -129,7 +129,7 @@ int test_channels() { int test_add_del_pmt() { int i; - SPMT *pmt0, *pmt1, *pmt2, *pmt3; + SPMT *pmt0, *pmt1, *pmt2, *pmt3, *pmt4; ddci_device_t d0, d1; ca_device_t ca0, ca1; adapter ad, a0, a1; @@ -142,6 +142,7 @@ int test_add_del_pmt() { pmt1 = create_pmt(8, 200, 201, 202, 0x100, 0x500); pmt2 = create_pmt(8, 300, 301, 302, 0x500, 0x100); pmt3 = create_pmt(8, 400, 401, 402, 0x600, 0x601); + pmt4 = create_pmt(8, 500, 501, 502, 0x500, 0x500); memset(&d0, 0, sizeof(d0)); memset(&d1, 0, sizeof(d1)); memset(&d0.pmt, -1, sizeof(d0.pmt)); @@ -155,6 +156,14 @@ int test_add_del_pmt() { create_hash_table(&d1.mapping, 30); create_hash_table(&channels, 30); + // Save a channel for pmt4, we'll check that it is not assigned to + // any DDCI adapter even though its CAID matches etc. + Sddci_channel c4; + memset(&c4, 0, sizeof(c4)); + c4.sid = pmt4->sid; + c4.ddcis = 0; + setItem(&channels, c4.sid, &c4, sizeof(c4)); + int dvbca_id = add_ca(&dvbca, 0xFFFFFFFF); // DD 0 - 0x100, DD 1 - 0x500 add_caid_mask(dvbca_id, 0, 0x100, 0xFFFF); @@ -177,6 +186,9 @@ int test_add_del_pmt() { ASSERT(ddci_process_pmt(&ad, pmt3) == TABLES_RESULT_ERROR_NORETRY, "DDCI ready, expected no retry"); + ASSERT(ddci_process_pmt(&ad, pmt4) == TABLES_RESULT_ERROR_NORETRY, + "Channel not assigned to any DDCI adapter, expected no retry"); + // One matching channel ASSERT(ddci_process_pmt(&ad, pmt0) == TABLES_RESULT_OK, "DDCI matching DD 0");