-
Notifications
You must be signed in to change notification settings - Fork 154
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
flac, isomp4, mkv: optimize length detection by using byte_len() instead of seeking #340
base: master
Are you sure you want to change the base?
Conversation
…ead of seeking Replace seeking to end of file with byte_len() call to get total stream length. This improves performance when streaming files by avoiding an unnecessary seek operation. The change maintains the same functionality while being more efficient, especially for remote/streaming files where seeks are expensive. Also fixes some minor code formatting issues with else clauses.
Uses a patched version of Symphonia that improves seeking performance in FLAC and MP4 files by using the known file byte length instead of seeking to the end of file to determine length. This is a temporary patch that will be removed once pdeljanov/Symphonia#340 is merged upstream.
Hi @roderickvd, yeah, that's the way it should've been. Nice find! The reason you got so many formatting changes is because we need the nightly rustfmt to use the brace style option. Are you able to run that with By the way, please keep an eye on the |
Cool. Some questions after thinking over seeking some more:
Can move to a discussion to not lead this PR astray too much.
Sure, just did so in f143245. Let me know if you want me to squash.
Yes, will do. You think this could make its way into a |
Refactoring Rodio's decoder to take a |
There's no timing information in a MP3, so to seek in a sample accurate manner the only way is to count samples from the beginning of the file. In contrast, coarse seeking estimates the average bitrate of the file and then uses it to estimate where the reader should seek to. If the underlying source is unseekable, then we assume the Other formats are generally better in this regard. They either contain a table mapping timestamp-to-position, or each packet is timestamped so we can binary search. If I recall correctly, most of these readers will perform a seek in two stages: seek to a rough position guided by the table or binary search algorithm, then refine the position by reading sequentially until the desired timestamp is found. Usually, if the source is unseekable, it just falls back to the second stage so forward seeks work atleast. If this fallback is missing anywhere, then yes, we could do it!
I believe this should work, but it may be failing if
Hmm, possibly. Though I guess this would be an optimistic optimization: no need to seek multiple times for a binary search if a first-try guess gets close enough.
Noted! Since I imagine most people use Symphonia through Rodio, this would be a pretty bad breakage. It would be great if Rodio could push an update with your fixes before this lands in a Symphonia release.
Yeah, we could include this in 0.5.5. I was planning to cut a release before |
Replace seeking to end of file with
byte_len()
call to get total stream length. This improves performance when streaming files by avoiding an unnecessary seek operation. The change maintains the same functionality while being more efficient, especially for remote/streaming files where seeks are expensive.As used in
pleezer
, previously seeking would block the application until the full file was downloaded, even if the position seemed to was already buffered. With this change, such seeks are nearly instantaneous.Optionally, we could first try
byte_len()
and fall back to seeking to the end whenbyte_len()
is not available.The actual change is just three lines, but my editor automatically fixed some minor code formatting issues with else clauses. Nothing more than
rustfmt
.