Opened 3 years ago

Closed 2 years ago

Last modified 18 months ago

#13015 closed Bug Report - General (fixed)

mythfilldatabase may fail with leap second

Reported by: Peter Bennett Owned by: Peter Bennett
Priority: minor Milestone: unknown
Component: MythTV - Mythfilldatabase Version: 0.28.1
Severity: medium Keywords:
Cc: Ticket locked: no

Description

There was a leap second on 12/31/2016. Some grabbers caused mythtv to drop data with the below error for two weeks prior to the leap second.

mythfilldatabase: Ignoring unknown timestamp format: 20161231235960

See https://lists.gt.net/mythtv/users/605460#605460

This seems to be due the calculation of end time, adding 1 hour to time 23:00:00, instead of getting 0:00:00 of the next day, getting a time of 23:59:60.

Change History (5)

comment:1 Changed 3 years ago by Gary Buhrmaster <gary.buhrmaster@…>

Replying to pbennett:

This seems to be due the calculation of end time, adding 1 hour to time 23:00:00, instead of getting 0:00:00 of the next day, getting a time of 23:59:60.

Not necessarily. If the origin source data provided a start time of 23:00:00 and indicated the program duration was indeed 60 minutes (3600 seconds), the end time *IS* 23:59:60 (on a leap second day). But the duration almost certainly should have been 3601 seconds (and not 3600) in the origin data (unless the channel(s) in question shows a black screen for a second I guess). It is possible that the grabber itself(*) might have done some internal adjustments to deal with the leap second (depending on the source data). But neither origin sources, nor grabbers, tend to handle the leap seconds completely correctly and reliably.

AFAIK, Qt has never handled leap seconds. I do not really expect that situation to change (for multiple hard reasons). And trying to properly handle it within MythTV would mean trying to do all the work that Qt has not done (for all those multiple hard reasons).

Here are two different proposed patch solutions (both untested). Neither do leap seconds technically right, but are both potentially viable.

First, just truncate any leap second. A side effect might be that a recording is off by a second in some cases.

diff --git a/mythtv/programs/mythfilldatabase/xmltvparser.cpp b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
index f3249ce..ba69500 100644
--- a/mythtv/programs/mythfilldatabase/xmltvparser.cpp
+++ b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
@@ -199,7 +199,11 @@ static void fromXMLTVDate(QString &timestr, QDateTime &dt)
     {
         QString tsTime = ts.mid(8);
         if (tsTime.length() == 6)
+        {
+            if (tsTime == "235960")
+                tsTime = "235959";
             tmpTime = QTime::fromString(tsTime, "HHmmss");
+        }
         else if (tsTime.length() == 4)
             tmpTime = QTime::fromString(tsTime, "HHmm");
         else if (tsTime.length() == 2)

Second, attempt to move to the first second of the next day. A side effect is that this might generate overlapping program elements which are handled in a different way which can result in some recording being off by a second.

diff --git a/mythtv/programs/mythfilldatabase/xmltvparser.cpp b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
index f3249ce..d3fc243 100644
--- a/mythtv/programs/mythfilldatabase/xmltvparser.cpp
+++ b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
@@ -199,7 +199,14 @@ static void fromXMLTVDate(QString &timestr, QDateTime &dt)
     {
         QString tsTime = ts.mid(8);
         if (tsTime.length() == 6)
+        {
+            if (tsTime == "235960")
+            {
+                tmpDate = tmpDate.addDays(1);
+                tsTime = "000000";
+            }
             tmpTime = QTime::fromString(tsTime, "HHmmss");
+        }
         else if (tsTime.length() == 4)
             tmpTime = QTime::fromString(tsTime, "HHmm");
         else if (tsTime.length() == 2)

Personally, I can not really decide which proposed patch is less wrong for the potential source data being fed to Myth, but in any case, a second here, a second there, it probably does not matter, and accepting the programme element is probably more useful than trying to handle the extra (or missing) second.

(*) The particular grabber that returned the valid leap second was later adjusted due to other code refactor such that it will not be returning the leap second in the future (so a fortunate side effect of the refactor). But other grabbers may still legitimately return a leap second.

comment:2 Changed 3 years ago by Peter Bennett

I prefer option 1, to keep it simple. I cannot see anybody having a problem with a recording being very occasionally 1 second short.

comment:3 Changed 2 years ago by Peter Bennett

Owner: changed from stuartm to Peter Bennett
Status: newaccepted

comment:4 Changed 2 years ago by Gary Buhrmaster <gary.buhrmaster@…>

Resolution: fixed
Status: acceptedclosed

In 499e541d806095000d5157725adbc1f6c20699e2/mythtv:

Fixes #13015 - mythfilldatabase leap-second problem.

Signed-off-by: Peter Bennett <pbennett@…>

comment:5 Changed 18 months ago by Peter Bennett

Owner: changed from Peter Bennett to Peter Bennett
Note: See TracTickets for help on using tickets.