Skip to content

Commit f4e7a74

Browse files
committed
Improve Matroska API
Make AttachedFile immutable. This is consistent with SimpleTag and Chapter and avoids using attached files which do not have all required attributes. Provide methods to insert and remove a single simple tag, so that they can be modified without setting all of them while still not exposing internal lists to the API. Use DATE_RECORDED instead of DATE_RELEASED for year() and the "DATE" property. This is more consistent with other tag formats, e.g. for ID3v2 "TDRC" is used, which is the recording time.
1 parent 68a514f commit f4e7a74

10 files changed

Lines changed: 99 additions & 124 deletions

examples/matroskawriter.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,8 @@ int main(int argc, char *argv[])
3636
TagLib::FileStream image(argv[2]);
3737
auto data = image.readBlock(image.length());
3838
auto attachments = file.attachments(true);
39-
TagLib::Matroska::AttachedFile attachedFile;
40-
attachedFile.setFileName("cover.jpg");
41-
attachedFile.setMediaType("image/jpeg");
42-
attachedFile.setData(data);
43-
//attachedFile.setUID(5081000385627515072ull);
44-
attachments->addAttachedFile(attachedFile);
39+
attachments->addAttachedFile(TagLib::Matroska::AttachedFile(
40+
data, "cover.jpg", "image/jpeg"));
4541

4642
file.save();
4743

taglib/matroska/ebml/ebmlmkattachments.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,9 @@ std::unique_ptr<Matroska::Attachments> EBML::MkAttachments::parse() const
7373
if(!(filename && data))
7474
continue;
7575

76-
Matroska::AttachedFile file;
77-
file.setFileName(*filename);
78-
file.setData(*data);
79-
if(description)
80-
file.setDescription(*description);
81-
if(mediaType)
82-
file.setMediaType(*mediaType);
83-
if(uid)
84-
file.setUID(uid);
85-
86-
attachments->addAttachedFile(file);
76+
attachments->addAttachedFile(Matroska::AttachedFile(
77+
*data, *filename, mediaType ? *mediaType : String(),
78+
uid, description ? *description : String()));
8779
}
8880
return attachments;
8981
}

taglib/matroska/matroskaattachedfile.cpp

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,17 @@
1919
***************************************************************************/
2020

2121
#include "matroskaattachedfile.h"
22-
#include "tstring.h"
2322
#include "tbytevector.h"
2423

2524
using namespace TagLib;
2625

2726
class Matroska::AttachedFile::AttachedFilePrivate
2827
{
2928
public:
30-
AttachedFilePrivate() = default;
29+
AttachedFilePrivate(const ByteVector &data, const String &fileName,
30+
const String &mediaType, UID uid, const String &description) :
31+
fileName(fileName), description(description), mediaType(mediaType),
32+
data(data), uid(uid) {}
3133
~AttachedFilePrivate() = default;
3234
String fileName;
3335
String description;
@@ -40,8 +42,10 @@ class Matroska::AttachedFile::AttachedFilePrivate
4042
// public members
4143
////////////////////////////////////////////////////////////////////////////////
4244

43-
Matroska::AttachedFile::AttachedFile() :
44-
d(std::make_unique<AttachedFilePrivate>())
45+
Matroska::AttachedFile::AttachedFile(const ByteVector &data,
46+
const String &fileName, const String &mediaType, UID uid,
47+
const String &description) :
48+
d(std::make_unique<AttachedFilePrivate>(data, fileName, mediaType, uid, description))
4549
{
4650
}
4751

@@ -69,51 +73,26 @@ void Matroska::AttachedFile::swap(AttachedFile &other) noexcept
6973
swap(d, other.d);
7074
}
7175

72-
void Matroska::AttachedFile::setFileName(const String &fileName)
73-
{
74-
d->fileName = fileName;
75-
}
76-
7776
const String &Matroska::AttachedFile::fileName() const
7877
{
7978
return d->fileName;
8079
}
8180

82-
void Matroska::AttachedFile::setDescription(const String &description)
83-
{
84-
d->description = description;
85-
}
86-
8781
const String &Matroska::AttachedFile::description() const
8882
{
8983
return d->description;
9084
}
9185

92-
void Matroska::AttachedFile::setMediaType(const String &mediaType)
93-
{
94-
d->mediaType = mediaType;
95-
}
96-
9786
const String &Matroska::AttachedFile::mediaType() const
9887
{
9988
return d->mediaType;
10089
}
10190

102-
void Matroska::AttachedFile::setData(const ByteVector &data)
103-
{
104-
d->data = data;
105-
}
106-
10791
const ByteVector &Matroska::AttachedFile::data() const
10892
{
10993
return d->data;
11094
}
11195

112-
void Matroska::AttachedFile::setUID(UID uid)
113-
{
114-
d->uid = uid;
115-
}
116-
11796
Matroska::AttachedFile::UID Matroska::AttachedFile::uid() const
11897
{
11998
return d->uid;

taglib/matroska/matroskaattachedfile.h

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#define TAGLIB_MATROSKAATTACHEDFILE_H
2323

2424
#include <memory>
25+
#include "tstring.h"
2526
#include "taglib_export.h"
2627

2728
namespace TagLib {
@@ -39,7 +40,9 @@ namespace TagLib {
3940
/*!
4041
* Construct an attached file.
4142
*/
42-
AttachedFile();
43+
AttachedFile(const ByteVector &data, const String &fileName,
44+
const String &mediaType, UID uid = 0,
45+
const String &description = String());
4346

4447
/*!
4548
* Construct an attached file as a copy of \a other.
@@ -71,51 +74,26 @@ namespace TagLib {
7174
*/
7275
void swap(AttachedFile &other) noexcept;
7376

74-
/*!
75-
* Set the \a fileName of the attached file.
76-
*/
77-
void setFileName(const String &fileName);
78-
7977
/*!
8078
* Returns the filename of the attached file.
8179
*/
8280
const String &fileName() const;
8381

84-
/*!
85-
* Set a human-friendly \a description for the attached file.
86-
*/
87-
void setDescription(const String &description);
88-
8982
/*!
9083
* Returns the human-friendly description for the attached file.
9184
*/
9285
const String &description() const;
9386

94-
/*!
95-
* Set the \a mediaType of the attached file.
96-
*/
97-
void setMediaType(const String &mediaType);
98-
9987
/*!
10088
* Returns the media type of the attached file.
10189
*/
10290
const String &mediaType() const;
10391

104-
/*!
105-
* Set the data of the attached file.
106-
*/
107-
void setData(const ByteVector &data);
108-
10992
/*!
11093
* Returns the data of the attached file.
11194
*/
11295
const ByteVector &data() const;
11396

114-
/*!
115-
* Set the \a uid representing the file, as random as possible.
116-
*/
117-
void setUID(UID uid);
118-
11997
/*!
12098
* Returns the UID of the attached file.
12199
*/

taglib/matroska/matroskafile.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,9 @@ bool Matroska::File::setComplexProperties(const String &key, const List<VariantM
335335
fileName = "attachment." + ext;
336336
}
337337
if(!mimeType.isEmpty() && !fileName.isEmpty()) {
338-
AttachedFile attachedFile;
339-
attachedFile.setData(data);
340-
attachedFile.setMediaType(mimeType);
341-
attachedFile.setDescription(property.value("description").value<String>());
342-
attachedFile.setFileName(fileName);
343-
attachedFile.setUID(uid);
344-
d->attachments->addAttachedFile(attachedFile);
338+
d->attachments->addAttachedFile(AttachedFile(
339+
data, fileName, mimeType, uid,
340+
property.value("description").value<String>()));
345341
}
346342
}
347343
return true;

taglib/matroska/matroskasimpletag.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ using namespace TagLib;
2828
class Matroska::SimpleTag::SimpleTagPrivate
2929
{
3030
public:
31-
explicit SimpleTagPrivate(const String &name, const String &value,
31+
SimpleTagPrivate(const String &name, const String &value,
3232
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage,
3333
unsigned long long trackUid) :
3434
value(value), name(name), language(language), trackUid(trackUid),
3535
targetTypeValue(targetTypeValue), defaultLanguageFlag(defaultLanguage) {}
36-
explicit SimpleTagPrivate(const String &name, const ByteVector &value,
36+
SimpleTagPrivate(const String &name, const ByteVector &value,
3737
TargetTypeValue targetTypeValue, const String &language, bool defaultLanguage,
3838
unsigned long long trackUid) :
3939
value(value), name(name), language(language), trackUid(trackUid),

taglib/matroska/matroskatag.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,35 @@ void Matroska::Tag::addSimpleTag(const SimpleTag &tag)
8989
setNeedsRender(true);
9090
}
9191

92+
void Matroska::Tag::addSimpleTags(const SimpleTagsList& simpleTags)
93+
{
94+
d->tags.append(simpleTags);
95+
setNeedsRender(true);
96+
}
97+
98+
void Matroska::Tag::insertSimpleTag(unsigned int index, const SimpleTag &tag)
99+
{
100+
if(index < d->tags.size()) {
101+
auto it = d->tags.begin();
102+
std::advance(it, index);
103+
d->tags.insert(it, tag);
104+
}
105+
else {
106+
d->tags.append(tag);
107+
}
108+
setNeedsRender(true);
109+
}
110+
111+
void Matroska::Tag::removeSimpleTag(unsigned int index)
112+
{
113+
if(index < d->tags.size()) {
114+
auto it = d->tags.begin();
115+
std::advance(it, index);
116+
d->tags.erase(it);
117+
setNeedsRender(true);
118+
}
119+
}
120+
92121
void Matroska::Tag::removeSimpleTag(const String &name,
93122
SimpleTag::TargetTypeValue targetTypeValue,
94123
unsigned long long trackUid)
@@ -323,7 +352,7 @@ namespace
323352
std::tuple("DISCNUMBER", "PART_NUMBER", Matroska::SimpleTag::TargetTypeValue::Album, true),
324353
std::tuple("TRACKTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Track, false),
325354
std::tuple("DISCTOTAL", "TOTAL_PARTS", Matroska::SimpleTag::TargetTypeValue::Album, true),
326-
std::tuple("DATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Album, false),
355+
std::tuple("DATE", "DATE_RECORDED", Matroska::SimpleTag::TargetTypeValue::Track, false),
327356
std::tuple("TITLESORT", "TITLESORT", Matroska::SimpleTag::TargetTypeValue::Track, false),
328357
std::tuple("ALBUMSORT", "TITLESORT", Matroska::SimpleTag::TargetTypeValue::Album, true),
329358
std::tuple("ARTISTSORT", "ARTISTSORT", Matroska::SimpleTag::TargetTypeValue::Track, false),
@@ -334,7 +363,7 @@ namespace
334363
std::tuple("DJMIXER", "MIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track, false),
335364
std::tuple("REMIXER", "REMIXED_BY", Matroska::SimpleTag::TargetTypeValue::Track, false),
336365
std::tuple("INITIALKEY", "INITIAL_KEY", Matroska::SimpleTag::TargetTypeValue::Track, false),
337-
std::tuple("RELEASEDATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Track, false),
366+
std::tuple("RELEASEDATE", "DATE_RELEASED", Matroska::SimpleTag::TargetTypeValue::Album, false),
338367
std::tuple("ENCODINGTIME", "DATE_ENCODED", Matroska::SimpleTag::TargetTypeValue::Track, false),
339368
std::tuple("TAGGINGDATE", "DATE_TAGGED", Matroska::SimpleTag::TargetTypeValue::Track, false),
340369
std::tuple("ENCODEDBY", "ENCODER", Matroska::SimpleTag::TargetTypeValue::Track, false),

taglib/matroska/matroskatag.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,21 @@ namespace TagLib {
9999
*/
100100
void addSimpleTag(const SimpleTag &tag);
101101

102+
/*!
103+
* Add multiple tag attributes.
104+
*/
105+
void addSimpleTags(const SimpleTagsList &simpleTags);
106+
107+
/*!
108+
* Insert a tag attribute at position \a index.
109+
*/
110+
void insertSimpleTag(unsigned int index, const SimpleTag &tag);
111+
112+
/*!
113+
* Remove a tag attribute at position \a index.
114+
*/
115+
void removeSimpleTag(unsigned int index);
116+
102117
/*!
103118
* Remove a tag attribute.
104119
*/

taglib/toolkit/propertymapping.dox

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
| CONDUCTOR | TPE3 | | \----:com.apple.iTunes:CONDUCTOR | | WM/Conductor | |
3535
| COPYRIGHT | TCOP | ICOP | cprt | | | |
3636
| COPYRIGHTURL | WCOP | | | | | |
37-
| DATE | TDRC | ICRD | &copy;day | YEAR | WM/Year | DATE_RELEASED/50 |
37+
| DATE | TDRC | ICRD | &copy;day | YEAR | WM/Year | DATE_RECORDED/30 |
3838
| DISCNUMBER | TPOS | | disk | DISC | WM/PartOfSet | PART_NUMBER/50 |
3939
| DISCSUBTITLE | TSST | PRT1 | \----:com.apple.iTunes:DISCSUBTITLE | | WM/SetSubTitle | |
4040
| DISCTOTAL | | | | | | TOTAL_PARTS/50 |

0 commit comments

Comments
 (0)