Re: [pulseaudio-discuss] Rust binding licensing

[jnqnfe]

> Ok, thanks for looking into it :)
> 
> I've sent a message to the FSF (LGPL authors) to ask for a
> clarification from them.
> 
> I'll have a think about whether it is at all feasible to either split
> the documentation out like that, or to build a list of copyright
> owners
> that would need to grant permission myself (I couldn't expect you to
> put the effort in on my behalf).
> 
> Regards,
> Lyndon
> 

I've not yet received any response from FSF. I did however re-discover
the GPL/LGPL FAQ entry that clarifies that fair use does apply. I may
proceed on that basis.

Regards,
Lyndon

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Rust binding licensing

[jnqnfe]

> The documentation is copyrighted, and since there are no special
> exceptions mentioned anywhere, it's covered by LGPL like everything
> else. I now read what the license says, and I got the impression that
> if you copy the documentation, you have created a derivative work,
> and the whole work must be licensed under LGPL, and you can't change
> the licensing terms by adding alternative licenses.
>
> Granting an exception for your project probably isn't feasible. The
> copyrights to PulseAudio code isn't held by any centralized entity.
> Someone would have to inspect the history of everything you've copied
> to find out who are the people who hold copyrights to the
> documentation. Only those who hold the copyrights can grant
> exceptions.
>
> One possibility might be that you split off the documentation to a
> separate work (probably best to create a separate git repository) so
> that only the documentation needs to be under LGPL and the bindings
> themselves you can license however you wish.
>
>-- 
>Tanu

Ok, thanks for looking into it :)

I've sent a message to the FSF (LGPL authors) to ask for a
clarification from them.

I'll have a think about whether it is at all feasible to either split
the documentation out like that, or to build a list of copyright owners
that would need to grant permission myself (I couldn't expect you to
put the effort in on my behalf).

Regards,
Lyndon

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] Rust binding licensing

[jnqnfe]

Hi,

I don't know if you remember me, but I'm the author of the Rust binding
(https://github.com/jnqnfe/pulse-binding-rust) for PA.

I am a little uncertain about licensing with regards to my binding,
specifically whether it in any way actually does fall under the concept
of "derived work" and thus is subject to LGPL, or whether it is simply
a copyright matter.

The things it all hinges upon I suppose is the fact that I've copied
much of the documentation, and since licensing is primarily concerned
with actual code and use of code in compiled form within
binaries/shared-libs/firmwares, I'm just not certain where I stand with
just the documentation aspect and got nowhere googling the topic.

I have outlined much of what needs to be said regarding licensing wrt.
my Rust binding in the project's main readme (
https://github.com/jnqnfe/pulse-binding-rust/blob/master/README.md).

The big issue I have with LGPL is that in the Rust world the default
way "crates" (libraries) are consumed is static compiling (you just
specify the name of a crate as a dependency in your project and it is
just automatically downloaded and compiled in). Many Rust crates are
MIT+Apache licensed, meaning little or no impact regarding the
licensing of the end product they are built into. When it comes to LGPL
crates however, for compliance you must either make the product
GPL/LGPL or else compile the dependent crate (i.e. my binding here) as
a dynamic shared lib.

Since the default with Rust crates is static compiling, there's great
danger in people just plowing on ahead with using it in non-GPL/LGPL
work, not even realising they're breaking LGPL licensing requirements
(either directly, or indirectly as an inherited dependency from
something else). If they do follow the requirements, building as a
shared lib, then that becomes rather a pain to deal with and an
inefficiency issue, just for a thin binding layer in this case.

As discussed in the project readme, I leave open the possibility of
using my work under MIT+Apache under strict conditions, while
officially marking it in the crates as LGPL. It would be great though
to resolve the unanswered question.

If what I have done with copying the documentation *is* covered by the
LGPL such that it *is* a derived work and thus I am forced by default
to also use LGPL, then would you considered granting my project an
exception, specifically just for the aspects that apply, i.e. copying
the documentation? Such that my binding can freely be MIT+Apache
licensed and statically compiled, while not affecting the use of PA
libs themselves of course.

Regards,
Lyndon

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #4 (pa_mainloop_api)

[jnqnfe]

On Tue, 2018-07-10 at 13:54 +0300, Tanu Kaskinen wrote:
> On Mon, 2018-07-09 at 18:16 +0100, jnq...@gmail.com wrote:
> > On Mon, 2018-07-09 at 13:23 +0300, Tanu Kaskinen wrote:
> > > On Thu, 2018-07-05 at 23:51 +0100, jnq...@gmail.com wrote:
> > > > On Wed, 2018-07-04 at 11:46 +0300, Tanu Kaskinen wrote:
> > > > > Patch 4 changes the callback signatures.
> > > > 
> > > > Right, but only in the mainloop api vtable, which user's only
> > > > read
> > > > values from, so I wasn't expecting that to be an issue.
> > > 
> > > Applications can also write to it, if they implement their own
> > > mainloops.
> > 
> > Ah, that use case was not mentioned in our previous discussion. I'd
> > asked if the api struct was immutable, or whether there was any
> > intention to allow users to modify it, to say hijack one or more
> > function implementations (i.e. a partial customised mainloop), and
> > you'd said that there was no such intention. That discussion left
> > me
> > with the understanding that custom/partial-custom mainloops weren't
> > a
> > thing.
> > 
> > Do you happen to know of any examples of such programs implementing
> > custom mainloops that create and pass around a mainloop-api vtable
> > populated with their own pointers?
> 
> Debian Code Search gives a few examples:
> 
> https://codesearch.debian.net/search?q=defer_new+%3D+

Ok, thanks I'll take a look at those

> > > When I wrote my previous mail, I thought that patch 4 also
> > > changes
> > > the event callback types (pa_io_event_cb_t etc.), and that was
> > > what I
> > > was primarily concerned about
> > 
> > Yeah I deliberately left change to those callback signatures until
> > a
> > later commit (#25 of 26 I believe), for precisely those same
> > concerns.
> 
> I don't know if you realized, but already patch 2 depends on that
> change.

Yes, but patches 2 & 3 only depend on it to avoid a warning, which I
hoped was acceptable until the later few API change patches would make
it into a new major version

> > > (but applications implementing their own mainloops is still a
> > > real
> > > problem).
> > 
> > Right... :/
> > Well, I guess no matter how rare custom mainloops might be, if
> > custom
> > mainloops are a legit supported feature, then this change breaks it
> > and
> > thus must be considered API breakage, and that holds back
> > effectively
> > the entire patch set until you're willing to issue a new API-break
> > release :/
> 
> If we're ever going to make "libpulse 2", we need to have a plan
> about
> everything that we want to change. I don't want to create a new
> library
> just to constify the mainloop API... Feel free to start a wiki page
> that lists compatibility breaking changes that we might want to make.

Oh. I'm not used to library maintenance. I wasn't expecting a new lib
to be necessary for some ABI-compatible API changes, just a new major
version with an API bump to the API declaration in the version
header...

On the topic of such a list though, all that I would have in mind
besides the mainloop-api thing would be the purging of autoload (plus
removal of a related attribute in the middle of a struct that my
previous 'purge-autoload' patch deliberately left untouched), and a
switch to use of boolean parameters.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #4 (pa_mainloop_api)

[jnqnfe]

On Mon, 2018-07-09 at 13:23 +0300, Tanu Kaskinen wrote:
> On Thu, 2018-07-05 at 23:51 +0100, jnq...@gmail.com wrote:
> > On Wed, 2018-07-04 at 11:46 +0300, Tanu Kaskinen wrote:
> > > On Mon, 2018-07-02 at 17:58 +0100, jnq...@gmail.com wrote:
> > > > On Fri, 2018-06-29 at 16:06 +0300, Tanu Kaskinen wrote:
> > > > > On Thu, 2018-06-28 at 04:08 +0100, jnq...@gmail.com wrote:
> > > > > > It's possible for patches #21, #23 and #24 to be bumped
> > > > > > down
> > > > > > before
> > > > > > the
> > > > > > API change of #20, but as things are they are blocked by it
> > > > > > and
> > > > > > it
> > > > > > would require a temporary hack... #23 and #24 (utils)
> > > > > > depend
> > > > > > upon
> > > > > > pa_signal_init(), done in #21 which involves changing the
> > > > > > underlying
> > > > > > static; this depends upon changing pa_signal_destroy_cb_t
> > > > > > (#20),
> > > > > > because of the way pa_signal_free() works (passing that
> > > > > > static
> > > > > > api
> > > > > > ref
> > > > > > to the destroy callback which requires non-const). Getting
> > > > > > around
> > > > > > this
> > > > > > would require injecting a temporary hack into
> > > > > > pa_signal_free()
> > > > > > giving
> > > > > > alternate means of obtaining a non-const api pointer - i.e.
> > > > > > api
> > > > > > function pointer comparison would be necessary to tell us
> > > > > > which
> > > > > > mainloop is in use, to then get what we need using the
> > > > > > correct
> > > > > > *_get_api() with api->userdata.
> > > > > 
> > > > > We can't change the signatures of publicly declared callback
> > > > > types.
> > > > > That will cause incompatibilities (compiler warnings at
> > > > > least,
> > > > > which
> > > > > is
> > > > > bad enough) in applications.
> > > > 
> > > > Sure
> > > > 
> > > > > Therefore the only patch that I could
> > > > > apply is the first one.
> > > > 
> > > > But the types aren't changed until #19/26 ...
> > > 
> > > Patch 4 changes the callback signatures.
> > 
> > Right, but only in the mainloop api vtable, which user's only read
> > values from, so I wasn't expecting that to be an issue.
> 
> Applications can also write to it, if they implement their own
> mainloops.

Ah, that use case was not mentioned in our previous discussion. I'd
asked if the api struct was immutable, or whether there was any
intention to allow users to modify it, to say hijack one or more
function implementations (i.e. a partial customised mainloop), and
you'd said that there was no such intention. That discussion left me
with the understanding that custom/partial-custom mainloops weren't a
thing.

Do you happen to know of any examples of such programs implementing
custom mainloops that create and pass around a mainloop-api vtable
populated with their own pointers?

> When I wrote my previous mail, I thought that patch 4 also changes
> the event callback types (pa_io_event_cb_t etc.), and that was what I
> was primarily concerned about

Yeah I deliberately left change to those callback signatures until a
later commit (#25 of 26 I believe), for precisely those same concerns.

> (but applications implementing their own mainloops is still a real
> problem).

Right... :/
Well, I guess no matter how rare custom mainloops might be, if custom
mainloops are a legit supported feature, then this change breaks it and
thus must be considered API breakage, and that holds back effectively
the entire patch set until you're willing to issue a new API-break
release :/

> I can't now find the patch that changes the callback types, but there
> are many patches that change the callback implementation signatures,
> and if there's no corresponding change to the callback types, that
> causes warnings (for example, try applying patch 2 alone, and you'll
> get warnings when building pulseaudio).

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #4 (pa_mainloop_api)

[jnqnfe]

On Wed, 2018-07-04 at 11:46 +0300, Tanu Kaskinen wrote:
> On Mon, 2018-07-02 at 17:58 +0100, jnq...@gmail.com wrote:
> > On Fri, 2018-06-29 at 16:06 +0300, Tanu Kaskinen wrote:
> > > On Thu, 2018-06-28 at 04:08 +0100, jnq...@gmail.com wrote:
> > > > It's possible for patches #21, #23 and #24 to be bumped down
> > > > before
> > > > the
> > > > API change of #20, but as things are they are blocked by it and
> > > > it
> > > > would require a temporary hack... #23 and #24 (utils) depend
> > > > upon
> > > > pa_signal_init(), done in #21 which involves changing the
> > > > underlying
> > > > static; this depends upon changing pa_signal_destroy_cb_t
> > > > (#20),
> > > > because of the way pa_signal_free() works (passing that static
> > > > api
> > > > ref
> > > > to the destroy callback which requires non-const). Getting
> > > > around
> > > > this
> > > > would require injecting a temporary hack into pa_signal_free()
> > > > giving
> > > > alternate means of obtaining a non-const api pointer - i.e. api
> > > > function pointer comparison would be necessary to tell us which
> > > > mainloop is in use, to then get what we need using the correct
> > > > *_get_api() with api->userdata.
> > > 
> > > We can't change the signatures of publicly declared callback
> > > types.
> > > That will cause incompatibilities (compiler warnings at least,
> > > which
> > > is
> > > bad enough) in applications.
> > 
> > Sure
> > 
> > > Therefore the only patch that I could
> > > apply is the first one.
> > 
> > But the types aren't changed until #19/26 ...
> 
> Patch 4 changes the callback signatures.

Right, but only in the mainloop api vtable, which user's only read
values from, so I wasn't expecting that to be an issue.

I've tested with gcc with `-Wall`, hacking the change into my locally
installed headers and recompiling a test program which utilises
`time_new`. No warnings resulted.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [patch] ops: catch repeated finalisation

[jnqnfe]

The internal operation_set_state function already returns early if the
new state is the same as the existing state. The attached patch extends
this to return early if already in a finalised (done/cancelled) state,
i.e. blocks attempts to re-finalise into a different state.

This helps avoid unlinking more than once (or crashing on ref count
assertion).

I was not certain whether an assertion would be a better alternative -
with such a crash helping highlight usage problems...

The situation that lead to this was the thought of someone stupidly
trying to pa_operation_cancel() a callback within the callback
execution itself, while designing a solution for a memory leak related
to cancellation within my Rust binding. While no-one should do such a
thing, if they did, they'd either trip up a ref count assertion, or the
operation would be unlinked twice, which would be bad. It's a simple
thing to catch and mitigate, and could prove to be a useful
bulletproofing measure for this function in general.From ae1751da90d1dfcef7c40ee5e2061173fb781def Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Thu, 5 Jul 2018 04:54:03 +0100
Subject: operation: avoid state change from final state


diff --git a/src/pulse/operation.c b/src/pulse/operation.c
index 61adf69b0..3f396f008 100644
--- a/src/pulse/operation.c
+++ b/src/pulse/operation.c
@@ -102,6 +102,9 @@ static void operation_set_state(pa_operation *o, pa_operation_state_t st) {
 if (st == o->state)
 return;
 
+if ((o->state == PA_OPERATION_DONE) || (o->state == PA_OPERATION_CANCELED))
+return;
+
 pa_operation_ref(o);
 
 o->state = st;
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #4 (pa_mainloop_api)

[jnqnfe]

On Fri, 2018-06-29 at 16:06 +0300, Tanu Kaskinen wrote:
> On Thu, 2018-06-28 at 04:08 +0100, jnq...@gmail.com wrote:
> > constification round #4 (pa_mainloop_api)
> > 
> > 26 patches in total. I attached a zipped copy also.
> > 
> > The first 19 avoid (client) API change; the final 7 do not and will
> > have to wait until ready and happy to do an API bump.
> > 
> > I'm not certain if there's a public API for 3rd-party modules, but
> > if
> > so then I can anticipate issue being taken with respect to
> > pa_iochannel_get_mainloop_api() in #14. Unpicking that though would
> > not
> > be so easy :/
> 
> There's no public API for modules.

Ok

> > It's possible for patches #21, #23 and #24 to be bumped down before
> > the
> > API change of #20, but as things are they are blocked by it and it
> > would require a temporary hack... #23 and #24 (utils) depend upon
> > pa_signal_init(), done in #21 which involves changing the
> > underlying
> > static; this depends upon changing pa_signal_destroy_cb_t (#20),
> > because of the way pa_signal_free() works (passing that static api
> > ref
> > to the destroy callback which requires non-const). Getting around
> > this
> > would require injecting a temporary hack into pa_signal_free()
> > giving
> > alternate means of obtaining a non-const api pointer - i.e. api
> > function pointer comparison would be necessary to tell us which
> > mainloop is in use, to then get what we need using the correct
> > *_get_api() with api->userdata.
> 
> We can't change the signatures of publicly declared callback types.
> That will cause incompatibilities (compiler warnings at least, which
> is
> bad enough) in applications.

Sure

> Therefore the only patch that I could
> apply is the first one.

But the types aren't changed until #19/26 ...
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] question: is the mainloop api vtable mutable?

[jnqnfe]

On Mon, 2018-06-25 at 18:57 +0300, Tanu Kaskinen wrote:
> On Sat, 2018-06-16 at 20:01 +0100, jnq...@gmail.com wrote:
> > Is it intended that the mainloop API vtable be mutable, allowing
> > users
> > to hijack it with their own (proxy) methods, or should it and the
> > 'get_api' functions really be constified?
> > 
> > I held off on constifying this thus far (in both patching PA itself
> > and
> > in my Rust bindings) because I really wasn't sure what the
> > intention
> > was...
> > 
> > With my latest work tidying things up in v2.x of the binding (just
> > released), and with needing to now look to whether `Futures` is
> > something to implement (with which I need to understand how future
> > execution would couple with the mainloop), I really need to know
> > this
> > now...
> 
> The vtable isn't meant to be mutable, but pa_mainloop_get_api() can't
> be changed to return a const pointer, because that will break
> applications that assign the result to a non-const variable. Only
> function arguments can be constified, not function return values.

Ok, right yes. I had to rush that email out and did not think that bit
through.

I've just sent in a load of patches relating to it in a separate email.
I meant to reply to this previously but an update broke my mail client
briefly :/

Thanks :)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #3

[jnqnfe]

On Tue, 2018-06-26 at 12:15 +0300, Tanu Kaskinen wrote:
> On Tue, 2018-06-19 at 17:39 +0100, jnq...@gmail.com wrote:
> > On Mon, 2018-06-18 at 10:47 +0300, Tanu Kaskinen wrote:
> > > Your patches are based on the view that setting the error code
> > > isn't
> > > considered mutating the context object. I think constifying
> > > pa_context_set_error() is fully in line with that view. Or do you
> > > think
> > > that when the validation macros set the code, that's not mutating
> > > the
> > > context object, but when other code calls pa_context_set_error(),
> > > that
> > > is mutating the context object?
> > 
> > Yes, the latter.
> > 
> > > Anyway, I'll send the constification patch.
> > 
> > Ok :)
> 
> I applied my patch now, and the rest of your patches. Thanks for the
> patches!

Ok, no problem :)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #3

[jnqnfe]

On Mon, 2018-06-18 at 10:47 +0300, Tanu Kaskinen wrote:
> On Fri, 2018-06-15 at 21:34 +0100, jnq...@gmail.com wrote:
> > On Fri, 2018-06-15 at 14:15 +0300, Tanu Kaskinen wrote:
> > > On Thu, 2018-06-07 at 05:01 +0100, jnq...@gmail.com wrote:
> > > > API constification set #3
> > > > 
> > > > Some API functions perform validation routines which may modify
> > > > the
> > > > 'error' attribute of a context object. For API functions where
> > > > the
> > > > "primary" object is not a context object, and the object holds
> > > > a
> > > > non-
> > > > const context pointer, this internal mechanism could be
> > > > successfully
> > > > hidden - such functions were constified in my previous two
> > > > patch
> > > > sets.
> > > > 
> > > > However, some functions, namely those related directly to
> > > > context
> > > > objects, had to be passed over previously. These could not be
> > > > constified because the validation routines accessed the context
> > > > object
> > > > directly to change the error attribute.
> > > > 
> > > > This patch set addresses this shortcoming.
> > > > 
> > > > Firstly the patch set moves the context's error attribute
> > > > behind a
> > > > pointer. The validation routines are then changed to store the
> > > > error
> > > > value via this pointer. That then allows a collection of
> > > > further
> > > > API
> > > > functions to be constified.
> > > > 
> > > > (actually pa_context_errno and the two rttime ones could have
> > > > been
> > > > done
> > > > previously on second inspection)
> > > > 
> > > > Although the indirection of the error attribute is obviously
> > > > ever
> > > > so
> > > > slightly worse off for efficiency, it is surely worth the
> > > > price.
> > > > After
> > > > all, the error setting of the validation checks is just an
> > > > artifact
> > > > of
> > > > an internal mechanism and should not be allowed to influence
> > > > the
> > > > public
> > > > API like it currently does.
> > > 
> > > Thanks! I applied patches 1 and 4. Patches 2 and 3 seem
> > > unnecessarily
> > > complicated, wouldn't it be better to just constify
> > > pa_context_set_error()?
> > 
> > It would certainly be more simple, but I would say that it comes
> > down
> > to perceived purpose of a `pa_context_set_error` function.
> > Logically
> > any and all `pa_context_set_*` functions should be expected to
> > mutate
> > the context object in some way and thus take a non-const pointer.
> > Diverging from this in general risks confusion (even though it's
> > internal only), and as a rule I'd err away from making such a
> > function
> > immutable just because an implementation detail allows you to,
> > that's
> > why I went down the route I did.
> > 
> > On the other hand if this function were renamed to something that
> > sufficiently conveyed a 'modify internal mutable component of
> > otherwise
> > immutable object' purpose, with an explanation in the
> > documentation,
> > then that could be perfectly sufficient (perhaps
> > `pa_context_set_internal_error` would do), but then you're risking
> > pushing implementation detail (via the name) out to every place in
> > the
> > library that wants to set an error value, not just the validation
> > routines. Is that acceptable?
> > 
> > Ultimately though it's not at all important to me, I'm happy for
> > you
> > guys to go whatever way you decide and I'm not going to kick up a
> > fuss
> > if you do what you suggest. :)
> 
> Your patches are based on the view that setting the error code isn't
> considered mutating the context object. I think constifying
> pa_context_set_error() is fully in line with that view. Or do you
> think
> that when the validation macros set the code, that's not mutating the
> context object, but when other code calls pa_context_set_error(),
> that
> is mutating the context object?

Yes, the latter.

> Anyway, I'll send the constification patch.

Ok :)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] question: is the mainloop api vtable mutable?

[jnqnfe]

Is it intended that the mainloop API vtable be mutable, allowing users
to hijack it with their own (proxy) methods, or should it and the
'get_api' functions really be constified?

I held off on constifying this thus far (in both patching PA itself and
in my Rust bindings) because I really wasn't sure what the intention
was...

With my latest work tidying things up in v2.x of the binding (just
released), and with needing to now look to whether `Futures` is
something to implement (with which I need to understand how future
execution would couple with the mainloop), I really need to know this
now...
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification round #3

[jnqnfe]

On Fri, 2018-06-15 at 14:15 +0300, Tanu Kaskinen wrote:
> On Thu, 2018-06-07 at 05:01 +0100, jnq...@gmail.com wrote:
> > API constification set #3
> > 
> > Some API functions perform validation routines which may modify the
> > 'error' attribute of a context object. For API functions where the
> > "primary" object is not a context object, and the object holds a
> > non-
> > const context pointer, this internal mechanism could be
> > successfully
> > hidden - such functions were constified in my previous two patch
> > sets.
> > 
> > However, some functions, namely those related directly to context
> > objects, had to be passed over previously. These could not be
> > constified because the validation routines accessed the context
> > object
> > directly to change the error attribute.
> > 
> > This patch set addresses this shortcoming.
> > 
> > Firstly the patch set moves the context's error attribute behind a
> > pointer. The validation routines are then changed to store the
> > error
> > value via this pointer. That then allows a collection of further
> > API
> > functions to be constified.
> > 
> > (actually pa_context_errno and the two rttime ones could have been
> > done
> > previously on second inspection)
> > 
> > Although the indirection of the error attribute is obviously ever
> > so
> > slightly worse off for efficiency, it is surely worth the price.
> > After
> > all, the error setting of the validation checks is just an artifact
> > of
> > an internal mechanism and should not be allowed to influence the
> > public
> > API like it currently does.
> 
> Thanks! I applied patches 1 and 4. Patches 2 and 3 seem unnecessarily
> complicated, wouldn't it be better to just constify
> pa_context_set_error()?

It would certainly be more simple, but I would say that it comes down
to perceived purpose of a `pa_context_set_error` function. Logically
any and all `pa_context_set_*` functions should be expected to mutate
the context object in some way and thus take a non-const pointer.
Diverging from this in general risks confusion (even though it's
internal only), and as a rule I'd err away from making such a function
immutable just because an implementation detail allows you to, that's
why I went down the route I did.

On the other hand if this function were renamed to something that
sufficiently conveyed a 'modify internal mutable component of otherwise
immutable object' purpose, with an explanation in the documentation,
then that could be perfectly sufficient (perhaps
`pa_context_set_internal_error` would do), but then you're risking
pushing implementation detail (via the name) out to every place in the
library that wants to set an error value, not just the validation
routines. Is that acceptable?

Ultimately though it's not at all important to me, I'm happy for you
guys to go whatever way you decide and I'm not going to kick up a fuss
if you do what you suggest. :)

> I altered the commit message of patch 1, because "See email
> discussion"
> seemed like something that could be very annoying to read in the
> commit
> message e.g. ten years from now. I replaced it with a short summary
> of
> the justification for the change.

Yeah, I agree it can be annoying, no problem :)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [patch] purge autoload API

[jnqnfe]

deprecated since 2009From 65c162c725302d862520ec23941fc3a188602fe6 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Wed, 13 Jun 2018 20:32:21 +0100
Subject: introspection: purge autoload

deprecated since 2009

diff --git a/src/map-file b/src/map-file
index 9b6cba22..baf3799f 100644
--- a/src/map-file
+++ b/src/map-file
@@ -27,15 +27,11 @@ pa_channel_position_from_string;
 pa_channel_position_to_pretty_string;
 pa_channel_position_to_string;
 pa_channels_valid;
-pa_context_add_autoload;
 pa_context_connect;
 pa_context_disconnect;
 pa_context_drain;
 pa_context_errno;
 pa_context_exit_daemon;
-pa_context_get_autoload_info_by_index;
-pa_context_get_autoload_info_by_name;
-pa_context_get_autoload_info_list;
 pa_context_get_card_info_by_index;
 pa_context_get_card_info_by_name;
 pa_context_get_card_info_list;
@@ -82,8 +78,6 @@ pa_context_play_sample_with_proplist;
 pa_context_proplist_remove;
 pa_context_proplist_update;
 pa_context_ref;
-pa_context_remove_autoload_by_index;
-pa_context_remove_autoload_by_name;
 pa_context_remove_sample;
 pa_context_rttime_new;
 pa_context_rttime_restart;
diff --git a/src/pulse/def.h b/src/pulse/def.h
index 100df5b5..91966b62 100644
--- a/src/pulse/def.h
+++ b/src/pulse/def.h
@@ -557,10 +557,7 @@ typedef enum pa_subscription_mask {
 PA_SUBSCRIPTION_MASK_SERVER = 0x0080U,
 /**< Other global server changes. */
 
-/** \cond fulldocs */
-PA_SUBSCRIPTION_MASK_AUTOLOAD = 0x0100U,
-/**< \deprecated Autoload table events. */
-/** \endcond */
+/* 0x0100U previously assigned */
 
 PA_SUBSCRIPTION_MASK_CARD = 0x0200U,
 /**< Card events. \since 0.9.15 */
@@ -595,10 +592,7 @@ typedef enum pa_subscription_event_type {
 PA_SUBSCRIPTION_EVENT_SERVER = 0x0007U,
 /**< Event type: Global server change, only occurring with PA_SUBSCRIPTION_EVENT_CHANGE. */
 
-/** \cond fulldocs */
-PA_SUBSCRIPTION_EVENT_AUTOLOAD = 0x0008U,
-/**< \deprecated Event type: Autoload table changes. */
-/** \endcond */
+/* 0x0008U previously assigned */
 
 PA_SUBSCRIPTION_EVENT_CARD = 0x0009U,
 /**< Event type: Card \since 0.9.15 */
diff --git a/src/pulse/introspect.c b/src/pulse/introspect.c
index 510d784a..310cc660 100644
--- a/src/pulse/introspect.c
+++ b/src/pulse/introspect.c
@@ -1926,63 +1926,6 @@ pa_operation* pa_context_set_port_latency_offset(pa_context *c, const char *card
 return o;
 }
 
-/*** Autoload stuff ***/
-
-PA_WARN_REFERENCE(pa_context_get_autoload_info_by_name, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_get_autoload_info_by_name(pa_context *c, const char *name, pa_autoload_type_t type, pa_autoload_info_cb_t cb, void *userdata) {
-
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
-PA_WARN_REFERENCE(pa_context_get_autoload_info_by_index, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_get_autoload_info_by_index(pa_context *c, uint32_t idx, pa_autoload_info_cb_t cb, void *userdata) {
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
-PA_WARN_REFERENCE(pa_context_get_autoload_info_list, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_get_autoload_info_list(pa_context *c, pa_autoload_info_cb_t cb, void *userdata) {
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
-PA_WARN_REFERENCE(pa_context_add_autoload, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_add_autoload(pa_context *c, const char *name, pa_autoload_type_t type, const char *module, const char*argument, pa_context_index_cb_t cb, void* userdata) {
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
-PA_WARN_REFERENCE(pa_context_remove_autoload_by_name, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_remove_autoload_by_name(pa_context *c, const char *name, pa_autoload_type_t type, pa_context_success_cb_t cb, void* userdata) {
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
-PA_WARN_REFERENCE(pa_context_remove_autoload_by_index, "Module auto-loading no longer supported.");
-
-pa_operation* pa_context_remove_autoload_by_index(pa_context *c, uint32_t idx, pa_context_success_cb_t cb, void* userdata) {
-pa_assert(c);
-pa_assert(PA_REFCNT_VALUE(c) >= 1);
-
-PA_FAIL_RETURN_NULL(c, PA_ERR_OBSOLETE);
-}
-
 pa_operation* pa_context_move_sink_input_by_name(pa_context *c, uint32_t idx, const char *sink_name, pa_context_success_cb_t cb, void* userdata) {
 pa_operation *o;
 pa_tagstruct *t;
diff --git a/src/pulse/introspect.h b/src/pulse/introspect.h
index 43389b73..0d00f33d 100644
--- a/src/pulse/introspect.h
+++ b/src/pulse/introspect.h
@@ -709,52 +709,6 @@ pa_operation* pa_context_get_sample_info_list(pa_context *c, pa_sample_info_cb_t
 

Re: [pulseaudio-discuss] [announce] Rust bindings!

[jnqnfe]

On Tue, 2018-06-12 at 11:22 +0300, Tanu Kaskinen wrote:
> On Mon, 2018-05-28 at 19:37 +0100, jnq...@gmail.com wrote:
> > Hi everyone!
> > 
> > Back in February I released 'binding' and 'sys' crates for using
> > pulseaudio from Rust code. I had intended to make an announcement
> > here
> > at the time, but I failed to do so, so I'm doing it now.
> > 
> > fyi, the 'sys' crates provide a simple description of the C
> > interface;
> > The 'binding' crates take this further, providing a cleaner/safer
> > Rust
> > interface.
> > 
> > I have provided separate crates for each PA system library (per
> > Rust
> > guidelines for 'sys' crates), thus totalling six in all:
> > 
> > [binding crates]
> >  - libpulse-binding: https://crates.io/crates/libpulse-binding
> >  - libpulse-simple-binding: https://crates.io/crates/libpulse-simpl
> > e-bi
> > nding
> >  - libpulse-mainloop-glib-binding: https://crates.io/crates/libpuls
> > e-gl
> > ib-binding
> > [sys crates]
> >  - libpulse-sys: https://crates.io/crates/libpulse-sys
> >  - libpulse-simple-sys: https://crates.io/crates/libpulse-simple-sy
> > s
> >  - libpulse-mainloop-glib-sys: https://crates.io/crates/libpulse-ma
> > inlo
> > op-glib-sys
> > 
> > The 'binding' crates include plenty of documentation (taken from
> > the C
> > API). This can be built locally of course (cargo doc), but is also
> > available online at docs.rs, example: https://docs.rs/libpulse-bind
> > ing/
> > 
> > Long term I hope that the owners of the PA project itself would
> > like to
> > take over ownership and maintenance. Even longer term hopefully we
> > will
> > see PA itself converting to Rust - fyi the PA projects has my full
> > consent to use this work of mine in converting PA itself.
> 
> Cool, thanks for the bindings! I'm afraid you'll have to keep
> maintaining the bindings yourself for the foreseeable future - I
> don't
> really want to take more work for myself at this point (I can of
> course
> only talk only for myself, but I don't expect the other maintainers
> to
> be enthusiastically adopting the bindings either). That said,
> converting PA to Rust might very well be a good idea. From what I've
> heard about combining Rust with C, such conversion could be done
> gradually.

Ok no problem :)

I am very glad to hear that you are open to a Rust conversion. I'm very
busy at the moment, but I have given a little thought to it over the
past few days; perhaps I will try to tackle it at some point.
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [patches] constification round #3

[jnqnfe]

API constification set #3

Some API functions perform validation routines which may modify the
'error' attribute of a context object. For API functions where the
"primary" object is not a context object, and the object holds a non-
const context pointer, this internal mechanism could be successfully
hidden - such functions were constified in my previous two patch sets.

However, some functions, namely those related directly to context
objects, had to be passed over previously. These could not be
constified because the validation routines accessed the context object
directly to change the error attribute.

This patch set addresses this shortcoming.

Firstly the patch set moves the context's error attribute behind a
pointer. The validation routines are then changed to store the error
value via this pointer. That then allows a collection of further API
functions to be constified.

(actually pa_context_errno and the two rttime ones could have been done
previously on second inspection)

Although the indirection of the error attribute is obviously ever so
slightly worse off for efficiency, it is surely worth the price. After
all, the error setting of the validation checks is just an artifact of
an internal mechanism and should not be allowed to influence the public
API like it currently does.From b1659b17ef72f44ab299384026027eb4f8afde4f Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Thu, 7 Jun 2018 02:43:56 +0100
Subject: context: hide error attr behind pointer

Paves the way towards more of the API using const pointers.
(See email discussion).

diff --git a/src/pulse/context.c b/src/pulse/context.c
index adbeb153..d298ae3e 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -139,6 +139,9 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *
 c = pa_xnew0(pa_context, 1);
 PA_REFCNT_INIT(c);
 
+c->error = pa_xnew0(pa_context_error, 1);
+assert(c->error);
+
 c->proplist = p ? pa_proplist_copy(p) : pa_proplist_new();
 
 if (name)
@@ -156,7 +159,7 @@ pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *
 PA_LLIST_HEAD_INIT(pa_stream, c->streams);
 PA_LLIST_HEAD_INIT(pa_operation, c->operations);
 
-c->error = PA_OK;
+c->error->error = PA_OK;
 c->state = PA_CONTEXT_UNCONNECTED;
 
 reset_callbacks(c);
@@ -315,7 +318,7 @@ int pa_context_set_error(pa_context *c, int error) {
 pa_assert(error < PA_ERR_MAX);
 
 if (c)
-c->error = error;
+c->error->error = error;
 
 return error;
 }
@@ -1072,7 +1075,7 @@ int pa_context_errno(pa_context *c) {
 
 pa_assert(PA_REFCNT_VALUE(c) >= 1);
 
-return c->error;
+return c->error->error;
 }
 
 void pa_context_set_state_callback(pa_context *c, pa_context_notify_cb_t cb, void *userdata) {
diff --git a/src/pulse/internal.h b/src/pulse/internal.h
index 01d2b6e4..04f35e9a 100644
--- a/src/pulse/internal.h
+++ b/src/pulse/internal.h
@@ -55,6 +55,10 @@
 #define PA_PROTOCOL_FLAG_SHM 0x8000U
 #define PA_PROTOCOL_FLAG_MEMFD 0x4000U
 
+typedef struct pa_context_error {
+int error;
+} pa_context_error;
+
 struct pa_context {
 PA_REFCNT_DECLARE;
 
@@ -80,7 +84,7 @@ struct pa_context {
 uint32_t version;
 uint32_t ctag;
 uint32_t csyncid;
-int error;
+pa_context_error *error;
 pa_context_state_t state;
 
 pa_context_notify_cb_t state_callback;
From 590ee9dd0c9d513872ee0150c52f895a01dd04a8 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Thu, 7 Jun 2018 02:50:54 +0100
Subject: add internal pa_context_error_set_error function

pa_context_set_error now redirects to this

Further paving the way towards more of the API using const pointers.
(See email discussion).

diff --git a/src/pulse/context.c b/src/pulse/context.c
index d298ae3e..45ff3fbb 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -313,16 +313,20 @@ void pa_context_set_state(pa_context *c, pa_context_state_t st) {
 pa_context_unref(c);
 }
 
-int pa_context_set_error(pa_context *c, int error) {
+int pa_context_error_set_error(pa_context_error *ce, int error) {
 pa_assert(error >= 0);
 pa_assert(error < PA_ERR_MAX);
 
-if (c)
-c->error->error = error;
+if (ce)
+ce->error = error;
 
 return error;
 }
 
+int pa_context_set_error(pa_context *c, int error) {
+return pa_context_error_set_error(c->error, error);
+}
+
 void pa_context_fail(pa_context *c, int error) {
 pa_assert(c);
 pa_assert(PA_REFCNT_VALUE(c) >= 1);
diff --git a/src/pulse/internal.h b/src/pulse/internal.h
index 04f35e9a..725b7b46 100644
--- a/src/pulse/internal.h
+++ b/src/pulse/internal.h
@@ -281,6 +281,8 @@ pa_operation* pa_context_send_simple_command(pa_context *c, uint32_t command, vo
 
 void pa_stream_set_state(pa_stream *s, pa_stream_state_t st);
 
+int pa_context_error_set_error(pa_context_error *ce, int error);
+
 pa_tagstruct *pa_tagstruct_command(pa_context *c, uint32_t command, uint32_t *tag);
 
 #define 

[pulseaudio-discuss] [patch] const stream internal

[jnqnfe]

quick patch to constify a couple of internal stream functionsFrom fdc7b0ebd82a98348d35ac1d76c937e021985a21 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Thu, 7 Jun 2018 03:15:41 +0100
Subject: stream: constify internal functions


diff --git a/src/pulse/stream.c b/src/pulse/stream.c
index 73743cbb..f0c8034a 100644
--- a/src/pulse/stream.c
+++ b/src/pulse/stream.c
@@ -1731,7 +1731,7 @@ pa_operation * pa_stream_drain(pa_stream *s, pa_stream_success_cb_t cb, void *us
 return o;
 }
 
-static pa_usec_t calc_time(pa_stream *s, bool ignore_transport) {
+static pa_usec_t calc_time(const pa_stream *s, bool ignore_transport) {
 pa_usec_t usec;
 
 pa_assert(s);
@@ -2485,7 +2485,7 @@ int pa_stream_get_time(pa_stream *s, pa_usec_t *r_usec) {
 return 0;
 }
 
-static pa_usec_t time_counter_diff(pa_stream *s, pa_usec_t a, pa_usec_t b, int *negative) {
+static pa_usec_t time_counter_diff(const pa_stream *s, pa_usec_t a, pa_usec_t b, int *negative) {
 pa_assert(s);
 pa_assert(PA_REFCNT_VALUE(s) >= 1);
 
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [patches] constification 1/2

[jnqnfe]

On Mon, 2018-06-04 at 13:28 +0300, Tanu Kaskinen wrote:
> On Mon, 2018-05-28 at 01:49 +0100, jnq...@gmail.com wrote:
> > Constification patch set ** 1 of 2 **
> > 
> > Collection of 16 patches constifying pointers in various parts of
> > the
> > API.
> > 
> > This collection of patches has interdependencies, they must be
> > applied
> > in (roughly) the given order.
> > 
> > These start off with constifying some core hashmap functions, which
> > then allows various proplist related functions to be changed. A
> > couple
> > of tagstruct functions are in there, and finally a couple of
> > context+proplist related functions.
> > 
> > I have not been in a position to try and compile these changes. I
> > have
> > identified one possible problem - the hashmap.c BY_HASH macro - I'm
> > not
> > certain offhand if a const version will be required or if the
> > compiler
> > will be happy casting as is. Otherwise I'm fairly certain there are
> > no
> > (obvious) issues.
> 
> Thanks! I pushed these to the "next" branch. The only issue was in
> the
> last patch - the function reused the constified variable when
> creating
> a new proplist, and the compiler didn't like when that temporary
> proplist was freed. I took the liberty of amending your patch with a
> fix.

No problem, thanks :)
I noticed set 2/2 isn't there...
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] timer events

[jnqnfe]

Nevermind, I solved it myself - I failed to realise that I needed to
add the result of pa_rtclock_now() to the time values I was specifying
:/
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [patches] constification 2/2

[jnqnfe]

Constification patch set ** 2 of 2 **

Collection of 15 patches constifying pointers in various parts of the
API.

This collection of patches have no interdependencies, they can be
applied in any order. They do *not* depend upon the 'constification
1/2' set, they can be applied independently.

fyi, I have not had an opportunity to compile them, but I don't see any
obvious issue.From 1c86d891062e22494f998bfd39987ef505dfb5e4 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Sun, 27 May 2018 07:56:11 +0100
Subject: volume: pa_cvolume_get_position: constify


diff --git a/src/pulse/volume.c b/src/pulse/volume.c
index ead54152..fc6ac8d2 100644
--- a/src/pulse/volume.c
+++ b/src/pulse/volume.c
@@ -911,7 +911,7 @@ pa_cvolume* pa_cvolume_set_position(
 }
 
 pa_volume_t pa_cvolume_get_position(
-pa_cvolume *cv,
+const pa_cvolume *cv,
 const pa_channel_map *map,
 pa_channel_position_t t) {
 
diff --git a/src/pulse/volume.h b/src/pulse/volume.h
index 2503c3f6..fe44b0bb 100644
--- a/src/pulse/volume.h
+++ b/src/pulse/volume.h
@@ -416,7 +416,7 @@ pa_cvolume* pa_cvolume_set_position(pa_cvolume *cv, const pa_channel_map *map, p
  * position. Will return 0 if there is no channel at the position
  * specified. You can check if a channel map includes a specific
  * position by calling pa_channel_map_has_position(). \since 0.9.16 */
-pa_volume_t pa_cvolume_get_position(pa_cvolume *cv, const pa_channel_map *map, pa_channel_position_t t) PA_GCC_PURE;
+pa_volume_t pa_cvolume_get_position(const pa_cvolume *cv, const pa_channel_map *map, pa_channel_position_t t) PA_GCC_PURE;
 
 /** This goes through all channels in a and b and sets the
  * corresponding channel in dest to the greater volume of both. a, b
From 651e090f8461eec114158aba805be791b57a1561 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Sat, 26 May 2018 23:50:55 +0100
Subject: context: pa_context_new_with_proplist: constify proplist param


diff --git a/src/pulse/context.c b/src/pulse/context.c
index e7695009..8722f350 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -125,7 +125,7 @@ static void reset_callbacks(pa_context *c) {
 c->ext_stream_restore.userdata = NULL;
 }
 
-pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *name, pa_proplist *p) {
+pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *name, const pa_proplist *p) {
 pa_context *c;
 pa_mem_type_t type;
 
diff --git a/src/pulse/context.h b/src/pulse/context.h
index 2cb0776e..9d4c926e 100644
--- a/src/pulse/context.h
+++ b/src/pulse/context.h
@@ -174,7 +174,7 @@ pa_context *pa_context_new(pa_mainloop_api *mainloop, const char *name);
 /** Instantiate a new connection context with an abstract mainloop API
  * and an application name, and specify the initial client property
  * list. \since 0.9.11 */
-pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *name, pa_proplist *proplist);
+pa_context *pa_context_new_with_proplist(pa_mainloop_api *mainloop, const char *name, const pa_proplist *proplist);
 
 /** Decrease the reference counter of the context by one */
 void pa_context_unref(pa_context *c);
From 40cab01edae1ebed3490a39cd3141070d016b8b8 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Mon, 28 May 2018 01:12:39 +0100
Subject: context: pa_context_get_protocol_version: constify


diff --git a/src/pulse/context.c b/src/pulse/context.c
index b7556343..adbeb153 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -1331,7 +1331,7 @@ const char* pa_context_get_server(pa_context *c) {
 return c->server;
 }
 
-uint32_t pa_context_get_protocol_version(pa_context *c) {
+uint32_t pa_context_get_protocol_version(const pa_context *c) {
 return PA_PROTOCOL_VERSION;
 }
 
diff --git a/src/pulse/context.h b/src/pulse/context.h
index dad7aa50..7a38ff23 100644
--- a/src/pulse/context.h
+++ b/src/pulse/context.h
@@ -236,7 +236,7 @@ pa_operation* pa_context_set_name(pa_context *c, const char *name, pa_context_su
 const char* pa_context_get_server(pa_context *c);
 
 /** Return the protocol version of the library. */
-uint32_t pa_context_get_protocol_version(pa_context *c);
+uint32_t pa_context_get_protocol_version(const pa_context *c);
 
 /** Return the protocol version of the connected server.
  * Returns PA_INVALID_INDEX on error. */
From 558bafe46d1079be68449a4116c25749031892f5 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Sun, 27 May 2018 06:24:17 +0100
Subject: context: pa_context_get_state: constify


diff --git a/src/pulse/context.c b/src/pulse/context.c
index 8722f350..b7556343 100644
--- a/src/pulse/context.c
+++ b/src/pulse/context.c
@@ -1058,7 +1058,7 @@ void pa_context_disconnect(pa_context *c) {
 pa_context_set_state(c, PA_CONTEXT_TERMINATED);
 }
 
-pa_context_state_t pa_context_get_state(pa_context *c) {
+pa_context_state_t pa_context_get_state(const pa_context *c) {
 pa_assert(c);
 pa_assert(PA_REFCNT_VALUE(c) >= 1);
 
diff --git 

[pulseaudio-discuss] [patches] constification 1/2

[jnqnfe]

Constification patch set ** 1 of 2 **

Collection of 16 patches constifying pointers in various parts of the
API.

This collection of patches has interdependencies, they must be applied
in (roughly) the given order.

These start off with constifying some core hashmap functions, which
then allows various proplist related functions to be changed. A couple
of tagstruct functions are in there, and finally a couple of
context+proplist related functions.

I have not been in a position to try and compile these changes. I have
identified one possible problem - the hashmap.c BY_HASH macro - I'm not
certain offhand if a const version will be required or if the compiler
will be happy casting as is. Otherwise I'm fairly certain there are no
(obvious) issues.From ae549e3b6a02673cb128fb8a07c6d8f74c2b79ee Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Sun, 27 May 2018 02:27:43 +0100
Subject: core/hashmap: constify hashmap ptr for various functions


diff --git a/src/pulsecore/hashmap.c b/src/pulsecore/hashmap.c
index 2385c55c..6477783e 100644
--- a/src/pulsecore/hashmap.c
+++ b/src/pulsecore/hashmap.c
@@ -231,7 +231,7 @@ void pa_hashmap_remove_all(pa_hashmap *h) {
 }
 }
 
-void *pa_hashmap_iterate(pa_hashmap *h, void **state, const void **key) {
+void *pa_hashmap_iterate(const pa_hashmap *h, void **state, const void **key) {
 struct hashmap_entry *e;
 
 pa_assert(h);
@@ -264,7 +264,7 @@ at_end:
 return NULL;
 }
 
-void *pa_hashmap_iterate_backwards(pa_hashmap *h, void **state, const void **key) {
+void *pa_hashmap_iterate_backwards(const pa_hashmap *h, void **state, const void **key) {
 struct hashmap_entry *e;
 
 pa_assert(h);
@@ -297,7 +297,7 @@ at_beginning:
 return NULL;
 }
 
-void* pa_hashmap_first(pa_hashmap *h) {
+void* pa_hashmap_first(const pa_hashmap *h) {
 pa_assert(h);
 
 if (!h->iterate_list_head)
@@ -306,7 +306,7 @@ void* pa_hashmap_first(pa_hashmap *h) {
 return h->iterate_list_head->value;
 }
 
-void* pa_hashmap_last(pa_hashmap *h) {
+void* pa_hashmap_last(const pa_hashmap *h) {
 pa_assert(h);
 
 if (!h->iterate_list_tail)
@@ -329,13 +329,13 @@ void* pa_hashmap_steal_first(pa_hashmap *h) {
 return data;
 }
 
-unsigned pa_hashmap_size(pa_hashmap *h) {
+unsigned pa_hashmap_size(const pa_hashmap *h) {
 pa_assert(h);
 
 return h->n_entries;
 }
 
-bool pa_hashmap_isempty(pa_hashmap *h) {
+bool pa_hashmap_isempty(const pa_hashmap *h) {
 pa_assert(h);
 
 return h->n_entries == 0;
diff --git a/src/pulsecore/hashmap.h b/src/pulsecore/hashmap.h
index b1027e75..ea883765 100644
--- a/src/pulsecore/hashmap.h
+++ b/src/pulsecore/hashmap.h
@@ -61,10 +61,10 @@ int pa_hashmap_remove_and_free(pa_hashmap *h, const void *key);
 void pa_hashmap_remove_all(pa_hashmap *h);
 
 /* Return the current number of entries of the hashmap */
-unsigned pa_hashmap_size(pa_hashmap *h);
+unsigned pa_hashmap_size(const pa_hashmap *h);
 
 /* Return true if the hashmap is empty */
-bool pa_hashmap_isempty(pa_hashmap *h);
+bool pa_hashmap_isempty(const pa_hashmap *h);
 
 /* May be used to iterate through the hashmap. Initially the opaque
pointer *state has to be set to NULL. The hashmap may not be
@@ -72,19 +72,19 @@ bool pa_hashmap_isempty(pa_hashmap *h);
via pa_hashmap_remove(). The key of the entry is returned in *key,
if key is non-NULL. After the last entry in the hashmap NULL is
returned. */
-void *pa_hashmap_iterate(pa_hashmap *h, void **state, const void**key);
+void *pa_hashmap_iterate(const pa_hashmap *h, void **state, const void**key);
 
 /* Same as pa_hashmap_iterate() but goes backwards */
-void *pa_hashmap_iterate_backwards(pa_hashmap *h, void **state, const void**key);
+void *pa_hashmap_iterate_backwards(const pa_hashmap *h, void **state, const void**key);
 
 /* Remove the oldest entry in the hashmap and return it */
 void *pa_hashmap_steal_first(pa_hashmap *h);
 
 /* Return the oldest entry in the hashmap */
-void* pa_hashmap_first(pa_hashmap *h);
+void* pa_hashmap_first(const pa_hashmap *h);
 
 /* Return the newest entry in the hashmap */
-void* pa_hashmap_last(pa_hashmap *h);
+void* pa_hashmap_last(const pa_hashmap *h);
 
 /* A macro to ease iteration through all entries */
 #define PA_HASHMAP_FOREACH(e, h, state) \
From b87af9de83f95eb2271112b36ebc32da44c64d12 Mon Sep 17 00:00:00 2001
From: Lyndon Brown 
Date: Sun, 27 May 2018 02:56:11 +0100
Subject: core/hashmap: constify pointer of private hash_scan function

paves the way for doing the same for pa_hashmap_get and users of it

diff --git a/src/pulsecore/hashmap.c b/src/pulsecore/hashmap.c
index 6477783e..c03af3bc 100644
--- a/src/pulsecore/hashmap.c
+++ b/src/pulsecore/hashmap.c
@@ -119,7 +119,7 @@ void pa_hashmap_free(pa_hashmap *h) {
 pa_xfree(h);
 }
 
-static struct hashmap_entry *hash_scan(pa_hashmap *h, unsigned hash, const void *key) {
+static struct hashmap_entry *hash_scan(const pa_hashmap *h, unsigned hash, const void *key) {
 struct hashmap_entry *e;
 pa_assert(h);

[pulseaudio-discuss] timer events

[jnqnfe]

tldr: timer events - am I doing something wrong, or is this feature
broken?

When putting together the Rust bindings a few months ago (see separate
announcement email), I created a few small test programs to test a
handful of features. One of the last features I tried to play with was
timer events. These events though seem to always fire immediately,
which puzzles me.

Yesterday I recreated the primary test program in C, to rule out any
Rust binding related issues, and it exhibits the same problem.

So question, am I doing something wrong (quite likely), or is this
feature broken?

I have attached a copy of both the Rust crate test program, and the C
version.

The test program is very simple, but deserves a word on usage. It
requires one cmd-line arg, a file to "play". Whatever file you point it
at it pretends contains raw audio, which it serves to PA one second's
worth at a time. What I did was to use VLC to capture the raw audio of
a video to a file, which I point it at.#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

void context_state_change_cb(pa_context *context, void *data);
void stream_state_change_cb(pa_stream *stream, void *data);
void drain_cb(pa_stream *stream, int i, void *data);

void timer_event_cb(pa_mainloop_api*a, pa_time_event* e, const struct timeval *tv, void *userdata);
void timer_event_cb_2(pa_mainloop_api*a, pa_time_event* e, const struct timeval *tv, void *userdata);
void timer_event_cb_3(pa_mainloop_api*a, pa_time_event* e, const struct timeval *tv, void *userdata);

void main(int argc, char *argv[]) {
if (argc < 2) {
printf("Error: too few arguments!\n");
printf("Usage: %s FILENAME\n", argv[0]);
return;
}

char *filename = argv[1];

printf("opening: %s\n", filename);
int fd = open(filename, O_RDONLY);
if (!fd) {
printf("Error: failed to open file!\n");
return;
}

pa_sample_spec spec = { PA_SAMPLE_S16NE, 44100, 2 };
assert(pa_sample_spec_valid());

printf("bytes per second: %i\n", pa_bytes_per_second());
printf("frame size: %i\n", pa_frame_size());
printf("sample size: %i\n", pa_sample_size());

#define BYTES_PER_SEC 176400
#define BUFFER_SIZE BYTES_PER_SEC
char *buffer = malloc(BUFFER_SIZE);
if (!buffer) {
printf("Error: failed to alloc buffer!\n");
return;
}
assert(BUFFER_SIZE % pa_frame_size() == 0);

pa_threaded_mainloop *mainloop = pa_threaded_mainloop_new();
if (!mainloop) {
printf("Error: failed to create mainloop!\n");
return;
}

static char * APP_NAME = "FooAppContext";
pa_context *context = pa_context_new(pa_threaded_mainloop_get_api(mainloop), APP_NAME);

pa_context_set_state_callback(context, context_state_change_cb, (void*) mainloop);

if (pa_context_connect(context, NULL, 0, NULL) < 0) {
printf("Error: context connection failed!\n");
pa_context_set_state_callback(context, NULL, NULL);
pa_context_unref(context);
pa_threaded_mainloop_free(mainloop);
return;
}

pa_threaded_mainloop_lock(mainloop);
if (pa_threaded_mainloop_start(mainloop) < 0) {
printf("Error: failed to start mainloop!\n");
pa_threaded_mainloop_unlock(mainloop);
pa_context_disconnect(context);
pa_context_set_state_callback(context, NULL, NULL);
pa_context_unref(context);
pa_threaded_mainloop_free(mainloop);
return;
}

// Wait for context to be ready
printf("waiting for context state change...\n");
for(;;) {
bool ready = false;
switch (pa_context_get_state(context)) {
case PA_CONTEXT_READY:
ready = true;
break;
case PA_CONTEXT_FAILED:
case PA_CONTEXT_TERMINATED:
printf("context state failed/terminated, quitting...\n");
pa_threaded_mainloop_unlock(mainloop);
pa_threaded_mainloop_stop(mainloop);
return;
default:
pa_threaded_mainloop_wait(mainloop);
}
if (ready)
break;
}

pa_buffer_attr ba = {
(unsigned int) -1,
(unsigned int) -1,
(unsigned int) -1,
(unsigned int) -1,
(unsigned int) -1,
};

printf("creating stream...\n");
pa_stream *stream = pa_stream_new(context, "Music", , NULL);
if (!stream) {
printf("Error: failed to create stream!\n");
pa_threaded_mainloop_unlock(mainloop);
pa_threaded_mainloop_stop(mainloop);
pa_context_disconnect(context);
pa_context_set_state_callback(context, NULL, NULL);
pa_context_unref(context);
pa_threaded_mainloop_free(mainloop);
return;
}

pa_stream_set_state_callback(stream, stream_state_change_cb, 

[pulseaudio-discuss] [announce] Rust bindings!

[jnqnfe]

Hi everyone!

Back in February I released 'binding' and 'sys' crates for using
pulseaudio from Rust code. I had intended to make an announcement here
at the time, but I failed to do so, so I'm doing it now.

fyi, the 'sys' crates provide a simple description of the C interface;
The 'binding' crates take this further, providing a cleaner/safer Rust
interface.

I have provided separate crates for each PA system library (per Rust
guidelines for 'sys' crates), thus totalling six in all:

[binding crates]
 - libpulse-binding: https://crates.io/crates/libpulse-binding
 - libpulse-simple-binding: https://crates.io/crates/libpulse-simple-bi
nding
 - libpulse-mainloop-glib-binding: https://crates.io/crates/libpulse-gl
ib-binding
[sys crates]
 - libpulse-sys: https://crates.io/crates/libpulse-sys
 - libpulse-simple-sys: https://crates.io/crates/libpulse-simple-sys
 - libpulse-mainloop-glib-sys: https://crates.io/crates/libpulse-mainlo
op-glib-sys

The 'binding' crates include plenty of documentation (taken from the C
API). This can be built locally of course (cargo doc), but is also
available online at docs.rs, example: https://docs.rs/libpulse-binding/

Long term I hope that the owners of the PA project itself would like to
take over ownership and maintenance. Even longer term hopefully we will
see PA itself converting to Rust - fyi the PA projects has my full
consent to use this work of mine in converting PA itself.

fyi, I am not subscribed to this list; I am contactable at this email
address though, and bug-reports/questions/etc specific to the bindings
can be made on github (https://github.com/jnqnfe/pulse-binding-rust),
but understand that I am no PA guru :)
I do not currently have a home internet connection with my current
living situation, and manage to get online typically only once a week
at the moment, so bare in mind that it could take a while to get a
response. Appoligies in advance if I keep you waiting, I know it's not
ideal.

News related, I have two new version, v1.0.5 and v1.1.0 ready to
publish (when I next get an internet connection, at which point this
email will also get sent out of my outbox :p). The next thing planned
is to consider whether use of "Futures" would be a good idea for use in
the bindings to tidy up use of callbacks - there are some details, most
specifically callbacks, where underlying C details are exposed.

Regards,
Lyndon Brown (aka jnqnfe)
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] some patches

[jnqnfe]

some patches for master attached

1) Constify a param in pa_cvolume_scale_mask
2) A small efficiency tweak to volume handling
3) A big collection of API documentation improvements
Includes typo fixes; neatening; addition of more return info; and such
4) A patch merging and deduplicating some pa_buffer_attr documentationFrom 60ce6302d2df32a3fe756ebb4e142ce09b044000 Mon Sep 17 00:00:00 2001
From: jnqnfe <jnq...@gmail.com>
Date: Thu, 31 Aug 2017 18:32:47 +0100
Subject: pa_cvolume_scale_mask: constify param


diff --git a/src/pulse/volume.c b/src/pulse/volume.c
index ffd42ecb..6b0c4b68 100644
--- a/src/pulse/volume.c
+++ b/src/pulse/volume.c
@@ -789,7 +789,7 @@ pa_cvolume* pa_cvolume_scale(pa_cvolume *v, pa_volume_t max) {
 return v;
 }
 
-pa_cvolume* pa_cvolume_scale_mask(pa_cvolume *v, pa_volume_t max, pa_channel_map *cm, pa_channel_position_mask_t mask) {
+pa_cvolume* pa_cvolume_scale_mask(pa_cvolume *v, pa_volume_t max, const pa_channel_map *cm, pa_channel_position_mask_t mask) {
 unsigned c;
 pa_volume_t t = 0;
 
diff --git a/src/pulse/volume.h b/src/pulse/volume.h
index 03497f74..08db7cfe 100644
--- a/src/pulse/volume.h
+++ b/src/pulse/volume.h
@@ -396,7 +396,7 @@ pa_cvolume* pa_cvolume_scale(pa_cvolume *v, pa_volume_t max);
  * of all channels selected via cm/mask equals max. This also modifies
  * the volume of those channels that are unmasked. The proportions
  * between the channel volumes are kept. \since 0.9.16 */
-pa_cvolume* pa_cvolume_scale_mask(pa_cvolume *v, pa_volume_t max, pa_channel_map *cm, pa_channel_position_mask_t mask);
+pa_cvolume* pa_cvolume_scale_mask(pa_cvolume *v, pa_volume_t max, const pa_channel_map *cm, pa_channel_position_mask_t mask);
 
 /** Set the passed volume to all channels at the specified channel
  * position. Will return the updated volume struct, or NULL if there
From e7c89c7b78d6fcc003ec5e9fa8fad77d7b920b1f Mon Sep 17 00:00:00 2001
From: jnqnfe <jnq...@gmail.com>
Date: Mon, 4 Sep 2017 02:32:37 +0100
Subject: small volume tweak


diff --git a/src/pulse/volume.c b/src/pulse/volume.c
index 6b0c4b68..ead54152 100644
--- a/src/pulse/volume.c
+++ b/src/pulse/volume.c
@@ -477,10 +477,10 @@ pa_cvolume *pa_sw_cvolume_multiply(pa_cvolume *dest, const pa_cvolume *a, const
 pa_return_val_if_fail(pa_cvolume_valid(a), NULL);
 pa_return_val_if_fail(pa_cvolume_valid(b), NULL);
 
-for (i = 0; i < a->channels && i < b->channels; i++)
-dest->values[i] = pa_sw_volume_multiply(a->values[i], b->values[i]);
+dest->channels = PA_MIN(a->channels, b->channels);
 
-dest->channels = (uint8_t) i;
+for (i = 0; i < dest->channels; i++)
+dest->values[i] = pa_sw_volume_multiply(a->values[i], b->values[i]);
 
 return dest;
 }
@@ -512,10 +512,10 @@ pa_cvolume *pa_sw_cvolume_divide(pa_cvolume *dest, const pa_cvolume *a, const pa
 pa_return_val_if_fail(pa_cvolume_valid(a), NULL);
 pa_return_val_if_fail(pa_cvolume_valid(b), NULL);
 
-for (i = 0; i < a->channels && i < b->channels; i++)
-dest->values[i] = pa_sw_volume_divide(a->values[i], b->values[i]);
+dest->channels = PA_MIN(a->channels, b->channels);
 
-dest->channels = (uint8_t) i;
+for (i = 0; i < dest->channels; i++)
+dest->values[i] = pa_sw_volume_divide(a->values[i], b->values[i]);
 
 return dest;
 }
@@ -942,10 +942,10 @@ pa_cvolume* pa_cvolume_merge(pa_cvolume *dest, const pa_cvolume *a, const pa_cvo
 pa_return_val_if_fail(pa_cvolume_valid(a), NULL);
 pa_return_val_if_fail(pa_cvolume_valid(b), NULL);
 
-for (i = 0; i < a->channels && i < b->channels; i++)
-dest->values[i] = PA_MAX(a->values[i], b->values[i]);
+dest->channels = PA_MIN(a->channels, b->channels);
 
-dest->channels = (uint8_t) i;
+for (i = 0; i < dest->channels; i++)
+dest->values[i] = PA_MAX(a->values[i], b->values[i]);
 
 return dest;
 }
From 7212f99292e3c8f1167e814315528089a56033ae Mon Sep 17 00:00:00 2001
From: jnqnfe <jnq...@gmail.com>
Date: Mon, 4 Sep 2017 20:19:48 +0100
Subject: api documentation improvements

includes typo fixes, neatening, addition of more return info, and such.

diff --git a/src/pulse/channelmap.h b/src/pulse/channelmap.h
index 6eabe20b..290e1ed2 100644
--- a/src/pulse/channelmap.h
+++ b/src/pulse/channelmap.h
@@ -46,10 +46,12 @@
  *
  * \li pa_channel_map_init_mono() - Create a channel map with only mono audio.
  * \li pa_channel_map_init_stereo() - Create a standard stereo mapping.
- * \li pa_channel_map_init_auto() - Create a standard channel map for a specific number of channels
- * \li pa_channel_map_init_extend() - Similar to
- * pa_channel_map_init_auto() but synthesize a channel map if no
- * predefined one is known for the specified number of channels.
+ * \li pa_channel_map_init