From: Jonathan W. <jw...@ph...> - 2007-07-17 00:36:28
|
I'm now starting to feel that I'm making some headway into getting the MOTU fully functional under FFADO once more. There are still some startup issues I need to work through, but if it survived startup I was able to get glitch-free playback from the MOTU overnight. Unfortunately the key changes needed to make this happen are not in the MOTU stream processor but rather in the basic ffado infrastructure. Therefore I am interested in ideas as to how this can be resolved. The first of these I've mentioned before: the MOTU needs a ENABLE_DELAY_CYCLES (in StreamProcessorManager.cpp) somewhat larger than the current value of 100. As I've mentioned in the past this was reduced from 2000 to 100 around svn rev 435, but 100 doesn't give the DLL enough time to stabilise in the case of the MOTU. I'm still thinking about how this can be dealt with. At the moment I'm considering a new StreamProcessor method which returns its preferred ENABLE_DELAY_CYCLES value. The StreamProcessorManager can then call this function for every registered stream processor object and choose the largest one. That way interfaces which can get away with small delays won't be penalised by others which require longer startup delays (unless of course a longer-delay unit is used together with the short-delay unit). In the absence of howls of protest I may implement this idea later this week. Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect on the fine-grained timestamps being sent to the MOTU and therefore does nothing to fix the audio glitches I've been hearing. To fix these I found I needed to make a change to TimestampedBuffer::incrementFrameCounter() in TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the timestamp value estimated by the DLL in its previous update cycle. This in turn means that the buffer's "tail" timestamp is always slightly different to the timestamp passed in to incrementFrameCounter(). The difference might be subtle but it's clearly enough to upset the MOTU interface (which as I've found in the past is extremely sensitive to timestamp variations). If instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the audio glitches are no longer produced by the MOTU. This change does not alter the DLL used to produce and maintain m_dll_e2 which in turn is used to estimate the effective audio rate - and hense the per-sample timestamps within a packet being sent to the MOTU. It does however use exact timestamps from the MOTU to maintain m_buffer_tail_timestamp - this is no longer a DLL. This is a significant change from what is done currently and may upset other interfaces (although at the moment I can't see why m_buffer_tail_timestamp can't simply adopt the incoming timestamp). Assuming the current DLL control of m_buffer_tail_timestamp is required for other interfaces, the only way I can see this working is to introduce some option flags to TimestampedBuffer and define one to switch between these two alternative ways of maintaining TimestampedBuffer::m_buffer_tail_timestamp. The default behaviour can remain as is, and those stream processors which need the other mode can switch it as necessary. Again, unless there are objections (or I discover something else which renders this unnecessary) I intend to code something like this up over the next week. As always, thoughts and ideas would be more than welcome. Regards jonathan |
From: <gr...@gm...> - 2007-07-17 13:43:06
|
On 7/17/07, Jonathan Woithe <jw...@ph...> wrote: > The first of these I've mentioned before: the MOTU needs a > ENABLE_DELAY_CYCLES (in StreamProcessorManager.cpp) somewhat larger than the > current value of 100. As I've mentioned in the past this was reduced from > 2000 to 100 around svn rev 435, but 100 doesn't give the DLL enough time to > stabilise in the case of the MOTU. I'm still thinking about how this can be > dealt with. At the moment I'm considering a new StreamProcessor method > which returns its preferred ENABLE_DELAY_CYCLES value. The > StreamProcessorManager can then call this function for every registered > stream processor object and choose the largest one. That way interfaces > which can get away with small delays won't be penalised by others which > require longer startup delays (unless of course a longer-delay unit is used > together with the short-delay unit). In the absence of howls of protest I > may implement this idea later this week. I don't have any MOTU hardware here. but it always sounds very harsh to me - when someone depends _just_ on timeouts. Are you certain, there's no other way here ? -- GJ |
From: Jonathan W. <jw...@ph...> - 2007-07-18 02:55:20
|
> On 7/17/07, Jonathan Woithe <jw...@ph...> wrote: > > The first of these I've mentioned before: the MOTU needs a > > ENABLE_DELAY_CYCLES (in StreamProcessorManager.cpp) somewhat larger than the > > current value of 100. As I've mentioned in the past this was reduced from > > 2000 to 100 around svn rev 435, but 100 doesn't give the DLL enough time to > > stabilise in the case of the MOTU. I'm still thinking about how this can be > > dealt with. At the moment I'm considering a new StreamProcessor method > > which returns its preferred ENABLE_DELAY_CYCLES value. The > > StreamProcessorManager can then call this function for every registered > > stream processor object and choose the largest one. That way interfaces > > which can get away with small delays won't be penalised by others which > > require longer startup delays (unless of course a longer-delay unit is used > > together with the short-delay unit). In the absence of howls of protest I > > may implement this idea later this week. > I don't have any MOTU hardware here. but it always sounds very harsh > to me - when someone depends _just_ on timeouts. Are you certain, > there's no other way here ? What we're talking about here isn't a timeout. It's the length of time the sync source stream processor requires to ensure its software DLL is solidly locked to the audio clock using timestamp information being supplied to it via the firewire datastream. The time taken is a function of many things including the DLL implementation, the nature of the clock being locked to, the properties of the timestamp information available and the update frequency of the timestamp updates. Most of these details are different for different interfaces which is why it can happen that with some interfaces DLL lock can occur much quicker than with others. To be honest I'm not certain that there's no other way aside from a DLL to keep sync with the MOTU. However, based on observation its probably the most memory-efficient way. There might well be another trick we could use but it most likely requires in-depth knowledge about the MOTU's internal functionality, and unfortunately MOTU flatly refuse to help us out in this regard. Therefore we have to make do with what we can deduce. Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-18 15:12:38
|
Jonathan Woithe wrote: > I'm now starting to feel that I'm making some headway into getting the MOTU > fully functional under FFADO once more. There are still some startup issues > I need to work through, but if it survived startup I was able to get > glitch-free playback from the MOTU overnight. > > Unfortunately the key changes needed to make this happen are not in the MOTU > stream processor but rather in the basic ffado infrastructure. Therefore I > am interested in ideas as to how this can be resolved. > > The first of these I've mentioned before: the MOTU needs a > ENABLE_DELAY_CYCLES (in StreamProcessorManager.cpp) somewhat larger than the > current value of 100. As I've mentioned in the past this was reduced from > 2000 to 100 around svn rev 435, but 100 doesn't give the DLL enough time to > stabilise in the case of the MOTU. I'm still thinking about how this can be > dealt with. At the moment I'm considering a new StreamProcessor method > which returns its preferred ENABLE_DELAY_CYCLES value. The > StreamProcessorManager can then call this function for every registered > stream processor object and choose the largest one. That way interfaces > which can get away with small delays won't be penalised by others which > require longer startup delays (unless of course a longer-delay unit is used > together with the short-delay unit). In the absence of howls of protest I > may implement this idea later this week. > > Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; > this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect > on the fine-grained timestamps being sent to the MOTU and therefore does > nothing to fix the audio glitches I've been hearing. To fix these I found I > needed to make a change to TimestampedBuffer::incrementFrameCounter() in > TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the > timestamp value estimated by the DLL in its previous update cycle. This in > turn means that the buffer's "tail" timestamp is always slightly different > to the timestamp passed in to incrementFrameCounter(). The difference might > be subtle but it's clearly enough to upset the MOTU interface (which as I've > found in the past is extremely sensitive to timestamp variations). If > instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the > audio glitches are no longer produced by the MOTU. > > This change does not alter the DLL used to produce and maintain m_dll_e2 > which in turn is used to estimate the effective audio rate - and hense the > per-sample timestamps within a packet being sent to the MOTU. It does > however use exact timestamps from the MOTU to maintain > m_buffer_tail_timestamp - this is no longer a DLL. This is a significant > change from what is done currently and may upset other interfaces (although > at the moment I can't see why m_buffer_tail_timestamp can't simply adopt > the incoming timestamp). Assuming the current DLL control of > m_buffer_tail_timestamp is required for other interfaces, the only way I can > see this working is to introduce some option flags to TimestampedBuffer and > define one to switch between these two alternative ways of maintaining > TimestampedBuffer::m_buffer_tail_timestamp. The default behaviour can > remain as is, and those stream processors which need the other mode can > switch it as necessary. > > Again, unless there are objections (or I discover something else which > renders this unnecessary) I intend to code something like this up over the > next week. As always, thoughts and ideas would be more than welcome. I'm sorry for the slow response on my part. I've been out on vacation for a few days, and I left all work at home. I'm going to reread all your emails tomorrow and think about it a while. Meanwhile feel free to code whatever works for you. The m_buffer_tail_timestamp is set to the predicted timestamp in order to ensure that the timestamps are monotonic. This should not be a problem. I think we might need a higher order DLL or a lower bandwidth DLL to cope with the MOTU's situation. I also think that even for the other devices there are some issues regarding the current DLL implementation. They are just a little less sensitive to this. The jumps between different values you describe in another mail are not really normal. It might also be some sort of integer-float issue somewhere. I'll try and figure things out over the next days. Greets, Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-19 00:07:25
|
Hi Pieter > > Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; > > this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect > > on the fine-grained timestamps being sent to the MOTU and therefore does > > nothing to fix the audio glitches I've been hearing. To fix these I found I > > needed to make a change to TimestampedBuffer::incrementFrameCounter() in > > TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the > > timestamp value estimated by the DLL in its previous update cycle. This in > > turn means that the buffer's "tail" timestamp is always slightly different > > to the timestamp passed in to incrementFrameCounter(). The difference might > > be subtle but it's clearly enough to upset the MOTU interface (which as I've > > found in the past is extremely sensitive to timestamp variations). If > > instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the > > audio glitches are no longer produced by the MOTU. > > > > This change does not alter the DLL used to produce and maintain m_dll_e2 > > which in turn is used to estimate the effective audio rate - and hense the > > per-sample timestamps within a packet being sent to the MOTU. It does > > however use exact timestamps from the MOTU to maintain > > m_buffer_tail_timestamp - this is no longer a DLL. This is a significant > > change from what is done currently and may upset other interfaces (although > > at the moment I can't see why m_buffer_tail_timestamp can't simply adopt > > the incoming timestamp). Assuming the current DLL control of > > m_buffer_tail_timestamp is required for other interfaces, the only way I can > > see this working is to introduce some option flags to TimestampedBuffer and > > define one to switch between these two alternative ways of maintaining > > TimestampedBuffer::m_buffer_tail_timestamp. The default behaviour can > > remain as is, and those stream processors which need the other mode can > > switch it as necessary. > > > > Again, unless there are objections (or I discover something else which > > renders this unnecessary) I intend to code something like this up over the > > next week. As always, thoughts and ideas would be more than welcome. > > I'm sorry for the slow response on my part. I've been out on vacation > for a few days, and I left all work at home. Half your luck. I'd do the same. > I'm going to reread all your emails tomorrow and think about it a while. > Meanwhile feel free to code whatever works for you. Ok. > The m_buffer_tail_timestamp is set to the predicted timestamp in order > to ensure that the timestamps are monotonic. This should not be a > problem. I think we might need a higher order DLL or a lower bandwidth > DLL to cope with the MOTU's situation. I also think that even for the > other devices there are some issues regarding the current DLL > implementation. They are just a little less sensitive to this. Since the exact timestamp of the last sample is coming from the actual device (at least it is in the case of the MOTU) there's no possibility of it ever being non-monotonic except of course at wraparound. I think other interfaces only send a single timestamp per packet though, so maybe with these you can go non-monotonic due to the predictive nature of the "last" timestamp. Even then though I can't immediately see how. My gut feeling is that even a higher-order or lower bandwidth DLL may not help unless the MOTU can set m_buffer_tail_timestamp to the exact incoming timestamp. What I do know is that the MOTU is very sensitive to glitches in the timestamps being sent to it and it doesn't take much to upset it. DLLs are pretty good, but in simulations I did a few months back it was clear that a DLL will tend to "lag" reality in terms of *cumulative* summing errors. While many interfaces possibly just use relative timing between two adjacent timestamps, the MOTU does seem to accumulate timing information over a longer time. It seems that if its accumulated absolute clock drifts away from that of the incoming timestamps then you get glitches at best or (in the worst case) a complete loss of sync (and hense audio sensible output). > The jumps between different values you describe in another mail are not > really normal. It might also be some sort of integer-float issue > somewhere. Yes, I thought of that but at first glance couldn't see an obvious problematic location. At this stage my money's on the accumulated drift which tends to occur within the DLL unless m_buffer_tail_timestamp is set to the exact incoming timestamp. However, I still have more investigations to do. Regards jonathan |
From: Daniel W. <wa...@mo...> - 2007-07-19 06:47:19
|
> Since the exact timestamp of the last sample is coming from the actual > device (at least it is in the case of the MOTU) there's no possibility of it > ever being non-monotonic except of course at wraparound. I think other > interfaces only send a single timestamp per packet though, so maybe with > these you can go non-monotonic due to the predictive nature of the "last" > timestamp. Even then though I can't immediately see how. In case of a BeBoB device, the diff between two timestamps is monotonic increasing (with wrap). This depends also on the synchronization, e.g. sync on SPDIF or on CSP. There is only one timestamp for each packet and therefore I would assume that the prediction for each sample timestamp and consequently the diffs adhere to the same behavior as the diffs between the packet timestamps, no? Hmm, thinking a bit more on the (packet) timestamps, they are strictly monotonic increasing (with wrap). Sometimes it is really good to know the behavior of hardware :) daniel |
From: Jonathan W. <jw...@ph...> - 2007-07-19 07:00:43
|
> > Since the exact timestamp of the last sample is coming from the actual > > device (at least it is in the case of the MOTU) there's no possibility of it > > ever being non-monotonic except of course at wraparound. I think other > > interfaces only send a single timestamp per packet though, so maybe with > > these you can go non-monotonic due to the predictive nature of the "last" > > timestamp. Even then though I can't immediately see how. > > In case of a BeBoB device, the diff between two timestamps is monotonic > increasing (with wrap). This depends also on the synchronization, e.g. > sync on SPDIF or on CSP. > > There is only one timestamp for each packet and therefore I would assume > that the prediction for each sample timestamp and consequently the diffs > adhere to the same behavior as the diffs between the packet timestamps, no? I'd have thought so. > Hmm, thinking a bit more on the (packet) timestamps, they are strictly > monotonic increasing (with wrap). Sometimes it is really good to know > the behavior of hardware :) Indeed. So if this is the case then is there really any reason to not use the actual timestamp when setting m_buffer_tail_timestamp in TimestampedBuffer::incrementFrameCounter()? As far as I can tell this should result in more accurate timestamps being delivered, with the DLL still being used for rate estimation. Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-21 16:18:08
|
Jonathan Woithe wrote: > Hi Pieter > >>> Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; >>> this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect >>> on the fine-grained timestamps being sent to the MOTU and therefore does >>> nothing to fix the audio glitches I've been hearing. To fix these I found I >>> needed to make a change to TimestampedBuffer::incrementFrameCounter() in >>> TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the >>> timestamp value estimated by the DLL in its previous update cycle. This in >>> turn means that the buffer's "tail" timestamp is always slightly different >>> to the timestamp passed in to incrementFrameCounter(). The difference might >>> be subtle but it's clearly enough to upset the MOTU interface (which as I've >>> found in the past is extremely sensitive to timestamp variations). If >>> instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the >>> audio glitches are no longer produced by the MOTU. >>> >>> This change does not alter the DLL used to produce and maintain m_dll_e2 >>> which in turn is used to estimate the effective audio rate - and hense the >>> per-sample timestamps within a packet being sent to the MOTU. It does >>> however use exact timestamps from the MOTU to maintain >>> m_buffer_tail_timestamp - this is no longer a DLL. This is a significant >>> change from what is done currently and may upset other interfaces (although >>> at the moment I can't see why m_buffer_tail_timestamp can't simply adopt >>> the incoming timestamp). Assuming the current DLL control of >>> m_buffer_tail_timestamp is required for other interfaces, the only way I can >>> see this working is to introduce some option flags to TimestampedBuffer and >>> define one to switch between these two alternative ways of maintaining >>> TimestampedBuffer::m_buffer_tail_timestamp. The default behaviour can >>> remain as is, and those stream processors which need the other mode can >>> switch it as necessary. >>> >>> Again, unless there are objections (or I discover something else which >>> renders this unnecessary) I intend to code something like this up over the >>> next week. As always, thoughts and ideas would be more than welcome. >> I'm sorry for the slow response on my part. I've been out on vacation >> for a few days, and I left all work at home. > > Half your luck. I'd do the same. > >> I'm going to reread all your emails tomorrow and think about it a while. >> Meanwhile feel free to code whatever works for you. > > Ok. > >> The m_buffer_tail_timestamp is set to the predicted timestamp in order >> to ensure that the timestamps are monotonic. This should not be a >> problem. I think we might need a higher order DLL or a lower bandwidth >> DLL to cope with the MOTU's situation. I also think that even for the >> other devices there are some issues regarding the current DLL >> implementation. They are just a little less sensitive to this. > > Since the exact timestamp of the last sample is coming from the actual > device (at least it is in the case of the MOTU) there's no possibility of it > ever being non-monotonic except of course at wraparound. I think other > interfaces only send a single timestamp per packet though, so maybe with > these you can go non-monotonic due to the predictive nature of the "last" > timestamp. Even then though I can't immediately see how. > > My gut feeling is that even a higher-order or lower bandwidth DLL may not > help unless the MOTU can set m_buffer_tail_timestamp to the exact incoming > timestamp. What I do know is that the MOTU is very sensitive to glitches in > the timestamps being sent to it and it doesn't take much to upset it. DLLs > are pretty good, but in simulations I did a few months back it was clear > that a DLL will tend to "lag" reality in terms of *cumulative* summing > errors. While many interfaces possibly just use relative timing between two > adjacent timestamps, the MOTU does seem to accumulate timing information > over a longer time. It seems that if its accumulated absolute clock drifts > away from that of the incoming timestamps then you get glitches at best or > (in the worst case) a complete loss of sync (and hense audio sensible > output). > >> The jumps between different values you describe in another mail are not >> really normal. It might also be some sort of integer-float issue >> somewhere. > > Yes, I thought of that but at first glance couldn't see an obvious > problematic location. At this stage my money's on the accumulated drift > which tends to occur within the DLL unless m_buffer_tail_timestamp is set > to the exact incoming timestamp. However, I still have more investigations > to do. can you replace: return m_dll_e2/((float) m_update_period); with: return ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) m_update_period); in TimestampedBuffer::getRate() to see whether things work somewhat better? There is a significant offset in the rate calculation. Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-22 23:41:53
|
Hi Pieter > >>> Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; > >>> this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect > >>> on the fine-grained timestamps being sent to the MOTU and therefore does > >>> nothing to fix the audio glitches I've been hearing. To fix these I found I > >>> needed to make a change to TimestampedBuffer::incrementFrameCounter() in > >>> TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the > >>> timestamp value estimated by the DLL in its previous update cycle. This in > >>> turn means that the buffer's "tail" timestamp is always slightly different > >>> to the timestamp passed in to incrementFrameCounter(). The difference might > >>> be subtle but it's clearly enough to upset the MOTU interface (which as I've > >>> found in the past is extremely sensitive to timestamp variations). If > >>> instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the > >>> audio glitches are no longer produced by the MOTU. > >>> > : > >> The m_buffer_tail_timestamp is set to the predicted timestamp in order > >> to ensure that the timestamps are monotonic. This should not be a > >> problem. I think we might need a higher order DLL or a lower bandwidth > >> DLL to cope with the MOTU's situation. I also think that even for the > >> other devices there are some issues regarding the current DLL > >> implementation. They are just a little less sensitive to this. > : > >> The jumps between different values you describe in another mail are not > >> really normal. It might also be some sort of integer-float issue > >> somewhere. > > > > Yes, I thought of that but at first glance couldn't see an obvious > > problematic location. At this stage my money's on the accumulated drift > > which tends to occur within the DLL unless m_buffer_tail_timestamp is set > > to the exact incoming timestamp. However, I still have more investigations > > to do. > > can you replace: > return m_dll_e2/((float) m_update_period); > > with: > return > ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) > m_update_period); > > in TimestampedBuffer::getRate() to see whether things work somewhat better? > > There is a significant offset in the rate calculation. I'll give it a go tonight and let you know. I assume this is done after restoring the m_buffer_tail_timestamp assignment to the original (predicted) value in incrementFrameCounter(). Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-23 20:34:12
|
Pieter Palmers wrote: > Jonathan Woithe wrote: >> Hi Pieter >> >>>> Even with a larger ENABLE_DELAY_CYCLES startup is still a bit hit and miss; >>>> this is still being worked on. However, ENABLE_DELAY_CYCLES has no effect >>>> on the fine-grained timestamps being sent to the MOTU and therefore does >>>> nothing to fix the audio glitches I've been hearing. To fix these I found I >>>> needed to make a change to TimestampedBuffer::incrementFrameCounter() in >>>> TimestampedBuffer.cpp. Currently m_buffer_tail_timestamp takes the >>>> timestamp value estimated by the DLL in its previous update cycle. This in >>>> turn means that the buffer's "tail" timestamp is always slightly different >>>> to the timestamp passed in to incrementFrameCounter(). The difference might >>>> be subtle but it's clearly enough to upset the MOTU interface (which as I've >>>> found in the past is extremely sensitive to timestamp variations). If >>>> instead I set m_buffer_tail_timestamp to the incoming timestamp (ts) the >>>> audio glitches are no longer produced by the MOTU. >>>> >>>> This change does not alter the DLL used to produce and maintain m_dll_e2 >>>> which in turn is used to estimate the effective audio rate - and hense the >>>> per-sample timestamps within a packet being sent to the MOTU. It does >>>> however use exact timestamps from the MOTU to maintain >>>> m_buffer_tail_timestamp - this is no longer a DLL. This is a significant >>>> change from what is done currently and may upset other interfaces (although >>>> at the moment I can't see why m_buffer_tail_timestamp can't simply adopt >>>> the incoming timestamp). Assuming the current DLL control of >>>> m_buffer_tail_timestamp is required for other interfaces, the only way I can >>>> see this working is to introduce some option flags to TimestampedBuffer and >>>> define one to switch between these two alternative ways of maintaining >>>> TimestampedBuffer::m_buffer_tail_timestamp. The default behaviour can >>>> remain as is, and those stream processors which need the other mode can >>>> switch it as necessary. >>>> >>>> Again, unless there are objections (or I discover something else which >>>> renders this unnecessary) I intend to code something like this up over the >>>> next week. As always, thoughts and ideas would be more than welcome. >>> I'm sorry for the slow response on my part. I've been out on vacation >>> for a few days, and I left all work at home. >> Half your luck. I'd do the same. >> >>> I'm going to reread all your emails tomorrow and think about it a while. >>> Meanwhile feel free to code whatever works for you. >> Ok. >> >>> The m_buffer_tail_timestamp is set to the predicted timestamp in order >>> to ensure that the timestamps are monotonic. This should not be a >>> problem. I think we might need a higher order DLL or a lower bandwidth >>> DLL to cope with the MOTU's situation. I also think that even for the >>> other devices there are some issues regarding the current DLL >>> implementation. They are just a little less sensitive to this. >> Since the exact timestamp of the last sample is coming from the actual >> device (at least it is in the case of the MOTU) there's no possibility of it >> ever being non-monotonic except of course at wraparound. I think other >> interfaces only send a single timestamp per packet though, so maybe with >> these you can go non-monotonic due to the predictive nature of the "last" >> timestamp. Even then though I can't immediately see how. >> >> My gut feeling is that even a higher-order or lower bandwidth DLL may not >> help unless the MOTU can set m_buffer_tail_timestamp to the exact incoming >> timestamp. What I do know is that the MOTU is very sensitive to glitches in >> the timestamps being sent to it and it doesn't take much to upset it. DLLs >> are pretty good, but in simulations I did a few months back it was clear >> that a DLL will tend to "lag" reality in terms of *cumulative* summing >> errors. While many interfaces possibly just use relative timing between two >> adjacent timestamps, the MOTU does seem to accumulate timing information >> over a longer time. It seems that if its accumulated absolute clock drifts >> away from that of the incoming timestamps then you get glitches at best or >> (in the worst case) a complete loss of sync (and hense audio sensible >> output). >> >>> The jumps between different values you describe in another mail are not >>> really normal. It might also be some sort of integer-float issue >>> somewhere. >> Yes, I thought of that but at first glance couldn't see an obvious >> problematic location. At this stage my money's on the accumulated drift >> which tends to occur within the DLL unless m_buffer_tail_timestamp is set >> to the exact incoming timestamp. However, I still have more investigations >> to do. > > can you replace: > return m_dll_e2/((float) m_update_period); > > with: > return > ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) > m_update_period); > > in TimestampedBuffer::getRate() to see whether things work somewhat better? Things are a bit more complex, because the m_buffer_next_tail_timestamp-m_buffer_tail_timestamp has to be protected by the mutex, otherwise it can result in = 0 because these values are updated from another thread. My current version: float TimestampedBuffer::getRate() { int64_t diff; pthread_mutex_lock(&m_framecounter_lock); diff=m_buffer_next_tail_timestamp - m_buffer_tail_timestamp; pthread_mutex_unlock(&m_framecounter_lock); debugOutput(DEBUG_LEVEL_VERY_VERBOSE,"getRate: %f/%f=%f\n", (float)(diff), (float)m_update_period, ((float)(diff))/((float) m_update_period)); // the maximal difference we can allow (64secs) const int64_t max=m_wrap_at/2; if(diff > max) { diff -= m_wrap_at; } else if (diff < -max) { diff += m_wrap_at; } float rate=((float)diff)/((float) m_update_period); if (fabsf(m_nominal_rate - rate)>(m_nominal_rate*0.1)) { debugWarning("(%p) rate (%10.5f) more that 10%% off nominal (rate=%10.5f, diff=%lld, update_period=%d)\n", this, rate,m_nominal_rate,diff, m_update_period); dumpInfo(); return m_nominal_rate; } else { return rate; } } Probably this way of doing things will cause us some merge issues in the end, but I haven't tested this enough to commit it. Greets, Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-23 23:22:05
|
Hi Pieter > > can you replace: > > return m_dll_e2/((float) m_update_period); > > > > with: > > return > > ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) > > m_update_period); > > > > in TimestampedBuffer::getRate() to see whether things work somewhat better? Ok, I tried this overnight but it made no difference whatsoever. Before doing so I temporarily reverted my change to incrementFrameCounter() so that was back to the original for the duration of the test. The more tests I do the more I think the MOTU problem (with the original incrementFrameCounter()) is due to the cumulative drift in the tail timestamp resulting from the use of a DLL to derive its value. Having said that, perhaps it's a case of the rate calculation not having sufficient precision. As far as I can tell the rate is effectively derived from the difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, both of which are integers in units of ticks. With the realisation that these two values are only one packet apart, this constrains the possible rate values to a relatively course resolution. Perhaps this resolution is just too course for the MOTU (which we already know is picky about timing). > Things are a bit more complex, because the > m_buffer_next_tail_timestamp-m_buffer_tail_timestamp has to be protected > by the mutex, otherwise it can result in = 0 because these values are > updated from another thread. Fair enough. However, I don't think this was happening based on the monitoring output I used while testing. > My current version: > : Ok, I'll give this a test overnight and let you know how it stacks up. > Probably this way of doing things will cause us some merge issues in the > end, but I haven't tested this enough to commit it. That's fine - I'll test tonight and let you know what happens. Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-24 08:39:19
|
Jonathan Woithe wrote: > Hi Pieter > >>> can you replace: >>> return m_dll_e2/((float) m_update_period); >>> >>> with: >>> return >>> ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) >>> m_update_period); >>> >>> in TimestampedBuffer::getRate() to see whether things work somewhat better? > > Ok, I tried this overnight but it made no difference whatsoever. Before > doing so I temporarily reverted my change to incrementFrameCounter() so that > was back to the original for the duration of the test. > > The more tests I do the more I think the MOTU problem (with the original > incrementFrameCounter()) is due to the cumulative drift in the tail > timestamp resulting from the use of a DLL to derive its value. Having said Cumulative drift is not possible with a DLL, since it contains an integrating function that causes the error to average out to 0. There can (and will) be a phase offset, but the rate should be ok. > that, perhaps it's a case of the rate calculation not having sufficient > precision. As far as I can tell the rate is effectively derived from the > difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, > both of which are integers in units of ticks. With the realisation that > these two values are only one packet apart, this constrains the possible > rate values to a relatively course resolution. Perhaps this resolution is > just too course for the MOTU (which we already know is picky about timing). This is a very valid point. Maybe we need to switch to float for the timestamps. Maybe this is why the timestamps of the IEC streams cause less issues: each packet contains at least 8 frames, such that the timestamps are already further apart. What we could do is rewrite the timestampedbuffer to a generic ffado_timestamp_t type and make sure we never rely on it's type (i.e. no bitwise operators etc...). The switch between int-float-double should be trivial then. I'll do that today since I'm having some timestamp overflow errors that will be avoided if we use float. > >> Things are a bit more complex, because the >> m_buffer_next_tail_timestamp-m_buffer_tail_timestamp has to be protected >> by the mutex, otherwise it can result in = 0 because these values are >> updated from another thread. > > Fair enough. However, I don't think this was happening based on the > monitoring output I used while testing. Since the values of both timestamps are updated like this every packet: m_buffer_tail_timestamp=m_buffer_next_tail_timestamp; m_buffer_next_tail_timestamp += (int64_t)(m_dll_b * err + m_dll_e2); it's just a matter of time before the substraction in getRate() is done inbetween these two lines (resulting in a 0 result). I've seen it happen, it usually takes approx one minute. Most of the time this just causes an xrun and the system recovers, sometimes it even just recovers from this without an xrun. Pieter |
From: Pieter P. <pi...@jo...> - 2007-07-24 16:37:40
|
Pieter Palmers wrote: > Jonathan Woithe wrote: >> Hi Pieter >> >>>> can you replace: >>>> return m_dll_e2/((float) m_update_period); >>>> >>>> with: >>>> return >>>> ((float)(m_buffer_next_tail_timestamp-m_buffer_tail_timestamp))/((float) >>>> m_update_period); >>>> >>>> in TimestampedBuffer::getRate() to see whether things work somewhat better? >> Ok, I tried this overnight but it made no difference whatsoever. Before >> doing so I temporarily reverted my change to incrementFrameCounter() so that >> was back to the original for the duration of the test. >> >> The more tests I do the more I think the MOTU problem (with the original >> incrementFrameCounter()) is due to the cumulative drift in the tail >> timestamp resulting from the use of a DLL to derive its value. Having said > Cumulative drift is not possible with a DLL, since it contains an > integrating function that causes the error to average out to 0. There > can (and will) be a phase offset, but the rate should be ok. > >> that, perhaps it's a case of the rate calculation not having sufficient >> precision. As far as I can tell the rate is effectively derived from the >> difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, >> both of which are integers in units of ticks. With the realisation that >> these two values are only one packet apart, this constrains the possible >> rate values to a relatively course resolution. Perhaps this resolution is >> just too course for the MOTU (which we already know is picky about timing). > This is a very valid point. Maybe we need to switch to float for the > timestamps. Maybe this is why the timestamps of the IEC streams cause > less issues: each packet contains at least 8 frames, such that the > timestamps are already further apart. > > What we could do is rewrite the timestampedbuffer to a generic > ffado_timestamp_t type and make sure we never rely on it's type (i.e. no > bitwise operators etc...). The switch between int-float-double should be > trivial then. > > I'll do that today since I'm having some timestamp overflow errors that > will be avoided if we use float. I've completed the switch to float and it makes a huge difference regarding the stability of the derived rate (and timestamps). Where it used to switch between values around 48000 for a bebob device, it now locks to a perfect 48000. This is very good since the time source for bebob devices is the cycle timer itself, therefore there shouldn't be a difference. However for my setup it seems to be impossible to use a framerate other than 48000 or 96000, and it also doesn't seem to like a setup with an external sync'ed card. I'm not very satisfied with the current state of affairs, but I still have to wrap my head around all of this to come up with a less hacked approach. It seems that we really need to move to a clean state based model in which we can wait for sync and align phase before we start transmitting. Anyway I'm committing my stuff to the SVN repo such that your can check if the float stuff improves the situation. I'd advise you to backup your personal tree first though. Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-24 23:57:36
|
Hi Pieter I'm combining two emails here. > > The more tests I do the more I think the MOTU problem (with the original > > incrementFrameCounter()) is due to the cumulative drift in the tail > > timestamp resulting from the use of a DLL to derive its value. Having said > Cumulative drift is not possible with a DLL, since it contains an > integrating function that causes the error to average out to 0. There > can (and will) be a phase offset, but the rate should be ok. True, in theory. I have some simulation results based on what the MOTU provides to the DLL which shows that drift does occur, particularly in the early stages of stabilisation. Given sufficient resolution in the DLL (which can be tricky to get right) things do settle down after some time, but the steady state value the DLL converges to turns out to be noticeably different compared to the "real" value in the device. I did these tests back in February and have been thinking about them ever since. I suspect the problem is associated with the nature of the timestamps sent by the MOTU as well as the somewhat limited resolution of our DLL. It's pretty hard to get one's head around all this though. > > that, perhaps it's a case of the rate calculation not having sufficient > > precision. As far as I can tell the rate is effectively derived from the > > difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, > > both of which are integers in units of ticks. With the realisation that > > these two values are only one packet apart, this constrains the possible > > rate values to a relatively course resolution. Perhaps this resolution is > > just too course for the MOTU (which we already know is picky about timing). > This is a very valid point. Maybe we need to switch to float for the > timestamps. Maybe this is why the timestamps of the IEC streams cause > less issues: each packet contains at least 8 frames, such that the > timestamps are already further apart. Well, with the MOTU I am only updating timestamps once per packet even though I do have timestamps for each sample. My point wasn't necessarily about the frequency of the updates, but rather that integers were used for the DLL accumulators. > > What we could do is rewrite the timestampedbuffer to a generic > > ffado_timestamp_t type and make sure we never rely on it's type (i.e. no > > bitwise operators etc...). The switch between int-float-double should be > > trivial then. > > > > I'll do that today since I'm having some timestamp overflow errors that > > will be avoided if we use float. > > I've completed the switch to float and it makes a huge difference > regarding the stability of the derived rate (and timestamps). That sounds like a good result. > Where it used to switch between values around 48000 for a bebob device, > it now locks to a perfect 48000. This is very good since the time source > for bebob devices is the cycle timer itself, therefore there shouldn't > be a difference. Yep, this outcome doesn't surprise me. I also saw the "switching between values" and put this down to the highly quantised nature of the rate calculation. > However for my setup it seems to be impossible to use a framerate other > than 48000 or 96000, and it also doesn't seem to like a setup with an > external sync'ed card. This is interesting. I will test with my setup and let you know what I find. I can't immediately see why things would break for 44k1-derived rates unless there was a gremlin in the system. > I'm not very satisfied with the current state of affairs, but I still > have to wrap my head around all of this to come up with a less hacked > approach. It seems that we really need to move to a clean state based > model in which we can wait for sync and align phase before we start > transmitting. Isn't that more or less what happens now? First we wait for the stream processors to flag that they're running and then we instruct an enable at some point in the future. Or are you suggesting that instead of having a semi-fixed "wait before enable" we instead wait until the stream processors indicate they have locked sync? > Anyway I'm committing my stuff to the SVN repo such that your can check > if the float stuff improves the situation. I'd advise you to backup your > personal tree first though. :-) Noted. I'll give it a go and let you know what I find. I had a few things to commit based on my work last night but that will possibly have to wait now. One thing I managed to do was eliminate the "rather large" difference errors on startup. The problem seemed to be related to setTickOffset(). With a new offset larger than (say) 1000, the next attempt to incrementFrameCounter() is almost guaranteed to flag a rather large difference because setTickOffset() doesn't update the DLL's internal state. Once I did this I eliminated a number of glitches at startup. I also noted that during startup incrementFrameCounter() can be called multiple times with the same timestamp (because at that point the transmit stream isn't enabled and therefore its timestamp stands still). This also caused "rater large" differences to be reported. I added a check for "same timestamp" and that eliminated another source of timing glitches. Both of these are orthogonal to your changes so I might be able to commit them today. I'll see how I go once I see your latest changes. Regards jonathan |
From: Jonathan W. <jw...@ph...> - 2007-07-25 01:46:51
|
Hi again > > Anyway I'm committing my stuff to the SVN repo such that your can check > > if the float stuff improves the situation. I'd advise you to backup your > > personal tree first though. > > :-) Noted. I'll give it a go and let you know what I find. > > I had a few things to commit based on my work last night but that will > possibly have to wait now. One thing I managed to do was eliminate the > "rather large" difference errors on startup. The problem seemed to be > related to setTickOffset(). With a new offset larger than (say) 1000, the > next attempt to incrementFrameCounter() is almost guaranteed to flag a > rather large difference because setTickOffset() doesn't update the DLL's > internal state. Once I did this I eliminated a number of glitches at > startup. > > I also noted that during startup incrementFrameCounter() can be called > multiple times with the same timestamp (because at that point the transmit > stream isn't enabled and therefore its timestamp stands still). This also > caused "rater large" differences to be reported. I added a check for "same > timestamp" and that eliminated another source of timing glitches. > > Both of these are orthogonal to your changes so I might be able to commit > them today. I'll see how I go once I see your latest changes. Ok, as I expected your alterations didn't overlap mine significantly so I've committed changes for the above issues as rev 495 and 496 respectively. Note that I haven't compile-tested rev 495 or 496 since I'm at work which almost guarantees I've made a trivial merge error - so I apologise in advance. I'll try to test revisions 494 - 496 tonight and report back. Regards jonathan |
From: Jonathan W. <jw...@ph...> - 2007-07-26 00:43:14
|
Hi Pieter > >> that, perhaps it's a case of the rate calculation not having sufficient > >> precision. As far as I can tell the rate is effectively derived from the > >> difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, > >> both of which are integers in units of ticks. With the realisation that > >> these two values are only one packet apart, this constrains the possible > >> rate values to a relatively course resolution. Perhaps this resolution is > >> just too course for the MOTU (which we already know is picky about timing). > > This is a very valid point. Maybe we need to switch to float for the > > timestamps. Maybe this is why the timestamps of the IEC streams cause > > less issues: each packet contains at least 8 frames, such that the > > timestamps are already further apart. > > > > What we could do is rewrite the timestampedbuffer to a generic > > ffado_timestamp_t type and make sure we never rely on it's type (i.e. no > > bitwise operators etc...). The switch between int-float-double should be > > trivial then. > > > > I'll do that today since I'm having some timestamp overflow errors that > > will be avoided if we use float. > > I've completed the switch to float and it makes a huge difference > regarding the stability of the derived rate (and timestamps). Ok, I managed to grab 10 minutes last night to test svn496. The MOTU code suffered from some compile errors due to the timestamp type changes (which have hacked workarounds svn 497) but that aside I'd give a verdict of "promising". The MOTU ran, seemed to start cleanly and didn't loose sync within the first 10-20 seconds of running (I didn't have time to test longer). There was one regression: it seemed that every period (or at the very least quite a few of them) prompted a "rather large" warning from incrementFrameCounter() with a corresponding glitch in the audio output. However, I didn't get a chance to investigate this - it could for example be a consequence of my quick-and-dirty hacks for the MOTU stream processor code mentioned above. I should get some more time on this tonight so I'll provide more details in due course. One thing this did point out was that addTicks(), subtractTicks() and friends are still expecting int64_t timestamps. I suspect that all these need to be converted to ffado_timestamp_t as well so they are consistent with the type used for the actual timestamp. I will probably do this first up since I suspect this type mismatch (and the inherent rounding which results) may be part of the MOTU's problem. > However for my setup it seems to be impossible to use a framerate other > than 48000 or 96000, and it also doesn't seem to like a setup with an > external sync'ed card. My tests were at 44k1. Things weren't perfect but at least it ran (albeit with audio glitches). Is this what you meant by it being "impossible to use a framerate other than 48000 or 96000", or were your issues somewhat more terminal? I will try at 48k myself and see what happens, but I suspect much the same behaviour given the way the MOTU works. Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-26 09:41:47
|
Jonathan Woithe wrote: > Hi Pieter > >>>> that, perhaps it's a case of the rate calculation not having sufficient >>>> precision. As far as I can tell the rate is effectively derived from the >>>> difference between m_buffer_next_tail_timestamp and m_buffer_tail_timestamp, >>>> both of which are integers in units of ticks. With the realisation that >>>> these two values are only one packet apart, this constrains the possible >>>> rate values to a relatively course resolution. Perhaps this resolution is >>>> just too course for the MOTU (which we already know is picky about timing). >>> This is a very valid point. Maybe we need to switch to float for the >>> timestamps. Maybe this is why the timestamps of the IEC streams cause >>> less issues: each packet contains at least 8 frames, such that the >>> timestamps are already further apart. >>> >>> What we could do is rewrite the timestampedbuffer to a generic >>> ffado_timestamp_t type and make sure we never rely on it's type (i.e. no >>> bitwise operators etc...). The switch between int-float-double should be >>> trivial then. >>> >>> I'll do that today since I'm having some timestamp overflow errors that >>> will be avoided if we use float. >> I've completed the switch to float and it makes a huge difference >> regarding the stability of the derived rate (and timestamps). > > Ok, I managed to grab 10 minutes last night to test svn496. The MOTU code > suffered from some compile errors due to the timestamp type changes (which > have hacked workarounds svn 497) but that aside I'd give a verdict of > "promising". The MOTU ran, seemed to start cleanly and didn't loose sync > within the first 10-20 seconds of running (I didn't have time to test > longer). There was one regression: it seemed that every period (or at the > very least quite a few of them) prompted a "rather large" warning from > incrementFrameCounter() with a corresponding glitch in the audio output. > However, I didn't get a chance to investigate this - it could for example be > a consequence of my quick-and-dirty hacks for the MOTU stream processor code > mentioned above. I should get some more time on this tonight so I'll > provide more details in due course. That all sounds very good. > > One thing this did point out was that addTicks(), subtractTicks() and > friends are still expecting int64_t timestamps. I suspect that all these > need to be converted to ffado_timestamp_t as well so they are consistent > with the type used for the actual timestamp. I will probably do this > first up since I suspect this type mismatch (and the inherent rounding which > results) may be part of the MOTU's problem. I would like to keep the xxxTicks() functions with uint64_t since they are specific to the 1394 cycle timer (hence cycletimer.h). There are more places in that file where that assumption is made (e.g. the wraparounds are at 128sec). If we need similar functions for the timestamped buffer we should implement them there (if not already done). > >> However for my setup it seems to be impossible to use a framerate other >> than 48000 or 96000, and it also doesn't seem to like a setup with an >> external sync'ed card. > > My tests were at 44k1. Things weren't perfect but at least it ran (albeit > with audio glitches). Is this what you meant by it being "impossible to use > a framerate other than 48000 or 96000", or were your issues somewhat more > terminal? I will try at 48k myself and see what happens, but I suspect much > the same behaviour given the way the MOTU works. It meant complete chaos for me. Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-26 23:26:03
|
Hi Pieter > > Ok, I managed to grab 10 minutes last night to test svn496. The MOTU code > > suffered from some compile errors due to the timestamp type changes (which > > have hacked workarounds svn 497) but that aside I'd give a verdict of > > "promising". The MOTU ran, seemed to start cleanly and didn't loose sync > > within the first 10-20 seconds of running (I didn't have time to test > > longer). There was one regression: it seemed that every period (or at the > > very least quite a few of them) prompted a "rather large" warning from > > incrementFrameCounter() with a corresponding glitch in the audio output. > > However, I didn't get a chance to investigate this - it could for example be > > a consequence of my quick-and-dirty hacks for the MOTU stream processor code > > mentioned above. I should get some more time on this tonight so I'll > > provide more details in due course. > That all sounds very good. SVN revision 498 is up now. This contains the usual hacks around the MOTU code for my debugging, a neater solution to the ffado_timestamp_t/float issue within the MOTU code plus a change in the type of ffado_timestamp_t from float to double. After investigating time drift and course timestamp jumps (which is what was causing the MOTU's glitches) it became apparent that the root cause was in fact a lack of precision in the accumulators and the TimestampedBuffer's timestamps themselves. Because we run the timestamps in ticks and wrap only after 128 seconds, there simply isn't sufficient precision in a float to keep track of the timestamp. In short, there are 24576000 ticks per second. At the 50 second point (for example) the tick value is therefore 1228800000 (3145728000 at 128 seconds). In other words, there are 10 significant figures just for the units part of the timestamp. Tests with the MOTU and my simulations have shown that at least a couple of decimal places are needed if the DLL is to track the device to the required accuracy, so this demonstrates that we need about 12 significant figures if things are not to drift (preferably a few more). A float has no more than 9 significant decimal places. With the change in type from float to double I found that the MOTU would lock sync and remain stable for quite some time (I think the longest I tested was 30 seconds, but even that's more promising than other tests in recent weeks). Based on all this I think we're going to have to use doubles for ffado_timestamp_t, which is what revision 498 does. > > One thing this did point out was that addTicks(), subtractTicks() and > > friends are still expecting int64_t timestamps. I suspect that all these > > need to be converted to ffado_timestamp_t as well so they are consistent > > with the type used for the actual timestamp. I will probably do this > > first up since I suspect this type mismatch (and the inherent rounding which > > results) may be part of the MOTU's problem. > I would like to keep the xxxTicks() functions with uint64_t since they > are specific to the 1394 cycle timer (hence cycletimer.h). There are > more places in that file where that assumption is made (e.g. the > wraparounds are at 128sec). > > If we need similar functions for the timestamped buffer we should > implement them there (if not already done). Sure. As a result of my tests last night I no longer believe the xxxTicks() functions need to work with floats. > >> However for my setup it seems to be impossible to use a framerate other > >> than 48000 or 96000, and it also doesn't seem to like a setup with an > >> external sync'ed card. > > > > My tests were at 44k1. Things weren't perfect but at least it ran (albeit > > with audio glitches). Is this what you meant by it being "impossible to use > > a framerate other than 48000 or 96000", or were your issues somewhat more > > terminal? I will try at 48k myself and see what happens, but I suspect much > > the same behaviour given the way the MOTU works. > It meant complete chaos for me. Based on my tests last night and findings regarding precision I think it would be worthwhile for you to test 44k1 again using svn revision 498. The bebob devices work a bit differently to the MOTUs, and because 48k is a factor of 24576000 the lack of precision may not have been a factor for you with 48k rates. However, since 44k1 is not a factor of 24576000 the truncated precision could easily have lead to inaccurate timestamp prediction, which in turn sends the rate right off. Regards jonathan |
From: Pieter P. <pi...@jo...> - 2007-07-27 10:59:36
|
Jonathan Woithe wrote: > Hi Pieter > >>> Ok, I managed to grab 10 minutes last night to test svn496. The MOTU code >>> suffered from some compile errors due to the timestamp type changes (which >>> have hacked workarounds svn 497) but that aside I'd give a verdict of >>> "promising". The MOTU ran, seemed to start cleanly and didn't loose sync >>> within the first 10-20 seconds of running (I didn't have time to test >>> longer). There was one regression: it seemed that every period (or at the >>> very least quite a few of them) prompted a "rather large" warning from >>> incrementFrameCounter() with a corresponding glitch in the audio output. >>> However, I didn't get a chance to investigate this - it could for example be >>> a consequence of my quick-and-dirty hacks for the MOTU stream processor code >>> mentioned above. I should get some more time on this tonight so I'll >>> provide more details in due course. >> That all sounds very good. > > SVN revision 498 is up now. This contains the usual hacks around the MOTU > code for my debugging, a neater solution to the ffado_timestamp_t/float > issue within the MOTU code plus a change in the type of ffado_timestamp_t > from float to double. > > After investigating time drift and course timestamp jumps (which is what was > causing the MOTU's glitches) it became apparent that the root cause was in > fact a lack of precision in the accumulators and the TimestampedBuffer's > timestamps themselves. Because we run the timestamps in ticks and wrap only > after 128 seconds, there simply isn't sufficient precision in a float to > keep track of the timestamp. In short, there are 24576000 ticks per second. > At the 50 second point (for example) the tick value is therefore 1228800000 > (3145728000 at 128 seconds). In other words, there are 10 significant > figures just for the units part of the timestamp. Tests with the MOTU and > my simulations have shown that at least a couple of decimal places are > needed if the DLL is to track the device to the required accuracy, so this > demonstrates that we need about 12 significant figures if things are not to > drift (preferably a few more). A float has no more than 9 significant > decimal places. > > With the change in type from float to double I found that the MOTU would > lock sync and remain stable for quite some time (I think the longest I > tested was 30 seconds, but even that's more promising than other tests in > recent weeks). Based on all this I think we're going to have to use doubles > for ffado_timestamp_t, which is what revision 498 does. That's good news. At this point the switch to double is OK, but on the long run I would like to return to floats if possible, mainly for the cpu load. I think we can do this when we wrap earlier than 128 sec. I was already planning to do this in a later stage, maybe even dropping the seconds field altogether. But first let's get things working. > >>> One thing this did point out was that addTicks(), subtractTicks() and >>> friends are still expecting int64_t timestamps. I suspect that all these >>> need to be converted to ffado_timestamp_t as well so they are consistent >>> with the type used for the actual timestamp. I will probably do this >>> first up since I suspect this type mismatch (and the inherent rounding which >>> results) may be part of the MOTU's problem. >> I would like to keep the xxxTicks() functions with uint64_t since they >> are specific to the 1394 cycle timer (hence cycletimer.h). There are >> more places in that file where that assumption is made (e.g. the >> wraparounds are at 128sec). >> >> If we need similar functions for the timestamped buffer we should >> implement them there (if not already done). > > Sure. As a result of my tests last night I no longer believe the xxxTicks() > functions need to work with floats. I think no rounding errors are introduced by these operations, since their operand values are already in ticks, meaning that they are already integers and can't lose precision by addition/substraction. > >>>> However for my setup it seems to be impossible to use a framerate other >>>> than 48000 or 96000, and it also doesn't seem to like a setup with an >>>> external sync'ed card. >>> My tests were at 44k1. Things weren't perfect but at least it ran (albeit >>> with audio glitches). Is this what you meant by it being "impossible to use >>> a framerate other than 48000 or 96000", or were your issues somewhat more >>> terminal? I will try at 48k myself and see what happens, but I suspect much >>> the same behaviour given the way the MOTU works. >> It meant complete chaos for me. > > Based on my tests last night and findings regarding precision I think it > would be worthwhile for you to test 44k1 again using svn revision 498. The > bebob devices work a bit differently to the MOTUs, and because 48k is a > factor of 24576000 the lack of precision may not have been a factor for you > with 48k rates. However, since 44k1 is not a factor of 24576000 the > truncated precision could easily have lead to inaccurate timestamp > prediction, which in turn sends the rate right off. That was what I was suspecting, since that would explain the difficulties I had with 48k mode + external sync. Great to see some progress on this front. I'm glad I'm not alone anymore for this part because I find it very difficult (and frustrating) to get right. Greets, Pieter |
From: Jonathan W. <jw...@ph...> - 2007-07-30 00:00:25
|
Hi Pieter I've just committed SVN revision 512. As per the commit log message, this contains a number of minor changes to improve reliability of the MOTU code and the start of the code cleanup now that things are starting to look good once more. One thing which still bugs me is the "rather large" errors I still get during startup (and the corresponding glitches you get in the audio output). I have determined that they are due to DLL stabilisation, but that simply makes me want to work out why the DLL can sometimes be initialised so far away from its "correct" value (basically it can be up to one packet's worth of time out - approximately 4458 at 44k1 Hz). So far I haven't found a way to reliably rectify this, and perhaps it isn't possible (I am yet to fully analyse the DLL startup flow). To avoid clicks and pops during startup, the MOTU code currently mutes all outputs for the first 2 seconds after an enable. This gives a clean startup although I still view this as a bit of a hack. I've also added the functionality which sends a series of zero packets to the interface on closedown. With the old streaming system this was needed to ensure the high-frequecy "noise" signal didn't occur. Under the new streaming system it doesn't appear that this is needed but I figure it does no harm and adds an extra degree of safety to the whole thing. It should be noted that under other systems the driver sends a burst of zero data at the start and end of streaming operations, so it's probably reasonable for us to do that too. I ran my MOTU interface at 44k1 via JACK yesterday for just over 2.5 hours without an xrun or loss of sync. I also tested 48k but for much less time - only about 10 minutes - and it was similarly stable. Testing continues, mainly because on Saturday night I encountered a rather interesting effect and I don't know if it was indicating a gremlin in the system or was simply a side effect of the testing I was doing at the time. > > SVN revision 498 is up now. This contains the usual hacks around the MOTU > > code for my debugging, a neater solution to the ffado_timestamp_t/float > > issue within the MOTU code plus a change in the type of ffado_timestamp_t > > from float to double. > > > > After investigating time drift and course timestamp jumps (which is what was > > causing the MOTU's glitches) it became apparent that the root cause was in > > fact a lack of precision in the accumulators and the TimestampedBuffer's > > timestamps themselves. Because we run the timestamps in ticks and wrap only > > after 128 seconds, there simply isn't sufficient precision in a float to > > keep track of the timestamp. In short, there are 24576000 ticks per second. > > At the 50 second point (for example) the tick value is therefore 1228800000 > > (3145728000 at 128 seconds). In other words, there are 10 significant > > figures just for the units part of the timestamp. Tests with the MOTU and > > my simulations have shown that at least a couple of decimal places are > > needed if the DLL is to track the device to the required accuracy, so this > > demonstrates that we need about 12 significant figures if things are not to > > drift (preferably a few more). A float has no more than 9 significant > > decimal places. > > > > With the change in type from float to double I found that the MOTU would > > lock sync and remain stable for quite some time (I think the longest I > > tested was 30 seconds, but even that's more promising than other tests in > > recent weeks). Based on all this I think we're going to have to use doubles > > for ffado_timestamp_t, which is what revision 498 does. > That's good news. > > At this point the switch to double is OK, but on the long run I would > like to return to floats if possible, mainly for the cpu load. I think > we can do this when we wrap earlier than 128 sec. I was already planning > to do this in a later stage, maybe even dropping the seconds field > altogether. But first let's get things working. I agree we should get things working first. I should add that on my system the loading didn't appear to be affected measureably with the move to double, but a sample space of one system isn't really suffient to make a call like this. Dropping the seconds field altogether may work for some cards but I suspect it will still cause trouble for MOTU. The problem is that the number of significant bits in a 32-bit float (with an effective 24 bit signed mantissa) is limited to approximately log10(2^23) - or about 7. The number of ticks in 1 second is 24576000 which is 8 digits long. Therefore it would not be possible to keep all non-decimal digits in a 32 bit accumulator over the total range of that accumulator, let along any decimal places at all. While the operation of the DLL should ensure that things recover over time, the short-term loss of this precision as the accumulator heads towards 24576000 is most likely going to cause the MOTU some trouble. Having said that, using double for this does seem like overkill. I'm wondering whether it might not be better to consider a 64-bit fixed point DLL instead. Experiments and observations I've made seem to suggest that 3 decimal places ought to be sufficient for the MOTU. Therefore we could return to using a 64-bit integer type and have an implied decimal point in it. I'll give this some more thought. > >>>> However for my setup it seems to be impossible to use a framerate other > >>>> than 48000 or 96000, and it also doesn't seem to like a setup with an > >>>> external sync'ed card. > >>> My tests were at 44k1. Things weren't perfect but at least it ran (albeit > >>> with audio glitches). Is this what you meant by it being "impossible to use > >>> a framerate other than 48000 or 96000", or were your issues somewhat more > >>> terminal? I will try at 48k myself and see what happens, but I suspect much > >>> the same behaviour given the way the MOTU works. > >> It meant complete chaos for me. > > > > Based on my tests last night and findings regarding precision I think it > > would be worthwhile for you to test 44k1 again using svn revision 498. The > > bebob devices work a bit differently to the MOTUs, and because 48k is a > > factor of 24576000 the lack of precision may not have been a factor for you > > with 48k rates. However, since 44k1 is not a factor of 24576000 the > > truncated precision could easily have lead to inaccurate timestamp > > prediction, which in turn sends the rate right off. > > That was what I was suspecting, since that would explain the > difficulties I had with 48k mode + external sync. Indeed. > Great to see some progress on this front. I'm glad I'm not alone anymore > for this part because I find it very difficult (and frustrating) to get > right. :-) Yes, getting timing right with these interfaces is irritating and fiddly. It would be so much easier if they would lock their cycle timer to the audio clock, but of course that can't be done because there's no guarantee that the device will end up being the cycle master. Regards jonathan |