From 9c9f1154f83c5ee3363ede8385170e699d36a22d Mon Sep 17 00:00:00 2001 From: strellydev <113683950+strellydev@users.noreply.github.com> Date: Wed, 9 Aug 2023 17:40:08 +0100 Subject: [PATCH] Fix OGG audio loop offset pop. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: MJacred Co-authored-by: Ellen Poe Co-authored-by: Michael Wörner --- modules/ogg/ogg_packet_sequence.cpp | 18 +- modules/ogg/ogg_packet_sequence.h | 7 + modules/vorbis/audio_stream_ogg_vorbis.cpp | 161 ++++++++---------- modules/vorbis/audio_stream_ogg_vorbis.h | 4 +- .../vorbis/resource_importer_ogg_vorbis.cpp | 19 ++- 5 files changed, 107 insertions(+), 102 deletions(-) diff --git a/modules/ogg/ogg_packet_sequence.cpp b/modules/ogg/ogg_packet_sequence.cpp index fb29493ca9ca..1100367f03b3 100644 --- a/modules/ogg/ogg_packet_sequence.cpp +++ b/modules/ogg/ogg_packet_sequence.cpp @@ -159,7 +159,9 @@ bool OggPacketSequencePlayback::next_ogg_packet(ogg_packet **p_packet) const { *p_packet = packet; - packet_cursor++; + if (!packet->e_o_s) { // Added this so it doesn't try to go to the next packet if it's the last packet of the file. + packet_cursor++; + } return true; } @@ -216,6 +218,20 @@ bool OggPacketSequencePlayback::seek_page(int64_t p_granule_pos) { return true; } +int64_t OggPacketSequencePlayback::get_page_number() const { + return page_cursor; +} + +bool OggPacketSequencePlayback::set_page_number(int64_t p_page_number) { + if (p_page_number >= 0 && p_page_number < ogg_packet_sequence->page_data.size()) { + page_cursor = p_page_number; + packet_cursor = 0; + packetno = 0; + return true; + } + return false; +} + OggPacketSequencePlayback::OggPacketSequencePlayback() { packet = new ogg_packet(); } diff --git a/modules/ogg/ogg_packet_sequence.h b/modules/ogg/ogg_packet_sequence.h index 7085504500a7..922a16560ca2 100644 --- a/modules/ogg/ogg_packet_sequence.h +++ b/modules/ogg/ogg_packet_sequence.h @@ -120,6 +120,13 @@ class OggPacketSequencePlayback : public RefCounted { // Returns true on success, false on failure. bool seek_page(int64_t p_granule_pos); + // Gets the current page number. + int64_t get_page_number() const; + + // Moves to a specific page in the stream. + // Returns true on success, false if the page number is out of bounds. + bool set_page_number(int64_t p_page_number); + OggPacketSequencePlayback(); virtual ~OggPacketSequencePlayback(); }; diff --git a/modules/vorbis/audio_stream_ogg_vorbis.cpp b/modules/vorbis/audio_stream_ogg_vorbis.cpp index b155dfc5d9ec..8a265ffaf3af 100644 --- a/modules/vorbis/audio_stream_ogg_vorbis.cpp +++ b/modules/vorbis/audio_stream_ogg_vorbis.cpp @@ -264,11 +264,10 @@ void AudioStreamPlaybackOggVorbis::seek(double p_time) { return; } - vorbis_synthesis_restart(&dsp_state); - if (p_time >= vorbis_stream->get_length()) { p_time = 0; } + frames_mixed = uint32_t(vorbis_data->get_sampling_rate() * p_time); const int64_t desired_sample = p_time * get_stream_sampling_rate(); @@ -278,107 +277,81 @@ void AudioStreamPlaybackOggVorbis::seek(double p_time) { return; } - ogg_packet *packet; - if (!vorbis_data_playback->next_ogg_packet(&packet)) { - WARN_PRINT_ONCE("seeking beyond limits"); - return; + // We want to start decoding before the page that we expect the sample to be in (the sample may + // be part of a partial packet across page boundaries). Otherwise, the decoder may not have + // synchronized before reaching the sample. + int64_t start_page_number = vorbis_data_playback->get_page_number() - 1; + if (start_page_number < 0) { + start_page_number = 0; } - // The granule position of the page we're seeking through. - int64_t granule_pos = 0; - - int headers_remaining = 0; - int samples_in_page = 0; - int err; while (true) { - if (vorbis_synthesis_idheader(packet)) { - headers_remaining = 3; - } - if (!headers_remaining) { - err = vorbis_synthesis(&block, packet); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis synthesis " + itos(err)); - - err = vorbis_synthesis_blockin(&dsp_state, &block); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis block processing " + itos(err)); - - int samples_out = vorbis_synthesis_pcmout(&dsp_state, nullptr); - err = vorbis_synthesis_read(&dsp_state, samples_out); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis read updating " + itos(err)); - - samples_in_page += samples_out; - - } else { - headers_remaining--; - } - if (packet->granulepos != -1 && headers_remaining == 0) { - // This indicates the end of the page. - granule_pos = packet->granulepos; - break; - } - if (packet->e_o_s) { - break; - } - if (!vorbis_data_playback->next_ogg_packet(&packet)) { - // We should get an e_o_s flag before this happens. - WARN_PRINT("Vorbis file ended without warning."); - break; - } - } + ogg_packet *packet; + int err; - int64_t samples_to_burn = samples_in_page - (granule_pos - desired_sample); + // We start at an unknown granule position. + int64_t granule_pos = -1; - if (samples_to_burn > samples_in_page) { - WARN_PRINT_ONCE("Burning more samples than we have in this page. Check seek algorithm."); - } else if (samples_to_burn < 0) { - WARN_PRINT_ONCE("Burning negative samples doesn't make sense. Check seek algorithm."); - } + // Decode data until we get to the desired sample or notice that we have read past it. + vorbis_data_playback->set_page_number(start_page_number); + vorbis_synthesis_restart(&dsp_state); - // Seek again, this time we'll burn a specific number of samples instead of all of them. - if (!vorbis_data_playback->seek_page(desired_sample)) { - WARN_PRINT("seek failed"); - return; - } - - if (!vorbis_data_playback->next_ogg_packet(&packet)) { - WARN_PRINT_ONCE("seeking beyond limits"); - return; - } - vorbis_synthesis_restart(&dsp_state); + while (true) { + if (!vorbis_data_playback->next_ogg_packet(&packet)) { + WARN_PRINT_ONCE("Seeking beyond limits"); + return; + } - while (true) { - if (vorbis_synthesis_idheader(packet)) { - headers_remaining = 3; - } - if (!headers_remaining) { err = vorbis_synthesis(&block, packet); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis synthesis " + itos(err)); - - err = vorbis_synthesis_blockin(&dsp_state, &block); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis block processing " + itos(err)); - - int samples_out = vorbis_synthesis_pcmout(&dsp_state, nullptr); - int read_samples = samples_to_burn > samples_out ? samples_out : samples_to_burn; - err = vorbis_synthesis_read(&dsp_state, samples_out); - ERR_FAIL_COND_MSG(err != 0, "Error during vorbis read updating " + itos(err)); - samples_to_burn -= read_samples; - - if (samples_to_burn <= 0) { - break; + if (err != OV_ENOTAUDIO) { + ERR_FAIL_COND_MSG(err != 0, "Error during vorbis synthesis " + itos(err) + "."); + + err = vorbis_synthesis_blockin(&dsp_state, &block); + ERR_FAIL_COND_MSG(err != 0, "Error during vorbis block processing " + itos(err) + "."); + + int samples_out = vorbis_synthesis_pcmout(&dsp_state, nullptr); + + if (granule_pos < 0) { + // We don't know where we are yet, so just keep on decoding. + err = vorbis_synthesis_read(&dsp_state, samples_out); + ERR_FAIL_COND_MSG(err != 0, "Error during vorbis read updating " + itos(err) + "."); + } else if (granule_pos + samples_out >= desired_sample) { + // Our sample is in this block. Skip the beginning of the block up to the sample, then + // return. + int skip_samples = (int)(desired_sample - granule_pos); + err = vorbis_synthesis_read(&dsp_state, skip_samples); + ERR_FAIL_COND_MSG(err != 0, "Error during vorbis read updating " + itos(err) + "."); + have_samples_left = skip_samples < samples_out; + have_packets_left = !packet->e_o_s; + return; + } else { + // Our sample is not in this block. Skip it. + err = vorbis_synthesis_read(&dsp_state, samples_out); + ERR_FAIL_COND_MSG(err != 0, "Error during vorbis read updating " + itos(err) + "."); + granule_pos += samples_out; + } + } + if (packet->granulepos != -1) { + // We found an update to our granule position. + granule_pos = packet->granulepos; + if (granule_pos > desired_sample) { + // We've read past our sample. We need to start on an earlier page. + if (start_page_number == 0) { + // We didn't find the sample even reading from the beginning. + have_samples_left = false; + have_packets_left = !packet->e_o_s; + return; + } + start_page_number--; + break; + } + } + if (packet->e_o_s) { + // We've reached the end of the stream and didn't find our sample. + have_samples_left = false; + have_packets_left = false; + return; } - } else { - headers_remaining--; - } - if (packet->granulepos != -1 && headers_remaining == 0) { - // This indicates the end of the page. - break; - } - if (packet->e_o_s) { - break; - } - if (!vorbis_data_playback->next_ogg_packet(&packet)) { - // We should get an e_o_s flag before this happens. - WARN_PRINT("Vorbis file ended without warning."); - break; } } } diff --git a/modules/vorbis/audio_stream_ogg_vorbis.h b/modules/vorbis/audio_stream_ogg_vorbis.h index 41ce942eec6a..6abaeaaebdf6 100644 --- a/modules/vorbis/audio_stream_ogg_vorbis.h +++ b/modules/vorbis/audio_stream_ogg_vorbis.h @@ -107,9 +107,9 @@ class AudioStreamOggVorbis : public AudioStream { friend class AudioStreamPlaybackOggVorbis; int channels = 1; - float length = 0.0; + double length = 0.0; bool loop = false; - float loop_offset = 0.0; + double loop_offset = 0.0; // Performs a seek to the beginning of the stream, should not be called during playback! // Also causes allocation and deallocation. diff --git a/modules/vorbis/resource_importer_ogg_vorbis.cpp b/modules/vorbis/resource_importer_ogg_vorbis.cpp index b42cd2058948..a8c92f06f66e 100644 --- a/modules/vorbis/resource_importer_ogg_vorbis.cpp +++ b/modules/vorbis/resource_importer_ogg_vorbis.cpp @@ -97,7 +97,7 @@ void ResourceImporterOggVorbis::show_advanced_options(const String &p_path) { Error ResourceImporterOggVorbis::import(const String &p_source_file, const String &p_save_path, const HashMap &p_options, List *r_platform_variants, List *r_gen_files, Variant *r_metadata) { bool loop = p_options["loop"]; - float loop_offset = p_options["loop_offset"]; + double loop_offset = p_options["loop_offset"]; double bpm = p_options["bpm"]; int beat_count = p_options["beat_count"]; int bar_beats = p_options["bar_beats"]; @@ -184,7 +184,7 @@ Ref ResourceImporterOggVorbis::load_from_buffer(const Vect ERR_FAIL_COND_V_MSG(err != 0, Ref(), "Ogg stream error " + itos(err)); int desync_iters = 0; - Vector> packet_data; + RBMap>> sorted_packets; int64_t granule_pos = 0; while (true) { @@ -192,6 +192,7 @@ Ref ResourceImporterOggVorbis::load_from_buffer(const Vect if (err == -1) { // According to the docs this is usually recoverable, but don't sit here spinning forever. desync_iters++; + WARN_PRINT_ONCE("Desync during ogg import."); ERR_FAIL_COND_V_MSG(desync_iters > 100, Ref(), "Packet sync issue during Ogg import"); continue; } else if (err == 0) { @@ -207,16 +208,24 @@ Ref ResourceImporterOggVorbis::load_from_buffer(const Vect } break; } - granule_pos = packet.granulepos; + if (packet.granulepos > granule_pos) { + granule_pos = packet.granulepos; + } PackedByteArray data; data.resize(packet.bytes); memcpy(data.ptrw(), packet.packet, packet.bytes); - packet_data.push_back(data); + sorted_packets[granule_pos].push_back(data); packet_count++; } + Vector> packet_data; + for (const KeyValue>> &pair : sorted_packets) { + for (const Vector &packets : pair.value) { + packet_data.push_back(packets); + } + } if (initialized_stream) { - ogg_packet_sequence->push_page(granule_pos, packet_data); + ogg_packet_sequence->push_page(ogg_page_granulepos(&page), packet_data); } } if (initialized_stream) {