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

MediaMetadata.Builder().populateFromMetadata() miss some fields from .flac files #1958

Closed
1 task
cyanChill opened this issue Dec 7, 2024 · 2 comments
Closed
1 task
Assignees
Labels

Comments

@cyanChill
Copy link

cyanChill commented Dec 7, 2024

Version

Media3 1.5.0

More version details

I've also encountered the issue when using version 1.4.1.

Devices that reproduce the issue

  • OnePlus 6 running Android 11
  • Nothing Phone 2a running Android 14

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

I went through the route of using MetadataRetriever to get the metadata for audio files based on https://developer.android.com/media/media3/exoplayer/retrieving-metadata#wo-playback.

  • Basically: MetadataRetriever → TrackGroupArray → TrackGroup → Format → Metadata → MediaMetadata.

A basic summary of what I did is (assume everything is defined):

val mediaItem = MediaItem.fromUri(uri)
val trackGroupArray = MetadataRetriever.retrieveMetadata(context, mediaItem).get()
val format = trackGroupArray[0].getFormat(0)
val metadata = format.metadata
val mediaMetadata = MediaMetadata.Builder().populateFromMetadata(metadata).build()

Then I return the fields available such as artist, title, trackNumber, etc..

Expected result

Accessing the title, artist, discNumber, and trackNumber fields on the MediaMetadata object should return the respective values.

Actual result

The title & artist fields both return the expected strings while discNumber & trackNumber returns null. This only happens when following the process for .flac files.

I do know that the vorbis comments for the discNumber & trackNumber fields are present as running metadata.toString() returns:

entries=[
  VC: TITLE=One Step Closer,
  VC: TOTALDISCS=1,
  VC: TOTALTRACKS=12,
  VC: ARTIST=Linkin Park,
  VC: DISCNUMBER=1,
  VC: ALBUM=Hybrid Theory,
  VC: GENRE=Alternative,
  VC: GENRE=Metal,
  VC: GENRE=Nu Metal,
  VC: COMMENT=CD, Warner Records 093624893226,
  VC: ALBUMARTIST=Linkin Park,
  VC: DATE=2000,
  VC: TRACKNUMBER=02,
  Picture: mimeType=image/jpeg, description=
]

I'm currently working around this problem by using MediaMetadataRetriever for .flac files instead since it correctly returns the disc & track numbers.

Media

I'll send the file to the specified email, but I think it affects all .flac files.

Bug Report

  • You will email the zip file produced by adb bugreport to [email protected] after filing this issue.
@icbaker
Copy link
Collaborator

icbaker commented Dec 9, 2024

The conversion logic from vorbis comments to MediaMetadata is here:

public void populateMediaMetadata(MediaMetadata.Builder builder) {
switch (key) {
case "TITLE":
builder.setTitle(value);
break;
case "ARTIST":
builder.setArtist(value);
break;
case "ALBUM":
builder.setAlbumTitle(value);
break;
case "ALBUMARTIST":
builder.setAlbumArtist(value);
break;
case "DESCRIPTION":
builder.setDescription(value);
break;
default:
break;
}
}

Some of the tags mentioned at https://xiph.org/vorbis/doc/v-comment.html are missing from our logic, including TRACKNUMBER and GENRE - adding those is relatively uncontroversial.

The rest of the 'unrecognized' tags are less clear. It seems there's a lot of possible variety out in the wild, both in terms of the key names and the format of their values. Some examples of disc numbering from this 2009 comment:

  • DISC=n
  • DISCNUMBER=n
  • DISCNUMBER=n/x
  • DISC #=n
  • DISCC=x
  • DISCTOTAL=x
  • TOTALDISCS=x

Since TRACKNUMBER is one of the 'official' ones, and its description doesn't suggest the x/y formatting, it seems reasonable to me to derive that a TOTALTRACKS or similar key may also exist, and to follow the same convention for disc numbering. So I'll send a change adding support for:

TOTALDISCS
DISCNUMBER
TOTALTRACKS
TRACKNUMBER

If we get additional feedback with examples of the x/y numbering format, or different key names, we can add support for that too.

copybara-service bot pushed a commit that referenced this issue Dec 9, 2024
Only `TRACKNUMBER` and `GENRE` are listed here:
https://xiph.org/vorbis/doc/v-comment.html

The rest are derived from the example in Issue: #1958.

It's possible that other formats exist in the wild:
https://hydrogenaud.io/index.php/topic,69292.msg613808.html#msg613808

Issue: #1958
PiperOrigin-RevId: 704308788
@icbaker
Copy link
Collaborator

icbaker commented Dec 9, 2024

Disc and track number & totals (and genre) should now be populated by 1254607

@icbaker icbaker closed this as completed Dec 9, 2024
shahdDaghash pushed a commit that referenced this issue Dec 18, 2024
Only `TRACKNUMBER` and `GENRE` are listed here:
https://xiph.org/vorbis/doc/v-comment.html

The rest are derived from the example in Issue: #1958.

It's possible that other formats exist in the wild:
https://hydrogenaud.io/index.php/topic,69292.msg613808.html#msg613808

Issue: #1958
PiperOrigin-RevId: 704308788
(cherry picked from commit 1254607)
@androidx androidx locked and limited conversation to collaborators Feb 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants