From e087d35cd805565742d244848cf30bc13431f9c0 Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Fri, 25 Aug 2023 20:20:46 -0500 Subject: [PATCH] LibAudio: Accurately skip MP3 frames using the actual header size Prevously, the header size was used to calculate the `slot_count` field of `MP3::Header`, but `build_seek_table()` just used the maximum size of the header instead, causing it not to seek far enough, and in cases where a possible sync code occurred two bytes before the next frame, it would read that possible sync code as if it was a real frame. It would then either reject it due to bad field values, or could possibly skip over the next real frame due to a larger calculated frame size in the bogus frame. By fixing this issue, we now properly calculate the duration of MP3 files where these fake sync codes occur. In the case of the raw file for this podcast: https://changelog.com/podcast/554 the duration goes from 1:21:57 to 1:22:47, which is the real duration according to the player user interface. --- Userland/Libraries/LibAudio/MP3Loader.cpp | 9 ++++++--- Userland/Libraries/LibAudio/MP3Types.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibAudio/MP3Loader.cpp b/Userland/Libraries/LibAudio/MP3Loader.cpp index ccd1a5bfb5..17bff9f338 100644 --- a/Userland/Libraries/LibAudio/MP3Loader.cpp +++ b/Userland/Libraries/LibAudio/MP3Loader.cpp @@ -192,7 +192,7 @@ MaybeLoaderError MP3LoaderPlugin::build_seek_table() frame_count++; sample_count += MP3::frame_size; - TRY(m_stream->seek(error_or_header.value().frame_size - 6, SeekMode::FromCurrentPosition)); + TRY(m_stream->seek(error_or_header.value().frame_size - error_or_header.value().header_size, SeekMode::FromCurrentPosition)); // TODO: This is just here to clear the bitstream buffer. // Bitstream should have a method to sync its state to the underlying stream. @@ -223,10 +223,13 @@ ErrorOr MP3LoaderPlugin::read_header() header.copyright_bit = TRY(m_bitstream->read_bit()); header.original_bit = TRY(m_bitstream->read_bit()); header.emphasis = static_cast(TRY(m_bitstream->read_bits(2))); - if (!header.protection_bit) + header.header_size = 4; + if (!header.protection_bit) { header.crc16 = TRY(m_bitstream->read_bits(16)); + header.header_size += 2; + } header.frame_size = 144 * header.bitrate * 1000 / header.samplerate + header.padding_bit; - header.slot_count = header.frame_size - ((header.channel_count() == 2 ? 32 : 17) + (header.protection_bit ? 0 : 2) + 4); + header.slot_count = header.frame_size - ((header.channel_count() == 2 ? 32 : 17) + header.header_size); return header; } diff --git a/Userland/Libraries/LibAudio/MP3Types.h b/Userland/Libraries/LibAudio/MP3Types.h index 764b0a00f6..bf3d25db56 100644 --- a/Userland/Libraries/LibAudio/MP3Types.h +++ b/Userland/Libraries/LibAudio/MP3Types.h @@ -58,6 +58,7 @@ struct Header { bool original_bit { false }; Emphasis emphasis { Emphasis::None }; u16 crc16 { 0 }; + size_t header_size { 0 }; size_t frame_size { 0 }; size_t slot_count { 0 };