Skip to content

Commit

Permalink
roc-streaminggh-766: Refactored Boolean Returns to StatusCode in ICom…
Browse files Browse the repository at this point in the history
…poser
  • Loading branch information
sayedMurtadha authored and gavv committed Jan 21, 2025
1 parent 4e90f6b commit 2d97fd2
Show file tree
Hide file tree
Showing 21 changed files with 169 additions and 131 deletions.
27 changes: 18 additions & 9 deletions src/internal_modules/roc_audio/packetizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,17 @@ status::StatusCode Packetizer::end_packet_() {
// Fill protocol-specific fields.
sequencer_.next(*packet_, packet_cts_, (packet::stream_timestamp_t)packet_pos_);

status::StatusCode code = status::StatusOK;
// Apply padding if needed.
if (packet_pos_ < samples_per_packet_) {
pad_packet_(written_payload_size);
code = pad_packet_(written_payload_size);
}

const status::StatusCode code = writer_.write(packet_);
if (code != status::StatusOK) {
return code;
}

code = writer_.write(packet_);
if (code != status::StatusOK) {
return code;
}
Expand Down Expand Up @@ -196,9 +201,10 @@ status::StatusCode Packetizer::create_packet_() {
return status::StatusNoMem;
}

if (!composer_.prepare(*pp, buffer, payload_size_)) {
status::StatusCode status = composer_.prepare(*pp, buffer, payload_size_);
if (status != status::StatusOK) {
roc_log(LogError, "packetizer: can't prepare packet");
return status::StatusNoMem;
return status;
}
pp->add_flags(packet::Packet::FlagPrepared);

Expand All @@ -208,15 +214,18 @@ status::StatusCode Packetizer::create_packet_() {
return status::StatusOK;
}

void Packetizer::pad_packet_(size_t written_payload_size) {
status::StatusCode Packetizer::pad_packet_(size_t written_payload_size) {
if (written_payload_size == payload_size_) {
return;
return status::StatusOK;
}

if (!composer_.pad(*packet_, payload_size_ - written_payload_size)) {
roc_panic("packetizer: can't pad packet: orig_size=%lu actual_size=%lu",
(unsigned long)payload_size_, (unsigned long)written_payload_size);
status::StatusCode status =
composer_.pad(*packet_, payload_size_ - written_payload_size);
if (status != status::StatusOK) {
roc_log(LogError, "packetizer: can't pad packet: orig_size=%lu actual_size=%lu",
(unsigned long)payload_size_, (unsigned long)written_payload_size);
}
return status;
}

} // namespace audio
Expand Down
2 changes: 1 addition & 1 deletion src/internal_modules/roc_audio/packetizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class Packetizer : public IFrameWriter, public core::NonCopyable<> {
status::StatusCode end_packet_();

status::StatusCode create_packet_();
void pad_packet_(size_t written_payload_size);
status::StatusCode pad_packet_(size_t written_payload_size);

packet::IWriter& writer_;
packet::IComposer& composer_;
Expand Down
27 changes: 15 additions & 12 deletions src/internal_modules/roc_fec/block_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,10 @@ status::StatusCode BlockWriter::write_source_packet_(const packet::PacketPtr& pp

fill_packet_fec_fields_(pp, (packet::seqnum_t)cur_packet_);

if (!source_composer_.compose(*pp)) {
// TODO(gh-183): forward status from composer
return status::StatusBadBuffer;
status::StatusCode status = source_composer_.compose(*pp);
if (status != status::StatusOK) {
roc_log(LogError, "fec block writer: can't compose packet");
return status;
}
pp->add_flags(packet::Packet::FlagComposed);

Expand Down Expand Up @@ -261,16 +262,17 @@ status::StatusCode BlockWriter::make_repair_packet_(packet::seqnum_t pack_n,
return status::StatusNoMem;
}

if (!repair_composer_.align(buffer, 0, block_encoder_.buffer_alignment())) {
status::StatusCode status =
repair_composer_.align(buffer, 0, block_encoder_.buffer_alignment());
if (status != status::StatusOK) {
roc_log(LogError, "fec block writer: can't align packet buffer");
// TODO(gh-183): forward status from composer
return status::StatusBadBuffer;
return status;
}

if (!repair_composer_.prepare(*packet, buffer, cur_payload_size_)) {
status = repair_composer_.prepare(*packet, buffer, cur_payload_size_);
if (status != status::StatusOK) {
roc_log(LogError, "fec block writer: can't prepare packet");
// TODO(gh-183): forward status from composer
return status::StatusBadBuffer;
return status;
}
packet->add_flags(packet::Packet::FlagPrepared);

Expand Down Expand Up @@ -304,9 +306,10 @@ status::StatusCode BlockWriter::compose_repair_packets_() {
continue;
}

if (!repair_composer_.compose(*rp)) {
// TODO(gh-183): forward status from composer
return status::StatusBadBuffer;
status::StatusCode status = repair_composer_.compose(*rp);
if (status != status::StatusOK) {
roc_log(LogError, "fec block writer: can't compose packet");
return status;
}
rp->add_flags(packet::Packet::FlagComposed);
}
Expand Down
31 changes: 17 additions & 14 deletions src/internal_modules/roc_fec/composer.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
}

//! Check if the object was successfully constructed.
virtual status::StatusCode init_status() const {
ROC_ATTR_NODISCARD virtual status::StatusCode init_status() const {
return status::StatusOK;
}

//! Adjust buffer to align payload.
virtual bool
ROC_ATTR_NODISCARD virtual status::StatusCode
align(core::Slice<uint8_t>& buffer, size_t header_size, size_t payload_alignment) {
if ((unsigned long)buffer.data() % payload_alignment != 0) {
roc_panic("fec composer: unexpected non-aligned buffer");
Expand All @@ -59,18 +59,18 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
LogDebug,
"fec composer: not enough space for alignment: padding=%lu cap=%lu",
(unsigned long)padding, (unsigned long)buffer.capacity());
return false;
return status::StatusBadBuffer;
}

buffer.reslice(padding, padding);
return true;
return status::StatusOK;
} else {
return inner_composer_->align(buffer, header_size, payload_alignment);
}
}

//! Prepare buffer for composing a packet.
virtual bool
ROC_ATTR_NODISCARD virtual status::StatusCode
prepare(packet::Packet& packet, core::Slice<uint8_t>& buffer, size_t payload_size) {
core::Slice<uint8_t> payload_id = buffer.subslice(0, 0);

Expand All @@ -80,7 +80,7 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
"fec composer: not enough space for fec header: size=%lu cap=%lu",
(unsigned long)sizeof(PayloadID),
(unsigned long)payload_id.capacity());
return false;
return status::StatusBadBuffer;
}
payload_id.reslice(0, sizeof(PayloadID));
}
Expand All @@ -89,8 +89,10 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
payload_id.subslice(payload_id.size(), payload_id.size());

if (inner_composer_) {
if (!inner_composer_->prepare(packet, payload, payload_size)) {
return false;
status::StatusCode result =
inner_composer_->prepare(packet, payload, payload_size);
if (result != status::StatusOK) {
return result;
}
} else {
payload.reslice(0, payload_size);
Expand All @@ -104,7 +106,7 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
"fec composer: not enough space for fec header: size=%lu cap=%lu",
(unsigned long)sizeof(PayloadID),
(unsigned long)payload_id.capacity());
return false;
return status::StatusBadBuffer;
}
payload_id.reslice(0, sizeof(PayloadID));
}
Expand All @@ -123,21 +125,22 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {

buffer.reslice(0, payload_id.size() + payload.size());

return true;
return status::StatusOK;
}

//! Pad packet.
virtual bool pad(packet::Packet& packet, size_t padding_size) {
ROC_ATTR_NODISCARD virtual status::StatusCode pad(packet::Packet& packet,
size_t padding_size) {
if (inner_composer_) {
return inner_composer_->pad(packet, padding_size);
}

// padding not supported
return false;
return status::StatusBadOperation;
}

//! Compose packet to buffer.
virtual bool compose(packet::Packet& packet) {
ROC_ATTR_NODISCARD virtual status::StatusCode compose(packet::Packet& packet) {
if (!packet.fec()) {
roc_panic("fec composer: unexpected non-fec packet");
}
Expand Down Expand Up @@ -167,7 +170,7 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
return inner_composer_->compose(packet);
}

return true;
return status::StatusOK;
}

private:
Expand Down
11 changes: 6 additions & 5 deletions src/internal_modules/roc_packet/icomposer.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class IComposer : public core::ArenaAllocation {
//! @returns
//! status::StatusOK if composer was initialized correctly,
//! or error code otherwise.
virtual status::StatusCode init_status() const = 0;
ROC_ATTR_NODISCARD virtual status::StatusCode init_status() const = 0;

//! Adjust buffer to align payload.
//! @remarks
Expand All @@ -42,7 +42,7 @@ class IComposer : public core::ArenaAllocation {
//! @returns
//! true if the buffer was successfully adjusted or false if the @p buffer
//! capacity is not enough.
virtual bool
ROC_ATTR_NODISCARD virtual status::StatusCode
align(core::Slice<uint8_t>& buffer, size_t header_size, size_t payload_alignment) = 0;

//! Prepare buffer for composing a packet.
Expand All @@ -55,7 +55,7 @@ class IComposer : public core::ArenaAllocation {
//! @returns
//! true if the packet was successfully prepared or false if the @p buffer
//! capacity is not enough.
virtual bool
ROC_ATTR_NODISCARD virtual status::StatusCode
prepare(Packet& packet, core::Slice<uint8_t>& buffer, size_t payload_size) = 0;

//! Pad packet.
Expand All @@ -66,15 +66,16 @@ class IComposer : public core::ArenaAllocation {
//! @returns
//! true if the packet was successfully padded or false if parameters
//! are invalid or padding is not supported.
virtual bool pad(Packet& packet, size_t padding_size) = 0;
ROC_ATTR_NODISCARD virtual status::StatusCode pad(Packet& packet,
size_t padding_size) = 0;

//! Compose packet to buffer.
//! @remarks
//! Formats @p packet headers and payloads to the buffer attached to it during
//! a previous prepare() call.
//! @returns
//! true if the packet was successfully composed or false if an error occurred.
virtual bool compose(Packet& packet) = 0;
ROC_ATTR_NODISCARD virtual status::StatusCode compose(Packet& packet) = 0;
};

} // namespace packet
Expand Down
5 changes: 3 additions & 2 deletions src/internal_modules/roc_packet/shipper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ status::StatusCode Shipper::write(const PacketPtr& packet) {
}

if (!packet->has_flags(Packet::FlagComposed)) {
if (!composer_.compose(*packet)) {
status::StatusCode status = composer_.compose(*packet);
if (status != status::StatusOK) {
roc_log(LogError, "shipper: can't compose packet");
return status::StatusNoMem;
return status;
}
packet->add_flags(Packet::FlagComposed);
}
Expand Down
5 changes: 3 additions & 2 deletions src/internal_modules/roc_rtcp/communicator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,10 @@ status::StatusCode Communicator::generate_packet_(PacketType packet_type,
packet_buffer.reslice(0, 0);

// Prepare packet to be able to hold our RTCP packet data
if (!packet_composer_.prepare(*packet, packet_buffer, payload_buffer.size())) {
status = packet_composer_.prepare(*packet, packet_buffer, payload_buffer.size());
if (status != status::StatusOK) {
roc_log(LogError, "rtcp communicator: can't prepare packet");
return status::StatusNoMem;
return status;
}
packet->add_flags(packet::Packet::FlagPrepared);

Expand Down
35 changes: 17 additions & 18 deletions src/internal_modules/roc_rtcp/composer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ Composer::Composer(core::IArena& arena)
: IComposer(arena) {
}

status::StatusCode Composer::init_status() const {
ROC_ATTR_NODISCARD status::StatusCode Composer::init_status() const {
return status::StatusOK;
}

bool Composer::align(core::Slice<uint8_t>& buffer,
size_t header_size,
size_t payload_alignment) {
ROC_ATTR_NODISCARD status::StatusCode Composer::align(core::Slice<uint8_t>& buffer,
size_t header_size,
size_t payload_alignment) {
if ((unsigned long)buffer.data() % payload_alignment != 0) {
roc_panic("rtcp composer: unexpected non-aligned buffer");
}
Expand All @@ -35,51 +35,50 @@ bool Composer::align(core::Slice<uint8_t>& buffer,
roc_log(LogDebug,
"rtcp composer: not enough space for alignment: padding=%lu cap=%lu",
(unsigned long)padding, (unsigned long)buffer.capacity());
return false;
return status::StatusBadBuffer;
}

buffer.reslice(padding, padding);
return true;
return status::StatusOK;
}

bool Composer::prepare(packet::Packet& packet,
core::Slice<uint8_t>& buffer,
size_t payload_size) {
ROC_ATTR_NODISCARD status::StatusCode Composer::prepare(packet::Packet& packet,
core::Slice<uint8_t>& buffer,
size_t payload_size) {
buffer.reslice(0, payload_size);

packet.add_flags(packet::Packet::FlagControl);
packet.add_flags(packet::Packet::FlagRTCP);

packet.rtcp()->payload = buffer;

return true;
return status::StatusOK;
}

bool Composer::pad(packet::Packet& packet, size_t padding_size) {
ROC_ATTR_NODISCARD status::StatusCode Composer::pad(packet::Packet& packet,
size_t padding_size) {
// not supported

(void)packet;
(void)padding_size;

return false;
return status::StatusBadOperation;
}

bool Composer::compose(packet::Packet& packet) {
ROC_ATTR_NODISCARD status::StatusCode Composer::compose(packet::Packet& packet) {
if (!packet.rtcp()) {
roc_panic("rtcp composer: unexpected non-rctp packet");
}

if (!packet.rtcp()->payload) {
roc_log(LogError, "rtcp composer: unexpected null data");
return false;
roc_panic("rtcp composer: unexpected null data");
}

if (packet.rtcp()->payload.size() == 0) {
roc_log(LogError, "rtcp composer: unexpected zero data");
return false;
roc_panic("rtcp composer: unexpected zero data");
}

return true;
return status::StatusOK;
}

} // namespace rtcp
Expand Down
Loading

0 comments on commit 2d97fd2

Please sign in to comment.