Feature/fabrics/jonas/protocols audio#31
Closed
mlefebvre1 wants to merge 10 commits into
Closed
Conversation
jonasohland
reviewed
May 1, 2026
|
|
||
| std::optional<Target::SampleReadResult> RCTarget::readSamples() | ||
| { | ||
| if (auto res = readNext<QueueReadMode::NonBlocking>(std::chrono::steady_clock::duration::zero()); res) |
Owner
There was a problem hiding this comment.
For all of these cases, it should probably be an "invalid state" error instead of just returning nothing.
Collaborator
Author
There was a problem hiding this comment.
Hmm, I'm not sure about this. This change is consistent with the previous behavior where we return null which in the end gets translated to MXL_ERR_NOT_READY . The reason why it's ugly is that with C++20, there's no way to apply a monadic operation (transform) to an std::optional. We would have to wait for C++23 for that: https://en.cppreference.com/cpp/utility/optional/transform
Owner
There was a problem hiding this comment.
Ah, I seen.
I think we should change the behavior do a check before if the call to readNext can succeed with the desired result. If a target/initiator is configured for grains, trying to read samples should return an error that indicates that this operation is not supported for this type of target/initiator. (And vice versa).
Maybe we should even add an error for this specific case.
mlefebvre1
force-pushed
the
feature/fabrics/jonas/protocols-audio
branch
3 times, most recently
from
1339c8b to
bb59590
Compare
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
…st creation. Rename DataLayout substypes to Discrete/Continuous instead of Video/Audio. Change some variable declarations to the 'modern' way. Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
…ed 'count' parameter validations to ensure the user doesn't pass a value that would exceed the size of the bounce buffer entry. Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
…g audio entry header size of the entrySizeRequired. Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
…for a given protocol. Co-authored-by: Michael Lefebvre <michael.lefebvre.31@gmail.com> Signed-off-by: Michael Lefebvre <michael.lefebvre@riedel.net>
mlefebvre1
force-pushed
the
feature/fabrics/jonas/protocols-audio
branch
from
64806b4 to
93a3b0a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
*** Do not merge, just for review ***