From 7561c2113a2067205cbc8ff18d28197311a1e229 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Tue, 10 Jul 2018 15:37:55 +0200 Subject: [PATCH] Android: Honor disabling legacy video HW codec acceleration This CL is a forward fix of https://webrtc-review.googlesource.com/c/src/+/83729. That CL accidentally changed the behavior in a specific case when a client would both explicitly call setEnableVideoHwAcceleration(false) and also not inject neither a encoder nor a decoder factory. This CL restores the behavior for that specific case. Bug: webrtc:7925 Change-Id: I7653453d5dceb2e61fede164216ff2c879d760ed Reviewed-on: https://webrtc-review.googlesource.com/87847 Reviewed-by: Anders Carlsson Commit-Queue: Magnus Jedvert Cr-Commit-Position: refs/heads/master@{#23909} --- sdk/android/BUILD.gn | 2 + .../src/jni/pc/peerconnectionfactory.cc | 59 +++++++++++-------- sdk/android/src/jni/pc/video.cc | 14 +++-- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 33ecacec30..3ec3354be0 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -336,6 +336,8 @@ rtc_static_library("video_jni") { "../..:webrtc_common", "../../api:libjingle_peerconnection_api", "../../api/video:video_frame", + "../../api/video_codecs:builtin_video_decoder_factory", + "../../api/video_codecs:builtin_video_encoder_factory", "../../api/video_codecs:rtc_software_fallback_wrappers", "../../api/video_codecs:video_codecs_api", "../../common_video:common_video", diff --git a/sdk/android/src/jni/pc/peerconnectionfactory.cc b/sdk/android/src/jni/pc/peerconnectionfactory.cc index 5dcc7e1da4..62778c6667 100644 --- a/sdk/android/src/jni/pc/peerconnectionfactory.cc +++ b/sdk/android/src/jni/pc/peerconnectionfactory.cc @@ -243,36 +243,47 @@ jlong CreatePeerConnectionFactoryForJava( VideoEncoderFactory* legacy_video_encoder_factory = nullptr; VideoDecoderFactory* legacy_video_decoder_factory = nullptr; + std::unique_ptr video_encoder_factory; + std::unique_ptr video_decoder_factory; std::unique_ptr media_engine; - std::unique_ptr video_encoder_factory = nullptr; - if (jencoder_factory.is_null()) { - // TODO(bugs.webrtc.org/7925): When all clients switched to injectable - // factories, remove the legacy codec factories - std::unique_ptr legacy_factory = - CreateLegacyVideoEncoderFactory(); - legacy_video_encoder_factory = legacy_factory.get(); + if (jencoder_factory.is_null() && jdecoder_factory.is_null() && + !video_hw_acceleration_enabled) { + // Legacy path for clients that are explicitly calling + // setEnableVideoHwAcceleration(false) and not injecting neither encoder nor + // decoder. These clients should be migrated to only pass in + // SoftwareVideoEncoderFactory instead. video_encoder_factory = - WrapLegacyVideoEncoderFactory(std::move(legacy_factory)); - } else { - video_encoder_factory = std::unique_ptr( - CreateVideoEncoderFactory(jni, jencoder_factory)); - } - - std::unique_ptr video_decoder_factory = nullptr; - if (jdecoder_factory.is_null()) { - // TODO(bugs.webrtc.org/7925): When all clients switched to injectable - // factories, remove the legacy codec factories - std::unique_ptr legacy_factory = - CreateLegacyVideoDecoderFactory(); - legacy_video_decoder_factory = legacy_factory.get(); + WrapLegacyVideoEncoderFactory(/* legacy_encoder_factory= */ nullptr); video_decoder_factory = - WrapLegacyVideoDecoderFactory(std::move(legacy_factory)); + WrapLegacyVideoDecoderFactory(/* legacy_decoder_factory= */ nullptr); } else { - video_decoder_factory = std::unique_ptr( - CreateVideoDecoderFactory(jni, jdecoder_factory)); - } + if (jencoder_factory.is_null()) { + // TODO(bugs.webrtc.org/7925): When all clients switched to injectable + // factories, remove the legacy codec factories + std::unique_ptr legacy_factory = + CreateLegacyVideoEncoderFactory(); + legacy_video_encoder_factory = legacy_factory.get(); + video_encoder_factory = + WrapLegacyVideoEncoderFactory(std::move(legacy_factory)); + } else { + video_encoder_factory = std::unique_ptr( + CreateVideoEncoderFactory(jni, jencoder_factory)); + } + if (jdecoder_factory.is_null()) { + // TODO(bugs.webrtc.org/7925): When all clients switched to injectable + // factories, remove the legacy codec factories + std::unique_ptr legacy_factory = + CreateLegacyVideoDecoderFactory(); + legacy_video_decoder_factory = legacy_factory.get(); + video_decoder_factory = + WrapLegacyVideoDecoderFactory(std::move(legacy_factory)); + } else { + video_decoder_factory = std::unique_ptr( + CreateVideoDecoderFactory(jni, jdecoder_factory)); + } + } media_engine.reset(CreateMediaEngine( audio_device_module, audio_encoder_factory, audio_decoder_factory, std::move(video_encoder_factory), std::move(video_decoder_factory), diff --git a/sdk/android/src/jni/pc/video.cc b/sdk/android/src/jni/pc/video.cc index 83ef8c763b..027cada0f1 100644 --- a/sdk/android/src/jni/pc/video.cc +++ b/sdk/android/src/jni/pc/video.cc @@ -13,6 +13,8 @@ #include #include +#include "api/video_codecs/builtin_video_decoder_factory.h" +#include "api/video_codecs/builtin_video_encoder_factory.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" #include "api/videosourceproxy.h" @@ -88,14 +90,18 @@ std::unique_ptr CreateLegacyVideoDecoderFactory() { std::unique_ptr WrapLegacyVideoEncoderFactory( std::unique_ptr legacy_encoder_factory) { - return std::unique_ptr( - cricket::ConvertVideoEncoderFactory(std::move(legacy_encoder_factory))); + return legacy_encoder_factory ? std::unique_ptr( + cricket::ConvertVideoEncoderFactory( + std::move(legacy_encoder_factory))) + : CreateBuiltinVideoEncoderFactory(); } std::unique_ptr WrapLegacyVideoDecoderFactory( std::unique_ptr legacy_decoder_factory) { - return std::unique_ptr( - cricket::ConvertVideoDecoderFactory(std::move(legacy_decoder_factory))); + return legacy_decoder_factory ? std::unique_ptr( + cricket::ConvertVideoDecoderFactory( + std::move(legacy_decoder_factory))) + : CreateBuiltinVideoDecoderFactory(); } } // namespace jni