From: David Henningsson <david.henningsson@canonical.com>
To: "Alexander E. Patrakov" <patrakov@gmail.com>,
alsa-devel@alsa-project.org
Cc: tiwai@suse.de
Subject: Re: [PATCH] Fix forward/rewind support in iec958 plugin
Date: Thu, 24 Apr 2014 13:19:50 +0200 [thread overview]
Message-ID: <5358F356.3020701@canonical.com> (raw)
In-Reply-To: <1398278427-10637-1-git-send-email-patrakov@gmail.com>
On 2014-04-23 20:40, Alexander E. Patrakov wrote:
> When forwarding or rewinding, the frame counter was not updated. This
> could result in corrupted channel status words or misplaced Z-type
> preamble.
> ---
> I found the issue by inspecting the code. However, I have no hardware
> to test this fix on.
>
> src/pcm/pcm_iec958.c | 64 +++++++++++++++++++++++++++++++++++++++++++++-
> src/pcm/pcm_plugin.c | 28 ++++++++++----------
> src/pcm/pcm_plugin.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 145 insertions(+), 19 deletions(-)
>
> diff --git a/src/pcm/pcm_iec958.c b/src/pcm/pcm_iec958.c
> index d81b0a1..facf683 100644
> --- a/src/pcm/pcm_iec958.c
> +++ b/src/pcm/pcm_iec958.c
> @@ -416,6 +416,35 @@ static void snd_pcm_iec958_dump(snd_pcm_t *pcm, snd_output_t *out)
> snd_pcm_dump(iec->plug.gen.slave, out);
> }
>
> +static snd_pcm_sframes_t snd_pcm_iec958_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> +{
> + unsigned int counter_decrement;
> + snd_pcm_iec958_t *iec = pcm->private_data;
> + snd_pcm_sframes_t result = snd_pcm_plugin_rewind(pcm, frames);
> + if (result <= 0)
> + return result;
> +
> + counter_decrement = result % 192;
> + iec->counter += 192 - counter_decrement;
> + iec->counter %= 192;
> + return result;
> +}
> +
> +static snd_pcm_sframes_t snd_pcm_iec958_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> +
> +{
> + unsigned int counter_increment;
> + snd_pcm_iec958_t *iec = pcm->private_data;
> + snd_pcm_sframes_t result = snd_pcm_plugin_rewind(pcm, frames);
> + if (result <= 0)
> + return result;
> +
> + counter_increment = result % 192;
> + iec->counter += counter_increment;
> + iec->counter %= 192;
> + return result;
> +}
> +
> static const snd_pcm_ops_t snd_pcm_iec958_ops = {
> .close = snd_pcm_generic_close,
> .info = snd_pcm_generic_info,
> @@ -434,6 +463,39 @@ static const snd_pcm_ops_t snd_pcm_iec958_ops = {
> .set_chmap = snd_pcm_generic_set_chmap,
> };
>
> +
> +static const snd_pcm_fast_ops_t snd_pcm_iec958_fast_ops = {
> + .status = snd_pcm_plugin_status,
> + .state = snd_pcm_generic_state,
> + .hwsync = snd_pcm_generic_hwsync,
> + .delay = snd_pcm_plugin_delay,
> + .prepare = snd_pcm_plugin_prepare,
> + .reset = snd_pcm_plugin_reset,
> + .start = snd_pcm_generic_start,
> + .drop = snd_pcm_generic_drop,
> + .drain = snd_pcm_generic_drain,
> + .pause = snd_pcm_generic_pause,
> + .rewindable = snd_pcm_plugin_rewindable,
> + .rewind = snd_pcm_iec958_rewind,
> + .forwardable = snd_pcm_plugin_forwardable,
> + .forward = snd_pcm_iec958_forward,
> + .resume = snd_pcm_generic_resume,
> + .link = snd_pcm_generic_link,
> + .link_slaves = snd_pcm_generic_link_slaves,
> + .unlink = snd_pcm_generic_unlink,
> + .writei = snd_pcm_plugin_writei,
> + .writen = snd_pcm_plugin_writen,
> + .readi = snd_pcm_plugin_readi,
> + .readn = snd_pcm_plugin_readn,
> + .avail_update = snd_pcm_plugin_avail_update,
> + .mmap_commit = snd_pcm_plugin_mmap_commit,
> + .htimestamp = snd_pcm_generic_htimestamp,
> + .poll_descriptors_count = snd_pcm_generic_poll_descriptors_count,
> + .poll_descriptors = snd_pcm_generic_poll_descriptors,
> + .poll_revents = snd_pcm_generic_poll_revents,
> + .may_wait_for_avail_min = snd_pcm_generic_may_wait_for_avail_min,
> +};
> +
> /**
> * \brief Creates a new IEC958 subframe conversion PCM
> * \param pcmp Returns created PCM handle
> @@ -495,7 +557,7 @@ int snd_pcm_iec958_open(snd_pcm_t **pcmp, const char *name, snd_pcm_format_t sfo
> return err;
> }
> pcm->ops = &snd_pcm_iec958_ops;
> - pcm->fast_ops = &snd_pcm_plugin_fast_ops;
> + pcm->fast_ops = &snd_pcm_iec958_fast_ops;
Instead of making all pcm_plugin functions public, you can consider
something like this:
ptr = malloc(sizeof(snd_pcm_fast_ops_t));
*ptr = snd_pcm_plugin_fast_ops;
ptr->rewind = snd_pcm_iec958_rewind;
ptr->forward = snd_pcm_iec958_forward;
pcm->fast_ops = ptr;
(and deallocate ptr somewhere else)
> pcm->private_data = iec;
> pcm->poll_fd = slave->poll_fd;
> pcm->poll_events = slave->poll_events;
> diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c
> index 17157e8..022d908 100644
> --- a/src/pcm/pcm_plugin.c
> +++ b/src/pcm/pcm_plugin.c
> @@ -137,7 +137,7 @@ void snd_pcm_plugin_init(snd_pcm_plugin_t *plugin)
> snd_atomic_write_init(&plugin->watom);
> }
>
> -static int snd_pcm_plugin_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
> +int snd_pcm_plugin_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> snd_pcm_sframes_t sd;
> @@ -154,7 +154,7 @@ static int snd_pcm_plugin_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
> return 0;
> }
>
> -static int snd_pcm_plugin_prepare(snd_pcm_t *pcm)
> +int snd_pcm_plugin_prepare(snd_pcm_t *pcm)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> int err;
> @@ -175,7 +175,7 @@ static int snd_pcm_plugin_prepare(snd_pcm_t *pcm)
> return 0;
> }
>
> -static int snd_pcm_plugin_reset(snd_pcm_t *pcm)
> +int snd_pcm_plugin_reset(snd_pcm_t *pcm)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> int err;
> @@ -196,12 +196,12 @@ static int snd_pcm_plugin_reset(snd_pcm_t *pcm)
> return 0;
> }
>
> -static snd_pcm_sframes_t snd_pcm_plugin_rewindable(snd_pcm_t *pcm)
> +snd_pcm_sframes_t snd_pcm_plugin_rewindable(snd_pcm_t *pcm)
> {
> return snd_pcm_mmap_hw_avail(pcm);
> }
>
> -static snd_pcm_sframes_t snd_pcm_plugin_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> +snd_pcm_sframes_t snd_pcm_plugin_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> snd_pcm_sframes_t n = snd_pcm_mmap_hw_avail(pcm);
> @@ -224,12 +224,12 @@ static snd_pcm_sframes_t snd_pcm_plugin_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t
> return (snd_pcm_sframes_t) sframes;
> }
>
> -static snd_pcm_sframes_t snd_pcm_plugin_forwardable(snd_pcm_t *pcm)
> +snd_pcm_sframes_t snd_pcm_plugin_forwardable(snd_pcm_t *pcm)
> {
> return snd_pcm_mmap_avail(pcm);
> }
>
> -static snd_pcm_sframes_t snd_pcm_plugin_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> +snd_pcm_sframes_t snd_pcm_plugin_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> snd_pcm_sframes_t n = snd_pcm_mmap_avail(pcm);
> @@ -347,7 +347,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm,
> }
>
>
> -static snd_pcm_sframes_t
> +snd_pcm_sframes_t
> snd_pcm_plugin_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size)
> {
> snd_pcm_channel_area_t areas[pcm->channels];
> @@ -356,7 +356,7 @@ snd_pcm_plugin_writei(snd_pcm_t *pcm, const void *buffer, snd_pcm_uframes_t size
> snd_pcm_plugin_write_areas);
> }
>
> -static snd_pcm_sframes_t
> +snd_pcm_sframes_t
> snd_pcm_plugin_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
> {
> snd_pcm_channel_area_t areas[pcm->channels];
> @@ -365,7 +365,7 @@ snd_pcm_plugin_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
> snd_pcm_plugin_write_areas);
> }
>
> -static snd_pcm_sframes_t
> +snd_pcm_sframes_t
> snd_pcm_plugin_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
> {
> snd_pcm_channel_area_t areas[pcm->channels];
> @@ -374,7 +374,7 @@ snd_pcm_plugin_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_uframes_t size)
> snd_pcm_plugin_read_areas);
> }
>
> -static snd_pcm_sframes_t
> +snd_pcm_sframes_t
> snd_pcm_plugin_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
> {
> snd_pcm_channel_area_t areas[pcm->channels];
> @@ -383,7 +383,7 @@ snd_pcm_plugin_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_uframes_t size)
> snd_pcm_plugin_read_areas);
> }
>
> -static snd_pcm_sframes_t
> +snd_pcm_sframes_t
> snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
> snd_pcm_uframes_t offset ATTRIBUTE_UNUSED,
> snd_pcm_uframes_t size)
> @@ -452,7 +452,7 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm,
> return xfer;
> }
>
> -static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
> +snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> snd_pcm_t *slave = plugin->gen.slave;
> @@ -516,7 +516,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm)
> }
> }
>
> -static int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
> +int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
> {
> snd_pcm_plugin_t *plugin = pcm->private_data;
> snd_pcm_sframes_t err;
> diff --git a/src/pcm/pcm_plugin.h b/src/pcm/pcm_plugin.h
> index 7ee7c7f..7734369 100644
> --- a/src/pcm/pcm_plugin.h
> +++ b/src/pcm/pcm_plugin.h
> @@ -50,14 +50,42 @@ typedef struct {
> } snd_pcm_plugin_t;
>
> /* make local functions really local */
> -#define snd_pcm_plugin_init \
> - snd1_pcm_plugin_init
> -#define snd_pcm_plugin_fast_ops \
> - snd1_pcm_plugin_fast_ops
> #define snd_pcm_plugin_undo_read_generic \
> snd1_pcm_plugin_undo_read_generic
> #define snd_pcm_plugin_undo_write_generic \
> snd1_pcm_plugin_undo_write_generic
> +#define snd_pcm_plugin_init \
> + snd1_pcm_plugin_init
> +#define snd_pcm_plugin_delay \
> + snd1_pcm_plugin_delay
> +#define snd_pcm_plugin_prepare \
> + snd1_pcm_plugin_prepare
> +#define snd_pcm_plugin_reset \
> + snd1_pcm_plugin_reset
> +#define snd_pcm_plugin_rewindable \
> + snd1_pcm_plugin_rewindable
> +#define snd_pcm_plugin_rewind \
> + snd1_pcm_plugin_rewind
> +#define snd_pcm_plugin_forwardable \
> + snd1_pcm_plugin_forwardable
> +#define snd_pcm_plugin_forward \
> + snd1_pcm_plugin_forward
> +#define snd_pcm_plugin_writei \
> + snd1_pcm_plugin_writei
> +#define snd_pcm_plugin_writen \
> + snd1_pcm_plugin_writen
> +#define snd_pcm_plugin_readi \
> + snd1_pcm_plugin_readi
> +#define snd_pcm_plugin_readn \
> + snd1_pcm_plugin_readn
> +#define snd_pcm_plugin_mmap_commit \
> + snd1_pcm_plugin_mmap_commit
> +#define snd_pcm_plugin_avail_update \
> + snd1_pcm_plugin_avail_update
> +#define snd_pcm_plugin_status \
> + snd1_pcm_plugin_status
> +#define snd_pcm_plugin_fast_ops \
> + snd1_pcm_plugin_fast_ops
>
> void snd_pcm_plugin_init(snd_pcm_plugin_t *plugin);
>
> @@ -77,6 +105,42 @@ snd_pcm_sframes_t snd_pcm_plugin_undo_write_generic
> snd_pcm_uframes_t res_size, /* size of result areas */
> snd_pcm_uframes_t slave_undo_size);
>
> +int snd_pcm_plugin_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp);
> +int snd_pcm_plugin_prepare(snd_pcm_t *pcm);
> +int snd_pcm_plugin_reset(snd_pcm_t *pcm);
> +snd_pcm_sframes_t snd_pcm_plugin_rewindable(snd_pcm_t *pcm);
> +snd_pcm_sframes_t snd_pcm_plugin_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
> +snd_pcm_sframes_t snd_pcm_plugin_forwardable(snd_pcm_t *pcm);
> +snd_pcm_sframes_t snd_pcm_plugin_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_writei
> + (snd_pcm_t *pcm,
> + const void *buffer,
> + snd_pcm_uframes_t size);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_writen
> + (snd_pcm_t *pcm,
> + void **bufs,
> + snd_pcm_uframes_t size);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_readi
> + (snd_pcm_t *pcm,
> + void *buffer,
> + snd_pcm_uframes_t size);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_readn
> + (snd_pcm_t *pcm,
> + void **bufs,
> + snd_pcm_uframes_t size);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_mmap_commit
> + (snd_pcm_t *pcm,
> + snd_pcm_uframes_t offset,
> + snd_pcm_uframes_t size);
> +
> +snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm);
> +int snd_pcm_plugin_status(snd_pcm_t *pcm, snd_pcm_status_t * status);
> +
> /* make local functions really local */
> #define snd_pcm_linear_get_index snd1_pcm_linear_get_index
> #define snd_pcm_linear_put_index snd1_pcm_linear_put_index
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
next prev parent reply other threads:[~2014-04-24 11:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-23 18:40 [PATCH] Fix forward/rewind support in iec958 plugin Alexander E. Patrakov
2014-04-24 11:19 ` David Henningsson [this message]
2014-04-24 14:40 ` [PATCH v2] " Alexander E. Patrakov
2014-04-24 23:02 ` Raymond Yau
2014-04-25 6:00 ` Alexander E. Patrakov
2014-04-25 8:09 ` Clemens Ladisch
2014-04-28 16:10 ` Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5358F356.3020701@canonical.com \
--to=david.henningsson@canonical.com \
--cc=alsa-devel@alsa-project.org \
--cc=patrakov@gmail.com \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.