Compensate for resampler latency where it is utilized #531

mdsitton · 2019-08-17T18:50:17Z

Work on #527

kinetiknz

It looks like the OpenSL backend needs updating too, can you check please?

The Rust version of the CoreAudio backend will need to be adjusted too: https://github.com/ChunMinChang/cubeb-coreaudio-rs, cc @ChunMinChang

kinetiknz · 2019-08-19T00:06:43Z

src/cubeb_wasapi.cpp

@@ -2399,28 +2399,30 @@ int wasapi_stream_get_position(cubeb_stream * stm, uint64_t * position)
  return CUBEB_OK;
 }

-int wasapi_stream_get_latency(cubeb_stream * stm, uint32_t * latency)
+int wasapi_stream_get_latency(cubeb_stream *stm, uint32_t *latency)


Please retain the original formatting for all of this code.

kinetiknz · 2019-08-19T00:07:17Z

src/cubeb_wasapi.cpp

-  REFERENCE_TIME latency_hns;
-  HRESULT hr = stm->output_client->GetStreamLatency(&latency_hns);
-  if (FAILED(hr)) {
+  if (!has_output(stm))


Why is this check needed in addition to stm->output_client?

kinetiknz · 2019-08-19T23:44:01Z

We probably also want to adjust cubeb_stream_get_position to account for resampler latency. Otherwise, cubeb_stream_get_latency will report a different latency value to the value we're using to adjust the position reported via cubeb_stream_get_position.

mdsitton · 2019-08-29T01:03:55Z

@kinetiknz One thing for the opensl backend. There is no get_latency implementation at all currently. That's why i didn't do any changes there.

padenot · 2019-08-29T06:49:59Z

I can add it, we have the right number.

mdsitton · 2019-08-29T11:16:33Z

Alright, so other than the changes for OpenSL, and any rust backends that should be everything.

padenot · 2019-08-29T11:37:39Z

I can add it, we have the right number.

#535

achronop · 2019-09-03T11:58:47Z

Thank you for doing that. Have you tested it? Let me know if I can help with that? Also, did you check the numbers returned by cubeb_resampler_latency method? are they sensible? Just curiosity since this is the first time that this method will be used.

achronop · 2019-09-03T12:01:10Z

Also, the title of the patch is not very descriptive and indicates a windows 10 fix which is not the case.

kinetiknz · 2019-09-09T00:23:40Z

src/cubeb_audiounit.cpp

@@ -2972,7 +2972,7 @@ audiounit_stream_get_position(cubeb_stream * stm, uint64_t * position)
  if (stm->current_latency_frames > stm->frames_played) {
    *position = 0;
  } else {
-    *position = stm->frames_played - stm->current_latency_frames;
+    *position = stm->frames_played - (stm->current_latency_frames + cubeb_resampler_latency(stm->resampler.get()));


It's possible frames_played is less than the latency subtracted here, so this could wrap around. We'll need to clamp to 0 here (and in the other backends the same way).

kinetiknz · 2019-09-09T00:25:11Z

src/cubeb_wasapi.cpp

-  if (!stm->output_client) {
+  /* Verify that AudioClient has been initialized. */
+  if (!stm->output_client)
+  {


Move the { up to the previous line here and for the has_output check above.

kinetiknz · 2019-09-09T00:26:33Z

src/cubeb_wasapi.cpp

-  }
-  *latency = hns_to_frames(stm, latency_hns);
+  double delay = current_stream_delay(stm);
+  *latency = std::ceil(delay * stm->output_mix_params.rate) + cubeb_resampler_latency(stm->resampler.get());


Maybe we should just include the resampler delay inside the current_stream_delay implementation?

mdsitton · 2019-09-16T16:57:02Z

I'll work on these changes when i get time. Which will probably be in a few weeks.

mdsitton · 2019-10-24T05:58:21Z

Just an update, haven't forgotten but I've started a new job and have been in the process of moving things are starting to settle down so hopefully soon 🤞

ChunMinChang · 2020-03-25T16:07:33Z

@mdsitton, any update?

mdsitton · 2020-06-02T21:07:54Z

I'll try to get back on this soon, maybe this weekend.

mdsitton · 2020-06-07T05:13:44Z

Looks like the bug with wasapi i was also fixing in this pr has already been fixed on master so i've pulled that one out of this.

I've updated everything else mostly.

One thing i need to do still is convert the audiounit code to convert the re-sampler frame latency to the stream sample freqency instead of the hw output frequency. I think there may be some issues here on the audiounit code though i need to dig through the code some more to verify.

mdsitton · 2020-06-07T05:29:57Z

Tomorrow i'll pull out my macbook and work on the audiounit backend a bit closer. I haven't really tested the backends too much yet. Mostly just updating my previous code and reworking some parts.

Fix issue with jack, it doesn't use smart pointers Fix jack compiler error

…ream rate actually is. Main difference with audiounit is that the backend doesn't use the resampler all of the time. It is only used when the input and output sample rates differ.

mdsitton · 2020-06-11T03:04:48Z

@kinetiknz This should be ready to review again whenever you get the chance.

#559 - AAudio PR will need to be worked on for this whenever this is merged as well as the rust version of audiounit.

mdsitton · 2020-08-13T09:08:52Z

@kinetiknz any update on when this will be reviewed?

mdsitton · 2020-09-15T08:35:13Z

@padenot Perhaps you could take a look at this then? been waiting on someone to review for 3 months now! I'd certainly like this to get merged if possible. Or to get further feedback on the change.

Thanks!

padenot · 2020-09-15T08:45:33Z

src/cubeb_jack.cpp

@@ -955,7 +955,7 @@ cbjack_stream_stop(cubeb_stream * stream)
 static int
 cbjack_stream_get_position(cubeb_stream * stream, uint64_t * position)
 {
-  *position = stream->position;
+  *position = stream->position - ceil(cubeb_resampler_latency(stream->resampler) * stream->ratio);


This will underflow on stream start, you want to branch here and return 0 if this would be negative: if the stream has just started, it's still filling the buffers in the underlying layer, and nothing is being output. Because stream_get_position is the time of what is being heard, then 0 is the correct answer in this situation.

padenot · 2020-09-15T08:51:37Z

src/cubeb_wasapi.cpp

  /* Calculate how far behind the current stream head the playback cursor is. */
-  uint64_t stream_delay = static_cast<uint64_t>(current_stream_delay(stm) * stm->output_stream_params.rate);
+  uint64_t resampler_delay = ceil(cubeb_resampler_latency(stm->resampler.get()) * resample_ratio);


This is tricky. cubeb_resampler_latency returns the output latency, i.e., the number of frames of latency, expressed in output rate, so we don't need this, because stream_get_position returns its value in frames at the output (= stream) rate.

padenot · 2020-09-15T08:52:11Z

src/cubeb_wasapi.cpp

+
+
+  double resample_ratio = stream_to_mix_samplerate_ratio(stm->output_stream_params, stm->output_mix_params);
+  uint32_t resample_latency = ceil(cubeb_resampler_latency(stm->resampler.get()) * resample_ratio);


Same logic applies here.

padenot · 2020-09-15T08:54:09Z

src/cubeb_wasapi.cpp

@@ -2664,7 +2672,11 @@ int wasapi_stream_get_input_latency(cubeb_stream * stm, uint32_t * latency)
    return CUBEB_ERROR;
  }

-  *latency = hns_to_frames(stm, stm->input_latency_hns);
+
+  double resample_ratio = stream_to_mix_samplerate_ratio(stm->output_stream_params, stm->output_mix_params);


This is input latency, and the input mix rate is frequently different from the output rate (for example because the microphone and the output device are two separate devices, that have been configured with different rates: this is sub-optimal, but happens).

padenot · 2020-09-15T08:56:57Z

src/cubeb_wasapi.cpp

-  *latency = hns_to_frames(stm, stm->input_latency_hns);
+
+  double resample_ratio = stream_to_mix_samplerate_ratio(stm->output_stream_params, stm->output_mix_params);
+  uint32_t resample_latency = ceil(cubeb_resampler_latency(stm->resampler.get()) * resample_ratio);


It appears that you can't write this function with the current API: cubeb_resampler is a bidirectional resampler, that can simultaneously resample an input stream at rate A (microphone) and an output stream at rate B (speakers), both to a rate C (that can be equal to either A or B). There is nothing in the API that surfaces the input latency, I'll add it.

In general there is some work needed on the resampler, that became very clear when working on https://github.com/kinetiknz/cubeb/pull/598

padenot · 2020-09-15T08:57:30Z

src/cubeb_audiounit.cpp

@@ -2984,7 +2990,9 @@ audiounit_stream_get_latency(cubeb_stream * stm, uint32_t * latency)
  //TODO
  return CUBEB_ERROR_NOT_SUPPORTED;
 #else
-  *latency = stm->total_output_latency_frames;
+  uint32_t resample_latency = static_cast<uint32_t>(ceil(cubeb_resampler_latency(stm->resampler.get()) * stm->resample_ratio));


This is already in logical stream rate, no need to adjust.

mdsitton changed the title *stream_get_latency changes, adds resampler latency, and fixes wasapi latency windows 10 *_stream_get_latency changes, adds resampler latency, and fixes wasapi latency windows 10 Aug 17, 2019

kinetiknz reviewed Aug 19, 2019

View reviewed changes

kinetiknz reviewed Sep 9, 2019

View reviewed changes

mdsitton changed the title *_stream_get_latency changes, adds resampler latency, and fixes wasapi latency windows 10 Compensate for resampler latency where it is utilized Jun 7, 2020

mdsitton force-pushed the stream-latency branch from 3885ca9 to 6758579 Compare June 7, 2020 03:59

mdsitton force-pushed the stream-latency branch from a3b4fc7 to 6121ed9 Compare June 7, 2020 05:28

Add resampler latency to latency and position calculations

97bf554

Fix issue with jack, it doesn't use smart pointers Fix jack compiler error

mdsitton force-pushed the stream-latency branch from e6713cb to 97bf554 Compare June 7, 2020 17:53

Make sure we scale the returned number of samples to what the user st…

b758e0d

…ream rate actually is. Main difference with audiounit is that the backend doesn't use the resampler all of the time. It is only used when the input and output sample rates differ.

kinetiknz self-requested a review June 29, 2020 03:00

padenot self-requested a review September 15, 2020 08:41

padenot requested changes Sep 15, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compensate for resampler latency where it is utilized #531

Compensate for resampler latency where it is utilized #531

mdsitton commented Aug 17, 2019

kinetiknz left a comment

kinetiknz Aug 19, 2019

kinetiknz Aug 19, 2019

kinetiknz commented Aug 19, 2019

mdsitton commented Aug 29, 2019

padenot commented Aug 29, 2019

mdsitton commented Aug 29, 2019

padenot commented Aug 29, 2019

achronop commented Sep 3, 2019

achronop commented Sep 3, 2019

kinetiknz Sep 9, 2019

kinetiknz Sep 9, 2019

kinetiknz Sep 9, 2019

mdsitton commented Sep 16, 2019

mdsitton commented Oct 24, 2019

ChunMinChang commented Mar 25, 2020

mdsitton commented Jun 2, 2020

mdsitton commented Jun 7, 2020

mdsitton commented Jun 7, 2020

mdsitton commented Jun 11, 2020

mdsitton commented Aug 13, 2020

mdsitton commented Sep 15, 2020

padenot Sep 15, 2020

padenot Sep 15, 2020

padenot Sep 15, 2020

padenot Sep 15, 2020

padenot Sep 15, 2020

padenot Sep 15, 2020



		double resample_ratio = stream_to_mix_samplerate_ratio(stm->output_stream_params, stm->output_mix_params);
		uint32_t resample_latency = ceil(cubeb_resampler_latency(stm->resampler.get()) * resample_ratio);

Compensate for resampler latency where it is utilized #531

Are you sure you want to change the base?

Compensate for resampler latency where it is utilized #531

Conversation

mdsitton commented Aug 17, 2019

kinetiknz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinetiknz commented Aug 19, 2019

mdsitton commented Aug 29, 2019

padenot commented Aug 29, 2019

mdsitton commented Aug 29, 2019

padenot commented Aug 29, 2019

achronop commented Sep 3, 2019

achronop commented Sep 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdsitton commented Sep 16, 2019

mdsitton commented Oct 24, 2019

ChunMinChang commented Mar 25, 2020

mdsitton commented Jun 2, 2020

mdsitton commented Jun 7, 2020

mdsitton commented Jun 7, 2020

mdsitton commented Jun 11, 2020

mdsitton commented Aug 13, 2020

mdsitton commented Sep 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment