Open goulvench opened 4 months ago
I found a workaround!
I realized that this happens for the second file only, because the AttachedPictureFrame
is memoized —even if I call dup
on it.
So I need to build a whole new AttachedPictureFrame
for each file, even if the image is exactly the same. Maybe this is a limitation of the underlying C++
library, I don't know.
I think this might be the problem: https://github.com/robinst/taglib-ruby/blob/189fa6b1147025bc907a1119c1e8df57f70df822/ext/taglib_mpeg/taglib_mpeg.i#L60-L71
When the TagLib::MPEG::File.open
block is done, Ruby may garbage-collect the TagLib::MPEG::File
instance used by the block. That instance has a cleanup function which iterates through all the ID3v2 frames attached to the tag of the file and frees their underlying C++ objects.
@robinst looking at the SWIG documentation I wonder if we should move away from this pattern of using %freefunc
and use %markfunc
instead.
If we use %markfunc
correctly then we can make individual wrapper objects (like TagLib::ID3v2::Frame
) responsible for freeing C++ objects, rather than iterating over collections like we do now.
Ruby may garbage-collect
In fact, closing the file forces freeing of the objects: https://github.com/robinst/taglib-ruby/blob/189fa6b1147025bc907a1119c1e8df57f70df822/ext/taglib_mpeg/taglib_mpeg.i#L87-L89
Never mind, I don't think markfunc
is applicable here.
@goulvench it appears it is indeed because of the C++ library that you cannot reuse a frame. This program crashes with a segfault:
// testowner.cpp
// To compile: LDLIBS='-ltag -lz' make testowner
//
#include <iostream>
#include <cstdlib>
#include <taglib/taglib.h>
#include <taglib/mpegfile.h>
#include <taglib/id3v2tag.h>
#include <taglib/textidentificationframe.h>
using namespace TagLib;
int main(int argc, char **argv) {
if (argc != 2) {
std::cerr << "usage: " << argv[0] << " file.mp3" << std::endl;
return EXIT_FAILURE;
}
char *filename = argv[1];
ID3v2::TextIdentificationFrame *frame2 = new ID3v2::TextIdentificationFrame("TIT2");
frame2->setText("Title");
if(1){ // <- change to 0 and we no longer crash
MPEG::File file(filename);
ID3v2::Tag *tag = file.ID3v2Tag();
tag->addFrame(frame2);
}
std::cout << frame2->toString() <<std::endl;
}
Spot on, see the docs for addFrame
: https://taglib.org/api/classTagLib_1_1ID3v2_1_1Tag.html#ae090718d9acff1341530aa6438759b40
Add a frame to the tag. At this point the tag takes ownership of the frame and will handle freeing its memory.
Hi! When running code straight from the readme, I get the following error:
Any idea why this happens, or what I should change in my code for it to work?
Versions
3.1.4
on an Intel Mac (same error with3.2.0
).1.1.3
.brew info taglib
givestaglib: stable 1.13.1 (bottled), HEAD