fix: end a recording cleanly when its writer thread throws

docs/guide/media/media-recording.md

@@ -104,6 +104,8 @@ Calls arrive in order on a thread the recorder keeps for them, so a listener may
 
 `onError` also hears of a file that cannot be finished or closed when the recorder stops, e.g. because the last data does not fit on the disk. The recording then ends as `PARTIAL`.
 
+Running out of native memory while writing ends the recording the same way: `onError` is told, the frames still waiting are dropped, and the file is finished right away, so what is in it still plays. The recording ends as `PARTIAL`, or as `FAILED` if nothing had gone into the file yet.
+
 ## How Tracks Are Recorded
 
 - **Video starts at a key frame.** A decoder can do nothing with the frames before one, so they are skipped. The recorder asks the sender or receiver for a key frame, so recording starts within moments instead of waiting until WebRTC sends one of its own accord, which it rarely does.

webrtc-media/src/main/cpp/include/JNI_AllocationFault.h

@@ -46,6 +46,14 @@ extern "C" {
 	JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_player_AllocationFault_failNextThreadStart
 	(JNIEnv *, jclass);
 
+	/*
+	 * Class:     dev_onvoid_webrtc_media_player_AllocationFault
+	 * Method:    failWriterAllocation
+	 * Signature: (I)V
+	 */
+	JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_player_AllocationFault_failWriterAllocation
+	(JNIEnv *, jclass, jint);
+
 	/*
 	 * Class:     dev_onvoid_webrtc_media_player_AllocationFault
 	 * Method:    isArmed

webrtc-media/src/main/cpp/include/media/AllocationFault.h

@@ -35,6 +35,9 @@ namespace ffmpeg
 			// Fails the next thread start that is checked, once, as one does
 			// that the system has no memory for.
 			static void FailNextThreadStart();
+			// Fails an allocation of a recorder's writer thread, once, after
+			// it has handled the given number of frames.
+			static void FailWriterAllocation(int after_frames);
 			// Forgets a fault that has not fired.
 			static void Disarm();
 			// Whether a fault is armed that has not fired.
@@ -45,6 +48,9 @@ namespace ffmpeg
 			static void CheckAllocation();
 			// Throws std::system_error if a fault is armed, and disarms it.
 			static void CheckThreadStart();
+			// Throws std::bad_alloc if a writer fault is armed and its frames
+			// are counted down, and disarms it.
+			static void CheckWriterAllocation();
 	};
 }
 

webrtc-media/src/main/cpp/include/media/MediaRecorder.h

@@ -141,7 +141,15 @@ namespace ffmpeg
 			// one.
 			void RestartAtKeyFrame(int track);
 
+			// The entry of the writer thread, which runs Record() and lets no
+			// exception out of it.
 			void Run();
+			void Record();
+
+			// Ends a recording that Record() left by an exception: fails it,
+			// lets go of what waits to be written, reports it and finishes
+			// the file. Only to be called from a catch block.
+			void Abort() noexcept;
 
 			void Handle(RecordedFrame frame);
 			void Write(RecordedFrame & frame);
@@ -165,6 +173,9 @@ namespace ffmpeg
 
 			const webrtc_java_api * api_;
 			const std::string path_;
+			// Made in advance, since a writer that ran out of memory may not
+			// get any to describe that.
+			const std::string out_of_memory_error_;
 
 			std::unique_ptr<MediaRecorderObserver> observer_;
 

webrtc-media/src/main/cpp/src/JNI_AllocationFault.cpp

@@ -42,6 +42,12 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_player_AllocationFault_failN
 	ffmpeg::AllocationFault::FailNextThreadStart();
 }
 
+JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_player_AllocationFault_failWriterAllocation
+(JNIEnv * env, jclass caller, jint afterFrames)
+{
+	ffmpeg::AllocationFault::FailWriterAllocation(afterFrames);
+}
+
 JNIEXPORT jboolean JNICALL Java_dev_onvoid_webrtc_media_player_AllocationFault_isArmed
 (JNIEnv * env, jclass caller)
 {

webrtc-media/src/main/cpp/src/media/AllocationFault.cpp

@@ -25,6 +25,8 @@ namespace
 	std::atomic<bool> fail_string_copy{ false };
 	std::atomic<bool> fail_allocation{ false };
 	std::atomic<bool> fail_thread_start{ false };
+	std::atomic<bool> fail_writer_allocation{ false };
+	std::atomic<int> writer_frames_left{ 0 };
 }
 
 namespace ffmpeg
@@ -44,16 +46,24 @@ namespace ffmpeg
 		fail_thread_start.store(true);
 	}
 
+	void AllocationFault::FailWriterAllocation(int after_frames)
+	{
+		writer_frames_left.store(after_frames);
+		fail_writer_allocation.store(true);
+	}
+
 	void AllocationFault::Disarm()
 	{
 		fail_string_copy.store(false);
 		fail_allocation.store(false);
 		fail_thread_start.store(false);
+		fail_writer_allocation.store(false);
 	}
 
 	bool AllocationFault::IsArmed()
 	{
-		return fail_string_copy.load() || fail_allocation.load() || fail_thread_start.load();
+		return fail_string_copy.load() || fail_allocation.load() || fail_thread_start.load()
+				|| fail_writer_allocation.load();
 	}
 
 	void AllocationFault::CheckStringCopy()
@@ -77,4 +87,14 @@ namespace ffmpeg
 					"Starting a thread failed");
 		}
 	}
+
+	void AllocationFault::CheckWriterAllocation()
+	{
+		if (!fail_writer_allocation.load() || writer_frames_left.fetch_sub(1) > 0) {
+			return;
+		}
+		if (fail_writer_allocation.exchange(false)) {
+			throw std::bad_alloc();
+		}
+	}
 }

webrtc-media/src/main/cpp/src/media/MediaRecorder.cpp

@@ -21,6 +21,8 @@
 
 #include <chrono>
 #include <cstring>
+#include <exception>
+#include <new>
 #include <utility>
 
 extern "C" {
@@ -60,7 +62,8 @@ namespace ffmpeg
 
 	MediaRecorder::MediaRecorder(const webrtc_java_api * api, std::string path) :
 		api_(api),
-		path_(std::move(path))
+		path_(std::move(path)),
+		out_of_memory_error_("Recording to " + path_ + " failed: " + ErrorText(AVERROR(ENOMEM)))
 	{
 	}
 
@@ -422,6 +425,19 @@ namespace ffmpeg
 	}
 
 	void MediaRecorder::Run()
+	{
+		// An exception that left the thread would end the process. Whatever
+		// the loop held, such as the frames it had taken from the queue, and
+		// the lock, is released on the way out.
+		try {
+			Record();
+		}
+		catch (...) {
+			Abort();
+		}
+	}
+
+	void MediaRecorder::Record()
 	{
 		std::deque<RecordedFrame> batch;
 		std::vector<std::string> warnings;
@@ -487,6 +503,10 @@ namespace ffmpeg
 			return;
 		}
 
+		// Stands for what handling a frame may allocate, such as describing
+		// its track or holding it back for the header.
+		AllocationFault::CheckWriterAllocation();
+
 		RecorderTrack & track = *tracks_[frame.track];
 
 		if (track.GetStatus() == RecorderTrack::Status::kPending) {
@@ -670,6 +690,77 @@ namespace ffmpeg
 		}
 	}
 
+	void MediaRecorder::Abort() noexcept
+	{
+		// Nothing more goes into the file, and what waits for it is let go
+		// now rather than once the recorder is stopped, which may be a while.
+		// A recording that had failed already was reported then.
+		const bool reported = failed_;
+
+		failed_ = true;
+
+		std::deque<RecordedFrame> dropped;
+
+		try {
+			std::lock_guard<std::mutex> lock(mutex_);
+
+			accepting_ = false;
+			queue_.swap(dropped);
+			queued_bytes_ = 0;
+		}
+		catch (...) {
+			// Without the lock the callbacks go on queueing, up to the limit,
+			// until the recorder is stopped.
+		}
+
+		dropped.clear();
+		pending_.clear();
+		pending_bytes_ = 0;
+
+		const std::string * message = &out_of_memory_error_;
+		std::string described;
+
+		try {
+			throw;
+		}
+		catch (const std::bad_alloc &) {
+		}
+		catch (const std::exception & e) {
+			try {
+				described = "Recording to " + path_ + " failed: " + e.what();
+				message = &described;
+			}
+			catch (...) {
+			}
+		}
+		catch (...) {
+			try {
+				described = "Recording to " + path_ + " failed";
+				message = &described;
+			}
+			catch (...) {
+			}
+		}
+
+		try {
+			if (observer_ != nullptr && !reported) {
+				observer_->OnError(*message);
+			}
+		}
+		catch (...) {
+		}
+
+		// Once failed, finishing writes no more media and reports nothing,
+		// so it needs no memory: the trailer keeps what is in the file
+		// playable. Should it throw all the same, Stop() still closes the
+		// file.
+		try {
+			Finish();
+		}
+		catch (...) {
+		}
+	}
+
 	void MediaRecorder::Warn(const std::string & message)
 	{
 		if (observer_ != nullptr) {

webrtc-media/src/main/cpp/src/media/RecorderTrack.cpp

@@ -38,6 +38,16 @@ namespace ffmpeg
 		// put a frame means the sender restarted its RTP clock.
 		constexpr int64_t kMaxDriftSeconds = 10;
 
+		struct BsfDeleter
+		{
+			void operator()(AVBSFContext * bsf) const
+			{
+				av_bsf_free(&bsf);
+			}
+		};
+
+		using BsfPtr = std::unique_ptr<AVBSFContext, BsfDeleter>;
+
 		struct CodecInfo
 		{
 			const char * mime_type;
@@ -452,42 +462,46 @@ namespace ffmpeg
 	bool RecorderTrack::ExtractExtradata(const AVPacket * packet)
 	{
 		const AVBitStreamFilter * filter = av_bsf_get_by_name("extract_extradata");
-		AVBSFContext * bsf = nullptr;
+		AVBSFContext * context = nullptr;
 
-		if (filter == nullptr || av_bsf_alloc(filter, &bsf) < 0) {
+		if (filter == nullptr || av_bsf_alloc(filter, &context) < 0) {
 			return false;
 		}
 
+		// Owned, so that the filter and the packet are freed also when the
+		// copy of what it found throws.
+		BsfPtr bsf(context);
+
 		bsf->par_in->codec_type = AVMEDIA_TYPE_VIDEO;
 		bsf->par_in->codec_id = codec_id_;
 
-		bool found = false;
+		if (av_bsf_init(bsf.get()) < 0) {
+			return false;
+		}
 
-		if (av_bsf_init(bsf) >= 0) {
-			// The filter takes the packet over, so it gets a reference of its
-			// own rather than the frame's.
-			AVPacket * copy = av_packet_clone(packet);
+		// The filter takes the packet over, so it gets a reference of its own
+		// rather than the frame's.
+		PacketPtr copy(av_packet_clone(packet));
 
-			if (copy != nullptr && av_bsf_send_packet(bsf, copy) >= 0) {
-				while (av_bsf_receive_packet(bsf, copy) >= 0) {
-					size_t size = 0;
-					const uint8_t * data = av_packet_get_side_data(copy,
-							AV_PKT_DATA_NEW_EXTRADATA, &size);
+		if (copy == nullptr || av_bsf_send_packet(bsf.get(), copy.get()) < 0) {
+			return false;
+		}
 
-					if (data != nullptr && size > 0) {
-						extradata_.assign(data, data + size);
-						found = true;
-					}
+		bool found = false;
 
-					av_packet_unref(copy);
-				}
+		while (av_bsf_receive_packet(bsf.get(), copy.get()) >= 0) {
+			size_t size = 0;
+			const uint8_t * data = av_packet_get_side_data(copy.get(),
+					AV_PKT_DATA_NEW_EXTRADATA, &size);
+
+			if (data != nullptr && size > 0) {
+				extradata_.assign(data, data + size);
+				found = true;
 			}
 
-			av_packet_free(&copy);
+			av_packet_unref(copy.get());
 		}
 
-		av_bsf_free(&bsf);
-
 		return found;
 	}
 

webrtc-media/src/main/java/dev/onvoid/webrtc/media/recorder/MediaRecorderListener.java

@@ -46,9 +46,10 @@ default void onWarning(String message) {
 	}
 
 	/**
-	 * Writing the file failed, e.g. because the disk is full. Nothing more
-	 * goes into the file; what is in it stays, and {@link MediaRecorder#stop()}
-	 * still finishes it as far as it can. Finishing and closing the file when
+	 * Writing the file failed, e.g. because the disk is full, or the native
+	 * writer ran out of memory. Nothing more goes into the file; what is in it
+	 * stays, and {@link MediaRecorder#stop()} still finishes it as far as it
+	 * can. Finishing and closing the file when
 	 * stopping may fail as well, and are reported here too. Once stopped,
 	 * {@link MediaRecorder#getResult()} tells what is left.
 	 *

webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/AllocationFault.java

@@ -54,6 +54,18 @@ private AllocationFault() {
 	 */
 	public static native void failNextThreadStart();
 
+	/**
+	 * Fails an allocation of a recorder's writer thread, as one fails that
+	 * holds back or describes a frame, once that thread has handled the given
+	 * number of frames. Unlike the other faults, it throws on a thread of
+	 * the native library's own, where no entry point turns the C++ exception
+	 * into a Java one.
+	 *
+	 * @param afterFrames How many frames the writer handles before the one
+	 *                    whose allocation fails.
+	 */
+	public static native void failWriterAllocation(int afterFrames);
+
 	/**
 	 * @return True if a fault is armed that has not fired yet.
 	 */