From: FFADO <ffa...@ff...> - 2011-01-27 02:21:02
|
#321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long -------------------------------+-------------------------------------------- Reporter: cladisch | Owner: Type: bug | Status: new Priority: minor | Milestone: Component: generic/streaming | Version: FFADO SVN (trunk) Keywords: | Device_name: -------------------------------+-------------------------------------------- AMDTP defines the TRANSFER_DELAY as the time between the reception (sampling) of the event at the sending device and the intended presentation time at the receiving device. FFADO measures AMDTP_TRANSMIT_TRANSFER_DELAY from the cycle in which the packet is transmitted, which is one cycle later than the sampling, so the "+125µs" must be dropped. -- Ticket URL: <http://subversion.ffado.org/ticket/321> FFADO <http://subversion.ffado.org/index.fcgi> Free Firewire Audio Drivers |
From: FFADO <ffa...@ff...> - 2011-01-27 09:22:59
|
#321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long ---------------------------------+------------------------------------------ Reporter: cladisch | Owner: Type: bug | Status: closed Priority: minor | Milestone: Component: generic/streaming | Version: FFADO SVN (trunk) Resolution: fixed | Keywords: Device_name: | ---------------------------------+------------------------------------------ Changes (by adi): * status: new => closed * resolution: => fixed Comment: Fixed in r1950. Thanks for spotting it. -- Ticket URL: <http://subversion.ffado.org/ticket/321#comment:1> FFADO <http://subversion.ffado.org/index.fcgi> Free Firewire Audio Drivers |
From: Holger D. <deh...@ah...> - 2011-02-16 11:15:28
|
> #321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long > ---------------------------------+----------------------------------------- > - Reporter: cladisch | Owner: > Type: bug | Status: closed > Priority: minor | Milestone: > Component: generic/streaming | Version: FFADO SVN (trunk) > Resolution: fixed | Keywords: > Device_name: | > ---------------------------------+----------------------------------------- > - Changes (by adi): > > * status: new => closed > * resolution: => fixed > > Comment: > > Fixed in r1950. Thanks for spotting it. Unfortunately, this fix 'unfixes' another fix;-) Yesterday night I updated my ffado repository to test if Ticket 310 does any harm. The result was, that my lovely Mackie stops working again. Going back revision by revision identified changeset 1950 as the reason. I suppose, this is in conjunction with the changeset 1945 where we readjust the previously miscalculated ringbuffer size. I fear there is something fundamentally wrong with this calculation but I must admit, I do not understand at all what happens there. Could anyone go into this or explain to me the details of this calculation;-) Thank you! Holger |
From: Holger D. <deh...@ah...> - 2011-02-16 21:02:08
|
Am Mittwoch, 16. Februar 2011, 11:56:34 schrieb Holger Dehnhardt: > > #321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long > > ---------------------------------+--------------------------------------- > > -- > > > > - Reporter: cladisch | Owner: > > Type: bug | Status: closed > > > > Priority: minor | Milestone: > > Component: generic/streaming | Version: FFADO SVN (trunk) > > > > Resolution: fixed | Keywords: > > Device_name: | > > ---------------------------------+--------------------------------------- > > -- > > > > - Changes (by adi): > > * status: new => closed > > * resolution: => fixed > > > > Comment: > > Fixed in r1950. Thanks for spotting it. > > Unfortunately, this fix 'unfixes' another fix;-) > > Yesterday night I updated my ffado repository to test if Ticket 310 does > any harm. The result was, that my lovely Mackie stops working again. > Going back revision by revision identified changeset 1950 as the reason. > > I suppose, this is in conjunction with the changeset 1945 where we readjust > the previously miscalculated ringbuffer size. > > I fear there is something fundamentally wrong with this calculation but I > must admit, I do not understand at all what happens there. > > Could anyone go into this or explain to me the details of this > calculation;-) > > Thank you! > > Holger > > --------------------------------------------------------------------------- > --- The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio > XE: Pinpoint memory and threading errors before they happen. > Find and fix more than 250 security defects in the development cycle. > Locate bottlenecks in serial and parallel code that limit performance. > http://p.sf.net/sfu/intel-dev2devfeb > _______________________________________________ > FFADO-devel mailing list > FFA...@li... > https://lists.sourceforge.net/lists/listinfo/ffado-devel Ok, I have seen that the value xmit_transfer_delay can be set (overwritten) in the configuration. So here's a patch to do so. But I really would be happy if someone could have a glance at the sourcecode as I do not know what I'm doing here! Thanks Holger Index: configuration =================================================================== --- configuration (Revision 1955) +++ configuration (Arbeitskopie) @@ -22,6 +22,7 @@ vendorname = "Loud Technologies Inc."; modelname = "Onyx-i"; driver = 4; # Oxford + xmit_transfer_delay = 11776U; }, { vendorid = 0x0003db; |
From: Adrian K. <ad...@dr...> - 2011-02-16 21:25:51
|
On Wed, Feb 16, 2011 at 10:01:28PM +0100, Holger Dehnhardt wrote: > > > Fixed in r1950. Thanks for spotting it. > > Unfortunately, this fix 'unfixes' another fix;-) > So here's a patch to do so. Thanks, applied. > But I really would be happy if someone could have a glance at the sourcecode > as I do not know what I'm doing here! Me neither. ;) Cheers -- mail: ad...@th... http://adi.thur.de PGP/GPG: key via keyserver |
From: Clemens L. <cl...@la...> - 2011-02-17 09:33:09
|
Holger Dehnhardt wrote: > Am Mittwoch, 16. Februar 2011, 11:56:34 schrieb Holger Dehnhardt: > > > #321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long > > > > > > - Changes (by adi): > > > * status: new => closed > > > * resolution: => fixed > > > > Unfortunately, this fix 'unfixes' another fix;-) > > > > Yesterday night I updated my ffado repository to test if Ticket 310 does > > any harm. The result was, that my lovely Mackie stops working again. > > Going back revision by revision identified changeset 1950 as the reason. > > Ok, I have seen that the value xmit_transfer_delay can be set (overwritten) > in the configuration. > > So here's a patch to do so. > But I really would be happy if someone could have a glance at the sourcecode > as I do not know what I'm doing here! It turns out that my fix is theoretically correct, but that in practice, it makes worse the effect of another FFADO bug. The AMDTP standard defines the DEFAULT_TRANSFER_DELAY as 479.17 µs (11776 ticks). However, this delay is only to be used for non-blocking transmission; for blocking transmission (which FFADO uses), another frequency-dependent delay must be added to adjust for the empty packets due to the blocking: 32 kHz: 250 µs (6144 ticks) 44.1 kHz: 181.41 µs (4458 ticks) (also 88.2 and 176.4 kHz) 48 kHz: 166.67 µs (4096 ticks) (also 96 and 192 kHz) FFADO currently does not add this delay (bug #2). Due to differences in how delays are measured, the xmit_transfer_delay as used by FFADO must be one cycle (125 µs = 3072 ticks) less than AMDTP's DEFAULT_TRANSFER_DELAY (bug #1, fixed by my patch). So before my patch, FFADO's transfer delay forgot to subtract 3072 and also forgot to add the rate-dependent 6144/4458/4096, so the overall effect was that it was a little too small. After my patch, the transfer delay was 3072 ticks smaller than before, so it became much too small, which broke the Onyx-i. Other Oxford- based devices are likely to have the same problem. (It has been confirmed that Oxford chips work just fine with the correct transfer delay in non-blocking mode, but FFADO doesn't use this at the moment.) As a quick fix, it would be a good idea to revert my patch, or even to increase the xmit_transfer_delay to 11776-3072+4458 = 13162. As a proper fix, AmdtpTransmitStreamProcessor needs to add the correct frequency-dependent delay. Regards, Clemens |
From: Holger D. <deh...@ah...> - 2011-02-17 14:19:53
|
Am Thursday 17 February 2011 10:33:49 schrieb Clemens Ladisch: > Holger Dehnhardt wrote: > > Am Mittwoch, 16. Februar 2011, 11:56:34 schrieb Holger Dehnhardt: > > > > #321: AMDTP_TRANSMIT_TRANSFER_DELAY is one cycle too long > > > > > > > > - Changes (by adi): > > > > * status: new => closed > > > > * resolution: => fixed > > > > > > Unfortunately, this fix 'unfixes' another fix;-) > > > > > > Yesterday night I updated my ffado repository to test if Ticket 310 > > > does any harm. The result was, that my lovely Mackie stops working > > > again. Going back revision by revision identified changeset 1950 as > > > the reason. > > > > Ok, I have seen that the value xmit_transfer_delay can be set > > (overwritten) in the configuration. > > > > So here's a patch to do so. > > But I really would be happy if someone could have a glance at the > > sourcecode as I do not know what I'm doing here! > > It turns out that my fix is theoretically correct, but that in practice, > it makes worse the effect of another FFADO bug. > > The AMDTP standard defines the DEFAULT_TRANSFER_DELAY as 479.17 µs > (11776 ticks). However, this delay is only to be used for non-blocking > transmission; for blocking transmission (which FFADO uses), another > frequency-dependent delay must be added to adjust for the empty packets > due to the blocking: > 32 kHz: 250 µs (6144 ticks) > 44.1 kHz: 181.41 µs (4458 ticks) (also 88.2 and 176.4 kHz) > 48 kHz: 166.67 µs (4096 ticks) (also 96 and 192 kHz) > > FFADO currently does not add this delay (bug #2). > > Due to differences in how delays are measured, the xmit_transfer_delay > as used by FFADO must be one cycle (125 µs = 3072 ticks) less than > AMDTP's DEFAULT_TRANSFER_DELAY (bug #1, fixed by my patch). > > So before my patch, FFADO's transfer delay forgot to subtract 3072 and > also forgot to add the rate-dependent 6144/4458/4096, so the overall > effect was that it was a little too small. > > After my patch, the transfer delay was 3072 ticks smaller than before, > so it became much too small, which broke the Onyx-i. Other Oxford- > based devices are likely to have the same problem. (It has been > confirmed that Oxford chips work just fine with the correct transfer > delay in non-blocking mode, but FFADO doesn't use this at the moment.) > > > As a quick fix, it would be a good idea to revert my patch, or even to > increase the xmit_transfer_delay to 11776-3072+4458 = 13162. > > As a proper fix, AmdtpTransmitStreamProcessor needs to add the correct > frequency-dependent delay. > > > Regards, > Clemens > > --------------------------------------------------------------------------- > --- The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio > XE: Pinpoint memory and threading errors before they happen. > Find and fix more than 250 security defects in the development cycle. > Locate bottlenecks in serial and parallel code that limit performance. > http://p.sf.net/sfu/intel-dev2devfeb > _______________________________________________ > FFADO-devel mailing list > FFA...@li... > https://lists.sourceforge.net/lists/listinfo/ffado-devel Clemens, thank you for your explanation!!! I will try to make a fix for (bug #2). Even if it might take some days, please dont unfix your fix. Holger |