View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001742 | ardour | bugs | public | 2007-06-24 10:11 | 2020-04-19 20:12 |
Reporter | lincoln | Assigned To | paul | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 2.0 | ||||
Summary | 0001742: Latency over-compensation when recording hydrogen drums | ||||
Description | When recording drums from hydrogen I noticed that the drums were eing recorded ahead. I checked the numer of frames by which the recording was ahead and it became clear the the number of frames involved are equal to jack's stated latency. It seems that the latency compensation is over compensating here. | ||||
Tags | No tags attached. | ||||
|
i have spent half a day searching for mistakes in the latency compensation. i did not find any. but when uncommenting the slave_follow debug output: libs/ardour/session_process.cc:468 #if 1 i get this: delta = 1024 speed = 1 ts = 1 M@ 165888 S@ 164864 avgdelta = 1800 ardour NEVER locks onto the jack-transport. this goes unnoticed for clients interpreting BBT correctly, because of ardours timebase callback being somewhat misinterpreted. i try to fix this issue... |
2007-07-18 01:11
|
transport-diff.diff (2,687 bytes)
Index: libs/ardour/ardour/audioengine.h =================================================================== --- libs/ardour/ardour/audioengine.h (revision 2138) +++ libs/ardour/ardour/audioengine.h (working copy) @@ -84,10 +84,7 @@ return jack_frame_time (_jack); } - nframes_t transport_frame () const { - if (!_running || !_jack) return 0; - return jack_get_current_transport_frame (_jack); - } + nframes_t transport_frame () const; int request_buffer_size (nframes_t); Index: libs/ardour/audioengine.cc =================================================================== --- libs/ardour/audioengine.cc (revision 2138) +++ libs/ardour/audioengine.cc (working copy) @@ -694,6 +694,17 @@ } } + +nframes_t +AudioEngine::transport_frame () const +{ + if (!_running || !_jack) return 0; + return jack_get_current_transport_frame (_jack); + //jack_position_t pos; + //jack_transport_query( _jack, &pos ); + //return pos.frame; +} + Port * AudioEngine::get_port_by_name (const string& portname, bool keep) { Index: libs/ardour/session_time.cc =================================================================== --- libs/ardour/session_time.cc (revision 2138) +++ libs/ardour/session_time.cc (working copy) @@ -497,15 +497,15 @@ /* frame info */ - pos->frame = _transport_frame; + //pos->frame = _transport_frame; pos->valid = JackPositionTimecode; /* BBT info */ if (_tempo_map) { - TempoMap::Metric metric (_tempo_map->metric_at (_transport_frame)); - _tempo_map->bbt_time_with_metric (_transport_frame, bbt, metric); + TempoMap::Metric metric (_tempo_map->metric_at (pos->frame)); + _tempo_map->bbt_time_with_metric (pos->frame, bbt, metric); pos->bar = bbt.bars; pos->beat = bbt.beats; Index: libs/ardour/session_process.cc =================================================================== --- libs/ardour/session_process.cc (revision 2138) +++ libs/ardour/session_process.cc (working copy) @@ -46,9 +46,7 @@ Session::process (nframes_t nframes) { if (synced_to_jack() && waiting_to_start) { - if ( _engine.transport_state() == AudioEngine::TransportRolling) { actually_start_transport (); - } } if (non_realtime_work_pending()) { Index: libs/ardour/session_transport.cc =================================================================== --- libs/ardour/session_transport.cc (revision 2138) +++ libs/ardour/session_transport.cc (working copy) @@ -879,6 +879,9 @@ void Session::start_transport () { + if (synced_to_jack() && !_exporting) { + _transport_frame += get_block_size(); + } _last_roll_location = _transport_frame; /* if record status is Enabled, move it to Recording. if its |
|
the attached patch removes the issue. however ardour now start recording at period_size which is not really better. it also contains a fix to the timebase_callback. ardour was overwriting pos->frame which is not allowed. the other hunk just moves a function from the headerfile to the .cc file. i should have removed it. |
|
i have fixed the bug in jack. there might be issues with this patch, but i think its complete. please fix ardour timebase callback anyways. it should not be critical anymore, but its a bad example. |
2007-07-18 03:19
|
jack-transport-start-at-zero-fix.diff (1,613 bytes)
Index: jackd/transengine.c =================================================================== --- jackd/transengine.c (revision 1050) +++ jackd/transengine.c (working copy) @@ -395,6 +395,7 @@ { jack_control_t *ectl = engine->control; transport_command_t cmd; /* latest transport command */ + int dont_trans_advance = FALSE; /* Promote pending_time to current_time. Maintain the usecs, * frame_rate and frame values, clients may not set them. */ @@ -409,6 +410,10 @@ if ((ectl->sync_remain == 0) || (jack_sync_timeout(engine))) { ectl->transport_state = JackTransportRolling; + + // dont advance the transport this is a statechange not seen + // by the switch statement below + dont_trans_advance = TRUE; VERBOSE (engine, "transport Rolling, %8.6f sec" " left for poll\n", (double) (ectl->sync_time_left / 1000000.0)); @@ -468,7 +473,12 @@ ectl->transport_state = JackTransportStarting; jack_sync_poll_start(engine); } + } else if ( ! dont_trans_advance ) { + // ok... no statechange happened go transport go... + ectl->pending_time.frame = + ectl->current_time.frame + ectl->buffer_size; } + break; default: @@ -476,12 +486,6 @@ ectl->transport_state); } - /* Update timebase, if needed. */ - if (ectl->transport_state == JackTransportRolling) { - ectl->pending_time.frame = - ectl->current_time.frame + ectl->buffer_size; - } - /* See if an asynchronous position request arrived during the * last cycle. The request_time could change during the * guarded copy. If so, we use the newest request. */ |
|
mhmhm... its not complete :( |
|
finally managed to find time to try the patch. It now places the region late but if you snap to bar and move it everything falls into place as expected. So your patch does improve things quite a bit. It looks like the region start just needs to be offset back by 1 period_size when recording in this situation. |
|
did you apply both patches ? i had a report that only applying the jack patch fixes the problem for hydrogen. for muse-0.9 however it is not fixed. but after a quick skimming over muses code i have the feeling, that its transport code is not correct for this use case anyways. |
|
Yes I only applied the jack patch and yes it does work. With the ardour patch it would make the recording late. I have just tested this against hydrogen from svn. |
2008-09-23 11:19
|
diffs.diff (876 bytes)
Index: libs/ardour/session_process.cc =================================================================== --- libs/ardour/session_process.cc (revision 3794) +++ libs/ardour/session_process.cc (working copy) @@ -47,6 +47,9 @@ { _silent = false; + if( !_exporting && synced_to_jack() && _slave ) { + follow_slave(nframes,0); + } if (synced_to_jack() && waiting_to_start) { if ( _engine.transport_state() == AudioEngine::TransportRolling) { actually_start_transport (); @@ -301,9 +304,11 @@ } if (!_exporting && _slave) { + if( !synced_to_jack() ) { if (!follow_slave (nframes, 0)) { return; } + } } if (_transport_speed == 0) { @@ -733,9 +738,11 @@ } if (!_exporting && _slave) { + if( !synced_to_jack() ) { if (!follow_slave (nframes, 0)) { return; } + } } if (_transport_speed == 0) { |
|
the problem is, that the start_transport() function only sets waiting_to_start = TRUE, but does not do anything further. waiting_to_start is evaluated in Session::process() before calling (this->*process_function) (nframes); this results in ardours internal transport starting one period too late. the attached file diffs.diff fixes the problem. there might be a solution which is more elegant, but i guess this is up to paul :) because i dont understand why actually_start_transport cant be called here. |
|
torben's suggestion for a "clean" fix has been applied to 2.X and 3.0. it has not been thoroughly tested and may require changes to hydrogen since it was reported that they modified their code to workaround this bug in ardour. however, ardour's behaviour was clearly incorrect before and is now correct, so i am marking this resolved. |
|
Issue has been closed automatically, by Trigger Close Plugin. Feel free to re-open with additional information if you think the issue is not resolved. |
Date Modified | Username | Field | Change |
---|---|---|---|
2007-06-24 10:11 | lincoln | New Issue | |
2007-07-17 22:45 | torbenh3 | Note Added: 0004150 | |
2007-07-18 01:11 | torbenh3 | File Added: transport-diff.diff | |
2007-07-18 01:15 | torbenh3 | Note Added: 0004151 | |
2007-07-18 03:19 | torbenh3 | Note Added: 0004153 | |
2007-07-18 03:19 | torbenh3 | File Added: jack-transport-start-at-zero-fix.diff | |
2007-07-18 20:54 | torbenh3 | Note Added: 0004160 | |
2007-08-15 12:44 | lincoln | Note Added: 0004268 | |
2008-09-12 23:42 | torbenh | Note Added: 0005133 | |
2008-09-13 08:00 | lincoln | Note Added: 0005135 | |
2008-09-23 11:19 | torbenh | File Added: diffs.diff | |
2008-09-23 11:31 | torbenh | Note Added: 0005148 | |
2008-10-11 11:59 | paul | cost | => 0.00 |
2008-10-11 11:59 | paul | Status | new => resolved |
2008-10-11 11:59 | paul | Resolution | open => fixed |
2008-10-11 11:59 | paul | Assigned To | => paul |
2008-10-11 11:59 | paul | Note Added: 0005180 | |
2020-04-19 20:12 | system | Note Added: 0021534 | |
2020-04-19 20:12 | system | Status | resolved => closed |