| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This is untested.
Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/55>
|
|
Since commit ad447d14682 (in 2009) pa_read and pa_write take care of
handling EINTR error.
So, pa_read, pa_write, pa_iochannel_read and pa_iochannel_write can not
exit with errno set to EINTR, and testing it is useless.
|
|
This should make it easier for clients to elevate their audio threads to
real time priority without having to dig through much through specific
system internals.
|
|
Source and sink are passed in arguments to set_state_in_io_thread()
callbacks. There is optimal to access them directly.
|
|
The suspend cause isn't yet used by any of the callbacks. The alsa sink
and source will use it to sync the mixer when the SESSION suspend cause
is removed. Currently the syncing is done in pa_sink/source_suspend(),
and I want to change that, because pa_sink/source_suspend() shouldn't
have any alsa specific code.
|
|
There are no behaviour changes, the code from almost all the SET_STATE
handlers is moved with minimal changes to the newly introduced
set_state_in_io_thread() callback. The only exception is module-tunnel,
which has to call pa_sink_render() after pa_sink.thread_info.state has
been updated. The set_state_in_io_thread() callback is called before
updating that variable, so moving the SET_STATE handler code to the
callback isn't possible.
The purpose of this change is to make it easier to get state change
handling right in modules. Hooking to the SET_STATE messages in modules
required care in calling pa_sink/source_process_msg() at the right time
(or not calling it at all, as was the case on resume failures), and
there were a few bugs (fixed before this patch). Now the core takes care
of ordering things correctly.
Another motivation for this change is that there was some talk about
adding a suspend_cause variable to pa_sink/source.thread_info. The
variable would be updated in the core SET_STATE handler, but that would
not work with the old design, because in case of resume failures modules
didn't call the core message handler.
|
|
Suspending never fails.
|
|
This removes the symdef header generation m4 magic in favour of a
simpler macro method, allowing us to skip one unnecessary build step
while moving to meson, and removing an 11 year old todo!
|
|
|
|
negative values
The reported latency of source or sink is based on measured initial conditions.
If the conditions contain an error, the estimated latency values may become negative.
This does not indicate that the latency is indeed negative but can be considered
merely an offset error. The current get_latency_in_thread() calls and the
implementations of the PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY messages truncate negative
latencies because they do not make sense from a physical point of view. In fact,
the values are truncated twice, once in the message handler and a second time in
the pa_{source,sink}_get_latency_within_thread() call itself.
This leads to two problems for the latency controller within module-loopback:
- Truncating leads to discontinuities in the latency reports which then trigger
unwanted end to end latency corrections.
- If a large negative port latency offsets is set, the reported latency is always 0,
making it impossible to control the end to end latency at all.
This patch is a pre-condition for solving these problems.
It adds a new flag to pa_{sink,source}_get_latency_within_thread() to allow
negative return values. Truncating is also removed in all implementations of the
PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY message handlers. The allow_negative flag
is set to false for all calls of pa_{sink,source}_get_latency_within_thread()
except when used within PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY. This means that the
original behavior is not altered in most cases. Only if a positive latency offset
is set and the message returns a negative value, the reported latency is smaller
because the values are not truncated twice.
Additionally let PA_SOURCE_MESSAGE_GET_LATENCY return -pa_sink_get_latency_within_thread()
for monitor sources because the source gets the data before it is played.
|
|
Bug 96741 shows a case where an assertion is hit, because
pa_asyncq_new() failed due to running out of file descriptors.
pa_asyncq_new() is used in only one place (not counting the call in
asyncq-test): pa_asyncmsgq_new(). Now pa_asyncmsgq_new() can fail too,
which requires error handling in many places. One of those places is
pa_thread_mq_init(), which can now fail too, and that needs additional
error handling in many more places. Luckily there weren't any places
where adding better error handling wouldn't have been easy, so there are
many changes in this patch, but they are not complicated.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=96741
|
|
Patch upstreamed from pkgsrc by Kamil Rytarowski <n54@gmx.com>.
See commit e4a7625ba884c5cce20468d75937857343751c35 for why this was
originally done.
|
|
FSF addresses used in PA sources are no longer valid and rpmlint
generates numerous warnings during packaging because of this.
This patch changes all FSF addresses to FSF web page according to
the GPL how-to: https://www.gnu.org/licenses/gpl-howto.en.html
Done automatically by sed-ing through sources.
|
|
Commit fa092af59cf64902a5caa99 removed an argument to pa_rtpoll_run, but
forgot to remove that argument for all callers to pa_rtpoll_run.
This commit removes the remaining ones.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
|
|
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
|
|
|
|
Forcing all mute changes to go through set_mute() makes it easier to
check where the muted field is changed, and it also allows us to have
only one place where notifications for changed mute are sent.
|
|
commands used for this (executed from the pulseaudio/src directory):
find . -regex '\(.*\.[hc]\|.*\.cc\|.*\.m4\)' -not -name 'macro.h' \
-a -not -name 'reserve.[ch]' -a -not -name 'reserve-monitor.[ch]' \
-a -not -name 'glib-mainloop.c' -a -not -name 'gkt-test.c' \
-a -not -name 'glib-mainloop.c' -a -not -name 'gkt-test.c' \
-a -not -name 'poll-win32.c' -a -not -name 'thread-win32.c' \
-a -not -name 'dllmain.c' -a -not -name 'gconf-helper.c' \
-exec sed -i -e 's/\bpa_bool_t\b/bool/g' \
-e 's/\bTRUE\b/true/g' -e 's/\bFALSE\b/false/g' {} \;
and:
sed -i -e '181,194!s/\bpa_bool_t\b/bool/' \
-e '181,194!s/\bTRUE\b/true/' -e \
'181,194!s/\bFALSE\b/false/' pulsecore/macro.h
|
|
Patch by Brian Cameron <brian.cameron@oracle.com>.
|
|
Based on a patch by Brian Cameron <brian.cameron@oracle.com>.
|
|
Since some devices can be chatty with regards to how often they return
from poll(), this adds a PA_UNLIKELY() to all the the rewind_requested
checks in our sink modules to make the general case (no rewind was
requested) the fast path.
|
|
When a rewind is requested on a sink input, the request parameters are
stored in the pa_sink_input struct. The parameters are reset during
rewind processing, and if the sink decides to ignore the rewind
request due to being suspended, stale parameters are left in
pa_sink_input. It's particularly problematic if the rewrite_bytes
parameter is left at -1, because that will prevent all future rewind
processing on that sink input. So, in order to avoid stale parameters,
every rewind request needs to be processed, even if the sink is
suspended.
Reported-by: Uoti Urpala
|
|
This got missed when other bits were updated. Patch submitted by
Brian Cameron <brian.cameron@oracle.com>.
|
|
Some sink flags are really just a product of what callbacks
are set on the device. We still enforce a degree of sanity
that the flags match the callbacks set, but we also set the
flags automatically in our callback setter functions to
help ensure that a) people use them and b) flags & callbacks
are kept in sync.
|
|
This is not currently useful but future commits will make further
changes concerning automatic setting of flags and event delivery
that makes this structure necessary.
|
|
|
|
Only whitespace changes in here
|
|
|
|
|
|
|
|
Prevent partially played memchunks from getting lost.
If the sink has a memblock, don't leak it when rewinding.
|
|
Set a fixed latency based on the given buffer size, which is constrained to
the 128 KB limit on buffered writes. Also fix an error path.
|
|
Make use of the smoother, just in case.
|
|
Fix bit rot due to recent flat volume changes.
|
|
I hope this is enough, since the removal of the other pa_rtpoll_install()
calls deemed Linux good enough, but said nothing about Solaris, but since
the function is gone there is little more to do.
|
|
|
|
|
|
|
|
Lennart wrote,
>
> Hmm, yes. As it seems I broke the build for non-dbus builds.
Well, you also broke the solaris module between 0.9.15-test8 and 0.9.15.
Have you considered release candidates?
Patch follows. It would be nice if API changes could be made without
breaking things when the effort to avoid that is trivial.
Finn
|
|
On Wed, 4 Mar 2009, Lennart Poettering wrote:
[snip]
> > This patch disables link map/library versioning unless ld is GNU ld.
> > Another approach for solaris would be to use that linker's -M option,
> > but I couldn't make that work (due to undefined mainloop, browse and
> > simple symbols when linking pacat. I can post the errors if anyone is
> > intested.)
>
> The linking in PA is a bit weird since we have a cyclic dependency
> between libpulse and libpulsecommon which however is not explicit.
Could that affect the pacat link somehow?
What are the implications for client apps that link with the non-versioned
libraries I've been building on solaris?
[snip]
> > struct userdata {
> > pa_core *core;
> > @@ -87,15 +92,24 @@ struct userdata {
> >
> > pa_memchunk memchunk;
> >
> > - unsigned int page_size;
> > -
> > uint32_t frame_size;
> > - uint32_t buffer_size;
> > - unsigned int written_bytes, read_bytes;
> > + int32_t buffer_size;
> > + volatile uint64_t written_bytes, read_bytes;
> > + pa_mutex *written_bytes_lock;
>
> Hmm, we generally try do do things without locking in PA. This smells as
> if it was solvable using atomic ints as well.
>
> Actually, looking at this again I get the impression these mutex are
> completely unnecessary here. All functions that lock these mutexes are
> called from the IO thread so no locking should be nessary.
>
> Please don't use volatile here. I am pretty sure it is a misuse. Also
> see http://kernel.org/doc/Documentation/volatile-considered-harmful.txt
> which applies here too I think.
OK, I've removed the locks. For some reason I thought that the get_latency
function was called from two different threads.
> > +static void sink_set_volume(pa_sink *s) {
> > + struct userdata *u;
> > + audio_info_t info;
> > +
> > + pa_assert_se(u = s->userdata);
> > +
> > + if (u->fd >= 0) {
> > + AUDIO_INITINFO(&info);
> > +
> > + info.play.gain = pa_cvolume_avg(&s->virtual_volume) * AUDIO_MAX_GAIN / PA_VOLUME_NORM;
> > + assert(info.play.gain <= AUDIO_MAX_GAIN);
>
> I'd prefer if you'd use pa_cvolume_max here instead of pa_cvolume_avg()
> because this makes the volume independant of the balance.
>
> > - info.play.error = 0;
> > + info.play.gain = pa_cvolume_avg(&s->virtual_volume) * AUDIO_MAX_GAIN / PA_VOLUME_NORM;
> > + assert(info.play.gain <= AUDIO_MAX_GAIN);
>
> Same here. (i.e. for the source)
Done and done.
> > + if (u->sink->thread_info.rewind_requested)
> > + pa_sink_process_rewind(u->sink, 0);
>
> This is correct.
>
> >
> > err = ioctl(u->fd, AUDIO_GETINFO, &info);
> > pa_assert(err >= 0);
>
> Hmm, if at all this should be pa_assert_se(), not pa_assert() (so that
> it is not defined away by -DNDEBUG). However I'd prefer if the error
> would be could correctly. (I see that this code is not yours, but
> still...)
Done.
> > + case EINTR:
> > + break;
>
> I think you should simply try again in this case...
Done.
> > + case EAGAIN:
> > + u->buffer_size = u->buffer_size * 18 / 25;
> > + u->buffer_size -= u->buffer_size % u->frame_size;
> > + u->buffer_size = PA_MAX(u->buffer_size, (int32_t)MIN_BUFFER_SIZE);
> > + pa_sink_set_max_request(u->sink, u->buffer_size);
> > + pa_log("EAGAIN. Buffer size is now %u bytes (%llu buffered)", u->buffer_size, buffered_bytes);
> > + break;
>
> Hmm, care to explain this?
EAGAIN happens when the user requests a buffer size that is too large for
the STREAMS layer to accept. We end up looping with EAGAIN every time we
try to write out the rest of the buffer, which burns enough CPU time to
trip the CPU limit.
So, I reduce the buffer size with each EAGAIN. This gets us reasonably
close to the largest usable buffer size. (Perhaps there's a better way to
determine what that limit is, but I don't know how.)
> > +
> > + pa_rtpoll_set_timer_absolute(u->rtpoll, xtime0 + pa_bytes_to_usec(buffered_bytes / 2, &u->sink->sample_spec));
> > + } else {
> > + pa_rtpoll_set_timer_disabled(u->rtpoll);
> > }
>
> Hmm, you schedule audio via timers? Is that a good idea?
Perhaps not. I won't know until I test on more hardware.
But, given that we have rt priority and high resolution timers on solaris,
I think it is OK in theory...
The reason I used a timer was to minimise CPU usage and avoid the CPU
limit. Recall that getting woken up by poll is not an option for playback
unfortunately. We can arrange for a signal when the FD becomes writable,
but that throws out the whole buffer size concept, which acts to reduce
latency.
> That really only makes sense if you have to deal with large buffers and
> support rewinding.
I've implemented rewind support, but I'm still not sure that I have
understood the concept; I take it that we "rewind" (from the point-of-view
of the renderer, not the sink) so that some rendered but as yet unplayed
portion of the memblock/buffers can then be rendered again?
> Please keep in mind that the system clock and the sound card clock
> deviate. If you use the system timers to do PCM scheduling ou might need
> a pa_smoother object that is able to estimate the deviation for you.
Actually, in an earlier version I did use a smoother (after reading about
that in the wiki). But because of the non-monotonic sample counter (bug?)
I decided that it probably wasn't worth the added complexity so I removed
it. I'll put the smoother back if I can figure out the problem with the
sample counter.
>
> > + u->frame_size = pa_frame_size(&ss);
> >
> > - if ((fd = open(p = pa_modargs_get_value(ma, "device", DEFAULT_DEVICE), mode | O_NONBLOCK)) < 0)
> > + u->buffer_size = 16384;
>
> It would appear more appropriate to me if the buffer size is adjusted by
> the sample spec used.
Done.
> One last thing: it would probably be a good idea to allocate a pa_card
> object and attach the sink and the source to it.
It is possible to open /dev/audio twice by loading the solaris module
twice -- once for the sink (passing record=0) and once for source (passing
playback=0), thus giving seperate threads/LWPs for source and sink. It
might be misleading to allocate two cards in that situation?
> Right now pa_cards are mostly useful for switching profiles but even if
> you do not allow switching profiles on-the-fly it is of some value to
> find out via the cards object which source belongs to which sink.
>
> Otherwise I am happy!
>
> Thanks for your patch! I'd be thankful if you could fix the issues
> pointed out and prepare another patch on top of current git!
No problem. Patch follows. It also includes a portability fix for
pa_realpath and a fix for a bug in the pa_signal_new() error path that
causes signal data be freed if you attempt to register the same signal
twice.
> I hope I answered all your questions,
Your answers were very helpful, thanks.
Finn
>
> Lennart
>
>
|
|
|
|
|
|
Hi All,
This patch fixes the solaris audio device source and sink, and fixes some
portability issues that break the build on solaris. Questions and comments
welcomed.
I've tested this patch only with OpenSolaris Express snv 103. Eventually I
hope to be able to test a few older releases and older hardware (though it
is hard to say whether there is much interest in those).
This is my first brush with pulseaudio and so I read the wiki docs and
some of the source code but I'm still unsure of a few things. In
particular I'm wondering about rewind processing, corking and what (if
anything) the module needs for those. I'm also unclear on the implications
of thread_info.buffer_size, .fragment_size and .max_request, and whether
my code is correct or not.
This patch disables link map/library versioning unless ld is GNU ld.
Another approach for solaris would be to use that linker's -M option, but
I couldn't make that work (due to undefined mainloop, browse and simple
symbols when linking pacat. I can post the errors if anyone is intested.)
Thanks,
Finn Thain
|
|
|
|
|
|
git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@1986 fefdeb5f-60dc-0310-8127-8f9354f1896f
|
|
git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@1971 fefdeb5f-60dc-0310-8127-8f9354f1896f
|
|
git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@1426 fefdeb5f-60dc-0310-8127-8f9354f1896f
|
|
mmmkay?
git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@1418 fefdeb5f-60dc-0310-8127-8f9354f1896f
|
|
efficient blocks.
git-svn-id: file:///home/lennart/svn/public/pulseaudio/trunk@1327 fefdeb5f-60dc-0310-8127-8f9354f1896f
|
