Modify

Opened 3 years ago

Closed 9 months ago

Last modified 4 months ago

#11739 closed Bug Report - General (fixed)

EIT over the air program guide title description mismatch FIXED/SOLVED

Reported by: deadletterfile@… Owned by: dekarl
Priority: minor Milestone: 0.28
Component: MythTV - ATSC Version: Master Head
Severity: medium Keywords: EIT title description
Cc: stuarta Ticket locked: no

Description (last modified by dekarl)

Summary: Eliminating two lines of code from EITHelper::ProcessEvents (void) eliminates 99% of Mythtv generated title/description mismatches in the over the air program guide

  • eitList_lock.unlock();
  • eitList_lock.lock();

I believe the use of QMutexLocker in function uint EITHelper::ProcessEvents (void) as currently implemented is incorrect, allowing a threaded CPU to run concurrent processing of events (individual program listings). For a given channel, this allows descriptions from other programs to be associated with the wrong program title.

libs/libmythtv/programdata.cpp For the same program listing as one previously processed, tests the length of new program description against the current stored description, replacing old with new if new is longer. I had a case where a long incorrect description was not replaced by a subsequent, shorter correct description. Possible fix in the patch.

The patch is against the master branch: commit 619d87bdc8a (Aug 8, 2013)

My fix for ticket #11476 is also in the patch as it stabilized use of the EIT scanner for me.

sha1sum /data/wrkbin/computer/libmythtv.patch 922fb3eedc55c65de7b97328a7c5215d37b13060

Anyone testing the patch should be vigilant in examination before applying. Code changes (ignoring logging) are few enough to be made manually. Place the patch in the mythtv/libs/libmythtv directory and apply:

patch -Np1 < libmythtv.patch

With this patch installed, my EIT scanner does not unnecessarily abort. Event/program information taken from the broadcast data stream correctly populates the Mythtv program guide. Titles and descriptions match. I am very happy with the results and (until proven otherwise) consider this issue fixed/solved/closed. I have even installed custom recording rules based on description for the first time. Note that I do not believe these .cpp files have been altered in some time and a clean patch is perhaps likely on older 'master' branches or even 0.26 and 0.26/fixes.

Attachments (9)

libmythtv.patch (13.4 KB) - added by Ronald L Humble (Royboy626) <deadletterfile@…> 3 years ago.
patch for <tv_rec programdata eitscanner eithelper>.cpp
libmythtv.140128.patch (15.5 KB) - added by deadletterfile@… 3 years ago.
patch to stabilize EIT over-the-air broachast schedule
libmythtv.140430.patch (15.1 KB) - added by deadletterfile@… 2 years ago.
astc EIT fixes #11476 #11739; off commit befdc4d4b9
151110.eithelper.cpp (330 bytes) - added by deadletterfile@… 10 months ago.
Eliminate use of ETT cached data
151112.tv_rec.patch (430 bytes) - added by deadletterfile@… 10 months ago.
Eliminate use of ETT cached data
eit_ett_staleness_fix_20151120.patch (8.1 KB) - added by carlpny <carlpny@…> 9 months ago.
Proposed bug fix for EIT/ETT mismatched.
eit.160127.patch (850 bytes) - added by deadletterfile@… 7 months ago.
patch for EIT/ETT mismatches (trimmed from libmythtv.140430.patch)
mutex.160127.patch (1.1 KB) - added by deadletterfile@… 7 months ago.
tv_rec.<h cpp> patch adding "return NULL" and single mutex locker
libmythtv.160508.v0.28.patch (2.8 KB) - added by deadletterfile@… 4 months ago.
libmythtv patch for v0.28

Download all attachments as: .zip

Change History (63)

Changed 3 years ago by Ronald L Humble (Royboy626) <deadletterfile@…>

patch for <tv_rec programdata eitscanner eithelper>.cpp

comment:1 Changed 3 years ago by deadletterfile@…

I am an idiot and a fool. Alas, the Internet is forever.

I still hope the programmers will look at the QMutexLocker implementation in ProcessEvents as I believe this change has merit.

However my original coding removed:

if (match.description.length() >= ldesc.length())

ldesc = match.description;

from processdata.cpp. This worked well until a block of channel listing lost their program descriptions. The patch solution was no better and I have returned the above code to service. It appears a bad program description once broadcast by a channel, but later corrected, will likely not be correct in the Mythtv EIT program listings.

I changed complex code in programdata.cpp before understanding it. I feel like further improvement in title/description mismatches can be made in programdata.cpp, and will make the attempt in the coming months. I still think I can contribute, but will be more careful and measured in my response.

I apologize to the Mythtv community and especially the programmers.

comment:2 Changed 3 years ago by stuarta

  • Owner set to stuarta
  • Status changed from new to assigned

I'm guessing from the previous comment that this ticket can be closed?

comment:3 Changed 3 years ago by deadletterfile@…

Thank you for you response. The problem persists. I was simply stupid and hasty in working with the code. I hope someone might still address the program/description mismatches that occur with EIT data in Mythtv. I have found the atsc_epg program from linuxtv.org dvb-apps useful. I am still working with it but it seems to provide superior results to Mythtv. After several months, I am finally looking at this issue again. Obviously it is your call as to closing the ticket, but program/description mismatch using broadcast EIT data it still a problem for me with the latest stable version of Mythtv.

I do not think #11476 has been addressed either. My changes have completely stabilized the display of EIT program data for me. Let me know if I can provide any assistance on either of these issues. -- Royboy626

comment:4 Changed 3 years ago by stuarta

Hi,

Not planning to close it, I was trying to better understand the problem in order to verify that your fixes are correct. I need to understand the signalmonitor changes better and try to work out why they have an influence on your issue.

comment:5 Changed 3 years ago by stuarta

  • Milestone changed from unknown to 0.28

comment:6 Changed 3 years ago by deadletterfile@…

I apologize for my patch being a unified patch of my Mythtv changes regarding bugs 11739 and 11476. It confused things for the Mythtv programmers trying to address these potential issues.

I have two avenues to which you might address your attention, in addition to any you investigate.

1) I reverted back to eliminating the three lines of code mentioned in comment #1. I am not sure of the programmer's original intent, but have been pleased with the result of omitting these lines in eliminating mismatches in Mythtv.

2) At the same time, I changed set up in mythtv-setup to only use one tuner card for EIT data collection. I believe multiple cards collecting EIT data can cause program/description mismatches.

I still strongly believe the implementation of the QMutexLocker is flawed in eithelper.cpp -> EITHelper::ProcessEvents. I eliminated these two lines:

eitList_lock.unlock(); eitList_lock.lock();

from the function. It appears to me use of the above lines would apply to QMutex code, and should not be used. If I am correct, perhaps this bug exists elsewhere in the code. Removal of these lines seems to solve 2) for me.

http://qt-project.org/doc/qt-4.8/qmutexlocker.html

Details...

I made two changes after posting comment #1 and have been running on those changes since then and am very pleased with the result. 1) I went back to commenting out the three lines of code in programdata.cpp DBEvent::UpdateDB as noted below:

if (match.subtitle.length() >= lsubtitle.length())

lsubtitle = match.subtitle;

// if (match.description.length() >= ldesc.length()) // if (match.description.length() >= 2) // ldesc = match.description;

if (lcategory.isEmpty() && !match.category.isEmpty())

lcategory = match.category;

2) I have 22 stations on about 15 transponders. I have been using only one tuner card to collect EIT data for months. Over 48 hours, in casually attempting to find a program/description mismatch, I could find only one. Last night I set all tuner cards to collect EIT data. This morning, program/description mismatches were common, but only on one transponder (with three stations). This was decidedly NOT the behavior months ago, it was over all transponders. However this was a short test. I returned to using only one card for EIT collection, waited an hour for EIT data refresh, and all program/description mismatches resolved themselves. I have never had mismatched descriptions cross channels or transponders. If a program had a mismatched description, it was always from another program on that channel.

After making this test, I discovered the two lines from 2) had crept back into my code. I recompiled and retested with multiple cards gathering EIT data. It has been two hours and no program/mismatches have been found. I admit this is a short test but hints at 2) being an issue if multiple cards are gathering EIT data. Thank you for your time.

I look forward to your investigation of these issues. I will be happy to assist if requested.

Changed 3 years ago by deadletterfile@…

patch to stabilize EIT over-the-air broachast schedule

comment:7 Changed 3 years ago by deadletterfile@…

mythbackend version: (detached from f19d9f8) [v0.28-pre-755-gf19d9f8-dirty]

Attached is libmyth.140128.patch. It is a unifed patch of my changes which have stablized the use of over-the-air broadcast information on an atsc broadcast stream. The patch is against the master branch updated to commit f19d9f8062 (Jan 27,2014). As it includes proposed fixes for submitted bugs 11476 and 11739 I will attempt to briefly comment only on changes related to title/description mismatches here.

The latest adjustment was the elimination of the ETT cache in EITHelper::AddETT

What I theorize has been happening is EIT/ETT mismatches have occurred at times when tables were updated in the broadcast stream. I do not think EIT or ETT are checked against the version_number in the latest MGT, therefore potentially an old table could be used. This theory matches the observation of others that the problem gets worse with time (as more stations encounter this problem), and after reboot, there are initially no new title/description mismatches (with the UpdateDB function adjusted as below).

I have run the patch on gf19d9f8-dirty for four days (and for 48 hours on my prior build) and have seen no degradation in service. At least in my case, the ETT caching seems unnecessary. Alternately, caching could be checked and selectively purged before use, or the Add<ETT EIT> functions could be synced and process data only after the latest ETTs and EITs are known to exist.

As earlier reported, I continue to eliminate the overwriting of program/event descriptions based on length in the 'uint DBEvent::UpdateDB' function, although I admit the original intent of the code escapes me.

Other changes to eithelper.<h cpp> eitscanner.cpp and atscstreamdata.cpp have to do with converting QMutex to QMutexLocker or with bug 11476 and my attempts to resolve that issue. I admit to being less than a newbie with locking. The attempt was to clarify code and reduce the risk of thread collisions. I do not vouch for their integrity, however I am having no issues.

At this time, with this patch installed, all my broadcast schedule issues are solved. Scheduling is working optimally for me.

Thank you for your time. - RoyBoy626

comment:8 Changed 3 years ago by stuarta

  • Cc stuarta added
  • Owner changed from stuarta to dekarl

Changed 2 years ago by deadletterfile@…

astc EIT fixes #11476 #11739; off commit befdc4d4b9

comment:9 Changed 2 years ago by deadletterfile@…

libmythtv.140430.patch patches v0.27.1 (commit ecfefabd22 - May 26,2014). ATSC EIT Program Guide works with no detected issues.

comment:10 Changed 2 years ago by dekarl

  • Component changed from MythTV - General to MythTV - ATSC

comment:11 Changed 2 years ago by deadletterfile@…

libmythtv.140430.patch (Comment #7) patches v0.27.3 (commit 9d871a999c - July 4,2014). ATSC EIT Program Guide works with no detected issues.

comment:12 Changed 22 months ago by deadletterfile@…

libmythtv.140430.patch (Comment #7) patches v0.27.4 (commit e4f65c8797 - Oct 15, 2014). ATSC EIT Program Guide works with no detected issues.

comment:13 Changed 18 months ago by sagaciouskjb@…

I just applied this patch against v.0.27.4 ( forgive me but I don't know how to reference commit number, I just pulled it from the repository last night ) and noticed that the "browse" mode still sometimes mismatches the description when watching LiveTV. In my case it mismatched the description of "Adventure's of Superman" with the episode of "Bonanza" that had aired on the same channel the previous hour ( a full hour ). However when I loaded the actual EPG it showed the correct description, and when I dropped back from the EPG to the browse mode it had updated as well.

comment:14 Changed 18 months ago by deadletterfile@…

Thanks for the report. Did you wait until completely new listings were imported? If a station listing goes out 12 hours, wait 14 hours or so and check the listings. I have 23 stations, and in casual almost daily use over ten months, I have noted only two mismatches. There is still at least one bug I have not discovered, but for me, the improvement by applying libmythtv.140430.patch has been dramatic. In my amateur opinion, one problem that greatly reduced mismatches was in EITHelper::AddETT. The program caches ETT data, but the data can be accessed when it is old. A missing return statement in TVRec::TuningSignalCheck? (I think) causes the EIT grabber to crash. Third (and where undiscovered bugs perhaps are located) was improper mutex locking.

comment:15 Changed 18 months ago by sagaciouskjb@…

Yeah, but it did happen shortly after I thought they were all done, so perhaps some were still download/updating. I haven't noticed a repeat of the behavior since. Seems to be working 100% in the 24 hours since my comment. Thanks for the fix

comment:16 Changed 17 months ago by geoffp@…

For what it's worth, I've been running the most recent patch for this against the current Ubuntu build (mythtv-0.27.1+fixes.20140624.aa822f5) for about a month with no apparent issues. It's a dramatic improvement for me.

Even if it's not 100% correct yet (though it appears correct for me), it's a major UX improvement over every description being wrong all the time. Kudos to deadletterfile and all testers.

comment:17 Changed 17 months ago by ironstorm@…

I'm also running Ubuntu, however my results are not good.

I tried:

  • mythtv-0.27.1+fixes.20140624.aa822f5 + libmythtv.140430.patch (stock ubuntu mythtv package)
  • mythtv_0.27.4+fixes.20150305.f1115fc-0ubuntu0mythbuntu2 + libmythtv.140430.patch (mythbuntu package)

The results are the same with each, EIT information is fixed each time the patch is applied - which is good because the vast majority of my EIT info is garbage right now (it's rare when any titles and descriptions match up). However after a few minutes of running, mythbackend begins to consume all available CPU (~144% on my dual core chromebox), this continues even after the backend goes "idle" (no recording, flagging, transcoding, watching, etc).

A restart brings the CPU back in line, but it quickly goes back up again. The original un-patched packages do not suffer from this problem. This level of CPU consumption make watching on a local mythfrontend impossible due to choppiness, drop outs and errors. Reverting back to the stock packages makes the CPU problem go away.

My set-up is:

  • Asus Chromebox serving as backend and front end
  • 1 HDHomeRun Plus (onboard transcoding) -> OTA antenna
  • 1 HDHomeRun Original (no transcoding) -> same OTA antenna

comment:18 Changed 17 months ago by geoffp@…

My set up is:

  • Separate backend on Ubuntu Server, Kodi frontend
  • HDHomeRun Dual (ATSC/North America)

Maybe the differential is the second tuner? Or a second, very similar tuner on the same source?

comment:19 Changed 17 months ago by Wolfgange <s.matthew.31@…>

I'm a bit confused, where exactly is the mythtv folder that has libs/libmythtv? I cannot find a libs folder in /var/lib/mythtv/.

comment:20 Changed 17 months ago by ironstorm@…

@geoffp,

I just did a test, disconnected the power to my HDHomeRun Original (it's a Dual Tuner, but is white with 2 coax ports), then I reinstalled the patched packages of mythtv_0.27.4+fixes.20150305.f1115fc-0ubuntu0mythbuntu2 and started everything up.

So far mythbackend has been behaving for the past 2 hours without any trace of the CPU issue. My guess is the changes in this patch relating to locking/unlocking/mutexes tuners have some kind of problem.

@Wolfgange, its inside the mythtv source folder... Here's how I built it and installed it

cd ~/source
curl -O https://code.mythtv.org/trac/raw-attachment/ticket/11739/libmythtv.140430.patch
sudo apt-get update
sudo apt-get source mythtv
cd mythtv-0.27.4+fixes*/mythtv/libs
cat ../../../libmythtv.140430.patch | patch -p0
cd ../..
sudo debian/rules binary
cd ..
for m in $(dpkg --get-selections | grep myth | cut -f1); do sudo dpkg -i $m*.deb; done

comment:21 Changed 17 months ago by ironstorm@…

@geoffp, forget what I said ... It's back to using 130%+ CPU doing nothing... looks like the patch is bad, just a matter of timing before myth backend goes nuts. :(

29010 mythtv     20   0 3658M 63612 29128 S 132.  0.4 29:27.21 /usr/bin/mythbackend --syslog local7 --user mythtv --daemon --noupnp

comment:22 Changed 17 months ago by ironstorm@…

@geoffp, nevermind... now I'm getting the CPU overload with my one tuner on the stock packages... maybe this is something to do with the HDHomerun Plus... :(

comment:23 Changed 17 months ago by stuartm

For dual tuners, try disabling EIT collection on just one of them. Some drivers/hardware don't handle repeated retuning on two tuners simultaneously. Technically it's possible for this to occur through normal usage, but the probability increases greatly with EIT scanning since it's constantly changing frequencies.

Last edited 17 months ago by stuartm (previous) (diff)

comment:24 Changed 17 months ago by geoffp@…

Good sleuthing, though, @ironstorm -- what appeared to be a problem with the patch was found not to be. That gets us closer to the truth.

comment:25 Changed 17 months ago by geoffp@…

Interestingly -- I did glance at CPU usage on my backend, and to my surprise, mythbackend was at 100%, but it subsided after a few minutes. I seem to remember that CPU usage spikes when it's doing rescheduling. I've been watching to see if I can catch it happening again so I can correlate it with log entries, but I haven't seen it happen since.

I have seen plenty of EIT scanning activity in the log since then, though, which does *not* correlate with a CPU usage spike. Therefore, I can say with very high confidence that with patches applied, EIT scanning activity doesn't always spike CPU, and I think I can say with pretty decent confidence that I've never even seen it happen once.

comment:26 Changed 17 months ago by geoffp@…

Update: I caught one of the CPU spikes. It lasted about 14 minutes, and correlates with this block of log entries:

Mar 31 10:02:52 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2139 (HandleReschedule) Reschedule requested for MATCH 0 1 4 2015-04-02T14:50:00Z EITScanner
Mar 31 10:02:52 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2252 (HandleReschedule) Scheduled 44 items in 0.0 = 0.01 match + 0.00 check + 0.03 place
Mar 31 10:05:34 basementcat mythbackend: mythbackend[10810]: N Expire autoexpire.cpp:264 (CalcParams) AutoExpire: CalcParams(): Max required Free Space: 90.0 GB w/freq: 15 min
Mar 31 10:08:15 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2139 (HandleReschedule) Reschedule requested for MATCH 0 1 10 2015-04-03T14:00:00Z EITScanner
Mar 31 10:08:15 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2252 (HandleReschedule) Scheduled 44 items in 0.0 = 0.01 match + 0.00 check + 0.03 place
Mar 31 10:13:38 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2139 (HandleReschedule) Reschedule requested for MATCH 0 1 1 2015-04-02T14:30:00Z EITScanner
Mar 31 10:13:38 basementcat mythbackend: mythbackend[10810]: I Scheduler scheduler.cpp:2252 (HandleReschedule) Scheduled 44 items in 0.0 = 0.01 match + 0.00 check + 0.03 place

The backend CPU is an Intel(R) Core(TM)2 Duo E8400 @ 3.00GHz. Not a speed demon by today's standards.

comment:27 Changed 12 months ago by deadletterfile@…

libmythtv.140430.patch (Comment #7) patches v0.27.5 (commit ad97d2435230 - Jun 16, 2015). ATSC EIT Program Guide works quite well. I have used this patch for almost 17 months. With almost daily use I have noted about six title/description mismatches in that time. The patch is not perfect, but nearly so. No issues with this patch detected.

I hope dekarl is happy and well.

I have also noticed a race condition at times with mythbackend. I have seen code (where, I have forgotten) where developers attempted a fix. I think it could well be related to using EIT decoding but I have not had time to investigate.

comment:28 Changed 12 months ago by jtlpa@…

2015-09-10 with a fresh/clean install of MythTV 27.5 on Debian 8.1 using deb-mulitmedia, I still find that ATSC EIT descriptions do not match what is actual broadcast over the air.

It appears to happen when the same show has different episodes within a 24 hour time period.

For example on GRIT TV MythTV is set to record all episodes, however, the descriptions for a particular episodes will be for an episode that aired previously and can be on to two episodes off.

comment:29 Changed 12 months ago by deadletterfile@…

jtlpa,

Developers of Mythtv have not yet addressed this patch or a fix for the EIT problem. libmythtv.140430.patch (available above) can be applied to the source tarball (easy) or a git source tree (more knowledge involved). Below is a script I call patchmyth that is used with the git source tree properly pointing to v0.27.5. Paths would need to be adjusted. However the key points can be seen: libmythtv.140430.patch is placed in mythtv/libs/ and the command "patch -Np1 < ../libmythtv.140430.patch" is run from mythtv/libs/libmythtv

Compiling a program and going through "dependency hell" in the configuration stage is not beyond the average computer user. Backup your Mythtv database first as a precaution. Once it is set up, the process is quite easy after that. The compiled programs install under /usr/local and running "make uninstall" from mythtv/ will make the compiled version disappear, leaving you with your distribution's install of Mythtv.

There is also a page in the Mythtv wiki regarding EIT (I believe a post there was not using the latest patch referenced here). Even if you know next to nothing about code, look through the patch before applying to see that it indeed will not erase your hard drive nor consume your cat or houseplants.

#!/bin/bash
echo "[Ctrl] [C] to exit now"
echo "Usage: patchmyth [checkout | patch ]"
echo "'checkout' checks out clean git files so 'git checkout' may be used to update the source tree."
echo "'patch' will patch the clean source with RLH's EIT patch fixes"
read -p "[Ctrl][C] to exit; [Enter] to continue..."
if [ "$1" == "checkout" ]; then

echo "cd /data/packages/mythtvgit"
cd /data/packages/mythtvgit
echo "git checkout mythtv/libs/libmythtv/eithelper.cpp mythtv/libs/libmythtv/eitscanner.cpp mythtv/libs/libmythtv/eitscanner.h mythtv/libs/libmythtv/mpeg/atscstreamdata.cpp mythtv/libs/libmythtv/programdata.cpp mythtv/libs/libmythtv/tv_rec.cpp mythtv/libs/libmyth/programinfo.cpp"
git checkout mythtv/libs/libmythtv/eithelper.cpp mythtv/libs/libmythtv/eitscanner.cpp mythtv/libs/libmythtv/eitscanner.h mythtv/libs/libmythtv/mpeg/atscstreamdata.cpp mythtv/libs/libmythtv/programdata.cpp mythtv/libs/libmythtv/tv_rec.cpp mythtv/libs/libmyth/programinfo.cpp

elif [ "$1" == "patch" ]; then

echo "cd /data/packages/mythtvgit/mythtv/libs/libmythtv"
cd /data/packages/mythtvgit/mythtv/libs/libmythtv
echo "patch -Np1 < ../libmythtv.140430.patch"
patch -Np1 < ../libmythtv.140430.patch
echo "cd /data/packages/mythtvgit/mythtv/libs/libmyth"
cd /data/packages/mythtvgit/mythtv/libs/libmyth
echo "Patch below not needed at this time - RLH"
echo "patch -p0 programinfo.cpp < ../programinfo.cpp.patch"

# patch -p0 programinfo.cpp < ../programinfo.cpp.patch
fi

comment:30 Changed 12 months ago by jtlpa@…

deadletterfile@… Wow, thanks for fast response. Was not aware developers of Mythtv have not yet addressed this patch. Thanks for the suggestions, but will have to wait until they get around to it. Any idea when MythTV developer will implement the patch?

comment:31 Changed 12 months ago by stuartm

One of the reasons the patch hasn't been committed is because it changes a lot more than it needs to resolve this particular bug. This makes reviewing much more difficult. The best patches change as little as possible, and certainly nothing which doesn't relate to the bug. It also comes with a precise explanation of the bug and comments explaining the intent of each change.

The ticket itself is long, full of conjecture, discussion and 'me toos' and that again makes it difficult for a reviewing developer to pick out the relevant information. Even once reviewed, the patch then needs testing on multiple systems of varying speeds - important because it changes locking which can cause race conditions leading to deadlocks on fast or slow machines. It also has to be tested with EIT from multiple different sources - networks and countries to ensure it doesn't cause regressions. This all takes time.

comment:32 Changed 12 months ago by jtlpa@…

Stuartm Thanks for the quick reply and understand what you are saying.

Don't know if this will help you, but this has been an issue for me for a long long time . . .

My history of MyTV usage:

Mythbuntu v.25 & v.26 (2013 [just one tuner] through 2014 [then two tuners]) MythTV .27 on Debian, sourced from deb-multimedia.org (mid 2014 forward [two tuners])

Also sometimes the program description come from a different show on another channel.

I have never used Schedules Direct and always used the off-the-air guide and not connected to cable or satellite.

Hope that help you JTL

comment:33 Changed 12 months ago by geoffp@…

stuartm, can you give us some idea of what in the patch is out of scope for this bug? Would you want the whitespace changes, logging code, and commented out blocks removed?

Or, should we close this and open a new ticket that summarizes all the long-term testing results and experimentation that's been done in this one?

comment:34 Changed 12 months ago by deadletterfile@…

I apologize for gumming up the works with a poor, overly complex patch. I have only a "production" machine and WAF is a big concern. I got something that almost completely solved my problems with EIT and shared it in hope of having the developers use the patch as a guide to inquiry. Stuartm's criticism was very helpful.

In the patch, libmythtv/tv_rec.cpp: the addition of "return NULL;" in the function MPEGStreamData *TVRec::TuningSignalCheck?(void) is a "one-liner" and was documented in bug #11476 and more properly belongs there. I believe the absence of this one line may be totally responsible for my EIT scanner crashing, leading to missing program guide data during times of Mythtv inactivity.

I renamed variable DESC to DESCR as (I believe) DESC is a reserved mysql variable. The rename could potentially avoid confusion.

The randomization of channels I did was before I discovered the above "return NULL" and was my attempt to not lose program data by attempting to update the oldest channels first. If the "return NULL" fixes the problem of EIT crashes, then this is unnecessary and should be ignored/removed. Not doing so was laziness on my part.

The removal of caching of ETT data appears to have solved a big source of title/description mismatches.

I believe everything else was my attempt, however poorly done, to completely solve the title/description mismatch. I believe until proven otherwise, mutex locking is a part of the problem (and has not been completely solved by the submitted patch). It is more than possible that I did something unnecessarily or even made things worse. I am not much of a programmer.

If stuartm indicates progress on EIT would be better served by submitting smaller patches, I can attempt to do that. Otherwise, I will try and minimize the damage done and stay out of it.

To my knowledge, no Mythtv developers are paid to work on Mythtv. I understand there are priorities and what I would like to see fixed is perhaps low on their list. I hope I have exhibited patience, and apologize if that had not been the case.

comment:35 Changed 12 months ago by stuarta

I'm going to attempt to pull the patch apart in to the various different bits which may or may not contribute to resolving the issue, and then get our willing bunch of testers to try out the combinations and see which work. :)

comment:36 Changed 12 months ago by deadletterfile@…

Perhaps this is obvious, but I respectfully suggest testers have at least two tuners collecting EIT data to properly test for any mutex locking changes.

If it is suggested that only one tuner should collect EIT data, I would be happy to file a bug/feature request to restrict the user from selecting more than one card for EIT data collection.

comment:37 Changed 11 months ago by markfriesen604@…

I'm not a programmer so I can't help with any of this other than say I too have a dual tuner card system (2 hvr-1250 -ATSC reception.) I have also had hit and miss EIT collection. When my guide hasn't populated correctly I can look at the individual EPG data (as viewed from a Kodi PVR plugin) and all of the upcoming data is there. The guide still remains incomplete even after forcing the EPG update by viewing the data as described. Anyway reading the thread here I decided to turn off EIT collection on the second card. The guide immediately populated better than it has been for the last 2 days. I'll check this for a few more days and report back.

Changed 10 months ago by deadletterfile@…

Eliminate use of ETT cached data

comment:38 Changed 10 months ago by deadletterfile@…

151110.eithelper.cpp is split from libmythtv.140430.patch. If I recall correctly, this code snippet eliminates the use of cached ETT data. If I am correct, there was no check on ETT data before use, and old ETT data could be used during the time of new EIT/ETT transmission. Instead of adding complicated checking, I simply eliminated use of the cached ETT data. This improved program title/description matching (but does not eliminate mismatches due to [possible] improper locking of EIT scanner threads).

Place 151110.eithelper.cpp in libmythtv and execute: patch -p0 eithelper.cpp < 151110.eithelper.cpp

Changed 10 months ago by deadletterfile@…

Eliminate use of ETT cached data

comment:39 Changed 10 months ago by markfriesen604@…

Noob question, how do I apply this patch?

comment:40 Changed 10 months ago by deadletterfile@…

The complete patch to apply is libmythtv.140430.patch​ (available above). At least two people seem to have success with it. MStuart wanted a patch in smaller pieces and 151112.tv_rec.patch is a subset of the April patch.

You must get the source from Git or tarball for 0.27.5. libmythtv.140430.patch is placed in mythtv/libs. A Bash script in comment 29 would need to be adjusted for you paths. However instead, you could change directory to mythtv/libs/libmythtv then execute: patch -Np1 < ../libmythtv.140430.patch If there are errors it should be evident.

Output might be similar to: patch -Np1 < ../libmythtv.140430.patch

patching file eithelper.cpp

patching file eitscanner.cpp

patching file eitscanner.h

patching file mpeg/atscstreamdata.cpp

patching file programdata.cpp

Hunk #1 succeeded at 488 (offset -118 lines).

Hunk #2 succeeded at 534 (offset -121 lines).

Hunk #3 succeeded at 554 (offset -121 lines).

Hunk #4 succeeded at 1207 (offset -194 lines).

Hunk #5 succeeded at 1233 (offset -195 lines).

patching file tv_rec.cpp

Hunk #1 succeeded at 1266 (offset -1 lines).

Hunk #2 succeeded at 1419 (offset -1 lines).

Hunk #3 succeeded at 3941 (offset -1 lines).

comment:41 Changed 10 months ago by deadletterfile@…

markfriesen604:

Of course then you would have to compile and install Mythtv (instructions elsewhere on the Mythtv site). It is not really hard but can take a little effort the first time if you have never done such things.

This is not a forum, so if you have issues bring it up in a forum web site. Good Luck.

Changed 9 months ago by carlpny <carlpny@…>

Proposed bug fix for EIT/ETT mismatched.

comment:42 Changed 9 months ago by carlpny <carlpny@…>

I've had this problem for a while, so I spent some time looking through the code. I believe that there is a single bug that is responsible for most (probably all) title/description mismatches. Looking back through the comments, deadletterfile roughly identified the problem as being stale eit data. I believe deadletterfile is correct, and I will describe below how there is a particular aspect of how eit data is matched that exacerbates the problem with stale data.

Background:

The atsc standard separates the Event information (EIT) (title, start time, etc) from the ETT (detailed description). They are separate tables ("table" here not in the sense of mysql, but in the sense of broadcast data) and they come at different times while a channel is tuned.

These two tables are joined via an integer "event_id" key.

To perform this join, the EITHelper holds two mappings in memory:

  • incomplete_events: source to event_id/EIT - (title, start time, etc)
  • unmatched_etts : source to event_id/ETT - (description)

As an EIT or ETT table is processed, these two maps are referenced and updated. When processing an EIT event, if there is already an ETT present in unmatched_etts, then the two values are joined and processed. If not, then the event added to incomplete_events for subsequent processing. A similar process is followed when handing ETT data.

Observed bug behavior:

In the early lifetime of a fresh event database, the events appear healthy and show strictly matching titles and descriptions.

Over an unknown time period, as the EIT is updated, gradually, the EIT table appears to be corrupted. Although all the events appear to have the correct time and title, the descriptions are typically copied from some other show *in the same channel*.

Others have reported that this error happens when they have dual tuners, although I speculate that to be a red herring. Either way, I have not investigated that aspect, and I have not tried parsing through the complicated locking behavior of EIT.

Root Cause:

The EITHelper's lifespan is for the duration of the mythbackend process. The unmatched_etts and incomplete_events maps are member variables of the EITHelper. However, these maps are never cleared. Their values are only deleted when a successful match is found.

Here is a description of a common scenario to leading to mismatched title/description.

  • mythbackend runs and starts scanning for EIT at regular intervals.
  • The Foo channel is tuned.
  • The Foo channel gets an EIT event, and adds it to "incomplete_events".
  • The Foo channel gets an ETT event with description A, finds the corresponding EIT event, and completes the event.
  • The Foo channel gets a second ETT event with description A, and adds it to "unmatched_etts".
  • The channel is changed as the scan proceeds. "unmatched_etts" still contains A.
  • Some time later the scan again reaches the Foo channel. By this time, the broadcaster has recycled some of their event_ids to correspond to different shows.
  • The foo channel gets an ETT event with *updated* description B. However, there is a check inside the EITHelper that says, if an unmatched ett already exists for a given event ID, don't overwrite it, so the A value is retained. Here is the check: https://github.com/MythTV/mythtv/blob/fixes/0.27/mythtv/libs/libmythtv/eithelper.cpp#L209
  • The Foo channel sends an EIT event. There is a matching event_id already present in "unmatched_etts", so the Foo channel completes the event with erroneous description A, even though description A actually corresponds to some event reported a long time in the past.

Proposed FIx:

Barring some larger refactor, I am attaching a patch which does two things:

  • Allows unmatched_etts to be overwritten by fresh data.
  • Ignored ETTs and EITs that are unreasonably old.

The patch includes a debug statement which is a smoking gun for the problem. The output below demonstrates that an old text description shared the same event id with a newer event. Previously, the old text description would have been retained and reused.

2015-11-20 01:13:01.360345 I [29224/29509] HDHRStreamHandler eithelper.cpp:239 (AddETT) - EITHelper: Overwriting previously unmatched ett. stale: 1 major: 44 minor: 1 old ett: Paid programming. new ett: News and features with focus on local news, traffic and weather.

Testing:

I have tested the patch on the latest 0.27 branch, and I have verified that the patch will at least apply cleanly in the master branch. I have not tried running this for an extended period of time, and I cannot say that there are no other mismatch bugs. However, the change is isolated, and I don't expect it to cause any regressions.

Applying the Patch:

Assuming you are using the 0.27 branch and git, do the following:

comment:43 Changed 9 months ago by carlpny <carlpny@…>

I have left my backend running for a couple of days with my proposed fix eit_ett_staleness_fix_20151120.patch. EIT continues to function correctly, and the mismatched description problem appears resolved. You can can compare before and after for the same channel in these two screenshots:

I believe this patch is low risk and well-targeted to this bug. Any thoughts on committing it?

comment:44 Changed 9 months ago by Karl Dietz <dekarl@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In e7a20aa6235de469a5a5589ad8f0aaa79a822a3f/mythtv:

Fix handling of ATSC EIT/ETT

A longer explanation can be found in the ticket.
https://code.mythtv.org/trac/ticket/11739#comment:42

Patch by Carl Nygaard
Fixes #11739

comment:45 Changed 9 months ago by Sagaciouskjb@…

Either git apply didn't patch the files correctly with the given behavior, or I am still getting mismatched descriptions. The program names are right Hut the description contains data for other programs.

comment:46 Changed 9 months ago by carlpny <carlpny@…>

I assume you're talking about eit_ett_staleness_fix_20151120.patch? It's possible there are other things that can cause mismatching descriptions. e.g. I did not look into the locking issues that deadletterfile mentioned. You can see from the screenshots in comment:43 they were really bad for me, but seem essentially fine now.

Descriptions that were already in the database from before you patched will continue to be incorrect. I'll assume you waited around for the EIT to update per comment:14.

One thing you can try is running the patched mythbackend as follows:

mythbackend --loglevel debug -v general,eit --logpath /tmp/mythtv/

After a long time (several hours to a day), the logs should eventually show messages like this:

2015-11-20 01:13:01.360345 I [29224/29509] HDHRStreamHandler eithelper.cpp:239 (AddETT) - EITHelper: Overwriting previously unmatched ett. stale: 1 major: 44 minor: 1 old ett: Paid programming. new ett: News and features with focus on local news, traffic and weather.

If you never see that message, then either your mismatched titles are caused by something else or your mythbackend was not correctly patched. If you do see that message, then I'd expect your mismatch problem to go away (or at least be greatly improved).

comment:47 Changed 9 months ago by Sagaciouskjb@…

Well, I'm pretty sure the patches applied now. I looked at eithelper.cpp and searched for the comment about removing old ett data, as well as the comment below it linking back to this page... So it appears the changes were all made correctly.

I would have to relaunch mythbackend manually to set the log level to debug to see if that ever shows up in the logs, my system startup scripts only set it to info.

However I also looked up when I applied the changes because I felt like it had been a while, and it was all the way back on November 23. Then on November 24th it started mismatching descriptions. It always uses a description from another program on the same channel a few hours or so previous.

I'll try relaunching with the appropriate log level after my backend has finished doing stuff and report back, anything I should look for besides that?

comment:48 Changed 9 months ago by carlpny <carlpny@…>

FWIW, you also need the -v general,eit (in addition to debug) so it prints out EIT-specific stuff.

One thing that might be helpful (although very spammy) is to print out each and every title/description found. e.g. inside EITHelper::CompleteEvent(), around this line:

https://github.com/MythTV/mythtv/blob/fixes/0.27/mythtv/libs/libmythtv/eithelper.cpp#L783

You could add something like this (untested):

        LOG(VB_EIT, LOG_INFO,
            LOC + QString("Adding EIT\ntitle:\n$1\nsubtitle:\n$2.")
                .arg(event.title).arg(ett));

Then, once you see a mismatched entry you can go and look in your logs and to see if the mismatch was indeed generated by eithelper.

comment:49 Changed 9 months ago by deadletterfile@…

Applied patch to fixes/v0.27 branch as per comment #42. Code examination shows patch applied. Waited 24 hours for fresh EIT data. Running two tuner cards collecting EIT data.

I am grateful for the superior work done on the patch and its inclusion in the master branch. It is a big improvement over the unpatched v0.27.5. However I must report three title/description mismatches in brief casual inspection.

Although inferior in thought and coding, my libmythtv.140430.patch is giving me superior results. I am not ready to dismiss thread locking as an issue here.

I am busy with other obligations, but will try to cull only the locking code from my patch and apply in addition to the staleness_fix patch and report. It may be a while though.

comment:50 Changed 9 months ago by dekarl

  • Description modified (diff)

comment:51 Changed 7 months ago by Sagaciouskjb@…

I have also reverted to using deadletters patch. I still have not set up the backend to produce more EIT debug information, but shouldn't this be marked as open still since the other patch proved unsuccessful? I was hoping to move away from a source built install and using the version in Ubuntu repository. If eit_ett_staleness_fix_20151120.patch is committed will libmythtv.140430.patch still apply the same?

comment:52 Changed 7 months ago by deadletterfile@…

Hoped to wait, but in regard to comment 51, I agree. I recently set EIT collection to only one tuner card in mythtv-setup while using only the "staleness" patch (did not check with "eit" setting in mythbackend though). Two days and found no mismatches. At three days I found two, after a week mismatches are not common, but neither are they rare, and are easily found.

I patched libmythtv.140430.patch over the "staleness" patch and the patch failed. I used "git checkout" to remove "staleness" patch and libmythtv.140430.patch succeeded. I am on v0.27.5. I am guessing the "staleness" patch still allows mismatches when new EIT/ETT data arrives (no proof).

I am still running only the "staleness" patch. I began experimenting with QMutexLocker today and will report in time. I was never bright enough to understand the need for ETT caching and libmythtv.140430.patch never used the cache (I think), in addition to some locker adjustments.

comment:53 Changed 7 months ago by deadletterfile@…

Submitted are eit.160127.patch and mutex.160127.patch. These patches are a stripped version (with a minimal modification) of libmythtv.140430.patch.

eit.160127.patch removes six lines of code. It is a patch against fixes/0.27 but should patch a clean v0.27.5 tree (and likely older versions).

mutex.160127.patch adds three lines of code. The “return NULL” seems necessary in light of the code calling TuningSignalCheck. I omit the running of StopActiveScan by placement of the return as in the past, during periods of no user interaction with Mythtv, I lost all program data.

The other two added lines throw a mutex locker around the entire TVRec::run(void) function. If I understand locking, this renders all other locking within this function and its children inert.

The patches are applied as with libmythtv.140430.patch. Place the patches in mythtv/libs then:

cd path_to_mythtv/libs/libmythtv
patch -Np1 < ../eit.160127.patch
patch -Np1 < ../mutex.160127.patch

Experimenters can apply the patches to a clean tree in any order, perhaps monitoring several days before applying the second patch. In my short test period, eit.160127.patch yielded the biggest improvement with three mismatches in five days. Adding the mutex.160127.patch has over a six additional days yielded no new mismatches (testing continues). These two patches may be minimally superior to libmythtv.140430.patch (but tested for only a couple of weeks). I know of no issues with running these patches at this time and have been very pleased with the results during an admittedly short test period.

Changed 7 months ago by deadletterfile@…

patch for EIT/ETT mismatches (trimmed from libmythtv.140430.patch)

Changed 7 months ago by deadletterfile@…

tv_rec.<h cpp> patch adding "return NULL" and single mutex locker

Changed 4 months ago by deadletterfile@…

libmythtv patch for v0.28

comment:54 Changed 4 months ago by deadletterfile@…

Submitted libmythtv.160508.v0.28.patch for those that might wish to experiment. I have run this patch for four days and casual inspection has shown no title/description mismatches (though the patch is in no way a fix). I removed the "return NULL" (comment #53) which was in previous patches as mythbackend was crashing. This change should reduce/eliminate mythbackend race conditions this may have caused. The original intent was to keep the eitscanner running. In older versions of Mythtv, the scanner would crash and not be restarted during long periods of no user interaction.

The only additional observation is from v0.27.6: On two occasions, a three hour block of ETT data would be duplicated, and prematurely advanced. As an example, EIT/ETT data for 09-12 UTC would be matched and correct, then the ETT description data would be improperly advanced/duplicated and appear in both the 06-09 UTC and 09-12 UTC Program Guide time slots for a short time.

To patch Mythtv v0.28 on GNU/Linux, place the patch in mythtv/libs and execute:

patch -Np0 < libmythtv.160508.v0.28.patch

Add Comment

Modify Ticket

Action
as closed The owner will remain dekarl.
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.