* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-02-28 20:26 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-02-28 20:26 UTC (permalink / raw)
To: alsa-devel
The following issue has been SUBMITTED.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 02-28-2005 21:26 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-01 8:36 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-01 8:36 UTC (permalink / raw)
To: alsa-devel
The following issue has been set as RELATED TO issue 0000946.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-01-2005 09:36 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-01 8:40 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-01 8:40 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-01-2005 09:40 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-01-05 09:40
----------------------------------------------------------------------
Could you split your patch to single very elementary ones?
Thanks.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-01 10:15 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-01 10:15 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-01-2005 11:15 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-01-05 09:40
----------------------------------------------------------------------
Could you split your patch to single very elementary ones?
Thanks.
----------------------------------------------------------------------
charles_levert - 03-01-05 11:15
----------------------------------------------------------------------
The snd_pcm_tick_prepare() change could be on its own.
That's one chunk with 2 old lines replaced by 1 new line.
The runtime->rate and runtime->sleep_min in
snd_pcm_tick_prepare() could be together on their own.
That's one chunk with 3 old lines replaced by 3 new lines.
Everything else, however, forms a coherent ensemble.
For example, snd_pcm_playback_silence() now relies on
snd_pcm_update_hw_ptr_interrupt() having already updated
runtime->hw_ptr_interrupt and done the snd_pcm_next_buf()
type of update. Splitting this in a fashion that would
leave the code in a still-coherent state in-between each
individual patch would mean writing intermediary code that
does not exist at this point. It is far from obvious that
what would so be made to appear as intermediary points
would bring any more insight into the single main issue.
It could present things in a worst way, actually.
I've been burned doing this type of work for nothing in
the past. Please provide me with a preliminary evaluation
of the patch and some rough directions about what you would
like to appear as intermediary points (and in which order
of individual patch application), if any is justified,
before I go any further.
The patch is actually not that big. I just was very
verbose in its description.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-02 8:22 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-02 8:22 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-02-2005 09:22 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-01-05 11:15
----------------------------------------------------------------------
The snd_pcm_tick_prepare() change could be on its own.
That's one chunk with 2 old lines replaced by 1 new line.
The runtime->rate and runtime->sleep_min in
snd_pcm_tick_prepare() could be together on their own.
That's one chunk with 3 old lines replaced by 3 new lines.
Everything else, however, forms a coherent ensemble.
For example, snd_pcm_playback_silence() now relies on
snd_pcm_update_hw_ptr_interrupt() having already updated
runtime->hw_ptr_interrupt and done the snd_pcm_next_buf()
type of update. Splitting this in a fashion that would
leave the code in a still-coherent state in-between each
individual patch would mean writing intermediary code that
does not exist at this point. It is far from obvious that
what would so be made to appear as intermediary points
would bring any more insight into the single main issue.
It could present things in a worst way, actually.
I've been burned doing this type of work for nothing in
the past. Please provide me with a preliminary evaluation
of the patch and some rough directions about what you would
like to appear as intermediary points (and in which order
of individual patch application), if any is justified,
before I go any further.
The patch is actually not that big. I just was very
verbose in its description.
----------------------------------------------------------------------
Clemens Ladisch - 03-02-05 09:22
----------------------------------------------------------------------
As far as I can see there are several changes to fix bugs, and other
changes
that are enhancements not intended to change the outwards behaviour of
ALSA.
Please make one patch for each bug. (Each _logical_ change should be one
patch, see section 1.3 in SubmittingPatches.)
There are several problems with this version of the patch:
* snd_pcm_update_hw_ptr_interrupt() doesn't compute the time of the next
interrupt; runtime->hw_ptr_interrupt is the beginning of the last
(current) period. It is not sufficient do add one period_size because
some PCM interrupts might have been lost, therefore the new value must
be derived from the actual hardware pointer. (The same applies to the
dist calculation in snd_pcm_tick_prepare().)
* Xrun debugging was dropped. Why?
* The documented behaviour for frame pointers is to wrap around to zero
when the boundary is reached. Safe zones can only be implemented if the
pointer values exported to user space and OSS emulation still show
backwards-compatible behaviour.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-02 12:01 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-02 12:01 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-02-2005 13:01 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
Clemens Ladisch - 03-02-05 09:22
----------------------------------------------------------------------
As far as I can see there are several changes to fix bugs, and other
changes
that are enhancements not intended to change the outwards behaviour of
ALSA.
Please make one patch for each bug. (Each _logical_ change should be one
patch, see section 1.3 in SubmittingPatches.)
There are several problems with this version of the patch:
* snd_pcm_update_hw_ptr_interrupt() doesn't compute the time of the next
interrupt; runtime->hw_ptr_interrupt is the beginning of the last
(current) period. It is not sufficient do add one period_size because
some PCM interrupts might have been lost, therefore the new value must
be derived from the actual hardware pointer. (The same applies to the
dist calculation in snd_pcm_tick_prepare().)
* Xrun debugging was dropped. Why?
* The documented behaviour for frame pointers is to wrap around to zero
when the boundary is reached. Safe zones can only be implemented if the
pointer values exported to user space and OSS emulation still show
backwards-compatible behaviour.
----------------------------------------------------------------------
charles_levert - 03-02-05 13:01
----------------------------------------------------------------------
> There are several problems with this version of the patch:
Then I'm glad I asked before doing anything. I would have
done a useless iteration otherwise. Some design issues
do need to be discussed before C-language implementation
and patch details.
> snd_pcm_update_hw_ptr_interrupt() doesn't compute the
> time of the next interrupt; runtime->hw_ptr_interrupt is
> the beginning of the last (current) period.
I knew that it was about the last/current period. It is
still one of the the next ones compared to the previous
value.
The main point, though, is in the way that "the beginning"
is interpreted. See the rounding-to-nearest and lost IRQ
discussion below.
> It is not sufficient do add one period_size because
> some PCM interrupts might have been lost, therefore
> the new value must be derived from the actual hardware
> pointer.
Ok. In that case, there are two problems with the existing
implementation.
-- One is that rounding down (truncating) is absolutely
not appropriate. Rounding to nearest should be
performed because of the possibility of early
DMA IRQs.
-- The other one is that buffer_size is not enforced to
be an exact multiple of period_size. More
specifically, it is boundary (or "boundary -
buffer_size" in my proposed implementation) that
is not that. The first condition will imply the
second one since boundary is itself a multiple of
buffer_size, and is a simple way to do things.
Since this condition is not enforced, we need to keep
track of a proper offset to add before the modulo
period_size to perform rounding. This offset needs
to be recomputed each time a boundary wrapping occurs.
I think I have an idea on how to do this simply
(without new variables or struct fields), correctly,
and still rather efficiently, since lost IRQs won't
number in the dozen each time it happens.
(I did anticipate that lost IRQs could be an issue as I
hinted in my patch description at the minimum acceptable
possibility of adopting a rounding-to-nearest approach.
I didn't know for sure and that's why I mentioned it in
the hope that whoever read it might bring it up if it was
effectively an issue.)
> (The same applies to the dist calculation in
> snd_pcm_tick_prepare().)
Not in the use that's made of it just
after snd_pcm_update_hw_ptr_interrupt() and
snd_pcm_update_hw_ptr(), as it is just really a
_prediction_ of the next interrupt from the just newly
computed one (as opposed to an update) and adding a
period_size is equivalent to a properly rounded computation
then, but maybe for other uses if runtime->status->hw_ptr
has been updated in the mean time by some other code.
*** Is that last thing possible? ***
You know the rest of the code better than I do.
I do also predict the next interrupt in my new version of
snd_pcm_playback_silence(), although still in the case
where a simple addition suffices, so I am beginning to
think that the logic for this may gain from being isolated
in its own inline function, if it is needed.
> Xrun debugging was dropped. Why?
Because once the issue of early DMA IRQs is addressed
and that is accepted as a normal occurrence, this is no
longer an exceptional situation that needs to be reported.
When we couple the early IRQ issue with the dropped
IRQ one, the proper approach that imposes itself in the
rounding-to-nearest one. It is really that part of the
conditional expression that disappeared, not the inner
"runtime->periods > 1 && substream->pstr->xrun_debug"
one which has been logically short-circuited by then
("0 && foo").
Of course, it is always possible to retain the message,
but it would just be an annoyance on hardware where early
DMA IRQs are a regular fact of life and it seems it was
originally there because that vision wasn't accepted.
> The documented behaviour for frame pointers is to wrap
> around to zero when the boundary is reached. Safe zones can
> only be implemented if the pointer values exported to user
> space and OSS emulation still show backwards-compatible
> behaviour.
Ok. I did address the OSS emulation part (with an added
commentary warning about this that OSS emulation did play
is pcm_lib.c's yard), but since user space is outside
the kernel code's control, a compatible solution needs
to be devised. It really is risky programming to do
this boundary wrapping all over the place with different
variables to be wrapped and I do think that the centralized
wrapping approach can be a more sensible one.
I will think about this one and get back to you on this. I
may consider just fixing the existing bugs with this where
they are and adapting my new snd_pcm_playback_silence()
to the decentralized approach.
If I do this, I will put existing bugs in separate patches.
I am still likely to not break the snd_pcm_update_hw_ptr*()
stuff and snd_pcm_playback_silence() stuff in separate
patches. I will see. Coupled issues are effectively one
logical issue and split-to-the-extreme patches just should
not be dogma.
Please be sure to provide insight on the question I
highlighted before then. (Thanks.)
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-02 12:39 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-02 12:39 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-02-2005 13:39 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-02-05 13:01
----------------------------------------------------------------------
> There are several problems with this version of the patch:
Then I'm glad I asked before doing anything. I would have
done a useless iteration otherwise. Some design issues
do need to be discussed before C-language implementation
and patch details.
> snd_pcm_update_hw_ptr_interrupt() doesn't compute the
> time of the next interrupt; runtime->hw_ptr_interrupt is
> the beginning of the last (current) period.
I knew that it was about the last/current period. It is
still one of the the next ones compared to the previous
value.
The main point, though, is in the way that "the beginning"
is interpreted. See the rounding-to-nearest and lost IRQ
discussion below.
> It is not sufficient do add one period_size because
> some PCM interrupts might have been lost, therefore
> the new value must be derived from the actual hardware
> pointer.
Ok. In that case, there are two problems with the existing
implementation.
-- One is that rounding down (truncating) is absolutely
not appropriate. Rounding to nearest should be
performed because of the possibility of early
DMA IRQs.
-- The other one is that buffer_size is not enforced to
be an exact multiple of period_size. More
specifically, it is boundary (or "boundary -
buffer_size" in my proposed implementation) that
is not that. The first condition will imply the
second one since boundary is itself a multiple of
buffer_size, and is a simple way to do things.
Since this condition is not enforced, we need to keep
track of a proper offset to add before the modulo
period_size to perform rounding. This offset needs
to be recomputed each time a boundary wrapping occurs.
I think I have an idea on how to do this simply
(without new variables or struct fields), correctly,
and still rather efficiently, since lost IRQs won't
number in the dozen each time it happens.
(I did anticipate that lost IRQs could be an issue as I
hinted in my patch description at the minimum acceptable
possibility of adopting a rounding-to-nearest approach.
I didn't know for sure and that's why I mentioned it in
the hope that whoever read it might bring it up if it was
effectively an issue.)
> (The same applies to the dist calculation in
> snd_pcm_tick_prepare().)
Not in the use that's made of it just
after snd_pcm_update_hw_ptr_interrupt() and
snd_pcm_update_hw_ptr(), as it is just really a
_prediction_ of the next interrupt from the just newly
computed one (as opposed to an update) and adding a
period_size is equivalent to a properly rounded computation
then, but maybe for other uses if runtime->status->hw_ptr
has been updated in the mean time by some other code.
*** Is that last thing possible? ***
You know the rest of the code better than I do.
I do also predict the next interrupt in my new version of
snd_pcm_playback_silence(), although still in the case
where a simple addition suffices, so I am beginning to
think that the logic for this may gain from being isolated
in its own inline function, if it is needed.
> Xrun debugging was dropped. Why?
Because once the issue of early DMA IRQs is addressed
and that is accepted as a normal occurrence, this is no
longer an exceptional situation that needs to be reported.
When we couple the early IRQ issue with the dropped
IRQ one, the proper approach that imposes itself in the
rounding-to-nearest one. It is really that part of the
conditional expression that disappeared, not the inner
"runtime->periods > 1 && substream->pstr->xrun_debug"
one which has been logically short-circuited by then
("0 && foo").
Of course, it is always possible to retain the message,
but it would just be an annoyance on hardware where early
DMA IRQs are a regular fact of life and it seems it was
originally there because that vision wasn't accepted.
> The documented behaviour for frame pointers is to wrap
> around to zero when the boundary is reached. Safe zones can
> only be implemented if the pointer values exported to user
> space and OSS emulation still show backwards-compatible
> behaviour.
Ok. I did address the OSS emulation part (with an added
commentary warning about this that OSS emulation did play
is pcm_lib.c's yard), but since user space is outside
the kernel code's control, a compatible solution needs
to be devised. It really is risky programming to do
this boundary wrapping all over the place with different
variables to be wrapped and I do think that the centralized
wrapping approach can be a more sensible one.
I will think about this one and get back to you on this. I
may consider just fixing the existing bugs with this where
they are and adapting my new snd_pcm_playback_silence()
to the decentralized approach.
If I do this, I will put existing bugs in separate patches.
I am still likely to not break the snd_pcm_update_hw_ptr*()
stuff and snd_pcm_playback_silence() stuff in separate
patches. I will see. Coupled issues are effectively one
logical issue and split-to-the-extreme patches just should
not be dogma.
Please be sure to provide insight on the question I
highlighted before then. (Thanks.)
----------------------------------------------------------------------
perex - 03-02-05 13:39
----------------------------------------------------------------------
Please, don't take our comments as offensive to your work. We really
appreciate it, but we must verify the patches and think about the possible
behaviour.
Actually, having "early interrupts" is not a big fun, because it really
changes the expectations of the midlevel code and applications. Both are
expecting that hardware acknowledges the whole chunk (period) not only
part of it. I would suggest in this case to handle the behaviour in the
driver code rather than in midlevel, because we will lose the detection of
missed interrupts then. For example, the trident driver has same problem
and various workarounds for this behaviour.
There are several ways to fix the driver:
1) use more periods and handle "extra" periods internally only in the
driver
2) use another timing source
Please, at first stage, provide the "bugfix" patch only for current ALSA
tree and then send a message to alsa-devel mailing list, explaining the
"early interrupt" handling and pointing to this bug-page... I think that
we need to do more discussion if it's worth to implement it to midlevel..
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-02 13:53 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-02 13:53 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-02-2005 14:53 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-02-05 13:39
----------------------------------------------------------------------
Please, don't take our comments as offensive to your work. We really
appreciate it, but we must verify the patches and think about the possible
behaviour.
Actually, having "early interrupts" is not a big fun, because it really
changes the expectations of the midlevel code and applications. Both are
expecting that hardware acknowledges the whole chunk (period) not only
part of it. I would suggest in this case to handle the behaviour in the
driver code rather than in midlevel, because we will lose the detection of
missed interrupts then. For example, the trident driver has same problem
and various workarounds for this behaviour.
There are several ways to fix the driver:
1) use more periods and handle "extra" periods internally only in the
driver
2) use another timing source
Please, at first stage, provide the "bugfix" patch only for current ALSA
tree and then send a message to alsa-devel mailing list, explaining the
"early interrupt" handling and pointing to this bug-page... I think that
we need to do more discussion if it's worth to implement it to midlevel..
----------------------------------------------------------------------
charles_levert - 03-02-05 14:53
----------------------------------------------------------------------
> Please, don't take our comments as offensive to your work.
It's OK. Actually, I wanted the comments, even if summary,
rather than just an evaluation based on apparent patch
size and coverage.
I have decided to go the with the existing boundary
wrapping approach and just fix its bugs.
So I will be able to send tiny patches for that.
There was a problem with my patch and its removal of
"Xrun debugging"; I confused buffer_size and period_size
in one test, but never noticed it presumably because I
never experienced lost IRQs in testing.
So there might just be an acceptable generic midlevel
solution that doesn't loose lost-IRQ detection. If not,
I'll just scrap that part.
Doing something like "hw_ptr_interrupt % period_size"
after boundary wrapping still appears incorrect (because
of the non-multiple thing) and should be fixed. The fix
will initially use rounding-to-nearest, but would still
remain relevant without it.
There are issues in snd_pcm_playback_silence() that need
to be addressed so I still think rewriting its first part
is a good thing.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 13:37 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 13:37 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 14:37 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-02-05 14:53
----------------------------------------------------------------------
> Please, don't take our comments as offensive to your work.
It's OK. Actually, I wanted the comments, even if summary,
rather than just an evaluation based on apparent patch
size and coverage.
I have decided to go the with the existing boundary
wrapping approach and just fix its bugs.
So I will be able to send tiny patches for that.
There was a problem with my patch and its removal of
"Xrun debugging"; I confused buffer_size and period_size
in one test, but never noticed it presumably because I
never experienced lost IRQs in testing.
So there might just be an acceptable generic midlevel
solution that doesn't loose lost-IRQ detection. If not,
I'll just scrap that part.
Doing something like "hw_ptr_interrupt % period_size"
after boundary wrapping still appears incorrect (because
of the non-multiple thing) and should be fixed. The fix
will initially use rounding-to-nearest, but would still
remain relevant without it.
There are issues in snd_pcm_playback_silence() that need
to be addressed so I still think rewriting its first part
is a good thing.
----------------------------------------------------------------------
charles_levert - 03-03-05 14:37
----------------------------------------------------------------------
Attached file "early-irqs.patch" is hereby obsoleted.
It is replaced by the following four patches:
-- "hz-int-div.0b03.patch":
Fix rounded-up integer division bug.
-- "rate-sleep-ticks.0b03.patch":
Fix bugs with usage of runtime->tick_time.
-- "appl_ptr-boundary.0b03.patch":
Fix bugs with incorrectly wrapped appl_ptr.
-- "early-irqs.0b03.patch":
Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations.
The bulk of changes can be found in the last patch, but
their number has been significantly reduced in some places.
E.g., snd_pcm_update_hw_ptr() is no longer modified and
snd_pcm_update_hw_ptr_interrupt() is much less modified.
The existing boundary-rounding intervals are no longer
changed so user space should not show the possibility of
being adversely affected.
The OSS changes are also no longer required.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 14:03 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 14:03 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 15:03 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-03-05 14:37
----------------------------------------------------------------------
Attached file "early-irqs.patch" is hereby obsoleted.
It is replaced by the following four patches:
-- "hz-int-div.0b03.patch":
Fix rounded-up integer division bug.
-- "rate-sleep-ticks.0b03.patch":
Fix bugs with usage of runtime->tick_time.
-- "appl_ptr-boundary.0b03.patch":
Fix bugs with incorrectly wrapped appl_ptr.
-- "early-irqs.0b03.patch":
Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations.
The bulk of changes can be found in the last patch, but
their number has been significantly reduced in some places.
E.g., snd_pcm_update_hw_ptr() is no longer modified and
snd_pcm_update_hw_ptr_interrupt() is much less modified.
The existing boundary-rounding intervals are no longer
changed so user space should not show the possibility of
being adversely affected.
The OSS changes are also no longer required.
----------------------------------------------------------------------
perex - 03-03-05 15:03
----------------------------------------------------------------------
It seems that hz-int-div.0b03.patch and rate-sleep-ticks.0b03.patch
contains same patch but different description....
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 14:34 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 14:34 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 15:34 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-03-05 15:03
----------------------------------------------------------------------
It seems that hz-int-div.0b03.patch and rate-sleep-ticks.0b03.patch
contains same patch but different description....
----------------------------------------------------------------------
charles_levert - 03-03-05 15:34
----------------------------------------------------------------------
Please use "rate-sleep-ticks.0b03x.patch" instead.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 14:52 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 14:52 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 15:52 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-03-05 15:34
----------------------------------------------------------------------
Please use "rate-sleep-ticks.0b03x.patch" instead.
----------------------------------------------------------------------
perex - 03-03-05 15:52
----------------------------------------------------------------------
Note that appl_ptr-boundary.0b03.patch is only cosmetic change and it does
not fix anything, because the previous code does wrapping the transferred
chunk to continuous memory area, thus wrapping to 0 is correct. But your
code looks better, thus I applied this patch and hz-int-div.0b03.patch to
CVS.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 15:05 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 15:05 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 16:05 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-03-05 15:52
----------------------------------------------------------------------
Note that appl_ptr-boundary.0b03.patch is only cosmetic change and it does
not fix anything, because the previous code does wrapping the transferred
chunk to continuous memory area, thus wrapping to 0 is correct. But your
code looks better, thus I applied this patch and hz-int-div.0b03.patch to
CVS.
----------------------------------------------------------------------
perex - 03-03-05 16:05
----------------------------------------------------------------------
The rate-sleep-ticks.0b03x.patch looks wrong to me. We need the time in
timer ticks, so we need to divide the microsecond value with tick_time
(it's only in one expression). The snd_pcm_tick_set() function accepts
ticks not microseconds. And your patch gives the time distance in
microseconds as result.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-03 19:30 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-03 19:30 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-03-2005 20:30 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-03-05 16:05
----------------------------------------------------------------------
The rate-sleep-ticks.0b03x.patch looks wrong to me. We need the time in
timer ticks, so we need to divide the microsecond value with tick_time
(it's only in one expression). The snd_pcm_tick_set() function accepts
ticks not microseconds. And your patch gives the time distance in
microseconds as result.
----------------------------------------------------------------------
rlrevell - 03-03-05 20:30
----------------------------------------------------------------------
* Xrun debugging was dropped. Why?
This is kind of a hack anyway. There are much, much cleaner ways to debug
this now. Just my $0.02.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-04 2:34 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-04 2:34 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-04-2005 03:34 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
rlrevell - 03-03-05 20:30
----------------------------------------------------------------------
* Xrun debugging was dropped. Why?
This is kind of a hack anyway. There are much, much cleaner ways to debug
this now. Just my $0.02.
----------------------------------------------------------------------
charles_levert - 03-04-05 03:34
----------------------------------------------------------------------
> The snd_pcm_tick_set() function accepts
> ticks not microseconds.
Ah, good! We might be zeroing-in on the problem. I.e.,
does what the implemention match the design intentions?
Please look at what snd_pcm_system_tick_set() actually
does, regardless of the name of its "ticks" argument.
Ignoring the rounding part of the division, it does
(mathematically, in real numbers not integers):
mod_timer_ticks := "ticks" / (1000000 / HZ)
= ("ticks" / 1000000) * HZ
It really seems to me that
-- The alsa_ticks unit are no longer involved at this
point.
-- The input "ticks" argument really is treated by this
function as microseconds.
-- There is a first (mathematical) conversion from
microseconds to seconds (the /1000000 part).
-- There is a second (mathematical) conversion from
seconds to kernel_timer units (its own ticks, the
*HZ part).
Note that the only two other calls to snd_pcm_tick_set(),
which are in file pcm_native.c, set the "ticks" argument
to zero so no difference is felt from their point of view.
In the event that a combined solution to what's
covered in both "hz-int-div.0b03.patch" and
"rate-sleep-ticks.0b03x.patch" modifies things so that
snd_pcm_tick_set() really takes its "ticks" argument in
alsa_ticks units (adding a first [mathematical] conversion
from alsa_ticks units to microseconds), please verify
the following. (The "rate-sleep-ticks.0b03x.patch"
would no longer be necessary as it stands then.)
There is effectively a division by runtime->tick_time
at the end of snd_pcm_tick_prepare(); for that matter,
there is still a division by runtime->rate. Since those
values were initially obtained from the user, were they
ever validated to be non-zero at any point? What would
be the consequences of a division by zero there? I know
that setting these values to zero is non-sensical from an
usefulness point-of-view, but is an attack on the system
from a local user possible there? I prefer asking and
being told I'm wrong than not asking but having been right.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-04 3:46 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-04 3:46 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-04-2005 04:46 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-04-05 03:34
----------------------------------------------------------------------
> The snd_pcm_tick_set() function accepts
> ticks not microseconds.
Ah, good! We might be zeroing-in on the problem. I.e.,
does what the implemention match the design intentions?
Please look at what snd_pcm_system_tick_set() actually
does, regardless of the name of its "ticks" argument.
Ignoring the rounding part of the division, it does
(mathematically, in real numbers not integers):
mod_timer_ticks := "ticks" / (1000000 / HZ)
= ("ticks" / 1000000) * HZ
It really seems to me that
-- The alsa_ticks unit are no longer involved at this
point.
-- The input "ticks" argument really is treated by this
function as microseconds.
-- There is a first (mathematical) conversion from
microseconds to seconds (the /1000000 part).
-- There is a second (mathematical) conversion from
seconds to kernel_timer units (its own ticks, the
*HZ part).
Note that the only two other calls to snd_pcm_tick_set(),
which are in file pcm_native.c, set the "ticks" argument
to zero so no difference is felt from their point of view.
In the event that a combined solution to what's
covered in both "hz-int-div.0b03.patch" and
"rate-sleep-ticks.0b03x.patch" modifies things so that
snd_pcm_tick_set() really takes its "ticks" argument in
alsa_ticks units (adding a first [mathematical] conversion
from alsa_ticks units to microseconds), please verify
the following. (The "rate-sleep-ticks.0b03x.patch"
would no longer be necessary as it stands then.)
There is effectively a division by runtime->tick_time
at the end of snd_pcm_tick_prepare(); for that matter,
there is still a division by runtime->rate. Since those
values were initially obtained from the user, were they
ever validated to be non-zero at any point? What would
be the consequences of a division by zero there? I know
that setting these values to zero is non-sensical from an
usefulness point-of-view, but is an attack on the system
from a local user possible there? I prefer asking and
being told I'm wrong than not asking but having been right.
----------------------------------------------------------------------
charles_levert - 03-04-05 04:46
----------------------------------------------------------------------
I've though about it some more, and here's my
recommendation, FWIW.
In the event that the units of kernel_ticks and alsa_ticks
are close to one another, but not quite equal, having a
snd_pcm_tick_set() that accepts alsa_ticks and converts
them to kernel_ticks might result in an important
cumulative loss of precision (given that the only current
non-zero usage, the one at hand in snd_pcm_tick_prepare(),
started from a number of samples [aka frames]).
So I would advocate leaving the current implementation
of snd_pcm_tick_set() and snd_pcm_system_tick_set()
as it is, but renaming them to snd_pcm_timer_set() and
snd_pcm_system_timer_set() and renaming their "ticks"
argument to "microseconds". Thus revising the documented
design, as opposed to the implementation.
Assuming some future non-zero usages are to come, it
think it's actually better to require some callers to
themselves convert from alsa_ticks to microseconds using a
multiplication, than to require other callers (akin to the
only current non-zero one) to convert from microseconds to
alsa_ticks using a division. For me, the winners are both
imposing the simpler operation (multiplication) and having
an argument with more precision (while still leaving room
for a value thousands of seconds long to be specified)
that is less inducive to cumulative loss of precision.
Yes, I know the division is combined and would be there
anyway in the only current non-zero use, but in general
it might be otherwise (for other future non-zero uses).
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-04 5:26 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-04 5:26 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-04-2005 06:26 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-04-05 04:46
----------------------------------------------------------------------
I've though about it some more, and here's my
recommendation, FWIW.
In the event that the units of kernel_ticks and alsa_ticks
are close to one another, but not quite equal, having a
snd_pcm_tick_set() that accepts alsa_ticks and converts
them to kernel_ticks might result in an important
cumulative loss of precision (given that the only current
non-zero usage, the one at hand in snd_pcm_tick_prepare(),
started from a number of samples [aka frames]).
So I would advocate leaving the current implementation
of snd_pcm_tick_set() and snd_pcm_system_tick_set()
as it is, but renaming them to snd_pcm_timer_set() and
snd_pcm_system_timer_set() and renaming their "ticks"
argument to "microseconds". Thus revising the documented
design, as opposed to the implementation.
Assuming some future non-zero usages are to come, it
think it's actually better to require some callers to
themselves convert from alsa_ticks to microseconds using a
multiplication, than to require other callers (akin to the
only current non-zero one) to convert from microseconds to
alsa_ticks using a division. For me, the winners are both
imposing the simpler operation (multiplication) and having
an argument with more precision (while still leaving room
for a value thousands of seconds long to be specified)
that is less inducive to cumulative loss of precision.
Yes, I know the division is combined and would be there
anyway in the only current non-zero use, but in general
it might be otherwise (for other future non-zero uses).
----------------------------------------------------------------------
charles_levert - 03-04-05 06:26
----------------------------------------------------------------------
> * Xrun debugging was dropped. Why?
>
> This is kind of a hack anyway. There are much, much
> cleaner ways to debug this now. Just my $0.02.
My initial motivation for removing it altogether was the
result of an error on my part. It's now back.
Can you elaborate on the two points in your statement?
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-04 10:00 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-04 10:00 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-04-2005 11:00 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-04-05 06:26
----------------------------------------------------------------------
> * Xrun debugging was dropped. Why?
>
> This is kind of a hack anyway. There are much, much
> cleaner ways to debug this now. Just my $0.02.
My initial motivation for removing it altogether was the
result of an error on my part. It's now back.
Can you elaborate on the two points in your statement?
----------------------------------------------------------------------
perex - 03-04-05 11:00
----------------------------------------------------------------------
Yes, my fault, I overlooked the conversion from microseconds to ticks in
snd_pcm_system_tick_set().
Renaming snd_pcm_tick_set() to snd_pcm_timer_set() and renaming argument
from "ticks" to "us" seems like a good idea to me. Could you provide a
patch including your fix, please? I will apply it
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
03-04-05 11:00 perex Note Added: 0003799
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-04 10:57 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-04 10:57 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-04-2005 11:57 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
perex - 03-04-05 11:01
----------------------------------------------------------------------
Yes, my fault, I overlooked the conversion from microseconds to ticks in
snd_pcm_system_tick_set().
Renaming snd_pcm_tick_set() to snd_pcm_timer_set() and renaming argument
from "ticks" to "us" seems like a good idea to me. Could you provide a
patch including your fix, please? I will apply it immediately. Thanks.
----------------------------------------------------------------------
charles_levert - 03-04-05 11:57
----------------------------------------------------------------------
There you go: "timer_usec.0b05.patch".
This includes the same changes as
"rate-sleep-ticks.0b03x.patch" with the new names, since
they are a consequence of moving to a microsecond calling
convention.
I used "usec" instead of "us", although "s" is the standard
SI symbol, because it's less ambiguous while still short.
It's also what's used in "struct timeval" ("tv_usec").
I produced the patch against CVS and tweaked it by hand
since my main patch also changed neighboring lines, but I
successfully tested it with "patch --dry-run" against CVS,
so it should work.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
03-04-05 11:00 perex Note Added: 0003799
03-04-05 11:01 perex Note Edited: 0003799
03-04-05 11:47 charles_levert File Added: timer_usec.0b05.patch
03-04-05 11:57 charles_levert Note Added: 0003800
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-17 23:07 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-17 23:07 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-18-2005 00:07 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-04-05 11:57
----------------------------------------------------------------------
There you go: "timer_usec.0b05.patch".
This includes the same changes as
"rate-sleep-ticks.0b03x.patch" with the new names, since
they are a consequence of moving to a microsecond calling
convention.
I used "usec" instead of "us", although "s" is the standard
SI symbol, because it's less ambiguous while still short.
It's also what's used in "struct timeval" ("tv_usec").
I produced the patch against CVS and tweaked it by hand
since my main patch also changed neighboring lines, but I
successfully tested it with "patch --dry-run" against CVS,
so it should work.
----------------------------------------------------------------------
charles_levert - 03-18-05 00:07
----------------------------------------------------------------------
No activity on this issue in almost two weeks!
Is everybody involved still following and in
good health?
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
03-04-05 11:00 perex Note Added: 0003799
03-04-05 11:01 perex Note Edited: 0003799
03-04-05 11:47 charles_levert File Added: timer_usec.0b05.patch
03-04-05 11:57 charles_levert Note Added: 0003800
03-18-05 00:07 charles_levert Note Added: 0003987
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2005-03-18 17:12 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2005-03-18 17:12 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-18-2005 18:12 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
charles_levert - 03-18-05 00:07
----------------------------------------------------------------------
No activity on this issue in almost two weeks!
Is everybody involved still following and in
good health?
----------------------------------------------------------------------
tiwai - 03-18-05 18:12
----------------------------------------------------------------------
IMO, snd_pcm_timer_set() is confusing with the functions in pcm_timer.c.
Also, if we use snd_pcm_timer_set(), we should rename
snd_pcm_tick_prepare(), too.
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
03-04-05 11:00 perex Note Added: 0003799
03-04-05 11:01 perex Note Edited: 0003799
03-04-05 11:47 charles_levert File Added: timer_usec.0b05.patch
03-04-05 11:57 charles_levert Note Added: 0003800
03-18-05 00:07 charles_levert Note Added: 0003987
03-18-05 18:12 tiwai Note Added: 0003998
======================================================================
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
^ permalink raw reply [flat|nested] 22+ messages in thread
* [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations
@ 2006-03-22 1:51 bugtrack
0 siblings, 0 replies; 22+ messages in thread
From: bugtrack @ 2006-03-22 1:51 UTC (permalink / raw)
To: alsa-devel
A NOTE has been added to this issue.
======================================================================
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=951>
======================================================================
Reported By: charles_levert
Assigned To:
======================================================================
Project: ALSA - driver
Issue ID: 951
Category: CORE - pcm
Reproducibility: always
Severity: major
Priority: normal
Status: new
Distribution:
Kernel Version: 2.4.22
======================================================================
Date Submitted: 02-28-2005 21:26 CET
Last Modified: 03-22-2006 02:51 CET
======================================================================
Summary: Fix bugs with early IRQs mishandling, missing
silence fill, and wrong timing calculations
Description:
A patch is forthcoming.
Here is the text from the patch preamble.
This patch is against alsa-driver-1.0.8.
It covers:
alsa-kernel/core/pcm_lib.c
alsa-kernel/include/pcm.h
alsa-kernel/core/pcm_native.c
alsa-kernel/core/oss/pcm_oss.c
The main changes are in the first listed file.
An original bug description was filed
under "[ALSA - driver] PCI - cmipci":
<https://bugtrack.alsa-project.org/alsa-bug/view.php?id=946>
since the problems were first observed on this hardware.
The patch fixes the following problems:
-- Properly handle early IRQs in
snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c.
This does occur, at least with CMI8738MC6 hardware.
This bug had a major impact that caused the DMAs
to run for too long and irritating noise to be
heard at the end of playback.
-- Properly compute estimated time of next interrupt
in snd_pcm_update_hw_ptr_interrupt() in pcm_lib.c;
this is no longer done with a
"... % runtime->period_size" kludge (which didn't
even attempt to round to nearest instead of merely
truncating).
-- Test whether hw_avail is big enough to handle
the next runtime->period_size in
snd_pcm_update_hw_ptr_post() in pcm_lib.c,
otherwise call snd_pcm_tick_prepare() to
tentatively set up a timer if needed (to be able
to take earlier action in that case).
-- Centralize all folding of values with regard to
runtime->boundary in the new snd_pcm_next_buf()
function in pcm_lib.c.
All relevant values are now systematically in
coherent synchronization without resorting to the
ugly and error-prone "value %= runtime->boundary"
kludge all over the place; unobvious wrapping
conditions testing is no longer needed in these
places.
A safe zone worth one runtime->buffer_size is now
kept on the left side of the operating interval,
just like it was the case for the right side;
this is necessary for proper simplified operation.
-- Direct manipulations of values with respect to
runtime->boundary needed to be brought in line
with new proper practices (and thus simplified)
in pcm.h, pcm_lib.c, pcm_native.c, and pcm_oss.c.
-- Isolate the second part of
snd_pcm_playback_silence() in the new
snd_pcm_playback_silence_fill() function in
pcm_lib.c.
This makes the code more readable since the
task this performs should not be confused with
what is done in the remaining first part of
snd_pcm_playback_silence().
The isolated code is unchanged.
-- Rewrite the remaining first part of
snd_pcm_playback_silence() to properly handle all
situations, including computing minimum silence
insertion even when "runtime->silence_size == 0".
This is needed for proper operation of the aplay
program, for one, and is much more forgiving on the
user; insertion of silence samples in user space
was always useless and a wrong solution anyway (but
user space specification of kernel space silence
insertion is useful and still very much supported).
The code is now much more straightforward to read.
Updates of runtime->control->appl_ptr (due
to the user writing samples) and updates to
runtime->status->hw_ptr (due to the audio hardware
consuming samples) are now _both_ properly
accounted for in all cases; the new code makes
this much more obvious.
-- All calls to snd_pcm_playback_silence() in
pcm_lib.c and pcm_native.c must no longer be under
the "runtime->silence_size > 0" condition.
-- Properly perform rounded-up integer division in
snd_pcm_system_tick_set() in pcm_lib.c.
This only had a minor impact.
-- Properly compute distance to next interrupt in
snd_pcm_tick_prepare() in pcm_lib.c; again, avoid
using any "... % runtime->period_size" kludge.
-- The usage of runtime->rate in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the timer was systematically
set to 1 single tick when it needed to be set to
typically hundreds of ticks.
-- The usage of runtime->sleep_min in
snd_pcm_tick_prepare() in pcm_lib.c was incorrect,
regarless of other changes.
The impact was that the number of ticks for
the timer was not properly adjusted up to what
sleep_min is documented to mean, when needed.
======================================================================
Relationships ID Summary
----------------------------------------------------------------------
related to 0000946 Early playback DMA IRQ with CMI8738MC6 ...
======================================================================
----------------------------------------------------------------------
tiwai - 03-18-05 18:12
----------------------------------------------------------------------
IMO, snd_pcm_timer_set() is confusing with the functions in pcm_timer.c.
Also, if we use snd_pcm_timer_set(), we should rename
snd_pcm_tick_prepare(), too.
----------------------------------------------------------------------
rlrevell - 03-22-06 02:51
----------------------------------------------------------------------
What is the status of this bug?
Issue History
Date Modified Username Field Change
======================================================================
02-28-05 21:26 charles_levert New Issue
02-28-05 21:26 charles_levert Kernel Version => 2.4.22
02-28-05 21:29 charles_levert File Added: early-irqs.patch
03-01-05 09:36 perex Relationship added related to 0000946
03-01-05 09:40 perex Note Added: 0003755
03-01-05 11:15 charles_levert Note Added: 0003757
03-02-05 09:22 Clemens LadischNote Added: 0003761
03-02-05 13:01 charles_levert Note Added: 0003768
03-02-05 13:39 perex Note Added: 0003771
03-02-05 14:53 charles_levert Note Added: 0003776
03-03-05 14:21 charles_levert File Added: hz-int-div.0b03.patch
03-03-05 14:22 charles_levert File Added: rate-sleep-ticks.0b03.patch
03-03-05 14:23 charles_levert File Added: appl_ptr-boundary.0b03.patch
03-03-05 14:23 charles_levert File Added: early-irqs.0b03.patch
03-03-05 14:37 charles_levert Note Added: 0003786
03-03-05 15:03 perex Note Added: 0003787
03-03-05 15:33 charles_levert File Added: rate-sleep-ticks.0b03x.patch
03-03-05 15:34 charles_levert Note Added: 0003788
03-03-05 15:52 perex Note Added: 0003789
03-03-05 16:05 perex Note Added: 0003790
03-03-05 20:30 rlrevell Note Added: 0003792
03-04-05 03:34 charles_levert Note Added: 0003795
03-04-05 04:46 charles_levert Note Added: 0003797
03-04-05 06:26 charles_levert Note Added: 0003798
03-04-05 11:00 perex Note Added: 0003799
03-04-05 11:01 perex Note Edited: 0003799
03-04-05 11:47 charles_levert File Added: timer_usec.0b05.patch
03-04-05 11:57 charles_levert Note Added: 0003800
03-18-05 00:07 charles_levert Note Added: 0003987
03-18-05 18:12 tiwai Note Added: 0003998
03-22-06 02:51 rlrevell Note Added: 0008788
======================================================================
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2006-03-22 1:51 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-02 8:22 [ALSA - driver 0000951]: Fix bugs with early IRQs mishandling, missing silence fill, and wrong timing calculations bugtrack
-- strict thread matches above, loose matches on Subject: below --
2006-03-22 1:51 bugtrack
2005-03-18 17:12 bugtrack
2005-03-17 23:07 bugtrack
2005-03-04 10:57 bugtrack
2005-03-04 10:00 bugtrack
2005-03-04 5:26 bugtrack
2005-03-04 3:46 bugtrack
2005-03-04 2:34 bugtrack
2005-03-03 19:30 bugtrack
2005-03-03 15:05 bugtrack
2005-03-03 14:52 bugtrack
2005-03-03 14:34 bugtrack
2005-03-03 14:03 bugtrack
2005-03-03 13:37 bugtrack
2005-03-02 13:53 bugtrack
2005-03-02 12:39 bugtrack
2005-03-02 12:01 bugtrack
2005-03-01 10:15 bugtrack
2005-03-01 8:40 bugtrack
2005-03-01 8:36 bugtrack
2005-02-28 20:26 bugtrack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox