[mythtv] Ticket #7484: MythVideo image retrieval creating incorrectly named files for series containing '?'
Robin Hill
myth at robinhill.me.uk
Fri Nov 20 14:51:12 UTC 2009
On Thu Nov 12, 2009 at 12:08:02PM +0000, Robin Hill wrote:
> On Wed Nov 11, 2009 at 08:54:03PM +0000, Robin Hill wrote:
>
> > On Wed Nov 11, 2009 at 06:23:42PM -0000, MythTV wrote:
> >
> > > #7484: MythVideo image retrieval creating incorrectly named files for series
> > > containing '?'
> > > -----------------------------------------------+----------------------------
> > > Reporter: Robin Hill <myth@…> | Owner: awithers
> > > Type: defect | Status: closed
> > > Priority: minor | Milestone: unknown
> > > Component: Plugin - MythVideo | Version: 0.22rc1
> > > Severity: medium | Resolution: fixed
> > > Mlocked: 0 |
> > > -----------------------------------------------+----------------------------
> > >
> > > Comment(by robertm):
> > >
> > > (In [22795]) Refs #7484, #7485. Backport rest of image redownload fixes,
> > > and replace a couple illegal characters in titles.
> > >
> >
> > Thanks for this - do similar changes need making in JAMU to ensure it
> > creates compatible filenames?
> >
> Also, shouldn't the illegal character removal code also be done in
> GetLocalVideoImage, otherwise it'll be redownloading the images every
> time.
>
> It might also be better if this illegal character stripping was done
> prior to deciding whether a directory name matches a series name (for
> showing covers/screenshots).
>
The attached patch (against 0.22 r22866) moves the illegal character
stripping from videodlg.cpp to the metadata class, as a new GetSafeTitle
method.
This is then used throughout videodlg.cpp to retrieve/store the metadata
in a safe format. Both the original and stripped title are used for
retrieving images, meaning existing images will still get retrieved.
The stripped title is also used for checking directory names against
series names, allowing coverart to be shown for series containing
invalid characters.
The only change I made to the character stripping was to instead replace
the ':' with ' -', as I feel that's the logical way to name series
directories where the series contains a ':' (e.g. "Terminator - The
Sarah Connor Chronicles").
Cheers,
Robin
--
___
( ' } | Robin Hill <myth at robinhill.me.uk> |
/ / ) | Little Jim says .... |
// !! | "He fallen in de water !!" |
-------------- next part --------------
Index: mythplugins/mythvideo/mythvideo/metadata.cpp
===================================================================
--- mythplugins/mythvideo/mythvideo/metadata.cpp (revision 22866)
+++ mythplugins/mythvideo/mythvideo/metadata.cpp (working copy)
@@ -1034,6 +1034,26 @@
m_imp->SetTitle(title);
}
+QString Metadata::GetSafeTitle()
+{
+ QString safe_title = m_imp->getTitle();
+
+ safe_title.remove('?');
+ safe_title.remove('<');
+ safe_title.remove('>');
+ safe_title.remove('/');
+ safe_title.remove('\\');
+ safe_title.remove('|');
+ safe_title.remove('*');
+ safe_title.remove('[');
+ safe_title.remove(']');
+ safe_title.remove('"');
+ safe_title.remove('^');
+ safe_title.replace(":", " -");
+
+ return safe_title;
+}
+
const QString &Metadata::GetSubtitle() const
{
return m_imp->getSubtitle();
Index: mythplugins/mythvideo/mythvideo/videodlg.cpp
===================================================================
--- mythplugins/mythvideo/mythvideo/videodlg.cpp (revision 22866)
+++ mythplugins/mythvideo/mythvideo/videodlg.cpp (working copy)
@@ -2126,6 +2126,8 @@
if (parent && metadata && ((QString::compare(parent->getString(),
metadata->GetTitle(), Qt::CaseInsensitive) == 0) ||
+ (QString::compare(parent->getString(),
+ metadata->GetSafeTitle(), Qt::CaseInsensitive) == 0) ||
parent->getString().startsWith(tr("Season"), Qt::CaseInsensitive)))
item->SetText(metadata->GetSubtitle());
else
@@ -2139,6 +2141,8 @@
if (!screenshot.isEmpty() && parent && metadata &&
((QString::compare(parent->getString(),
metadata->GetTitle(), Qt::CaseInsensitive) == 0) ||
+ (QString::compare(parent->getString(),
+ metadata->GetSafeTitle(), Qt::CaseInsensitive) == 0) ||
parent->getString().startsWith(tr("Season"), Qt::CaseInsensitive)))
{
item->SetImage(screenshot);
@@ -2349,6 +2353,9 @@
imageTypes.append(metadata->GetTitle() + ".png");
imageTypes.append(metadata->GetTitle() + ".jpg");
imageTypes.append(metadata->GetTitle() + ".gif");
+ imageTypes.append(metadata->GetSafeTitle() + ".png");
+ imageTypes.append(metadata->GetSafeTitle() + ".jpg");
+ imageTypes.append(metadata->GetSafeTitle() + ".gif");
imageTypes.append("*.png");
imageTypes.append("*.jpg");
imageTypes.append("*.gif");
@@ -2674,6 +2681,7 @@
QString test_file;
QString host = metadata->GetHost();
QString title = metadata->GetTitle();
+ QString safe_title = metadata->GetSafeTitle();
if (type == "Coverart" && !host.isEmpty() &&
!metadata->GetCoverFile().startsWith("/"))
@@ -2686,7 +2694,8 @@
if (!test_file.endsWith("/") && !test_file.isEmpty() &&
!IsDefaultCoverFile(test_file) && (gpnode.isEmpty() ||
- (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0)))
+ (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0) ||
+ (QString::compare(gpnode, safe_title, Qt::CaseInsensitive) == 0)))
{
icon_file = test_file;
break;
@@ -2703,7 +2712,8 @@
if (!test_file.endsWith("/") && !test_file.isEmpty() &&
test_file != VIDEO_FANART_DEFAULT && (gpnode.isEmpty() ||
- (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0)))
+ (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0) ||
+ (QString::compare(gpnode, safe_title, Qt::CaseInsensitive) == 0)))
{
icon_file = test_file;
break;
@@ -2720,7 +2730,8 @@
if (!test_file.endsWith("/") && !test_file.isEmpty() &&
test_file != VIDEO_BANNER_DEFAULT && (gpnode.isEmpty() ||
- (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0)))
+ (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0) ||
+ (QString::compare(gpnode, safe_title, Qt::CaseInsensitive) == 0)))
{
icon_file = test_file;
break;
@@ -2737,7 +2748,8 @@
if (!test_file.endsWith("/") && !test_file.isEmpty() &&
test_file != VIDEO_SCREENSHOT_DEFAULT && (gpnode.isEmpty() ||
- (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0)))
+ (QString::compare(gpnode, title, Qt::CaseInsensitive) == 0) ||
+ (QString::compare(gpnode, safe_title, Qt::CaseInsensitive) == 0)))
{
icon_file = test_file;
break;
@@ -4231,38 +4243,53 @@
metadata->Reset();
QString cover_file;
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
QStringList(m_d->m_artDir), cover_file,
+ metadata->GetSafeTitle(), metadata->GetSeason(),
+ metadata->GetHost(), "Coverart", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ QStringList(m_d->m_artDir), cover_file,
metadata->GetTitle(), metadata->GetSeason(),
- metadata->GetHost(), "Coverart", metadata->GetEpisode()))
+ metadata->GetHost(), "Coverart", metadata->GetEpisode())))
{
metadata->SetCoverFile(cover_file);
}
QString screenshot_file;
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
QStringList(m_d->m_sshotDir), screenshot_file,
+ metadata->GetSafeTitle(), metadata->GetSeason(), metadata->GetHost(),
+ "Screenshots", metadata->GetEpisode(), true)) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ QStringList(m_d->m_sshotDir), screenshot_file,
metadata->GetTitle(), metadata->GetSeason(), metadata->GetHost(),
- "Screenshots", metadata->GetEpisode(), true))
+ "Screenshots", metadata->GetEpisode(), true)))
{
metadata->SetScreenshot(screenshot_file);
}
-
QString fanart_file;
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
QStringList(m_d->m_fanDir), fanart_file,
+ metadata->GetSafeTitle(), metadata->GetSeason(),
+ metadata->GetHost(), "Fanart", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ QStringList(m_d->m_fanDir), fanart_file,
metadata->GetTitle(), metadata->GetSeason(),
- metadata->GetHost(), "Fanart", metadata->GetEpisode()))
+ metadata->GetHost(), "Fanart", metadata->GetEpisode())))
{
metadata->SetFanart(fanart_file);
}
QString banner_file;
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
QStringList(m_d->m_banDir), banner_file,
+ metadata->GetSafeTitle(), metadata->GetSeason(),
+ metadata->GetHost(), "Banners", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ QStringList(m_d->m_banDir), banner_file,
metadata->GetTitle(), metadata->GetSeason(),
- metadata->GetHost(), "Banners", metadata->GetEpisode()))
+ metadata->GetHost(), "Banners", metadata->GetEpisode())))
{
metadata->SetBanner(banner_file);
}
@@ -4289,10 +4316,14 @@
if (metadata->GetCoverFile().isEmpty() ||
IsDefaultCoverFile(metadata->GetCoverFile()))
{
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ cover_dirs, cover_file, metadata->GetSafeTitle(),
+ metadata->GetSeason(), metadata->GetHost(),
+ "Coverart", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
cover_dirs, cover_file, metadata->GetTitle(),
metadata->GetSeason(), metadata->GetHost(),
- "Coverart", metadata->GetEpisode()))
+ "Coverart", metadata->GetEpisode())))
{
metadata->SetCoverFile(cover_file);
OnVideoImageSetDone(metadata);
@@ -4315,10 +4346,14 @@
if (metadata->GetFanart().isEmpty())
{
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ fanart_dirs, fanart_file, metadata->GetSafeTitle(),
+ metadata->GetSeason(), metadata->GetHost(),
+ "Fanart", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
fanart_dirs, fanart_file, metadata->GetTitle(),
metadata->GetSeason(), metadata->GetHost(),
- "Fanart", metadata->GetEpisode()))
+ "Fanart", metadata->GetEpisode())))
{
metadata->SetFanart(fanart_file);
OnVideoImageSetDone(metadata);
@@ -4341,10 +4376,14 @@
if (metadata->GetBanner().isEmpty())
{
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ banner_dirs, banner_file, metadata->GetSafeTitle(),
+ metadata->GetSeason(), metadata->GetHost(),
+ "Banners", metadata->GetEpisode())) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
banner_dirs, banner_file, metadata->GetTitle(),
metadata->GetSeason(), metadata->GetHost(),
- "Banners", metadata->GetEpisode()))
+ "Banners", metadata->GetEpisode())))
{
metadata->SetBanner(banner_file);
OnVideoImageSetDone(metadata);
@@ -4368,10 +4407,14 @@
if (metadata->GetScreenshot().isEmpty())
{
- if (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ if ((GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
+ screenshot_dirs, screenshot_file, metadata->GetSafeTitle(),
+ metadata->GetSeason(), metadata->GetHost(), "Screenshots",
+ metadata->GetEpisode(), true)) ||
+ (GetLocalVideoImage(metadata->GetInetRef(), metadata->GetFilename(),
screenshot_dirs, screenshot_file, metadata->GetTitle(),
metadata->GetSeason(), metadata->GetHost(), "Screenshots",
- metadata->GetEpisode(), true))
+ metadata->GetEpisode(), true)))
{
metadata->SetScreenshot(screenshot_file);
OnVideoImageSetDone(metadata);
@@ -4455,25 +4498,12 @@
// Name TV downloads so that they already work with the PBB
if (type == "Screenshots")
- title = QString("%1 Season %2x%3_%4").arg(metadata->GetTitle())
+ title = QString("%1 Season %2x%3_%4").arg(metadata->GetSafeTitle())
.arg(season).arg(episode).arg(suffix);
else
- title = QString("%1 Season %2_%3").arg(metadata->GetTitle())
+ title = QString("%1 Season %2_%3").arg(metadata->GetSafeTitle())
.arg(season).arg(suffix);
- title.remove('?');
- title.remove('<');
- title.remove('>');
- title.remove('/');
- title.remove('\\');
- title.remove('|');
- title.remove('*');
- title.remove('[');
- title.remove(']');
- title.remove(':');
- title.remove('"');
- title.remove('^');
-
if (!host.isEmpty())
{
QString combFileName = QString("%1.%2").arg(title)
Index: mythplugins/mythvideo/mythvideo/metadata.h
===================================================================
--- mythplugins/mythvideo/mythvideo/metadata.h (revision 22866)
+++ mythplugins/mythvideo/mythvideo/metadata.h (working copy)
@@ -96,6 +96,8 @@
const QString &GetTitle() const;
void SetTitle(const QString& title);
+ QString GetSafeTitle();
+
const QString &GetSubtitle() const;
void SetSubtitle(const QString &subtitle);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://mythtv.org/pipermail/mythtv-dev/attachments/20091120/8c5dacda/attachment.pgp>
More information about the mythtv-dev
mailing list