Mailing List Archive

Re: Ticket #13623: mythfilldatabase may not process valid XMLTV files if program starttime order is not increasing
Op vr 22 mei 2020 23:12 schreef MythTV <noreply@mythtv.org>:

> #13623: mythfilldatabase may not process valid XMLTV files if program
> starttime
> order is not increasing
> -------------------------------------------+--------------------------
> Reporter: Gary Buhrmaster | Owner: (none)
> Type: Bug Report - General | Status: new
> Priority: minor | Milestone: needs_triage
> Component: MythTV - Mythfilldatabase | Version: Master Head
> Severity: medium | Keywords:
> Ticket locked: 0 |
> -------------------------------------------+--------------------------
> mythfilldatabase (with a9aa006139) may no longer process valid XMLTV files
> if the program starttime order is not increasing.
>
> Originally reported to the XMLTV project, where it was pointed out that
> the issue is actually due to a MythTV issue. As the original reporter
> chose not to create a MythTV ticket (s/he found their own workaround),
> here is what seems to be the essential part of the original issue so that
> it can be addressed here:
>
> In a9aa006139 a new (to mythfilldatabase) test was added to check that the
> starttime was increasing with each new program element on the same
> channel, and reject the input file if it was not.
>
> While many (most?) XMLTV grabbers appear to return the data in starttime
> order (and for that matter, channel and starttime order), any such order
> requirement is not part of the specification, and the original reporter
> ran into this issue with their chosen grabber for a particular day.
>
> The order check should be removed to support compliant source files.
>
> FWIW, as I recall (it has been a long time since I last looked at the code
> in question), MythTV itself does its own internal sorting of the program
> data that it processes for its own purposes.
>
> The mitigation until this issue is resolved is to run the output of a
> grabber through TV_SORT (but note that if there is a lot of source data
> TV_SORT can be very resource intensive (lots of memory and cpu)).
>
> **COMPLETELY** untested patch (not compiled, let alone tested, just sort
> of looks right):
>
> {{{
> diff --git a/mythtv/programs/mythfilldatabase/xmltvparser.cpp
> b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
> index 78f22170cd..bbce136c9d 100644
> --- a/mythtv/programs/mythfilldatabase/xmltvparser.cpp
> +++ b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
> @@ -205,8 +205,6 @@ bool XMLTVParser::parseFile(
> QString aggregatedTitle;
> QString aggregatedDesc;
> bool haveReadTV = false;
> - QString last_channel = ""; //xmltvId of the last program element we
> read
> - QDateTime last_starttime; //starttime of the last program element we
> read
> while (!xml.atEnd() && !xml.hasError() && (! (xml.isEndElement() &&
> xml.name() == "tv")))
> {
> if (xml.readNextStartElement())
> @@ -709,23 +707,6 @@ bool XMLTVParser::parseFile(
> else
> {
> // so we have a (relatively) clean program element
> now, which is good enough to process or to store
> - if (pginfo->m_channel != last_channel) {
> - //we have a channel change here
> - last_channel = pginfo->m_channel;
> - last_starttime = QDateTime(QDate(1970, 1, 1),
> QTime(0, 0, 0)); //initialize it to a time far, far away ...
> - }
> - else {
> - //we are still on the same channel
> - if (pginfo->m_starttime >= last_starttime) {
> - last_starttime = pginfo->m_starttime;
> - }
> - else {
> - LOG(VB_GENERAL, LOG_ERR, QString("Malformed
> XML file, program out of order at line %1,
> %2").arg(xml.lineNumber()).arg(xml.errorString()));
> - delete pginfo;
> - return false;
> - }
> - }
> -
> if (pginfo->m_clumpidx.isEmpty())
> (*proglist)[pginfo->m_channel].push_back(*pginfo);
> else
> }}}
>
> --
> Ticket URL: <https://code.mythtv.org/trac/ticket/13623>
> MythTV <http://www.mythtv.org>
> MythTV Media Center
>

I agree with the patch, I did add this check in the code because I
understood you wanted strict checking of the XMLTV rules; after rechecking
the rules I see there is no explicit rule enforcing this order.

Sorting through TV_SORT is unnecessary because after the pg_info is pushed
on the stack, it is sorted before checking for overlaps.

Just removing the check will be sufficient. I am currently not at the
location of my mythtv machine, so I cannot make the patch and test it
myself, but the patch above looks fine!

Sorry for the inconvenience. Hans.

>