Merge pull request #684 from TsudaKageyu/save-mpeg

Fix saving MPEG files.
This commit is contained in:
Tsuda Kageyu 2015-12-17 11:24:25 +09:00
commit 546870d83a
4 changed files with 222 additions and 185 deletions

View File

@ -24,6 +24,7 @@
***************************************************************************/
#include <tagunion.h>
#include <tagutils.h>
#include <id3v2tag.h>
#include <id3v2header.h>
#include <id3v1tag.h>
@ -64,20 +65,14 @@ namespace
class MPEG::File::FilePrivate
{
public:
FilePrivate(ID3v2::FrameFactory *frameFactory = ID3v2::FrameFactory::instance()) :
FilePrivate(const ID3v2::FrameFactory *frameFactory = ID3v2::FrameFactory::instance()) :
ID3v2FrameFactory(frameFactory),
ID3v2Location(-1),
ID3v2OriginalSize(0),
APELocation(-1),
APEFooterLocation(-1),
APEOriginalSize(0),
ID3v1Location(-1),
hasID3v2(false),
hasID3v1(false),
hasAPE(false),
properties(0)
{
}
properties(0) {}
~FilePrivate()
{
@ -97,13 +92,6 @@ public:
TagUnion tag;
// These indicate whether the file *on disk* has these tags, not if
// this data structure does. This is used in computing offsets.
bool hasID3v2;
bool hasID3v1;
bool hasAPE;
Properties *properties;
};
@ -194,17 +182,6 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version)
bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags)
{
if(tags == NoTags && stripOthers)
return strip(AllTags);
if(!ID3v2Tag() && !ID3v1Tag() && !APETag()) {
if((d->hasID3v1 || d->hasID3v2 || d->hasAPE) && stripOthers)
return strip(AllTags);
return true;
}
if(readOnly()) {
debug("MPEG::File::save() -- File is read only.");
return false;
@ -212,7 +189,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
// Create the tags if we've been asked to.
if (duplicateTags) {
if(duplicateTags) {
// Copy the values from the tag that does exist into the new tag,
// except if the existing tag is to be stripped.
@ -224,79 +201,93 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica
Tag::duplicate(ID3v2Tag(), ID3v1Tag(true), false);
}
bool success = true;
// Remove all the tags not going to be saved.
if(stripOthers)
strip(~tags, false);
if(ID3v2 & tags) {
if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) {
if(!d->hasID3v2)
// ID3v2 tag is not empty. Update the old one or create a new one.
if(d->ID3v2Location < 0)
d->ID3v2Location = 0;
insert(ID3v2Tag()->render(id3v2Version), d->ID3v2Location, d->ID3v2OriginalSize);
const ByteVector data = ID3v2Tag()->render(id3v2Version);
insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
d->hasID3v2 = true;
if(d->APELocation >= 0)
d->APELocation += (data.size() - d->ID3v2OriginalSize);
// v1 tag location has changed, update if it exists
if(d->ID3v1Location >= 0)
d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
if(ID3v1Tag())
d->ID3v1Location = findID3v1();
// APE tag location has changed, update if it exists
if(APETag())
findAPE();
d->ID3v2OriginalSize = data.size();
}
else {
// ID3v2 tag is empty. Remove the old one.
strip(ID3v2, false);
}
else if(stripOthers)
success = strip(ID3v2, false) && success;
}
else if(d->hasID3v2 && stripOthers)
success = strip(ID3v2) && success;
if(ID3v1 & tags) {
if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) {
int offset = d->hasID3v1 ? -128 : 0;
seek(offset, End);
writeBlock(ID3v1Tag()->render());
d->hasID3v1 = true;
d->ID3v1Location = findID3v1();
}
else if(stripOthers)
success = strip(ID3v1) && success;
}
else if(d->hasID3v1 && stripOthers)
success = strip(ID3v1, false) && success;
// Dont save an APE-tag unless one has been created
// ID3v1 tag is not empty. Update the old one or create a new one.
if((APE & tags) && APETag()) {
if(d->hasAPE)
insert(APETag()->render(), d->APELocation, d->APEOriginalSize);
else {
if(d->hasID3v1) {
insert(APETag()->render(), d->ID3v1Location, 0);
d->APEOriginalSize = APETag()->footer()->completeTagSize();
d->hasAPE = true;
d->APELocation = d->ID3v1Location;
d->ID3v1Location += d->APEOriginalSize;
if(d->ID3v1Location >= 0) {
seek(d->ID3v1Location);
}
else {
seek(0, End);
d->APELocation = tell();
APE::Tag *apeTag = d->tag.access<APE::Tag>(APEIndex, false);
d->APEFooterLocation = d->APELocation
+ apeTag->footer()->completeTagSize()
- APE::Footer::size();
writeBlock(APETag()->render());
d->APEOriginalSize = APETag()->footer()->completeTagSize();
d->hasAPE = true;
d->ID3v1Location = tell();
}
writeBlock(ID3v1Tag()->render());
}
else {
// ID3v1 tag is empty. Remove the old one.
strip(ID3v1, false);
}
}
else if(d->hasAPE && stripOthers)
success = strip(APE, false) && success;
return success;
if(APE & tags) {
if(APETag() && !APETag()->isEmpty()) {
// APE tag is not empty. Update the old one or create a new one.
if(d->APELocation < 0) {
if(d->ID3v1Location >= 0)
d->APELocation = d->ID3v1Location;
else
d->APELocation = length();
}
const ByteVector data = APETag()->render();
insert(data, d->APELocation, d->APEOriginalSize);
if(d->ID3v1Location >= 0)
d->ID3v1Location += (data.size() - d->APEOriginalSize);
d->APEOriginalSize = data.size();
}
else {
// APE tag is empty. Remove the old one.
strip(APE, false);
}
}
return true;
}
ID3v2::Tag *MPEG::File::ID3v2Tag(bool create)
@ -326,44 +317,39 @@ bool MPEG::File::strip(int tags, bool freeMemory)
return false;
}
if((tags & ID3v2) && d->hasID3v2) {
if((tags & ID3v2) && d->ID3v2Location >= 0) {
removeBlock(d->ID3v2Location, d->ID3v2OriginalSize);
if(d->APELocation >= 0)
d->APELocation -= d->ID3v2OriginalSize;
if(d->ID3v1Location >= 0)
d->ID3v1Location -= d->ID3v2OriginalSize;
d->ID3v2Location = -1;
d->ID3v2OriginalSize = 0;
d->hasID3v2 = false;
if(freeMemory)
d->tag.set(ID3v2Index, 0);
// v1 tag location has changed, update if it exists
if(ID3v1Tag())
d->ID3v1Location = findID3v1();
// APE tag location has changed, update if it exists
if(APETag())
findAPE();
}
if((tags & ID3v1) && d->hasID3v1) {
if((tags & ID3v1) && d->ID3v1Location >= 0) {
truncate(d->ID3v1Location);
d->ID3v1Location = -1;
d->hasID3v1 = false;
if(freeMemory)
d->tag.set(ID3v1Index, 0);
}
if((tags & APE) && d->hasAPE) {
if((tags & APE) && d->APELocation >= 0) {
removeBlock(d->APELocation, d->APEOriginalSize);
if(d->ID3v1Location >= 0)
d->ID3v1Location -= d->APEOriginalSize;
d->APELocation = -1;
d->APEFooterLocation = -1;
d->hasAPE = false;
if(d->hasID3v1) {
if(d->ID3v1Location > d->APELocation)
d->ID3v1Location -= d->APEOriginalSize;
}
d->APEOriginalSize = 0;
if(freeMemory)
d->tag.set(APEIndex, 0);
@ -457,17 +443,17 @@ long MPEG::File::lastFrameOffset()
bool MPEG::File::hasID3v1Tag() const
{
return d->hasID3v1;
return (d->ID3v1Location >= 0);
}
bool MPEG::File::hasID3v2Tag() const
{
return d->hasID3v2;
return (d->ID3v2Location >= 0);
}
bool MPEG::File::hasAPETag() const
{
return d->hasAPE;
return (d->APELocation >= 0);
}
////////////////////////////////////////////////////////////////////////////////
@ -481,35 +467,25 @@ void MPEG::File::read(bool readProperties)
d->ID3v2Location = findID3v2();
if(d->ID3v2Location >= 0) {
d->tag.set(ID3v2Index, new ID3v2::Tag(this, d->ID3v2Location, d->ID3v2FrameFactory));
d->ID3v2OriginalSize = ID3v2Tag()->header()->completeTagSize();
if(ID3v2Tag()->header()->tagSize() <= 0)
d->tag.set(ID3v2Index, 0);
else
d->hasID3v2 = true;
}
// Look for an ID3v1 tag
d->ID3v1Location = findID3v1();
d->ID3v1Location = Utils::findID3v1(this);
if(d->ID3v1Location >= 0) {
if(d->ID3v1Location >= 0)
d->tag.set(ID3v1Index, new ID3v1::Tag(this, d->ID3v1Location));
d->hasID3v1 = true;
}
// Look for an APE tag
findAPE();
d->APELocation = Utils::findAPE(this, d->ID3v1Location);
if(d->APELocation >= 0) {
d->tag.set(APEIndex, new APE::Tag(this, d->APEFooterLocation));
d->tag.set(APEIndex, new APE::Tag(this, d->APELocation));
d->APEOriginalSize = APETag()->footer()->completeTagSize();
d->hasAPE = true;
d->APELocation = d->APELocation + APE::Footer::size() - d->APEOriginalSize;
}
if(readProperties)
@ -556,36 +532,3 @@ long MPEG::File::findID3v2()
return tagOffset;
}
long MPEG::File::findID3v1()
{
if(isValid()) {
seek(-128, End);
long p = tell();
if(readBlock(3) == ID3v1::Tag::fileIdentifier())
return p;
}
return -1;
}
void MPEG::File::findAPE()
{
if(isValid()) {
seek(d->hasID3v1 ? -160 : -32, End);
long p = tell();
if(readBlock(8) == APE::Tag::fileIdentifier()) {
d->APEFooterLocation = p;
seek(d->APEFooterLocation);
APE::Footer footer(readBlock(APE::Footer::size()));
d->APELocation = d->APEFooterLocation - footer.completeTagSize()
+ APE::Footer::size();
return;
}
}
d->APELocation = -1;
d->APEFooterLocation = -1;
}

View File

@ -175,9 +175,6 @@ namespace TagLib {
* If you would like more granular control over the content of the tags,
* with the concession of generality, use parameterized save call below.
*
* \warning In the current implementation, it's dangerous to call save()
* repeatedly. At worst it will corrupt the file.
*
* \see save(int tags)
*/
virtual bool save();
@ -190,9 +187,6 @@ namespace TagLib {
* 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.
*
* \warning In the current implementation, it's dangerous to call save()
* repeatedly. At worst it will corrupt the file.
*/
bool save(int tags);
@ -204,9 +198,6 @@ namespace TagLib {
* 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.
*
* \warning In the current implementation, it's dangerous to call save()
* repeatedly. At worst it will corrupt the file.
*/
// BIC: combine with the above method
bool save(int tags, bool stripOthers);
@ -222,9 +213,6 @@ namespace TagLib {
*
* The \a id3v2Version parameter specifies the version of the saved
* ID3v2 tag. It can be either 4 or 3.
*
* \warning In the current implementation, it's dangerous to call save()
* repeatedly. At worst it will corrupt the file.
*/
// BIC: combine with the above method
bool save(int tags, bool stripOthers, int id3v2Version);
@ -243,9 +231,6 @@ namespace TagLib {
*
* If \a duplicateTags is true and at least one tag -- ID3v1 or ID3v2 --
* exists this will duplicate its content into the other tag.
*
* \warning In the current implementation, it's dangerous to call save()
* repeatedly. At worst it will corrupt the file.
*/
// BIC: combine with the above method
bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags);
@ -391,8 +376,6 @@ namespace TagLib {
void read(bool readProperties);
long findID3v2();
long findID3v1();
void findAPE();
class FilePrivate;
FilePrivate *d;

View File

@ -1062,29 +1062,20 @@ public:
{
MPEG::File f(newname.c_str());
ID3v2::Tag *tag = f.ID3v2Tag(true);
ID3v2::TextIdentificationFrame *frame1 = new ID3v2::TextIdentificationFrame("TIT2");
frame1->setText("Title");
tag->addFrame(frame1);
ID3v2::AttachedPictureFrame *frame2 = new ID3v2::AttachedPictureFrame();
frame2->setPicture(ByteVector(100 * 1024, '\xff'));
tag->addFrame(frame2);
f.save();
CPPUNIT_ASSERT(f.length() > 100 * 1024);
f.ID3v2Tag()->setTitle(std::wstring(64 * 1024, L'X').c_str());
f.save(MPEG::File::ID3v2, true);
}
{
MPEG::File f(newname.c_str());
CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag());
ID3v2::Tag *tag = f.ID3v2Tag();
tag->removeFrames("APIC");
f.save();
CPPUNIT_ASSERT(f.length() < 10 * 1024);
CPPUNIT_ASSERT(f.hasID3v2Tag());
CPPUNIT_ASSERT_EQUAL(74789L, f.length());
f.ID3v2Tag()->setTitle("ABCDEFGHIJ");
f.save(MPEG::File::ID3v2, true);
}
{
MPEG::File f(newname.c_str());
CPPUNIT_ASSERT(f.hasID3v2Tag());
CPPUNIT_ASSERT_EQUAL(9263L, f.length());
}
}

View File

@ -31,6 +31,12 @@ class TestMPEG : public CppUnit::TestFixture
CPPUNIT_TEST(testFuzzedFile);
CPPUNIT_TEST(testFrameOffset);
CPPUNIT_TEST(testStripAndProperties);
CPPUNIT_TEST(testRepeatedSave1);
CPPUNIT_TEST(testRepeatedSave2);
CPPUNIT_TEST(testRepeatedSave3);
CPPUNIT_TEST(testEmptyID3v2);
CPPUNIT_TEST(testEmptyID3v1);
CPPUNIT_TEST(testEmptyAPE);
CPPUNIT_TEST_SUITE_END();
public:
@ -253,6 +259,120 @@ public:
}
}
void testRepeatedSave1()
{
ScopedFileCopy copy("xing", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
f.ID3v2Tag(true)->setTitle(std::string(4096, 'X').c_str());
f.save();
}
{
MPEG::File f(copy.fileName().c_str());
f.ID3v2Tag(true)->setTitle("");
f.save();
f.ID3v2Tag(true)->setTitle(std::string(4096, 'X').c_str());
f.save();
CPPUNIT_ASSERT_EQUAL(5141L, f.firstFrameOffset());
}
}
void testRepeatedSave2()
{
ScopedFileCopy copy("xing", ".mp3");
MPEG::File f(copy.fileName().c_str());
f.ID3v2Tag(true)->setTitle("0123456789");
f.save();
f.save();
CPPUNIT_ASSERT_EQUAL(-1L, f.find("ID3", 3));
}
void testRepeatedSave3()
{
ScopedFileCopy copy("xing", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(!f.hasAPETag());
CPPUNIT_ASSERT(!f.hasID3v1Tag());
f.APETag(true)->setTitle("01234 56789 ABCDE FGHIJ");
f.save();
f.APETag()->setTitle("0");
f.save();
f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ");
f.APETag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789");
f.save();
}
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(f.hasAPETag());
CPPUNIT_ASSERT(f.hasID3v1Tag());
}
}
void testEmptyID3v2()
{
ScopedFileCopy copy("xing", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
f.ID3v2Tag(true)->setTitle("0123456789");
f.save(MPEG::File::ID3v2);
}
{
MPEG::File f(copy.fileName().c_str());
f.ID3v2Tag(true)->setTitle("");
f.save(MPEG::File::ID3v2, false);
}
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(!f.hasID3v2Tag());
}
}
void testEmptyID3v1()
{
ScopedFileCopy copy("xing", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
f.ID3v1Tag(true)->setTitle("0123456789");
f.save(MPEG::File::ID3v1);
}
{
MPEG::File f(copy.fileName().c_str());
f.ID3v1Tag(true)->setTitle("");
f.save(MPEG::File::ID3v1, false);
}
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(!f.hasID3v1Tag());
}
}
void testEmptyAPE()
{
ScopedFileCopy copy("xing", ".mp3");
{
MPEG::File f(copy.fileName().c_str());
f.APETag(true)->setTitle("0123456789");
f.save(MPEG::File::APE);
}
{
MPEG::File f(copy.fileName().c_str());
f.APETag(true)->setTitle("");
f.save(MPEG::File::APE, false);
}
{
MPEG::File f(copy.fileName().c_str());
CPPUNIT_ASSERT(!f.hasAPETag());
}
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);