[haiku-bugs] Re: [Haiku] #7285: BSoundPlayer sends buffers late, with ever-increasing latency

From: "bonefish" <trac@xxxxxxxxxxxx>
Date: Tue, 03 May 2011 14:16:19 -0000
#7285: BSoundPlayer sends buffers late, with ever-increasing latency
------------------------------+-----------------------
   Reporter:  Pete            |      Owner:  axeld
       Type:  bug             |     Status:  new
   Priority:  normal          |  Milestone:  R1
  Component:  Kits/Media Kit  |    Version:  R1/alpha2
 Resolution:                  |   Keywords:
 Blocked By:                  |   Blocking:
Has a Patch:  1               |   Platform:  All
------------------------------+-----------------------

Comment (by bonefish):

 Replying to [comment:11 Pete]:
 > '''MediaEventLooper''':[[BR]]
 > In the original, the latency was ''subtracted'' from "now" before
 checking against the first event start time, so that could never be seen
 as late!

 Good catch!

 > Latency is now added to the current time as it should be.
 > (The actual calculation of "lateness" was at one point moved to that
 point at the beginning of the loop, but it seems better that the true
 lateness when the buffer is returned from the loop be provided, so this
 check is still done after wakeup, as it was originally.)

 Due to the violation of the seamless latency computation it is still
 incorrect, though. If the behavior is worse with the correct
 implementation, there's obviously something else wrong. Since after with
 your changes the mixer completely ignores the value computed here, the
 issue is probably in a different node up- or downstream.

 > '''AudioMixer''':[[BR]]
 > Checking the lateness of incoming buffers -- and notifying the producer
 -- is now done in BufferReceived, rather than when the buffer is returned
 from the loop in HandleInputBuffer.

 Well, that is a work-around for the incorrect lateness computation in
 `BMediaEventLooper::ControlLoop()`.

 > There is a new test in the latter for actual lateness ( wrt downstream
 latency -- ''not'' the total event latency) which will adjust the Mixer's
 internal latency.

 While it doesn't harm, I don't think this is necessary either. I would
 rather see the lateness be computed correctly in `BMediaEventLooper` and
 handled here instead of in `BufferReceived()`. That only leaves the wait
 itself, which should be covered by the scheduling latency. Though I
 wouldn't bother adjusting it, but rather let the internal latency
 compensate. The internal latency could be checked after the mixing has
 happened (i.e. whether the lateness after mixing is greater than the
 greatest lateness of the processed input buffers (using the lateness value
 computed by `BMediaEventLooper`)). Doing that isn't strictly necessary
 either, since we should get a late notice when the buffer arrives late
 downstream.

 > LateNoticeReceived will also adjust internal latency, but as far as I
 can tell is never invoked!

 That's bad and should be fixed.

 > In all cases, lateness is only acted upon if it is an ''initial''
 report, as multiple buffers following in the queue will also be flagged
 late.  The fLastNotification variable has been redirected for this
 purpose, as its original use in LateNoticeReceived seemed to be pointless.

 The purpose of `fLastLateNotification` is to prevent a reaction to a late
 notice before the latency adjustment caused by the previous late notice
 could have taken effect. Looking at the code I'd say the value it is
 currently set to (`TimeSource()->Now()`) should be increased by the
 downstream and internal latency, since any buffer that already has been or
 is currently being processed won't benefit from the latency change. So in
 that your change is correct, but the respective change in
 `HandleInputBuffer()` seems superfluous at best.

 Regarding your comment in `BufferReceived()`:
 {{{
         312     // I believe this is pointless here, because the relevant
 run mode is the source, mot the mixer!
         313     //      if (RunMode() == B_DROP_DATA || RunMode() ==
 B_DECREASE_PRECISION
         314     //          || RunMode() == B_INCREASE_LATENCY) {
 }}}
 Your assumption is not correct. Whether the mixer needs the buffers in
 time depends on the mixer's run mode, not that of its source. Whether the
 source chooses to ignore the notice is another matter.

 Regarding coding style: The assignment operator should be treated like any
 other operator when wrapping the line (i.e. break before). Local variable
 names are camel-case.

-- 
Ticket URL: <http://dev.haiku-os.org/ticket/7285#comment:12>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
[haiku-bugs] Re: [Haiku] #7285: BSoundPlayer sends buffers late, with ever-increasing latency

Other related posts:
