[Merge] lp:~phablet-team/qtubuntu-media/bg-playlist-fixes into lp:qtubuntu-media/stable
Alfonso Sanchez-Beato
alfonso.sanchez-beato at canonical.com
Fri Nov 13 14:44:18 UTC 2015
Will address comments asap. Please see also comment below.
Diff comments:
>
> === modified file 'src/aal/aalmediaplaylistprovider.cpp'
> --- src/aal/aalmediaplaylistprovider.cpp 2015-10-23 15:08:10 +0000
> +++ src/aal/aalmediaplaylistprovider.cpp 2015-11-12 18:12:18 +0000
> @@ -228,6 +276,19 @@
>
> try {
> m_hubTrackList->reset();
> +
The idea is to do nothing when the signal arrives. Also, clearing the lut before actually calling reset is wrong because the call to m_hubTrackList->reset() could fail. Finally, if the thread that is calling reset() is different from the one that processes the TrackReset signal, which is the case, modifying the lut from that other thread would create a race condition and nothing but some sort locking can save us from that (think of the lut being modified in L1 cache of one core and then the second core accessing to a different copy of the lut in its cache).
> + // We do not wait for the TrackListReset signal to empty the lut to
> + // avoid sync problems.
> + // TODO: Do the same in other calls if possible, as these calls are
> + // considered to be synchronous, and the job can be considered finished
> + // when the DBus call returns without error. If we need some information
> + // from the signal we should block until it arrives.
> + int num_tracks = track_index_lut.size();
> + if (num_tracks > 0) {
> + track_index_lut.clear();
> + Q_EMIT mediaAboutToBeRemoved(0, num_tracks - 1);
> + Q_EMIT mediaRemoved(0, num_tracks - 1);
> + }
> }
> catch (const std::runtime_error &e) {
> qWarning() << "Failed to clear the playlist: " << e.what();
--
https://code.launchpad.net/~phablet-team/qtubuntu-media/bg-playlist-fixes/+merge/276164
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu-media/stable.
More information about the Ubuntu-reviews
mailing list