Permalink
Browse files

Some refactoring according to jmarshallnz's coments

  • Loading branch information...
1 parent 035f9d8 commit c5d32c31e3254d946d07ea7ccd467e42f40a40a9 psyton committed Jun 5, 2012
View
@@ -2786,13 +2786,7 @@ bool CFileItem::LoadMusicTag()
}
// load tag from file
CLog::Log(LOGDEBUG, "%s: loading tag information for file: %s", __FUNCTION__, m_strPath.c_str());
- CMusicInfoTagLoaderFactory factory;
- auto_ptr<IMusicInfoTagLoader> pLoader (factory.CreateLoader(m_strPath));
- if (NULL != pLoader.get())
- {
- if (pLoader->Load(m_strPath, *GetMusicInfoTag()))
- return true;
- }
+ GetMusicInfoTag()->LoadFromFile(m_strPath);
// no tag - try some other things
if (IsCDDA())
{
@@ -2892,12 +2886,6 @@ MUSIC_INFO::CMusicInfoTag* CFileItem::GetMusicInfoTag()
if (!m_musicInfoTag)
m_musicInfoTag = new MUSIC_INFO::CMusicInfoTag;
- if (!m_musicInfoTag->Loaded())
- { // read the tag from a file
- auto_ptr<IMusicInfoTagLoader> pLoader (CMusicInfoTagLoaderFactory::CreateLoader(GetPath()));
- if (NULL != pLoader.get())
- pLoader->Load(GetPath(), *m_musicInfoTag);
- }
return m_musicInfoTag;
}
@@ -50,9 +50,6 @@ bool CueParser::parse(CueParserCallback& callback)
{
if (!callback.onDataNeeded(strLine))
break;
- if (strLine.find('\n') != -1)
- CLog::Log(LOGINFO, "Error!!!");
-
CStdString cmd = cutFirstWord(strLine);
if (cmd == "INDEX")
{
@@ -328,7 +325,7 @@ bool CueParser::extractString(const CStdString &line, CStdString &outStr, bool s
}
else
{
- if (spaceIsError && line.find(' ') >= 0)
+ if (spaceIsError && line.Find(' ') >= 0)
return false;
outStr = line.Mid(0);
}
@@ -64,7 +64,7 @@ bool CueSongsCollector::load(const CStdString &filePath)
extension.Equals((char*)".wv"))
{
const MUSIC_INFO::CMusicInfoTag& t = tag(filePath);
- if (t.hasEmbeddedCue())
+ if (t.HasEmbeddedCue())
{
m_reader = boost::shared_ptr<CueReader>(new TagCueReader(t.GetEmbeddedCue()));
m_external = false;
@@ -114,7 +114,7 @@ bool CueSongsCollector::onFile(CStdString& filePath)
}
if (bFoundMediaFile)
{ // if file has a cue, we stop parsing.
- if (tag(filePath).hasEmbeddedCue())
+ if (tag(filePath).HasEmbeddedCue())
{
itemstodelete.insert(filePath);
return false;
@@ -36,7 +36,7 @@ bool TagCueReader::isValid() const
bool TagCueReader::ReadNextLine(CStdString &line)
{
// Read the next line.
- line = "";
+ line.clear();
bool stop = false;
while (m_currentPos < m_data.length())
{
@@ -479,6 +479,10 @@ int CMusicInfoScanner::RetrieveMusicInfo(CFileItemList& items, const CStdString&
CSong *dbSong = songsMap.Find(pItem->GetPath());
CMusicInfoTag& tag = *pItem->GetMusicInfoTag();
+ if (!tag.Loaded())
+ { // read the tag from a file
+ tag.LoadFromFile(pItem->GetPath());
+ }
// if we have the itemcount, notify our
// observer with the progress we made
@@ -32,12 +32,8 @@ using namespace MUSIC_INFO;
CMusicInfoTag::CMusicInfoTag(const CStdString& strMediaFile /*= CStdString()*/)
{
Clear();
- if (XFILE::CFile::Exists(strMediaFile))
- {
- std::auto_ptr<IMusicInfoTagLoader> pLoader (CMusicInfoTagLoaderFactory::CreateLoader(strMediaFile));
- if (NULL != pLoader.get())
- pLoader->Load(strMediaFile, *this);
- }
+ if (!strMediaFile.IsEmpty())
+ LoadFromFile(strMediaFile);
@jmarshallnz

jmarshallnz Jun 5, 2012

I'd accept this only if it's well doxy'd that the constructor reads the tag from the file if a non-empty path is specified.

}
CMusicInfoTag::CMusicInfoTag(const CMusicInfoTag& tag)
@@ -434,13 +430,13 @@ bool CMusicInfoTag::HasEmbeddedCue() const
{
return !GetEmbeddedCue().IsEmpty();
}
-
+
const CStdString& CMusicInfoTag::GetEmbeddedCue() const
{
return m_strCue;
}
-void CMusicInfoTag::setEmbeddedCue(const CStdString& cuesheet)
+void CMusicInfoTag::SetEmbeddedCue(const CStdString& cuesheet)
{
m_strCue = cuesheet;
}
@@ -593,3 +589,13 @@ CStdString CMusicInfoTag::Trim(const CStdString &value) const
trimmedValue.TrimRight(" \n\r");
return trimmedValue;
}
+
+void CMusicInfoTag::LoadFromFile(const CStdString& strMediaFile)
+{
+ if (XFILE::CFile::Exists(strMediaFile))
+ {
+ std::auto_ptr<IMusicInfoTagLoader> pLoader (CMusicInfoTagLoaderFactory::CreateLoader(strMediaFile));
+ if (NULL != pLoader.get())
+ pLoader->Load(strMediaFile, *this);
+ }
+}
@@ -121,7 +121,9 @@ class CMusicInfoTag : public IArchivable, public ISerializable
bool HasEmbeddedCue() const;
const CStdString& GetEmbeddedCue() const;
- void setEmbeddedCue(const CStdString& cuesheet);
+ void SetEmbeddedCue(const CStdString& cuesheet);
+
@jmarshallnz

jmarshallnz Jun 5, 2012

whitespace at eol

+ void LoadFromFile(const CStdString& strMediaFile);
virtual void Archive(CArchive& ar);
virtual void Serialize(CVariant& ar);
@@ -63,7 +63,7 @@ bool CMusicInfoTagLoaderApe::Load(const CStdString& strFileName, CMusicInfoTag&
tag.SetMusicBrainzArtistID(myTag.GetMusicBrainzArtistID());
tag.SetMusicBrainzTrackID(myTag.GetMusicBrainzTrackID());
tag.SetMusicBrainzTRMID(myTag.GetMusicBrainzTRMID());
- tag.setEmbeddedCue(myTag.GetEmbeddedCue());
+ tag.SetEmbeddedCue(myTag.GetEmbeddedCue());
SYSTEMTIME dateTime;
ZeroMemory(&dateTime, sizeof(SYSTEMTIME));
dateTime.wYear = atoi(myTag.GetYear());
@@ -133,7 +133,7 @@ bool CMusicInfoTagLoaderMP3::Load(const CStdString& strFileName, CMusicInfoTag&
if (apeTag.GetReplayGain().iHasGainInfo)
m_replayGainInfo = apeTag.GetReplayGain();
if (apeTag.GetEmbeddedCue().size())
- tag.setEmbeddedCue(apeTag.GetEmbeddedCue());
+ tag.SetEmbeddedCue(apeTag.GetEmbeddedCue());
if (apeTag.GetRating() > '0')
tag.SetRating(apeTag.GetRating());
}
@@ -128,7 +128,7 @@ int CVorbisTag::ParseTagEntry(CStdString& strTagEntry)
if (strTagType=="CUESHEET")
{
- tag.setEmbeddedCue(strTagValue);
+ tag.SetEmbeddedCue(strTagValue);
}
// Get new style replay gain info

0 comments on commit c5d32c3

Please sign in to comment.