smithjd added a comment.
This test fails:
FAIL! : TagLibExtractorTest::testMp4(mp4) Compared values are not the same
Actual (resultMp4.properties().value(Property::Rating).toInt()): 0
Expected (8) : 8
Loc:
[/home/enderw
This revision was automatically updated to reflect the committed changes.
Closed by commit R286:649555ee3182: Rewrite the taglib extractor to use the
generic PropertyMap interface (authored by astippich).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D18826?vs=53587&id=53599#toc
REPOSITO
bruns accepted this revision.
bruns added a comment.
This revision is now accepted and ready to land.
Thanks, looks good to me now.
REPOSITORY
R286 KFileMetaData
BRANCH
arcpatch-D18826
REVISION DETAIL
https://phabricator.kde.org/D18826
To: astippich, ngraham, bruns, mgallien
Cc: smith
astippich updated this revision to Diff 53587.
astippich marked an inline comment as done.
astippich added a comment.
- spelling fixes
REPOSITORY
R286 KFileMetaData
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18826?vs=53585&id=53587
BRANCH
arcpatch-D18826
REVISION DETAIL
astippich marked 3 inline comments as done.
astippich added inline comments.
INLINE COMMENTS
> bruns wrote in taglibextractor.cpp:364
> This intermediate list is not required, you can directly call result->add()
> for each attribute in lstASF.
After further investigation there is no need to loo
astippich updated this revision to Diff 53585.
astippich added a comment.
- update
REPOSITORY
R286 KFileMetaData
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18826?vs=52890&id=53585
BRANCH
arcpatch-D18826
REVISION DETAIL
https://phabricator.kde.org/D18826
AFFECTED FILES
bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> taglibextractor.cpp:364
> +lstASF = asfTags->attribute("Author");
> +QStringList authors;
> for (const auto& attribute : qAsConst(lstASF)) {
This i
astippich added a comment.
ping
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D18826
To: astippich, ngraham, bruns, mgallien
Cc: smithjd, kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov,
michaelh, astippich, spoorun, ngraham, bruns, abrahams
astippich added inline comments.
INLINE COMMENTS
> bruns wrote in taglibextractor.cpp:208
> Can you also use the same style as above, i.e. `const auto` and `for( : )`?
Oh, damn copy&paste
> smithjd wrote in taglibextractor.cpp:195
> lstASF = asfTags->attribute("WM/Writer");
> ...
>
> The exist
astippich updated this revision to Diff 52890.
astippich marked 8 inline comments as done.
astippich added a comment.
- rebase on master
- implement review comments
- cleanup includes
REPOSITORY
R286 KFileMetaData
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18826?vs=52679&i
bruns added inline comments.
INLINE COMMENTS
> taglibextractor.cpp:184
> +if (savedProperties.contains("COMPOSER")) {
> +const auto composersString =
> TStringToQString(savedProperties["COMPOSER"].toString(";")).trimmed();
> +const auto composers = contactsFromString(compose
smithjd added inline comments.
INLINE COMMENTS
> astippich wrote in taglibextractor.cpp:195
> I wanted to do so first, but that will require to also put the PropertyMap
> into the extractAsfTags method, which I think is not worth it.
lstASF = asfTags->attribute("WM/Writer");
...
The existing c
astippich updated this revision to Diff 52679.
astippich added a comment.
- remove now unnecessary include
REPOSITORY
R286 KFileMetaData
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18826?vs=52675&id=52679
BRANCH
rewrite_taglib_extractor
REVISION DETAIL
https://phabricator
astippich marked an inline comment as done.
astippich added inline comments.
INLINE COMMENTS
> smithjd wrote in taglibextractor.cpp:195
> Can this be moved into the asf-specific extractions?
I wanted to do so first, but that will require to also put the PropertyMap into
the extractAsfTags metho
astippich updated this revision to Diff 52675.
astippich added a comment.
- return early if map is empty
REPOSITORY
R286 KFileMetaData
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18826?vs=51128&id=52675
BRANCH
rewrite_taglib_extractor
REVISION DETAIL
https://phabricator.k
smithjd added inline comments.
INLINE COMMENTS
> taglibextractor.cpp:109
> +{
> +if (savedProperties.contains("TITLE")) {
> +result->add(Property::Title,
> TStringToQString(savedProperties["TITLE"].toString()).trimmed());
This could return early if the property map is empty.
> tagl
astippich added a comment.
ping. I know this is quite a large diff, but it fixes a potential crash
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D18826
To: astippich, ngraham, bruns, mgallien
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich
astippich edited the test plan for this revision.
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D18826
To: astippich, ngraham, bruns, mgallien
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
astippich created this revision.
astippich added reviewers: ngraham, bruns, mgallien.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.
REVISION SUMMARY
Rewrite the taglib extractor to use taglib's
Prop
19 matches
Mail list logo
