View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001286OpenMPTlibopenmptpublic2020-01-07 10:242020-01-07 14:50
ReporterSaga Musix Assigned ToSaga Musix  
PrioritynormalSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Target VersionOpenMPT 1.29.01.00 / libopenmpt 0.5.0 (upgrade first)Fixed in VersionOpenMPT 1.29.01.00 / libopenmpt 0.5.0 (upgrade first) 
Summary0001286: Provide real BPM estimation
Description

get_current_tempo does not always return real BPM, and computing the real BPM is not always possible based on the limited metadata libopenmpt exposes (rows per beat and tempo mode are missing). Maybe it would be a good idea to add a library function get_estimated_bpm (or similar) which simply forwards to CSoundFile::GetCurrentBPM, which already does all the calculations. Documentation should state that this function is not always accurate (most importantly because many oldskool formats do not store rows per beat).

For a use case, see https://forum.openmpt.org/index.php?topic=6241.msg46716#msg46716

TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Relationships

related to 0001287 acknowledgedmanx make libopenmpt samplerate non-variable 

Activities

Saga Musix

Saga Musix

2020-01-07 13:00

administrator   ~0004179

First implementation. Up for discussion: Change the default sample rate in MixerSettings::MixerSettings() from 44100 to 48000 for the case of estimating the tempo before the first tick is played, to make it consistent with the recommended default. This can be done outside of this patch, though.

estimate_bpm_v1.patch (5,045 bytes)    
Index: libopenmpt/bindings/freebasic/libopenmpt.bi
===================================================================
--- libopenmpt/bindings/freebasic/libopenmpt.bi	(revision 12451)
+++ libopenmpt/bindings/freebasic/libopenmpt.bi	(working copy)
@@ -1055,6 +1055,15 @@
 '/
 Declare Function openmpt_module_get_metadata_ Alias "openmpt_module_get_metadata" (ByVal module As openmpt_module Ptr, ByVal key As Const ZString Ptr) As Const ZString Ptr
 
+/'* \brief Get the current estimated beats per minute (BPM).
+
+  \param module The module handle to work on.
+  \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+  \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+  \return The current estimated BPM.
+'/
+Declare Function openmpt_module_get_estimated_bpm(ByVal module As openmpt_module Ptr) As Double
+
 /'* \brief Get the current speed
 
   \param module The module handle to work on.
Index: libopenmpt/libopenmpt.h
===================================================================
--- libopenmpt/libopenmpt.h	(revision 12451)
+++ libopenmpt/libopenmpt.h	(working copy)
@@ -1140,6 +1140,14 @@
  */
 LIBOPENMPT_API const char * openmpt_module_get_metadata( openmpt_module * mod, const char * key );
 
+/*! Get the current estimated beats per minute (BPM).
+ *
+ * \param mod The module handle to work on.
+ * \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+ * \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+ * \return The current estimated BPM.
+ */
+double openmpt_module_get_estimated_bpm( openmpt_module * mod );
 /*! \brief Get the current speed
  *
  * \param mod The module handle to work on.
Index: libopenmpt/libopenmpt.hpp
===================================================================
--- libopenmpt/libopenmpt.hpp	(revision 12451)
+++ libopenmpt/libopenmpt.hpp	(working copy)
@@ -815,6 +815,13 @@
 	*/
 	std::string get_metadata( const std::string & key ) const;
 
+	//! Get the current estimated beats per minute (BPM).
+	/*!
+	  \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+	  \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+	  \return The current estimated BPM.
+	*/
+	double get_estimated_bpm() const;
 	//! Get the current speed
 	/*!
 	  \return The current speed in ticks per row.
Index: libopenmpt/libopenmpt_c.cpp
===================================================================
--- libopenmpt/libopenmpt_c.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_c.cpp	(working copy)
@@ -924,6 +924,15 @@
 	return NULL;
 }
 
+LIBOPENMPT_API double openmpt_module_get_estimated_bpm( openmpt_module * mod ) {
+	try {
+		openmpt::interface::check_soundfile( mod );
+		return mod->impl->get_estimated_bpm();
+	} catch ( ... ) {
+		openmpt::report_exception( __func__, mod );
+	}
+	return 0.0;
+}
 LIBOPENMPT_API int32_t openmpt_module_get_current_speed( openmpt_module * mod ) {
 	try {
 		openmpt::interface::check_soundfile( mod );
Index: libopenmpt/libopenmpt_cxx.cpp
===================================================================
--- libopenmpt/libopenmpt_cxx.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_cxx.cpp	(working copy)
@@ -305,6 +305,9 @@
 	return impl->get_metadata( key );
 }
 
+double module::get_estimated_bpm() const {
+	return impl->get_estimated_bpm();
+}
 std::int32_t module::get_current_speed() const {
 	return impl->get_current_speed();
 }
Index: libopenmpt/libopenmpt_impl.cpp
===================================================================
--- libopenmpt/libopenmpt_impl.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_impl.cpp	(working copy)
@@ -1275,6 +1275,9 @@
 	return "";
 }
 
+double module_impl::get_estimated_bpm() const {
+	return m_sndFile->GetCurrentBPM();
+}
 std::int32_t module_impl::get_current_speed() const {
 	return m_sndFile->m_PlayState.m_nMusicSpeed;
 }
Index: libopenmpt/libopenmpt_impl.hpp
===================================================================
--- libopenmpt/libopenmpt_impl.hpp	(revision 12451)
+++ libopenmpt/libopenmpt_impl.hpp	(working copy)
@@ -198,6 +198,7 @@
 	std::size_t read_interleaved_quad( std::int32_t samplerate, std::size_t count, float * interleaved_quad );
 	std::vector<std::string> get_metadata_keys() const;
 	std::string get_metadata( const std::string & key ) const;
+	double get_estimated_bpm() const;
 	std::int32_t get_current_speed() const;
 	std::int32_t get_current_tempo() const;
 	std::int32_t get_current_order() const;
estimate_bpm_v1.patch (5,045 bytes)   
manx

manx

2020-01-07 14:01

administrator   ~0004180

First implementation.

I would prefer "get_current_estimated_bpm" as the API name, because it emphasizes the fact that this value can change over time. Looks good otherwise.

Up for discussion: Change the default sample rate in MixerSettings::MixerSettings() from 44100 to 48000 for the case of estimating the tempo before the first tick is played, to make it consistent with the recommended default. This can be done outside of this patch, though.

Very likely a good idea. Would there be any implications for OpenMPT doing that? (sound devices default to 48000 for a long time (1.23.03 / 2014-05) already anyway).
Saga Musix

Saga Musix

2020-01-07 14:35

administrator   ~0004181

New version with renamed function and changing the default sample rate.

Would there be any implications for OpenMPT doing that?

There shouldn't be any changes because the current audio parameters are applied instantly in the CModDoc constructor, long before calling CSoundFile::Create.

estimate_bpm_v2.patch (5,455 bytes)    
Index: libopenmpt/bindings/freebasic/libopenmpt.bi
===================================================================
--- libopenmpt/bindings/freebasic/libopenmpt.bi	(revision 12451)
+++ libopenmpt/bindings/freebasic/libopenmpt.bi	(working copy)
@@ -1055,6 +1055,15 @@
 '/
 Declare Function openmpt_module_get_metadata_ Alias "openmpt_module_get_metadata" (ByVal module As openmpt_module Ptr, ByVal key As Const ZString Ptr) As Const ZString Ptr
 
+/'* \brief Get the current estimated beats per minute (BPM).
+
+  \param module The module handle to work on.
+  \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+  \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+  \return The current estimated BPM.
+'/
+Declare Function openmpt_module_get_current_estimated_bpm(ByVal module As openmpt_module Ptr) As Double
+
 /'* \brief Get the current speed
 
   \param module The module handle to work on.
Index: libopenmpt/libopenmpt.h
===================================================================
--- libopenmpt/libopenmpt.h	(revision 12451)
+++ libopenmpt/libopenmpt.h	(working copy)
@@ -1140,6 +1140,14 @@
  */
 LIBOPENMPT_API const char * openmpt_module_get_metadata( openmpt_module * mod, const char * key );
 
+/*! Get the current estimated beats per minute (BPM).
+ *
+ * \param mod The module handle to work on.
+ * \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+ * \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+ * \return The current estimated BPM.
+ */
+double openmpt_module_get_current_estimated_bpm( openmpt_module * mod );
 /*! \brief Get the current speed
  *
  * \param mod The module handle to work on.
Index: libopenmpt/libopenmpt.hpp
===================================================================
--- libopenmpt/libopenmpt.hpp	(revision 12451)
+++ libopenmpt/libopenmpt.hpp	(working copy)
@@ -815,6 +815,13 @@
 	*/
 	std::string get_metadata( const std::string & key ) const;
 
+	//! Get the current estimated beats per minute (BPM).
+	/*!
+	  \remarks Many module formats lack time signature metadata. It is common that this estimate is off by a factor of two, but other multipliers are also possible.
+	  \remarks Due to the nature of how module tempo works, the estimate may change slightly after switching libopenmpt's output to a different sample rate.
+	  \return The current estimated BPM.
+	*/
+	double get_current_estimated_bpm() const;
 	//! Get the current speed
 	/*!
 	  \return The current speed in ticks per row.
Index: libopenmpt/libopenmpt_c.cpp
===================================================================
--- libopenmpt/libopenmpt_c.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_c.cpp	(working copy)
@@ -924,6 +924,15 @@
 	return NULL;
 }
 
+LIBOPENMPT_API double openmpt_module_get_current_estimated_bpm( openmpt_module * mod ) {
+	try {
+		openmpt::interface::check_soundfile( mod );
+		return mod->impl->get_current_estimated_bpm();
+	} catch ( ... ) {
+		openmpt::report_exception( __func__, mod );
+	}
+	return 0.0;
+}
 LIBOPENMPT_API int32_t openmpt_module_get_current_speed( openmpt_module * mod ) {
 	try {
 		openmpt::interface::check_soundfile( mod );
Index: libopenmpt/libopenmpt_cxx.cpp
===================================================================
--- libopenmpt/libopenmpt_cxx.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_cxx.cpp	(working copy)
@@ -305,6 +305,9 @@
 	return impl->get_metadata( key );
 }
 
+double module::get_current_estimated_bpm() const {
+	return impl->get_current_estimated_bpm();
+}
 std::int32_t module::get_current_speed() const {
 	return impl->get_current_speed();
 }
Index: libopenmpt/libopenmpt_impl.cpp
===================================================================
--- libopenmpt/libopenmpt_impl.cpp	(revision 12451)
+++ libopenmpt/libopenmpt_impl.cpp	(working copy)
@@ -1275,6 +1275,9 @@
 	return "";
 }
 
+double module_impl::get_current_estimated_bpm() const {
+	return m_sndFile->GetCurrentBPM();
+}
 std::int32_t module_impl::get_current_speed() const {
 	return m_sndFile->m_PlayState.m_nMusicSpeed;
 }
Index: libopenmpt/libopenmpt_impl.hpp
===================================================================
--- libopenmpt/libopenmpt_impl.hpp	(revision 12451)
+++ libopenmpt/libopenmpt_impl.hpp	(working copy)
@@ -198,6 +198,7 @@
 	std::size_t read_interleaved_quad( std::int32_t samplerate, std::size_t count, float * interleaved_quad );
 	std::vector<std::string> get_metadata_keys() const;
 	std::string get_metadata( const std::string & key ) const;
+	double get_current_estimated_bpm() const;
 	std::int32_t get_current_speed() const;
 	std::int32_t get_current_tempo() const;
 	std::int32_t get_current_order() const;
Index: soundlib/MixerSettings.cpp
===================================================================
--- soundlib/MixerSettings.cpp	(revision 12451)
+++ soundlib/MixerSettings.cpp	(working copy)
@@ -26,7 +26,7 @@
 
 	// Mixing Configuration
 	gnChannels = 2;
-	gdwMixingFreq = 44100;
+	gdwMixingFreq = 48000;
 
 	m_nPreAmp = 128;
estimate_bpm_v2.patch (5,455 bytes)   
manx

manx

2020-01-07 14:43

administrator   ~0004182

Looks good.
manx

manx

2020-01-07 14:50

administrator   ~0004183

r12452 r12453 r12454

Issue History

Date Modified Username Field Change
2020-01-07 10:24 Saga Musix New Issue
2020-01-07 10:26 manx Target Version => OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first)
2020-01-07 10:54 manx Relationship added related to 0001287
2020-01-07 13:00 Saga Musix Note Added: 0004179
2020-01-07 13:00 Saga Musix File Added: estimate_bpm_v1.patch
2020-01-07 13:01 Saga Musix Target Version OpenMPT 1.30.01.00 / libopenmpt 0.6.0 (upgrade first) => OpenMPT 1.29.01.00 / libopenmpt 0.5.0 (upgrade first)
2020-01-07 14:01 manx Note Added: 0004180
2020-01-07 14:35 Saga Musix Note Added: 0004181
2020-01-07 14:35 Saga Musix File Added: estimate_bpm_v2.patch
2020-01-07 14:43 manx Note Added: 0004182
2020-01-07 14:50 manx Assigned To => Saga Musix
2020-01-07 14:50 manx Status new => resolved
2020-01-07 14:50 manx Resolution open => fixed
2020-01-07 14:50 manx Fixed in Version => OpenMPT 1.29.01.00 / libopenmpt 0.5.0 (upgrade first)
2020-01-07 14:50 manx Note Added: 0004183