Type mismatch when adding an AttachedPictureFrame

[goulvench]

Hi! When running code straight from the readme, I get the following error:

# `add_frame': Expected argument 1 of type TagLib::ID3v2::Frame *, 
#  but got TagLib::ID3v2::AttachedPictureFrame #<TagLib::ID3v2::AttachedPictu... (ObjectPreviouslyDeleted)
#    in SWIG method 'addFrame'

# Here's the code I'm using:
apic = TagLib::ID3v2::AttachedPictureFrame.new
apic.type = TagLib::ID3v2::AttachedPictureFrame::FrontCover
apic.picture = File.open(image_path, "rb") { |f| f.read }
apic.mime_type = "image/jpeg"
apic.description = "Cover"

TagLib::MPEG::File.open("file.mp3") do |file|
  tag = file.id3v2_tag
  tag.artist = metadata[:artist]
  tag.title = metadata[:album]
  tag.year = metadata[:year].to_i
  tag.track = index
  tag.add_frame(apic)
  file.save
end

Any idea why this happens, or what I should change in my code for it to work?

Versions

• ruby 3.1.4 on an Intel Mac (same error with 3.2.0).
• gem version 1.1.3.
• brew info taglib gives taglib: stable 1.13.1 (bottled), HEAD

[goulvench]

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.

[jacobvosmaer]

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.

[jacobvosmaer]

@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.

[jacobvosmaer]

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

[jacobvosmaer]

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;
}

[robinst]

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.