mirror/taglib
1
0
Fork 0
mirror of https://github.com/taglib/taglib.git synced 2026-11-05 22:18:50 -05:00
Code Issues Packages Projects Releases Wiki Activity

ID3v2: Update an edited frame in place in setProperties (#1481)

Browse Source 
When exactly one frame holds an edited key and its type can hold the
new value, setProperties() now updates that frame instead of deleting
it and appending a new one. A COMM or USLT keeps its language, a TXXX
its description's spelling, and each keeps its position and its stored
text encoding, which is promoted at render when the new text needs it.
This commit is contained in:
Ryan Francesconi
2026-11-01 06:29:52 +01:00
committed by GitHub
parent 3bce7df062
commit 80e42828a8
2 changed files with 150 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
taglib/mpeg/id3v2/id3v2tag.cpp
+57 -2
View File
@@ -58,6 +58,46 @@ namespace
constexpr long MaxPaddingSize = 1024 * 1024;
constexpr unsigned int MAX_ID3V2_FRAME_COUNT = 50000;
/*!
* Sets \a values as the value of \a frame, the only frame holding \a key,
* if its type can hold them, keeping its other fields.
* \return false, leaving \a frame unchanged, if it cannot.
*/
bool updateFrameInPlace(ID3v2::Frame *frame, const String &key, const StringList &values)
{
if(auto f = dynamic_cast<ID3v2::UserTextIdentificationFrame *>(frame)) {
f->setText(values);
return true;
}
if(auto f = dynamic_cast<ID3v2::TextIdentificationFrame *>(frame)) {
if(ID3v2::Frame::keyToFrameID(key) != frame->frameID())
return false;
f->setText(values);
return true;
}
if(values.size() != 1)
return false;
if(auto f = dynamic_cast<ID3v2::CommentsFrame *>(frame)) {
f->setText(values.front());
return true;
}
if(auto f = dynamic_cast<ID3v2::UnsynchronizedLyricsFrame *>(frame)) {
f->setText(values.front());
return true;
}
if(auto f = dynamic_cast<ID3v2::UserUrlLinkFrame *>(frame)) {
f->setUrl(values.front());
return true;
}
if(auto f = dynamic_cast<ID3v2::UrlLinkFrame *>(frame)) {
if(ID3v2::Frame::keyToFrameID(key) != frame->frameID())
return false;
f->setUrl(values.front());
return true;
}
return false;
}
/*!
* Downgrade ID3v2.4 text \a encoding to value supported by ID3v2.3.
*/
@@ -437,9 +477,13 @@ PropertyMap ID3v2::Tag::setProperties(const PropertyMap &origProps)
// languages), whose values properties() merges in frame order. Keep all of
// them if the requested values are the merged ones.
PropertyMap spreadProperties;
Map<String, unsigned int> framesPerKey;
for(const auto &frame : std::as_const(frameList())) {
if(const ByteVector id = frame->frameID(); id != "TIPL" && id != "TMCL") {
if(PropertyMap frameProperties = frame->asProperties(); frameProperties.size() == 1)
PropertyMap frameProperties = frame->asProperties();
for(const auto &[key, values] : std::as_const(frameProperties))
framesPerKey[key] += 1;
if(frameProperties.size() == 1)
spreadProperties.merge(frameProperties);
}
}
@@ -467,8 +511,19 @@ PropertyMap ID3v2::Tag::setProperties(const PropertyMap &origProps)
}
else if(frameProperties.size() == 1 && keptKeys.contains(frameProperties.begin()->first))
continue;
else if(!singleFrameProperties.contains(frameProperties))
else if(!singleFrameProperties.contains(frameProperties)) {
// The only frame holding an edited key keeps its other fields if it can hold the new value.
if(frameProperties.size() == 1) {
const String &key = frameProperties.begin()->first;
if(framesPerKey[key] == 1 && singleFrameProperties.contains(key) &&
!singleFrameProperties[key].isEmpty() &&
updateFrameInPlace(frame, key, singleFrameProperties[key])) {
singleFrameProperties.erase(key);
continue;
}
}
framesToDelete.append(frame);
}
else
singleFrameProperties.erase(frameProperties);
}
tests/test_id3v2.cpp
+93
View File
@@ -131,6 +131,7 @@ class TestID3v2 : public CppUnit::TestFixture
CPPUNIT_TEST(testPropertiesMovement);
CPPUNIT_TEST(testPropertyGrouping);
CPPUNIT_TEST(testSetPropertiesKeepsSpreadFrames);
CPPUNIT_TEST(testSetPropertiesUpdatesFrameInPlace);
CPPUNIT_TEST(testDeleteFrame);
CPPUNIT_TEST(testSaveAndStripID3v1ShouldNotAddFrameFromID3v1ToId3v2);
CPPUNIT_TEST(testParseChapterFrame);
@@ -1487,6 +1488,98 @@ public:
}
}
void testSetPropertiesUpdatesFrameInPlace()
{
{
// An edited value goes into the frame that held it, which keeps its
// language, description, position and encoding.
ID3v2::Tag tag;
auto title = new ID3v2::TextIdentificationFrame("TIT2", String::Latin1);
title->setText("Title");
tag.addFrame(title);
auto comment = new ID3v2::CommentsFrame(String::Latin1);
comment->setLanguage("deu");
comment->setText("hallo");
tag.addFrame(comment);
auto lyrics = new ID3v2::UnsynchronizedLyricsFrame(String::Latin1);
lyrics->setLanguage("deu");
lyrics->setText("la la");
tag.addFrame(lyrics);
auto userText = new ID3v2::UserTextIdentificationFrame("MyTag", StringList("x"),
String::Latin1);
tag.addFrame(userText);
PropertyMap properties = tag.properties();
CPPUNIT_ASSERT(properties.contains("LYRICS"));
CPPUNIT_ASSERT(properties.contains("MYTAG"));
properties["TITLE"] = StringList("Title, edited");
properties["COMMENT"] = StringList("tschuess");
properties["LYRICS"] = StringList("la la la");
properties["MYTAG"] = StringList("y");
CPPUNIT_ASSERT(tag.setProperties(properties).isEmpty());
const ID3v2::FrameList frames = tag.frameList();
CPPUNIT_ASSERT_EQUAL(4u, frames.size());
CPPUNIT_ASSERT_EQUAL(static_cast<ID3v2::Frame *>(title), frames[0]);
CPPUNIT_ASSERT_EQUAL(static_cast<ID3v2::Frame *>(comment), frames[1]);
CPPUNIT_ASSERT_EQUAL(static_cast<ID3v2::Frame *>(lyrics), frames[2]);
CPPUNIT_ASSERT_EQUAL(static_cast<ID3v2::Frame *>(userText), frames[3]);
CPPUNIT_ASSERT_EQUAL(String("Title, edited"), title->toString());
CPPUNIT_ASSERT_EQUAL(String::Latin1, title->textEncoding());
CPPUNIT_ASSERT_EQUAL(ByteVector("deu"), comment->language());
CPPUNIT_ASSERT_EQUAL(String("tschuess"), comment->text());
CPPUNIT_ASSERT_EQUAL(ByteVector("deu"), lyrics->language());
CPPUNIT_ASSERT_EQUAL(String("la la la"), lyrics->text());
CPPUNIT_ASSERT_EQUAL(String("MyTag"), userText->description());
CPPUNIT_ASSERT_EQUAL(StringList("y"), tag.properties()["MYTAG"]);
}
{
// A value the frame cannot hold, and a removed key, are handled as before.
ID3v2::Tag tag;
auto comment = new ID3v2::CommentsFrame;
comment->setLanguage("deu");
comment->setText("hallo");
tag.addFrame(comment);
tag.addFrame(new ID3v2::UserTextIdentificationFrame("MyTag", StringList("x")));
PropertyMap properties;
properties["COMMENT"] = StringList("a").append("b");
tag.setProperties(properties);
CPPUNIT_ASSERT(tag.frameList("COMM").isEmpty());
CPPUNIT_ASSERT_EQUAL(1u, tag.frameList("TXXX").size());
CPPUNIT_ASSERT(ID3v2::UserTextIdentificationFrame::find(&tag, "COMMENT"));
CPPUNIT_ASSERT(!ID3v2::UserTextIdentificationFrame::find(&tag, "MyTag"));
}
// A Latin-1 frame given text Latin-1 cannot hold is promoted when rendered.
ScopedFileCopy copy("xing", ".mp3");
const String text("\xc5\x81\xc3\xb3" "d\xc5\xba \xe2\x9c\x93", String::UTF8);
{
MPEG::File f(copy.fileName().c_str());
auto comment = new ID3v2::CommentsFrame(String::Latin1);
comment->setLanguage("deu");
comment->setText("hallo");
f.ID3v2Tag(true)->addFrame(comment);
f.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3);
}
{
MPEG::File f(copy.fileName().c_str());
PropertyMap properties = f.properties();
properties["COMMENT"] = StringList(text);
f.setProperties(properties);
f.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3);
}
{
MPEG::File f(copy.fileName().c_str());
const ID3v2::FrameList comments = f.ID3v2Tag()->frameList("COMM");
CPPUNIT_ASSERT_EQUAL(1u, comments.size());
auto comment = dynamic_cast<ID3v2::CommentsFrame *>(comments.front());
CPPUNIT_ASSERT(comment);
CPPUNIT_ASSERT_EQUAL(ByteVector("deu"), comment->language());
CPPUNIT_ASSERT_EQUAL(text, comment->text());
}
}
void testDeleteFrame()
{
ScopedFileCopy copy("rare_frames", ".mp3");