Re: alsa pulse bugs

Re: alsa pulse bugs

[Colin Guthrie]

tom@dbservice.com wrote:
> I reported three bugs to the alsa bugtracker:
> 
> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3942
> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3943
> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3944
> 
> It's three bugs after all. Once these are fixed I expect Wine to work.  
> *fingers crossed*

Ace Tom :)

I've cross posted this to the alsa devel list. Hopefully Takashi (who's 
committed some pulse-related stuff recently) will have some insight here.

Re the snd_pcm_delay() including network latency (#3945), this clearly 
makes sense for network streams. Does you proposed fix include this 
delay (albeit with the improvement that it also will drop to 0 if there 
are no samples queued)?

Also re #3942, I believe (but not sure) that the max buffer in the 
glitch free branch has been increased significantly (as it now needs to 
keep some degree of past sound as well as future buffer to allow the 
rewinds to work properly (this is no doubt an inaccurate description of 
why it's needed.... :p)) I'm not sure how this would affect this 
solution, but Lennart will be able to advise better.

Hopefully he'll have some comments on all of the above bugs as you've 
clearly put in a lot of work to try and nail these issues :)

Col

Re: alsa pulse bugs

[tom]

Quoting Colin Guthrie <gmane@colin.guthr.ie>:

> tom@dbservice.com wrote:
>> I reported three bugs to the alsa bugtracker:
>>
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3942
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3943
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3944
>>
>> It's three bugs after all. Once these are fixed I expect Wine to work.
>> *fingers crossed*
>
> Ace Tom :)
>
> I've cross posted this to the alsa devel list. Hopefully Takashi (who's
> committed some pulse-related stuff recently) will have some insight here.
>
> Re the snd_pcm_delay() including network latency (#3945), this clearly
> makes sense for network streams. Does you proposed fix include this
> delay (albeit with the improvement that it also will drop to 0 if there
> are no samples queued)?

snd_pcm_delay() should not include any network latency. The API is  
defined as 'read pointer - write pointer', and applications expect  
that. Or at least they expect that when all samples are played that  
the delay drops to zero.

And no, my current patch does not include network latency at all. It  
simply does what alsa does, 'read pointer - write pointer'.

> Also re #3942, I believe (but not sure) that the max buffer in the
> glitch free branch has been increased significantly (as it now needs to
> keep some degree of past sound as well as future buffer to allow the
> rewinds to work properly (this is no doubt an inaccurate description of
> why it's needed.... :p)) I'm not sure how this would affect this
> solution, but Lennart will be able to advise better.

I'd suggest to export MAX_MEMBLOCKQ_LENGTH in a public API so that  
applications can make use of it. If the max buffer length has been  
increased, then the alsa pulse plugin should be able to propagate that  
to applications using the alsa API.

tom

Re: alsa pulse bugs

[Colin Guthrie]

tom@dbservice.com wrote:
>> Re the snd_pcm_delay() including network latency (#3945), this clearly
>> makes sense for network streams. Does you proposed fix include this
>> delay (albeit with the improvement that it also will drop to 0 if there
>> are no samples queued)?
> 
> snd_pcm_delay() should not include any network latency. The API is  
> defined as 'read pointer - write pointer', and applications expect  
> that. Or at least they expect that when all samples are played that  
> the delay drops to zero.

With the caveat of very limited technical knowledge, I can agree on the 
latter point (drop to 0 when all samples are played), but if it was 
implemented sans net-delay in pulse would this not cause e.g. a-v sync 
issues when playing via alsa to a networked PA server? If so then this 
fix would introduce another bug.

>> Also re #3942, I believe (but not sure) that the max buffer in the
>> glitch free branch has been increased significantly (as it now needs to
>> keep some degree of past sound as well as future buffer to allow the
>> rewinds to work properly (this is no doubt an inaccurate description of
>> why it's needed.... :p)) I'm not sure how this would affect this
>> solution, but Lennart will be able to advise better.
> 
> I'd suggest to export MAX_MEMBLOCKQ_LENGTH in a public API so that  
> applications can make use of it. If the max buffer length has been  
> increased, then the alsa pulse plugin should be able to propagate that  
> to applications using the alsa API.

Well I'm not sure of the internals of the glitch-free stuff, but I doubt 
it's a buffer that can be used in quite the same way as that. Lennart 
will be able to advise if I've got the wrong end of the stick in my 
comments :)

Col

Re: alsa pulse bugs

[tom]

Quoting Colin Guthrie <gmane@colin.guthr.ie>:

Attached patches to #3943 and #3944. Please read the comments there.

> tom@dbservice.com wrote:
>>> Re the snd_pcm_delay() including network latency (#3945), this clearly
>>> makes sense for network streams. Does you proposed fix include this
>>> delay (albeit with the improvement that it also will drop to 0 if there
>>> are no samples queued)?
>>
>> snd_pcm_delay() should not include any network latency. The API is
>> defined as 'read pointer - write pointer', and applications expect
>> that. Or at least they expect that when all samples are played that
>> the delay drops to zero.
>
> With the caveat of very limited technical knowledge, I can agree on the
> latter point (drop to 0 when all samples are played), but if it was
> implemented sans net-delay in pulse would this not cause e.g. a-v sync
> issues when playing via alsa to a networked PA server? If so then this
> fix would introduce another bug.

Your arguments seem reasonable. Let's see what others have to say to  
it. It would be interesting to see how much delay such change  
introduces. I can't test PA over network at home, but if someone  
wants, the patch is attached to the bug entry in the alsa bugtracker.

>>> Also re #3942, I believe (but not sure) that the max buffer in the
>>> glitch free branch has been increased significantly (as it now needs to
>>> keep some degree of past sound as well as future buffer to allow the
>>> rewinds to work properly (this is no doubt an inaccurate description of
>>> why it's needed.... :p)) I'm not sure how this would affect this
>>> solution, but Lennart will be able to advise better.
>>
>> I'd suggest to export MAX_MEMBLOCKQ_LENGTH in a public API so that
>> applications can make use of it. If the max buffer length has been
>> increased, then the alsa pulse plugin should be able to propagate that
>> to applications using the alsa API.
>
> Well I'm not sure of the internals of the glitch-free stuff, but I doubt
> it's a buffer that can be used in quite the same way as that. Lennart
> will be able to advise if I've got the wrong end of the stick in my
> comments :)

tom

Re: alsa pulse bugs

[Colin Guthrie]

Colin Guthrie wrote:
> tom@dbservice.com wrote:
>>> Re the snd_pcm_delay() including network latency (#3945), this clearly
>>> makes sense for network streams. Does you proposed fix include this
>>> delay (albeit with the improvement that it also will drop to 0 if there
>>> are no samples queued)?
>> snd_pcm_delay() should not include any network latency. The API is  
>> defined as 'read pointer - write pointer', and applications expect  
>> that. Or at least they expect that when all samples are played that  
>> the delay drops to zero.
> 
> With the caveat of very limited technical knowledge, I can agree on the 
> latter point (drop to 0 when all samples are played), but if it was 
> implemented sans net-delay in pulse would this not cause e.g. a-v sync 
> issues when playing via alsa to a networked PA server? If so then this 
> fix would introduce another bug.

Actually just having a very quick glance at the Alsa API docs, it 
doesn't mention that this value should be 0 if there are no samples to play:

http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga0d9e14a4be65209eb549e48a9f07302

Closest it says is: "It's positive and less than buffer size in normal 
situation".

So perhaps this is an invalid assumption at the wine side?

Is there perhaps a more appropriate API call they can use to do whatever 
test they are doing?

Col

Re: alsa pulse bugs

[tom]

Quoting Colin Guthrie <gmane@colin.guthr.ie>:
> Actually just having a very quick glance at the Alsa API docs, it
> doesn't mention that this value should be 0 if there are no samples to play:
>
> http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga0d9e14a4be65209eb549e48a9f07302
>
> Closest it says is: "It's positive and less than buffer size in normal
> situation".
>
> So perhaps this is an invalid assumption at the wine side?

However this part:

Delay is distance between current application frame position and sound  
frame position.

suggests that it indeed can be zero. Or why would a soundcard have a  
gap between the read and write positions if it has played everything?

If Wine uses a wrong assumption, then please clarify the function description.

> Is there perhaps a more appropriate API call they can use to do whatever
> test they are doing?

What Wine needs to know, is whether a particular sample has already  
been played or not. It does 'bytes_written - delay_in_bytes' to find  
out how much of the written data the soundcard has already played. If  
there is a better test for it, please explain.

tom

Re: alsa pulse bugs

[Colin Guthrie]

tom@dbservice.com wrote:
>> http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga0d9e14a4be65209eb549e48a9f07302
>>
>> Closest it says is: "It's positive and less than buffer size in normal
>> situation".
>>
>> So perhaps this is an invalid assumption at the wine side?
> 
> However this part:
> 
> Delay is distance between current application frame position and sound  
> frame position.
> 
> suggests that it indeed can be zero. Or why would a soundcard have a  
> gap between the read and write positions if it has played everything?

Indeed. I agree. However, for me this is something that should not be in 
high level documentation, that should be an internal thing (i.e. it's a 
detail of the implementation). It's perfectly true of hardware devices 
(no doubt the context when the docs were written) but perhaps this 
should not be true of non-h/w or ioplug based "devices"? Someone who 
vaguely understands the ALSA stuff should have a better insight on this 
to comment :p

>> Is there perhaps a more appropriate API call they can use to do whatever
>> test they are doing?
> 
> What Wine needs to know, is whether a particular sample has already  
> been played or not. It does 'bytes_written - delay_in_bytes' to find  
> out how much of the written data the soundcard has already played. If  
> there is a better test for it, please explain.

I've no idea, I'm just throwing up ideas/suggestions/opinions... there 
is little technical background from my comments, just trying to raise 
some points so people more familiar than I can comment :)

Take care

Col

Re: [pulseaudio-discuss] alsa pulse bugs

[Lennart Poettering]

On Mon, 05.05.08 09:58, Colin Guthrie (gmane@colin.guthr.ie) wrote:

> 
> tom@dbservice.com wrote:
> > I reported three bugs to the alsa bugtracker:
> > 
> > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3942
> > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3943
> > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3944
> > 
> > It's three bugs after all. Once these are fixed I expect Wine to work.  
> > *fingers crossed*
> 
> Ace Tom :)
> 
> I've cross posted this to the alsa devel list. Hopefully Takashi (who's 
> committed some pulse-related stuff recently) will have some insight here.
> 
> Re the snd_pcm_delay() including network latency (#3945), this clearly 
> makes sense for network streams. Does you proposed fix include this 
> delay (albeit with the improvement that it also will drop to 0 if there 
> are no samples queued)?
> 
> Also re #3942, I believe (but not sure) that the max buffer in the 
> glitch free branch has been increased significantly (as it now needs to 
> keep some degree of past sound as well as future buffer to allow the 
> rewinds to work properly (this is no doubt an inaccurate description of 
> why it's needed.... :p)) I'm not sure how this would affect this 
> solution, but Lennart will be able to advise better.

Yes, with glitch-free maxlength becomes mostly irrelevant. It should
default to "0" know, to let the server decide.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: alsa pulse bugs

[Colin Guthrie]

Cross posting Lennart's reply to the Alsa list as requested.

Please can you guys apply #3942 and #3944 (see below), as per Lennart's 
recommendations?

Cheers :)

Col

Lennart Poettering wrote:
> On Mon, 05.05.08 10:34, tom@dbservice.com (tom@dbservice.com) wrote:
> 
>> I reported three bugs to the alsa bugtracker:
>>
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3942
> 
> I though I already posted a patch like this a while back. This really
> should be merged!
> 
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3943
> 
> This is not correct. snd_pcm_delay() is the delay that it takes until
> a sample you write now to the PCM stream takes to be played. For PCI
> devices this is usually the same as write_index-read_index, but not
> for virtual devices like PA, and not even for USB.
> 
> This is the only way snd_pcm_delay() has any value for synchronization
> purposes (i.e. syncing video to audio).
> 
> I think there's simply in invalid assumption made by the WINE devs (or
> possible even the MS devs who designed the API): they assume that all
> sound cards are PCI sound cards. Which ain't really true.
> 
> In short: the transport latency *needs* to be taken into account,
> there is no way around this.
> 
>> https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3944
> 
> This looks good to me and should also be applied.
> 
> Could you please post the patch for 3942 and 3944 to the ALSA ml? The
> truth is that the ALSA devs are very good at ignoring the ALSA BTS. If
> you want your stuff merged, post it to the ALSA ML, don't bother with
> the BTS.
> 
> And thank you very much for your patches!
> 
> Lennart
>