implement more tags for asf metadata
ClosedPublic

Authored by astippich on Oct 17 2018, 7:41 PM.

Details

Summary

add some more tags for asf audio files

Diff Detail

Repository
R286 KFileMetaData
Branch
extract_asf
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4034
Build 4052: arc lint + arc unit
astippich created this revision.Oct 17 2018, 7:41 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptOct 17 2018, 7:41 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
astippich requested review of this revision.Oct 17 2018, 7:41 PM

for some reason the TagLib::ASF::File tag() function returns only an invalid pointer, so I decided to use the dynamic_cast here.

bruns added inline comments.Oct 17 2018, 9:42 PM
src/extractors/taglibextractor.cpp
719

// 0->0, 1->2, 25->4, 50->6, 75->8, 99->10

735

you don't need the isEmpty() here, as you iterate over it.

736

I think you can write instead:
for (const auto& attribute : qAsConst(lstAsf)) { ... },
as TagLib::List<T> has begin() and end()

738

this makes me always wonder why we don't do:

// decltype(data.albumArtists) == QStringList
for (attribute : lstAsf) {
  QString t = TStringToQString(attribute.toString());
  data.albumArtists << contactsFromString(t);
}
src/extractors/taglibextractor.h
28

Side note - I think tfilestream.h is no longer needed.

astippich updated this revision to Diff 43885.Oct 18 2018, 5:54 PM
astippich marked 3 inline comments as done.
  • fix comment and simplify iterating
  • cleanup
astippich added inline comments.Oct 18 2018, 5:55 PM
src/extractors/taglibextractor.cpp
738

Yeah, there is a lot of unneccesary stuff in there that must be cleaned up. But before I get to that, I would like to add tests for multiple entries, and before that we have to agree how the output of mutliple entries shall look like, which brings me to D12950 :)

src/extractors/taglibextractor.h
28

Is it okay if I push this directly?

bruns accepted this revision.Oct 28 2018, 3:57 PM

Add the comment, otherwise its fine.

src/extractors/taglibextractor.cpp
843

Can you add a comment here for why the dynamic_cast is used here?

This revision is now accepted and ready to land.Oct 28 2018, 3:57 PM
This revision was automatically updated to reflect the committed changes.