dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
If this extractor is worse than the taglib one, shouldn't the cmake run issue
a warning about that? Or does it already do so because "taglib not found"?
INLINE COMMENTS
> qtmultimediaextractortest.cpp:2
> +/*
> +<<<<<<< HEAD
> + * TagLibExtractor tests.
you probably don't want to commit a merge conflict...
> qtmultimediaextractortest.cpp:8
> + *
> + * Copyright (C) 2015 Juan Palacios <jpalacios...@gmail.com>
> + *
+ your own copyright?
> qtmultimediaextractortest.cpp:39
> +{
> + return QLatin1String(INDEXER_TESTS_SAMPLE_FILES_PATH) +
> QDir::separator() + fileName;
> +}
What if the path contains non-latin characters?
This should use QFile::decodeName instead.
> qtmultimediaextractor.cpp:2
> +/*
> + <one line to give the library's name and an idea of what it does.>
> + Copyright (C) 2012 Vishesh Handa <m...@vhanda.in>
todo
> qtmultimediaextractor.cpp:78
> + if (available && mMetadataExtractor->isMetaDataAvailable()) {
> + album = mMetadataExtractor->metaData(QMediaMetaData::AlbumTitle);
> + title = mMetadataExtractor->metaData(QMediaMetaData::Title);
The mutex needs to be locked for all these writes.
> qtmultimediaextractor.cpp:210
> +
> + auto extractionResult = mSynchronizeCondition.wait(&mSynchronizeMutex,
> 400);
> +
If setMediaSource calls wakeAll before this thread gets here, we will wait
forever (QWaitCondition = if you're not sleeping, you miss the wakeup call).
Use a QSemaphore instead of a QWaitCondition to fix this problem.
> qtmultimediaextractor.cpp:213
> + if (!extractionResult) {
> + mSynchronizeMutex.unlock();
> + return;
Use QMutexLocker instead of manual lock/unlock, to make sure no unlock is
forgotten.
> qtmultimediaextractor.h:2
> +/*
> + <one line to give the library's name and an idea of what it does.>
> + Copyright (C) 2012 Vishesh Handa <m...@vhanda.in>
todo ;)
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D5417
To: mgallien, #frameworks, dfaure
Cc: dfaure, #frameworks
