[pulseaudio-discuss] [PATCH 08/11] client: Allow client to receive the srchannel memblock

[Tanu Kaskinen]

On Tue, 2014-04-29 at 17:29 +0200, David Henningsson wrote:
> 
> On 2014-04-29 15:22, David Henningsson wrote:
> > We assume it's an srchannel memblock if it is writable and does not come
> > from our own mempool.
> >
> > In a future implementation, maybe we can have more than one
> > srchannel open at the same time, but at this point we only
> > support one.
> >
> > Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> > ---
> >   src/pulse/context.c  | 26 ++++++++++++++++++++++++++
> >   src/pulse/internal.h |  1 +
> >   2 files changed, 27 insertions(+)
> >
> > diff --git a/src/pulse/context.c b/src/pulse/context.c
> > index f3adf4c..7456ee5 100644
> > --- a/src/pulse/context.c
> > +++ b/src/pulse/context.c
> > @@ -213,6 +213,11 @@ static void context_unlink(pa_context *c) {
> >           c->pstream = NULL;
> >       }
> >
> > +    if (c->srmemblock) {
> > +        pa_memblock_unref(c->srmemblock);
> > +        c->srmemblock = NULL;
> > +    }
> > +
> >       if (c->client) {
> >           pa_socket_client_unref(c->client);
> >           c->client = NULL;
> > @@ -338,6 +343,17 @@ static void pstream_packet_callback(pa_pstream *p, pa_packet *packet, const pa_a
> >       pa_context_unref(c);
> >   }
> >
> 
> Btw, I wasn't sure if the below is a good heuristic for figuring out 
> whether this is an srchannel memblock or not. I was considering 
> (ab)using pa_seek_mode_t instead, i e, add a new seek mode indicating 
> that this is an srmemblock. What do you think?

It would nice to have explicit information available. What would you
think about adding a new memblock type, or adding function
pa_memblock_is_srchannel_buffer()?

> > +static bool is_srmemblock(pa_context *c, const pa_memchunk *chunk)
> > +{
> > +    if (!chunk || !chunk->memblock)
> > +        return false;
> > +    if (pa_memblock_is_read_only(chunk->memblock))
> > +        return false;
> > +    if (pa_memblock_is_ours(chunk->memblock))
> > +        return false;
> > +    return true;
> > +}
> > +
> >   static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t offset, pa_seek_mode_t seek, const pa_memchunk *chunk, void *userdata) {
> >       pa_context *c = userdata;
> >       pa_stream *s;
> > @@ -350,6 +366,16 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
> >
> >       pa_context_ref(c);
> >
> > +    if (is_srmemblock(c, chunk)) {
> > +        if (c->srmemblock) {
> > +            pa_log_warn("There is already an srmemblock. Ignoring the new one.");
> > +        }
> > +        else {
> > +            c->srmemblock = chunk->memblock;
> > +            pa_memblock_ref(c->srmemblock);
> > +        }

I think it would be good to return here, or somehow skip the code that
tries to treat the memblock as audio stream data.

> > +    }
> > +
> >       if ((s = pa_hashmap_get(c->record_streams, PA_UINT32_TO_PTR(channel)))) {
> >
> >           if (chunk->memblock) {

-- 
Tanu