Skip to content

Commit

Permalink
Merge pull request #10875 from daschuer/musicbrainz_fixes
Browse files Browse the repository at this point in the history 
2.3 Musicbrainz fixes
  • Loading branch information
@ywwg
ywwg authored Nov 3, 2022
2 parents e561dce + bb21444 commit 76fbfb2
Show file tree
Hide file tree
Showing 14 changed files with 546 additions and 167 deletions.
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1580,7 +1580,9 @@ add_executable(mixxx-test
src/test/metaknob_link_test.cpp
src/test/midicontrollertest.cpp
src/test/mixxxtest.cpp
src/test/mock_networkaccessmanager.cpp
src/test/movinginterquartilemean_test.cpp
src/test/musicbrainzrecordingstasktest.cpp
src/test/nativeeffects_test.cpp
src/test/performancetimer_test.cpp
src/test/playcountertest.cpp
Expand Down
60 changes: 29 additions & 31 deletions src/library/dlgtagfetcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,36 +89,44 @@ void DlgTagFetcher::init() {
}

void DlgTagFetcher::slotNext() {
QModelIndex nextRow = m_currentTrackIndex.sibling(
m_currentTrackIndex.row() + 1, m_currentTrackIndex.column());
if (nextRow.isValid()) {
loadTrack(nextRow);
if (isSignalConnected(QMetaMethod::fromSignal(&DlgTagFetcher::next))) {
emit next();
} else {
QModelIndex nextRow = m_currentTrackIndex.sibling(
m_currentTrackIndex.row() + 1, m_currentTrackIndex.column());
if (nextRow.isValid()) {
loadTrack(nextRow);
}
}
}

void DlgTagFetcher::slotPrev() {
QModelIndex prevRow = m_currentTrackIndex.sibling(
m_currentTrackIndex.row() - 1, m_currentTrackIndex.column());
if (prevRow.isValid()) {
loadTrack(prevRow);
if (isSignalConnected(QMetaMethod::fromSignal(&DlgTagFetcher::previous))) {
emit previous();
} else {
QModelIndex prevRow = m_currentTrackIndex.sibling(
m_currentTrackIndex.row() - 1, m_currentTrackIndex.column());
if (prevRow.isValid()) {
loadTrack(prevRow);
}
}
}

void DlgTagFetcher::loadTrackInternal(const TrackPointer& track) {
if (!track) {
return;
void DlgTagFetcher::loadTrack(const TrackPointer& pTrack) {
if (m_track) {
results->clear();
disconnect(m_track.get(),
&Track::changed,
this,
&DlgTagFetcher::slotTrackChanged);
m_data = Data();
m_networkResult = NetworkResult::Ok;
}
results->clear();
disconnect(m_track.get(),
&Track::changed,
this,
&DlgTagFetcher::slotTrackChanged);

m_track = track;
m_data = Data();
m_networkResult = NetworkResult::Ok;
m_track = pTrack;
if (!m_track) {
return;
}

connect(m_track.get(),
&Track::changed,
Expand All @@ -130,20 +138,10 @@ void DlgTagFetcher::loadTrackInternal(const TrackPointer& track) {
updateStack();
}

void DlgTagFetcher::loadTrack(const TrackPointer& track) {
VERIFY_OR_DEBUG_ASSERT(!m_pTrackModel) {
return;
}
loadTrackInternal(track);
}

void DlgTagFetcher::loadTrack(const QModelIndex& index) {
VERIFY_OR_DEBUG_ASSERT(m_pTrackModel) {
return;
}
TrackPointer pTrack = m_pTrackModel->getTrack(index);
m_currentTrackIndex = index;
loadTrackInternal(pTrack);
TrackPointer pTrack = m_pTrackModel->getTrack(index);
loadTrack(pTrack);
}

void DlgTagFetcher::slotTrackChanged(TrackId trackId) {
Expand Down
1 change: 0 additions & 1 deletion src/library/dlgtagfetcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher {
void slotPrev();

private:
void loadTrackInternal(const TrackPointer& track);
void updateStack();
void addDivider(const QString& text, QTreeWidget* parent) const;

Expand Down
4 changes: 2 additions & 2 deletions src/library/dlgtrackinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void DlgTrackInfo::loadTrack(TrackPointer pTrack) {
return;
}
loadTrackInternal(pTrack);
if (m_pDlgTagFetcher && m_pLoadedTrack) {
if (m_pDlgTagFetcher && m_pDlgTagFetcher->isVisible()) {
m_pDlgTagFetcher->loadTrack(m_pLoadedTrack);
}
}
Expand All @@ -327,7 +327,7 @@ void DlgTrackInfo::loadTrack(const QModelIndex& index) {
TrackPointer pTrack = m_pTrackModel->getTrack(index);
m_currentTrackIndex = index;
loadTrackInternal(pTrack);
if (m_pDlgTagFetcher && m_currentTrackIndex.isValid()) {
if (m_pDlgTagFetcher && m_pDlgTagFetcher->isVisible()) {
m_pDlgTagFetcher->loadTrack(m_currentTrackIndex);
}
}
Expand Down
102 changes: 46 additions & 56 deletions src/musicbrainz/tagfetcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ TagFetcher::TagFetcher(QObject* parent)
void TagFetcher::startFetch(
TrackPointer pTrack) {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
cancel();
terminate();

m_pTrack = pTrack;

Expand Down Expand Up @@ -58,6 +58,26 @@ void TagFetcher::cancel() {
}
}

void TagFetcher::terminate() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
m_pTrack.reset();

m_fingerprintWatcher.disconnect(this);
m_fingerprintWatcher.cancel();

if (m_pAcoustIdTask) {
m_pAcoustIdTask->disconnect(this);
m_pAcoustIdTask->deleteLater();
m_pAcoustIdTask = nullptr;
}

if (m_pMusicBrainzTask) {
m_pMusicBrainzTask->disconnect(this);
m_pMusicBrainzTask->deleteLater();
m_pMusicBrainzTask = nullptr;
}
}

void TagFetcher::slotFingerprintReady() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!m_pTrack ||
Expand Down Expand Up @@ -115,7 +135,7 @@ void TagFetcher::slotAcoustIdTaskSucceeded(

if (recordingIds.isEmpty()) {
auto pTrack = std::move(m_pTrack);
cancel();
terminate();

emit resultAvailable(
std::move(pTrack),
Expand Down Expand Up @@ -149,28 +169,14 @@ void TagFetcher::slotAcoustIdTaskSucceeded(
kMusicBrainzTimeoutMillis);
}

bool TagFetcher::onAcoustIdTaskTerminated() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
auto* const pAcoustIdTask = m_pAcoustIdTask.get();
DEBUG_ASSERT(sender());
VERIFY_OR_DEBUG_ASSERT(pAcoustIdTask ==
qobject_cast<mixxx::AcoustIdLookupTask*>(sender())) {
return false;
}
m_pAcoustIdTask = nullptr;
const auto taskDeleter = mixxx::ScopedDeleteLater(pAcoustIdTask);
pAcoustIdTask->disconnect(this);
return true;
}

void TagFetcher::slotAcoustIdTaskFailed(
const mixxx::network::JsonWebResponse& response) {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onAcoustIdTaskTerminated()) {
if (m_pAcoustIdTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();

emit networkError(
response.statusCode(),
Expand All @@ -181,69 +187,53 @@ void TagFetcher::slotAcoustIdTaskFailed(

void TagFetcher::slotAcoustIdTaskAborted() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onAcoustIdTaskTerminated()) {
if (m_pAcoustIdTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();
}

void TagFetcher::slotAcoustIdTaskNetworkError(
QNetworkReply::NetworkError errorCode,
const QString& errorString,
const mixxx::network::WebResponseWithContent& responseWithContent) {
Q_UNUSED(responseWithContent);
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onAcoustIdTaskTerminated()) {
if (m_pAcoustIdTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();

emit networkError(
mixxx::network::kHttpStatusCodeInvalid,
responseWithContent.statusCode(),
QStringLiteral("AcoustID"),
errorString,
errorCode);
}

bool TagFetcher::onMusicBrainzTaskTerminated() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
auto* const pMusicBrainzTask = m_pMusicBrainzTask.get();
DEBUG_ASSERT(sender());
VERIFY_OR_DEBUG_ASSERT(pMusicBrainzTask ==
qobject_cast<mixxx::MusicBrainzRecordingsTask*>(sender())) {
return false;
}
m_pMusicBrainzTask = nullptr;
const auto taskDeleter = mixxx::ScopedDeleteLater(pMusicBrainzTask);
pMusicBrainzTask->disconnect(this);
return true;
}

void TagFetcher::slotMusicBrainzTaskAborted() {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onMusicBrainzTaskTerminated()) {
if (m_pMusicBrainzTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();
}

void TagFetcher::slotMusicBrainzTaskNetworkError(
QNetworkReply::NetworkError errorCode,
const QString& errorString,
const mixxx::network::WebResponseWithContent& responseWithContent) {
Q_UNUSED(responseWithContent);
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onMusicBrainzTaskTerminated()) {
if (m_pMusicBrainzTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();

emit networkError(
mixxx::network::kHttpStatusCodeInvalid,
responseWithContent.statusCode(),
QStringLiteral("MusicBrainz"),
errorString,
errorCode);
Expand All @@ -254,11 +244,11 @@ void TagFetcher::slotMusicBrainzTaskFailed(
int errorCode,
const QString& errorMessage) {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onMusicBrainzTaskTerminated()) {
if (m_pMusicBrainzTask.get() != sender()) {
// stray call from an already aborted try
return;
}

cancel();
terminate();

emit networkError(
response.statusCode(),
Expand All @@ -270,12 +260,12 @@ void TagFetcher::slotMusicBrainzTaskFailed(
void TagFetcher::slotMusicBrainzTaskSucceeded(
const QList<mixxx::musicbrainz::TrackRelease>& guessedTrackReleases) {
DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
if (!onMusicBrainzTaskTerminated()) {
if (m_pMusicBrainzTask.get() != sender()) {
// stray call from an already aborted try
return;
}

auto pTrack = std::move(m_pTrack);
cancel();
auto pTrack = m_pTrack;
terminate();

emit resultAvailable(
std::move(pTrack),
Expand Down
3 changes: 1 addition & 2 deletions src/musicbrainz/tagfetcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ class TagFetcher : public QObject {
const mixxx::network::WebResponseWithContent& responseWithContent);

private:
bool onAcoustIdTaskTerminated();
bool onMusicBrainzTaskTerminated();
void terminate();

QNetworkAccessManager m_network;

Expand Down
Loading

0 comments on commit 76fbfb2

Please sign in to comment.