Differential D16283 Diff 44366 src/extractors/taglibextractor.cpp

Changeset View

Standalone View

View Options

src/extractors/taglibextractor.cpp

Show All 19 Lines
20 20​
21#include "taglibextractor.h" 21​#include "taglibextractor.h"
22 22​
23// Taglib includes 23​// Taglib includes
24#include <fileref.h> 24​#include <fileref.h>
25#include <tfilestream.h> 25​#include <tfilestream.h>
26#include <flacfile.h> 26​#include <flacfile.h>
27#include <apefile.h> 27​#include <apefile.h>
28​#include <asftag.h>
29​#include <asfattribute.h>
28#include <apetag.h> 30​#include <apetag.h>
29#include <mpcfile.h> 31​#include <mpcfile.h>
30#include <id3v2tag.h> 32​#include <id3v2tag.h>
31#include <id3v1genres.h> 33​#include <id3v1genres.h>
32#include <mpegfile.h> 34​#include <mpegfile.h>
33#include <oggfile.h> 35​#include <oggfile.h>
34#include <mp4file.h> 36​#include <mp4file.h>
35#include <mp4tag.h> 37​#include <mp4tag.h>
▲ Show 20 Lines • Show All 666 Lines • ▼ Show 20 Line(s)522{
702 } 704​ }
703 705​
704 itOgg = lstOgg.find("REPLAYGAIN_ALBUM_PEAK"); 706​ itOgg = lstOgg.find("REPLAYGAIN_ALBUM_PEAK");
705 if (itOgg != lstOgg.end()) { 707​ if (itOgg != lstOgg.end()) {
706 data.replayGainAlbumPeak = TStringToQString((*itOgg).second.toString("")); 708​ data.replayGainAlbumPeak = TStringToQString((*itOgg).second.toString(""));
707 } 709​ }
708} 710​}
709 711​
712​void TagLibExtractor::extractAsfTags(TagLib::ASF::Tag* asfTags, ExtractedData& data)
713​{
714​ if (asfTags->isEmpty()) {
715​ return;
716​ }
717​
718​ if (!asfTags->copyright().isEmpty()) {
719​ data.copyright = asfTags->copyright();
720​ }
721​
brunsUnsubmitted
Done

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

bruns: `// 0->0, 1->2, 25->4, 50->6, 75->8, 99->10`
722​ TagLib::ASF::AttributeList lstASF = asfTags->attribute("WM/SharedUserRating");
723​ if (!lstASF.isEmpty()) {
724​ int rating = lstASF.front().toString().toInt();
725​ //map the rating values of WMP to Baloo rating
726​ //0->0, 1->2, 25->4, 50->6, 75->8, 99->10
727​ if (rating == 0) {
728​ data.rating = 0;
729​ } else if (rating == 1) {
730​ data.rating = 2;
731​ } else {
732​ data.rating = static_cast<uint>(0.09 * rating + 2);
733​ }
734​ }
735​
736​ lstASF = asfTags->attribute("WM/PartOfSet");
737​ if (!lstASF.isEmpty()) {
brunsUnsubmitted
Done

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

bruns: you don't need the `isEmpty()` here, as you iterate over it.
738​ data.discNumber = lstASF.front().toString().toInt();
brunsUnsubmitted
Done

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

bruns: I think you can write instead: `for (const auto& attribute : qAsConst(lstAsf)) { ... }`, as…
739​ }
740​
brunsUnsubmitted
Not Done

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);
}
bruns: this makes me always wonder why we don't do: ``` // decltype(data.albumArtists) == QStringList…
astippichAuthorUnsubmitted
Not Done

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 :)

astippich: Yeah, there is a lot of unneccesary stuff in there that must be cleaned up. But before I get to…
741​ lstASF = asfTags->attribute("WM/AlbumArtist");
742​ for (const auto& attribute : qAsConst(lstASF)) {
743​ if (!data.albumArtists.isEmpty()) {
744​ data.albumArtists += ", ";
745​ }
746​ data.albumArtists += attribute.toString();
747​ }
748​
749​ lstASF = asfTags->attribute("WM/Composer");
750​ for (const auto& attribute : qAsConst(lstASF)) {
751​ if (!data.composers.isEmpty()) {
752​ data.composers += ", ";
753​ }
754​ data.composers += attribute.toString();
755​ }
756​
757​ lstASF = asfTags->attribute("WM/Conductor");
758​ for (const auto& attribute : qAsConst(lstASF)) {
759​ if (!data.conductor.isEmpty()) {
760​ data.conductor += ", ";
761​ }
762​ data.conductor += attribute.toString();
763​ }
764​
765​ lstASF = asfTags->attribute("WM/Writer");
766​ for (const auto& attribute : qAsConst(lstASF)) {
767​ if (!data.lyricists.isEmpty()) {
768​ data.lyricists += ", ";
769​ }
770​ data.lyricists += attribute.toString();
771​ }
772​
773​ lstASF = asfTags->attribute("WM/Publisher");
774​ for (const auto& attribute : qAsConst(lstASF)) {
775​ if (!data.publisher.isEmpty()) {
776​ data.publisher += ", ";
777​ }
778​ data.publisher += attribute.toString();
779​ }
780​
781​ lstASF = asfTags->attribute("Author");
782​ for (const auto& attribute : qAsConst(lstASF)) {
783​ if (!data.author.isEmpty()) {
784​ data.author += ", ";
785​ }
786​ data.author += attribute.toString();
787​ }
788​}
789​
710void TagLibExtractor::extract(ExtractionResult* result) 790​void TagLibExtractor::extract(ExtractionResult* result)
711{ 791​{
712 const QString fileUrl = result->inputUrl(); 792​ const QString fileUrl = result->inputUrl();
713 const QString mimeType = result->inputMimetype(); 793​ const QString mimeType = result->inputMimetype();
714 794​
715 // Open the file readonly. Important if we're sandboxed. 795​ // Open the file readonly. Important if we're sandboxed.
716 TagLib::FileStream stream(fileUrl.toUtf8().constData(), true); 796​ TagLib::FileStream stream(fileUrl.toUtf8().constData(), true);
717 if (!stream.isOpen()) { 797​ if (!stream.isOpen()) {
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Line(s)844 } else if (mimeType == QLatin1String("audio/ogg") || mimeType == QLatin1String("audio/x-vorbis+ogg")) {
766 if (oggFile.tag()) { 846​ if (oggFile.tag()) {
767 extractVorbisTags(oggFile.tag(), data); 847​ extractVorbisTags(oggFile.tag(), data);
768 } 848​ }
769 } else if (mimeType == QLatin1String("audio/opus") || mimeType == QLatin1String("audio/x-opus+ogg")) { 849​ } else if (mimeType == QLatin1String("audio/opus") || mimeType == QLatin1String("audio/x-opus+ogg")) {
770 TagLib::Ogg::Opus::File opusFile(&stream, true); 850​ TagLib::Ogg::Opus::File opusFile(&stream, true);
771 if (opusFile.tag()) { 851​ if (opusFile.tag()) {
772 extractVorbisTags(opusFile.tag(), data); 852​ extractVorbisTags(opusFile.tag(), data);
773 } 853​ }
854​ } else if (mimeType == QLatin1String("audio/x-ms-wma")) {
855​ /* For some reason, TagLib::ASF::File tag() returns only an invalid pointer.
brunsUnsubmitted
Not Done

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

bruns: Can you add a comment here for why the dynamic_cast is used here?
856​ * Use the dynamic_cast instead. */
857​ TagLib::ASF::Tag* asfTags = dynamic_cast<TagLib::ASF::Tag*>(tags);
858​ if (asfTags) {
859​ extractAsfTags(asfTags, data);
860​ }
774 } 861​ }
775 862​
776 if (!tags->isEmpty()) { 863​ if (!tags->isEmpty()) {
777 QString title = TStringToQString(tags->title()); 864​ QString title = TStringToQString(tags->title());
778 if (!title.isEmpty()) { 865​ if (!title.isEmpty()) {
779 result->add(Property::Title, title); 866​ result->add(Property::Title, title);
780 } 867​ }
781 868​
▲ Show 20 Lines • Show All 240 Lines • Show Last 20 Lines