diff --git a/webrtc-jni/src/main/cpp/include/media/video/codec/JavaLocalFrame.h b/webrtc-jni/src/main/cpp/include/media/video/codec/JavaLocalFrame.h index 23c46af6b..eb0da4855 100644 --- a/webrtc-jni/src/main/cpp/include/media/video/codec/JavaLocalFrame.h +++ b/webrtc-jni/src/main/cpp/include/media/video/codec/JavaLocalFrame.h @@ -25,18 +25,27 @@ namespace jni // thread stays attached to the JVM for as long as it runs, so without a // frame of their own the references made for one call would pile up // behind those of the next. + // + // A frame that cannot be pushed reports the JVM's exception as a codec + // exception, and the call it was meant for gives up rather than make + // references that nothing would free soon: see pushed(). class JavaLocalFrame { public: - JavaLocalFrame(JNIEnv * env, jint capacity); + // The context names the call in the log. + JavaLocalFrame(JNIEnv * env, jint capacity, const char * context); ~JavaLocalFrame(); JavaLocalFrame(const JavaLocalFrame &) = delete; JavaLocalFrame & operator=(const JavaLocalFrame &) = delete; + // Whether the frame is in place, which a call checks before it + // makes any local reference. + bool pushed() const; + private: JNIEnv * env; - bool pushed; + const bool isPushed; }; } diff --git a/webrtc-jni/src/main/cpp/include/media/video/codec/VideoDecoderWrapper.h b/webrtc-jni/src/main/cpp/include/media/video/codec/VideoDecoderWrapper.h index 8192decb6..3a4ad414f 100644 --- a/webrtc-jni/src/main/cpp/include/media/video/codec/VideoDecoderWrapper.h +++ b/webrtc-jni/src/main/cpp/include/media/video/codec/VideoDecoderWrapper.h @@ -117,8 +117,10 @@ namespace jni Settings settings; bool initialized; - // Parsing stops while the decoder provides the QP itself. - bool qpParsingEnabled; + // Parsing stops while the decoder provides the QP itself. Read on + // the decoder thread and written on the Java decoder's, which + // may deliver its frames from a thread of its own. + std::atomic qpParsingEnabled; webrtc::H264BitstreamParser h264BitstreamParser; }; } diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/JavaLocalFrame.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/JavaLocalFrame.cpp index f735831cd..d84eea04d 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/JavaLocalFrame.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/JavaLocalFrame.cpp @@ -15,23 +15,34 @@ */ #include "media/video/codec/JavaLocalFrame.h" +#include "media/video/codec/VideoCodecUtils.h" + +#include "rtc_base/logging.h" namespace jni { - JavaLocalFrame::JavaLocalFrame(JNIEnv * env, jint capacity) : + JavaLocalFrame::JavaLocalFrame(JNIEnv * env, jint capacity, const char * context) : env(env), - pushed(env->PushLocalFrame(capacity) == JNI_OK) + isPushed(env->PushLocalFrame(capacity) == JNI_OK) { - if (!pushed) { - // Out of memory: the references then go to the enclosing frame. - env->ExceptionClear(); + if (!isPushed) { + // The JVM is out of memory and has thrown, which is reported like + // any exception of a codec call. + ClearCodecException(env, context); + + RTC_LOG(LS_WARNING) << "No local JNI frame for " << context << ", giving up the call"; } } JavaLocalFrame::~JavaLocalFrame() { - if (pushed) { + if (isPushed) { env->PopLocalFrame(nullptr); } } + + bool JavaLocalFrame::pushed() const + { + return isPushed; + } } diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderFactoryWrapper.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderFactoryWrapper.cpp index ce5bdd671..2ac11453f 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderFactoryWrapper.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderFactoryWrapper.cpp @@ -53,7 +53,11 @@ namespace jni return nullptr; } - JavaLocalFrame localFrame(env, 16); + JavaLocalFrame localFrame(env, 16, "createDecoder"); + + if (!localFrame.pushed()) { + return nullptr; + } try { JavaLocalRef info = VideoCodecInfo::toJava(env, format); diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderWrapper.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderWrapper.cpp index 08b352339..97b5a2833 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderWrapper.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoDecoderWrapper.cpp @@ -57,7 +57,12 @@ namespace jni initialized(false), qpParsingEnabled(true) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "getImplementationName"); + + if (!localFrame.pushed()) { + // A decoder without a name, taken for a software one. + return; + } jobject name = env->CallObjectMethod(decoder, javaClass->getImplementationName); @@ -102,7 +107,16 @@ namespace jni bool VideoDecoderWrapper::ConfigureInternal(JNIEnv * env) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "initDecode"); + + if (!localFrame.pushed()) { + // Detaching the earlier callback makes no local reference. + InvalidateCallback(env); + + initialized = false; + + return false; + } webrtc::RenderResolution resolution = settings.max_render_resolution(); @@ -154,7 +168,13 @@ namespace jni return WEBRTC_VIDEO_CODEC_ERROR; } - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "decode"); + + if (!localFrame.pushed()) { + // Given up before a record of the image is made, so the decoder + // is not waited for. + return WEBRTC_VIDEO_CODEC_ERROR; + } // The capture time identifies the frame, and received images carry // none, so it is derived from the RTP timestamp. @@ -215,11 +235,18 @@ namespace jni int32_t VideoDecoderWrapper::ReleaseInternal(JNIEnv * env) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + // Releasing goes ahead without a frame, since giving it up would keep + // the Java decoder's resources, and the status is the only reference + // it makes. + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "release"); jobject status = env->CallObjectMethod(decoder, javaClass->release); int32_t result = ToNativeCodecStatus(env, status, "release"); + if (!localFrame.pushed() && status != nullptr) { + env->DeleteLocalRef(status); + } + RTC_LOG(LS_INFO) << "Java decoder release: " << result; InvalidateCallback(env); diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderFactoryWrapper.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderFactoryWrapper.cpp index 9bcdd31d3..e3257cb9f 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderFactoryWrapper.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderFactoryWrapper.cpp @@ -53,7 +53,11 @@ namespace jni return nullptr; } - JavaLocalFrame localFrame(env, 16); + JavaLocalFrame localFrame(env, 16, "createEncoder"); + + if (!localFrame.pushed()) { + return nullptr; + } try { JavaLocalRef info = VideoCodecInfo::toJava(env, format); diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderWrapper.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderWrapper.cpp index b4f8daa90..fc989dcd2 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderWrapper.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoEncoderWrapper.cpp @@ -106,7 +106,16 @@ namespace jni int32_t VideoEncoderWrapper::InitEncodeInternal(JNIEnv * env) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "initEncode"); + + if (!localFrame.pushed()) { + // Detaching the earlier callback makes no local reference. + InvalidateCallback(env); + + initialized = false; + + return WEBRTC_VIDEO_CODEC_ERROR; + } bool automaticResizeOn; @@ -192,11 +201,18 @@ namespace jni int32_t VideoEncoderWrapper::ReleaseInternal(JNIEnv * env) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + // Releasing goes ahead without a frame, since giving it up would keep + // the Java encoder's resources, and the status is the only reference + // it makes. + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "release"); jobject status = env->CallObjectMethod(encoder, javaClass->release); int32_t result = ToNativeCodecStatus(env, status, "release"); + if (!localFrame.pushed() && status != nullptr) { + env->DeleteLocalRef(status); + } + RTC_LOG(LS_INFO) << "Java encoder release: " << result; InvalidateCallback(env); @@ -224,7 +240,15 @@ namespace jni return WEBRTC_VIDEO_CODEC_ERROR; } - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "encode"); + + if (!localFrame.pushed()) { + // Dropped rather than failed, since WebRTC gives up an encoder that + // fails, and running short of memory is no fault of the encoder. + ReportDroppedFrame(frame.rtp_timestamp()); + + return WEBRTC_VIDEO_CODEC_OK; + } const auto javaFrameClass = JavaClasses::get(env); jobject jFrame = nullptr; @@ -309,7 +333,12 @@ namespace jni return; } - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "setRates"); + + if (!localFrame.pushed()) { + // The encoder keeps its rates until the next call. + return; + } jobjectArray layers = env->NewObjectArray(webrtc::kMaxSpatialLayers, javaClass->intArrayClass, nullptr); @@ -465,7 +494,12 @@ namespace jni void VideoEncoderWrapper::UpdateEncoderInfo(JNIEnv * env) { - JavaLocalFrame localFrame(env, kLocalFrameCapacity); + JavaLocalFrame localFrame(env, kLocalFrameCapacity, "getEncoderInfo"); + + if (!localFrame.pushed()) { + // The info stays as it was. + return; + } encoderInfo.supports_native_handle = false; encoderInfo.scaling_settings = GetScalingSettings(env); diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java index ba487a3ef..e42694adf 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java @@ -56,6 +56,9 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -198,14 +201,26 @@ void customCodecIsAdvertised() { @Test void javaCodecRoundTrips() throws Exception { - assertRoundTrip(new VideoCodecInfo(TEST_CODEC), false); + assertRoundTrip(new VideoCodecInfo(TEST_CODEC), false, false); } @Test void javaFrameBufferRoundTrips() throws Exception { // The decoder hands back frames whose buffer is no NativeI420Buffer, // which WebRTC has to copy. - assertRoundTrip(new VideoCodecInfo(TEST_CODEC), true); + assertRoundTrip(new VideoCodecInfo(TEST_CODEC), true, false); + } + + @Test + void asyncJavaDecoderRoundTrips() throws Exception { + // The decoder hands its frames over on a thread of its own, now with + // a QP and now without, which switches the wrapper's QP parsing on + // and off while WebRTC's decoder thread goes on decoding. + TestDecoderFactory decoderFactory = assertRoundTrip(new VideoCodecInfo(TEST_CODEC), false, true); + TestDecoder decoder = decoderFactory.last.get(); + + assertTrue(decoder.framesWithQp.get() > 0, "no frame was handed over with a QP"); + assertTrue(decoder.framesWithoutQp.get() > 0, "no frame was handed over without a QP"); } @Test @@ -213,17 +228,19 @@ void h265CallRoundTrips() throws Exception { // WebRTC has no H.265 codec of its own, so this is H.265 as a codec // factory brings it: negotiated, packetized into RTP and depacketized // as H.265, which WebRTC does only when built with it. - assertRoundTrip(H265, false); + assertRoundTrip(H265, false, false); } /** * Sends a call through the test codec and checks that every frame * received is one that was sent, and how WebRTC used the codecs. */ - private void assertRoundTrip(VideoCodecInfo codec, boolean javaBuffers) throws Exception { + private TestDecoderFactory assertRoundTrip(VideoCodecInfo codec, boolean javaBuffers, boolean asyncOutput) + throws Exception { TestEncoderFactory encoderFactory = new TestEncoderFactory(codec); TestDecoderFactory decoderFactory = new TestDecoderFactory(codec); decoderFactory.javaBuffers = javaBuffers; + decoderFactory.asyncOutput = asyncOutput; PeerConnectionFactory built = testCodecFactory(encoderFactory, decoderFactory); Set encodedSizes = ConcurrentHashMap.newKeySet(); @@ -279,6 +296,8 @@ private void assertRoundTrip(VideoCodecInfo codec, boolean javaBuffers) throws E assertTrue(encoder.rates.get() > 0, "rates were never set"); assertTrue(encoder.released.get(), "encoder was never released"); assertTrue(decoderFactory.last.get().released.get(), "decoder was never released"); + + return decoderFactory; } @Test @@ -681,6 +700,8 @@ static class TestDecoderFactory implements VideoDecoderFactory { volatile boolean javaBuffers; + volatile boolean asyncOutput; + TestDecoderFactory() { this(new VideoCodecInfo(TEST_CODEC, Collections.emptyMap())); @@ -706,7 +727,7 @@ public VideoDecoder createDecoder(VideoCodecInfo info) { return builtIn.createDecoder(info); } - TestDecoder decoder = new TestDecoder(javaBuffers, isH265(codec)); + TestDecoder decoder = new TestDecoder(javaBuffers, isH265(codec), asyncOutput); last.set(decoder); return decoder; } @@ -810,22 +831,31 @@ public ScalingSettings getScalingSettings() { /** * Decodes what {@link TestEncoder} produces into a frame of the encoded - * size, filled with the encoded luma sample. + * size, filled with the encoded luma sample. With asynchronous output it + * hands the frames over on a thread of its own, every other one with a + * QP, as a hardware decoder does from its output queue. */ static class TestDecoder implements VideoDecoder { final AtomicReference released = new AtomicReference<>(false); + final AtomicInteger framesWithQp = new AtomicInteger(); + final AtomicInteger framesWithoutQp = new AtomicInteger(); private final boolean javaBuffers; private final boolean h265; + private final ExecutorService output; + private Callback callback; + private int frames; + - TestDecoder(boolean javaBuffers, boolean h265) { + TestDecoder(boolean javaBuffers, boolean h265, boolean asyncOutput) { this.javaBuffers = javaBuffers; this.h265 = h265; + this.output = asyncOutput ? Executors.newSingleThreadExecutor() : null; } @Override @@ -836,6 +866,17 @@ public VideoCodecStatus initDecode(Settings settings, Callback callback) { @Override public VideoCodecStatus release() { + if (output != null) { + output.shutdown(); + + try { + output.awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + released.set(true); return VideoCodecStatus.OK; } @@ -868,8 +909,28 @@ public VideoCodecStatus decode(EncodedImage image) { } VideoFrame frame = new VideoFrame(buffer, image.getCaptureTimeNs()); - callback.onDecodedFrame(frame, null, null); - frame.release(); + + if (output == null) { + callback.onDecodedFrame(frame, null, null); + frame.release(); + + return VideoCodecStatus.OK; + } + + Callback frameCallback = callback; + Integer qp = frames++ % 2 == 0 ? 30 : null; + + try { + output.execute(() -> { + frameCallback.onDecodedFrame(frame, null, qp); + frame.release(); + + (qp != null ? framesWithQp : framesWithoutQp).incrementAndGet(); + }); + } + catch (RejectedExecutionException e) { + frame.release(); + } return VideoCodecStatus.OK; }