Tvheadend

Confirmed.

My system, running current master, has tuners for DVB-T and DVB-S, and collects EPG from UK Freeview and Freesat with grabbers set to default priorities. If I enable the OpenTV Sky UK grabber with default priority, I can collect EPG for Sky channels which are not part of Freesat (for example CNN HD). However although the times and titles look correct, the descriptions are completely wrong - they are from different programmes on different channels but I can't work out where.

yeah i get the same issue I also get random season and episode data attached to movie information. i think the issue is there implementation of the opentv EPG as I didn't get this issue on windows with EPG collector.
https://drive.google.com/drive/folders/18FN9_JTpbz9SwgqQvzKFBavOAcbSttP4?usp=sharing this is a link to my log file if it will help anyone work out what's wrong

Thanks for the log which matches what I'm seeing.

I'm going to build another Tvheadend instance using just the OpenTV grabber to simplify things.

One oddity I've noticed. Taking a typical entry from your log:

2021-01-17 15:03:21.141 [  DEBUG]:opentv: find by time start 1611334800 stop 1611338400 eid 431 = 0x7f421e9faf20
2021-01-17 15:03:21.141 [  DEBUG]:opentv:     title 'Diabolical: Deadly Love'
2021-01-17 15:03:21.141 [  DEBUG]:opentv:   summary 'An international news bulletin, presented live from the newsroom.'

Any news on what could be the issue with the opentv module?

I set up another TVH instance on a RPi3 using only the OpenTV grabber. That gave the same result but had several data overrun errors in the log (the "too much queued table input data (over 2MB), discarding new" error). Switching to a Pi4 gave the same errors. Installing TVH on an old laptop with a Core i3 processor fixed the overflow errors but there are still a lot of events with missing summaries and a few where the wrong summary is shown.

I haven't been able to find a description of the (proprietary, reverse-engineered) OpenTV encoding format, but from the code it seems that the title and summary fields are broadcast on separate PIDs and so the grabber must try to match them up; presumably it is this which is failing. However the code has been part of TVHeadend for many years and presumably worked in the past, and there have been no recent changes.

In short, I'm stumped.

From what I've been searching PID 17 is for channel list, PIDs 48 - 55 is for titles and PIDs 64 - 71 is for summaries these where found from crossepg source code whic is used on the linux STBs and apparently they're still working so i would imagine that its the way TVheadend is handling the data incorrectly thus giving us errors in the guide data

The "too much queued table input data (over 2MB), discarding new" error is possibly where it's getting stuck in a loop because my collection is timing out even when I've got the setting set to 7200

Interesting, thanks. I tried going back to the last Tvheadend version which was reported working in the issues log (4.3.1605) but still hit the same problem. If there was a change in the OpenTV format that would explain it.

I then tried forking your "RadiotimesXmltvEmulator" (https://github.com/dave-p/openTVtoXML) to feed into Tvheadend. That works, but is more cumbersome to use and doesn't capture all the information.

Just to say that I also suffer from the exact same issue described here - currently running v4.3-1923~gaaca05cc1.

I did some debugging on this issue and found that within the OpenTV SkyUK data some events will have the same event id on another channel. This seems to be why the openTVtoXML emulator works perfectly - as that matches on a combination of Event ID and channel ID - whereas TVheadend matches only on the event id (in opentv.c: opentv_find_entry -> _entry_cmp)

It seems to use opentv_find_entry when trying to link a summary to an existing event, and this is where things start to go wrong. It also uses the same "_entry_cmp" function when looking where to insert new events into the RB array.

As it's suggested that the code has worked previously, maybe Sky made changes whereby an event id is now only unique for a given channel.

With the following patch, we can make Tvheadend compare using a combination of eid and cid:

diff --git a/src/epggrab/module/opentv.c b/src/epggrab/module/opentv.c
index fa73927c6..7fde6d426 100644
--- a/src/epggrab/module/opentv.c
+++ b/src/epggrab/module/opentv.c
@@ -140,6 +140,7 @@ typedef struct opentv_event
   char                  *desc;        ///< Event description
   uint8_t                cat;         ///< Event category
   uint16_t               serieslink;  ///< Series link ID
+  int                    cid;         ///< Channel ID
 } opentv_event_t;

 /* Queued (unresolved) event entry */
@@ -202,20 +203,30 @@ static void _opentv_event_free(opentv_event_t *ev)
   free(ev->desc);
 }

-/* Compare event id codes */
+/* Compare event id and channel id codes */
 static int _entry_cmp ( void *a, void *b )
 {
-  return (int)(((opentv_entry_t*)a)->event.eid) -
-         (int)(((opentv_entry_t*)b)->event.eid);
+  char str_cmp_a[10], str_cmp_b[10];
+
+  sprintf(str_cmp_a, "%d%u",
+          ((opentv_entry_t*)a)->event.cid,
+          ((opentv_entry_t*)a)->event.eid);
+
+  sprintf(str_cmp_b, "%d%u",
+          ((opentv_entry_t*)b)->event.cid,
+          ((opentv_entry_t*)b)->event.eid);
+
+  return atoi(str_cmp_a) - atoi(str_cmp_b);
 }

 /* Find event entry */
-static opentv_entry_t *opentv_find_entry(opentv_status_t *sta, uint16_t eid)
+static opentv_entry_t *opentv_find_entry(opentv_status_t *sta, uint16_t eid, int cid)
 {
   opentv_entry_t *oe, _tmp;

   if (sta == NULL) return NULL;
   _tmp.event.eid = eid;
+  _tmp.event.cid = cid;
   oe = RB_FIND(&sta->os_entries, &_tmp, link, _entry_cmp);
   return oe;
 }
@@ -341,6 +352,7 @@ static int _opentv_parse_event
   }

   ev->eid = ((uint16_t)buf[0] << 8) | buf[1];
+  ev->cid = cid;

   /* Process records */
   while (i < slen+4) {
@@ -503,7 +515,7 @@ opentv_parse_event_section_one
     if (ebc) {
       save |= opentv_do_event(mod, ebc, &ev, ch, lang, &changes);
       if (!merge) {
-        entry = opentv_find_entry(mod->sta, ev.eid);
+        entry = opentv_find_entry(mod->sta, ev.eid, cid);
         if (entry) {
           save |= opentv_do_event(mod, ebc, &entry->event, ch, lang, &changes);
           opentv_remove_entry(mod->sta, entry);

I did find that on slower machines (e.g. an old x86_64 Revo 3600) I get the same "too much queued table input data" errors which causes gaps in the guide data so you may need a couple of scrapes to get the whole lot.

I was able to get around that by increasing the max amount of table data it queues from 2MB to 8MB:

diff --git a/src/input/mpegts/mpegts_input.c b/src/input/mpegts/mpegts_input.c
index bebb1de29..9c689f84d 100644
--- a/src/input/mpegts/mpegts_input.c
+++ b/src/input/mpegts/mpegts_input.c
@@ -1543,9 +1543,9 @@ mpegts_input_process
           if (type & MPS_FTABLE)
             mpegts_input_table_dispatch(mm, mm->mm_nicename, tsb, llen, 1);
           if (type & MPS_TABLE) {
-            if (mi->mi_table_queue_size >= 2*1024*1024) {
+            if (mi->mi_table_queue_size >= 8*1024*1024) {
               if (tvhlog_limit(&mi->mi_input_queue_loglimit, 10)) {
-                tvhwarn(LS_MPEGTS, "too much queued table input data (over 2MB) for %s, discarding new", mi->mi_name);
+                tvhwarn(LS_MPEGTS, "too much queued table input data (over 8MB) for %s, discarding new", mi->mi_name);
                 if (tvhtrace_enabled())
                   mpegts_input_analyze_table_queue(mi);
               }

I plan to submit a PR for these two changes, but would be good if anyone else is also able to test these patches first...

The 8MB-change won't make it in. Simply increasing the buffer is not a good idea, even slower systems would/could use an even larger buffer.....

The _entry_cmp function is written in the most inefficient way I could imagine. Also this kind of defeats the purpose of this kind of comparison. Please check if it is really necessary to return an int and if it isn't then just return a boolean and make the comparison easier. If it is necessary then please do so in a way that doesn't involve two string/int conversions.

Hi @Flole Systems,

OK - ignoring the "too much queued table input data" side-issue...

Looking at redblack.h seems it does require an integer to be returned from the nominated compare function.

Seems it uses this integer (+/-) to decide whether to navigate left/right when iterating the entries in RB_FIND, and also when inserting a new record in RB_INSERT_SORTED. Both of which are used by opentv.c.

I fully agree though - my initial attempt to compare based on a combination of eid and cid was inefficient... and ugly.

This version should work just as well and avoids those unnecessary conversions:

-/* Compare event id codes */
+/* Compare event id and channel id codes */
 static int _entry_cmp ( void *a, void *b )
 {
-  return (int)(((opentv_entry_t*)a)->event.eid) -
-         (int)(((opentv_entry_t*)b)->event.eid);
+  int eid_cmp, cid_cmp;
+
+  eid_cmp = (int)(((opentv_entry_t*)a)->event.eid) -
+            (int)(((opentv_entry_t*)b)->event.eid);
+
+  cid_cmp = ((opentv_entry_t*)a)->event.cid -
+            ((opentv_entry_t*)b)->event.cid;
+
+  return eid_cmp != 0 ? eid_cmp : cid_cmp;
 }

Does this look any better to your expert eyes?

I have created a pull request here with the modified (more efficient) version:

https://github.com/tvheadend/tvheadend/pull/1420

I tried your 'improved' patch against current git master on a Pi4 and it seemed to work; there were no mismatched titles / summaries and the EPG fully populated if left running long enough.

However I then installed an older version of Tvheadend - 4.3.1967 - without your patch and had exactly the same result!

Have Sky changed something since I last tried this in January?

"Have Sky changed something since I last tried this in January?"

I suspect we'll never know for sure as it's propriatary and undocumented (publically at least).

All I can say is that if the current code base was ever working correctly then I can only assume that Event IDs were previously unique across the whole guide; and maybe this changed (back in Jan?) to only be unique for a given channel.

However this is pure speculation...I simply debugged the current code, against the current opentv data, and found the issue.

I guess this is the best we can do when working with a propriatary format ;-)

A couple of days ago I put this patch on my production environment for some fuller tests.

The patch definitely resolves the issue of matching summaries onto the wrong events, so IMO could be merged.

However, I've now found a further issue...

The first scrape gets the whole guide, so you have complete summary data for all events.

However, subsequent scrapes are wiping the summary data for some events within the guide which were there previously, even when allowing it to scrape for the maximum 2hrs. Surely when updating an event it should retain the summary rather than updating it when it has a blank summary in it's hand. I think the answer to this separate issue probably lies somewhere within the matching logic here I'll try to find time to bottom out this additional issue at some point in the future, but am currently short on time available to further debug this.

I had not picked this up during my dev testing since I was removing the whole EPG database (epgdb.v3) each time before running a test to see that it was able to populate the whole guide from scratch.

I have now found a fix to the issue of summary data going missing after subsequent scrapes; was quite an easy 1-liner fix to the merge logic in the end.

I've opened the following pull request with a full write-up for consideration.

Well done!

Tested with Sky UK on RPi3 and confirmed working.