diff --git a/src/sources/soundsourceproxy.cpp b/src/sources/soundsourceproxy.cpp index 9ffaa12ee31..396b3f8a821 100644 --- a/src/sources/soundsourceproxy.cpp +++ b/src/sources/soundsourceproxy.cpp @@ -311,7 +311,7 @@ SoundSourceProxy::exportTrackMetadataBeforeSaving( const UserSettingsPointer& pConfig) { DEBUG_ASSERT(pTrack); const auto fileInfo = pTrack->getFileInfo(); - mixxx::MetadataSourcePointer pMetadataSource; + mixxx::SoundSourcePointer pSoundSource; { auto proxy = SoundSourceProxy(fileInfo.toQUrl()); // Ensure that the actual audio properties of the @@ -319,27 +319,25 @@ SoundSourceProxy::exportTrackMetadataBeforeSaving( // This might be needed for converting sample positions // to time positions and vice versa. if (!pTrack->hasStreamInfoFromSource()) { - auto pAudioSource = proxy.openAudioSource(); - if (pAudioSource) { + if (proxy.openSoundSource()) { + pSoundSource = proxy.m_pSoundSource; pTrack->updateStreamInfoFromSource( - pAudioSource->getStreamInfo()); + pSoundSource->getStreamInfo()); DEBUG_ASSERT(pTrack->hasStreamInfoFromSource()); // Make sure that the track file is closed after reading. // Otherwise writing of track metadata into file tags // might fail. - pAudioSource->close(); + pSoundSource->close(); } else { kLogger.warning() << "Failed to update stream info from audio " "source before exporting metadata"; } } - pMetadataSource = proxy.m_pSoundSource; + pSoundSource = proxy.m_pSoundSource; } - if (pMetadataSource) { - return pTrack->exportMetadata( - *pMetadataSource, - pConfig); + if (pSoundSource) { + return pTrack->exportMetadata(*pSoundSource, pConfig); } else { kLogger.warning() << "Unable to export track metadata into file" @@ -348,31 +346,30 @@ SoundSourceProxy::exportTrackMetadataBeforeSaving( } } +// Used during tests only SoundSourceProxy::SoundSourceProxy( TrackPointer pTrack, - const mixxx::SoundSourceProviderPointer& pProvider) + mixxx::SoundSourceProviderPointer pProvider) : m_pTrack(std::move(pTrack)), m_url(m_pTrack ? m_pTrack->getFileInfo().toQUrl() : QUrl()), - m_providerRegistrations(allProviderRegistrationsForUrl(m_url)), m_providerRegistrationIndex(-1) { - initSoundSource(pProvider); + initSoundSourceWithProvider(std::move(pProvider)); } -SoundSourceProxy::SoundSourceProxy( - const QUrl& url, - const mixxx::SoundSourceProviderPointer& pProvider) +SoundSourceProxy::SoundSourceProxy(TrackPointer pTrack) + : m_pTrack(std::move(pTrack)), + m_url(m_pTrack ? m_pTrack->getFileInfo().toQUrl() : QUrl()), + m_providerRegistrations(allProviderRegistrationsForUrl(m_url)) { + findProviderAndInitSoundSource(); +} + +SoundSourceProxy::SoundSourceProxy(const QUrl& url) : m_url(url), - m_providerRegistrations(allProviderRegistrationsForUrl(m_url)), - m_providerRegistrationIndex(-1) { - initSoundSource(pProvider); + m_providerRegistrations(allProviderRegistrationsForUrl(m_url)) { + findProviderAndInitSoundSource(); } -mixxx::SoundSourceProviderPointer SoundSourceProxy::primaryProvider( - const mixxx::SoundSourceProviderPointer& pProvider) { - if (pProvider) { - m_providerRegistrationIndex = -1; - return pProvider; - } +mixxx::SoundSourceProviderPointer SoundSourceProxy::primaryProvider() { m_providerRegistrationIndex = 0; if (m_providerRegistrationIndex < m_providerRegistrations.size()) { return m_providerRegistrations[m_providerRegistrationIndex].getProvider(); @@ -424,49 +421,54 @@ SoundSourceProxy::nextProviderWithOpenMode( } } -void SoundSourceProxy::initSoundSource( - const mixxx::SoundSourceProviderPointer& pProvider) { +void SoundSourceProxy::findProviderAndInitSoundSource() { DEBUG_ASSERT(!m_pProvider); DEBUG_ASSERT(!m_pSoundSource); - auto pNextProvider = primaryProvider(pProvider); - while (!m_pSoundSource && pNextProvider) { - m_pSoundSource = pNextProvider->newSoundSource(m_url); - if (m_pSoundSource) { - m_pProvider = std::move(pNextProvider); - if (kLogger.debugEnabled()) { - kLogger.debug() << "SoundSourceProvider" - << m_pProvider->getDisplayName() - << "created a SoundSource for file" - << getUrl().toString() - << "of type" - << m_pSoundSource->getType(); - } - // Done - return; - } - kLogger.warning() << "SoundSourceProvider" - << pNextProvider->getDisplayName() - << "failed to create a SoundSource for file" - << getUrl().toString(); - if (pProvider) { - // Only a single attempt for the given provider - return; - } - // Switch to next available provider - pNextProvider = nextProvider(); - if (pNextProvider) { + for (m_providerRegistrationIndex = 0; + m_providerRegistrationIndex < m_providerRegistrations.size(); + ++m_providerRegistrationIndex) { + mixxx::SoundSourceProviderPointer pProvider = + m_providerRegistrations[m_providerRegistrationIndex] + .getProvider(); + VERIFY_OR_DEBUG_ASSERT(pProvider) { continue; } - if (!getUrl().isEmpty()) { - kLogger.warning() - << "No SoundSourceProvider for file" - << getUrl().toString(); + if (initSoundSourceWithProvider(std::move(pProvider))) { + return; // Success } - // Abort after failure - return; + } + if (!getUrl().isEmpty()) { + kLogger.warning() + << "No SoundSourceProvider for file" + << getUrl().toString(); } } +bool SoundSourceProxy::initSoundSourceWithProvider( + mixxx::SoundSourceProviderPointer&& pProvider) { + DEBUG_ASSERT(!m_pProvider); + DEBUG_ASSERT(!m_pSoundSource); + DEBUG_ASSERT(pProvider); + m_pSoundSource = pProvider->newSoundSource(m_url); + if (m_pSoundSource) { + m_pProvider = pProvider; + if (kLogger.debugEnabled()) { + kLogger.debug() << "SoundSourceProvider" + << m_pProvider->getDisplayName() + << "created a SoundSource for file" + << getUrl().toString() + << "of type" + << m_pSoundSource->getType(); + } + return true; + } + kLogger.warning() << "SoundSourceProvider" + << pProvider->getDisplayName() + << "failed to create a SoundSource for file" + << getUrl().toString(); + return false; +} + namespace { inline std::pair @@ -767,7 +769,7 @@ bool SoundSourceProxy::updateTrackFromSource( return true; } -mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( +bool SoundSourceProxy::openSoundSource( const mixxx::AudioSource::OpenParams& params) { auto openMode = mixxx::SoundSource::OpenMode::Strict; int attemptCount = 0; @@ -777,16 +779,7 @@ mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( m_pSoundSource->open(openMode, params); if (openResult == mixxx::SoundSource::OpenResult::Succeeded) { if (m_pSoundSource->verifyReadable()) { - // The internal m_pTrack might be null when opening the AudioSource - // before exporting metadata. In this case the caller (this class) - // is responsible for updating the stream info if needed. - if (!m_pTrack) { - return m_pSoundSource; - } - // Overwrite metadata with actual audio properties - m_pTrack->updateStreamInfoFromSource( - m_pSoundSource->getStreamInfo()); - return mixxx::AudioSourceTrackProxy::create(m_pTrack, m_pSoundSource); + return true; } kLogger.warning() << "Failed to read file" @@ -807,7 +800,7 @@ mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( // Do NOT retry with the next SoundSource provider if the file // itself seems to be the cause when opening still fails during // the 2nd (= permissive) round. - return nullptr; + return false; } else { // Continue and give other providers the chance to open the file // in turn. @@ -828,7 +821,7 @@ mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( openMode = nextProviderWithOpenModePair.second; m_pProvider.reset(); m_pSoundSource.reset(); - initSoundSource(std::move(pNextProvider)); + initSoundSourceWithProvider(std::move(pNextProvider)); // try again } // All available providers have returned OpenResult::Aborted when @@ -839,5 +832,19 @@ mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( << "after" << attemptCount << "unsuccessful attempts"; - return nullptr; + return false; +} + +mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource( + const mixxx::AudioSource::OpenParams& params) { + VERIFY_OR_DEBUG_ASSERT(m_pTrack) { + return nullptr; + } + if (!openSoundSource(params)) { + return nullptr; + } + // Overwrite metadata with actual audio properties + m_pTrack->updateStreamInfoFromSource( + m_pSoundSource->getStreamInfo()); + return mixxx::AudioSourceTrackProxy::create(m_pTrack, m_pSoundSource); } diff --git a/src/sources/soundsourceproxy.h b/src/sources/soundsourceproxy.h index 72d91309590..fab94dd989d 100644 --- a/src/sources/soundsourceproxy.h +++ b/src/sources/soundsourceproxy.h @@ -47,9 +47,12 @@ class SoundSourceProxy { static mixxx::SoundSourceProviderPointer getPrimaryProviderForFileExtension( const QString& fileExtension); + explicit SoundSourceProxy(TrackPointer pTrack); + + // Used during unit tests to check SoundSources explicit explicit SoundSourceProxy( TrackPointer pTrack, - const mixxx::SoundSourceProviderPointer& pProvider = nullptr); + mixxx::SoundSourceProviderPointer pProvider); /// The track object that has been passed at construction. /// @@ -181,9 +184,10 @@ class SoundSourceProxy { // Special case: Construction from a url is needed // for writing metadata immediately before the TIO is destroyed. - explicit SoundSourceProxy( - const QUrl& url, - const mixxx::SoundSourceProviderPointer& pProvider = nullptr); + explicit SoundSourceProxy(const QUrl& url); + + bool openSoundSource( + const mixxx::AudioSource::OpenParams& params = mixxx::AudioSource::OpenParams()); const TrackPointer m_pTrack; @@ -196,11 +200,12 @@ class SoundSourceProxy { // provider and is initialized with -1 if no int m_providerRegistrationIndex; - void initSoundSource( - const mixxx::SoundSourceProviderPointer& pProvider); + void findProviderAndInitSoundSource(); + + bool initSoundSourceWithProvider( + mixxx::SoundSourceProviderPointer&& pProvider); - mixxx::SoundSourceProviderPointer primaryProvider( - const mixxx::SoundSourceProviderPointer& pProvider = nullptr); + mixxx::SoundSourceProviderPointer primaryProvider(); mixxx::SoundSourceProviderPointer nextProvider(); std::pair nextProviderWithOpenMode(mixxx::SoundSource::OpenMode);