From 6c78307a21252c2dbd704f6d5e92a220fb722ed4 Mon Sep 17 00:00:00 2001 From: ehmaldonado Date: Thu, 3 Nov 2016 07:33:17 -0700 Subject: [PATCH] Revert of Remove deprected functions from EncodedImageCallback and RtpRtcp (patchset #4 id:100001 of https://codereview.webrtc.org/2405173006/ ) Reason for revert: This might be breaking projects downstream. Original issue's description: > Remove deprected functions from EncodedImageCallback and RtpRtcp > > Removed EncodedImageCallback::Encoded() and RtpRtcp::SendOutgoingData(). > These methods should no longer be used anywhere and it's safe to remove > them. > > BUG=chromium:621691 > > Committed: https://crrev.com/fa565842718ad178a7562721b25d916fbabc2b92 > Cr-Commit-Position: refs/heads/master@{#14902} TBR=mflodman@webrtc.org,stefan@webrtc.org,sergeyu@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:621691 Review-Url: https://codereview.webrtc.org/2474433008 Cr-Commit-Position: refs/heads/master@{#14914} --- .../android/jni/androidmediaencoder_jni.cc | 15 +++++---- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 19 +++++++++++ .../codecs/h264/h264_encoder_impl.cc | 4 +-- .../codecs/h264/h264_video_toolbox_encoder.mm | 7 ++-- .../modules/video_coding/codecs/i420/i420.cc | 3 +- .../vp8/simulcast_encoder_adapter_unittest.cc | 6 ++-- .../video_coding/codecs/vp8/vp8_impl.cc | 4 +-- .../video_coding/codecs/vp9/vp9_impl.cc | 4 +-- webrtc/modules/video_coding/video_receiver.cc | 4 +-- .../test/configurable_frame_size_encoder.cc | 2 +- webrtc/test/fake_encoder.cc | 4 +-- webrtc/video/payload_router_unittest.cc | 32 +++++-------------- webrtc/video/video_send_stream_tests.cc | 4 +-- webrtc/video/vie_encoder_unittest.cc | 9 +++--- webrtc/video_encoder.h | 21 +++++++++--- 15 files changed, 74 insertions(+), 64 deletions(-) diff --git a/webrtc/api/android/jni/androidmediaencoder_jni.cc b/webrtc/api/android/jni/androidmediaencoder_jni.cc index 92b5aae65c..0e6e96a6c0 100644 --- a/webrtc/api/android/jni/androidmediaencoder_jni.cc +++ b/webrtc/api/android/jni/androidmediaencoder_jni.cc @@ -263,8 +263,8 @@ class MediaCodecVideoEncoder : public webrtc::VideoEncoder, // |input_frame_infos_|. // Frame size in bytes fed to MediaCodec. int yuv_size_; - // True only when between a callback_->OnEncodedImage() call return a positive - // value and the next Encode() call being ignored. + // True only when between a callback_->Encoded() call return a positive value + // and the next Encode() call being ignored. bool drop_next_input_frame_; // Global references; must be deleted in Release(). std::vector input_buffers_; @@ -1063,8 +1063,7 @@ bool MediaCodecVideoEncoder::DeliverPendingOutputs(JNIEnv* jni) { } // Callback - return encoded frame. - webrtc::EncodedImageCallback::Result callback_result( - webrtc::EncodedImageCallback::Result::OK); + int32_t callback_status = 0; if (callback_) { std::unique_ptr image( new webrtc::EncodedImage(payload, payload_size, payload_size)); @@ -1175,7 +1174,7 @@ bool MediaCodecVideoEncoder::DeliverPendingOutputs(JNIEnv* jni) { } } - callback_result = callback_->OnEncodedImage(*image, &info, &header); + callback_status = callback_->Encoded(*image, &info, &header); } // Return output buffer back to the encoder. @@ -1209,9 +1208,11 @@ bool MediaCodecVideoEncoder::DeliverPendingOutputs(JNIEnv* jni) { current_encoding_time_ms_ += frame_encoding_time_ms; LogStatistics(false); - // Errors in callback_result are currently ignored. - if (callback_result.drop_next_frame) + if (callback_status > 0) { drop_next_input_frame_ = true; + // Theoretically could handle callback_status<0 here, but unclear what + // that would mean for us. + } } return true; } diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 0c36117cea..9fa3959c90 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -227,6 +227,7 @@ class RtpRtcp : public Module { // as layers or RED // |transport_frame_id_out| - set to RTP timestamp. // Returns true on success. + virtual bool SendOutgoingData(FrameType frame_type, int8_t payload_type, uint32_t timestamp, @@ -237,6 +238,24 @@ class RtpRtcp : public Module { const RTPVideoHeader* rtp_video_header, uint32_t* transport_frame_id_out) = 0; + // Deprecated version of the method above. + int32_t SendOutgoingData( + FrameType frame_type, + int8_t payload_type, + uint32_t timestamp, + int64_t capture_time_ms, + const uint8_t* payload_data, + size_t payload_size, + const RTPFragmentationHeader* fragmentation = nullptr, + const RTPVideoHeader* rtp_video_header = nullptr) { + return SendOutgoingData(frame_type, payload_type, timestamp, + capture_time_ms, payload_data, payload_size, + fragmentation, rtp_video_header, + /*frame_id_out=*/nullptr) + ? 0 + : -1; + } + virtual bool TimeToSendPacket(uint32_t ssrc, uint16_t sequence_number, int64_t capture_time_ms, diff --git a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc index 5c0aa1bd45..e32a2caf0c 100644 --- a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc +++ b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc @@ -377,8 +377,8 @@ int32_t H264EncoderImpl::Encode(const VideoFrame& input_frame, // Deliver encoded image. CodecSpecificInfo codec_specific; codec_specific.codecType = kVideoCodecH264; - encoded_image_callback_->OnEncodedImage(encoded_image_, &codec_specific, - &frag_header); + encoded_image_callback_->Encoded(encoded_image_, &codec_specific, + &frag_header); // Parse and report QP. h264_bitstream_parser_.ParseBitstream(encoded_image_._buffer, diff --git a/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm b/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm index 538734b0f4..8276448e2c 100644 --- a/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm +++ b/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm @@ -650,10 +650,9 @@ void H264VideoToolboxEncoder::OnEncodedFrame( quality_scaler_.ReportQP(qp); } - EncodedImageCallback::Result result = - callback_->OnEncodedImage(frame, &codec_specific_info, header.get()); - if (result.error != EncodedImageCallback::Result::OK) { - LOG(LS_ERROR) << "Encode callback failed: " << result.error; + int result = callback_->Encoded(frame, &codec_specific_info, header.get()); + if (result != 0) { + LOG(LS_ERROR) << "Encode callback failed: " << result; return; } bitrate_adjuster_.Update(frame._size); diff --git a/webrtc/modules/video_coding/codecs/i420/i420.cc b/webrtc/modules/video_coding/codecs/i420/i420.cc index ad4a8a1a40..d0c8d0c62c 100644 --- a/webrtc/modules/video_coding/codecs/i420/i420.cc +++ b/webrtc/modules/video_coding/codecs/i420/i420.cc @@ -116,8 +116,7 @@ int I420Encoder::Encode(const VideoFrame& inputImage, return WEBRTC_VIDEO_CODEC_MEMORY; _encodedImage._length = ret_length + kI420HeaderSize; - _encodedCompleteCallback->OnEncodedImage(_encodedImage, nullptr, nullptr); - + _encodedCompleteCallback->Encoded(_encodedImage, NULL, NULL); return WEBRTC_VIDEO_CODEC_OK; } diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc index 9a06c0ac5f..8bc3b489ff 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc @@ -152,9 +152,9 @@ class MockVideoEncoder : public VideoEncoder { EncodedImage image; image._encodedWidth = width; image._encodedHeight = height; - CodecSpecificInfo codec_specific_info; - memset(&codec_specific_info, 0, sizeof(codec_specific_info)); - callback_->OnEncodedImage(image, &codec_specific_info, NULL); + CodecSpecificInfo codecSpecificInfo; + memset(&codecSpecificInfo, 0, sizeof(codecSpecificInfo)); + callback_->Encoded(image, &codecSpecificInfo, NULL); } void set_supports_native_handle(bool enabled) { diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc index 1767ec4758..523c19c257 100644 --- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc @@ -1024,8 +1024,8 @@ int VP8EncoderImpl::GetEncodedPartitions(const VideoFrame& input_image, vpx_codec_control(&encoders_[encoder_idx], VP8E_GET_LAST_QUANTIZER, &qp_128); encoded_images_[encoder_idx].qp_ = qp_128; - encoded_complete_callback_->OnEncodedImage(encoded_images_[encoder_idx], - &codec_specific, &frag_info); + encoded_complete_callback_->Encoded(encoded_images_[encoder_idx], + &codec_specific, &frag_info); } else if (codec_.mode == kScreensharing) { result = WEBRTC_VIDEO_CODEC_TARGET_BITRATE_OVERSHOOT; } diff --git a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc index 8c798db665..3a4efb3c3a 100644 --- a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -705,8 +705,8 @@ int VP9EncoderImpl::GetEncodedLayerFrame(const vpx_codec_cx_pkt* pkt) { int qp = -1; vpx_codec_control(encoder_, VP8E_GET_LAST_QUANTIZER, &qp); encoded_image_.qp_ = qp; - encoded_complete_callback_->OnEncodedImage(encoded_image_, &codec_specific, - &frag_info); + encoded_complete_callback_->Encoded(encoded_image_, &codec_specific, + &frag_info); } return WEBRTC_VIDEO_CODEC_OK; } diff --git a/webrtc/modules/video_coding/video_receiver.cc b/webrtc/modules/video_coding/video_receiver.cc index 475f686d23..8b615244d3 100644 --- a/webrtc/modules/video_coding/video_receiver.cc +++ b/webrtc/modules/video_coding/video_receiver.cc @@ -270,8 +270,8 @@ int32_t VideoReceiver::Decode(uint16_t maxWaitTimeMs) { if (qp_parser_.GetQp(*frame, &qp)) { encoded_image.qp_ = qp; } - pre_decode_image_callback_->OnEncodedImage(encoded_image, - frame->CodecSpecific(), nullptr); + pre_decode_image_callback_->Encoded(encoded_image, frame->CodecSpecific(), + nullptr); } rtc::CritScope cs(&receive_crit_); diff --git a/webrtc/test/configurable_frame_size_encoder.cc b/webrtc/test/configurable_frame_size_encoder.cc index 905b69aae8..9cb0c871ae 100644 --- a/webrtc/test/configurable_frame_size_encoder.cc +++ b/webrtc/test/configurable_frame_size_encoder.cc @@ -52,7 +52,7 @@ int32_t ConfigurableFrameSizeEncoder::Encode( RTPFragmentationHeader* fragmentation = NULL; CodecSpecificInfo specific; memset(&specific, 0, sizeof(specific)); - callback_->OnEncodedImage(encodedImage, &specific, fragmentation); + callback_->Encoded(encodedImage, &specific, fragmentation); return WEBRTC_VIDEO_CODEC_OK; } diff --git a/webrtc/test/fake_encoder.cc b/webrtc/test/fake_encoder.cc index f518ce3919..9a4cc7e3aa 100644 --- a/webrtc/test/fake_encoder.cc +++ b/webrtc/test/fake_encoder.cc @@ -112,10 +112,8 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image, encoded.rotation_ = input_image.rotation(); RTC_DCHECK(callback_ != NULL); specifics.codec_name = ImplementationName(); - if (callback_->OnEncodedImage(encoded, &specifics, NULL).error != - EncodedImageCallback::Result::OK) { + if (callback_->Encoded(encoded, &specifics, NULL).error != 0) return -1; - } bits_available -= std::min(encoded._length * 8, bits_available); } return 0; diff --git a/webrtc/video/payload_router_unittest.cc b/webrtc/video/payload_router_unittest.cc index fa5c35d6db..5b87554a54 100644 --- a/webrtc/video/payload_router_unittest.cc +++ b/webrtc/video/payload_router_unittest.cc @@ -45,9 +45,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) .Times(0); - EXPECT_NE( - EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); + EXPECT_EQ(-1, payload_router.Encoded(encoded_image, nullptr, nullptr)); payload_router.set_active(true); EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, @@ -55,9 +53,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) .Times(1); - EXPECT_EQ( - EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); + EXPECT_EQ(0, payload_router.Encoded(encoded_image, nullptr, nullptr)); payload_router.set_active(false); EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, @@ -65,9 +61,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) .Times(0); - EXPECT_NE( - EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); + EXPECT_EQ(-1, payload_router.Encoded(encoded_image, nullptr, nullptr)); payload_router.set_active(true); EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, @@ -75,9 +69,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) .Times(1); - EXPECT_EQ( - EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); + EXPECT_EQ(0, payload_router.Encoded(encoded_image, nullptr, nullptr)); } TEST(PayloadRouterTest, SendSimulcast) { @@ -111,9 +103,7 @@ TEST(PayloadRouterTest, SendSimulcast) { encoded_image._length, nullptr, _, _)) .Times(1); EXPECT_CALL(rtp_2, SendOutgoingData(_, _, _, _, _, _, _, _, _)).Times(0); - EXPECT_EQ(EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, &codec_info_1, nullptr) - .error); + EXPECT_EQ(0, payload_router.Encoded(encoded_image, &codec_info_1, nullptr)); CodecSpecificInfo codec_info_2; memset(&codec_info_2, 0, sizeof(CodecSpecificInfo)); @@ -127,9 +117,7 @@ TEST(PayloadRouterTest, SendSimulcast) { .Times(1); EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _, _)) .Times(0); - EXPECT_EQ(EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, &codec_info_2, nullptr) - .error); + EXPECT_EQ(0, payload_router.Encoded(encoded_image, &codec_info_2, nullptr)); // Inactive. payload_router.set_active(false); @@ -137,12 +125,8 @@ TEST(PayloadRouterTest, SendSimulcast) { .Times(0); EXPECT_CALL(rtp_2, SendOutgoingData(_, _, _, _, _, _, _, _, _)) .Times(0); - EXPECT_NE(EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, &codec_info_1, nullptr) - .error); - EXPECT_NE(EncodedImageCallback::Result::OK, - payload_router.OnEncodedImage(encoded_image, &codec_info_2, nullptr) - .error); + EXPECT_EQ(-1, payload_router.Encoded(encoded_image, &codec_info_1, nullptr)); + EXPECT_EQ(-1, payload_router.Encoded(encoded_image, &codec_info_2, nullptr)); } TEST(PayloadRouterTest, MaxPayloadLength) { diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 2b28f4fad4..6b11b94726 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -2419,10 +2419,8 @@ TEST_F(VideoSendStreamTest, ReportsSentResolution) { encoded._encodedWidth = kEncodedResolution[i].width; encoded._encodedHeight = kEncodedResolution[i].height; RTC_DCHECK(callback_); - if (callback_->OnEncodedImage(encoded, &specifics, nullptr).error != - EncodedImageCallback::Result::OK) { + if (callback_->Encoded(encoded, &specifics, nullptr) != 0) return -1; - } } observation_complete_.Set(); diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 9f4fdc47e3..895109773f 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -208,15 +208,14 @@ class ViEEncoderTest : public ::testing::Test { } private: - Result OnEncodedImage( - const EncodedImage& encoded_image, - const CodecSpecificInfo* codec_specific_info, - const RTPFragmentationHeader* fragmentation) override { + int32_t Encoded(const EncodedImage& encoded_image, + const CodecSpecificInfo* codec_specific_info, + const RTPFragmentationHeader* fragmentation) override { rtc::CritScope lock(&crit_); EXPECT_TRUE(expect_frames_); timestamp_ = encoded_image._timeStamp; encoded_frame_event_.Set(); - return Result(Result::OK, timestamp_); + return 0; } void OnEncoderConfigurationChanged(std::vector streams, diff --git a/webrtc/video_encoder.h b/webrtc/video_encoder.h index 8bfa72fe1e..d8b7921bc0 100644 --- a/webrtc/video_encoder.h +++ b/webrtc/video_encoder.h @@ -54,10 +54,23 @@ class EncodedImageCallback { }; // Callback function which is called when an image has been encoded. - virtual Result OnEncodedImage( - const EncodedImage& encoded_image, - const CodecSpecificInfo* codec_specific_info, - const RTPFragmentationHeader* fragmentation) = 0; + virtual Result OnEncodedImage(const EncodedImage& encoded_image, + const CodecSpecificInfo* codec_specific_info, + const RTPFragmentationHeader* fragmentation) { + return (Encoded(encoded_image, codec_specific_info, fragmentation) == 0) + ? Result(Result::OK, 0) + : Result(Result::ERROR_SEND_FAILED); + } + + // DEPRECATED. + // TODO(sergeyu): Remove this method. + virtual int32_t Encoded(const EncodedImage& encoded_image, + const CodecSpecificInfo* codec_specific_info, + const RTPFragmentationHeader* fragmentation) { + Result result = + OnEncodedImage(encoded_image, codec_specific_info, fragmentation); + return (result.error != Result::OK) ? -1 : (result.drop_next_frame ? 1 : 0); + } }; class VideoEncoder {