Skip to content

Commit

Permalink
Fix extensibility of ID3v2 FrameFactory
Browse files Browse the repository at this point in the history
Because the main extension point of FrameFactory was using a protected
Frame subclass, it was not really possible to implement a custom frame
factory. Existing Frame subclasses also show that access to the frame
header might be needed when implementing a Frame subclass.
  • Loading branch information
ufleisch committed Nov 18, 2023
1 parent 9679b08 commit 2756a89
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 16 deletions.
10 changes: 5 additions & 5 deletions taglib/mpeg/id3v2/id3v2frame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ ByteVector Frame::render() const
return headerData + fieldData;
}

Frame::Header *Frame::header() const
{
return d->header;
}

////////////////////////////////////////////////////////////////////////////////
// protected members
////////////////////////////////////////////////////////////////////////////////
Expand All @@ -197,11 +202,6 @@ Frame::Frame(Header *h) :
d->header = h;
}

Frame::Header *Frame::header() const
{
return d->header;
}

void Frame::setHeader(Header *h, bool deleteCurrent)
{
if(deleteCurrent)
Expand Down
16 changes: 6 additions & 10 deletions taglib/mpeg/id3v2/id3v2frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,9 @@ namespace TagLib {
class TAGLIB_EXPORT Frame
{
friend class Tag;
friend class FrameFactory;
friend class TableOfContentsFrame;
friend class ChapterFrame;

public:
class Header;

/*!
* Creates a textual frame which corresponds to a single key in the PropertyMap
Expand Down Expand Up @@ -122,6 +120,11 @@ namespace TagLib {
*/
ByteVector render() const;

/*!
* Returns a pointer to the frame header.
*/
Header *header() const;

/*!
* Returns the text delimiter that is used between fields for the string
* type \a t.
Expand Down Expand Up @@ -151,8 +154,6 @@ namespace TagLib {
static const String urlPrefix;

protected:
class Header;

/*!
* Constructs an ID3v2 frame using \a data to read the header information.
* All other processing of \a data should be handled in a subclass.
Expand All @@ -170,11 +171,6 @@ namespace TagLib {
*/
Frame(Header *h);

/*!
* Returns a pointer to the frame header.
*/
Header *header() const;

/*!
* Sets the header to \a h. If \a deleteCurrent is true, this will free
* the memory of the current header.
Expand Down
5 changes: 5 additions & 0 deletions taglib/mpeg/id3v2/id3v2framefactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@ void FrameFactory::setDefaultTextEncoding(String::Type encoding)
d->defaultEncoding = encoding;
}

bool FrameFactory::isUsingDefaultTextEncoding() const
{
return d->useDefaultEncoding;
}

////////////////////////////////////////////////////////////////////////////////
// protected members
////////////////////////////////////////////////////////////////////////////////
Expand Down
15 changes: 14 additions & 1 deletion taglib/mpeg/id3v2/id3v2framefactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ namespace TagLib {
* factory to be the default factory in ID3v2::Tag constructor you can
* implement behavior that will allow for new ID3v2::Frame subclasses (also
* provided by you) to be used.
* See \c testFrameFactory() in \e tests/test_mpeg.cpp for an example.
*
* This implements both <i>abstract factory</i> and <i>singleton</i> patterns
* of which more information is available on the web and in software design
* textbooks (Notably <i>Design Patters</i>).
* textbooks (Notably <i>Design Patterns</i>).
*
* \note You do not need to use this factory to create new frames to add to
* an ID3v2::Tag. You can instantiate frame subclasses directly (with new)
Expand Down Expand Up @@ -105,6 +106,18 @@ namespace TagLib {
*/
void setDefaultTextEncoding(String::Type encoding);

/*!
* Returns true if defaultTextEncoding() is used.
* The default text encoding is used when setDefaultTextEncoding() has
* been called. In this case, reimplementations of FrameFactory should
* use defaultTextEncoding() on the frames (having a text encoding field)
* they create.
*
* \see defaultTextEncoding()
* \see setDefaultTextEncoding()
*/
bool isUsingDefaultTextEncoding() const;

protected:
/*!
* Constructs a frame factory. Because this is a singleton this method is
Expand Down
93 changes: 93 additions & 0 deletions tests/test_mpeg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@
#include <cstdio>
#include <array>

#include "tbytevector.h"
#include "tstring.h"
#include "tpropertymap.h"
#include "mpegfile.h"
#include "id3v2tag.h"
#include "id3v1tag.h"
#include "id3v2frame.h"
#include "id3v2framefactory.h"
#include "apetag.h"
#include "mpegproperties.h"
#include "xingheader.h"
Expand Down Expand Up @@ -71,6 +74,7 @@ class TestMPEG : public CppUnit::TestFixture
CPPUNIT_TEST(testIgnoreGarbage);
CPPUNIT_TEST(testExtendedHeader);
CPPUNIT_TEST(testReadStyleFast);
CPPUNIT_TEST(testFrameFactory);
CPPUNIT_TEST_SUITE_END();

public:
Expand Down Expand Up @@ -620,6 +624,95 @@ class TestMPEG : public CppUnit::TestFixture
}
}

void testFrameFactory()
{
class CustomFrameFactory;

// Just a silly example of a custom frame holding a number.
class CustomFrame : public ID3v2::Frame
{
friend class CustomFrameFactory;
public:
explicit CustomFrame(unsigned int value = 0)
: Frame("CUST"), m_value(value) {}
CustomFrame(const CustomFrame &) = delete;
CustomFrame &operator=(const CustomFrame &) = delete;
~CustomFrame() override = default;
String toString() const override { return String::number(m_value); }
PropertyMap asProperties() const override {
return SimplePropertyMap{{"CUSTOM", StringList(String::number(m_value))}};
}
unsigned int value() const { return m_value; }

protected:
void parseFields(const ByteVector &data) override {
m_value = data.toUInt();
}
ByteVector renderFields() const override {
return ByteVector::fromUInt(m_value);
}

private:
CustomFrame(const ByteVector &data, Header *h) : Frame(h) {
parseFields(fieldData(data));
}
unsigned int m_value;
};

// Example for frame factory with support for CustomFrame.
class CustomFrameFactory : public ID3v2::FrameFactory {
protected:
ID3v2::Frame *createFrame(const ByteVector &data, ID3v2::Frame::Header *header,
const ID3v2::Header *tagHeader) const override {
if(header->frameID() == "CUST") {
return new CustomFrame(data, header);
}
return ID3v2::FrameFactory::createFrame(data, header, tagHeader);
}
};

CustomFrameFactory factory;

ScopedFileCopy copy("lame_cbr", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(f.isValid());
CPPUNIT_ASSERT(f.hasID3v2Tag());
ID3v2::Tag *tag = f.ID3v2Tag();
tag->addFrame(new CustomFrame(1234567890));
f.save();
}
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(f.isValid());
CPPUNIT_ASSERT(f.hasID3v2Tag());
ID3v2::Tag *tag = f.ID3v2Tag();
const auto &frames = tag->frameList("CUST");
CPPUNIT_ASSERT(!frames.isEmpty());
// Without a specialized FrameFactory, you can add custom frames,
// but your cannot parse them.
CPPUNIT_ASSERT(!dynamic_cast<CustomFrame *>(frames.front()));
}
{
MPEG::File f(copy.fileName().c_str(), &factory);
CPPUNIT_ASSERT(f.isValid());
CPPUNIT_ASSERT(f.hasID3v2Tag());
ID3v2::Tag *tag = f.ID3v2Tag();
const auto &frames = tag->frameList("CUST");
CPPUNIT_ASSERT(!frames.isEmpty());
auto frame = dynamic_cast<CustomFrame *>(frames.front());
CPPUNIT_ASSERT(frame);
CPPUNIT_ASSERT_EQUAL(1234567890U, frame->value());
PropertyMap properties = tag->properties();
CPPUNIT_ASSERT_EQUAL(StringList("1234567890"),
properties.value("CUSTOM"));
CPPUNIT_ASSERT_EQUAL(StringList("-1.020000 dB"),
properties.value("REPLAYGAIN_TRACK_GAIN"));
CPPUNIT_ASSERT_EQUAL(StringList("0.920032"),
properties.value("REPLAYGAIN_TRACK_PEAK"));
}
}

};

CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);

0 comments on commit 2756a89

Please sign in to comment.