Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WTrackMenu: Display a modal progress dialog #2899

Merged
merged 8 commits into from
Jul 10, 2020
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,11 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/library/starrating.cpp
src/library/tableitemdelegate.cpp
src/library/trackcollection.cpp
src/library/trackcollectioniterator.cpp
src/library/trackcollectionmanager.cpp
src/library/trackloader.cpp
src/library/trackmodeliterator.cpp
src/library/trackprocessing.cpp
src/library/traktor/traktorfeature.cpp
src/library/treeitem.cpp
src/library/treeitemmodel.cpp
Expand Down Expand Up @@ -779,6 +782,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/util/statsmanager.cpp
src/util/tapfilter.cpp
src/util/task.cpp
src/util/taskmonitor.cpp
src/util/threadcputimer.cpp
src/util/time.cpp
src/util/timer.cpp
Expand Down
4 changes: 4 additions & 0 deletions build/depends.py
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,9 @@ def sources(self, build):
"src/library/coverart.cpp",
"src/library/coverartcache.cpp",
"src/library/coverartutils.cpp",
"src/library/trackcollectioniterator.cpp",
"src/library/trackmodeliterator.cpp",
"src/library/trackprocessing.cpp",

"src/library/crate/cratestorage.cpp",
"src/library/crate/cratefeature.cpp",
Expand Down Expand Up @@ -1302,6 +1305,7 @@ def sources(self, build):
"src/util/file.cpp",
"src/util/mac.cpp",
"src/util/task.cpp",
"src/util/taskmonitor.cpp",
"src/util/experiment.cpp",
"src/util/xml.cpp",
"src/util/tapfilter.cpp",
Expand Down
14 changes: 0 additions & 14 deletions src/library/coverartutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,20 +223,6 @@ void guessTrackCoverInfoConcurrently(
}
}

void guessTrackCoverInfoConcurrently(
QList<TrackPointer> tracks) {
if (tracks.isEmpty()) {
return;
}
if (s_enableConcurrentGuessingOfTrackCoverInfo) {
QtConcurrent::run([tracks] {
CoverInfoGuesser().guessAndSetCoverInfoForTracks(tracks);
});
} else {
CoverInfoGuesser().guessAndSetCoverInfoForTracks(tracks);
}
}

void disableConcurrentGuessingOfTrackCoverInfoDuringTests() {
s_enableConcurrentGuessingOfTrackCoverInfo = false;
}
1 change: 0 additions & 1 deletion src/library/coverartutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class CoverInfoGuesser {
// metadata and folders for image files. All I/O is done in a separate
// thread.
void guessTrackCoverInfoConcurrently(TrackPointer pTrack);
void guessTrackCoverInfoConcurrently(QList<TrackPointer> tracks);

// Concurrent guessing of track covers during short running
// tests may cause spurious test failures due to timing issues.
Expand Down
21 changes: 21 additions & 0 deletions src/library/trackcollectioniterator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#include "library/trackcollectioniterator.h"

#include "library/trackcollection.h"

namespace mixxx {

std::optional<TrackPointer> TrackByIdCollectionIterator::nextItem() {
const auto nextTrackId =
m_trackIdListIter.nextItem();
if (!nextTrackId) {
return std::nullopt;
}
const auto trackPtr =
m_pTrackCollection->getTrackById(*nextTrackId);
if (!trackPtr) {
return std::nullopt;
}
return std::make_optional(trackPtr);
}

} // namespace mixxx
43 changes: 43 additions & 0 deletions src/library/trackcollectioniterator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/// Utilities for iterating through a selection or collection
/// of tracks.

#pragma once

#include <QModelIndex>

#include "track/trackiterator.h"

class TrackCollection;

namespace mixxx {

/// Iterate over selected and valid(!) track pointers in a TrackModel.
/// Invalid (= nullptr) track pointers are skipped silently.
class TrackByIdCollectionIterator final
: public virtual TrackPointerIterator {
public:
TrackByIdCollectionIterator(
const TrackCollection* pTrackCollection,
const TrackIdList& trackIds)
: m_pTrackCollection(pTrackCollection),
m_trackIdListIter(trackIds) {
DEBUG_ASSERT(m_pTrackCollection);
}
~TrackByIdCollectionIterator() override = default;

void reset() override {
m_trackIdListIter.reset();
}

std::optional<int> estimateItemsRemaining() override {
return m_trackIdListIter.estimateItemsRemaining();
}

std::optional<TrackPointer> nextItem() override;

private:
const TrackCollection* const m_pTrackCollection;
TrackIdListIterator m_trackIdListIter;
};

} // namespace mixxx
35 changes: 35 additions & 0 deletions src/library/trackmodeliterator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#include "library/trackmodeliterator.h"

#include "library/trackmodel.h"

namespace mixxx {

std::optional<TrackId> TrackIdModelIterator::nextItem() {
const auto nextModelIndex =
m_modelIndexListIter.nextItem();
if (!nextModelIndex) {
return std::nullopt;
}
const auto trackId =
m_pTrackModel->getTrackId(*nextModelIndex);
if (!trackId.isValid()) {
return std::nullopt;
}
return std::make_optional(trackId);
}

std::optional<TrackPointer> TrackPointerModelIterator::nextItem() {
const auto nextModelIndex =
m_modelIndexListIter.nextItem();
if (!nextModelIndex) {
return std::nullopt;
}
const auto trackPtr =
m_pTrackModel->getTrack(*nextModelIndex);
if (!trackPtr) {
return std::nullopt;
}
return std::make_optional(trackPtr);
}

} // namespace mixxx
72 changes: 72 additions & 0 deletions src/library/trackmodeliterator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/// Utilities for iterating through a selection or collection
/// of tracks identified by QModelIndex.

#pragma once

#include <QModelIndex>

#include "track/trackiterator.h"

class TrackModel;

namespace mixxx {

/// Iterate over selected, valid track ids in a TrackModel.
/// Invalid track ids are skipped silently.
class TrackIdModelIterator final
: public virtual TrackIdIterator {
public:
TrackIdModelIterator(
const TrackModel* pTrackModel,
const QModelIndexList& indexList)
: m_pTrackModel(pTrackModel),
m_modelIndexListIter(indexList) {
DEBUG_ASSERT(m_pTrackModel);
}
~TrackIdModelIterator() override = default;

void reset() override {
m_modelIndexListIter.reset();
}

std::optional<int> estimateItemsRemaining() override {
return m_modelIndexListIter.estimateItemsRemaining();
}

std::optional<TrackId> nextItem() override;

private:
const TrackModel* const m_pTrackModel;
ListItemIterator<QModelIndex> m_modelIndexListIter;
};

/// Iterate over selected, valid track pointers in a TrackModel.
/// Invalid (= nullptr) track pointers are skipped silently.
class TrackPointerModelIterator final
: public virtual TrackPointerIterator {
public:
TrackPointerModelIterator(
const TrackModel* pTrackModel,
const QModelIndexList& indexList)
: m_pTrackModel(pTrackModel),
m_modelIndexListIter(indexList) {
DEBUG_ASSERT(m_pTrackModel);
}
~TrackPointerModelIterator() override = default;

void reset() override {
m_modelIndexListIter.reset();
}

std::optional<int> estimateItemsRemaining() override {
return m_modelIndexListIter.estimateItemsRemaining();
}

std::optional<TrackPointer> nextItem() override;

private:
const TrackModel* const m_pTrackModel;
ListItemIterator<QModelIndex> m_modelIndexListIter;
};

} // namespace mixxx
111 changes: 111 additions & 0 deletions src/library/trackprocessing.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
#include "library/trackprocessing.h"

#include <QThread>

#include "library/trackcollection.h"
#include "library/trackcollectionmanager.h"
#include "util/logger.h"

namespace mixxx {

namespace {

const Logger kLogger("ModalTrackBatchProcessor");

} // anonymous namespace

int ModalTrackBatchProcessor::processTracks(
const QString& progressLabelText,
TrackCollectionManager* pTrackCollectionManager,
TrackPointerIterator* pTrackPointerIterator) {
DEBUG_ASSERT(pTrackCollectionManager);
DEBUG_ASSERT(pTrackPointerIterator);
DEBUG_ASSERT(QThread::currentThread() ==
pTrackCollectionManager->thread());
uklotzde marked this conversation as resolved.
Show resolved Hide resolved
int trackCount = 0;
int estimatedTotalCount =
pTrackPointerIterator->estimateItemsRemaining().value_or(trackCount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused. Total count and remaining count are not the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Estimated = guessed. If unknown we take the number of processed tracks and adjust it when exceeding the maximum.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while of reading the code to understand but it makes sense. Please add a comment:
"Before starting any tasks, total = remaining"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for insisting on a clarification. The update during the loop contains a bug. This never happened, because we only iterate over sequences of known size.

m_bAborted = false;
TaskMonitor taskMonitor(
progressLabelText,
m_minimumProgressDuration,
this);
taskMonitor.registerTask(this);
while (auto nextTrackPointer = pTrackPointerIterator->nextItem()) {
const auto pTrack = *nextTrackPointer;
VERIFY_OR_DEBUG_ASSERT(pTrack) {
kLogger.warning()
<< progressLabelText
<< "failed to load next track for processing";
continue;
}
if (m_bAborted) {
kLogger.info()
<< "Aborting"
<< progressLabelText
<< "after processing"
<< trackCount
<< "of"
<< estimatedTotalCount
<< "track(s)";
return trackCount;
}
switch (doProcessNextTrack(pTrack)) {
case ProcessNextTrackResult::AbortProcessing:
kLogger.info()
<< progressLabelText
<< "aborted while processing"
<< trackCount + 1
<< "of"
<< estimatedTotalCount
<< "track(s)";
return trackCount;
case ProcessNextTrackResult::ContinueProcessing:
break;
case ProcessNextTrackResult::SaveTrackAndContinueProcessing:
pTrackCollectionManager->saveTrack(pTrack);
break;
}
++trackCount;
if (trackCount > estimatedTotalCount) {
estimatedTotalCount =
pTrackPointerIterator->estimateItemsRemaining().value_or(trackCount);
}
taskMonitor.reportTaskProgress(
this,
kPercentageOfCompletionMin +
(kPercentageOfCompletionMax -
kPercentageOfCompletionMin) *
trackCount /
static_cast<PercentageOfCompletion>(
estimatedTotalCount));
}
return trackCount;
}

ModalTrackBatchOperationProcessor::ModalTrackBatchOperationProcessor(
const TrackPointerOperation* pTrackPointerOperation,
Mode mode,
Duration progressGracePeriod,
QObject* parent)
: ModalTrackBatchProcessor(progressGracePeriod, parent),
m_pTrackPointerOperation(pTrackPointerOperation),
m_mode(mode) {
DEBUG_ASSERT(m_pTrackPointerOperation);
}

ModalTrackBatchProcessor::ProcessNextTrackResult
ModalTrackBatchOperationProcessor::doProcessNextTrack(
const TrackPointer& pTrack) {
m_pTrackPointerOperation->apply(pTrack);
switch (m_mode) {
case Mode::Apply:
return ProcessNextTrackResult::ContinueProcessing;
case Mode::ApplyAndSave:
return ProcessNextTrackResult::SaveTrackAndContinueProcessing;
}
DEBUG_ASSERT(!"unreachable");
return ProcessNextTrackResult::AbortProcessing;
}

} // namespace mixxx
Loading