From 6bf15126dca907d69e6f6873c9c4c8997b8e8227 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Thu, 28 Mar 2019 11:13:02 +0100 Subject: [PATCH] Refactor VideoEncoderWrapper, to let EncodedImage own the data buffer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9378 Change-Id: I0c218511251e6460f7a9f2e044eb61d0d6bf635d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129921 Commit-Queue: Niels Moller Reviewed-by: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#27334} --- sdk/android/src/jni/video_encoder_wrapper.cc | 125 +++++++++---------- sdk/android/src/jni/video_encoder_wrapper.h | 4 +- 2 files changed, 59 insertions(+), 70 deletions(-) diff --git a/sdk/android/src/jni/video_encoder_wrapper.cc b/sdk/android/src/jni/video_encoder_wrapper.cc index 4cf8febc81..5ba140424c 100644 --- a/sdk/android/src/jni/video_encoder_wrapper.cc +++ b/sdk/android/src/jni/video_encoder_wrapper.cc @@ -226,78 +226,67 @@ void VideoEncoderWrapper::OnEncodedFrame(JNIEnv* jni, static_cast(jni->GetDirectBufferAddress(j_buffer.obj())); const size_t buffer_size = jni->GetDirectBufferCapacity(j_buffer.obj()); - std::vector buffer_copy(buffer_size); - memcpy(buffer_copy.data(), buffer, buffer_size); - const int qp = JavaToNativeOptionalInt(jni, j_qp).value_or(-1); + EncodedImage frame; + frame.Allocate(buffer_size); + frame.set_size(buffer_size); + memcpy(frame.data(), buffer, buffer_size); + frame._encodedWidth = encoded_width; + frame._encodedHeight = encoded_height; + frame.rotation_ = (VideoRotation)rotation; + frame._completeFrame = complete_frame; - struct Lambda { - VideoEncoderWrapper* video_encoder_wrapper; - std::vector task_buffer; - int qp; - jint encoded_width; - jint encoded_height; - jlong capture_time_ns; - jint frame_type; - jint rotation; - jboolean complete_frame; - std::deque* frame_extra_infos; - EncodedImageCallback* callback; + const absl::optional qp = JavaToNativeOptionalInt(jni, j_qp); - void operator()() const { - // Encoded frames are delivered in the order received, but some of them - // may be dropped, so remove records of frames older than the current one. - // - // NOTE: if the current frame is associated with Encoder A, in the time - // since this frame was received, Encoder A could have been Release()'ed, - // Encoder B InitEncode()'ed (due to reuse of Encoder A), and frames - // received by Encoder B. Thus there may be frame_extra_infos entries that - // don't belong to us, and we need to be careful not to remove them. - // Removing only those entries older than the current frame provides this - // guarantee. - while (!frame_extra_infos->empty() && - frame_extra_infos->front().capture_time_ns < capture_time_ns) { - frame_extra_infos->pop_front(); - } - if (frame_extra_infos->empty() || - frame_extra_infos->front().capture_time_ns != capture_time_ns) { - RTC_LOG(LS_WARNING) - << "Java encoder produced an unexpected frame with timestamp: " - << capture_time_ns; - return; - } - FrameExtraInfo frame_extra_info = std::move(frame_extra_infos->front()); - frame_extra_infos->pop_front(); - - RTPFragmentationHeader header = - video_encoder_wrapper->ParseFragmentationHeader(task_buffer); - EncodedImage frame(const_cast(task_buffer.data()), - task_buffer.size(), task_buffer.size()); - frame._encodedWidth = encoded_width; - frame._encodedHeight = encoded_height; - frame.SetTimestamp(frame_extra_info.timestamp_rtp); - frame.capture_time_ms_ = capture_time_ns / rtc::kNumNanosecsPerMillisec; - frame._frameType = (VideoFrameType)frame_type; - frame.rotation_ = (VideoRotation)rotation; - frame._completeFrame = complete_frame; - if (qp == -1) { - frame.qp_ = video_encoder_wrapper->ParseQp(task_buffer); - } else { - frame.qp_ = qp; - } - - CodecSpecificInfo info( - video_encoder_wrapper->ParseCodecSpecificInfo(frame)); - callback->OnEncodedImage(frame, &info, &header); - } - }; + frame._frameType = (VideoFrameType)frame_type; { rtc::CritScope lock(&encoder_queue_crit_); if (encoder_queue_ != nullptr) { - encoder_queue_->PostTask(ToQueuedTask( - Lambda{this, std::move(buffer_copy), qp, encoded_width, - encoded_height, capture_time_ns, frame_type, rotation, - complete_frame, &frame_extra_infos_, callback_})); + encoder_queue_->PostTask(ToQueuedTask([this, frame, qp, + capture_time_ns]() { + // Encoded frames are delivered in the order received, but some of them + // may be dropped, so remove records of frames older than the current + // one. + // + // NOTE: if the current frame is associated with Encoder A, in the time + // since this frame was received, Encoder A could have been + // Release()'ed, Encoder B InitEncode()'ed (due to reuse of Encoder A), + // and frames received by Encoder B. Thus there may be frame_extra_infos + // entries that don't belong to us, and we need to be careful not to + // remove them. Removing only those entries older than the current frame + // provides this guarantee. + while (!frame_extra_infos_.empty() && + frame_extra_infos_.front().capture_time_ns < capture_time_ns) { + frame_extra_infos_.pop_front(); + } + if (frame_extra_infos_.empty() || + frame_extra_infos_.front().capture_time_ns != capture_time_ns) { + RTC_LOG(LS_WARNING) + << "Java encoder produced an unexpected frame with timestamp: " + << capture_time_ns; + return; + } + FrameExtraInfo frame_extra_info = std::move(frame_extra_infos_.front()); + frame_extra_infos_.pop_front(); + + // This is a bit subtle. The |frame| variable from the lambda capture is + // const. Which implies that (i) we need to make a copy to be able to + // write to the metadata, and (ii) we should avoid using the .data() + // method (including implicit conversion to ArrayView) on the non-const + // copy, since that would trigget a copy operation on the underlying + // CopyOnWriteBuffer. + EncodedImage frame_copy = frame; + + frame_copy.SetTimestamp(frame_extra_info.timestamp_rtp); + frame_copy.capture_time_ms_ = + capture_time_ns / rtc::kNumNanosecsPerMillisec; + + RTPFragmentationHeader header = ParseFragmentationHeader(frame); + frame_copy.qp_ = qp ? *qp : ParseQp(frame); + CodecSpecificInfo info(ParseCodecSpecificInfo(frame)); + + callback_->OnEncodedImage(frame_copy, &info, &header); + })); } } } @@ -329,7 +318,7 @@ int32_t VideoEncoderWrapper::HandleReturnCode(JNIEnv* jni, } RTPFragmentationHeader VideoEncoderWrapper::ParseFragmentationHeader( - const std::vector& buffer) { + rtc::ArrayView buffer) { RTPFragmentationHeader header; if (codec_settings_.codecType == kVideoCodecH264) { h264_bitstream_parser_.ParseBitstream(buffer.data(), buffer.size()); @@ -361,7 +350,7 @@ RTPFragmentationHeader VideoEncoderWrapper::ParseFragmentationHeader( return header; } -int VideoEncoderWrapper::ParseQp(const std::vector& buffer) { +int VideoEncoderWrapper::ParseQp(rtc::ArrayView buffer) { int qp; bool success; switch (codec_settings_.codecType) { diff --git a/sdk/android/src/jni/video_encoder_wrapper.h b/sdk/android/src/jni/video_encoder_wrapper.h index f6f86abb43..af369c6ee3 100644 --- a/sdk/android/src/jni/video_encoder_wrapper.h +++ b/sdk/android/src/jni/video_encoder_wrapper.h @@ -78,8 +78,8 @@ class VideoEncoderWrapper : public VideoEncoder { const char* method_name); RTPFragmentationHeader ParseFragmentationHeader( - const std::vector& buffer); - int ParseQp(const std::vector& buffer); + rtc::ArrayView buffer); + int ParseQp(rtc::ArrayView buffer); CodecSpecificInfo ParseCodecSpecificInfo(const EncodedImage& frame); ScopedJavaLocalRef ToJavaBitrateAllocation( JNIEnv* jni,