Message ID | 20230818032217.31134-1-M_O_Bz@163.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | HDA codec: Fix wanted_r/w position overflow | expand |
Hi, > From: zeroway <linwei6@ruijie.com.cn> > > when the duration now - buft_start reach to some kind of value, > which will get the multiply hda_bytes_per_second(st) * (now - buft_start) overflow, > instead of calculate the wanted_r/wpos from start time to current time, > here calculate the each timer tick delta data first in wanted_r/wpos_delta, > and sum it all to wanted_r/wpos to avoid the overflow you could avoid the multiplication overflow with the following code #include "qemu/host-utils.h" int64_t wanted_rpos = muldiv64(now - buft_start, hda_bytes_per_second(st), NANOSECONDS_PER_SECOND); and int64_t wanted_wpos = muldiv64(now - buft_start, hda_bytes_per_second(st), NANOSECONDS_PER_SECOND); This would be a less intrusive change. The wanted_pos with your code will grow slower than with the original code because you sum up truncated results from the division by NANOSECONDS_PER_SECONDS rounded down to next multiple of 4. With best regards, Volker > > Signed-off-by: zeroway <linwei6@ruijie.com.cn> > --- > hw/audio/hda-codec.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c > index c51d8ba617..747188221a 100644 > --- a/hw/audio/hda-codec.c > +++ b/hw/audio/hda-codec.c > @@ -169,6 +169,8 @@ struct HDAAudioStream { > uint8_t buf[8192]; /* size must be power of two */ > int64_t rpos; > int64_t wpos; > + int64_t wanted_rpos; > + int64_t wanted_wpos; > QEMUTimer *buft; > int64_t buft_start; > }; > @@ -226,16 +228,18 @@ static void hda_audio_input_timer(void *opaque) > int64_t wpos = st->wpos; > int64_t rpos = st->rpos; > > - int64_t wanted_rpos = hda_bytes_per_second(st) * (now - buft_start) > + int64_t wanted_rpos_delta = hda_bytes_per_second(st) * (now - buft_start) > / NANOSECONDS_PER_SECOND; > - wanted_rpos &= -4; /* IMPORTANT! clip to frames */ > + st->wanted_rpos += wanted_rpos_delta; > + st->wanted_rpos &= -4; /* IMPORTANT! clip to frames */ > > - if (wanted_rpos <= rpos) { > + st->buft_start = now; > + if (st->wanted_rpos <= rpos) { > /* we already transmitted the data */ > goto out_timer; > } > > - int64_t to_transfer = MIN(wpos - rpos, wanted_rpos - rpos); > + int64_t to_transfer = MIN(wpos - rpos, st->wanted_rpos - rpos); > while (to_transfer) { > uint32_t start = (rpos & B_MASK); > uint32_t chunk = MIN(B_SIZE - start, to_transfer); > @@ -290,16 +294,18 @@ static void hda_audio_output_timer(void *opaque) > int64_t wpos = st->wpos; > int64_t rpos = st->rpos; > > - int64_t wanted_wpos = hda_bytes_per_second(st) * (now - buft_start) > + int64_t wanted_wpos_delta = hda_bytes_per_second(st) * (now - buft_start) > / NANOSECONDS_PER_SECOND; > - wanted_wpos &= -4; /* IMPORTANT! clip to frames */ > + st->wanted_wpos += wanted_wpos_delta; > + st->wanted_wpos &= -4; /* IMPORTANT! clip to frames */ > > - if (wanted_wpos <= wpos) { > + st->buft_start = now; > + if (st->wanted_wpos <= wpos) { > /* we already received the data */ > goto out_timer; > } > > - int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), wanted_wpos - wpos); > + int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), st->wanted_wpos - wpos); > while (to_transfer) { > uint32_t start = (wpos & B_MASK); > uint32_t chunk = MIN(B_SIZE - start, to_transfer); > @@ -420,6 +426,8 @@ static void hda_audio_set_running(HDAAudioStream *st, bool running) > int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > st->rpos = 0; > st->wpos = 0; > + st->wanted_rpos = 0; > + st->wanted_wpos = 0; > st->buft_start = now; > timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS); > } else {
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c index c51d8ba617..747188221a 100644 --- a/hw/audio/hda-codec.c +++ b/hw/audio/hda-codec.c @@ -169,6 +169,8 @@ struct HDAAudioStream { uint8_t buf[8192]; /* size must be power of two */ int64_t rpos; int64_t wpos; + int64_t wanted_rpos; + int64_t wanted_wpos; QEMUTimer *buft; int64_t buft_start; }; @@ -226,16 +228,18 @@ static void hda_audio_input_timer(void *opaque) int64_t wpos = st->wpos; int64_t rpos = st->rpos; - int64_t wanted_rpos = hda_bytes_per_second(st) * (now - buft_start) + int64_t wanted_rpos_delta = hda_bytes_per_second(st) * (now - buft_start) / NANOSECONDS_PER_SECOND; - wanted_rpos &= -4; /* IMPORTANT! clip to frames */ + st->wanted_rpos += wanted_rpos_delta; + st->wanted_rpos &= -4; /* IMPORTANT! clip to frames */ - if (wanted_rpos <= rpos) { + st->buft_start = now; + if (st->wanted_rpos <= rpos) { /* we already transmitted the data */ goto out_timer; } - int64_t to_transfer = MIN(wpos - rpos, wanted_rpos - rpos); + int64_t to_transfer = MIN(wpos - rpos, st->wanted_rpos - rpos); while (to_transfer) { uint32_t start = (rpos & B_MASK); uint32_t chunk = MIN(B_SIZE - start, to_transfer); @@ -290,16 +294,18 @@ static void hda_audio_output_timer(void *opaque) int64_t wpos = st->wpos; int64_t rpos = st->rpos; - int64_t wanted_wpos = hda_bytes_per_second(st) * (now - buft_start) + int64_t wanted_wpos_delta = hda_bytes_per_second(st) * (now - buft_start) / NANOSECONDS_PER_SECOND; - wanted_wpos &= -4; /* IMPORTANT! clip to frames */ + st->wanted_wpos += wanted_wpos_delta; + st->wanted_wpos &= -4; /* IMPORTANT! clip to frames */ - if (wanted_wpos <= wpos) { + st->buft_start = now; + if (st->wanted_wpos <= wpos) { /* we already received the data */ goto out_timer; } - int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), wanted_wpos - wpos); + int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), st->wanted_wpos - wpos); while (to_transfer) { uint32_t start = (wpos & B_MASK); uint32_t chunk = MIN(B_SIZE - start, to_transfer); @@ -420,6 +426,8 @@ static void hda_audio_set_running(HDAAudioStream *st, bool running) int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); st->rpos = 0; st->wpos = 0; + st->wanted_rpos = 0; + st->wanted_wpos = 0; st->buft_start = now; timer_mod_anticipate_ns(st->buft, now + HDA_TIMER_TICKS); } else {