roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.09k stars 213 forks source link

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

Closed runei closed 4 months ago

runei commented 4 months ago

Fixes #738

github-actions[bot] commented 4 months ago

:robot: Upon creation, pull request description does not have a link to an issue. If there is a related issue, please add it to the description using any of the supported formats.

gavv commented 4 months ago

BTW you can use NoopArena or MockArena to emulate allocation error.

gavv commented 4 months ago

Another note: please don't use CHECK_EQUAL, it usually introduces bugs into your tests because it computes its arguments more than once. This may cause problems when its arguments have side effects.

#define CHECK_EQUAL_LOCATION(expected, actual, text, file, line)\
  do { if ((expected) != (actual)) { \
      if ((actual) != (actual)) \
          UtestShell::getCurrent()->print("WARNING:\n\tThe \"Actual Parameter\" parameter is evaluated multiple times resulting in different values.\n\tThus the value in the error message is probably incorrect.", file, line); \
      if ((expected) != (expected)) \
          UtestShell::getCurrent()->print("WARNING:\n\tThe \"Expected Parameter\" parameter is evaluated multiple times resulting in different values.\n\tThus the value in the error message is probably incorrect.", file, line); \
      UtestShell::getCurrent()->assertEquals(true, StringFrom(expected).asCharString(), StringFrom(actual).asCharString(), text, file, line); \
  } \
  else \
  { \
    UtestShell::getCurrent()->assertLongsEqual((long)0, (long)0, NULLPTR, file, line); \
  } } while(0)
github-actions[bot] commented 4 months ago

:robot: The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

gavv commented 4 months ago

Thank for update, LGTM!

Could you rebase (not merge) your branch on fresh develop? Currently it's not rebasable due to conflicts.

PS. Please don't resolve discussions by yourself, usually reviewer does it when they re-check code. Instead you can use a thumbs up or leave a comment that the issue is addressed (if you need to track discussions for yourself).

runei commented 4 months ago

Thank for update, LGTM!

Could you rebase (not merge) your branch on fresh develop? Currently it's not rebasable due to conflicts.

PS. Please don't resolve discussions by yourself, usually reviewer does it when they re-check code. Instead you can use a thumbs up or leave a comment that the issue is addressed (if you need to track discussions for yourself).

Hi @gavv , the branch is rebased.

gavv commented 4 months ago

Thank you!