Add input stream latency test#590
Conversation
It calls the stream ops and ask for input latency if the output device has type INPUT.
|
@achronop if you can have a look. |
achronop
left a comment
achronop
left a comment
There was a problem hiding this comment.
Thanks a lot Koalab99, I'll check them all together tomorrow morning.
achronop
left a comment
achronop
left a comment
There was a problem hiding this comment.
Looks good thank you very much. I see something that does not work as expected and that's why I request changes. Other than that I have added a couple of reformatting changes. TBH half of them are not your fault. The problems pre-existed and it would be great if you would like to improve them since you are updating that part. They are small things.
| uint64_t pos = cl.get_stream_position(); | ||
| uint64_t latency = cl.get_stream_latency(); | ||
| fprintf(stderr, "stream position %" PRIu64 " (latency %" PRIu64 ")\n", pos, latency); | ||
| if(op->collection_device_type & CUBEB_DEVICE_TYPE_INPUT) { |
There was a problem hiding this comment.
This is testing the collection device type which is used for cubeb_register_device_collection_changed(). Here we need to check if the mic is open. This happens when the operation_data::play::mode is RECORD or DUBLEX so we need something like: op->pm == RECORD || op->pm == DUPLEX
Can you please go one step further and update the output case as well in a similar way. Right now the code assumes that we always have the output open which is not the case. This is not your fault but it would be great if you can update it since we are dealing with this part.
| fprintf(stderr, "stream position %" PRIu64 " (latency %" PRIu64 ")\n", pos, latency); | ||
| if(op->collection_device_type & CUBEB_DEVICE_TYPE_INPUT) { | ||
| latency = cl.get_input_stream_latency(); | ||
| fprintf(stderr, "input stream latency %" PRIu64 ")\n", latency); |
There was a problem hiding this comment.
I see a parenthesis close ) but not a parenthesis open in the message.
That said can you please make it in an away that it will print in one line. Routhly something like:
get possition;
fprintf("position message"); // note no endl '\n'
if (DUBLEX || PLAYBACK)
get output latency
fprintf("output latency"); // again no endl
if (DUBLEX || RECORD)
get input latency
fprintf("input latency"); // again no endl
fprinf("\n");
| void set_latency_frames(uint32_t latency_frames); | ||
| uint64_t get_stream_position() const; | ||
| uint32_t get_stream_latency() const; | ||
| uint32_t get_input_stream_latency() const; |
There was a problem hiding this comment.
Can you change the get_input_stream_latency() to get_stream_input_latency.
Can you please also change the get_stream_latency() to get_stream_output_latency for clarity. This is a wrapper not cubeb itself so we can change the ABI without tears.
| @@ -439,6 +450,10 @@ bool choose_action(cubeb_client& cl, operation_data * op, int c) { | |||
| uint64_t pos = cl.get_stream_position(); | |||
| uint64_t latency = cl.get_stream_latency(); | |||
There was a problem hiding this comment.
get_stream_*_latency() return uint32_t but we store it in a uint64_t. It's not your fault that code was there. Do you mind if you change it to use uint32_t and replace the format specifiers in the print messages?
|
I think it fixed the issues, let me know if something is wrong with it |
achronop
left a comment
achronop
left a comment
There was a problem hiding this comment.
Yeah it's perfect, thank you.
3 participants
It calls the stream ops and ask for input latency if the device
has type INPUT.