Skip to content

Commit

Permalink
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.

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-Commit-Position: refs/heads/main@{#35100}
  • Loading branch information
Tony Herre authored and WebRTC LUCI CQ committed Sep 27, 2021
1 parent 6ee9734 commit 8fb41a3
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 8fb41a3

Please sign in to comment.