On Mon, Mar 17, 2025 at 9:07 AM Akihiko Odaki <akihiko.od...@daynix.com> wrote:
>
> Commit 90320051ea99 ("spiceaudio: add a pcm_ops buffer_get_free
> function") caused to emit messages saying "Resetting rate control"
> frequently when the guest generates no frames.
>
> audio_rate_peek_bytes() resets the rate control when frames < 0 ||
> frames > 65536 where frames is the rate-limited number of frames.
> Resetting when frames < 0 is sensible as the number simply doesn't make
> sense.
>
> There is a problem when frames > 65536. It implies the guest stopped
> generating frames for a while so it makes sense to reset the rate
> control when the guest resumed generating frames. However, the
> commit mentioned earlier broke this assumption by letting spiceaudio
> call audio_rate_peek_bytes() whether the guest is generating frames or
> not.
>
> Reset the rate control in audio_rate_add_bytes(), which is called only
> when actually adding frames, according to the previous call to
> audio_rate_peek_bytes() to avoid frequent rate control resets even when
> the guest generates no frame.
>
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> audio/audio_int.h | 1 +
> audio/audio.c | 14 ++++++++------
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 2d079d00a259..f78ca05f92fb 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -255,6 +255,7 @@ const char *audio_application_name(void);
> typedef struct RateCtl {
> int64_t start_ticks;
> int64_t bytes_sent;
> + int64_t peeked_frames;
> } RateCtl;
>
> void audio_rate_start(RateCtl *rate);
> diff --git a/audio/audio.c b/audio/audio.c
> index 41ee11aaad6d..049d9d07aa58 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -2274,17 +2274,19 @@ size_t audio_rate_peek_bytes(RateCtl *rate, struct
> audio_pcm_info *info)
> ticks = now - rate->start_ticks;
> bytes = muldiv64(ticks, info->bytes_per_second, NANOSECONDS_PER_SECOND);
> frames = (bytes - rate->bytes_sent) / info->bytes_per_frame;
> - if (frames < 0 || frames > 65536) {
> - AUD_log(NULL, "Resetting rate control (%" PRId64 " frames)\n",
> frames);
> - audio_rate_start(rate);
> - frames = 0;
> - }
> + rate->peeked_frames = frames;
>
> - return frames * info->bytes_per_frame;
> + return frames < 0 ? 0 : frames * info->bytes_per_frame;
> }
>
> void audio_rate_add_bytes(RateCtl *rate, size_t bytes_used)
> {
> + if (rate->peeked_frames < 0 || rate->peeked_frames > 65536) {
> + AUD_log(NULL, "Resetting rate control (%" PRId64 " frames)\n",
> + rate->peeked_frames);
> + audio_rate_start(rate);
> + }
> +
> rate->bytes_sent += bytes_used;
> }
>
>
> ---
> base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
> change-id: 20250317-rate-308c5c79f50f
>
> Best regards,
> --
> Akihiko Odaki <akihiko.od...@daynix.com>
>
>
--
Marc-André Lureau
