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

Return status code instead of bool in IBlockEncoder & IBlockDecoder #738

Closed
gavv opened this issue Jun 23, 2024 · 4 comments
Closed

Return status code instead of bool in IBlockEncoder & IBlockDecoder #738

gavv opened this issue Jun 23, 2024 · 4 comments
Assignees
Labels
easy hacks The solution is expected to be straightforward even if you are new to the project help wanted An important and awaited task but we have no human resources for it yet refactoring
Milestone

Comments

@gavv
Copy link
Member

gavv commented Jun 23, 2024

Summary

fec::IBlockEncoder and fec::IBlockDecoder are interfaces for codec-specific encoding and decoding of FEC packets used for packet loss recovery. See documentation.

Currently they both have begin_block() method that returns bool, which is true on success or false on error. We need to replace bool with status::StatusCode and return code that described why the operation failed.

Implementation

  • Update IBlockEncoder and IBlockDecoder interfaces
  • Update implementations:
    • fec::OpenfecEncoder
    • fec::OpenfecDecoder
  • Report appropriate statuses:
    • when allocation failed, return StatusNoMem
    • on success, return StatusOK
  • Update fec::BlockWriter and fec::BlockReader, which use IBlockEncoder and IBlockDecoder interfaces. When encoder or decoder return errors, writer and reader should forward it to upper level.

Testing

  • Add unit test to test_block_encoder_decoder.cpp to check that encoder/decoder returns StatusNoMem when allocation fails.
  • Add unit test to test_block_writer_reader_errors.cpp to check that writer/reader forward error from encoder/decoder to upper level.
@gavv gavv added refactoring help wanted An important and awaited task but we have no human resources for it yet easy hacks The solution is expected to be straightforward even if you are new to the project labels Jun 23, 2024
@runei
Copy link

runei commented Jul 10, 2024

Hi, I am interested in solving this issue.

@gavv
Copy link
Member Author

gavv commented Jul 10, 2024

Great, thank you

@gavv
Copy link
Member Author

gavv commented Jul 26, 2024

Landed

@gavv gavv closed this as completed Jul 26, 2024
@gavv
Copy link
Member Author

gavv commented Jul 27, 2024

I've reworked BlockReader and BlockWriter to use status codes instead of alive_ flag (which is now removed): ff877bb (see commit message for details)

And created a follow-up issue to forward OpenFEC errors too: #767

jeshwanthreddy13 pushed a commit to jeshwanthreddy13/roc-toolkit that referenced this issue Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy hacks The solution is expected to be straightforward even if you are new to the project help wanted An important and awaited task but we have no human resources for it yet refactoring
Projects
Status: Done
Development

No branches or pull requests

2 participants