Add MP3 import

[LostRobotMusic]

Allows AudioFileProcessor and Sample Tracks to load MP3 files. Finally.

Fixes #2000

[Veratil]

We'll want to make sure that libsndfile is >=1.1.0 for this like @PhysSong mentioned.

[sakertooth]

Adding support for MP3 was really just a handful of lines. Sorry if I threw you off by saying there might be a merge conflict 😅 (since I moved SampleBuffer::openAudioFile into its own class, there might be, but it would be be extremely simple to resolve).

[LostRobotMusic]

Alright, as @sakertooth pointed out, apparently LMMS already had MP3 import capabilities this whole time. It just... wasn't enabled. I guess. Kind of at a loss for words, I'll admit.
So I've deleted everything in the PR except the part that enables MP3 import.

[Veratil]

Too easy 😂

[sakertooth]

I still think the CMake changes are necessary though, because we still need to ensure the version for sndfile is sndfile>=1.1.0, in the event that someone is building with an outdated version.

[sakertooth]

Alright, as @sakertooth pointed out, apparently LMMS already had MP3 import capabilities this whole time. It just... wasn't enabled. I guess. Kind of at a loss for words, I'll admit. So I've deleting everything in the PR except the part that enables MP3 import.

We only got MP3 import because it was added to sndfile in v1.1.0, and we were already using sndfile to begin with to decode audio files from disk, so increasing our sndfile version to v1.1.0 was the majority of the work for adding MP3.

[LostRobotMusic]

I still think the CMake changes are necessary though, because we still need to ensure the version for sndfile is sndfile>=1.1.0, in the event that someone is building with an outdated version.

Do we need #ifdefs anywhere? Or are we only checking the version so we can output a warning when the version's too low?

[Veratil]

I still think the CMake changes are necessary though, because we still need to ensure the version for sndfile is sndfile>=1.1.0, in the event that someone is building with an outdated version.

Do we need #ifdefs anywhere? Or are we only checking the version so we can output a warning when the version's too low?

Yes, we should add some. 1.1.0 was released in 2022 so not every distro will have it.

[LostRobotMusic]

@Veratil I meant to ask where the #ifdefs should be placed. I notice OGG is still kept in the supported files list even when the user doesn't have OGG support. Should MP3 be treated differently?

[Veratil]

@Veratil I meant to ask where the #ifdefs should be placed. I notice OGG is still kept in the supported files list even when the user doesn't have OGG support. Should MP3 be treated differently?

Well that should be fixed actually.

[sakertooth]

@Veratil I meant to ask where the #ifdefs should be placed. I notice OGG is still kept in the supported files list even when the user doesn't have OGG support. Should MP3 be treated differently?

I would think OGG support is available for the majority, if not all users, no?

[LostRobotMusic]

Alright, I added all the needed checks for MP3 support.

[Veratil]

I would think OGG support is available for the majority, if not all users, no?

Probably, if using libsndfile for it then it's been there since 2009. But if for some reason we have a build without support we shouldn't advertise the extension.

[superpaik]

I think both mingw and msvc in the CI are build with libsndfile 1.0.28 so it can't be tested.
LMMS_SNDFILE_MP3 is set to false, though, so mp3 is correctly not available as file type.

[Rossmaxx]

I think both mingw and msvc in the CI are build with libsndfile 1.0.28 so it can't be tested.

It's just mingw. MSVC uses the latest versions available on vcpkg (it's 1.2.0 for vckpg)

[superpaik]

It's just mingw. MSVC uses the latest versions available on vcpkg (it's 1.2.0 for vckpg)

it's not working to me. MSVC comes with sndfile.dll (which is 1.2.0 and has as product name libsndfile-1.dll). Maybe that is why is not working.

File type options:

sndfile.dll properties:

[sakertooth]

I think both mingw and msvc in the CI are build with libsndfile 1.0.28 so it can't be tested. LMMS_SNDFILE_MP3 is set to false, though, so mp3 is correctly not available as file type.

Try force setting it to true to see if it you can still decode MP3s successfully, or if you end up crashing or it fails to load. That'll tell us its at least sndfile>=1.1.0.

[tresf]

I think both mingw and msvc in the CI are build with libsndfile 1.0.28 so it can't be tested. LMMS_SNDFILE_MP3 is set to false, though, so mp3 is correctly not available as file type.

Try force setting it to true to see if it you can still decode MP3s successfully, or if you end up crashing or it fails to load. That'll tell us its at least sndfile>=1.1.0.

I think 1.0.28 is still the current version for mingw builds, per https://launchpad.net/~tobydox/+archive/ubuntu/mingw-w64. We can ping @tobydox to see if there's a newer version available.

[superpaik]

Try force setting it to true to see if it you can still decode MP3s successfully, or if you end up crashing or it fails to load. That'll tell us its at least sndfile>=1.1.0.

I don't know how to force setting the variable in the CI. I've tried to build it locally but it doesn't work, but I not sure if the problem is with my local environment and not sndfile. Maybe someone can push a commit without checking the version. Or maybe the problem is related to what PhysSong said about it before.

[tresf]

I don't know how to force setting the variable in the CI. I've tried to build it locally but it doesn't work, but I not sure if the problem is with my local environment and not sndfile. Maybe someone can push a commit without checking the version. Or maybe the problem is related to what PhysSong said about it before.

Perhaps:

IF(SNDFILE_FOUND)
+       SET(SNDFILE_VERSION "1.2.0") # TODO: REMOVE THIS LINE!
	IF(SNDFILE_VERSION VERSION_GREATER_EQUAL 1.1.0)
		SET(LMMS_SNDFILE_MP3 TRUE)
	ELSE()
		MESSAGE("libsndfile version is < 1.1.0; MP3 import disabled")
		SET(LMMS_SNDFILE_MP3 FALSE)
	ENDIF()
ELSE()

[Gabrielxd195]

Sorry, this looks merged but I still can't find a way to import audio in mp3 format, at least in the appimage, has this been forgotten?

[Rossmaxx]

Linux appimage doesn't support mp3 import due to outdated libs used by the CI. If you build it yourself, you can use mp3, unless you are on ubuntu/derivatives

[Gabrielxd195]

Linux appimage doesn't support mp3 import due to outdated libs used by the CI. If you build it yourself, you can use mp3, unless you are on ubuntu/derivatives

But these obsolete libraries will not be updated? Do you think this prevents other improvements from being implemented in the future? I know I can build it myself, but believe me, the average user doesn't even know the most basic of those code things.

[messmerd]

@Gabrielxd195 Linux AppImages will have mp3 and opus support once #7316 is merged.

[Gabrielxd195]

@Gabrielxd195 Linux AppImages will have mp3 and opus support once #7316 is merged.

Thank you very much, users will be happy with this improvement.

[bratpeki]

Trying MP3 out with #7526 (comment).

Works great on Windows when compiled with MSYS2 UCRT64 and MINGW64.