Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#13028 closed Patch - Bug Fix (fixed)

Patch - xmltvparser.cpp choose first valid icon

Reported by: Gary Buhrmaster <gary.buhrmaster@…> Owned by: Peter Bennett
Priority: minor Milestone: 30.0
Component: MythTV - Mythfilldatabase Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

[Cleaning up some old local stashes regarding compliance validation to try to not lose something that someone might find useful at some future point]

The xmltv dtd specifies that a channel can have multiple icons, and while the dtd is not explicit regarding the icon, for the station url, the most authoritative should be first, which is likely the intended order for icons too.

AFAIK no grabber currently returns multiple icons (although there are some EPG sources that now provide them), but MythTV appears to choose the last provided in the xml file for a station (which is likely not what is desirable).

This proposed patch changes MythTV to use the first valid icon if multiple are provided.

To the dev who reviews the patch, the patch is a bit ugly due to git diff (sorry about that), but essentially just wraps the icon assignment inside a "isEmpty()" check, so only the first valid icon provided will be used.

Untested (Caveat Emptor) [I compiled it at one point in the past, but I do not recall creating a test dataset]

diff --git a/mythtv/programs/mythfilldatabase/xmltvparser.cpp b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
index f3249ce..efb6dba 100644
--- a/mythtv/programs/mythfilldatabase/xmltvparser.cpp
+++ b/mythtv/programs/mythfilldatabase/xmltvparser.cpp
@@ -89,18 +89,21 @@ ChannelInfo *XMLTVParser::parseChannel(QDomElement &element, QUrl &baseUrl)
         {
             if (info.tagName() == "icon")
             {
-                QString path = info.attribute("src", "");
-                if (!path.isEmpty() && !path.contains("://"))
+                if (chaninfo->icon.isEmpty())
                 {
-                    QString base = baseUrl.toString(QUrl::StripTrailingSlash);
-                    chaninfo->icon = base +
-                        ((path.startsWith("/")) ? path : QString("/") + path);
-                }
-                else if (!path.isEmpty())
-                {
-                    QUrl url(path);
-                    if (url.isValid())
-                        chaninfo->icon = url.toString();
+                    QString path = info.attribute("src", "");
+                    if (!path.isEmpty() && !path.contains("://"))
+                    {
+                        QString base = baseUrl.toString(QUrl::StripTrailingSlash);
+                        chaninfo->icon = base +
+                            ((path.startsWith("/")) ? path : QString("/") + path);
+                    }
+                    else if (!path.isEmpty())
+                    {
+                        QUrl url(path);
+                        if (url.isValid())
+                            chaninfo->icon = url.toString();
+                    }
                 }
             }
             else if (info.tagName() == "display-name")

Change History (5)

comment:1 Changed 7 years ago by Peter Bennett

Owner: changed from stuartm to Peter Bennett
Status: newassigned

comment:2 Changed 7 years ago by Stuart Auchterlonie

Milestone: unknown30.0

comment:3 Changed 6 years ago by Peter Bennett

I tested this with xmltv, there is only 1 icon for each channel, I got the station icons successfully. Assume that it will work as intended with multiple icons per station.

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

Resolution: fixed
Status: assignedclosed

In 96e307a20df735286ca3b660af271bf6538726a5/mythtv:

mythfilldatabase: choose first valid icon for channel

Choose first icon if multiple are supplied.

Fixes #13028

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

comment:5 Changed 6 years ago by Peter Bennett

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