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

Disable metadata parsing #2077

Open
OxygenCobalt opened this issue Jan 24, 2025 · 10 comments
Open

Disable metadata parsing #2077

OxygenCobalt opened this issue Jan 24, 2025 · 10 comments
Assignees
Labels

Comments

@OxygenCobalt
Copy link
Contributor

OxygenCobalt commented Jan 24, 2025

[REQUIRED] Use case description

My app now uses it's own metadata extractor, and has no dependence on ExoPlayer's metadata extractor, only it's basic container parsing and codec handling. As a result, this default metadata code remains and both bloats app size and uses additional memory during playback.

Proposed solution

A way to configure extractors to not parse (optional) metadata in such a way that proguard can strip it out from the app.

Alternatives considered

Patching the support out myself, but this seems brittle.

@tonihei
Copy link
Collaborator

tonihei commented Jan 24, 2025

Do you have a concrete MetadataExtractor in mind (and which other extractor it might be used from)? This will help to say how easy or complicated this could be to achieve.

As a result, this default metadata code remains and both bloats app size and uses additional memory during playback.

The general goal makes sense to me, but metadata is likely quite small both in app size and memory during playback. Have you measured the potential difference somehow to feel it's worth doing in the first place?

@tonihei tonihei self-assigned this Jan 24, 2025
@OxygenCobalt
Copy link
Contributor Author

Do you have a concrete MetadataExtractor in mind (and which other extractor it might be used from)? This will help to say how easy or complicated this could be to achieve.

My thinking is that each individual extractor would take some kind of generic metadata extractor interface (i.e Mp3Extractor would take maybe some Id3Parser, OggExtractor might take VorbisCommentParser), or similar.

The general goal makes sense to me, but metadata is likely quite small both in app size and memory during playback. Have you measured the potential difference somehow to feel it's worth doing in the first place?

Everything except images. My users can have libraries with huge cover art files (on the order of a megabyte decoded), which often leads to a lot of aggressive GCing on low-end devices if not outright OOMs.

@tonihei

@tonihei
Copy link
Collaborator

tonihei commented Jan 27, 2025

Everything except images. My users can have libraries with huge cover art files (on the order of a megabyte decoded), which often leads to a lot of aggressive GCing on low-end devices if not outright OOMs.

We already have extractor flags to suppress the parsing itself, e.g. DefaultExtractorsFactory.setMp3ExtractorFlags(FLAG_DISABLE_ID3_METADATA). This should help with the Bitmap problem.

bloats app size

I built the demo app without modifications to see how much space the metadata parsing uses in the APK and it seems to be 27KB overall, which doesn't sound too bad to me? Hiding the logic behind an interface may help of course, but this doesn't look like a major benefit?

Image

@Tolriq
Copy link
Contributor

Tolriq commented Jan 27, 2025

Jumping in, but being able to keep the parsing but skip bitmaps would still be a nice addition.

Until #418 the parsing can be useful for replay gains for example, but large images are really a pain #369 only workaround the memory issues about them, it will still put pressure on the loading, caching and other aspect of the app.

@OxygenCobalt
Copy link
Contributor Author

We already have extractor flags to suppress the parsing itself, e.g. DefaultExtractorsFactory.setMp3ExtractorFlags(FLAG_DISABLE_ID3_METADATA). This should help with the Bitmap problem.

Cool, this should work for me then @tonihei.

I built the demo app without modifications to see how much space the metadata parsing uses in the APK and it seems to be 27KB overall, which doesn't sound too bad to me? Hiding the logic behind an interface may help of course, but this doesn't look like a major benefit?

Guess so, I just don't like redundant code (My metadata parser has to duplicate a lot of the containers and metadata stuff). Either way, my primary use is bitmap size.

@OxygenCobalt
Copy link
Contributor Author

Until #418 the parsing can be useful for replay gains for example, but large images are really a pain #369 only workaround the memory issues about them, it will still put pressure on the loading, caching and other aspect of the app.

Interested about this, isn't MediaItem data still blocked by the same issue that blocks track change updates @Tolriq?

@Tolriq
Copy link
Contributor

Tolriq commented Jan 28, 2025

@OxygenCobalt not sure to understand your question.

@OxygenCobalt
Copy link
Contributor Author

Okay, I invesigated on my end @tonihei and the ability to fully disable metadata parsing is still quite limited. You can only really turn off ID3 parsing, and since covers can still come from FLAC pictures and vorbis comments it sadly doens't solve my problem. I need the ability to fully disable metadata support.

@OxygenCobalt
Copy link
Contributor Author

OxygenCobalt commented Jan 30, 2025

@Tolriq I mean in #418, as far as I'm aware not even the parsed MediaItem metadata can be delivered in time for an AudioProcessor to start using them before it starts processing. So there should be no difference, right?

@Tolriq
Copy link
Contributor

Tolriq commented Jan 31, 2025

Yes you can it's a little convoluted as you need a ForwardingAudioSink to get the configure call at the right time to pass the metadata to the processor. But after #594 it is possible.

What's missing is a way to manually augment the metadata or get access to the proper MediaItem for external metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants