Skip to content

Conversation

@Serial-ATA
Copy link
Owner

Tag::save_to() and Into::<Id3v2Tag>::into(tag) would take different paths, causing frames to not get merged (and possibly other weird behavior). Now, both take the same path.

All frames have been updated to use Cow, to avoid allocations in the Tag::save_to() path (unless absolutely necessary, like for track numbers).

This should be easier to maintain now that everything's unified. And possibly more efficient, since it's no longer cloning everything.

closes #349

@Serial-ATA Serial-ATA added this to the 0.23.0 milestone Nov 13, 2025
@Serial-ATA Serial-ATA marked this pull request as draft November 13, 2025 04:38
`Tag::save_to()` and `Into::<Id3v2Tag>::into(tag)` would take different paths, causing frames to not get merged (and possibly other weird behavior). Now, both take the same path.

All frames have been updated to use `Cow`, to avoid allocations in the `Tag::save_to()` path (unless absolutely necessary, like for track numbers).

This should be easier to maintain now that everything's unified. And possibly more efficient, since it's no longer cloning everything.
@Serial-ATA Serial-ATA marked this pull request as ready for review November 13, 2025 15:30
@Serial-ATA Serial-ATA merged commit 10be465 into main Nov 13, 2025
4 checks passed
@Serial-ATA Serial-ATA deleted the id3v2-conversions branch November 13, 2025 17:37
@uklotzde
Copy link
Contributor

After writing ItemKey::IntegerBpm as ID3v2 into an empty file no tags are written and the file remains empty.

let mut file = copy_named_temp_file("tests/assets/empty.mp3");

  {
      let tagged_file = Probe::new(&mut file.as_file())
          .guess_file_type()
          .unwrap()
          .read()
          .unwrap();
      assert_eq!(FileType::Mpeg, tagged_file.file_type());
      assert!(tagged_file.tags().is_empty());
  }

  let mut tag = Tag::new(TagType::Id3v2);
  tag.insert_text(
      ItemKey::IntegerBpm,
     122.to_string(),
  );
  tag.save_to_path(file.path(), WriteOptions::default())
      .unwrap();

@uklotzde
Copy link
Contributor

@Serial-ATA
Copy link
Owner Author

@uklotzde Good catch, fixed in #579.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can not write multiple artists with mp3 and generic Tag

3 participants