Skip to content

MP3Decoder: Accurately inform when no more data #6230

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

Merged
merged 2 commits into from
Apr 5, 2022

Conversation

jepler
Copy link

@jepler jepler commented Apr 4, 2022

Some audio implementations, notably samd, really don't like it when you return 0 samples of data. This was the case when reaching the end of an MP3 file.

Now, we read forward in an MP3 file to the next sync word during "get_buffer", so that we can accurately return GET_BUFFER_DONE when the NEXT call WOULD HAVE resulted in 0 samples.

Tested with @gamblor21's "laugh.mp3" file on a Trellis M4 Express.

Closes: #5415.

Some audio implementations, notably samd, really don't like it when
you return 0 samples of data. This was the case when reaching the
end of an MP3 file.

Now, we read forward in an MP3 file to the next sync word during
"get_buffer", so that we can accurately return GET_BUFFER_DONE when the
NEXT call WOULD HAVE resulted in 0 samples.

Tested with @gamblor21's "laugh.mp3" file on a Trellis M4 Express.
dhalbert
dhalbert previously approved these changes Apr 5, 2022
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I thought I fixed the issue of handling zero bytes returned from the stream, though. What is an example of code that is doing the wrong thing?

@dhalbert dhalbert added this to the 7.2.x milestone Apr 5, 2022
@dhalbert
Copy link
Collaborator

dhalbert commented Apr 5, 2022

Advancing the cache ID fixes the espressif build issues, so merging this first to get that fix in.

@dhalbert dhalbert merged commit c3cf926 into adafruit:7.2.x Apr 5, 2022
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

Successfully merging this pull request may close these issues.

2 participants