Skip to content

Commit

Permalink
[Merge to 95] Add Direction indicator to TransformableFrames
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 8fb41a3)

Bug: chromium:1247260
Change-Id: I9cbb66962ea0718ed47c5e5dba19a8ff9635b0b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232301
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tony Herre <toprice@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#35100}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/233941
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/branch-heads/4638@{#2}
Cr-Branched-From: fb50179-refs/heads/main@{#34960}
  • Loading branch information
Tony Herre authored and WebRTC LUCI CQ committed Oct 4, 2021
1 parent 494c787 commit 8d8c0b4
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 21 deletions.
10 changes: 10 additions & 0 deletions api/frame_transformer_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 12 additions & 7 deletions audio/channel_receive_frame_transformer_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@
namespace webrtc {
namespace {

class TransformableAudioFrame : public TransformableAudioFrameInterface {
class TransformableIncomingAudioFrame
: public TransformableAudioFrameInterface {
public:
TransformableAudioFrame(rtc::ArrayView<const uint8_t> payload,
const RTPHeader& header,
uint32_t ssrc)
TransformableIncomingAudioFrame(rtc::ArrayView<const uint8_t> payload,
const RTPHeader& header,
uint32_t ssrc)
: payload_(payload.data(), payload.size()),
header_(header),
ssrc_(ssrc) {}
~TransformableAudioFrame() override = default;
~TransformableIncomingAudioFrame() override = default;
rtc::ArrayView<const uint8_t> GetData() const override { return payload_; }

void SetData(rtc::ArrayView<const uint8_t> data) override {
Expand All @@ -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_;
Expand Down Expand Up @@ -72,7 +74,7 @@ void ChannelReceiveFrameTransformerDelegate::Transform(
uint32_t ssrc) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
frame_transformer_->Transform(
std::make_unique<TransformableAudioFrame>(packet, header, ssrc));
std::make_unique<TransformableIncomingAudioFrame>(packet, header, ssrc));
}

void ChannelReceiveFrameTransformerDelegate::OnTransformedFrame(
Expand All @@ -89,7 +91,10 @@ void ChannelReceiveFrameTransformerDelegate::ReceiveFrame(
RTC_DCHECK_RUN_ON(&sequence_checker_);
if (!receive_frame_callback_)
return;
auto* transformed_frame = static_cast<TransformableAudioFrame*>(frame.get());
RTC_CHECK_EQ(frame->GetDirection(),
TransformableFrameInterface::Direction::kReceiver);
auto* transformed_frame =
static_cast<TransformableIncomingAudioFrame*>(frame.get());
receive_frame_callback_(transformed_frame->GetData(),
transformed_frame->GetHeader());
}
Expand Down
33 changes: 19 additions & 14 deletions audio/channel_send_frame_transformer_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@
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),
rtp_start_timestamp_(rtp_start_timestamp),
payload_(payload_data, payload_size),
absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms),
ssrc_(ssrc) {}
~TransformableAudioFrame() override = default;
~TransformableOutgoingAudioFrame() override = default;
rtc::ArrayView<const uint8_t> GetData() const override { return payload_; }
void SetData(rtc::ArrayView<const uint8_t> data) override {
payload_.SetData(data.data(), data.size());
Expand All @@ -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_;
Expand Down Expand Up @@ -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<TransformableAudioFrame>(
frame_type, payload_type, rtp_timestamp, rtp_start_timestamp,
payload_data, payload_size, absolute_capture_timestamp_ms, ssrc));
frame_transformer_->Transform(
std::make_unique<TransformableOutgoingAudioFrame>(
frame_type, payload_type, rtp_timestamp, rtp_start_timestamp,
payload_data, payload_size, absolute_capture_timestamp_ms, ssrc));
}

void ChannelSendFrameTransformerDelegate::OnTransformedFrame(
Expand All @@ -111,9 +113,12 @@ void ChannelSendFrameTransformerDelegate::SendFrame(
std::unique_ptr<TransformableFrameInterface> 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<TransformableAudioFrame*>(frame.get());
auto* transformed_frame =
static_cast<TransformableOutgoingAudioFrame*>(frame.get());
send_frame_callback_(transformed_frame->GetFrameType(),
transformed_frame->GetPayloadType(),
transformed_frame->GetTimestamp() -
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class TransformableVideoSenderFrame : public TransformableVideoFrameInterface {
return expected_retransmission_time_ms_;
}

Direction GetDirection() const override { return Direction::kSender; }

private:
rtc::scoped_refptr<EncodedImageBufferInterface> encoded_data_;
const RTPVideoHeader header_;
Expand Down Expand Up @@ -147,6 +149,8 @@ void RTPSenderVideoFrameTransformerDelegate::OnTransformedFrame(
void RTPSenderVideoFrameTransformerDelegate::SendVideo(
std::unique_ptr<TransformableFrameInterface> transformed_frame) const {
RTC_CHECK(encoder_queue_->IsCurrent());
RTC_CHECK_EQ(transformed_frame->GetDirection(),
TransformableFrameInterface::Direction::kSender);
MutexLock lock(&sender_lock_);
if (!sender_)
return;
Expand Down
4 changes: 4 additions & 0 deletions video/rtp_video_stream_receiver_frame_transformer_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ class TransformableVideoReceiverFrame
return std::move(frame_);
}

Direction GetDirection() const override { return Direction::kReceiver; }

private:
std::unique_ptr<RtpFrameObject> frame_;
const VideoFrameMetadata metadata_;
Expand Down Expand Up @@ -111,6 +113,8 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::OnTransformedFrame(
void RtpVideoStreamReceiverFrameTransformerDelegate::ManageFrame(
std::unique_ptr<TransformableFrameInterface> frame) {
RTC_DCHECK_RUN_ON(&network_sequence_checker_);
RTC_CHECK_EQ(frame->GetDirection(),
TransformableFrameInterface::Direction::kReceiver);
if (!receiver_)
return;
auto transformed_frame = absl::WrapUnique(
Expand Down

0 comments on commit 8d8c0b4

Please sign in to comment.