From 8fb41a39e1a2d151d1c00c409630dcee80adeb76 Mon Sep 17 00:00:00 2001 From: Tony Herre Date: Fri, 24 Sep 2021 12:05:20 +0000 Subject: [PATCH] Add Direction indicator to TransformableFrames Currently the implementation of FrameTransformers uses distinct, incompatible types for recevied vs about-to-be-sent frames. This adds a flag in the interface so we can at least check that we are being given the correct type. crbug.com/1250638 tracks removing the need for this. Chrome will be updated after this to check the direction flag and provide a javascript error if the wrong type of frame is written into the encoded insertable streams writable stream, rather than crashing. Bug: chromium:1247260 Change-Id: I9cbb66962ea0718ed47c5e5dba19a8ff9635b0b1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232301 Reviewed-by: Harald Alvestrand Commit-Queue: Tony Herre Cr-Commit-Position: refs/heads/main@{#35100} --- api/frame_transformer_interface.h | 10 ++++++ ...nnel_receive_frame_transformer_delegate.cc | 19 +++++++---- ...channel_send_frame_transformer_delegate.cc | 33 +++++++++++-------- ...sender_video_frame_transformer_delegate.cc | 4 +++ ...eam_receiver_frame_transformer_delegate.cc | 4 +++ 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/api/frame_transformer_interface.h b/api/frame_transformer_interface.h index 7653bee3fb..de2c612ac0 100644 --- a/api/frame_transformer_interface.h +++ b/api/frame_transformer_interface.h @@ -36,6 +36,16 @@ class TransformableFrameInterface { virtual uint8_t GetPayloadType() const = 0; virtual uint32_t GetSsrc() const = 0; virtual uint32_t GetTimestamp() const = 0; + + enum class Direction { + kUnknown, + kReceiver, + kSender, + }; + // TODO(crbug.com/1250638): Remove this distinction between receiver and + // sender frames to allow received frames to be directly re-transmitted on + // other PeerConnectionss. + virtual Direction GetDirection() const { return Direction::kUnknown; } }; class TransformableVideoFrameInterface : public TransformableFrameInterface { diff --git a/audio/channel_receive_frame_transformer_delegate.cc b/audio/channel_receive_frame_transformer_delegate.cc index 101fd3ff3f..c9e8a8b29d 100644 --- a/audio/channel_receive_frame_transformer_delegate.cc +++ b/audio/channel_receive_frame_transformer_delegate.cc @@ -18,15 +18,16 @@ namespace webrtc { namespace { -class TransformableAudioFrame : public TransformableAudioFrameInterface { +class TransformableIncomingAudioFrame + : public TransformableAudioFrameInterface { public: - TransformableAudioFrame(rtc::ArrayView payload, - const RTPHeader& header, - uint32_t ssrc) + TransformableIncomingAudioFrame(rtc::ArrayView payload, + const RTPHeader& header, + uint32_t ssrc) : payload_(payload.data(), payload.size()), header_(header), ssrc_(ssrc) {} - ~TransformableAudioFrame() override = default; + ~TransformableIncomingAudioFrame() override = default; rtc::ArrayView GetData() const override { return payload_; } void SetData(rtc::ArrayView data) override { @@ -37,6 +38,7 @@ class TransformableAudioFrame : public TransformableAudioFrameInterface { uint32_t GetSsrc() const override { return ssrc_; } uint32_t GetTimestamp() const override { return header_.timestamp; } const RTPHeader& GetHeader() const override { return header_; } + Direction GetDirection() const override { return Direction::kReceiver; } private: rtc::Buffer payload_; @@ -72,7 +74,7 @@ void ChannelReceiveFrameTransformerDelegate::Transform( uint32_t ssrc) { RTC_DCHECK_RUN_ON(&sequence_checker_); frame_transformer_->Transform( - std::make_unique(packet, header, ssrc)); + std::make_unique(packet, header, ssrc)); } void ChannelReceiveFrameTransformerDelegate::OnTransformedFrame( @@ -89,7 +91,10 @@ void ChannelReceiveFrameTransformerDelegate::ReceiveFrame( RTC_DCHECK_RUN_ON(&sequence_checker_); if (!receive_frame_callback_) return; - auto* transformed_frame = static_cast(frame.get()); + RTC_CHECK_EQ(frame->GetDirection(), + TransformableFrameInterface::Direction::kReceiver); + auto* transformed_frame = + static_cast(frame.get()); receive_frame_callback_(transformed_frame->GetData(), transformed_frame->GetHeader()); } diff --git a/audio/channel_send_frame_transformer_delegate.cc b/audio/channel_send_frame_transformer_delegate.cc index e71460e63e..eee4cd0d96 100644 --- a/audio/channel_send_frame_transformer_delegate.cc +++ b/audio/channel_send_frame_transformer_delegate.cc @@ -15,16 +15,16 @@ namespace webrtc { namespace { -class TransformableAudioFrame : public TransformableFrameInterface { +class TransformableOutgoingAudioFrame : public TransformableFrameInterface { public: - TransformableAudioFrame(AudioFrameType frame_type, - uint8_t payload_type, - uint32_t rtp_timestamp, - uint32_t rtp_start_timestamp, - const uint8_t* payload_data, - size_t payload_size, - int64_t absolute_capture_timestamp_ms, - uint32_t ssrc) + TransformableOutgoingAudioFrame(AudioFrameType frame_type, + uint8_t payload_type, + uint32_t rtp_timestamp, + uint32_t rtp_start_timestamp, + const uint8_t* payload_data, + size_t payload_size, + int64_t absolute_capture_timestamp_ms, + uint32_t ssrc) : frame_type_(frame_type), payload_type_(payload_type), rtp_timestamp_(rtp_timestamp), @@ -32,7 +32,7 @@ class TransformableAudioFrame : public TransformableFrameInterface { payload_(payload_data, payload_size), absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms), ssrc_(ssrc) {} - ~TransformableAudioFrame() override = default; + ~TransformableOutgoingAudioFrame() override = default; rtc::ArrayView GetData() const override { return payload_; } void SetData(rtc::ArrayView data) override { payload_.SetData(data.data(), data.size()); @@ -48,6 +48,7 @@ class TransformableAudioFrame : public TransformableFrameInterface { int64_t GetAbsoluteCaptureTimestampMs() const { return absolute_capture_timestamp_ms_; } + Direction GetDirection() const override { return Direction::kSender; } private: AudioFrameType frame_type_; @@ -90,9 +91,10 @@ void ChannelSendFrameTransformerDelegate::Transform( size_t payload_size, int64_t absolute_capture_timestamp_ms, uint32_t ssrc) { - frame_transformer_->Transform(std::make_unique( - frame_type, payload_type, rtp_timestamp, rtp_start_timestamp, - payload_data, payload_size, absolute_capture_timestamp_ms, ssrc)); + frame_transformer_->Transform( + std::make_unique( + frame_type, payload_type, rtp_timestamp, rtp_start_timestamp, + payload_data, payload_size, absolute_capture_timestamp_ms, ssrc)); } void ChannelSendFrameTransformerDelegate::OnTransformedFrame( @@ -111,9 +113,12 @@ void ChannelSendFrameTransformerDelegate::SendFrame( std::unique_ptr frame) const { MutexLock lock(&send_lock_); RTC_DCHECK_RUN_ON(encoder_queue_); + RTC_CHECK_EQ(frame->GetDirection(), + TransformableFrameInterface::Direction::kSender); if (!send_frame_callback_) return; - auto* transformed_frame = static_cast(frame.get()); + auto* transformed_frame = + static_cast(frame.get()); send_frame_callback_(transformed_frame->GetFrameType(), transformed_frame->GetPayloadType(), transformed_frame->GetTimestamp() - diff --git a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc index e08479a848..377f6c4fbf 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc @@ -78,6 +78,8 @@ class TransformableVideoSenderFrame : public TransformableVideoFrameInterface { return expected_retransmission_time_ms_; } + Direction GetDirection() const override { return Direction::kSender; } + private: rtc::scoped_refptr encoded_data_; const RTPVideoHeader header_; @@ -147,6 +149,8 @@ void RTPSenderVideoFrameTransformerDelegate::OnTransformedFrame( void RTPSenderVideoFrameTransformerDelegate::SendVideo( std::unique_ptr transformed_frame) const { RTC_CHECK(encoder_queue_->IsCurrent()); + RTC_CHECK_EQ(transformed_frame->GetDirection(), + TransformableFrameInterface::Direction::kSender); MutexLock lock(&sender_lock_); if (!sender_) return; diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc index c9069815b1..c54939fe5a 100644 --- a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc +++ b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc @@ -59,6 +59,8 @@ class TransformableVideoReceiverFrame return std::move(frame_); } + Direction GetDirection() const override { return Direction::kReceiver; } + private: std::unique_ptr frame_; const VideoFrameMetadata metadata_; @@ -111,6 +113,8 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::OnTransformedFrame( void RtpVideoStreamReceiverFrameTransformerDelegate::ManageFrame( std::unique_ptr frame) { RTC_DCHECK_RUN_ON(&network_sequence_checker_); + RTC_CHECK_EQ(frame->GetDirection(), + TransformableFrameInterface::Direction::kReceiver); if (!receiver_) return; auto transformed_frame = absl::WrapUnique(