Merge pull request xbmc#1687 from jmarshallnz/fix_image_urls

Fix embedded image urls
avdleeuw · Oct 27, 2012 · ecf53e8 · ecf53e8
2 parents 1677554 + 8521e5e
commit ecf53e8
Show file tree

Hide file tree

Showing 14 changed files with 119 additions and 32 deletions.
diff --git a/project/VS2010Express/XBMC.vcxproj b/project/VS2010Express/XBMC.vcxproj
@@ -972,6 +972,12 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (DirectX)|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (OpenGL)|Win32'">true</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\xbmc\test\TestTextureCache.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (DirectX)|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (OpenGL)|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (DirectX)|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (OpenGL)|Win32'">true</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\xbmc\test\TestUtils.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (DirectX)|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (OpenGL)|Win32'">true</ExcludedFromBuild>

diff --git a/project/VS2010Express/XBMC.vcxproj.filters b/project/VS2010Express/XBMC.vcxproj.filters
@@ -2933,6 +2933,9 @@
     <ClCompile Include="..\..\xbmc\test\TestFileItem.cpp">
       <Filter>test</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\xbmc\test\TestTextureCache.cpp">
+      <Filter>test</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\xbmc\interfaces\json-rpc\PVROperations.cpp">
       <Filter>interfaces\json-rpc</Filter>
     </ClCompile>

diff --git a/xbmc/TextureCache.cpp b/xbmc/TextureCache.cpp
@@ -101,15 +101,16 @@ CStdString CTextureCache::GetWrappedImageURL(const CStdString &image, const CStd
   if (image.compare(0, 8, "image://") == 0)
     return image; // already wrapped
 
-  CStdString encoded(image);
-  CURL::Encode(encoded);
-  CStdString url = "image://";
-  if (!type.IsEmpty())
-    url += type + "@";
-  url += encoded;
+  CURL url;
+  url.SetProtocol("image");
+  url.SetUserName(type);
+  url.SetHostName(image);
   if (!options.IsEmpty())
-    url += "/transform?" + options;
-  return url;
+  {
+    url.SetFileName("transform");
+    url.SetOptions("?" + options);
+  }
+  return url.Get();
 }
 
 CStdString CTextureCache::GetWrappedThumbURL(const CStdString &image)
@@ -123,15 +124,16 @@ CStdString CTextureCache::UnwrapImageURL(const CStdString &image)
   {
     CURL url(image);
     if (url.GetUserName().IsEmpty() && url.GetOptions().IsEmpty())
-    {
-      CStdString file(url.GetHostName());
-      CURL::Decode(file);
-      return file;
-    }
+      return url.GetHostName();
   }
   return image;
 }
 
+bool CTextureCache::CanCacheImageURL(const CURL &url)
+{
+  return (url.GetUserName().empty() || url.GetUserName() == "music");
+}
+
 CStdString CTextureCache::CheckCachedImage(const CStdString &url, bool returnDDS, bool &needsRecaching)
 {
   CStdString cachedHash;

diff --git a/xbmc/TextureCache.h b/xbmc/TextureCache.h
@@ -26,6 +26,7 @@
 #include "TextureDatabase.h"
 #include "threads/Event.h"
 
+class CURL;
 class CBaseTexture;
 
 /*!
@@ -134,6 +135,12 @@ class CTextureCache : public CJobQueue
    */
   static CStdString UnwrapImageURL(const CStdString &image);
 
+  /*! \brief check whether an image:// URL may be cached
+   \param url the URL to the image
+   \return true if the given URL may be cached, false otherwise
+   */
+  static bool CanCacheImageURL(const CURL &url);
+
   /*! \brief Add this image to the database
    Thread-safe wrapper of CTextureDatabase::AddCachedTexture
    \param image url of the original image

diff --git a/xbmc/TextureCacheJob.cpp b/xbmc/TextureCacheJob.cpp
@@ -124,16 +124,12 @@ CStdString CTextureCacheJob::DecodeImageURL(const CStdString &url, unsigned int
     // format is image://[type@]<url_encoded_path>?options
     CURL thumbURL(url);
 
-    if (!thumbURL.GetUserName().IsEmpty())
-    {
-      if (thumbURL.GetUserName() == "music")
-        additional_info = "music";
-      else
-        return ""; // we don't re-cache special images (eg picturefolder/video embedded thumbs)
-    }
+    if (!CTextureCache::CanCacheImageURL(thumbURL))
+      return "";
+    if (thumbURL.GetUserName() == "music")
+      additional_info = "music";
 
     image = thumbURL.GetHostName();
-    CURL::Decode(image);
 
     CStdString optionString = thumbURL.GetOptions().Mid(1);
     optionString.TrimRight('/'); // in case XBMC adds a slash

diff --git a/xbmc/filesystem/ImageFile.cpp b/xbmc/filesystem/ImageFile.cpp
@@ -60,12 +60,10 @@ bool CImageFile::Exists(const CURL& url)
     return CFile::Exists(cachedFile);
 
   // need to check if the original can be cached on demand and that the file exists 
-  if (!url.GetUserName().IsEmpty())
-    return false; // not in the cache, and can't be cached on demand
+  if (!CTextureCache::CanCacheImageURL(url))
+    return false;
 
-  CStdString image = url.GetHostName();
-  CURL::Decode(image);
-  return CFile::Exists(image);
+  return CFile::Exists(url.GetHostName());
 }
 
 int CImageFile::Stat(const CURL& url, struct __stat64* buffer)

diff --git a/xbmc/music/MusicDatabase.cpp b/xbmc/music/MusicDatabase.cpp
@@ -3636,6 +3636,19 @@ bool CMusicDatabase::UpdateOldVersion(int version)
     g_settings.Save();
   }
 
+  if (version < 29)
+  { // update old art URLs
+    m_pDS->query("select art_id,url from art where url like 'image://%%'");
+    vector< pair<int, string> > art;
+    while (!m_pDS->eof())
+    {
+      art.push_back(make_pair(m_pDS->fv(0).get_asInt(), CURL(m_pDS->fv(1).get_asString()).Get()));
+      m_pDS->next();
+    }
+    m_pDS->close();
+    for (vector< pair<int, string> >::iterator i = art.begin(); i != art.end(); ++i)
+      m_pDS->exec(PrepareSQL("update art set url='%s' where art_id=%d", i->second.c_str(), i->first));
+  }
   // always recreate the views after any table change
   CreateViews();
 

diff --git a/xbmc/music/MusicDatabase.h b/xbmc/music/MusicDatabase.h
@@ -278,7 +278,7 @@ class CMusicDatabase : public CDatabase
   std::map<CStdString, CAlbum> m_albumCache;
 
   virtual bool CreateTables();
-  virtual int GetMinVersion() const { return 28; };
+  virtual int GetMinVersion() const { return 29; };
   const char *GetBaseDBName() const { return "MyMusic"; };
 
   int AddSong(const CSong& song, bool bCheck = true, int idAlbum = -1);

diff --git a/xbmc/network/httprequesthandler/HTTPImageHandler.cpp b/xbmc/network/httprequesthandler/HTTPImageHandler.cpp
@@ -37,9 +37,7 @@ int CHTTPImageHandler::HandleHTTPRequest(const HTTPRequest &request)
     m_path = request.url.substr(7);
 
     XFILE::CImageFile imageFile;
-    if (imageFile.Exists(m_path) ||
-       // temporary workaround for music images until they are integrated into CTextureCache and therefore CImageFile
-       (m_path.Left(10) == "special://" && m_path.Right(4) == ".tbn" && XFILE::CFile::Exists(m_path)))
+    if (imageFile.Exists(m_path))
     {
       m_responseCode = MHD_HTTP_OK;
       m_responseType = HTTPFileDownload;

diff --git a/xbmc/test/Makefile b/xbmc/test/Makefile
@@ -1,6 +1,7 @@
 SRCS=	\
 	TestBasicEnvironment.cpp \
 	TestFileItem.cpp \
+	TestTextureCache.cpp \
 	TestUtils.cpp \
 	xbmc-test.cpp
 

diff --git a/xbmc/test/TestTextureCache.cpp b/xbmc/test/TestTextureCache.cpp
@@ -0,0 +1,49 @@
+/*
+ *      Copyright (C) 2012 Team XBMC
+ *      http://www.xbmc.org
+ *
+ *  This Program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2, or (at your option)
+ *  any later version.
+ *
+ *  This Program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with XBMC; see the file COPYING.  If not, see
+ *  <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "URL.h"
+#include "TextureCache.h"
+
+#include "gtest/gtest.h"
+
+TEST(TestTextureCache, GetWrappedImageURL)
+{
+  typedef struct
+  {
+    const char *in;
+    const char *type;
+    const char *options;
+    const char *out;
+  } testfiles;
+
+  const testfiles test_files[] = {{ "/path/to/image/file.jpg", "", "", "image://%2fpath%2fto%2fimage%2ffile.jpg/" },
+                                  { "/path/to/image/file.jpg", "", "size=thumb", "image://%2fpath%2fto%2fimage%2ffile.jpg/transform?size=thumb" },
+                                  { "/path/to/video/file.mkv", "video", "", "image://video@%2fpath%2fto%2fvideo%2ffile.mkv/" },
+                                  { "/path/to/music/file.mp3", "music", "", "image://music@%2fpath%2fto%2fmusic%2ffile.mp3/" },
+                                  { "image://%2fpath%2fto%2fimage%2ffile.jpg/", "", "", "image://%2fpath%2fto%2fimage%2ffile.jpg/" },
+                                  { "image://%2fpath%2fto%2fimage%2ffile.jpg/transform?size=thumb", "", "size=thumb", "image://%2fpath%2fto%2fimage%2ffile.jpg/transform?size=thumb" }};
+
+  for (unsigned int i = 0; i < sizeof(test_files) / sizeof(testfiles); i++)
+  {
+    std::string expected = test_files[i].out;
+    std::string out = CTextureCache::GetWrappedImageURL(test_files[i].in, test_files[i].type, test_files[i].options);
+    EXPECT_EQ(out, expected);
+  }
+}
diff --git a/xbmc/utils/URIUtils.cpp b/xbmc/utils/URIUtils.cpp
@@ -237,7 +237,8 @@ bool URIUtils::ProtocolHasParentInHostname(const CStdString& prot)
 bool URIUtils::ProtocolHasEncodedHostname(const CStdString& prot)
 {
   return ProtocolHasParentInHostname(prot)
-      || prot.Equals("musicsearch");
+      || prot.Equals("musicsearch")
+      || prot.Equals("image");
 }
 
 bool URIUtils::ProtocolHasEncodedFilename(const CStdString& prot)

diff --git a/xbmc/video/VideoDatabase.cpp b/xbmc/video/VideoDatabase.cpp
@@ -4111,6 +4111,19 @@ bool CVideoDatabase::UpdateOldVersion(int iVersion)
     }
     m_pDS->exec("DROP TABLE IF EXISTS setlinkmovie");
   }
+  if (iVersion < 70)
+  { // update old art URLs
+    m_pDS->query("select art_id,url from art where url like 'image://%%'");
+    vector< pair<int, string> > art;
+    while (!m_pDS->eof())
+    {
+      art.push_back(make_pair(m_pDS->fv(0).get_asInt(), CURL(m_pDS->fv(1).get_asString()).Get()));
+      m_pDS->next();
+    }
+    m_pDS->close();
+    for (vector< pair<int, string> >::iterator i = art.begin(); i != art.end(); ++i)
+      m_pDS->exec(PrepareSQL("update art set url='%s' where art_id=%d", i->second.c_str(), i->first));
+  }
   // always recreate the view after any table change
   CreateViews();
   return true;

diff --git a/xbmc/video/VideoDatabase.h b/xbmc/video/VideoDatabase.h
@@ -804,7 +804,7 @@ class CVideoDatabase : public CDatabase
    */
   bool LookupByFolders(const CStdString &path, bool shows = false);
 
-  virtual int GetMinVersion() const { return 69; };
+  virtual int GetMinVersion() const { return 70; };
   virtual int GetExportVersion() const { return 1; };
   const char *GetBaseDBName() const { return "MyVideos"; };