roc-toolkit icon indicating copy to clipboard operation
roc-toolkit copied to clipboard

Support error codes in packet readers and writers

Open gavv opened this issue 6 years ago • 2 comments

Problem

Here is how packet::IReader and packet::IWriter interfaces look like currently:

class IReader {
public:
    virtual PacketPtr read() = 0;
};

class IWriter {
public:
    virtual void write(const PacketPtr&) = 0;
};

We need two improvements here:

  • Allow read() to report error code (the reason why it failed).
  • Allow write() to fail and also report error code (the reason why it failed).

Solution

Convert the above interfaces to the following:

class IReader {
public:
    virtual int read(PacketPtr&) = 0;
};

class IWriter {
public:
    virtual int write(const PacketPtr&) = 0;
};

In particular:

  • On success, read() and write() will return zero. read() will set the passed PacketPtr to the packet read.

  • On failure, read() and write() will return a negative error code defined in roc_error module (roc_error/error_code.h).

After changing the interfaces, we should also find and fix all packet::IReader and packet::IWriter implementations:

  • Fix function signature.

  • Fix invocation of nested audio readers and writers. If nested reader / writer returns a negative value, we should break and return that value to the upper level.

  • Fix audio::Depacketizer and audio::Packetizer. When #302 will be done, they should forward the error code returned by packet reader / writer to the upper level. Until #302 is done, they can just panic on such error.

  • Fix unit tests for the modified components and add a few tests for the new behavior (test that error codes are forwarded correctly).

Notes

Since currently read() and write() can not fail, after this change all implementations will actually always report success, and error handling code will never trigger, except unit tests.

However, after finishing this task we will be able to add error reporting o some components. This will be done separately.

gavv avatar Dec 19 '19 11:12 gavv

Is this issue dependent on #302 or can be done independently?

fusuiyi123 avatar Apr 11 '20 02:04 fusuiyi123

Technically we can do them in parallel, but I would postpone this one until #302 is merged. Maybe it will raise some new issues to us.

gavv avatar Apr 11 '20 10:04 gavv

It's been decided to change the initial design a little bit:

  • Introduce status codes instead of error codes, thus roc_error will be renamed into roc_status
  • packet::IReader, packet::IWriter will return status::StatusCode

FYI @gavv

dshil avatar Oct 06 '23 12:10 dshil

Landed!

gavv avatar Nov 07 '23 19:11 gavv