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

Proposal: seperate metadata reading from input plugins #617

Closed
nefthy opened this issue Jan 23, 2017 · 2 comments
Closed

Proposal: seperate metadata reading from input plugins #617

nefthy opened this issue Jan 23, 2017 · 2 comments

Comments

@nefthy
Copy link
Collaborator

nefthy commented Jan 23, 2017

Rational

There are IP plugins are known to be horribly inefficient at extracting metadata (ffmpeg). There are quite a few specialized libraries for extracting metadata from files. For example id3lib[1]. Another possibility would be to use libmad for metadata and ffmpeg for decoding. As metadata and decoding are separate tasks it seems sensible to decouple the code.

Implementation plan

  • Make a plugin API analogous to ip and op. This needs to be further defined if the proposal seems sensitive.
  • Port metadata code from input plugins to the new API
  • Implement metadata extraction for mp3 through

Further consideration

This is a major change and it should not be done before the next stable release.

A benchmark and test plan needs to be devised, to ensure functionality is not broken and performance is better or equal to the current.

This change will be primarily useful for working around the misbehaving ffmpeg, however from an architectural point of view it seems legit.

Feel free to comment. If there consensus, I will flesh out the details.

[1] http://id3lib.sourceforge.net/

@mahkoh
Copy link
Member

mahkoh commented Feb 4, 2017

There are IP plugins are known to be horribly inefficient at extracting metadata (ffmpeg).

That is so.

This all seems very reasonable but I don't think anyone is too bothered by this slowness in the first place. Thanks to the cache, I think this is a non-issue.

If we were to rewrite cmus from scratch, then this feature would be included. But after #505 and #574 I'm a bit weary of making any changes that don't result in clear benefits.

@nefthy
Copy link
Collaborator Author

nefthy commented Feb 9, 2017

I understand. please close.

@mahkoh mahkoh closed this as completed Feb 9, 2017
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

No branches or pull requests

2 participants