Reland of Stop using hardcoded payload types for video codecs (patchset #1 id:1 of https://codereview.webrtc.org/2513633002/ )
Reason for revert:
The WebRtcBrowserTest.NegotiateUnsupportedVideoCodec test has been fixed in Chromium with the following change:
function removeVideoCodec(offerSdp) {
- offerSdp = offerSdp.replace('a=rtpmap:100 VP8/90000\r\n',
- 'a=rtpmap:100 XVP8/90000\r\n');
+ offerSdp = offerSdp.replace(/a=rtpmap:(\d+)\ VP8\/90000\r\n/,
+ 'a=rtpmap:$1 XVP8/90000\r\n');
return offerSdp;
}
Original issue's description:
> Revert of Stop using hardcoded payload types for video codecs (patchset #6 id:210001 of https://codereview.webrtc.org/2493133002/ )
>
> Reason for revert:
> Breaks chromium.fyi test:
> WebRtcBrowserTest.NegotiateUnsupportedVideoCodec
>
> Original issue's description:
> > Stop using hardcoded payload types for video codecs
> >
> > This CL stops using hardcoded payload types for different video codecs
> > and will dynamically assign them payload types incrementally from 96 to
> > 127 instead.
> >
> > This CL:
> > * Replaces 'std::vector<VideoCodec> DefaultVideoCodecList()' in
> > webrtcvideoengine2.cc with an explicit WebRtcVideoEncoderFactory for
> > internally supported software codecs instead. The purpose is to
> > streamline the payload type assignment in webrtcvideoengine2.cc which
> > will now have two encoder factories of the same
> > WebRtcVideoEncoderFactory type; one internal and one external.
> > * Removes webrtc::VideoEncoder::EncoderType and use cricket::VideoCodec
> > instead.
> > * Removes 'static VideoEncoder* Create(EncoderType codec_type)' and
> > moves the create function to the internal encoder factory instead.
> > * Removes video_encoder.cc. webrtc::VideoEncoder is now just an
> > interface without any static functions.
> > * The function GetSupportedCodecs in webrtcvideoengine2.cc unifies
> > the internal and external codecs and assigns them payload types
> > incrementally from 96 to 127.
> > * Updates webrtcvideoengine2_unittest.cc and removes assumptions about
> > what payload types will be used.
> >
> > BUG=webrtc:6677,webrtc:6705
> > R=hta@webrtc.org, ossu@webrtc.org, stefan@webrtc.org
> >
> > Committed: https://crrev.com/42043b95872b51321f508bf255d804ce3dff366b
> > Cr-Commit-Position: refs/heads/master@{#15135}
>
> TBR=hta@webrtc.org,stefan@webrtc.org,ossu@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:6677,webrtc:6705
>
> Committed: https://crrev.com/eacbaea920797ff751ca83050d140821f5055591
> Cr-Commit-Position: refs/heads/master@{#15140}
TBR=hta@webrtc.org,stefan@webrtc.org,ossu@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6677,webrtc:6705
Review-Url: https://codereview.webrtc.org/2511933002
Cr-Commit-Position: refs/heads/master@{#15148}
diff --git a/webrtc/api/android/jni/androidmediaencoder_jni.cc b/webrtc/api/android/jni/androidmediaencoder_jni.cc
index 6fcb0fb..d5998c0 100644
--- a/webrtc/api/android/jni/androidmediaencoder_jni.cc
+++ b/webrtc/api/android/jni/androidmediaencoder_jni.cc
@@ -30,6 +30,7 @@
#include "webrtc/base/timeutils.h"
#include "webrtc/common_types.h"
#include "webrtc/common_video/h264/h264_bitstream_parser.h"
+#include "webrtc/media/engine/internalencoderfactory.h"
#include "webrtc/modules/video_coding/include/video_codec_interface.h"
#include "webrtc/modules/video_coding/utility/quality_scaler.h"
#include "webrtc/modules/video_coding/utility/vp8_header_parser.h"
@@ -96,7 +97,7 @@
public:
virtual ~MediaCodecVideoEncoder();
MediaCodecVideoEncoder(JNIEnv* jni,
- VideoCodecType codecType,
+ const cricket::VideoCodec& codec,
jobject egl_context);
// webrtc::VideoEncoder implementation. Everything trampolines to
@@ -186,7 +187,7 @@
void LogStatistics(bool force_log);
// Type of video codec.
- VideoCodecType codecType_;
+ const cricket::VideoCodec codec_;
// Valid all the time since RegisterEncodeCompleteCallback() Invoke()s to
// |codec_thread_| synchronously.
@@ -302,9 +303,9 @@
}
MediaCodecVideoEncoder::MediaCodecVideoEncoder(JNIEnv* jni,
- VideoCodecType codecType,
+ const cricket::VideoCodec& codec,
jobject egl_context)
- : codecType_(codecType),
+ : codec_(codec),
callback_(NULL),
codec_thread_(new Thread()),
j_media_codec_video_encoder_class_(
@@ -392,9 +393,10 @@
return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
}
// Factory should guard against other codecs being used with us.
- RTC_CHECK(codec_settings->codecType == codecType_)
+ const VideoCodecType codec_type = cricket::CodecTypeFromName(codec_.name);
+ RTC_CHECK(codec_settings->codecType == codec_type)
<< "Unsupported codec " << codec_settings->codecType << " for "
- << codecType_;
+ << codec_type;
if (sw_fallback_required_) {
return WEBRTC_VIDEO_CODEC_OK;
}
@@ -404,9 +406,9 @@
// Scaling is disabled for VP9, but optionally enabled for VP8.
// TODO(pbos): Extract automaticResizeOn out of VP8 settings.
scale_ = false;
- if (codecType_ == kVideoCodecVP8) {
+ if (codec_type == kVideoCodecVP8) {
scale_ = codec_settings->VP8().automaticResizeOn;
- } else if (codecType_ != kVideoCodecVP9) {
+ } else if (codec_type != kVideoCodecVP9) {
scale_ = true;
}
@@ -414,8 +416,8 @@
ALOGD << "Encoder automatic resize " << (scale_ ? "enabled" : "disabled");
if (scale_) {
- if (codecType_ == kVideoCodecVP8 || codecType_ == kVideoCodecH264) {
- quality_scaler_.Init(codecType_, codec_settings->startBitrate,
+ if (codec_type == kVideoCodecVP8 || codec_type == kVideoCodecH264) {
+ quality_scaler_.Init(codec_type, codec_settings->startBitrate,
codec_settings->width, codec_settings->height,
codec_settings->maxFramerate);
} else {
@@ -522,8 +524,8 @@
bool MediaCodecVideoEncoder::ProcessHWErrorOnCodecThread(
bool reset_if_fallback_unavailable) {
ALOGE << "ProcessHWErrorOnCodecThread";
- if (VideoEncoder::IsSupportedSoftware(
- VideoEncoder::CodecToEncoderType(codecType_))) {
+ if (FindMatchingCodec(cricket::InternalEncoderFactory().supported_codecs(),
+ codec_)) {
ALOGE << "Fallback to SW encoder.";
sw_fallback_required_ = true;
return false;
@@ -550,9 +552,9 @@
JNIEnv* jni = AttachCurrentThreadIfNeeded();
ScopedLocalRefFrame local_ref_frame(jni);
- ALOGD << "InitEncodeOnCodecThread Type: " << (int)codecType_ << ", " <<
- width << " x " << height << ". Bitrate: " << kbps <<
- " kbps. Fps: " << fps;
+ const VideoCodecType codec_type = cricket::CodecTypeFromName(codec_.name);
+ ALOGD << "InitEncodeOnCodecThread Type: " << (int)codec_type << ", " << width
+ << " x " << height << ". Bitrate: " << kbps << " kbps. Fps: " << fps;
if (kbps == 0) {
kbps = last_set_bitrate_kbps_;
}
@@ -591,7 +593,7 @@
// We enforce no extra stride/padding in the format creation step.
jobject j_video_codec_enum = JavaEnumFromIndexAndClassName(
- jni, "MediaCodecVideoEncoder$VideoCodecType", codecType_);
+ jni, "MediaCodecVideoEncoder$VideoCodecType", codec_type);
const bool encode_status = jni->CallBooleanMethod(
*j_media_codec_video_encoder_, j_init_encode_method_,
j_video_codec_enum, width, height, kbps, fps,
@@ -1065,6 +1067,7 @@
}
// Callback - return encoded frame.
+ const VideoCodecType codec_type = cricket::CodecTypeFromName(codec_.name);
webrtc::EncodedImageCallback::Result callback_result(
webrtc::EncodedImageCallback::Result::OK);
if (callback_) {
@@ -1083,8 +1086,8 @@
webrtc::CodecSpecificInfo info;
memset(&info, 0, sizeof(info));
- info.codecType = codecType_;
- if (codecType_ == kVideoCodecVP8) {
+ info.codecType = codec_type;
+ if (codec_type == kVideoCodecVP8) {
info.codecSpecific.VP8.pictureId = picture_id_;
info.codecSpecific.VP8.nonReference = false;
info.codecSpecific.VP8.simulcastIdx = 0;
@@ -1092,7 +1095,7 @@
info.codecSpecific.VP8.layerSync = false;
info.codecSpecific.VP8.tl0PicIdx = webrtc::kNoTl0PicIdx;
info.codecSpecific.VP8.keyIdx = webrtc::kNoKeyIdx;
- } else if (codecType_ == kVideoCodecVP9) {
+ } else if (codec_type == kVideoCodecVP9) {
if (key_frame) {
gof_idx_ = 0;
}
@@ -1121,13 +1124,13 @@
// Generate a header describing a single fragment.
webrtc::RTPFragmentationHeader header;
memset(&header, 0, sizeof(header));
- if (codecType_ == kVideoCodecVP8 || codecType_ == kVideoCodecVP9) {
+ if (codec_type == kVideoCodecVP8 || codec_type == kVideoCodecVP9) {
header.VerifyAndAllocateFragmentationHeader(1);
header.fragmentationOffset[0] = 0;
header.fragmentationLength[0] = image->_length;
header.fragmentationPlType[0] = 0;
header.fragmentationTimeDiff[0] = 0;
- if (codecType_ == kVideoCodecVP8 && scale_) {
+ if (codec_type == kVideoCodecVP8 && scale_) {
int qp;
if (webrtc::vp8::GetQp(payload, payload_size, &qp)) {
current_acc_qp_ += qp;
@@ -1135,7 +1138,7 @@
image->qp_ = qp;
}
}
- } else if (codecType_ == kVideoCodecH264) {
+ } else if (codec_type == kVideoCodecH264) {
if (scale_) {
h264_bitstream_parser_.ParseBitstream(payload, payload_size);
int qp;
@@ -1359,8 +1362,7 @@
}
if (FindMatchingCodec(supported_codecs_, codec)) {
ALOGD << "Create HW video encoder for " << codec.name;
- const VideoCodecType type = cricket::CodecTypeFromName(codec.name);
- return new MediaCodecVideoEncoder(AttachCurrentThreadIfNeeded(), type,
+ return new MediaCodecVideoEncoder(AttachCurrentThreadIfNeeded(), codec,
egl_context_);
}
ALOGW << "Can not find HW video encoder for type " << codec.name;
diff --git a/webrtc/api/webrtcsdp_unittest.cc b/webrtc/api/webrtcsdp_unittest.cc
index 3c6643f..7941323 100644
--- a/webrtc/api/webrtcsdp_unittest.cc
+++ b/webrtc/api/webrtcsdp_unittest.cc
@@ -2083,11 +2083,12 @@
}
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) {
- if (!webrtc::H264Encoder::IsSupported())
- return;
- for (const auto& codec : cricket::DefaultVideoCodecList()) {
- video_desc_->AddCodec(codec);
- }
+ cricket::VideoCodec h264_codec("H264");
+ h264_codec.SetParam("profile-level-id", "42e01f");
+ h264_codec.SetParam("level-asymmetry-allowed", "1");
+ h264_codec.SetParam("packetization-mode", "1");
+ video_desc_->AddCodec(h264_codec);
+
jdesc_.Initialize(desc_.Copy(), kSessionId, kSessionVersion);
std::string message = webrtc::SdpSerialize(jdesc_, false);
