Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue]: Invalid MusicBrainz track ID values in song/track metadata #11020

Closed
1 task done
lyarenei opened this issue Feb 16, 2024 · 8 comments · Fixed by #11301
Closed
1 task done

[Issue]: Invalid MusicBrainz track ID values in song/track metadata #11020

lyarenei opened this issue Feb 16, 2024 · 8 comments · Fixed by #11301
Labels
bug Something isn't working regression Regression from previous build upstream The issue is due to a upstream library or project

Comments

@lyarenei
Copy link
Contributor

lyarenei commented Feb 16, 2024

Please describe your bug

During file scan, Jellyfin incorrectly sets a MusicBrainz recording ID to a track ID field in song/track metadata (these are not the same, the hierarchy is explained here).

In Jellyfin 10.8.x, the value is set correctly, as that one uses older method of getting the metadata, which was then refactored for 10.9.x in #7514

After looking into it, it seems that TagLib incorrectly reads the values from the file and then returns a MusicBrainz recording ID value as a track ID. This can be seen when debugging AudioFileProber, specifically looking into the tags variable just after TagLib reads the metadata:

var file = TagLib.File.Create(audio.Path);
var tagTypes = file.TagTypesOnDisk;
Tag? tags = null;
if (tagTypes.HasFlag(TagTypes.Id3v2))
{
tags = file.GetTag(TagTypes.Id3v2);
}
else if (tagTypes.HasFlag(TagTypes.Ape))
{
tags = file.GetTag(TagTypes.Ape);
}
else if (tagTypes.HasFlag(TagTypes.FlacMetadata))
{
tags = file.GetTag(TagTypes.FlacMetadata);
}
else if (tagTypes.HasFlag(TagTypes.Apple))
{
tags = file.GetTag(TagTypes.Apple);
}
else if (tagTypes.HasFlag(TagTypes.Xiph))
{
tags = file.GetTag(TagTypes.Xiph);
}
else if (tagTypes.HasFlag(TagTypes.AudibleMetadata))
{
tags = file.GetTag(TagTypes.AudibleMetadata);
}
else if (tagTypes.HasFlag(TagTypes.Id3v1))
{
tags = file.GetTag(TagTypes.Id3v1);
}
if (tags is not null)

And there's an issue report in the taglib-sharp repository as well - which looks like has a pull request with a fix, but as the repository does not seem very active, I guess there's not much hope getting the fix merged.

Reproduction Steps

  1. Have an audio file with MusicBrainz ID metadata
  2. Add file to a Jellyfin Music library
  3. Scan for metadata
  4. Compare MusicBrainz IDs in Jellyfin with the ones written in the file
  5. Observe invalid track MBID value (matching recording MBID) - see attached screenshot

Jellyfin Version

Unstable (master branch)

if other:

commit 03ef9ae

Environment

- OS:Linux
- Linux Kernel: 6.7.4-zen1-1-zen
- Virtualization: None
- Clients: Browser
- Browser: Firefox 115.7.0esr
- FFmpeg Version: n6.1.1
- Playback Method: Direct
- Hardware Acceleration: No
- GPU Model: Radeon RX 6700 XT
- Plugins: Built-in + ListenBrainz
- Reverse Proxy: No
- Base URL: None
- Networking: Host
- Storage: Local

Jellyfin logs

There's not much to show, apart from scanning the file, which does not produce any errors.

[11:20:49] [DBG] [23] MediaBrowser.Providers.Music.AudioMetadataService: Running ProbeProvider for /mnt/data/stash/moozik/Binärpilot/Promo/01 Thunderdays.mp3
[11:20:49] [INF] [23] MediaBrowser.MediaEncoding.Encoder.MediaEncoder: Starting ffprobe with args -analyzeduration 200M -probesize 1G -i file:"/mnt/data/stash/moozik/Binärpilot/Promo/01 Thunderdays.mp3" -threads 0 -v warning -print_format json -show_streams -show_format
[11:20:50] [DBG] [23] MediaBrowser.Providers.MediaInfo.AudioFileProber: LUFS for Thunderdays is -9.3.

FFmpeg logs

Issue is not related to playback.

Please attach any browser or client logs here

Issue is not related to UI/UX or playback.

Please attach any screenshots here

Side-by-side view of values in Jellyfin in comparison to what's written in the file metadata:
image

Code of Conduct

  • I agree to follow this project's Code of Conduct

Edit: formatting

@lyarenei lyarenei added the bug Something isn't working label Feb 16, 2024
@felix920506 felix920506 added the upstream The issue is due to a upstream library or project label Feb 16, 2024
@lyarenei
Copy link
Contributor Author

@felix920506 Hi, is there anything I can do to (help) fixing this issue for the upcoming release? Or is it too late?

I wouldn't mind implementing the eventual fix, but it'd be nice to have some conversation going first and agree on a solution before doing that.

@felix920506
Copy link
Member

felix920506 commented Mar 27, 2024

Your issue report suggests that the issue is in taglib. If that is the case, there is nothing we can do about it other than fix taglib.

@lyarenei
Copy link
Contributor Author

I am aware. But as the taglib did not have a release for almost two years and the fix is not merged as well, I wouldn't place any hopes on fixing that. Unless the development is happening somewhere else, in which case updating the package could be enough.

If not, then one solution would be to have it forked and then maintain it - in similar fashion to ffmpeg. But I don't think this is something the Jellyfin team is interested in, so I thought it'd be good to discuss other options.

I think the best solution for now would be to add the ffprobe reading back and then use taglib as the primary source for metadata and ffprobe as secondary - in this specific case the track MBID value from ffprobe would be used for track MBID metadata instead.

What do you think?

@Shadowghost
Copy link
Contributor

The main issues we have with tags right now is that a) ffprobe is unreliable for some types of tags b) apparently taglib is not maintained and is also unreliable
What we could do is use the raw fields exposed by taglib to extract the info ourselves and map it on a per-tag-type basis but that will be a rather big change which will require quite some additional testing, so I don't think this would be doable for 10.9

@Bond-009 Bond-009 added the regression Regression from previous build label Mar 28, 2024
@lyarenei
Copy link
Contributor Author

lyarenei commented Apr 1, 2024

a) ffprobe is unreliable for some types of tags

Ah, I see. That's unfortunate.

What we could do is use the raw fields exposed by taglib to extract the info ourselves and map it on a per-tag-type basis but that will be a rather big change which will require quite some additional testing, so I don't think this would be doable for 10.9

Interesting, I did not know that would be possible. But I agree this sounds like quite a bit of work.

I also noticed Lidarr uses taglib as well, but they seem to have their own fork. Unfortunately the version they use in the current code is not publicly available, so I was not able to test that.

Wouldn't it be better to at least not set the ID at all if it's known it's not valid?

@Shadowghost
Copy link
Contributor

Shadowghost commented Apr 3, 2024

Might be the best to not set the ID by tags then and fallback to ffprobe.

@lyarenei
Copy link
Contributor Author

lyarenei commented Apr 4, 2024

Might be the best to not set the ID by tags then and fallback to ffprobe.

This is more or less what I proposed earlier. I assumed the problems with ffprobe were possibly related to the way the tags were written, or is that not the case?

@Shadowghost
Copy link
Contributor

Afair issue was that it didn't work properly for all the supported tag formats

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression Regression from previous build upstream The issue is due to a upstream library or project
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants