[Qemu-devel] [PATCH 1/3] audio/hda: adjust larger gaps faster

[Qemu-devel] [PATCH 1/3] audio/hda: adjust larger gaps faster

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/hda-codec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 31c66d4255..9f630fa37f 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -203,6 +203,9 @@ static inline void hda_timer_sync_adjust(HDAAudioStream *st, int64_t target_pos)
     if (target_pos < -limit) {
         corr = -HDA_TIMER_TICKS;
     }
+    if (target_pos < -(2 * limit)) {
+        corr = -(4 * HDA_TIMER_TICKS);
+    }
     if (corr == 0) {
         return;
     }
-- 
2.9.3

[Qemu-devel] [PATCH 2/3] audio/hda: fix CID 1393631

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/hda-codec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index 9f630fa37f..2b58c3505b 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -189,7 +189,7 @@ struct HDAAudioState {
 
 static inline int64_t hda_bytes_per_second(HDAAudioStream *st)
 {
-    return 2 * st->as.nchannels * st->as.freq;
+    return 2LL * st->as.nchannels * st->as.freq;
 }
 
 static inline void hda_timer_sync_adjust(HDAAudioStream *st, int64_t target_pos)
-- 
2.9.3

[Qemu-devel] [PATCH 3/3] audio: add audio timer trace points

[Gerd Hoffmann]

Track audio timer starts and stops.  Also trace delayed audio timer calls.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.c      | 28 +++++++++++++++++++++++++---
 audio/trace-events |  5 +++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d6e91901aa..d055b2236c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -29,6 +29,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "sysemu/replay.h"
+#include "audio/trace.h"
 
 #define AUDIO_CAP "audio"
 #include "audio_int.h"
@@ -1129,6 +1130,10 @@ static void audio_pcm_print_info (const char *cap, struct audio_pcm_info *info)
 /*
  * Timer
  */
+
+static bool audio_timer_running;
+static uint64_t audio_timer_last;
+
 static int audio_is_timer_needed (void)
 {
     HWVoiceIn *hwi = NULL;
@@ -1148,14 +1153,31 @@ static void audio_reset_timer (AudioState *s)
     if (audio_is_timer_needed ()) {
         timer_mod_anticipate_ns(s->ts,
             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
-    }
-    else {
-        timer_del (s->ts);
+        if (!audio_timer_running) {
+            audio_timer_running = true;
+            audio_timer_last = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+            trace_audio_timer_start(conf.period.ticks / 1000000);
+        }
+    } else {
+        timer_del(s->ts);
+        if (audio_timer_running) {
+            audio_timer_running = false;
+            trace_audio_timer_stop();
+        }
     }
 }
 
 static void audio_timer (void *opaque)
 {
+    int64_t now, diff;
+
+    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    diff = now - audio_timer_last;
+    if (diff > conf.period.ticks * 3 / 2) {
+        trace_audio_timer_delayed(diff / 1000000);
+    }
+    audio_timer_last = now;
+
     audio_run ("timer");
     audio_reset_timer (opaque);
 }
diff --git a/audio/trace-events b/audio/trace-events
index d37639e611..c986469319 100644
--- a/audio/trace-events
+++ b/audio/trace-events
@@ -15,3 +15,8 @@ alsa_no_frames(int state) "No frames available and ALSA state is %d"
 # audio/ossaudio.c
 oss_version(int version) "OSS version = 0x%x"
 oss_invalid_available_size(int size, int bufsize) "Invalid available size, size=%d bufsize=%d"
+
+# audio/audio.c
+audio_timer_start(int interval) "interval %d ms"
+audio_timer_stop(void) ""
+audio_timer_delayed(int interval) "interval %d ms"
-- 
2.9.3

Re: [Qemu-devel] [PATCH 3/3] audio: add audio timer trace points

[Philippe Mathieu-Daudé]

Hi Gerd,

On 07/02/2018 11:55 AM, Gerd Hoffmann wrote:
> Track audio timer starts and stops.  Also trace delayed audio timer calls.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  audio/audio.c      | 28 +++++++++++++++++++++++++---
>  audio/trace-events |  5 +++++
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index d6e91901aa..d055b2236c 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
>  #include "sysemu/replay.h"
> +#include "audio/trace.h"

Can you simply use "trace.h" like in the rest of the codebase?

>  
>  #define AUDIO_CAP "audio"
>  #include "audio_int.h"
> @@ -1129,6 +1130,10 @@ static void audio_pcm_print_info (const char *cap, struct audio_pcm_info *info)
>  /*
>   * Timer
>   */
> +
> +static bool audio_timer_running;
> +static uint64_t audio_timer_last;
> +
>  static int audio_is_timer_needed (void)
>  {
>      HWVoiceIn *hwi = NULL;
> @@ -1148,14 +1153,31 @@ static void audio_reset_timer (AudioState *s)
>      if (audio_is_timer_needed ()) {
>          timer_mod_anticipate_ns(s->ts,
>              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
> -    }
> -    else {
> -        timer_del (s->ts);
> +        if (!audio_timer_running) {
> +            audio_timer_running = true;
> +            audio_timer_last = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +            trace_audio_timer_start(conf.period.ticks / 1000000);

Can you use SCALE_MS?

> +        }
> +    } else {
> +        timer_del(s->ts);
> +        if (audio_timer_running) {
> +            audio_timer_running = false;
> +            trace_audio_timer_stop();
> +        }
>      }
>  }
>  
>  static void audio_timer (void *opaque)
>  {
> +    int64_t now, diff;
> +
> +    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    diff = now - audio_timer_last;
> +    if (diff > conf.period.ticks * 3 / 2) {
> +        trace_audio_timer_delayed(diff / 1000000);

Ditto.

> +    }
> +    audio_timer_last = now;
> +
>      audio_run ("timer");
>      audio_reset_timer (opaque);
>  }
> diff --git a/audio/trace-events b/audio/trace-events
> index d37639e611..c986469319 100644
> --- a/audio/trace-events
> +++ b/audio/trace-events
> @@ -15,3 +15,8 @@ alsa_no_frames(int state) "No frames available and ALSA state is %d"
>  # audio/ossaudio.c
>  oss_version(int version) "OSS version = 0x%x"
>  oss_invalid_available_size(int size, int bufsize) "Invalid available size, size=%d bufsize=%d"
> +
> +# audio/audio.c
> +audio_timer_start(int interval) "interval %d ms"
> +audio_timer_stop(void) ""
> +audio_timer_delayed(int interval) "interval %d ms"
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [Qemu-devel] [PATCH 3/3] audio: add audio timer trace points

[Gerd Hoffmann]

On Mon, Jul 02, 2018 at 12:25:37PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Gerd,
> 
> On 07/02/2018 11:55 AM, Gerd Hoffmann wrote:
> > Track audio timer starts and stops.  Also trace delayed audio timer calls.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  audio/audio.c      | 28 +++++++++++++++++++++++++---
> >  audio/trace-events |  5 +++++
> >  2 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/audio/audio.c b/audio/audio.c
> > index d6e91901aa..d055b2236c 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -29,6 +29,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "qemu/cutils.h"
> >  #include "sysemu/replay.h"
> > +#include "audio/trace.h"
> 
> Can you simply use "trace.h" like in the rest of the codebase?

Why?  I need the audio trace points only, and including only those
avoids object rebuilds in case some unrelated trace points are updated.

I'm pretty sure this is the reason we have splitted files now instead of
one huge trace.h

> > +            trace_audio_timer_start(conf.period.ticks / 1000000);
> 
> Can you use SCALE_MS?

Will do.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 3/3] audio: add audio timer trace points

[Daniel P. Berrangé]

On Mon, Jul 02, 2018 at 05:50:07PM +0200, Gerd Hoffmann wrote:
> On Mon, Jul 02, 2018 at 12:25:37PM -0300, Philippe Mathieu-Daudé wrote:
> > Hi Gerd,
> > 
> > On 07/02/2018 11:55 AM, Gerd Hoffmann wrote:
> > > Track audio timer starts and stops.  Also trace delayed audio timer calls.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > >  audio/audio.c      | 28 +++++++++++++++++++++++++---
> > >  audio/trace-events |  5 +++++
> > >  2 files changed, 30 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/audio/audio.c b/audio/audio.c
> > > index d6e91901aa..d055b2236c 100644
> > > --- a/audio/audio.c
> > > +++ b/audio/audio.c
> > > @@ -29,6 +29,7 @@
> > >  #include "sysemu/sysemu.h"
> > >  #include "qemu/cutils.h"
> > >  #include "sysemu/replay.h"
> > > +#include "audio/trace.h"
> > 
> > Can you simply use "trace.h" like in the rest of the codebase?
> 
> Why?  I need the audio trace points only, and including only those
> avoids object rebuilds in case some unrelated trace points are updated.

There is no global 'trace.h' any more - an unqualified  #include "trace.h"
will resolve to the local build directory first, so still pull in the same
audio/trace.h

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 2/3] audio/hda: fix CID 1393631

[Philippe Mathieu-Daudé]

On 07/02/2018 11:55 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/audio/hda-codec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
> index 9f630fa37f..2b58c3505b 100644
> --- a/hw/audio/hda-codec.c
> +++ b/hw/audio/hda-codec.c
> @@ -189,7 +189,7 @@ struct HDAAudioState {
>  
>  static inline int64_t hda_bytes_per_second(HDAAudioStream *st)
>  {
> -    return 2 * st->as.nchannels * st->as.freq;
> +    return 2LL * st->as.nchannels * st->as.freq;
>  }
>  
>  static inline void hda_timer_sync_adjust(HDAAudioStream *st, int64_t target_pos)
>