Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-766: Refactored Boolean Returns to StatusCode in IComposer #789

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/internal_modules/roc_audio/packetizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,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 @@ -213,7 +214,8 @@ void Packetizer::pad_packet_(size_t written_payload_size) {
return;
}

if (!composer_.pad(*packet_, payload_size_ - written_payload_size)) {
if (composer_.pad(*packet_, payload_size_ - written_payload_size)
!= status::StatusOK) {
roc_panic("packetizer: can't pad packet: orig_size=%lu actual_size=%lu",
(unsigned long)payload_size_, (unsigned long)written_payload_size);
}
Expand Down
25 changes: 13 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,9 @@ 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) {
return status;
}
pp->add_flags(packet::Packet::FlagComposed);

Expand Down Expand Up @@ -261,16 +261,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 +305,9 @@ 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) {
return status;
}
rp->add_flags(packet::Packet::FlagComposed);
}
Expand Down
28 changes: 15 additions & 13 deletions src/internal_modules/roc_fec/composer.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
}

//! Adjust buffer to align payload.
virtual bool
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
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,21 @@ 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) {
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) {
virtual status::StatusCode compose(packet::Packet& packet) {
if (!packet.fec()) {
roc_panic("fec composer: unexpected non-fec packet");
}
Expand Down Expand Up @@ -167,7 +169,7 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
return inner_composer_->compose(packet);
}

return true;
return status::StatusOK;
}

private:
Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_packet/icomposer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
virtual status::StatusCode
prepare(Packet& packet, core::Slice<uint8_t>& buffer, size_t payload_size) = 0;

//! Pad packet.
Expand All @@ -66,15 +66,15 @@ 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;
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;
virtual status::StatusCode compose(Packet& packet) = 0;
};

} // namespace packet
Expand Down
2 changes: 1 addition & 1 deletion src/internal_modules/roc_packet/shipper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ status::StatusCode Shipper::write(const PacketPtr& packet) {
}

if (!packet->has_flags(Packet::FlagComposed)) {
if (!composer_.compose(*packet)) {
if (composer_.compose(*packet) != status::StatusOK) {
roc_log(LogError, "shipper: can't compose packet");
return status::StatusNoMem;
}
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
30 changes: 15 additions & 15 deletions src/internal_modules/roc_rtcp/composer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ 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) {
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,51 @@ 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) {
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) {
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) {
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;
return status::StatusBadOperation;
}

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

return true;
return status::StatusOK;
}

} // namespace rtcp
Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_rtcp/composer.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ class Composer : public packet::IComposer, public core::NonCopyable<> {
virtual status::StatusCode init_status() const;

//! Adjust buffer to align payload.
virtual bool
virtual status::StatusCode
align(core::Slice<uint8_t>& buffer, size_t header_size, size_t payload_alignment);

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

//! Pad packet.
virtual bool pad(packet::Packet& packet, size_t padding_size);
virtual status::StatusCode pad(packet::Packet& packet, size_t padding_size);

//! Compose packet to buffer.
virtual bool compose(packet::Packet& packet);
virtual status::StatusCode compose(packet::Packet& packet);
};

} // namespace rtcp
Expand Down
Loading
Loading