From fc68f1f7d90a2ea4b2609bacbacd48f7ecc10d0f Mon Sep 17 00:00:00 2001 From: Tony Herre Date: Thu, 22 Jun 2023 13:32:24 +0000 Subject: [PATCH] Stop using TransformableAudioFrameInterface::GetHeader() within webrtc Instead switch to specific getters, or methods only defined on specific implementations rather than part of the public API. Once uses are removed from Chromium, I'll mark GetHeader() deprecated and eventually remove it. Bug: chromium:1456628 Change-Id: I19b80489b3a0322c201e24994494cfbb742ee13e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/309780 Commit-Queue: Tony Herre Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#40344} --- api/frame_transformer_interface.h | 12 ++++ api/test/mock_transformable_audio_frame.h | 8 +++ ...nnel_receive_frame_transformer_delegate.cc | 14 ++++- ...channel_send_frame_transformer_delegate.cc | 63 ++++++++++++++----- 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/api/frame_transformer_interface.h b/api/frame_transformer_interface.h index c47bad4243..47058d515f 100644 --- a/api/frame_transformer_interface.h +++ b/api/frame_transformer_interface.h @@ -83,6 +83,18 @@ class TransformableAudioFrameInterface : public TransformableFrameInterface { virtual const absl::optional SequenceNumber() const { return absl::nullopt; } + + // TODO(crbug.com/1456628): Change this to pure virtual after it + // is implemented everywhere. + virtual absl::optional AbsoluteCaptureTimestamp() const { + return absl::nullopt; + } + + enum class FrameType { kEmptyFrame, kAudioFrameSpeech, kAudioFrameCN }; + + // TODO(crbug.com/1456628): Change this to pure virtual after it + // is implemented everywhere. + virtual FrameType Type() const { return FrameType::kEmptyFrame; } }; // Objects implement this interface to be notified with the transformed frame. diff --git a/api/test/mock_transformable_audio_frame.h b/api/test/mock_transformable_audio_frame.h index 9b52e5c579..680d7a0fdc 100644 --- a/api/test/mock_transformable_audio_frame.h +++ b/api/test/mock_transformable_audio_frame.h @@ -37,6 +37,14 @@ class MockTransformableAudioFrame : public TransformableAudioFrameInterface { GetDirection, (), (const, override)); + MOCK_METHOD(absl::optional, + AbsoluteCaptureTimestamp, + (), + (const, override)); + MOCK_METHOD(TransformableAudioFrameInterface::FrameType, + Type, + (), + (const, override)); }; } // namespace webrtc diff --git a/audio/channel_receive_frame_transformer_delegate.cc b/audio/channel_receive_frame_transformer_delegate.cc index 8f817c6fe7..3cc9a01204 100644 --- a/audio/channel_receive_frame_transformer_delegate.cc +++ b/audio/channel_receive_frame_transformer_delegate.cc @@ -50,6 +50,18 @@ class TransformableIncomingAudioFrame return header_.sequenceNumber; } + absl::optional AbsoluteCaptureTimestamp() const override { + // This could be extracted from received header extensions + extrapolation, + // if required in future, eg for being able to re-send received frames. + return absl::nullopt; + } + const RTPHeader& Header() const { return header_; } + + FrameType Type() const override { + return header_.extension.voiceActivity ? FrameType::kAudioFrameSpeech + : FrameType::kAudioFrameCN; + } + private: rtc::Buffer payload_; RTPHeader header_; @@ -106,6 +118,6 @@ void ChannelReceiveFrameTransformerDelegate::ReceiveFrame( auto* transformed_frame = static_cast(frame.get()); receive_frame_callback_(transformed_frame->GetData(), - transformed_frame->GetHeader()); + transformed_frame->Header()); } } // namespace webrtc diff --git a/audio/channel_send_frame_transformer_delegate.cc b/audio/channel_send_frame_transformer_delegate.cc index 93aaa4324d..8c9725681b 100644 --- a/audio/channel_send_frame_transformer_delegate.cc +++ b/audio/channel_send_frame_transformer_delegate.cc @@ -15,6 +15,36 @@ namespace webrtc { namespace { +using IfaceFrameType = TransformableAudioFrameInterface::FrameType; + +IfaceFrameType InternalFrameTypeToInterfaceFrameType( + const AudioFrameType frame_type) { + switch (frame_type) { + case AudioFrameType::kEmptyFrame: + return IfaceFrameType::kEmptyFrame; + case AudioFrameType::kAudioFrameSpeech: + return IfaceFrameType::kAudioFrameSpeech; + case AudioFrameType::kAudioFrameCN: + return IfaceFrameType::kAudioFrameCN; + } + RTC_DCHECK_NOTREACHED(); + return IfaceFrameType::kEmptyFrame; +} + +AudioFrameType InterfaceFrameTypeToInternalFrameType( + const IfaceFrameType frame_type) { + switch (frame_type) { + case IfaceFrameType::kEmptyFrame: + return AudioFrameType::kEmptyFrame; + case IfaceFrameType::kAudioFrameSpeech: + return AudioFrameType::kAudioFrameSpeech; + case IfaceFrameType::kAudioFrameCN: + return AudioFrameType::kAudioFrameCN; + } + RTC_DCHECK_NOTREACHED(); + return AudioFrameType::kEmptyFrame; +} + class TransformableOutgoingAudioFrame : public TransformableAudioFrameInterface { public: @@ -44,11 +74,11 @@ class TransformableOutgoingAudioFrame uint32_t GetStartTimestamp() const { return rtp_start_timestamp_; } uint32_t GetSsrc() const override { return ssrc_; } - AudioFrameType GetFrameType() const { return frame_type_; } - uint8_t GetPayloadType() const override { return payload_type_; } - int64_t GetAbsoluteCaptureTimestampMs() const { - return absolute_capture_timestamp_ms_; + IfaceFrameType Type() const override { + return InternalFrameTypeToInterfaceFrameType(frame_type_); } + + uint8_t GetPayloadType() const override { return payload_type_; } Direction GetDirection() const override { return Direction::kSender; } // TODO(crbug.com/1453226): Remove once GetHeader() is removed from @@ -67,6 +97,10 @@ class TransformableOutgoingAudioFrame rtp_timestamp_ = timestamp - rtp_start_timestamp_; } + absl::optional AbsoluteCaptureTimestamp() const override { + return absolute_capture_timestamp_ms_; + } + private: AudioFrameType frame_type_; uint8_t payload_type_; @@ -140,24 +174,21 @@ void ChannelSendFrameTransformerDelegate::SendFrame( return; auto* transformed_frame = static_cast(frame.get()); - send_frame_callback_(transformed_frame->GetFrameType(), - transformed_frame->GetPayloadType(), - transformed_frame->GetTimestamp() - - transformed_frame->GetStartTimestamp(), - transformed_frame->GetData(), - transformed_frame->GetAbsoluteCaptureTimestampMs()); + send_frame_callback_( + InterfaceFrameTypeToInternalFrameType(transformed_frame->Type()), + transformed_frame->GetPayloadType(), + transformed_frame->GetTimestamp() - + transformed_frame->GetStartTimestamp(), + transformed_frame->GetData(), + *transformed_frame->AbsoluteCaptureTimestamp()); } std::unique_ptr CloneSenderAudioFrame( TransformableAudioFrameInterface* original) { - AudioFrameType audio_frame_type = - original->GetHeader().extension.voiceActivity - ? AudioFrameType::kAudioFrameSpeech - : AudioFrameType::kAudioFrameCN; - // TODO(crbug.com/webrtc/14949): Ensure the correct timestamps are passed. return std::make_unique( - audio_frame_type, original->GetPayloadType(), original->GetTimestamp(), + InterfaceFrameTypeToInternalFrameType(original->Type()), + original->GetPayloadType(), original->GetTimestamp(), /*rtp_start_timestamp=*/0u, original->GetData().data(), original->GetData().size(), original->GetTimestamp(), original->GetSsrc());