From d60d5c4479d2a01bcf56224bbaddf4b8c3e9c0d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Kalliom=C3=A4ki?= Date: Tue, 20 Feb 2018 11:35:53 +0100 Subject: [PATCH] Improve Java video codec error handling. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FALLBACK_SOFTWARE is now treated as a critical error and results in immediate fallback to software coding if available. If ERROR is returned, codec reset is attempted. If that fails, software fallback is used. Bug: b/73498933 Change-Id: I7fe163efd09e6f27c72491e9595954ddc59b1448 Reviewed-on: https://webrtc-review.googlesource.com/54901 Reviewed-by: Alex Glaznev Commit-Queue: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#22169} --- .../java/org/webrtc/HardwareVideoDecoder.java | 10 ++-- .../java/org/webrtc/HardwareVideoEncoder.java | 4 +- sdk/android/src/jni/videodecoderwrapper.cc | 48 +++++++++------ sdk/android/src/jni/videodecoderwrapper.h | 4 +- sdk/android/src/jni/videoencoderwrapper.cc | 60 +++++++++++-------- sdk/android/src/jni/videoencoderwrapper.h | 4 +- 6 files changed, 78 insertions(+), 52 deletions(-) diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java index 72d0ccf52e..85be9baf11 100644 --- a/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java +++ b/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java @@ -163,7 +163,7 @@ class HardwareVideoDecoder Logging.d(TAG, "initDecodeInternal"); if (outputThread != null) { Logging.e(TAG, "initDecodeInternal called while the codec is already running"); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.FALLBACK_SOFTWARE; } // Note: it is not necessary to initialize dimensions under the lock, since the output thread @@ -180,7 +180,7 @@ class HardwareVideoDecoder codec = MediaCodec.createByCodecName(codecName); } catch (IOException | IllegalArgumentException e) { Logging.e(TAG, "Cannot create media decoder " + codecName); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.FALLBACK_SOFTWARE; } try { MediaFormat format = MediaFormat.createVideoFormat(codecType.mimeType(), width, height); @@ -192,7 +192,7 @@ class HardwareVideoDecoder } catch (IllegalStateException e) { Logging.e(TAG, "initDecode failed", e); release(); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.FALLBACK_SOFTWARE; } running = true; outputThread = createOutputThread(); @@ -241,11 +241,11 @@ class HardwareVideoDecoder // Need to process a key frame first. if (frame.frameType != EncodedImage.FrameType.VideoFrameKey) { Logging.e(TAG, "decode() - key frame required first"); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.NO_OUTPUT; } if (!frame.completeFrame) { Logging.e(TAG, "decode() - complete frame required first"); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.NO_OUTPUT; } } diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java index 046ffb29dc..0882c4b1e6 100644 --- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java +++ b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java @@ -176,7 +176,7 @@ class HardwareVideoEncoder implements VideoEncoder { codec = MediaCodec.createByCodecName(codecName); } catch (IOException | IllegalArgumentException e) { Logging.e(TAG, "Cannot create media encoder " + codecName); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.FALLBACK_SOFTWARE; } final int colorFormat = useSurfaceMode ? surfaceColorFormat : yuvColorFormat; @@ -218,7 +218,7 @@ class HardwareVideoEncoder implements VideoEncoder { } catch (IllegalStateException e) { Logging.e(TAG, "initEncodeInternal failed", e); release(); - return VideoCodecStatus.ERROR; + return VideoCodecStatus.FALLBACK_SOFTWARE; } running = true; diff --git a/sdk/android/src/jni/videodecoderwrapper.cc b/sdk/android/src/jni/videodecoderwrapper.cc index 2525afa3eb..6f34dd934f 100644 --- a/sdk/android/src/jni/videodecoderwrapper.cc +++ b/sdk/android/src/jni/videodecoderwrapper.cc @@ -67,9 +67,10 @@ int32_t VideoDecoderWrapper::InitDecodeInternal(JNIEnv* jni) { Java_VideoDecoderWrapper_createDecoderCallback(jni, jlongFromPointer(this)); - ScopedJavaLocalRef ret = - Java_VideoDecoder_initDecode(jni, decoder_, settings, callback); - if (JavaToNativeVideoCodecStatus(jni, ret) == WEBRTC_VIDEO_CODEC_OK) { + int32_t status = JavaToNativeVideoCodecStatus( + jni, Java_VideoDecoder_initDecode(jni, decoder_, settings, callback)); + RTC_LOG(LS_INFO) << "initDecode: " << status; + if (status == WEBRTC_VIDEO_CODEC_OK) { initialized_ = true; } @@ -77,7 +78,7 @@ int32_t VideoDecoderWrapper::InitDecodeInternal(JNIEnv* jni) { // providing QP values. qp_parsing_enabled_ = true; - return HandleReturnCode(jni, ret); + return status; } int32_t VideoDecoderWrapper::Decode( @@ -116,7 +117,7 @@ int32_t VideoDecoderWrapper::Decode( ScopedJavaLocalRef decode_info; ScopedJavaLocalRef ret = Java_VideoDecoder_decode(env, decoder_, jinput_image, decode_info); - return HandleReturnCode(env, ret); + return HandleReturnCode(env, ret, "decode"); } int32_t VideoDecoderWrapper::RegisterDecodeCompleteCallback( @@ -128,13 +129,14 @@ int32_t VideoDecoderWrapper::RegisterDecodeCompleteCallback( int32_t VideoDecoderWrapper::Release() { JNIEnv* jni = AttachCurrentThreadIfNeeded(); - ScopedJavaLocalRef ret = Java_VideoDecoder_release(jni, decoder_); + int32_t status = JavaToNativeVideoCodecStatus( + jni, Java_VideoDecoder_release(jni, decoder_)); + RTC_LOG(LS_INFO) << "release: " << status; { rtc::CritScope cs(&frame_extra_infos_lock_); frame_extra_infos_.clear(); } initialized_ = false; - int32_t status = HandleReturnCode(jni, ret); // It is allowed to reinitialize the codec on a different thread. decoder_thread_checker_.DetachFromThread(); return status; @@ -193,19 +195,29 @@ void VideoDecoderWrapper::OnDecodedFrame( } int32_t VideoDecoderWrapper::HandleReturnCode(JNIEnv* jni, - const JavaRef& code) { - int32_t value = JavaToNativeVideoCodecStatus(jni, code); - if (value < 0) { // Any errors are represented by negative values. - // Reset the codec. - if (Release() == WEBRTC_VIDEO_CODEC_OK) { - InitDecodeInternal(jni); - } - - RTC_LOG(LS_WARNING) << "Falling back to software decoder."; - return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; - } else { + const JavaRef& j_value, + const char* method_name) { + int32_t value = JavaToNativeVideoCodecStatus(jni, j_value); + if (value >= 0) { // OK or NO_OUTPUT return value; } + + RTC_LOG(LS_WARNING) << method_name << ": " << value; + if (value == WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE || + value == WEBRTC_VIDEO_CODEC_UNINITIALIZED) { // Critical error. + RTC_LOG(LS_WARNING) << "Java decoder requested software fallback."; + return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; + } + + // Try resetting the codec. + if (Release() == WEBRTC_VIDEO_CODEC_OK && + InitDecodeInternal(jni) == WEBRTC_VIDEO_CODEC_OK) { + RTC_LOG(LS_WARNING) << "Reset Java decoder."; + return WEBRTC_VIDEO_CODEC_ERROR; + } + + RTC_LOG(LS_WARNING) << "Unable to reset Java decoder."; + return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; } rtc::Optional VideoDecoderWrapper::ParseQP( diff --git a/sdk/android/src/jni/videodecoderwrapper.h b/sdk/android/src/jni/videodecoderwrapper.h index 05053f8be3..c3cda742a1 100644 --- a/sdk/android/src/jni/videodecoderwrapper.h +++ b/sdk/android/src/jni/videodecoderwrapper.h @@ -73,7 +73,9 @@ class VideoDecoderWrapper : public VideoDecoder { // Takes Java VideoCodecStatus, handles it and returns WEBRTC_VIDEO_CODEC_* // status code. - int32_t HandleReturnCode(JNIEnv* jni, const JavaRef& code) + int32_t HandleReturnCode(JNIEnv* jni, + const JavaRef& j_value, + const char* method_name) RTC_RUN_ON(decoder_thread_checker_); rtc::Optional ParseQP(const EncodedImage& input_image) diff --git a/sdk/android/src/jni/videoencoderwrapper.cc b/sdk/android/src/jni/videoencoderwrapper.cc index ef0cdb485e..77598a9c50 100644 --- a/sdk/android/src/jni/videoencoderwrapper.cc +++ b/sdk/android/src/jni/videoencoderwrapper.cc @@ -31,8 +31,6 @@ namespace webrtc { namespace jni { -static const int kMaxJavaEncoderResets = 3; - VideoEncoderWrapper::VideoEncoderWrapper(JNIEnv* jni, const JavaRef& j_encoder) : encoder_(jni, j_encoder), int_array_class_(GetClass(jni, "[I")) { @@ -81,14 +79,14 @@ int32_t VideoEncoderWrapper::InitEncodeInternal(JNIEnv* jni) { Java_VideoEncoderWrapper_createEncoderCallback(jni, jlongFromPointer(this)); - ScopedJavaLocalRef ret = - Java_VideoEncoder_initEncode(jni, encoder_, settings, callback); + int32_t status = JavaToNativeVideoCodecStatus( + jni, Java_VideoEncoder_initEncode(jni, encoder_, settings, callback)); + RTC_LOG(LS_INFO) << "initEncode: " << status; - if (JavaToNativeVideoCodecStatus(jni, ret) == WEBRTC_VIDEO_CODEC_OK) { + if (status == WEBRTC_VIDEO_CODEC_OK) { initialized_ = true; } - - return HandleReturnCode(jni, ret); + return status; } int32_t VideoEncoderWrapper::RegisterEncodeCompleteCallback( @@ -99,11 +97,15 @@ int32_t VideoEncoderWrapper::RegisterEncodeCompleteCallback( int32_t VideoEncoderWrapper::Release() { JNIEnv* jni = AttachCurrentThreadIfNeeded(); - ScopedJavaLocalRef ret = Java_VideoEncoder_release(jni, encoder_); + + int32_t status = JavaToNativeVideoCodecStatus( + jni, Java_VideoEncoder_release(jni, encoder_)); + RTC_LOG(LS_INFO) << "release: " << status; frame_extra_infos_.clear(); initialized_ = false; encoder_queue_ = nullptr; - return HandleReturnCode(jni, ret); + + return status; } int32_t VideoEncoderWrapper::Encode( @@ -132,7 +134,7 @@ int32_t VideoEncoderWrapper::Encode( ScopedJavaLocalRef ret = Java_VideoEncoder_encode(jni, encoder_, j_frame, encode_info); ReleaseJavaVideoFrame(jni, j_frame); - return HandleReturnCode(jni, ret); + return HandleReturnCode(jni, ret, "encode"); } int32_t VideoEncoderWrapper::SetChannelParameters(uint32_t packet_loss, @@ -140,7 +142,7 @@ int32_t VideoEncoderWrapper::SetChannelParameters(uint32_t packet_loss, JNIEnv* jni = AttachCurrentThreadIfNeeded(); ScopedJavaLocalRef ret = Java_VideoEncoder_setChannelParameters( jni, encoder_, (jshort)packet_loss, (jlong)rtt); - return HandleReturnCode(jni, ret); + return HandleReturnCode(jni, ret, "setChannelParameters"); } int32_t VideoEncoderWrapper::SetRateAllocation( @@ -152,7 +154,7 @@ int32_t VideoEncoderWrapper::SetRateAllocation( ToJavaBitrateAllocation(jni, allocation); ScopedJavaLocalRef ret = Java_VideoEncoder_setRateAllocation( jni, encoder_, j_bitrate_allocation, (jint)framerate); - return HandleReturnCode(jni, ret); + return HandleReturnCode(jni, ret, "setRateAllocation"); } VideoEncoderWrapper::ScalingSettings VideoEncoderWrapper::GetScalingSettings() @@ -263,21 +265,29 @@ void VideoEncoderWrapper::OnEncodedFrame(JNIEnv* jni, } int32_t VideoEncoderWrapper::HandleReturnCode(JNIEnv* jni, - const JavaRef& code) { - int32_t value = JavaToNativeVideoCodecStatus(jni, code); - if (value < 0) { // Any errors are represented by negative values. - // Try resetting the codec. - if (++num_resets_ <= kMaxJavaEncoderResets && - Release() == WEBRTC_VIDEO_CODEC_OK) { - RTC_LOG(LS_WARNING) << "Reset Java encoder: " << num_resets_; - return InitEncodeInternal(jni); - } - - RTC_LOG(LS_WARNING) << "Falling back to software decoder."; - return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; - } else { + const JavaRef& j_value, + const char* method_name) { + int32_t value = JavaToNativeVideoCodecStatus(jni, j_value); + if (value >= 0) { // OK or NO_OUTPUT return value; } + + RTC_LOG(LS_WARNING) << method_name << ": " << value; + if (value == WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE || + value == WEBRTC_VIDEO_CODEC_UNINITIALIZED) { // Critical error. + RTC_LOG(LS_WARNING) << "Java encoder requested software fallback."; + return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; + } + + // Try resetting the codec. + if (Release() == WEBRTC_VIDEO_CODEC_OK && + InitEncodeInternal(jni) == WEBRTC_VIDEO_CODEC_OK) { + RTC_LOG(LS_WARNING) << "Reset Java encoder."; + return WEBRTC_VIDEO_CODEC_ERROR; + } + + RTC_LOG(LS_WARNING) << "Unable to reset Java encoder."; + return WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; } RTPFragmentationHeader VideoEncoderWrapper::ParseFragmentationHeader( diff --git a/sdk/android/src/jni/videoencoderwrapper.h b/sdk/android/src/jni/videoencoderwrapper.h index 385db511c3..fe553e9390 100644 --- a/sdk/android/src/jni/videoencoderwrapper.h +++ b/sdk/android/src/jni/videoencoderwrapper.h @@ -78,7 +78,9 @@ class VideoEncoderWrapper : public VideoEncoder { // Takes Java VideoCodecStatus, handles it and returns WEBRTC_VIDEO_CODEC_* // status code. - int32_t HandleReturnCode(JNIEnv* jni, const JavaRef& code); + int32_t HandleReturnCode(JNIEnv* jni, + const JavaRef& j_value, + const char* method_name); RTPFragmentationHeader ParseFragmentationHeader( const std::vector& buffer);