Skip to content

Commit

Permalink
Reland r256032 "Clean up usage of MediaCodecBridge/MediaDrmBridge::Is…
Browse files Browse the repository at this point in the history
…Available()".

Fixed VLOG include errors in the original CL.

Description of the original CL:

- All static methods in MediaCodecBridge and MediaDrmBridge check IsAvailable()
  internally so that callers don't need to check IsAvailable() before calling
  these static methods.
- Remove MediaSourcePlayer::IsTypeSupported() because it's obsolete now.
- Add MediaDrmBridge unittests.

BUG=303864
TBR=ddorwin@chromium.org,qinmin@chromium.org

Review URL: https://codereview.chromium.org/193353002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@256192 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
xhwang@chromium.org committed Mar 11, 2014
1 parent e85f0c1 commit 114f025
Show file tree
Hide file tree
Showing 15 changed files with 126 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ void EncryptedMediaMessageFilterAndroid::OnGetSupportedKeySystems(
return;
}

if (!MediaDrmBridge::IsAvailable() || !MediaCodecBridge::IsAvailable())
return;

// TODO(qinmin): Convert codecs to container types and check whether they
// are supported with the key system.
if (!MediaDrmBridge::IsKeySystemSupportedWithType(request.key_system, ""))
Expand Down
3 changes: 0 additions & 3 deletions content/common/gpu/media/android_video_decode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ bool AndroidVideoDecodeAccelerator::Initialize(media::VideoCodecProfile profile,

client_ = client;

if (!media::MediaCodecBridge::IsAvailable())
return false;

if (profile == media::VP8PROFILE_MAIN) {
codec_ = media::kCodecVP8;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ class AndroidVideoDecodeAcceleratorTest : public testing::Test {
};

TEST_F(AndroidVideoDecodeAcceleratorTest, ConfigureUnsupportedCodec) {
if (!media::MediaCodecBridge::IsAvailable())
return;
EXPECT_FALSE(Configure(media::kUnknownVideoCodec));
}

Expand Down
3 changes: 1 addition & 2 deletions content/common/gpu/media/android_video_encode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ void AndroidVideoEncodeAccelerator::Initialize(

client_ptr_factory_.reset(new base::WeakPtrFactory<Client>(client));

RETURN_ON_FAILURE(media::MediaCodecBridge::IsAvailable() &&
media::MediaCodecBridge::SupportsSetParameters() &&
RETURN_ON_FAILURE(media::MediaCodecBridge::SupportsSetParameters() &&
format == VideoFrame::I420 &&
output_profile == media::VP8PROFILE_MAIN,
"Unexpected combo: " << format << ", " << output_profile,
Expand Down
4 changes: 1 addition & 3 deletions content/renderer/media/media_stream_dependency_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,8 @@ void MediaStreamDependencyFactory::CreatePeerConnectionFactory() {
}

#if defined(OS_ANDROID)
if (!media::MediaCodecBridge::IsAvailable() ||
!media::MediaCodecBridge::SupportsSetParameters()) {
if (!media::MediaCodecBridge::SupportsSetParameters())
encoder_factory.reset();
}
#endif

EnsureWebRtcAudioDeviceImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ private void closeSession(ByteBuffer session) {
* Check whether the crypto scheme is supported for the given container.
* If |containerMimeType| is an empty string, we just return whether
* the crypto scheme is supported.
* TODO(qinmin): Implement the checking for container.
* TODO(xhwang): Implement container check. See: http://crbug.com/350481
*
* @return true if the container and the crypto scheme is supported, or
* false otherwise.
Expand Down
24 changes: 21 additions & 3 deletions media/base/android/media_codec_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ bool MediaCodecBridge::SupportsSetParameters() {
// static
std::vector<MediaCodecBridge::CodecsInfo> MediaCodecBridge::GetCodecsInfo() {
std::vector<CodecsInfo> codecs_info;
JNIEnv* env = AttachCurrentThread();
if (!IsAvailable())
return codecs_info;

JNIEnv* env = AttachCurrentThread();
std::string mime_type;
ScopedJavaLocalRef<jobjectArray> j_codec_info_array =
Java_MediaCodecBridge_getCodecsInfo(env);
Expand All @@ -144,6 +144,9 @@ std::vector<MediaCodecBridge::CodecsInfo> MediaCodecBridge::GetCodecsInfo() {

// static
bool MediaCodecBridge::CanDecode(const std::string& codec, bool is_secure) {
if (!IsAvailable())
return false;

JNIEnv* env = AttachCurrentThread();
std::string mime = CodecTypeToAndroidMimeType(codec);
if (mime.empty())
Expand All @@ -161,6 +164,9 @@ bool MediaCodecBridge::CanDecode(const std::string& codec, bool is_secure) {
// static
bool MediaCodecBridge::IsKnownUnaccelerated(const std::string& mime_type,
MediaCodecDirection direction) {
if (!IsAvailable())
return true;

std::string codec_type = AndroidMimeTypeToCodecType(mime_type);
std::vector<media::MediaCodecBridge::CodecsInfo> codecs_info =
MediaCodecBridge::GetCodecsInfo();
Expand Down Expand Up @@ -605,7 +611,11 @@ void AudioCodecBridge::SetVolume(double volume) {
Java_MediaCodecBridge_setVolume(env, media_codec(), volume);
}

// static
AudioCodecBridge* AudioCodecBridge::Create(const AudioCodec& codec) {
if (!MediaCodecBridge::IsAvailable())
return NULL;

const std::string mime = AudioCodecToAndroidMimeType(codec);
return mime.empty() ? NULL : new AudioCodecBridge(mime);
}
Expand All @@ -623,12 +633,15 @@ bool VideoCodecBridge::IsKnownUnaccelerated(const VideoCodec& codec,
VideoCodecToAndroidMimeType(codec), direction);
}

// static
VideoCodecBridge* VideoCodecBridge::CreateDecoder(const VideoCodec& codec,
bool is_secure,
const gfx::Size& size,
jobject surface,
jobject media_crypto) {
JNIEnv* env = AttachCurrentThread();
if (!MediaCodecBridge::IsAvailable())
return NULL;

const std::string mime = VideoCodecToAndroidMimeType(codec);
if (mime.empty())
return NULL;
Expand All @@ -638,6 +651,7 @@ VideoCodecBridge* VideoCodecBridge::CreateDecoder(const VideoCodec& codec,
if (!bridge->media_codec())
return NULL;

JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jstring> j_mime = ConvertUTF8ToJavaString(env, mime);
ScopedJavaLocalRef<jobject> j_format(
Java_MediaCodecBridge_createVideoDecoderFormat(
Expand All @@ -655,13 +669,16 @@ VideoCodecBridge* VideoCodecBridge::CreateDecoder(const VideoCodec& codec,
return bridge->StartInternal() ? bridge.release() : NULL;
}

// static
VideoCodecBridge* VideoCodecBridge::CreateEncoder(const VideoCodec& codec,
const gfx::Size& size,
int bit_rate,
int frame_rate,
int i_frame_interval,
int color_format) {
JNIEnv* env = AttachCurrentThread();
if (!MediaCodecBridge::IsAvailable())
return NULL;

const std::string mime = VideoCodecToAndroidMimeType(codec);
if (mime.empty())
return NULL;
Expand All @@ -671,6 +688,7 @@ VideoCodecBridge* VideoCodecBridge::CreateEncoder(const VideoCodec& codec,
if (!bridge->media_codec())
return NULL;

JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jstring> j_mime = ConvertUTF8ToJavaString(env, mime);
ScopedJavaLocalRef<jobject> j_format(
Java_MediaCodecBridge_createVideoEncoderFormat(env,
Expand Down
6 changes: 4 additions & 2 deletions media/base/android/media_codec_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ enum MediaCodecDirection {
class MEDIA_EXPORT MediaCodecBridge {
public:
// Returns true if MediaCodec is available on the device.
// All other static methods check IsAvailable() internally. There's no need
// to check IsAvailable() explicitly before calling them.
static bool IsAvailable();

// Returns true if MediaCodec.setParameters() is available on the device.
Expand All @@ -61,8 +63,8 @@ class MEDIA_EXPORT MediaCodecBridge {
static bool CanDecode(const std::string& codec, bool is_secure);

// Represents supported codecs on android.
// TODO(qinmin): Curretly the codecs string only contains one codec, do we
// need more specific codecs separated by comma. (e.g. "vp8" -> "vp8, vp8.0")
// TODO(qinmin): Currently the codecs string only contains one codec. Do we
// need to support codecs separated by comma. (e.g. "vp8" -> "vp8, vp8.0")?
struct CodecsInfo {
std::string codecs; // E.g. "vp8" or "avc1".
std::string name; // E.g. "OMX.google.vp8.decoder".
Expand Down
9 changes: 9 additions & 0 deletions media/base/android/media_drm_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,16 @@ bool MediaDrmBridge::IsAvailable() {

// static
bool MediaDrmBridge::IsSecureDecoderRequired(SecurityLevel security_level) {
DCHECK(IsAvailable());
return SECURITY_LEVEL_1 == security_level;
}

// static
bool MediaDrmBridge::IsSecurityLevelSupported(const std::string& key_system,
SecurityLevel security_level) {
if (!IsAvailable())
return false;

// Pass 0 as |cdm_id| and NULL as |manager| as they are not used in
// creation time of MediaDrmBridge.
scoped_ptr<MediaDrmBridge> media_drm_bridge =
Expand All @@ -202,9 +207,13 @@ bool MediaDrmBridge::IsSecurityLevelSupported(const std::string& key_system,
return media_drm_bridge->SetSecurityLevel(security_level);
}

// static
bool MediaDrmBridge::IsKeySystemSupportedWithType(
const std::string& key_system,
const std::string& container_mime_type) {
if (!IsAvailable())
return false;

std::vector<uint8> scheme_uuid = GetUUID(key_system);
if (scheme_uuid.empty())
return false;
Expand Down
5 changes: 2 additions & 3 deletions media/base/android/media_drm_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#define MEDIA_BASE_ANDROID_MEDIA_DRM_BRIDGE_H_

#include <jni.h>
#include <map>
#include <queue>
#include <string>
#include <vector>

Expand Down Expand Up @@ -39,9 +37,10 @@ class MEDIA_EXPORT MediaDrmBridge : public MediaKeys {
virtual ~MediaDrmBridge();

// Checks whether MediaDRM is available.
// All other static methods check IsAvailable() internally. There's no need
// to check IsAvailable() explicitly before calling them.
static bool IsAvailable();

// TODO(xhwang): Add tests for MediaDrmBridge. See http://crbug.com/303864
static bool IsSecurityLevelSupported(const std::string& key_system,
SecurityLevel security_level);

Expand Down
84 changes: 84 additions & 0 deletions media/base/android/media_drm_bridge_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/basictypes.h"
#include "base/logging.h"
#include "media/base/android/media_drm_bridge.h"
#include "testing/gtest/include/gtest/gtest.h"

#include "widevine_cdm_version.h" // In SHARED_INTERMEDIATE_DIR.

namespace media {

#define EXPECT_TRUE_IF_AVAILABLE(a) \
do { \
if (!MediaDrmBridge::IsAvailable()) { \
VLOG(0) << "MediaDrm not supported on device."; \
EXPECT_FALSE(a); \
} else { \
EXPECT_TRUE(a); \
} \
} while (0)

const char kAudioMp4[] = "audio/mp4";
const char kVideoMp4[] = "video/mp4";
const char kAudioWebM[] = "audio/webm";
const char kVideoWebM[] = "video/webm";
const char kInvalidKeySystem[] = "invalid.keysystem";
const MediaDrmBridge::SecurityLevel kLNone =
MediaDrmBridge::SECURITY_LEVEL_NONE;
const MediaDrmBridge::SecurityLevel kL1 = MediaDrmBridge::SECURITY_LEVEL_1;
const MediaDrmBridge::SecurityLevel kL3 = MediaDrmBridge::SECURITY_LEVEL_3;

static bool IsKeySystemSupportedWithType(
const std::string& key_system,
const std::string& container_mime_type) {
return MediaDrmBridge::IsKeySystemSupportedWithType(key_system,
container_mime_type);
}

static bool IsSecurityLevelSupported(
const std::string& key_system,
MediaDrmBridge::SecurityLevel security_level) {
return MediaDrmBridge::IsSecurityLevelSupported(key_system, security_level);
}

TEST(MediaDrmBridgeTest, IsSecurityLevelSupported_Widevine) {
EXPECT_FALSE(IsSecurityLevelSupported(kWidevineKeySystem, kLNone));
// We test "L3" fully. But for "L1" we don't check the result as it depends on
// whether the test device supports "L1".
EXPECT_TRUE_IF_AVAILABLE(IsSecurityLevelSupported(kWidevineKeySystem, kL3));
IsSecurityLevelSupported(kWidevineKeySystem, kL1);
}

// Invalid keysytem is NOT supported regardless whether MediaDrm is available.
TEST(MediaDrmBridgeTest, IsSecurityLevelSupported_InvalidKeySystem) {
EXPECT_FALSE(IsSecurityLevelSupported(kInvalidKeySystem, kLNone));
EXPECT_FALSE(IsSecurityLevelSupported(kInvalidKeySystem, kL1));
EXPECT_FALSE(IsSecurityLevelSupported(kInvalidKeySystem, kL3));
}

TEST(MediaDrmBridgeTest, IsTypeSupported_Widevine) {
EXPECT_TRUE_IF_AVAILABLE(
IsKeySystemSupportedWithType(kWidevineKeySystem, kAudioMp4));
EXPECT_TRUE_IF_AVAILABLE(
IsKeySystemSupportedWithType(kWidevineKeySystem, kVideoMp4));

// TODO(xhwang): MediaDrmBridge.IsKeySystemSupportedWithType() doesn't check
// the container type. Fix IsKeySystemSupportedWithType() and update this test
// as necessary. See: http://crbug.com/350481
EXPECT_TRUE_IF_AVAILABLE(
IsKeySystemSupportedWithType(kWidevineKeySystem, kAudioWebM));
EXPECT_TRUE_IF_AVAILABLE(
IsKeySystemSupportedWithType(kWidevineKeySystem, kVideoWebM));
}

// Invalid keysytem is NOT supported regardless whether MediaDrm is available.
TEST(MediaDrmBridgeTest, IsTypeSupported_InvalidKeySystem) {
EXPECT_FALSE(IsKeySystemSupportedWithType(kInvalidKeySystem, ""));
EXPECT_FALSE(IsKeySystemSupportedWithType(kInvalidKeySystem, kVideoMp4));
EXPECT_FALSE(IsKeySystemSupportedWithType(kInvalidKeySystem, kVideoWebM));
}

} // namespace media
29 changes: 0 additions & 29 deletions media/base/android/media_source_player.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,35 +31,6 @@ const int kBytesPerAudioOutputSample = 2;

namespace media {

// static
bool MediaSourcePlayer::IsTypeSupported(
const std::string& key_system,
MediaDrmBridge::SecurityLevel security_level,
const std::string& container,
const std::vector<std::string>& codecs) {
if (!MediaDrmBridge::IsKeySystemSupportedWithType(key_system, container)) {
DVLOG(1) << "Key system and container '" << container << "' not supported.";
return false;
}

if (!MediaDrmBridge::IsSecurityLevelSupported(key_system, security_level)) {
DVLOG(1) << "Key system and security level '" << security_level
<< "' not supported.";
return false;
}

bool is_secure = MediaDrmBridge::IsSecureDecoderRequired(security_level);
for (size_t i = 0; i < codecs.size(); ++i) {
if (!MediaCodecBridge::CanDecode(codecs[i], is_secure)) {
DVLOG(1) << "Codec '" << codecs[i] << "' "
<< (is_secure ? "in secure mode " : "") << "not supported.";
return false;
}
}

return true;
}

MediaSourcePlayer::MediaSourcePlayer(
int player_id,
MediaPlayerManager* manager,
Expand Down
5 changes: 0 additions & 5 deletions media/base/android/media_source_player.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,6 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid,
scoped_ptr<DemuxerAndroid> demuxer);
virtual ~MediaSourcePlayer();

static bool IsTypeSupported(const std::string& key_system,
MediaDrmBridge::SecurityLevel security_level,
const std::string& container,
const std::vector<std::string>& codecs);

// MediaPlayerAndroid implementation.
virtual void SetVideoSurface(gfx::ScopedJavaSurface surface) OVERRIDE;
virtual void Start() OVERRIDE;
Expand Down
Loading

0 comments on commit 114f025

Please sign in to comment.