On 2014-11-13 10:48, Peter Meerwald wrote:
From: Peter Meerwald <p.meerw...@bct-electronic.com>

snd_pcm_mmap_commit() actually transfers the memory area prepared by 
snd_pcm_mmap_begin(),
it returns the 'count of transferred frames' which should be equal to the 
number of frames
returned by snd_pcm_mmap_begin()

however, this identify is not checked and the number of frames prepared are 
accounted for,
not the number of frames commited -- this is wrong; the ALSA example codes 
bothers to
check snd_pcm_mmap_commit()'s returned number of frames

this patch just outputs a warning when sframes != frames -- let's see if it 
ever happens

It also seems to do refactoring?


v2: (thanks David Henningson)
* just log, no functional change as in v1
* fix typos, fix subject

Signed-off-by: Peter Meerwald <pme...@pmeerw.net>
---
  src/modules/alsa/alsa-sink.c   |   11 +++++++++--
  src/modules/alsa/alsa-source.c |   17 +++++++++++++----
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index e256bbd..35f013c 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -639,8 +639,7 @@ static int mmap_write(struct userdata *u, pa_usec_t 
*sleep_usec, bool polled, bo

              p = (uint8_t*) areas[0].addr + (offset * u->frame_size);

-            written = frames * u->frame_size;
-            chunk.memblock = pa_memblock_new_fixed(u->core->mempool, p, 
written, true);
+            chunk.memblock = pa_memblock_new_fixed(u->core->mempool, p, frames * 
u->frame_size, true);
              chunk.length = pa_memblock_get_length(chunk.memblock);
              chunk.index = 0;

@@ -660,6 +659,14 @@ static int mmap_write(struct userdata *u, pa_usec_t 
*sleep_usec, bool polled, bo

              work_done = true;

+            if (PA_UNLIKELY((snd_pcm_uframes_t) sframes != frames)) {
+                PA_ONCE_BEGIN {
+                    pa_log_warn("ALSA mmap write was set up for %lu frames, but 
unexpectedly commited %lu frames.\n",
+                        (unsigned long) frames, (unsigned long) sframes);
+                } PA_ONCE_END;
+            }
+
+            written = frames * u->frame_size;
              u->write_count += written;
              u->since_start += written;

diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index 111c517..97d0d22 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -562,6 +562,7 @@ static int mmap_read(struct userdata *u, pa_usec_t 
*sleep_usec, bool polled, boo
              const snd_pcm_channel_area_t *areas;
              snd_pcm_uframes_t offset, frames;
              snd_pcm_sframes_t sframes;
+            size_t got;

              frames = (snd_pcm_uframes_t) (n_bytes / u->frame_size);
  /*             pa_log_debug("%lu frames to read", (unsigned long) frames); */
@@ -613,16 +614,24 @@ static int mmap_read(struct userdata *u, pa_usec_t 
*sleep_usec, bool polled, boo

              work_done = true;

-            u->read_count += frames * u->frame_size;
+            if (PA_UNLIKELY((snd_pcm_uframes_t) sframes != frames)) {
+                PA_ONCE_BEGIN {
+                    pa_log_warn("ALSA mmap read was set up for %lu frames, but 
unexpectedly commited %lu frames.\n",
+                        (unsigned long) frames, (unsigned long) sframes);
+                } PA_ONCE_END;
+            }
+
+            got = frames * u->frame_size;
+            u->read_count += got;

  #ifdef DEBUG_TIMING
-            pa_log_debug("Read %lu bytes (of possible %lu bytes)", (unsigned 
long) (frames * u->frame_size), (unsigned long) n_bytes);
+            pa_log_debug("Read %lu bytes (of possible %lu bytes)", (unsigned 
long) got, (unsigned long) n_bytes);
  #endif

-            if ((size_t) frames * u->frame_size >= n_bytes)
+            if (got >= n_bytes)
                  break;

-            n_bytes -= (size_t) frames * u->frame_size;
+            n_bytes -= got;
          }
      }



--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to