From: Lukáš Lalinský Date: Tue, 15 Mar 2011 19:50:47 +0000 (+0100) Subject: Support for writing structuraly correct ID3v2.3 tags X-Git-Tag: v1.8beta~130^2~10 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1802237c75b7a188629bb5e24b75a8f1a0a0354f;p=taglib Support for writing structuraly correct ID3v2.3 tags We don't use the tag footer, extended header or unsynchronization, so we only need to change the frame size format. Note that this doesn't write correct ID3v2.3 tags, just tags that have the correct structure. The content is sill incorrect, unless you add the right frames manually to the tag instance. --- diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 786336b9..ef8e3eed 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -484,6 +484,11 @@ TagLib::uint Frame::Header::version() const return d->version; } +void Frame::Header::setVersion(TagLib::uint version) +{ + d->version = version; +} + bool Frame::Header::tagAlterPreservation() const { return d->tagAlterPreservation; @@ -538,7 +543,11 @@ ByteVector Frame::Header::render() const { ByteVector flags(2, char(0)); // just blank for the moment - ByteVector v = d->frameID + SynchData::fromUInt(d->frameSize) + flags; + ByteVector v = d->frameID + + (d->version == 3 + ? ByteVector::fromUInt(d->frameSize) + : SynchData::fromUInt(d->frameSize)) + + flags; return v; } diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h index 661be3d2..cd401c09 100644 --- a/taglib/mpeg/id3v2/id3v2frame.h +++ b/taglib/mpeg/id3v2/id3v2frame.h @@ -294,11 +294,17 @@ namespace TagLib { void setFrameSize(uint size); /*! - * Returns the ID3v2 version of the header (as passed in from the - * construction of the header). + * Returns the ID3v2 version of the header, as passed in from the + * construction of the header or set via setVersion(). */ uint version() const; + /*! + * Sets the ID3v2 version of the header, changing has impact on the + * correct parsing/rendering of frame data. + */ + void setVersion(uint version); + /*! * Returns the size of the frame header in bytes. * diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp index 31a5a1ec..e21292de 100644 --- a/taglib/mpeg/id3v2/id3v2header.cpp +++ b/taglib/mpeg/id3v2/id3v2header.cpp @@ -164,7 +164,7 @@ ByteVector Header::render() const // add the version number -- we always render a 2.4.0 tag regardless of what // the tag originally was. - v.append(char(4)); + v.append(char(majorVersion())); v.append(char(0)); // Currently we don't actually support writing extended headers, footers or diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 7e8012b2..c85ef59d 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -330,6 +330,11 @@ void ID3v2::Tag::removeFrames(const ByteVector &id) } ByteVector ID3v2::Tag::render() const +{ + return render(4); +} + +ByteVector ID3v2::Tag::render(int version) const { // We need to render the "tag data" first so that we have to correct size to // render in the tag's header. The "tag data" -- everything that is included @@ -343,6 +348,7 @@ ByteVector ID3v2::Tag::render() const // Loop through the frames rendering them and adding them to the tagData. for(FrameList::Iterator it = d->frameList.begin(); it != d->frameList.end(); it++) { + (*it)->header()->setVersion(version); if((*it)->header()->frameID().size() != 4) { debug("A frame of unsupported or unknown type \'" + String((*it)->header()->frameID()) + "\' has been discarded"); @@ -364,7 +370,8 @@ ByteVector ID3v2::Tag::render() const tagData.append(ByteVector(paddingSize, char(0))); - // Set the tag size. + // Set the version and data size. + d->header.setMajorVersion(version); d->header.setTagSize(tagData.size()); // TODO: This should eventually include d->footer->render(). diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h index a139d455..137139bd 100644 --- a/taglib/mpeg/id3v2/id3v2tag.h +++ b/taglib/mpeg/id3v2/id3v2tag.h @@ -265,6 +265,15 @@ namespace TagLib { */ ByteVector render() const; + /*! + * Render the tag back to binary data, suitable to be written to disk. + * + * The \a version parameter specifies the version of the rendered + * ID3v2 tag. It can be either 4 or 3. + */ + // BIC: combine with the above method + ByteVector render(int version) const; + protected: /*! * Reads data from the file specified in the constructor. It does basic diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 2fc1b380..3b3513ae 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -139,6 +139,11 @@ bool MPEG::File::save(int tags) } bool MPEG::File::save(int tags, bool stripOthers) +{ + return save(tags, stripOthers, 4); +} + +bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version) { if(tags == NoTags && stripOthers) return strip(AllTags); @@ -174,7 +179,7 @@ bool MPEG::File::save(int tags, bool stripOthers) if(!d->hasID3v2) d->ID3v2Location = 0; - insert(ID3v2Tag()->render(), d->ID3v2Location, d->ID3v2OriginalSize); + insert(ID3v2Tag()->render(id3v2Version), d->ID3v2Location, d->ID3v2OriginalSize); d->hasID3v2 = true; diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 282af775..7b41328a 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -161,6 +161,21 @@ namespace TagLib { // BIC: combine with the above method bool save(int tags, bool stripOthers); + /*! + * Save the file. This will attempt to save all of the tag types that are + * specified by OR-ing together TagTypes values. The save() method above + * uses AllTags. This returns true if saving was successful. + * + * If \a stripOthers is true this strips all tags not included in the mask, + * but does not modify them in memory, so later calls to save() which make + * use of these tags will remain valid. This also strips empty tags. + * + * The \a id3v2Version parameter specifies the version of the saved + * ID3v2 tag. It can be either 4 or 3. + */ + // BIC: combine with the above method + bool save(int tags, bool stripOthers, int id3v2Version); + /*! * Returns a pointer to the ID3v2 tag of the file. * diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index 6278ff55..6e85aed3 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -2,6 +2,8 @@ #include #include #include +#include +#include "utils.h" using namespace std; using namespace TagLib; @@ -10,6 +12,8 @@ class TestMPEG : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestMPEG); CPPUNIT_TEST(testVersion2DurationWithXingHeader); + CPPUNIT_TEST(testSaveID3v24); + CPPUNIT_TEST(testSaveID3v23); CPPUNIT_TEST_SUITE_END(); public: @@ -20,6 +24,38 @@ public: CPPUNIT_ASSERT_EQUAL(5387, f.audioProperties()->length()); } + void testSaveID3v24() + { + ScopedFileCopy copy("xing", ".mp3", false); + string newname = copy.fileName(); + + String xxx = ByteVector(254, 'X'); + MPEG::File f(newname.c_str()); + f.tag()->setTitle(xxx); + f.tag()->setArtist("Artist A"); + f.save(MPEG::File::AllTags, true, 4); + + MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); + CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); + } + + void testSaveID3v23() + { + ScopedFileCopy copy("xing", ".mp3", false); + string newname = copy.fileName(); + + String xxx = ByteVector(254, 'X'); + MPEG::File f(newname.c_str()); + f.tag()->setTitle(xxx); + f.tag()->setArtist("Artist A"); + f.save(MPEG::File::AllTags, true, 3); + + MPEG::File f2(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(String("Artist A"), f2.tag()->artist()); + CPPUNIT_ASSERT_EQUAL(xxx, f2.tag()->title()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);