mp3decoder inbuf update fix - #11219
Conversation
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
One, style comment. Good otherwise.
| return result; | ||
| } | ||
|
|
||
| static bool mp3file_update_inbuf_always_impl(audiomp3_mp3file_obj_t *self, bool block_ok) { |
There was a problem hiding this comment.
I wouldn't do it this way because it adds another function to think about. Instead, just put prevent and allow in the existing function.
There was a problem hiding this comment.
Changed in the latest commit to remove the extra function layer.
There was a problem hiding this comment.
This needs further changes to work. New commit coming in a bit.
There was a problem hiding this comment.
Fixed with the latest commit. This needed to have allow()s added to the other exit paths.
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
That's fine. Thanks! This actually fixes the exception case. It wouldn't be run with the wrapper because of the weird unwinding that exceptions do.
2 participants
This fixes an issue that can arise when the MP3Decoder is reading from a source that runs background tasks during the read. No such source exists today but #11217 adds one. The EMMC driver implemented in that PR runs background tasks, so decoding an MP3 from it caused it to call
mp3file_update_inbuf_cb()again while in the middle of the previous read.INPUT_BUFFER_SPACE() is signed but
to_readis not, so a negative space would wrap and be passed tostream_read()Fixed by preventing background tasks while mp3 decoder fills inbuff and clamping logic for
to_read.I confirmed that this fixes the issue with decoding mp3s from the sp1emmc module in #11217. I think this fix stands aside from the SP-1 support because it would bite any other source that runs runs background tasks while getting the file data.