RSDK-13200 Make mp3 decoding more reliable #24
Conversation
oliviamiller
force-pushed
the
mp3-id3-fix
branch
from
763f5cb to
fd0d9b1
Compare
| set(DISCOVERY_TIMEOUT_VALUE $ENV{GTEST_DISCOVERY_TIMEOUT}) | ||
| else() | ||
| set(DISCOVERY_TIMEOUT_VALUE 5) | ||
| set(DISCOVERY_TIMEOUT_VALUE 15) |
There was a problem hiding this comment.
this kept timing out for some reason so increased it
src/mp3_decoder.cpp
Outdated
| } | ||
|
|
||
| // helper to skip id3 tag: https://id3.org/id3v2.3.0 | ||
| static size_t get_id3v2_offset(const uint8_t* data, size_t size) { |
There was a problem hiding this comment.
Why are you passing the underlying pointer to data instead of a const reference to the buffer vector itself?
There was a problem hiding this comment.
no particular reason, will change to pass vector
src/mp3_decoder.cpp
Outdated
| // ID3v2 header | ||
| if (data[0] == 'I' && data[1] == 'D' && data[2] == '3') { | ||
| // Size is stored as a "syncsafe" integer (7 bits per byte) | ||
| size_t tag_size = ((data[6] & 0x7F) << 21) | ((data[7] & 0x7F) << 14) | ((data[8] & 0x7F) << 7) | (data[9] & 0x7F); |
There was a problem hiding this comment.
let's err on declaring all variables as const unless they are meant to be mutable.
src/mp3_decoder.cpp
Outdated
| } | ||
|
|
||
| VIAM_SDK_LOG(debug) << "Decoding " << (encoded_data.size()) << " bytes of MP3 data"; | ||
| std::vector<uint8_t> buffer(encoded_data.begin(), encoded_data.end()); |
There was a problem hiding this comment.
In this logic here, you can calculate the offset directly on the encoded_data vector as you are not modifying it, no need to copy it at this point.
Then, later on, you can copy only the portion of the encoded_data you need and keep using this as a vector, instead of operating on raw pointers as in here:
unsigned char* encoded_data_ptr = buffer.data() + offset;
const size_t mp3_data_length = buffer.size() - offset;
There was a problem hiding this comment.
good point, will rework this to not allocate extra memory
2 participants
On MP3 files that had headers before the actual audio data, we would fail to decode and did not play the audio. This scans for the start of MP3 data and decodes from there. Also retrying the flush call up to 10 times to ensure we get all decoded data and don't fail too early.
Tested various MP3 files generated by TTS libraries