Skip to content

Commit d47d28f

Browse files
ufleischufleisch
committed
Fix and simplify Matroska simple tag
Avoid use of raw pointers, fix property interface.
1 parent 3566b00 commit d47d28f

7 files changed

Lines changed: 329 additions & 305 deletions

File tree

‎examples/matroskareader.cpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,21 @@ int main(int argc, char *argv[])
3030
const TagLib::Matroska::SimpleTagsList &list = tag->simpleTagsList();
3131
printf("Found %u tag(s):\n", list.size());
3232

33-
for(TagLib::Matroska::SimpleTag *t : list) {
34-
PRINT_PRETTY("Tag Name", t->name().toCString(true));
33+
for(const TagLib::Matroska::SimpleTag &t : list) {
34+
PRINT_PRETTY("Tag Name", t.name().toCString(true));
3535

36-
TagLib::Matroska::SimpleTagString *tString = nullptr;
37-
TagLib::Matroska::SimpleTagBinary *tBinary = nullptr;
38-
if((tString = dynamic_cast<TagLib::Matroska::SimpleTagString*>(t)))
39-
PRINT_PRETTY("Tag Value", tString->value().toCString(true));
40-
else if((tBinary = dynamic_cast<TagLib::Matroska::SimpleTagBinary*>(t)))
36+
if(t.type() == TagLib::Matroska::SimpleTag::StringType)
37+
PRINT_PRETTY("Tag Value", t.toString().toCString(true));
38+
else if(t.type() == TagLib::Matroska::SimpleTag::BinaryType)
4139
PRINT_PRETTY("Tag Value",
42-
TagLib::Utils::formatString("Binary with size %i", tBinary->value().size()).toCString(false)
40+
TagLib::Utils::formatString("Binary with size %i", t.toByteVector().size()).toCString(false)
4341
);
4442

45-
auto targetTypeValue = static_cast<unsigned int>(t->targetTypeValue());
43+
auto targetTypeValue = static_cast<unsigned int>(t.targetTypeValue());
4644
PRINT_PRETTY("Target Type Value",
4745
targetTypeValue == 0 ? "None" : TagLib::Utils::formatString("%i", targetTypeValue).toCString(false)
4846
);
49-
const TagLib::String &language = t->language();
47+
const TagLib::String &language = t.language();
5048
PRINT_PRETTY("Language", !language.isEmpty() ? language.toCString(false) : "Not set");
5149

5250
printf("\n");

‎examples/matroskawriter.cpp‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,13 @@ int main(int argc, char *argv[])
2222
auto tag = file.tag(true);
2323
tag->clearSimpleTags();
2424

25-
auto simpleTag = new TagLib::Matroska::SimpleTagString();
26-
simpleTag->setName("Test Name 1");
27-
simpleTag->setTargetTypeValue(TagLib::Matroska::SimpleTag::TargetTypeValue::Track);
28-
simpleTag->setValue("Test Value 1");
29-
simpleTag->setLanguage("en");
30-
tag->addSimpleTag(simpleTag);
25+
tag->addSimpleTag(TagLib::Matroska::SimpleTag(
26+
"Test Name 1", TagLib::String("Test Value 1"),
27+
TagLib::Matroska::SimpleTag::TargetTypeValue::Track, "en"));
3128

32-
simpleTag = new TagLib::Matroska::SimpleTagString();
33-
simpleTag->setName("Test Name 2");
34-
simpleTag->setTargetTypeValue(TagLib::Matroska::SimpleTag::TargetTypeValue::Album);
35-
simpleTag->setValue("Test Value 2");
36-
tag->addSimpleTag(simpleTag);
29+
tag->addSimpleTag(TagLib::Matroska::SimpleTag(
30+
"Test Name 2", TagLib::String("Test Value 2"),
31+
TagLib::Matroska::SimpleTag::TargetTypeValue::Album));
3732
tag->setTitle("Test title");
3833
tag->setArtist("Test artist");
3934
tag->setYear(1969);

‎taglib/matroska/ebml/ebmlmktags.cpp‎

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,47 +68,33 @@ std::unique_ptr<Matroska::Tag> EBML::MkTags::parse()
6868

6969
// Parse each <SimpleTag>
7070
for(auto simpleTag : simpleTags) {
71-
const String *tagName = nullptr;
7271
const String *tagValueString = nullptr;
7372
const ByteVector *tagValueBinary = nullptr;
74-
const String *language = nullptr;
73+
String tagName;
74+
String language;
7575
bool defaultLanguageFlag = true;
7676

7777
for(const auto &simpleTagChild : *simpleTag) {
7878
Id id = simpleTagChild->getId();
79-
if(id == Id::MkTagName && !tagName)
80-
tagName = &(element_cast<Id::MkTagName>(simpleTagChild)->getValue());
79+
if(id == Id::MkTagName && tagName.isEmpty())
80+
tagName = element_cast<Id::MkTagName>(simpleTagChild)->getValue();
8181
else if(id == Id::MkTagString && !tagValueString)
8282
tagValueString = &(element_cast<Id::MkTagString>(simpleTagChild)->getValue());
8383
else if(id == Id::MkTagBinary && !tagValueBinary)
8484
tagValueBinary = &(element_cast<Id::MkTagBinary>(simpleTagChild)->getValue());
85-
else if(id == Id::MkTagsTagLanguage && !language)
86-
language = &(element_cast<Id::MkTagsTagLanguage>(simpleTagChild)->getValue());
85+
else if(id == Id::MkTagsTagLanguage && language.isEmpty())
86+
language = element_cast<Id::MkTagsTagLanguage>(simpleTagChild)->getValue();
8787
else if(id == Id::MkTagsLanguageDefault)
8888
defaultLanguageFlag = element_cast<Id::MkTagsLanguageDefault>(simpleTagChild)->getValue() ? true : false;
8989
}
90-
if(!tagName || (tagValueString && tagValueBinary) || (!tagValueString && !tagValueBinary))
90+
if(tagName.isEmpty() || (tagValueString && tagValueBinary) || (!tagValueString && !tagValueBinary))
9191
continue;
9292

93-
// Create a Simple Tag object and add it to the Tag object
94-
Matroska::SimpleTag *sTag = nullptr;
95-
if(tagValueString) {
96-
auto sTagString = new Matroska::SimpleTagString();
97-
sTagString->setTargetTypeValue(targetTypeValue);
98-
sTagString->setValue(*tagValueString);
99-
sTag = sTagString;
100-
}
101-
else { // tagValueBinary must be non null
102-
auto sTagBinary = new Matroska::SimpleTagBinary();
103-
sTagBinary->setTargetTypeValue(targetTypeValue);
104-
sTagBinary->setValue(*tagValueBinary);
105-
sTag = sTagBinary;
106-
}
107-
sTag->setName(*tagName);
108-
if(language)
109-
sTag->setLanguage(*language);
110-
sTag->setDefaultLanguageFlag(defaultLanguageFlag);
111-
mTag->addSimpleTag(sTag);
93+
mTag->addSimpleTag(tagValueString
94+
? Matroska::SimpleTag(tagName, *tagValueString,
95+
targetTypeValue, language, defaultLanguageFlag)
96+
: Matroska::SimpleTag(tagName, *tagValueBinary,
97+
targetTypeValue, language, defaultLanguageFlag));
11298
}
11399
}
114100
return mTag;

‎taglib/matroska/matroskasimpletag.cpp‎

Lines changed: 57 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
***************************************************************************/
2020

2121
#include "matroskasimpletag.h"
22+
#include <variant>
2223
#include "matroskatag.h"
2324
#include "tstring.h"
2425
#include "tbytevector.h"
@@ -28,101 +29,100 @@ using namespace TagLib;
2829
class Matroska::SimpleTag::SimpleTagPrivate
2930
{
3031
public:
31-
SimpleTagPrivate() = default;
32-
TargetTypeValue targetTypeValue = None;
33-
String name;
34-
String language;
35-
bool defaultLanguageFlag = true;
32+
explicit SimpleTagPrivate(const String &name, const String& value,
33+
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) :
34+
value(value), name(name), language(language),
35+
targetTypeValue(targetTypeValue), defaultLanguageFlag(defaultLanguage) {}
36+
explicit SimpleTagPrivate(const String &name, const ByteVector& value,
37+
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) :
38+
value(value), name(name), language(language),
39+
targetTypeValue(targetTypeValue), defaultLanguageFlag(defaultLanguage) {}
40+
const std::variant<String, ByteVector> value;
41+
const String name;
42+
const String language;
43+
const TargetTypeValue targetTypeValue;
44+
const bool defaultLanguageFlag;
3645
};
3746

38-
class Matroska::SimpleTagString::SimpleTagStringPrivate
39-
{
40-
public:
41-
SimpleTagStringPrivate() = default;
42-
String value;
43-
};
44-
45-
class Matroska::SimpleTagBinary::SimpleTagBinaryPrivate
46-
{
47-
public:
48-
SimpleTagBinaryPrivate() = default;
49-
ByteVector value;
50-
};
47+
////////////////////////////////////////////////////////////////////////////////
48+
// public members
49+
////////////////////////////////////////////////////////////////////////////////
5150

52-
Matroska::SimpleTag::SimpleTag() :
53-
d(std::make_unique<SimpleTagPrivate>())
51+
Matroska::SimpleTag::SimpleTag(const String &name, const String &value,
52+
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) :
53+
d(std::make_unique<SimpleTagPrivate>(name, value, targetTypeValue,
54+
language, defaultLanguage))
5455
{
5556
}
56-
Matroska::SimpleTag::~SimpleTag() = default;
5757

58-
Matroska::SimpleTag::TargetTypeValue Matroska::SimpleTag::targetTypeValue() const
58+
Matroska::SimpleTag::SimpleTag(const String &name, const ByteVector &value,
59+
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage) :
60+
d(std::make_unique<SimpleTagPrivate>(name, value, targetTypeValue,
61+
language, defaultLanguage))
5962
{
60-
return d->targetTypeValue;
6163
}
6264

63-
void Matroska::SimpleTag::setTargetTypeValue(TargetTypeValue targetTypeValue)
65+
Matroska::SimpleTag::SimpleTag(const SimpleTag &other) :
66+
d(std::make_unique<SimpleTagPrivate>(*other.d))
6467
{
65-
d->targetTypeValue = targetTypeValue;
6668
}
6769

68-
const String &Matroska::SimpleTag::name() const
69-
{
70-
return d->name;
71-
}
70+
Matroska::SimpleTag::SimpleTag(SimpleTag&& other) noexcept = default;
7271

73-
const String &Matroska::SimpleTag::language() const
74-
{
75-
return d->language;
76-
}
72+
Matroska::SimpleTag::~SimpleTag() = default;
7773

78-
void Matroska::SimpleTag::setLanguage(const String &language)
79-
{
80-
d->language = language;
81-
}
74+
Matroska::SimpleTag &Matroska::SimpleTag::operator=(SimpleTag &&other) = default;
8275

83-
bool Matroska::SimpleTag::defaultLanguageFlag() const
76+
Matroska::SimpleTag &Matroska::SimpleTag::operator=(const SimpleTag &other)
8477
{
85-
return d->defaultLanguageFlag;
78+
SimpleTag(other).swap(*this);
79+
return *this;
8680
}
8781

88-
void Matroska::SimpleTag::setDefaultLanguageFlag(bool flag)
82+
void Matroska::SimpleTag::swap(SimpleTag &other) noexcept
8983
{
90-
d->defaultLanguageFlag = flag;
84+
using std::swap;
85+
86+
swap(d, other.d);
9187
}
9288

93-
void Matroska::SimpleTag::setName(const String &name)
89+
Matroska::SimpleTag::TargetTypeValue Matroska::SimpleTag::targetTypeValue() const
9490
{
95-
d->name = name;
91+
return d->targetTypeValue;
9692
}
9793

98-
Matroska::SimpleTagString::SimpleTagString() :
99-
dd(std::make_unique<SimpleTagStringPrivate>())
94+
const String &Matroska::SimpleTag::name() const
10095
{
96+
return d->name;
10197
}
102-
Matroska::SimpleTagString::~SimpleTagString() = default;
10398

104-
const String &Matroska::SimpleTagString::value() const
99+
const String &Matroska::SimpleTag::language() const
105100
{
106-
return dd->value;
101+
return d->language;
107102
}
108103

109-
void Matroska::SimpleTagString::setValue(const String &value)
104+
bool Matroska::SimpleTag::defaultLanguageFlag() const
110105
{
111-
dd->value = value;
106+
return d->defaultLanguageFlag;
112107
}
113108

114-
Matroska::SimpleTagBinary::SimpleTagBinary() :
115-
dd(std::make_unique<SimpleTagBinaryPrivate>())
109+
Matroska::SimpleTag::ValueType Matroska::SimpleTag::type() const
116110
{
111+
return std::holds_alternative<ByteVector>(d->value) ? BinaryType : StringType;
117112
}
118-
Matroska::SimpleTagBinary::~SimpleTagBinary() = default;
119113

120-
const ByteVector &Matroska::SimpleTagBinary::value() const
114+
String Matroska::SimpleTag::toString() const
121115
{
122-
return dd->value;
116+
if(std::holds_alternative<String>(d->value)) {
117+
return std::get<String>(d->value);
118+
}
119+
return {};
123120
}
124121

125-
void Matroska::SimpleTagBinary::setValue(const ByteVector &value)
122+
ByteVector Matroska::SimpleTag::toByteVector() const
126123
{
127-
dd->value = value;
124+
if(std::holds_alternative<ByteVector>(d->value)) {
125+
return std::get<ByteVector>(d->value);
126+
}
127+
return {};
128128
}

0 commit comments

Comments
 (0)