Skip to content

Commit f1b40de

Browse files
committed
Unify File::save(...) APIs between file formats that support ID3v2
Closes #922
1 parent 0b99cd9 commit f1b40de

File tree

6 files changed

+84
-7
lines changed

6 files changed

+84
-7
lines changed

taglib/riff/aiff/aifffile.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ RIFF::AIFF::Properties *RIFF::AIFF::File::audioProperties() const
117117
}
118118

119119
bool RIFF::AIFF::File::save()
120+
{
121+
return save(ID3v2::v4);
122+
}
123+
124+
bool RIFF::AIFF::File::save(ID3v2::Version version)
120125
{
121126
if(readOnly()) {
122127
debug("RIFF::AIFF::File::save() -- File is read only.");
@@ -135,7 +140,7 @@ bool RIFF::AIFF::File::save()
135140
}
136141

137142
if(tag() && !tag()->isEmpty()) {
138-
setChunkData("ID3 ", d->tag->render());
143+
setChunkData("ID3 ", d->tag->render(version));
139144
d->hasID3v2 = true;
140145
}
141146

taglib/riff/aiff/aifffile.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ namespace TagLib {
119119
*/
120120
virtual bool save();
121121

122+
/*!
123+
* Save using a specific ID3v2 version (e.g. v3)
124+
*/
125+
bool save(ID3v2::Version version);
126+
122127
/*!
123128
* Returns whether or not the file on disk actually has an ID3v2 tag.
124129
*

taglib/riff/wav/wavfile.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,13 @@ bool RIFF::WAV::File::save()
151151
}
152152

153153
bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version)
154+
{
155+
return save(tags,
156+
stripOthers ? StripOthers : StripNone,
157+
id3v2Version == 3 ? ID3v2::v3 : ID3v2::v4);
158+
}
159+
160+
bool RIFF::WAV::File::save(TagTypes tags, StripTags strip, ID3v2::Version version)
154161
{
155162
if(readOnly()) {
156163
debug("RIFF::WAV::File::save() -- File is read only.");
@@ -162,14 +169,14 @@ bool RIFF::WAV::File::save(TagTypes tags, bool stripOthers, int id3v2Version)
162169
return false;
163170
}
164171

165-
if(stripOthers)
166-
strip(static_cast<TagTypes>(AllTags & ~tags));
172+
if(strip == StripOthers)
173+
File::strip(static_cast<TagTypes>(AllTags & ~tags));
167174

168175
if(tags & ID3v2) {
169176
removeTagChunks(ID3v2);
170177

171178
if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) {
172-
setChunkData("ID3 ", ID3v2Tag()->render(id3v2Version));
179+
setChunkData("ID3 ", ID3v2Tag()->render(version));
173180
d->hasID3v2 = true;
174181
}
175182
}

taglib/riff/wav/wavfile.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,19 @@ namespace TagLib {
159159
*/
160160
virtual bool save();
161161

162-
bool save(TagTypes tags, bool stripOthers = true, int id3v2Version = 4);
162+
/*!
163+
* \deprecated
164+
*/
165+
TAGLIB_DEPRECATED bool save(TagTypes tags, bool stripOthers, int id3v2Version = 4);
166+
167+
/*!
168+
* Save the file. If \a strip is specified, it is possible to choose if
169+
* tags not specified in \a tags should be stripped from the file or
170+
* retained. With \a version, it is possible to specify whether ID3v2.4
171+
* or ID3v2.3 should be used.
172+
*/
173+
bool save(TagTypes tags, StripTags strip = StripOthers,
174+
ID3v2::Version version = ID3v2::v4);
163175

164176
/*!
165177
* Returns whether or not the file on disk actually has an ID3v2 tag.

tests/test_aiff.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class TestAIFF : public CppUnit::TestFixture
4040
CPPUNIT_TEST(testAiffProperties);
4141
CPPUNIT_TEST(testAiffCProperties);
4242
CPPUNIT_TEST(testSaveID3v2);
43+
CPPUNIT_TEST(testSaveID3v23);
4344
CPPUNIT_TEST(testDuplicateID3v2);
4445
CPPUNIT_TEST(testFuzzedFile1);
4546
CPPUNIT_TEST(testFuzzedFile2);
@@ -105,6 +106,29 @@ class TestAIFF : public CppUnit::TestFixture
105106
}
106107
}
107108

109+
void testSaveID3v23()
110+
{
111+
ScopedFileCopy copy("empty", ".aiff");
112+
string newname = copy.fileName();
113+
114+
String xxx = ByteVector(254, 'X');
115+
{
116+
RIFF::AIFF::File f(newname.c_str());
117+
CPPUNIT_ASSERT_EQUAL(false, f.hasID3v2Tag());
118+
119+
f.tag()->setTitle(xxx);
120+
f.tag()->setArtist("Artist A");
121+
f.save(ID3v2::v3);
122+
CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
123+
}
124+
{
125+
RIFF::AIFF::File f2(newname.c_str());
126+
CPPUNIT_ASSERT_EQUAL((unsigned int)3, f2.tag()->header()->majorVersion());
127+
CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
128+
CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
129+
}
130+
}
131+
108132
void testDuplicateID3v2()
109133
{
110134
ScopedFileCopy copy("duplicate_id3v2", ".aiff");

tests/test_wav.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class TestWAV : public CppUnit::TestFixture
4444
CPPUNIT_TEST(testFloatProperties);
4545
CPPUNIT_TEST(testZeroSizeDataChunk);
4646
CPPUNIT_TEST(testID3v2Tag);
47+
CPPUNIT_TEST(testSaveID3v23);
4748
CPPUNIT_TEST(testInfoTag);
4849
CPPUNIT_TEST(testStripTags);
4950
CPPUNIT_TEST(testDuplicateTags);
@@ -139,6 +140,29 @@ class TestWAV : public CppUnit::TestFixture
139140
}
140141
}
141142

143+
void testSaveID3v23()
144+
{
145+
ScopedFileCopy copy("empty", ".wav");
146+
string newname = copy.fileName();
147+
148+
String xxx = ByteVector(254, 'X');
149+
{
150+
RIFF::WAV::File f(newname.c_str());
151+
CPPUNIT_ASSERT_EQUAL(false, f.hasID3v2Tag());
152+
153+
f.tag()->setTitle(xxx);
154+
f.tag()->setArtist("Artist A");
155+
f.save(RIFF::WAV::File::AllTags, File::StripOthers, ID3v2::v3);
156+
CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
157+
}
158+
{
159+
RIFF::WAV::File f2(newname.c_str());
160+
CPPUNIT_ASSERT_EQUAL((unsigned int)3, f2.ID3v2Tag()->header()->majorVersion());
161+
CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist());
162+
CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title());
163+
}
164+
}
165+
142166
void testInfoTag()
143167
{
144168
ScopedFileCopy copy("empty", ".wav");
@@ -191,7 +215,7 @@ class TestWAV : public CppUnit::TestFixture
191215
RIFF::WAV::File f(filename.c_str());
192216
CPPUNIT_ASSERT(f.hasID3v2Tag());
193217
CPPUNIT_ASSERT(f.hasInfoTag());
194-
f.save(RIFF::WAV::File::ID3v2, true);
218+
f.save(RIFF::WAV::File::ID3v2, File::StripOthers);
195219
}
196220
{
197221
RIFF::WAV::File f(filename.c_str());
@@ -205,7 +229,7 @@ class TestWAV : public CppUnit::TestFixture
205229
RIFF::WAV::File f(filename.c_str());
206230
CPPUNIT_ASSERT(f.hasID3v2Tag());
207231
CPPUNIT_ASSERT(f.hasInfoTag());
208-
f.save(RIFF::WAV::File::Info, true);
232+
f.save(RIFF::WAV::File::Info, File::StripOthers);
209233
}
210234
{
211235
RIFF::WAV::File f(filename.c_str());

0 commit comments

Comments
 (0)