Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 12 months ago

Closed 11 months ago

Last modified 10 months ago

#11538 closed Bug Report - General (fixed)

XMLTV date parsing issue

Reported by: richard@… Owned by: stuartm
Priority: minor Milestone: 0.27
Component: MythTV - Mythfilldatabase Version: Master Head
Severity: medium Keywords: XMLTV
Cc: Ticket locked: no

Description

Commit: ff5ab27842c522f9e054876a8eb69a51b8c86a2e seems to have introduced a problem with XMLTV date parsing.

My XMLTV data contains entries like this:
<programme start="20130512140000 +0000" stop="20130512150000 +0000" channel="6">

Which should be valid, however the code in mythtv/programs/mythfilldatabase/xmltvparser.cpp seems to be treating them as localtime, resulting in incorrect program times in the database.

The specific code fragment is this: (around line 181)

// While this seems like a hack, it's better than what was done before
QString isoDateString = QString("%1 %2").arg(tmpDT.toString(Qt::ISODate))
                                                 .arg(tmp);
dt = QDateTime::fromString(isoDateString, Qt::ISODate).toUTC();

Which is taking the ISO-formatted string (including UTC offset) and parsing it. The bit which I think is wrong is the toUTC() conversion at the end. This shouldn't be necessary as the parser will have already dealt with whatever offset was in the string (i.e. the QDateTime object should already be in UTC).

Removing the .toUTC() conversion fixes the problem for me.

Attachments (3)

date_test.cpp (2.9 KB) - added by richard@… 11 months ago.
date_test.2.cpp (3.0 KB) - added by richard@… 11 months ago.
xmltvparser.patch (910 bytes) - added by Richard Begg <richard@…> 11 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 12 months ago by stuartm

The source of qdatetime.cpp says that toUTC()/toTimeSpec(Qt::UTC) would be a no-op if the time spec was already UTC, so at worst it does nothing at all and otherwise it's converting a datetime in localtime to UTC. This code is working correctly for others including myself so there's something else going on here.

Which version of QT are you using? How far off are the times in the database? What is your current timezone and are the datetimes in the xml really correct?

Assuming AEST, "20130512140000 +0000" would be midnight local time on the 13th? Would that be right?

Changed 11 months ago by richard@…

comment:2 Changed 11 months ago by richard@…

Ok - I think I know what's going wrong.
If you give QDateTime::fromString() a string with a zero UTC offset (like this: "20130513090000 +0000") the resulting QDateTime object will have a Qt::TimeSpec? of Qt::LocalTime?, so therefore the .toUTC() call will incorrectly adjust the date.
If the string has a non-zero offset (e.g. the same date in localtime - "20130513190000 +1000"), the QDateTime object now has a TimeSpec? of Qt::OffsetFromUTC so the .toUTC() call works correctly. Personally, I think this is a QT defect... we've given it an offset, and it's ignored it. I've reproduced this behaviour with both Qt-4.8 and on my old box (Qt-3.3)

I've attached a standalone sample which illustrates the problem. If you run it with my XMLTV data which is already in UTC (i.e. +0000 offset), the result is wrong:

./date_test "20130513090000 +0000"
"Parsing: 2013-05-13T09:00:00 +0000"
"Parsed Result: Mon May 13 09:00:00 2013, TimeSpec: 0"
"toUTC() Result: Sun May 12 23:00:00 2013, TimeSpec: 1"
"Localtime QDateTime: Mon May 13 09:00:00 2013"

However, if you give it a date with a non-zero offset - you get the correct result:

./date_test "20130513190000 +1000"
"Parsing: 2013-05-13T19:00:00 +1000"
"Parsed Result: Mon May 13 19:00:00 2013, TimeSpec: 2"
"toUTC() Result: Mon May 13 09:00:00 2013, TimeSpec: 1"
"Localtime QDateTime: Mon May 13 19:00:00 2013"

Something like this seems to fix it:

*** a/date_test.cpp	2013-05-16 09:16:22.913187091 +1000
--- b/date_test.cpp	2013-05-16 09:26:12.725426207 +1000
***************
*** 70,75 ****
--- 70,78 ----
                                                  .arg(tmp);
  	qDebug() << QString("Parsing: %1").arg(isoDateString);
  	QDateTime dt2 = QDateTime::fromString(isoDateString, Qt::ISODate);
+ 	// QT seems to think that zero offset dates are LocalTime - force them to UTC
+ 	if (dt2.timeSpec() == Qt::LocalTime)
+ 	    dt2.setTimeSpec(Qt::UTC);
  	qDebug() << QString("Parsed Result: %1, TimeSpec: %2").arg(dt2.toString()).arg(dt2.timeSpec());
          dt = dt2.toUTC();
  	qDebug() << QString("toUTC() Result: %1, TimeSpec: %2").arg(dt.toString()).arg(dt.timeSpec());

Changed 11 months ago by richard@…

comment:3 Changed 11 months ago by richard@…

Note: ignore date_test.cpp - date_test.2.cpp is the one used for the testing results.

comment:4 Changed 11 months ago by Gary Buhrmaster <gary.buhrmaster@…>

Sorry for the duplication, but I wanted to get this detail into the ticket proper so the history for the required hack will be preserved.....

My first thoughts agree, a Qt defect. However, apparently this is the "expected" behavior in the test harness, brokenness and all. From the Qt tests (including the comments that this is probably wrong) extracted from tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp

// Not sure about these two... it will currently be created as LocalTime, but it
// should probably be UTC according to the ISO 8601 spec (see 4.2.5.1).
    QTest::newRow("ISO +0000") << QString::fromLatin1("1970-01-01T00:12:34+0000")
        << Qt::ISODate << QDateTime(QDate(1970, 1, 1), QTime(0, 12, 34), Qt::LocalTime);
    QTest::newRow("ISO +00:00") << QString::fromLatin1("1970-01-01T00:12:34+00:00")
        << Qt::ISODate << QDateTime(QDate(1970, 1, 1), QTime(0, 12, 34), Qt::LocalTime);

Changed 11 months ago by Richard Begg <richard@…>

comment:5 Changed 11 months ago by Richard Begg <richard@…>

Attached a patch (against trunk) to workaround this QT date parsing issue.

comment:6 Changed 11 months ago by Richard Begg <richard@…>

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

In e60495b73879452255f707e47630b172941e5d0d/mythtv:

Workaround QT ISO date parsing issue in mythfilldatabase. Fixes #11538

Signed-off-by: Stuart Morgan <smorgan@…>

comment:7 Changed 10 months ago by paulh

  • Milestone changed from unknown to 0.27

Add Comment

Modify Ticket

Action
as closed .
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.