Skip to content

Commit

Permalink
MythMusic: re-acquire the pointer to the current MusicMetadata when used
Browse files Browse the repository at this point in the history 
Don't store a pointer to the current MusicMetadata which can become a
stale pointer if for example a playing CD is ejected which removes the
CD tracks from the active playlist. It's much safer to update it each time
it's needed and NULL check the returned pointer.

Refs #11708.
  • Loading branch information
Paul Harrison committed Aug 11, 2013
1 parent 5aa92a1 commit ceb3182
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 47 deletions.
84 changes: 38 additions & 46 deletions mythplugins/mythmusic/mythmusic/musicplayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ MusicPlayer::MusicPlayer(QObject *parent)
m_bufferAvailable = 0;
m_bufferSize = 0;

m_currentMetadata = NULL;
m_oneshotMetadata = NULL;

m_isAutoplay = false;
Expand Down Expand Up @@ -436,7 +435,7 @@ void MusicPlayer::next(void)

changeCurrentTrack(currentTrack);

if (m_currentMetadata)
if (getCurrentMetadata())
play();
else
stop();
Expand All @@ -461,7 +460,7 @@ void MusicPlayer::previous(void)
{
changeCurrentTrack(currentTrack);

if (m_currentMetadata)
if (getCurrentMetadata())
play();
else
return;//stop();
Expand Down Expand Up @@ -554,15 +553,21 @@ void MusicPlayer::customEvent(QEvent *event)

m_lastTrackStart += m_currentTime;

mdata->setID(m_currentMetadata->ID());
mdata->setTrack(m_playedList.count() + 1);
mdata->setStation(m_currentMetadata->Station());
mdata->setChannel(m_currentMetadata->Channel());
if (getCurrentMetadata())
{
mdata->setID(getCurrentMetadata()->ID());
mdata->setTrack(m_playedList.count() + 1);
mdata->setStation(getCurrentMetadata()->Station());
mdata->setChannel(getCurrentMetadata()->Channel());

getCurrentMetadata()->setTitle(mdata->Title());
getCurrentMetadata()->setArtist(mdata->Artist());
getCurrentMetadata()->setAlbum(mdata->Album());
getCurrentMetadata()->setTrack(mdata->Track());
}

m_playedList.append(mdata);

m_currentMetadata = m_playedList.last();

if (m_isAutoplay && m_canShowPlayer && m_autoShowPlayer)
{
MythScreenStack *popupStack = GetMythMainWindow()->GetStack("popup stack");
Expand Down Expand Up @@ -795,8 +800,8 @@ void MusicPlayer::customEvent(QEvent *event)
{
// we update the lastplay and playcount after playing
// for m_lastplayDelay seconds or half the total track time
if ((m_currentMetadata && m_currentTime >
(m_currentMetadata->Length() / 1000) / 2) ||
if ((getCurrentMetadata() && m_currentTime >
(getCurrentMetadata()->Length() / 1000) / 2) ||
m_currentTime >= m_lastplayDelay)
{
updateLastplay();
Expand Down Expand Up @@ -824,17 +829,17 @@ void MusicPlayer::customEvent(QEvent *event)
}
else
{
if (m_playMode == PLAYMODE_TRACKS && m_currentMetadata &&
m_currentTime != m_currentMetadata->Length() / 1000)
if (m_playMode == PLAYMODE_TRACKS && getCurrentMetadata() &&
m_currentTime != getCurrentMetadata()->Length() / 1000)
{
LOG(VB_GENERAL, LOG_NOTICE, QString("MusicPlayer: Updating track length was %1s, should be %2s")
.arg(m_currentMetadata->Length() / 1000).arg(m_currentTime));
.arg(getCurrentMetadata()->Length() / 1000).arg(m_currentTime));

m_currentMetadata->setLength(m_currentTime * 1000);
m_currentMetadata->dumpToDatabase();
getCurrentMetadata()->setLength(m_currentTime * 1000);
getCurrentMetadata()->dumpToDatabase();

// this will update any track lengths displayed on screen
gPlayer->sendMetadataChangedEvent(m_currentMetadata->ID());
gPlayer->sendMetadataChangedEvent(getCurrentMetadata()->ID());

// this will force the playlist stats to update
MusicPlayerEvent me(MusicPlayerEvent::TrackChangeEvent, m_currentTrack);
Expand Down Expand Up @@ -911,8 +916,6 @@ void MusicPlayer::loadPlaylist(void)
else
m_currentTrack = 0;
}

m_currentMetadata = NULL;
}

void MusicPlayer::loadStreamPlaylist(void)
Expand Down Expand Up @@ -950,7 +953,7 @@ bool MusicPlayer::setCurrentTrackPos(int pos)
{
changeCurrentTrack(pos);

if (!m_currentMetadata)
if (!getCurrentMetadata())
{
stop();
return false;
Expand All @@ -964,16 +967,16 @@ bool MusicPlayer::setCurrentTrackPos(int pos)
void MusicPlayer::savePosition(void)
{
// can't save a bookmark if we don't know what we are playing
if (!m_currentMetadata)
if (!getCurrentMetadata())
return;

if (m_playMode == PLAYMODE_RADIO)
{
gCoreContext->SaveSetting("MusicRadioBookmark", m_currentMetadata->ID());
gCoreContext->SaveSetting("MusicRadioBookmark", getCurrentMetadata()->ID());
}
else
{
gCoreContext->SaveSetting("MusicBookmark", m_currentMetadata->ID());
gCoreContext->SaveSetting("MusicBookmark", getCurrentMetadata()->ID());
gCoreContext->SaveSetting("MusicBookmarkPosition", m_currentTime);
}
}
Expand Down Expand Up @@ -1004,9 +1007,7 @@ void MusicPlayer::restorePosition(void)
}
}

m_currentMetadata = m_currentPlaylist->getSongAt(m_currentTrack);

if (m_currentMetadata)
if (getCurrentMetadata())
{
play();

Expand Down Expand Up @@ -1060,11 +1061,8 @@ void MusicPlayer::changeCurrentTrack(int trackNo)
QString("MusicPlayer: asked to set the current track to an invalid track no. %1")
.arg(trackNo));
m_currentTrack = -1;
m_currentMetadata = NULL;
return;
}

m_currentMetadata = m_currentPlaylist->getSongAt(m_currentTrack);
}

/// get the metadata for the current track in the playlist
Expand All @@ -1073,15 +1071,10 @@ MusicMetadata *MusicPlayer::getCurrentMetadata(void)
if (m_oneshotMetadata)
return m_oneshotMetadata;

if (m_currentMetadata)
return m_currentMetadata;

if (!m_currentPlaylist || !m_currentPlaylist->getSongAt(m_currentTrack))
return NULL;

m_currentMetadata = m_currentPlaylist->getSongAt(m_currentTrack);

return m_currentMetadata;
return m_currentPlaylist->getSongAt(m_currentTrack);
}

/// get the metadata for the next track in the playlist
Expand All @@ -1091,7 +1084,7 @@ MusicMetadata *MusicPlayer::getNextMetadata(void)
return NULL;

if (m_oneshotMetadata)
return m_currentMetadata;
return getCurrentMetadata();

if (!m_currentPlaylist || !m_currentPlaylist->getSongAt(m_currentTrack))
return NULL;
Expand Down Expand Up @@ -1192,36 +1185,36 @@ void MusicPlayer::setShuffleMode(ShuffleMode mode)

void MusicPlayer::updateLastplay()
{
if (m_playMode == PLAYMODE_TRACKS && m_currentMetadata)
if (m_playMode == PLAYMODE_TRACKS && getCurrentMetadata())
{
m_currentMetadata->incPlayCount();
m_currentMetadata->setLastPlay();
getCurrentMetadata()->incPlayCount();
getCurrentMetadata()->setLastPlay();
}

m_updatedLastplay = true;
}

void MusicPlayer::updateVolatileMetadata(void)
{
if (m_playMode == PLAYMODE_TRACKS && m_currentMetadata && getDecoder())
if (m_playMode == PLAYMODE_TRACKS && getCurrentMetadata() && getDecoder())
{
if (m_currentMetadata->hasChanged())
if (getCurrentMetadata()->hasChanged())
{
m_currentMetadata->persist();
getCurrentMetadata()->persist();

// only write the playcount & rating to the tag if it's enabled by the user
if (GetMythDB()->GetNumSetting("AllowTagWriting", 0) == 1)
{
MetaIO *tagger = MetaIO::createTagger(m_currentMetadata->Filename(true));
MetaIO *tagger = MetaIO::createTagger(getCurrentMetadata()->Filename(true));

if (tagger)
{
tagger->writeVolatileMetadata(m_currentMetadata);
tagger->writeVolatileMetadata(getCurrentMetadata());
delete tagger;
}
}

sendTrackStatsChangedEvent(m_currentMetadata->ID());
sendTrackStatsChangedEvent(getCurrentMetadata()->ID());
}
}
}
Expand Down Expand Up @@ -1344,7 +1337,6 @@ void MusicPlayer::activePlaylistChanged(int trackID, bool deleted)
{
// all tracks were removed
m_currentTrack = -1;
m_currentMetadata = NULL;
stop(true);
MusicPlayerEvent me(MusicPlayerEvent::AllTracksRemovedEvent, 0);
dispatch(me);
Expand Down
1 change: 0 additions & 1 deletion mythplugins/mythmusic/mythmusic/musicplayer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ class MusicPlayer : public QObject, public MythObservable
int m_currentTrack;
int m_currentTime;

MusicMetadata *m_currentMetadata;
MusicMetadata *m_oneshotMetadata;

AudioOutput *m_output;
Expand Down

0 comments on commit ceb3182

Please sign in to comment.