diff --git a/docs/guide/media/media-recording.md b/docs/guide/media/media-recording.md index 0b206b48b..65c5f6a1f 100644 --- a/docs/guide/media/media-recording.md +++ b/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. diff --git a/webrtc-media/src/main/cpp/include/JNI_AllocationFault.h b/webrtc-media/src/main/cpp/include/JNI_AllocationFault.h index 9b440234b..d10f077b8 100644 --- a/webrtc-media/src/main/cpp/include/JNI_AllocationFault.h +++ b/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 diff --git a/webrtc-media/src/main/cpp/include/media/AllocationFault.h b/webrtc-media/src/main/cpp/include/media/AllocationFault.h index 016966e47..e68d5bde0 100644 --- a/webrtc-media/src/main/cpp/include/media/AllocationFault.h +++ b/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(); }; } diff --git a/webrtc-media/src/main/cpp/include/media/MediaRecorder.h b/webrtc-media/src/main/cpp/include/media/MediaRecorder.h index 1c4563eef..8dc801db8 100644 --- a/webrtc-media/src/main/cpp/include/media/MediaRecorder.h +++ b/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 observer_; diff --git a/webrtc-media/src/main/cpp/src/JNI_AllocationFault.cpp b/webrtc-media/src/main/cpp/src/JNI_AllocationFault.cpp index 6c951ab1f..3d793a06e 100644 --- a/webrtc-media/src/main/cpp/src/JNI_AllocationFault.cpp +++ b/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) { diff --git a/webrtc-media/src/main/cpp/src/media/AllocationFault.cpp b/webrtc-media/src/main/cpp/src/media/AllocationFault.cpp index abd4fc735..e0fa77b85 100644 --- a/webrtc-media/src/main/cpp/src/media/AllocationFault.cpp +++ b/webrtc-media/src/main/cpp/src/media/AllocationFault.cpp @@ -25,6 +25,8 @@ namespace std::atomic fail_string_copy{ false }; std::atomic fail_allocation{ false }; std::atomic fail_thread_start{ false }; + std::atomic fail_writer_allocation{ false }; + std::atomic 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(); + } + } } diff --git a/webrtc-media/src/main/cpp/src/media/MediaRecorder.cpp b/webrtc-media/src/main/cpp/src/media/MediaRecorder.cpp index 7fae0075a..fdc788ac2 100644 --- a/webrtc-media/src/main/cpp/src/media/MediaRecorder.cpp +++ b/webrtc-media/src/main/cpp/src/media/MediaRecorder.cpp @@ -21,6 +21,8 @@ #include #include +#include +#include #include 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 batch; std::vector 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 dropped; + + try { + std::lock_guard 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) { diff --git a/webrtc-media/src/main/cpp/src/media/RecorderTrack.cpp b/webrtc-media/src/main/cpp/src/media/RecorderTrack.cpp index 388beb705..f418b5ea1 100644 --- a/webrtc-media/src/main/cpp/src/media/RecorderTrack.cpp +++ b/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; + 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(©); + av_packet_unref(copy.get()); } - av_bsf_free(&bsf); - return found; } diff --git a/webrtc-media/src/main/java/dev/onvoid/webrtc/media/recorder/MediaRecorderListener.java b/webrtc-media/src/main/java/dev/onvoid/webrtc/media/recorder/MediaRecorderListener.java index 17f4a5028..c9b93146e 100644 --- a/webrtc-media/src/main/java/dev/onvoid/webrtc/media/recorder/MediaRecorderListener.java +++ b/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. * diff --git a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/AllocationFault.java b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/AllocationFault.java index f9a8a05d7..5c4acab4a 100644 --- a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/AllocationFault.java +++ b/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. */ diff --git a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/recorder/MediaRecorderTest.java b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/recorder/MediaRecorderTest.java index 96566b9a1..883654572 100644 --- a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/recorder/MediaRecorderTest.java +++ b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/recorder/MediaRecorderTest.java @@ -499,6 +499,64 @@ void failedWriteKeepsPartialRecording() throws Exception { assertTrue(readInfo(file).hasVideo()); } + @Test + void writerOutOfMemoryFailsRecording() throws Exception { + Path file = tempDir.resolve("writer-oom-early.mkv"); + Listener listener = new Listener(); + + // The first frame, which is held back until the header is written. + AllocationFault.failWriterAllocation(0); + + try (TestCall call = new TestCall(factory); + MediaRecorder recorder = new MediaRecorder(file)) { + recorder.setListener(listener); + recorder.addTrack(call.getVideoSender()); + recorder.start(); + + assertTrue(listener.failed.await(10, TimeUnit.SECONDS), "no error"); + assertFalse(AllocationFault.isArmed()); + + assertFalse(recorder.stop()); + assertEquals(MediaRecorderResult.FAILED, recorder.getResult()); + } + finally { + AllocationFault.disarm(); + } + + assertErrorStartsWith(listener, "Recording to " + file.toAbsolutePath() + " failed: "); + assertEquals(1, listener.started.getCount(), "reported as started"); + assertFalse(Files.exists(file)); + } + + @Test + void writerOutOfMemoryKeepsPartialRecording() throws Exception { + Path file = tempDir.resolve("writer-oom-late.mkv"); + Listener listener = new Listener(); + + // Well into the recording, once there is media in the file. + AllocationFault.failWriterAllocation(30); + + try (TestCall call = new TestCall(factory); + MediaRecorder recorder = new MediaRecorder(file)) { + recorder.setListener(listener); + recorder.addTrack(call.getVideoSender()); + recorder.start(); + + assertTrue(listener.failed.await(10, TimeUnit.SECONDS), "no error"); + assertEquals(0, listener.started.getCount(), "not started"); + + // The writer has ended, and the file was finished then. + assertTrue(recorder.stop()); + assertEquals(MediaRecorderResult.PARTIAL, recorder.getResult()); + } + finally { + AllocationFault.disarm(); + } + + assertErrorStartsWith(listener, "Recording to " + file.toAbsolutePath() + " failed: "); + assertTrue(readInfo(file).hasVideo()); + } + @Test void failedTrailerKeepsPartialRecording() throws Exception { assertFailedFinishKeepsFile("failed-trailer.mkv", WriteFault::failTrailer, "Finishing ");