Fix Ogg/Mp3 import loop point not having single-sample precision

doc/classes/AudioStream.xml

@@ -34,6 +34,11 @@
 				Return the controllable parameters of this stream. This array contains dictionaries with a property info description format (see [method Object.get_property_list]). Additionally, the default value for this parameter must be added tho each dictionary in "default_value" field.
 			</description>
 		</method>
+		<method name="_get_sampling_rate" qualifiers="virtual const">
+			<return type="int" />
+			<description>
+			</description>
+		</method>
 		<method name="_get_stream_name" qualifiers="virtual const">
 			<return type="String" />
 			<description>
@@ -55,6 +60,12 @@
 				Returns the length of the audio stream in seconds.
 			</description>
 		</method>
+		<method name="get_sampling_rate" qualifiers="const">
+			<return type="int" />
+			<description>
+				Returns the sampling rate of the audio stream in samples per second.
+			</description>
+		</method>
 		<method name="instantiate_playback">
 			<return type="AudioStreamPlayback" />
 			<description>

editor/import/audio_stream_import_settings.cpp

@@ -437,7 +437,17 @@ void AudioStreamImportSettingsDialog::edit(const String &p_path, const String &p
 			beats_edit->set_value(beats);
 			beats_enabled->set_pressed(beats > 0);
 			loop->set_pressed(config_file->get_value("params", "loop", false));
-			loop_offset->set_value(config_file->get_value("params", "loop_offset", 0));
+			loop_offset_unit->select((int)LoopOffsetUnit::LOOP_OFFSET_UNIT_SECONDS);
+			loop_offset->set_step(0.00001);
+			loop_offset->set_max(stream->get_length());
+			if (config_file->has_section_key("params", "loop_offset_samples")) {
+				_loop_offset_samples = config_file->get_value("params", "loop_offset_samples");
+				loop_offset->set_value(_samples_to_unit(_loop_offset_samples));
+			} else {
+				loop_offset->set_value(config_file->get_value("params", "loop_offset", 0));
+				_loop_offset_samples = _unit_to_samples(loop_offset->get_value());
+			}
+			stream->call("set_loop_offset", (double)_loop_offset_samples / stream->get_sampling_rate());
 			bar_beats_edit->set_value(config_file->get_value("params", "bar_beats", 4));
 
 			List<String> keys;
@@ -471,11 +481,20 @@ void AudioStreamImportSettingsDialog::_settings_changed() {
 
 	updating_settings = true;
 	stream->call("set_loop", loop->is_pressed());
-	stream->call("set_loop_offset", loop_offset->get_value());
+
+	const double current_loop_offset = _samples_to_unit(_loop_offset_samples);
+	const double new_loop_offset = loop_offset->get_value();
+	if (ABS(current_loop_offset - new_loop_offset) >= loop_offset->get_step() * 0.5) {
+		_loop_offset_samples = _unit_to_samples(new_loop_offset);
+		stream->call("set_loop_offset", (double)_loop_offset_samples / stream->get_sampling_rate());
+	}
+
 	if (loop->is_pressed()) {
 		loop_offset->set_editable(true);
+		loop_offset_unit->set_disabled(false);
 	} else {
 		loop_offset->set_editable(false);
+		loop_offset_unit->set_disabled(true);
 	}
 
 	if (bpm_enabled->is_pressed()) {
@@ -515,16 +534,84 @@ void AudioStreamImportSettingsDialog::_settings_changed() {
 	color_rect->queue_redraw();
 }
 
+void AudioStreamImportSettingsDialog::_unit_changed() {
+	if (updating_settings) {
+		return;
+	}
+
+	updating_settings = true;
+	loop_offset->set_max(_seconds_to_unit(stream->get_length()));
+	loop_offset->set_step(loop_offset_unit->get_selected() == (int)LoopOffsetUnit::LOOP_OFFSET_UNIT_SAMPLES ? 1.0 : 0.00001);
+	loop_offset->set_value(_samples_to_unit(_loop_offset_samples));
+	updating_settings = false;
+}
+
 void AudioStreamImportSettingsDialog::_reimport() {
 	params["loop"] = loop->is_pressed();
-	params["loop_offset"] = loop_offset->get_value();
+	params["loop_offset_samples"] = _loop_offset_samples;
 	params["bpm"] = bpm_enabled->is_pressed() ? double(bpm_edit->get_value()) : double(0);
 	params["beat_count"] = (bpm_enabled->is_pressed() && beats_enabled->is_pressed()) ? int(beats_edit->get_value()) : int(0);
 	params["bar_beats"] = (bpm_enabled->is_pressed()) ? int(bar_beats_edit->get_value()) : int(4);
 
 	EditorFileSystem::get_singleton()->reimport_file_with_custom_parameters(path, importer, params);
 }
 
+double AudioStreamImportSettingsDialog::_samples_to_unit(int64_t p_samples) const {
+	double value;
+	switch ((LoopOffsetUnit)loop_offset_unit->get_selected()) {
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SECONDS: {
+			value = (double)p_samples / stream->get_sampling_rate();
+			break;
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SAMPLES: {
+			return p_samples;
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_BEATS: {
+			value = (double)p_samples / stream->get_sampling_rate() /
+					60.0 * bpm_edit->get_value();
+			break;
+		}
+		default: {
+			return p_samples;
+		}
+	}
+	return Math::round(value / loop_offset->get_step()) * loop_offset->get_step();
+}
+
+double AudioStreamImportSettingsDialog::_seconds_to_unit(double p_seconds) const {
+	switch ((LoopOffsetUnit)loop_offset_unit->get_selected()) {
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SECONDS: {
+			return p_seconds;
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SAMPLES: {
+			return p_seconds * stream->get_sampling_rate();
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_BEATS: {
+			return p_seconds / 60.0 * bpm_edit->get_value();
+		}
+		default: {
+			return p_seconds;
+		}
+	}
+}
+
+int64_t AudioStreamImportSettingsDialog::_unit_to_samples(double p_value) const {
+	switch ((LoopOffsetUnit)loop_offset_unit->get_selected()) {
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SECONDS: {
+			return p_value * stream->get_sampling_rate();
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_SAMPLES: {
+			return p_value;
+		}
+		case LoopOffsetUnit::LOOP_OFFSET_UNIT_BEATS: {
+			return p_value / bpm_edit->get_value() * 60.0 * stream->get_sampling_rate();
+		}
+		default: {
+			return p_value;
+		}
+	}
+}
+
 AudioStreamImportSettingsDialog::AudioStreamImportSettingsDialog() {
 	get_ok_button()->set_text(TTR("Reimport"));
 	get_cancel_button()->set_text(TTR("Close"));
@@ -542,12 +629,16 @@ AudioStreamImportSettingsDialog::AudioStreamImportSettingsDialog() {
 	loop_hb->add_spacer();
 	loop_hb->add_child(memnew(Label(TTR("Offset:"))));
 	loop_offset = memnew(SpinBox);
-	loop_offset->set_max(10000);
-	loop_offset->set_step(0.001);
-	loop_offset->set_suffix("sec");
-	loop_offset->set_tooltip_text(TTR("Loop offset (from beginning). Note that if BPM is set, this setting will be ignored."));
+	loop_offset->set_custom_minimum_size(Size2(120, 0) * EDSCALE);
+	loop_offset->set_tooltip_text(TTR("Playback will loop back to this point after reaching the end of the stream."));
 	loop_offset->connect("value_changed", callable_mp(this, &AudioStreamImportSettingsDialog::_settings_changed).unbind(1));
 	loop_hb->add_child(loop_offset);
+	loop_offset_unit = memnew(OptionButton);
+	loop_offset_unit->add_item(TTR("Seconds"));
+	loop_offset_unit->add_item(TTR("Samples"));
+	loop_offset_unit->add_item(TTR("Beats"));
+	loop_offset_unit->connect("item_selected", callable_mp(this, &AudioStreamImportSettingsDialog::_unit_changed).unbind(1));
+	loop_hb->add_child(loop_offset_unit);
 	main_vbox->add_margin_child(TTR("Loop:"), loop_hb);
 
 	HBoxContainer *interactive_hb = memnew(HBoxContainer);

editor/import/audio_stream_import_settings.h

@@ -35,6 +35,7 @@
 #include "scene/audio/audio_stream_player.h"
 #include "scene/gui/color_rect.h"
 #include "scene/gui/dialogs.h"
+#include "scene/gui/option_button.h"
 #include "scene/gui/spin_box.h"
 #include "scene/resources/texture.h"
 
@@ -43,6 +44,12 @@ class CheckBox;
 class AudioStreamImportSettingsDialog : public ConfirmationDialog {
 	GDCLASS(AudioStreamImportSettingsDialog, ConfirmationDialog);
 
+	enum class LoopOffsetUnit {
+		LOOP_OFFSET_UNIT_SECONDS,
+		LOOP_OFFSET_UNIT_SAMPLES,
+		LOOP_OFFSET_UNIT_BEATS
+	};
+
 	CheckBox *bpm_enabled = nullptr;
 	SpinBox *bpm_edit = nullptr;
 	CheckBox *beats_enabled = nullptr;
@@ -51,6 +58,7 @@ class AudioStreamImportSettingsDialog : public ConfirmationDialog {
 	SpinBox *bar_beats_edit = nullptr;
 	CheckBox *loop = nullptr;
 	SpinBox *loop_offset = nullptr;
+	OptionButton *loop_offset_unit = nullptr;
 	ColorRect *color_rect = nullptr;
 	Ref<AudioStream> stream;
 	AudioStreamPlayer *_player = nullptr;
@@ -74,6 +82,7 @@ class AudioStreamImportSettingsDialog : public ConfirmationDialog {
 	bool _beat_len_dragging = false;
 	bool _pausing = false;
 	int _hovering_beat = -1;
+	int64_t _loop_offset_samples = 0;
 
 	HashMap<StringName, Variant> params;
 	String importer;
@@ -84,9 +93,14 @@ class AudioStreamImportSettingsDialog : public ConfirmationDialog {
 	static AudioStreamImportSettingsDialog *singleton;
 
 	void _settings_changed();
+	void _unit_changed();
 
 	void _reimport();
 
+	double _samples_to_unit(int64_t p_samples) const;
+	double _seconds_to_unit(double p_seconds) const;
+	int64_t _unit_to_samples(double p_unit) const;
+
 protected:
 	void _notification(int p_what);
 	void _preview_changed(ObjectID p_which);

modules/minimp3/audio_stream_mp3.cpp

@@ -253,6 +253,10 @@ bool AudioStreamMP3::is_monophonic() const {
 	return false;
 }
 
+int AudioStreamMP3::get_sampling_rate() const {
+	return (int)sample_rate;
+}
+
 void AudioStreamMP3::get_parameter_list(List<Parameter> *r_parameters) {
 	r_parameters->push_back(Parameter(PropertyInfo(Variant::BOOL, "looping", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CHECKABLE), Variant()));
 }

modules/minimp3/audio_stream_mp3.h

@@ -131,6 +131,8 @@ class AudioStreamMP3 : public AudioStream {
 
 	virtual bool is_monophonic() const override;
 
+	virtual int get_sampling_rate() const override;
+
 	virtual void get_parameter_list(List<Parameter> *r_parameters) override;
 
 	AudioStreamMP3();

modules/minimp3/doc_classes/ResourceImporterMP3.xml

@@ -28,10 +28,10 @@
 			If enabled, the audio will begin playing at the beginning after playback ends by reaching the end of the audio.
 			[b]Note:[/b] In [AudioStreamPlayer], the [signal AudioStreamPlayer.finished] signal won't be emitted for looping audio when it reaches the end of the audio file, as the audio will keep playing indefinitely.
 		</member>
-		<member name="loop_offset" type="float" setter="" getter="" default="0">
-			Determines where audio will start to loop after playback reaches the end of the audio. This can be used to only loop a part of the audio file, which is useful for some ambient sounds or music. The value is determined in seconds relative to the beginning of the audio. A value of [code]0.0[/code] will loop the entire audio file.
+		<member name="loop_offset_samples" type="int" setter="" getter="" default="0">
+			Determines where audio will start to loop after playback reaches the end of the audio. This can be used to only loop a part of the audio file, which is useful for some ambient sounds or music. The value is determined in samples relative to the beginning of the audio. A value of [code]0[/code] will loop the entire audio file.
 			Only has an effect if [member loop] is [code]true[/code].
-			A more convenient editor for [member loop_offset] is provided in the [b]Advanced Import Settings[/b] dialog, as it lets you preview your changes without having to reimport the audio.
+			A more convenient editor for [member loop_offset_samples] is provided in the [b]Advanced Import Settings[/b] dialog, as it lets you preview your changes without having to reimport the audio.
 		</member>
 	</members>
 </class>

modules/minimp3/resource_importer_mp3.cpp

@@ -76,7 +76,7 @@ String ResourceImporterMP3::get_preset_name(int p_idx) const {
 
 void ResourceImporterMP3::get_import_options(const String &p_path, List<ImportOption> *r_options, int p_preset) const {
 	r_options->push_back(ImportOption(PropertyInfo(Variant::BOOL, "loop"), false));
-	r_options->push_back(ImportOption(PropertyInfo(Variant::FLOAT, "loop_offset"), 0));
+	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "loop_offset_samples"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::FLOAT, "bpm", PROPERTY_HINT_RANGE, "0,400,0.01,or_greater"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "beat_count", PROPERTY_HINT_RANGE, "0,512,or_greater"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "bar_beats", PROPERTY_HINT_RANGE, "2,32,or_greater"), 4));
@@ -117,7 +117,6 @@ Ref<AudioStreamMP3> ResourceImporterMP3::import_mp3(const String &p_path) {
 
 Error ResourceImporterMP3::import(const String &p_source_file, const String &p_save_path, const HashMap<StringName, Variant> &p_options, List<String> *r_platform_variants, List<String> *r_gen_files, Variant *r_metadata) {
 	bool loop = p_options["loop"];
-	float loop_offset = p_options["loop_offset"];
 	double bpm = p_options["bpm"];
 	float beat_count = p_options["beat_count"];
 	float bar_beats = p_options["bar_beats"];
@@ -127,7 +126,11 @@ Error ResourceImporterMP3::import(const String &p_source_file, const String &p_s
 		return ERR_CANT_OPEN;
 	}
 	mp3_stream->set_loop(loop);
-	mp3_stream->set_loop_offset(loop_offset);
+	if (p_options.has("loop_offset_samples")) {
+		mp3_stream->set_loop_offset((double)p_options["loop_offset_samples"] / mp3_stream->get_sampling_rate());
+	} else {
+		mp3_stream->set_loop_offset(p_options["loop_offset"]);
+	}
 	mp3_stream->set_bpm(bpm);
 	mp3_stream->set_beat_count(beat_count);
 	mp3_stream->set_bar_beats(bar_beats);

modules/vorbis/audio_stream_ogg_vorbis.cpp

@@ -513,6 +513,11 @@ bool AudioStreamOggVorbis::is_monophonic() const {
 	return false;
 }
 
+int AudioStreamOggVorbis::get_sampling_rate() const {
+	ERR_FAIL_COND_V(packet_sequence.is_null(), 0);
+	return packet_sequence->get_sampling_rate();
+}
+
 void AudioStreamOggVorbis::get_parameter_list(List<Parameter> *r_parameters) {
 	r_parameters->push_back(Parameter(PropertyInfo(Variant::BOOL, "looping", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_CHECKABLE), Variant()));
 }

modules/vorbis/audio_stream_ogg_vorbis.h

@@ -157,6 +157,8 @@ class AudioStreamOggVorbis : public AudioStream {
 
 	virtual bool is_monophonic() const override;
 
+	virtual int get_sampling_rate() const override;
+
 	virtual void get_parameter_list(List<Parameter> *r_parameters) override;
 
 	AudioStreamOggVorbis();

modules/vorbis/doc_classes/ResourceImporterOggVorbis.xml

@@ -44,10 +44,10 @@
 			If enabled, the audio will begin playing at the beginning after playback ends by reaching the end of the audio.
 			[b]Note:[/b] In [AudioStreamPlayer], the [signal AudioStreamPlayer.finished] signal won't be emitted for looping audio when it reaches the end of the audio file, as the audio will keep playing indefinitely.
 		</member>
-		<member name="loop_offset" type="float" setter="" getter="" default="0">
-			Determines where audio will start to loop after playback reaches the end of the audio. This can be used to only loop a part of the audio file, which is useful for some ambient sounds or music. The value is determined in seconds relative to the beginning of the audio. A value of [code]0.0[/code] will loop the entire audio file.
+		<member name="loop_offset_samples" type="int" setter="" getter="" default="0">
+			Determines where audio will start to loop after playback reaches the end of the audio. This can be used to only loop a part of the audio file, which is useful for some ambient sounds or music. The value is determined in samples relative to the beginning of the audio. A value of [code]0[/code] will loop the entire audio file.
 			Only has an effect if [member loop] is [code]true[/code].
-			A more convenient editor for [member loop_offset] is provided in the [b]Advanced Import Settings[/b] dialog, as it lets you preview your changes without having to reimport the audio.
+			A more convenient editor for [member loop_offset_samples] is provided in the [b]Advanced Import Settings[/b] dialog, as it lets you preview your changes without having to reimport the audio.
 		</member>
 	</members>
 </class>

modules/vorbis/resource_importer_ogg_vorbis.cpp

@@ -75,7 +75,7 @@ String ResourceImporterOggVorbis::get_preset_name(int p_idx) const {
 
 void ResourceImporterOggVorbis::get_import_options(const String &p_path, List<ImportOption> *r_options, int p_preset) const {
 	r_options->push_back(ImportOption(PropertyInfo(Variant::BOOL, "loop"), false));
-	r_options->push_back(ImportOption(PropertyInfo(Variant::FLOAT, "loop_offset"), 0));
+	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "loop_offset_samples"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::FLOAT, "bpm", PROPERTY_HINT_RANGE, "0,400,0.01,or_greater"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "beat_count", PROPERTY_HINT_RANGE, "0,512,or_greater"), 0));
 	r_options->push_back(ImportOption(PropertyInfo(Variant::INT, "bar_beats", PROPERTY_HINT_RANGE, "2,32,or_greater"), 4));
@@ -97,7 +97,6 @@ void ResourceImporterOggVorbis::show_advanced_options(const String &p_path) {
 
 Error ResourceImporterOggVorbis::import(const String &p_source_file, const String &p_save_path, const HashMap<StringName, Variant> &p_options, List<String> *r_platform_variants, List<String> *r_gen_files, Variant *r_metadata) {
 	bool loop = p_options["loop"];
-	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"];
@@ -108,7 +107,11 @@ Error ResourceImporterOggVorbis::import(const String &p_source_file, const Strin
 	}
 
 	ogg_vorbis_stream->set_loop(loop);
-	ogg_vorbis_stream->set_loop_offset(loop_offset);
+	if (p_options.has("loop_offset_samples")) {
+		ogg_vorbis_stream->set_loop_offset((double)p_options["loop_offset_samples"] / ogg_vorbis_stream->get_packet_sequence()->get_sampling_rate());
+	} else {
+		ogg_vorbis_stream->set_loop_offset(p_options["loop_offset"]);
+	}
 	ogg_vorbis_stream->set_bpm(bpm);
 	ogg_vorbis_stream->set_beat_count(beat_count);
 	ogg_vorbis_stream->set_bar_beats(bar_beats);

servers/audio/audio_stream.cpp

@@ -216,6 +216,12 @@ bool AudioStream::is_monophonic() const {
 	return ret;
 }
 
+int AudioStream::get_sampling_rate() const {
+	int ret = 0;
+	GDVIRTUAL_CALL(_get_sampling_rate, ret);
+	return ret;
+}
+
 double AudioStream::get_bpm() const {
 	double ret = 0;
 	GDVIRTUAL_CALL(_get_bpm, ret);
@@ -274,11 +280,13 @@ void AudioStream::get_parameter_list(List<Parameter> *r_parameters) {
 void AudioStream::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_length"), &AudioStream::get_length);
 	ClassDB::bind_method(D_METHOD("is_monophonic"), &AudioStream::is_monophonic);
+	ClassDB::bind_method(D_METHOD("get_sampling_rate"), &AudioStream::get_sampling_rate);
 	ClassDB::bind_method(D_METHOD("instantiate_playback"), &AudioStream::instantiate_playback);
 	GDVIRTUAL_BIND(_instantiate_playback);
 	GDVIRTUAL_BIND(_get_stream_name);
 	GDVIRTUAL_BIND(_get_length);
 	GDVIRTUAL_BIND(_is_monophonic);
+	GDVIRTUAL_BIND(_get_sampling_rate);
 	GDVIRTUAL_BIND(_get_bpm)
 	GDVIRTUAL_BIND(_get_beat_count)
 	GDVIRTUAL_BIND(_get_parameter_list)

servers/audio/audio_stream.h

@@ -127,6 +127,7 @@ class AudioStream : public Resource {
 	GDVIRTUAL0RC(String, _get_stream_name)
 	GDVIRTUAL0RC(double, _get_length)
 	GDVIRTUAL0RC(bool, _is_monophonic)
+	GDVIRTUAL0RC(int, _get_sampling_rate)
 	GDVIRTUAL0RC(double, _get_bpm)
 	GDVIRTUAL0RC(bool, _has_loop)
 	GDVIRTUAL0RC(int, _get_bar_beats)
@@ -144,6 +145,7 @@ class AudioStream : public Resource {
 
 	virtual double get_length() const;
 	virtual bool is_monophonic() const;
+	virtual int get_sampling_rate() const;
 
 	void tag_used(float p_offset);
 	uint64_t get_tagged_frame() const;