From d019812e60400cf64321a62c486ecec1cbc67645 Mon Sep 17 00:00:00 2001 From: xerus2000 <27jf@pm.me> Date: Thu, 30 Jul 2020 21:09:53 +0200 Subject: test: Use tempdir for broadcast profiles --- src/test/broadcastprofile_test.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/test/broadcastprofile_test.cpp b/src/test/broadcastprofile_test.cpp index 3110338f8d..4358b2f8d0 100644 --- a/src/test/broadcastprofile_test.cpp +++ b/src/test/broadcastprofile_test.cpp @@ -42,40 +42,39 @@ TEST(BroadcastProfileTest, SaveAndLoadXML) { // Preliminary: set a discriminating value in one of the profile fields QString streamName("unit testing in progress"); - BroadcastProfile profile("Unit Testing Profile"); + BroadcastProfile profile("Broadcast Profile test"); profile.setStreamName(streamName); - QString filename = profile.getProfileName() + QString(".bcp.xml"); + QTemporaryDir tempDir; + ASSERT_TRUE(tempDir.isValid()); + QString filename = tempDir.filePath(profile.getProfileName() + QString(".bcp.xml")); - // Call save() on a profile and assert it actually exists - QFile::remove(filename); // First, make sure it doesn't exists profile.save(filename); ASSERT_TRUE(QFile::exists(filename)); // Load XML file using static loadFromFile and assert // the discriminating value is present BroadcastProfilePtr savedProfile = BroadcastProfile::loadFromFile(filename); - ASSERT_NE(savedProfile, nullptr); - ASSERT_TRUE(savedProfile->getStreamName() == streamName); + EXPECT_NE(savedProfile, nullptr); + EXPECT_TRUE(savedProfile->getStreamName() == streamName); } TEST(BroadcastProfileTest, SaveAndLoadXMLDotName) { - QString profileName("profile has a dot. (in the name)"); - + QString profileName("broadcast profile has a dot. (in the name) test"); BroadcastProfile profile(profileName); - QString filename = profile.getProfileName() + QString(".bcp.xml"); + QTemporaryDir tempDir; + ASSERT_TRUE(tempDir.isValid()); + QString filename = tempDir.filePath(profile.getProfileName() + QString(".bcp.xml")); - // Call save() on a profile and assert it actually exists - QFile::remove(filename); // First, make sure it doesn't exists profile.save(filename); ASSERT_TRUE(QFile::exists(filename)); // Load XML file using static loadFromFile and assert // the discriminating value is present BroadcastProfilePtr savedProfile = BroadcastProfile::loadFromFile(filename); - ASSERT_NE(savedProfile, nullptr); - ASSERT_TRUE(savedProfile->getProfileName() == profileName); + EXPECT_NE(savedProfile, nullptr); + EXPECT_TRUE(savedProfile->getProfileName() == profileName); } TEST(BroadcastProfileTest, SetGetValues) { -- cgit v1.2.3 From a6f59f7766a64cb44a67f02ff1705aaeb7e62b30 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 4 Aug 2020 12:24:38 +0200 Subject: mixer/playerinfo: Make implicit PlayerInfo creation explicit Until now, the `PlayerInfo` singleton was instantiated implicitly when it was first accessed. This commit makes the instantiation explicit by adding a dedicated `PlayerInfo::create()` method. When running tests directly via `mixxx-test` (i.e. not a via `ctest`), all tests will be executed in the *same* process. Thus, the singleton will be instantiated by the first test that accessses the instance, but stay alive until `mixxx-test` is terminated. The `PlayerInfo` instance will repeatedly call `updateCurrentlyPlayingDeck()` through the `timerEvent()` mehtod during its lifetime. This will access some COs, e.g. `[Master],num_decks`. Depending on the currently running test, these COs may or may not be present. When `QCoreApplication::processEvents()` is called, there might be a `timerEvent()` in the queue and will cause CO access. If the CO is *not* present for that test, the test will throw a `DEBUG_ASSERT`. By making the singleton instantiation more explicit, we can add `PlayerInfo::create()` and `PlayerInfo::destroy()` calls only to the constructor/destructor of text fixtures that need it, and ensure that the timer doesn't cause failed debug assertions in unrelated tests. This fixes the remaining `DEBUG_ASSERT` issues and paves the way for merging PR #2911. --- src/mixer/playerinfo.cpp | 10 +++++++++- src/mixer/playerinfo.h | 1 + src/mixxx.cpp | 1 + src/test/autodjprocessor_test.cpp | 20 ++++++++++++-------- src/test/signalpathtest.h | 21 +++++++++++---------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/mixer/playerinfo.cpp b/src/mixer/playerinfo.cpp index 2a9c98e206..47771586fc 100644 --- a/src/mixer/playerinfo.cpp +++ b/src/mixer/playerinfo.cpp @@ -42,9 +42,17 @@ PlayerInfo::~PlayerInfo() { clearControlCache(); } +PlayerInfo& PlayerInfo::create() { + VERIFY_OR_DEBUG_ASSERT(!s_pPlayerInfo) { + return *s_pPlayerInfo; + } + s_pPlayerInfo = new PlayerInfo(); + return *s_pPlayerInfo; +} + // static PlayerInfo& PlayerInfo::instance() { - if (!s_pPlayerInfo) { + VERIFY_OR_DEBUG_ASSERT(s_pPlayerInfo) { s_pPlayerInfo = new PlayerInfo(); } return *s_pPlayerInfo; diff --git a/src/mixer/playerinfo.h b/src/mixer/playerinfo.h index 9a94eb15a6..5aee4fc06c 100644 --- a/src/mixer/playerinfo.h +++ b/src/mixer/playerinfo.h @@ -28,6 +28,7 @@ class PlayerInfo : public QObject { Q_OBJECT public: + static PlayerInfo& create(); static PlayerInfo& instance(); static void destroy(); TrackPointer getTrackInfo(const QString& group); diff --git a/src/mixxx.cpp b/src/mixxx.cpp index d4bc6e4d12..869ab53db6 100644 --- a/src/mixxx.cpp +++ b/src/mixxx.cpp @@ -321,6 +321,7 @@ void MixxxMainWindow::initialize(QApplication* pApp, const CmdlineArgs& args) { &PlayerManager::noVinylControlInputConfigured, this, &MixxxMainWindow::slotNoVinylControlInputConfigured); + PlayerInfo::create(); for (int i = 0; i < kMicrophoneCount; ++i) { m_pPlayerManager->addMicrophone(); diff --git a/src/test/autodjprocessor_test.cpp b/src/test/autodjprocessor_test.cpp index ce85211060..dfb12f7e23 100644 --- a/src/test/autodjprocessor_test.cpp +++ b/src/test/autodjprocessor_test.cpp @@ -1,19 +1,21 @@ -#include +#include "library/autodj/autodjprocessor.h" + #include +#include -#include #include +#include -#include "test/librarytest.h" -#include "library/autodj/autodjprocessor.h" -#include "control/controlpushbutton.h" -#include "control/controlpotmeter.h" #include "control/controllinpotmeter.h" +#include "control/controlpotmeter.h" +#include "control/controlpushbutton.h" #include "engine/engine.h" -#include "mixer/playermanager.h" #include "mixer/basetrackplayer.h" -#include "track/track.h" +#include "mixer/playerinfo.h" +#include "mixer/playermanager.h" #include "sources/soundsourceproxy.h" +#include "test/librarytest.h" +#include "track/track.h" using ::testing::_; using ::testing::Return; @@ -187,6 +189,7 @@ class AutoDJProcessorTest : public LibraryTest { } pPlayerManager.reset(new MockPlayerManager()); + PlayerInfo::create(); // Setup 4 fake decks. ON_CALL(*pPlayerManager, getPlayer(QString("[Channel1]"))) @@ -210,6 +213,7 @@ class AutoDJProcessorTest : public LibraryTest { } virtual ~AutoDJProcessorTest() { + PlayerInfo::destroy(); } TrackId addTrackToCollection(const QString& trackLocation) { diff --git a/src/test/signalpathtest.h b/src/test/signalpathtest.h index d6bd67d070..10d5541651 100644 --- a/src/test/signalpathtest.h +++ b/src/test/signalpathtest.h @@ -1,25 +1,25 @@ -#ifndef ENGINEBACKENDTEST_H_ -#define ENGINEBACKENDTEST_H_ +#pragma once -#include #include +#include -#include #include +#include -#include "preferences/usersettings.h" #include "control/controlobject.h" -#include "mixer/deck.h" #include "effects/effectsmanager.h" -#include "engine/enginebuffer.h" #include "engine/bufferscalers/enginebufferscale.h" #include "engine/channels/enginechannel.h" #include "engine/channels/enginedeck.h" -#include "engine/enginemaster.h" #include "engine/controls/ratecontrol.h" +#include "engine/enginebuffer.h" +#include "engine/enginemaster.h" #include "engine/sync/enginesync.h" +#include "mixer/deck.h" +#include "mixer/playerinfo.h" #include "mixer/previewdeck.h" #include "mixer/sampler.h" +#include "preferences/usersettings.h" #include "test/mixxxtest.h" #include "util/defs.h" #include "util/memory.h" @@ -94,6 +94,8 @@ class BaseSignalPathTest : public MixxxTest { m_pEngineSync = m_pEngineMaster->getEngineSync(); ControlObject::set(ConfigKey("[Master]", "enabled"), 1.0); + + PlayerInfo::create(); } ~BaseSignalPathTest() override { @@ -111,6 +113,7 @@ class BaseSignalPathTest : public MixxxTest { delete m_pEffectsManager; delete m_pVisualsManager; delete m_pNumDecks; + PlayerInfo::destroy(); } void addDeck(EngineDeck* pDeck) { @@ -234,5 +237,3 @@ class SignalPathTest : public BaseSignalPathTest { loadTrack(m_pMixerDeck3, pTrack); } }; - -#endif /* ENGINEBACKENDTEST_H_ */ -- cgit v1.2.3 From f8fd6d1e8e822a3df8410d84c59e0a145118935a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 3 Aug 2020 18:56:29 +0200 Subject: DlgTrackInfo: move track comment to Summary tab, remove Comments tab --- src/library/dlgtrackinfo.ui | 297 ++++++++++++++++++++++++-------------------- 1 file changed, 160 insertions(+), 137 deletions(-) diff --git a/src/library/dlgtrackinfo.ui b/src/library/dlgtrackinfo.ui index d5516db82f..3b0e38c53b 100644 --- a/src/library/dlgtrackinfo.ui +++ b/src/library/dlgtrackinfo.ui @@ -43,6 +43,7 @@ 0 + Summary @@ -53,18 +54,31 @@ - - + + + - + 0 0 + + + false + + + + Title + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + - - + + + 0 @@ -79,15 +93,57 @@ - - + + + + + + 0 + 0 + + + + + 0 + + + QLayout::SetDefaultConstraint + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + + + + + 0 + 0 + + - Track # + Artist + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - + + + 0 @@ -102,8 +158,9 @@ - - + + + 0 @@ -111,25 +168,16 @@ - Album Artist - - - Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - - - - - - Composer + Album Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - + + + 0 @@ -144,39 +192,26 @@ - - + + + 0 0 - - - false - - - Title - - - Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - - - - - - Grouping + Album Artist Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - + + + 0 @@ -191,8 +226,20 @@ - - + + + + + Composer + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + + + + + + 0 @@ -207,38 +254,39 @@ - - + + + - Key + Year - - + + + 0 0 - - - 0 - 0 - - - - + + + - Year + Genre + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - - + + + 0 @@ -253,40 +301,45 @@ - - - - - 0 - 0 - - + + + - Artist - - - Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + Key - - + + + - + 0 0 + + + 0 + 0 + + + + + + + - Album + Grouping - Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop - - + + + 0 @@ -301,8 +354,17 @@ - - + + + + + Track # + + + + + + 0 @@ -317,17 +379,23 @@ - - + + + - Genre + Comments - Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop + + + + + @@ -351,50 +419,14 @@ 40 - 20 + 10 - - - - - 0 - 0 - - - - - 0 - - - QLayout::SetDefaultConstraint - - - 0 - - - 0 - - - 0 - - - 0 - - - - - QLayout::SetDefaultConstraint - - - - - - + @@ -583,7 +615,7 @@ 40 - 20 + 10 @@ -613,16 +645,7 @@ - - - Comments - - - - - - - + BPM -- cgit v1.2.3 From 093356539bd63cca1b480f644cfac0b34b1b5d8c Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Tue, 4 Aug 2020 12:21:01 -0400 Subject: Fix missing nullptr check. Clarify that pickNonSyncSyncTarget get return nullptr --- src/engine/sync/enginesync.h | 4 +++- src/mixer/basetrackplayer.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/engine/sync/enginesync.h b/src/engine/sync/enginesync.h index 8374d81155..29fc05ff2b 100644 --- a/src/engine/sync/enginesync.h +++ b/src/engine/sync/enginesync.h @@ -52,11 +52,13 @@ class EngineSync : public BaseSyncableListener { void notifyScratching(Syncable* pSyncable, bool scratching) override; // Used to pick a sync target for cases where master sync mode is not sufficient. - // Guaranteed to pick a Syncable that is a real deck and has an EngineBuffer. + // Guaranteed to pick a Syncable that is a real deck and has an EngineBuffer, + // but can return nullptr if there are no choices. // First choice is master sync, if it's a real deck, // then it will fall back to the first playing syncable deck, // then it will fall back to the first playing deck, // then it will fall back to the first non-playing deck. + // If there is literally nothing loaded, returns nullptr. Syncable* pickNonSyncSyncTarget(EngineChannel* pDontPick) const; // Used to test whether changing the rate of a Syncable would change the rate diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 2058d66d6a..ebd7b70fd3 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -525,7 +525,9 @@ TrackPointer BaseTrackPlayerImpl::getLoadedTrack() const { void BaseTrackPlayerImpl::slotCloneDeck() { Syncable* syncable = m_pEngineMaster->getEngineSync()->pickNonSyncSyncTarget(m_pChannel); - slotCloneChannel(syncable->getChannel()); + if (syncable) { + slotCloneChannel(syncable->getChannel()); + } } void BaseTrackPlayerImpl::slotCloneFromGroup(const QString& group) { -- cgit v1.2.3 From 10dd1c088063368d1e234f5713b921b5aad5701e Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 4 Aug 2020 19:18:54 +0200 Subject: DlgTrackInfo: add star rating below cover art --- src/library/dlgtrackinfo.cpp | 17 +++++++++++++---- src/library/dlgtrackinfo.h | 2 ++ src/library/dlgtrackinfo.ui | 35 +++++++++++++++++++++-------------- src/widget/wstarrating.cpp | 14 +++++++++----- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/library/dlgtrackinfo.cpp b/src/library/dlgtrackinfo.cpp index ea998a073f..e876f9f273 100644 --- a/src/library/dlgtrackinfo.cpp +++ b/src/library/dlgtrackinfo.cpp @@ -32,6 +32,7 @@ DlgTrackInfo::DlgTrackInfo(QWidget* parent, m_pTapFilter(new TapFilter(this, kFilterLength, kMaxInterval)), m_dLastTapedBpm(-1.), m_pWCoverArtLabel(new WCoverArtLabel(this)), + m_pWStarRating(new WStarRating(nullptr, this)), m_pConfig(pConfig), m_pTrackModel(trackModel) { init(); @@ -44,10 +45,17 @@ DlgTrackInfo::~DlgTrackInfo() { void DlgTrackInfo::init() { setupUi(this); - coverBox->setAlignment(Qt::AlignRight|Qt::AlignTop); - coverBox->setSpacing(0); - coverBox->setContentsMargins(0, 0, 0, 0); - coverBox->insertWidget(1, m_pWCoverArtLabel); + coverLayout->setAlignment(Qt::AlignRight | Qt::AlignTop); + coverLayout->setSpacing(0); + coverLayout->setContentsMargins(0, 0, 0, 0); + coverLayout->insertWidget(0, m_pWCoverArtLabel); + + starsLayout->setAlignment(Qt::AlignRight | Qt::AlignVCenter); + starsLayout->setSpacing(0); + starsLayout->setContentsMargins(0, 0, 0, 0); + starsLayout->insertWidget(0, m_pWStarRating); + // This is necessary to pass on mouseMove events to WStarRating + m_pWStarRating->setMouseTracking(true); m_pTagFetcher.reset(new DlgTagFetcher(this, m_pTrackModel)); if (m_pTrackModel) { @@ -269,6 +277,7 @@ void DlgTrackInfo::populateFields(const Track& track) { m_loadedCoverInfo = track.getCoverInfoWithLocation(); m_pWCoverArtLabel->setCoverArt(m_loadedCoverInfo, QPixmap()); CoverArtCache::requestCover(this, m_loadedCoverInfo); + m_pWStarRating->slotTrackLoaded(m_pLoadedTrack); } void DlgTrackInfo::reloadTrackBeats(const Track& track) { diff --git a/src/library/dlgtrackinfo.h b/src/library/dlgtrackinfo.h index 1f50804fbb..4f4e77ddbd 100644 --- a/src/library/dlgtrackinfo.h +++ b/src/library/dlgtrackinfo.h @@ -16,6 +16,7 @@ #include "util/types.h" #include "widget/wcoverartlabel.h" #include "widget/wcoverartmenu.h" +#include "widget/wstarrating.h" /// A dialog box to display and edit track properties. /// Use TrackPointer to load a track into the dialog or @@ -100,6 +101,7 @@ class DlgTrackInfo : public QDialog, public Ui::DlgTrackInfo { CoverInfo m_loadedCoverInfo; WCoverArtLabel* m_pWCoverArtLabel; + WStarRating* m_pWStarRating; UserSettingsPointer m_pConfig; const TrackModel* m_pTrackModel; diff --git a/src/library/dlgtrackinfo.ui b/src/library/dlgtrackinfo.ui index 3b0e38c53b..51946d11aa 100644 --- a/src/library/dlgtrackinfo.ui +++ b/src/library/dlgtrackinfo.ui @@ -94,7 +94,7 @@ - + @@ -102,25 +102,13 @@ 0 - + 0 QLayout::SetDefaultConstraint - - 0 - - - 0 - - - 0 - - - 0 - @@ -227,6 +215,25 @@ + + + + + 0 + 0 + + + + + 0 + + + QLayout::SetDefaultConstraint + + + + + diff --git a/src/widget/wstarrating.cpp b/src/widget/wstarrating.cpp index d2c6a8150b..db98a0ad7c 100644 --- a/src/widget/wstarrating.cpp +++ b/src/widget/wstarrating.cpp @@ -10,11 +10,15 @@ WStarRating::WStarRating(QString group, QWidget* pParent) m_starRating(0, 5), m_group(group), m_focused(false) { - // Controls to change the star rating with controllers - m_pStarsUp = std::make_unique(ConfigKey(group, "stars_up")); - m_pStarsDown = std::make_unique(ConfigKey(group, "stars_down")); - connect(m_pStarsUp.get(), SIGNAL(valueChanged(double)), this, SLOT(slotStarsUp(double))); - connect(m_pStarsDown.get(), SIGNAL(valueChanged(double)), this, SLOT(slotStarsDown(double))); + // Controls to change the star rating with controllers. + // Note that 'group' maybe NULLPTR, e.g. when called from DlgTrackInfo, + // so only create rate change COs if there's a group passed when creating deck widgets. + if (!m_group.isEmpty()) { + m_pStarsUp = std::make_unique(ConfigKey(group, "stars_up")); + m_pStarsDown = std::make_unique(ConfigKey(group, "stars_down")); + connect(m_pStarsUp.get(), SIGNAL(valueChanged(double)), this, SLOT(slotStarsUp(double))); + connect(m_pStarsDown.get(), SIGNAL(valueChanged(double)), this, SLOT(slotStarsDown(double))); + } } void WStarRating::setup(const QDomNode& node, const SkinContext& context) { -- cgit v1.2.3