Fixing regression inside AudioWaveformer so it uses a proper detached reader, and no longer mutates the Clip's reader (preventing a bug which caused our video to disappear when waveforms were used on the clip)

src/AudioWaveformer.cpp

@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <chrono>
+#include <memory>
 #include <thread>
 #include <vector>
 
@@ -31,7 +32,11 @@ using namespace openshot;
 
 
 // Default constructor
-AudioWaveformer::AudioWaveformer(ReaderBase* new_reader) : reader(new_reader)
+AudioWaveformer::AudioWaveformer(ReaderBase* new_reader) :
+	reader(new_reader),
+	detached_reader(nullptr),
+	resolved_reader(nullptr),
+	source_initialized(false)
 {
 
 }
@@ -50,7 +55,8 @@ AudioWaveformData AudioWaveformer::ExtractSamples(int channel, int num_per_secon
 		return data;
 	}
 
-	ReaderBase* source = ResolveSourceReader(reader);
+	ReaderBase* source = ResolveWaveformReader();
+
 	Fraction source_fps = ResolveSourceFPS(source);
 
 	AudioWaveformData base = ExtractSamplesFromReader(source, channel, num_per_second, false);
@@ -212,12 +218,9 @@ AudioWaveformData AudioWaveformer::ExtractSamplesFromReader(ReaderBase* source_r
 	}
 
 	// Open reader (if needed)
-	bool does_reader_have_video = source_reader->info.has_video;
 	if (!source_reader->IsOpen()) {
 		source_reader->Open();
 	}
-	// Disable video for faster processing
-	source_reader->info.has_video = false;
 
 	const auto retry_delay = std::chrono::milliseconds(100);
 	const auto max_wait_for_open = std::chrono::milliseconds(3000);
@@ -270,18 +273,15 @@ AudioWaveformData AudioWaveformer::ExtractSamplesFromReader(ReaderBase* source_r
 	}
 
 	if (!source_reader->info.has_audio) {
-		source_reader->info.has_video = does_reader_have_video;
 		return data;
 	}
 
 	int total_samples = static_cast<int>(std::ceil(reader_duration * num_per_second));
 	if (total_samples <= 0 || source_reader->info.channels == 0) {
-		source_reader->info.has_video = does_reader_have_video;
 		return data;
 	}
 
 	if (channel != -1 && (channel < 0 || channel >= source_reader->info.channels)) {
-		source_reader->info.has_video = does_reader_have_video;
 		return data;
 	}
 
@@ -348,7 +348,6 @@ AudioWaveformData AudioWaveformer::ExtractSamplesFromReader(ReaderBase* source_r
 			}
 		}
 	} catch (...) {
-		source_reader->info.has_video = does_reader_have_video;
 		throw;
 	}
 
@@ -369,8 +368,6 @@ AudioWaveformData AudioWaveformer::ExtractSamplesFromReader(ReaderBase* source_r
 		data.scale(total_samples, scale);
 	}
 
-	source_reader->info.has_video = does_reader_have_video;
-
 	return data;
 }
 
@@ -400,3 +397,34 @@ Fraction AudioWaveformer::ResolveSourceFPS(ReaderBase* source_reader) {
 	}
 	return source_reader->info.fps;
 }
+
+// Resolve and cache the reader used for waveform extraction (prefer a detached FFmpegReader clone)
+ReaderBase* AudioWaveformer::ResolveWaveformReader() {
+	if (source_initialized) {
+		return resolved_reader ? resolved_reader : reader;
+	}
+	source_initialized = true;
+
+	resolved_reader = ResolveSourceReader(reader);
+
+	// Prefer a detached, audio-only FFmpegReader clone so we never mutate the live reader used for preview.
+	if (auto ff_reader = dynamic_cast<FFmpegReader*>(resolved_reader)) {
+		const Json::Value ff_json = ff_reader->JsonValue();
+		const std::string path = ff_json.get("path", "").asString();
+		if (!path.empty()) {
+			try {
+				auto clone = std::make_unique<FFmpegReader>(path, false);
+				clone->SetJsonValue(ff_json);
+				clone->info.has_video = false; // explicitly audio-only for waveform extraction
+				detached_reader = std::move(clone);
+				resolved_reader = detached_reader.get();
+			} catch (...) {
+				// Fall back to using the original reader if cloning fails
+				detached_reader.reset();
+				resolved_reader = ResolveSourceReader(reader);
+			}
+		}
+	}
+
+	return resolved_reader ? resolved_reader : reader;
+}

src/AudioWaveformer.h

@@ -17,6 +17,7 @@
 #include "Frame.h"
 #include "KeyFrame.h"
 #include "Fraction.h"
+#include <memory>
 #include <vector>
 #include <string>
 
@@ -82,6 +83,9 @@ namespace openshot {
     class AudioWaveformer {
     private:
         ReaderBase* reader;
+        std::unique_ptr<ReaderBase> detached_reader; ///< Optional detached reader clone for waveform extraction
+        ReaderBase* resolved_reader = nullptr;       ///< Cached pointer to the reader used for extraction
+        bool source_initialized = false;
 
     public:
         /// Default constructor
@@ -123,6 +127,7 @@ namespace openshot {
         AudioWaveformData ExtractSamplesFromReader(openshot::ReaderBase* source_reader, int channel, int num_per_second, bool normalize);
         openshot::ReaderBase* ResolveSourceReader(openshot::ReaderBase* source_reader);
         openshot::Fraction ResolveSourceFPS(openshot::ReaderBase* source_reader);
+        openshot::ReaderBase* ResolveWaveformReader();
     };
 
 }

tests/AudioWaveformer.cpp

@@ -138,6 +138,31 @@ TEST_CASE( "Channel selection returns data and rejects invalid channel", "[libop
 	r.Close();
 }
 
+TEST_CASE( "Waveform extraction does not mutate source reader video flag", "[libopenshot][audiowaveformer][mutation]" )
+{
+	std::stringstream path;
+	path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4";
+	FFmpegReader reader(path.str());
+	Clip clip(&reader);
+	clip.Open();
+
+	const bool original_has_video_clip = clip.Reader()->info.has_video;
+	const bool original_has_video_reader = reader.info.has_video;
+	REQUIRE(original_has_video_clip == original_has_video_reader);
+	REQUIRE(original_has_video_reader);
+
+	AudioWaveformer waveformer(&clip);
+	AudioWaveformData waveform = waveformer.ExtractSamples(-1, 5, false);
+
+	// Extraction should not flip has_video on the live reader/clip
+	CHECK_FALSE(waveform.rms_samples.empty());
+	CHECK(clip.Reader()->info.has_video == original_has_video_clip);
+	CHECK(reader.info.has_video == original_has_video_reader);
+
+	clip.Close();
+	reader.Close();
+}
+
 
 TEST_CASE( "Extract waveform waits for reader reopen", "[libopenshot][audiowaveformer][stability]" )
 {
@@ -166,7 +191,7 @@ TEST_CASE( "Extract waveform waits for reader reopen", "[libopenshot][audiowavef
 	reader.Close();
 }
 
-TEST_CASE( "Extract waveform times out when reader stays closed", "[libopenshot][audiowaveformer][stability]" )
+TEST_CASE( "Extract waveform continues if caller closes original reader", "[libopenshot][audiowaveformer][stability]" )
 {
 	std::stringstream path;
 	path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4";
@@ -180,19 +205,13 @@ TEST_CASE( "Extract waveform times out when reader stays closed", "[libopenshot]
 		return waveformer.ExtractSamples(-1, samples_per_second, false);
 	});
 
+	// Closing the caller's reader should not affect a detached clone used for waveform extraction.
 	std::this_thread::sleep_for(std::chrono::milliseconds(50));
 	reader.Close();
 
-	const auto start = std::chrono::steady_clock::now();
-	try {
-		(void) future_waveform.get();
-		FAIL("Expected ReaderClosed to be thrown after timeout");
-	} catch (const openshot::ReaderClosed&) {
-		const auto elapsed = std::chrono::steady_clock::now() - start;
-		const auto elapsed_ms = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed);
-		CHECK(elapsed_ms.count() >= 2900);
-		CHECK(elapsed_ms.count() < 4500);
-	}
+	AudioWaveformData waveform;
+	REQUIRE_NOTHROW(waveform = future_waveform.get());
+	CHECK_FALSE(waveform.rms_samples.empty());
 
 	reader.Close();
 }