From 07229b64048c118bd8a25904c2cbc1a603fad838 Mon Sep 17 00:00:00 2001
From: Alexandre Almeida <wilsalx@gmail.com>
Date: Thu, 21 Sep 2023 20:18:23 -0300
Subject: [PATCH] Show error message when loading an invalid sample file
 (#6286)

---
 include/SampleClip.h      |  2 +-
 src/core/SampleBuffer.cpp | 62 +++++++++++++++++++++++++++++++--------
 src/core/SampleClip.cpp   | 29 ++++++++++--------
 3 files changed, 66 insertions(+), 27 deletions(-)

diff --git a/include/SampleClip.h b/include/SampleClip.h
index c9e247328..5246787bd 100644
--- a/include/SampleClip.h
+++ b/include/SampleClip.h
@@ -77,7 +77,7 @@ public:
 
 public slots:
 	void setSampleBuffer( lmms::SampleBuffer* sb );
-	void setSampleFile( const QString & _sf );
+	void setSampleFile( const QString & sf );
 	void updateLength();
 	void toggleRecord();
 	void playbackPositionChanged();
diff --git a/src/core/SampleBuffer.cpp b/src/core/SampleBuffer.cpp
index 775db125b..5e2d09c57 100644
--- a/src/core/SampleBuffer.cpp
+++ b/src/core/SampleBuffer.cpp
@@ -247,7 +247,15 @@ void SampleBuffer::update(bool keepSettings)
 	const int fileSizeMax = 300; // MB
 	const int sampleLengthMax = 90; // Minutes
 
-	bool fileLoadError = false;
+	enum class FileLoadError
+	{
+		None,
+		ReadPermissionDenied,
+		TooLarge,
+		Invalid
+	};
+	FileLoadError fileLoadError = FileLoadError::None;
+
 	if (m_audioFile.isEmpty() && m_origData != nullptr && m_origFrames > 0)
 	{
 		// TODO: reverse- and amplification-property is not covered
@@ -271,31 +279,40 @@ void SampleBuffer::update(bool keepSettings)
 		m_frames = 0;
 
 		const QFileInfo fileInfo(file);
-		if (fileInfo.size() > fileSizeMax * 1024 * 1024)
+		if (!fileInfo.isReadable())
 		{
-			fileLoadError = true;
+			fileLoadError = FileLoadError::ReadPermissionDenied;
+		}
+		else if (fileInfo.size() > fileSizeMax * 1024 * 1024)
+		{
+			fileLoadError = FileLoadError::TooLarge;
 		}
 		else
 		{
 			// Use QFile to handle unicode file names on Windows
 			QFile f(file);
-			SNDFILE * sndFile;
+			SNDFILE * sndFile = nullptr;
 			SF_INFO sfInfo;
 			sfInfo.format = 0;
+
 			if (f.open(QIODevice::ReadOnly) && (sndFile = sf_open_fd(f.handle(), SFM_READ, &sfInfo, false)))
 			{
 				f_cnt_t frames = sfInfo.frames;
 				int rate = sfInfo.samplerate;
 				if (frames / rate > sampleLengthMax * 60)
 				{
-					fileLoadError = true;
+					fileLoadError = FileLoadError::TooLarge;
 				}
 				sf_close(sndFile);
 			}
+			else
+			{
+				fileLoadError = FileLoadError::Invalid;
+			}
 			f.close();
 		}
 
-		if (!fileLoadError)
+		if (fileLoadError == FileLoadError::None)
 		{
 #ifdef LMMS_HAVE_OGGVORBIS
 			// workaround for a bug in libsndfile or our libsndfile decoder
@@ -322,7 +339,7 @@ void SampleBuffer::update(bool keepSettings)
 			}
 		}
 
-		if (m_frames == 0 || fileLoadError)  // if still no frames, bail
+		if (m_frames == 0 || fileLoadError != FileLoadError::None)  // if still no frames, bail
 		{
 			// sample couldn't be decoded, create buffer containing
 			// one sample-frame
@@ -363,16 +380,35 @@ void SampleBuffer::update(bool keepSettings)
 	}
 	Oscillator::generateAntiAliasUserWaveTable(this);
 
-	if (fileLoadError)
+	if (fileLoadError != FileLoadError::None)
 	{
 		QString title = tr("Fail to open file");
-		QString message = tr("Audio files are limited to %1 MB "
-				"in size and %2 minutes of playing time"
-				).arg(fileSizeMax).arg(sampleLengthMax);
+		QString message;
+
+		switch (fileLoadError)
+		{
+			case FileLoadError::None:
+				// present just to avoid a compiler warning
+				break;
+
+			case FileLoadError::ReadPermissionDenied:
+				message = tr("Read permission denied");
+				break;
+
+			case FileLoadError::TooLarge:
+				message = tr("Audio files are limited to %1 MB "
+					"in size and %2 minutes of playing time"
+					).arg(fileSizeMax).arg(sampleLengthMax);
+				break;
+
+			case FileLoadError::Invalid:
+				message = tr("Invalid audio file");
+				break;
+		}
+
 		if (gui::getGUI() != nullptr)
 		{
-			QMessageBox::information(nullptr,
-				title, message,	QMessageBox::Ok);
+			QMessageBox::information(nullptr, title, message, QMessageBox::Ok);
 		}
 		else
 		{
diff --git a/src/core/SampleClip.cpp b/src/core/SampleClip.cpp
index 592a63827..b09d7b3bb 100644
--- a/src/core/SampleClip.cpp
+++ b/src/core/SampleClip.cpp
@@ -143,23 +143,26 @@ void SampleClip::setSampleBuffer( SampleBuffer* sb )
 
 
 
-void SampleClip::setSampleFile( const QString & _sf )
+void SampleClip::setSampleFile(const QString & sf)
 {
-	int length;
-	if ( _sf.isEmpty() )
-	{	//When creating an empty sample clip make it a bar long
-		float nom = Engine::getSong()->getTimeSigModel().getNumerator();
-		float den = Engine::getSong()->getTimeSigModel().getDenominator();
-		length = DefaultTicksPerBar * ( nom / den );
-	}
-	else
-	{	//Otherwise set it to the sample's length
-		m_sampleBuffer->setAudioFile( _sf );
+	int length = 0;
+
+	if (!sf.isEmpty())
+	{
+		m_sampleBuffer->setAudioFile(sf);
 		length = sampleLength();
 	}
-	changeLength(length);
 
-	setStartTimeOffset( 0 );
+	if (length == 0)
+	{
+		//If there is no sample, make the clip a bar long
+		float nom = Engine::getSong()->getTimeSigModel().getNumerator();
+		float den = Engine::getSong()->getTimeSigModel().getDenominator();
+		length = DefaultTicksPerBar * (nom / den);
+	}
+
+	changeLength(length);
+	setStartTimeOffset(0);
 
 	emit sampleChanged();
 	emit playbackPositionChanged();