Bugzilla – Bug 641
Moonphase: Append record with overdub may cause spurious playthrough and may crash.
Last modified: 2018-08-20 11:45:17 UTC
1) Open Audacity 2) Press "R" to start recording 3) After 3 or 4 seconds, press Shift+A to stop and set cursor 4) Press Shift+R to Append Record After about 2 seconds Audacity may crash (usually does, tested with 2.0.4 alpha debug build on Debian stable). If Audacity does not crash, then recording continues but there is a brief section (about half a second) from the first recording that plays approximately every 6 seconds. The playback issue is repeatable on Windows. Audacity 2.0.0 did not crash from this problem, so this is a regression.
Steve wrote: > The playback issue is repeatable on Windows. Worth pointing out that it isn't repeatable on either of my Windows 7 machines. The laptop does not do it, the netbook started doing it after launching Audacity a third time and repeating the steps. As per your comment in the URL, having a selection does seem influential. As soon as I drew a selection in the first recording, the netbook stopped showing the symptoms. Is what you are recording relevant e.g. stereo mix versus mic? I don't see the problem often enough to decide. My guess is that the SHIFT + A is influential as a trigger, because I have (using standard Stop) not seen the issue for a long time despite trying whenever someone reports it. FWIW using the bug steps in 1.3.6 (Windows) many times just now in several sessions, the issue does not occur. 2.0.4 alpha on my Mac does not seem to show the bug. Other note: the playback frequency is definitely related to block writing, so if you set the project rate to 88200 Hz, you would see/hear the playback about every three seconds. I am not clear - is the crashing you see still confined to debug builds (release builds don't crash)? If so, that seems to suggest a lower rating than P2.
(In reply to comment #1) Gale wrote: > Is what you are recording relevant e.g. stereo mix versus mic? That does not appear to be relevant. The issue appears regardless of which input or device I use. > My guess is that the SHIFT + A is influential as a trigger, I first noticed this issue when using Shift+A to stop but it also occurs when using the Stop button or spacebar to stop playback. > the playback frequency is definitely related to block writing, so > if you set the project rate to 88200 Hz, you would see/hear the playback > about every three seconds. A With the sample rate set to 88200 Hz Audacity crashes within a couple of seconds of pressing Shift+R. With the sample rate set to 22050 Hz Audacity seems to be less prone to crashing, but the "feed-through" noise occurs at intervals of 12 seconds.
Does revision 12432 fix it?
(In reply to comment #3) That seems to have stopped the crashing, but I'm still getting a brief section of playthrough every 6 seconds. There is also another slight change in that when recording is resumed with Shift+R there is sometimes a brief bit of silence at the start of the new recording. The silence may or may not be separated from the rest of the recording by split lines. I've dropped this down to P3 as it is no longer crashing.
(In reply to comment #4) > That seems to have stopped the crashing, but I'm still getting a brief section > of playthrough every 6 seconds. I assume that is on Debian Debug Build? On my Windows netbook and laptop the playthrough is easier to reproduce than pre-patch. I did about 30 tests including reboots and saw the problem about 20 times (both machines). Comparable test before the patch was about 3 occurrences, only on netbook. Also SHIFT + A is now not more likely to trigger for me than other Stop methods. On Linux I got the problem six times out of 10 (more than before). > There is also another slight change in that when recording is resumed with > Shift+R there is sometimes a brief bit of silence at the start of the new > recording. Did not see it on Windows, but got the silence 10 times out of 10 on Ubuntu netbook. The "silence" is related to the "Audio to Buffer" setting (increase that setting and the silence increases, and vice-versa). Also the silence is trimmed by the value of the "Latency Correction" (if a negative value). > The silence may or may not be separated from the rest of the recording by split lines. I always get split line on either OS if I stopped the previous recording by SHIFT + A . > I've dropped this down to P3 as it is no longer crashing. On my current replicability, perhaps reasonable, but who knows what makes it happen and how many people are affected most times they record?
Testing more on Windows over the last couple of days, I'm fairly certain the first release that has the playthrough problem is 1.3.10. I don't get complete repeatability in 1.3.10 but if I use SHIFT+ A to provoke it, it will happen within four or five attempts. I cannot reproduce the problem at all in 1.3.9. 1.3.10 alpha was from 1 September to 30 November 2009, but I haven't found any obvious change specific to append recording in this period.
I debugged this a bit. Pretty sure this has to do with AudioIO.cpp around current lines 1763 through 1780. I think it has to do with the "cut the latency out" calculation being off. That code is mostly Martyn's, so I'll bring him into this conversation.
Steve wrote: > There is also another slight change in that when recording is resumed with > Shift+R there is sometimes a brief bit of silence at the start of the new > recording. I'm only seeing this "gap" when using PulseAudio. If I select hw:0,0 as the recording and playback device (so accessing the hardware more directly via ALSA) then I don't see these gaps. I presume that what is happening here is that there is some additional buffering occurring in PulseAudio that is not (and probably can not) be corrected in Audacity. > I'm still getting a brief section > of playthrough every 6 seconds. This happens whatever the sound system, Pulse, ALSA or Jack. A short section of what has just been recorded is echoed. Example: Record 10 seconds of silence, then stop. Append record and record sound that ends just before the recording reaches 12 seconds, and if you get the timing right that sound will "echo" just after 12 seconds. Although the above numbers might suggest that this is to do with block boundaries, loading the final .au file shows the recorded sound and the echo near the middle of the same block. The playthrough problem ONLY occurs when there is ONE audio track in the project. I have been unable to reproduce the issue if there is more than one audio track. The problem can occur whether the track being recorded is mono or stereo. Applying any kind of edit to the first recorded track prior to Append Record seems to prevent the echo from occurring. If "Latency correction" is set to zero, the debug build throws an error: AudioIO.cpp(1779): assert "bResult" failed in StopStream(). When using Jack, latency correction for Append Record needs to be set to zero, otherwise the appended recording has the start deleted (the amount removed is equal to the latency correction length). It seems that Jack does not use Audacity's audio buffer.
(In reply to comment #8) Steve wrote: > I'm only seeing this "gap" when using PulseAudio. This part is making sense now. Sorry, I think this was user error. The latency when using PulseAudio is greater than when using ALSA directly. Append record removes a bit of audio from the start of the new recording according to the latency correction setting. I had latency correction set up for using ALSA directly, so when I used PulseAudio there was insufficient latency correction, and thus a gap at the start of the appended recording. As far as I can see, the code in AudioIO.cpp lines 1772 through 1780 works correctly for ALSA and PulseAudio, provided that the user has correctly set up latency correction. Where it falls down is when Jack is used the latency correction should not be applied (though latency correction is required for overdubbing).
(In reply to comment #9) >> I'm only seeing this "gap" when using PulseAudio. > This part is making sense now. Sorry, I think this was user error. OK, so it looks here too on my Ubuntu netbook that the silence at the start of the append recording is not new in r12432. I was using the hw (direct hardware) device (avoiding Pulse) in earlier tests, but Pulse in the tests after 12432. On my netbook, Pulse has the silence at the start in 12432 and before 12432, unless I set audio to buffer < 50 ms. Otherwise the higher the buffer, the longer the silence. If using the hw device, (or Windows) I don't get any silence at the start, whatever the buffer setting, before or after 12432. So that is like Steve sees on JACK, but apparently he still sees the silence with the hw device? On Windows and Ubuntu (pulse) I do still seem to get greater replicability of the playthrough after 12432 than before.
(In reply to comment #10) Gale wrote: > So that is like Steve sees > on JACK, but apparently he still sees the silence with the hw device? No, I only see the gap with Pulse. > On my netbook, Pulse has the silence at the start in 12432 and before 12432, > unless I set audio to buffer < 50 ms. Otherwise the higher the buffer, the > longer the silence. Try setting up latency correction before you test. On my machine, if the latency correction is right, then that exactly consumes the silent gap. Steve wrote: > Where it falls down is when Jack is used the latency > correction should not be applied (though latency correction is required for > overdubbing). That's not quite correct (though it's difficult to see because the shift is so small). The latency correction part looks to be correct with Jack as well. The only issue I'm seeing here that is currently unresolved is the echo/playthrough problem.
(In reply to comment #3) > Does revision 12432 fix it? I think the 'crash' problem in debug is actually caused by the wxASSERT in Envelope::GetValues. It being called from the AudioThread and the comment that Michael left in AudioIO.cpp says that's not allowed: " //don't do anything if we have no length. In particular, Process() will fail an wxAssert //that causes a crash since this is not the GUI thread and wxASSERT is a GUI call." If this is truly the cause of the crash then we could just remove the assert in Envelope::GetValues and leave a comment there not to put one in, but then how do we debug that code next time? So those 12432 fixes work for me, but the comments with them don't.
(In reply to comment #12) >> Does revision 12432 fix it? > > I think the 'crash' problem in debug is actually caused by the wxASSERT in > Envelope::GetValues. Yes, definitely my fix in the caller, WaveTrack::GetEnvelopeValues(), was simply to stop that bad value being sent to Envelope::GetValues(). The wxASSERT that fails is actually there, with James's comment that "If bufferLen ==0 we have probably just allocated a zero sized buffer.". It being called from the AudioThread and the comment that > Michael left in AudioIO.cpp says that's not allowed: > " //don't do anything if we have no length. In particular, > Process() will fail an wxAssert > //that causes a crash since this is not the GUI thread and > wxASSERT is a GUI call." > > If this is truly the cause of the crash then we could just remove the assert in > Envelope::GetValues and leave a comment there not to put one in, but then how > do we debug that code next time? -1. The wxASSERT is there for a reason. Wouldn't fire in Release build, but would probably damage data. Rather than the fix I made in revision 12432, we could replace the wxASSERT with a return if (bufferlen <= 0), but that's not fixing the deeper issue. I'll look into a deeper fix. Overall, it's looking like an off-by-one error. Thanks for your help, Martyn, and sorry I was apparently wrong about it being your code -- was just using SVN Blame on the code in WaveTrack::GetEnvelopeValues(). On the bright side, you've helped! > > So those 12432 fixes work for me, but the comments with them don't. By "loop completion" I just meant that it shouldn't continue in that loop. Not the deep fix, but stops the crash.
(In reply to comment #1) > > I am not clear - is the crashing you see still confined to debug builds > (release builds don't crash)? If so, that seems to suggest a lower rating than > P2. > Your call, but wxASSERT is generally used for a good reason, that it's crucial the condition is met.
More debugging: This always occurs where avoiding the wxASSERT was in the second bit I added in revision 12432, where, for example: dClipStartTime 2.9372335600907031 dClipEndTime 5.4370068027210881 startTime 4.8120634920634924 endTime 5.4370068027210889 clip->GetStartSample() gives 129532 clip->GetEndSample() gives 129532 So a valid time span, that's big enough to have 27560 samples, gives nClipLen as 0 samples. So rlen was being set to 0, => wxASSERT. Means that somewhere mNumSamples is not being updated, I think. Will still work on it, but done for the day and hoping this might trigger some insight.
(In reply to comment #11) > > Steve wrote: > > Where it falls down is when Jack is used the latency > > correction should not be applied (though latency correction is required for > > overdubbing). > That's not quite correct (though it's difficult to see because the shift is so > small). The latency correction part looks to be correct with Jack as well. Perhaps, but doesn't this still mean (with Windows/hw/JACK) setting different latency corrections for append recording and overdubbing? Using Pulse I can get away without doing that. On Windows it looks like I need zero latency correction when append recording, otherwise some negative amount would be needed with any of the released hosts.
(In reply to comment #16) (Sorry this is a bit long winded. If any further discussion is required about this point then I think it should be moved to -quality@) Gale wrote: > On Windows it looks like I need zero latency > correction when append recording... That's what I thought too, but looking closer I now think not. For hw:, jack and apparently Windows, when Append Record starts it is like plugging into an existing stream, so that valid data comes through immediately. For PulseAudio, when Append Record starts it is like creating a new stream, so it takes a few moments before the data starts to flow. I therefore jumped to the conclusion (which I think you have too) that for Append Record when using Jack (or hw: or Windows) that there should be no latency correction. However I'd not really considered the case where Append Record may be used at the same time as overdub recording. If there is another track present, then latency correction must be used so that the new recording is synchronised with the other track. Hypothetically (I can't think of any practical way to test): 1) If there are two click tracks of different lengths and we Append Record the shorter one, then latency correction is required for correct synchronisation (this part can be easily tested and shown to be the case). 2) The same scenario as (1) but the longer track is muted. Although we cannot hear the longer track, we would still I think expect the same latency correction. The recorded audio will then be synchronised with muted track, and with the beat of the first part of the shorter track. 3) The same scenario as (2) but the longer track is Cut edited before we start recording. We would still expect the same latency correction so that the new recording is synchronised with the first part of the shorter track, and will be synchronised with the longer track if we paste it back in the same place. 4) The same scenario as (3) but we cut the longer track, then delete its wavetrack. Again we would expect the same latency correction for the same reasons. 5) The same as (4) but we never had the longer click track in the first place. I don't see any reason why the new audio should be in a different position (relative to the first part of the shorter track), so again the latency correction should be the same. As shown by PulseAudio, due to latency, it is not guaranteed that there will be any audio recorded in the first few milliseconds after the Record button is pressed (has this been tested with ASIO, WASAPI and on Mac OS X?). I don't think that we would want to have different Append Record behaviours depending on whether other tracks were present, muted, soloed etc. Provided that latency correction is set up correctly, one consistent scheme of applying latency correction with Append Record seems to consistently work correctly.
(In reply to comment #15) > --- Comment #15 from Vaughan Johnson <vaughan@audacityteam.org> 2013-07-24 00:09:41 EDT --- > More debugging: > > This always occurs where avoiding the wxASSERT was in the second bit I added in > revision 12432, where, for example: > > dClipStartTime 2.9372335600907031 > dClipEndTime 5.4370068027210881 > > startTime 4.8120634920634924 > endTime 5.4370068027210889 > > clip->GetStartSample() gives 129532 > clip->GetEndSample() gives 129532 > > So a valid time span, that's big enough to have 27560 > samples, gives nClipLen as 0 samples. Ouch! But why are we here? There is only one track and one clip and we are appending to it, so why look at playback envelopes? There appears to be a deeper problem here, that is, back in the stack. In 'ControlToolBar::OnRecord(' we appear to have 2 playbackTracks when we shouldn't have any (??) I gtg TTFN Martyn > So rlen was being set to 0, => wxASSERT. > > Means that somewhere mNumSamples is not being updated, I think. > > Will still work on it, but done for the day and hoping this might trigger some > insight. >
Vaughan's fix to prevent the crash is correct, for the AudioThread. There are outstanding issues on the 'normal' thread. The sequence to reproduce this is: 1) In a blank project, R (record) 2) Shift+A (calls Menus->OnPlayStopSelect) 3) Shift+R (calls OnRecordAppend which calls OnRecord with shift set on). Now, OnPlayStopSelect does not actually select the track, I think. (The comments say '// The code for "OnPlayStopSelect" is simply the code of "OnPlayStop" and "OnStopSelect" merged.') I note that OnStopSelect() is not currently in use. But is the track selected? I think not. ControlToolBar::OnRecord with shift set on (ie if we have 'Overdub' set on) sets 'duplex' and copies all the current tracks for playback (abt li 795). Then it selectively removes them from the list (li 827) (I don't understand this bit entirely) but since our tt isn't selected (see above), it isn't removed (but doesn't need playing, so should be). The values that I am seeing in 845 if (t1 < t0) { 5.4131519274376414 vs 5.4131519274376636 look suspiciously like they have fp rounding issues as well, and may account for the 'gap'/cutline. Another case for using TimeToLongSamples? TTFN Martyn
*** STEPS UPDATED *** There have been many changes in error handling since July 2013. Old steps were outdated, so I added new ones.
Work done: https://github.com/audacity/audacity/commit/c8b2cc9d3151d65b7e0a4368555416cf1716de2d This fixes the ASSERT seen with my steps to reproduce. It is not clear what the residual issue on this bug is/was. Either this bug should be closed, or further new steps to reproduce written. The title for the bug I fixed with this commit would be: "ASSERT when adding recording not at start of track."
(In reply to James Crook from comment #21) > Old steps were outdated This was a moonphasey bug and could well behave differently on different platforms, and with different latency correction settings. IMO it was unlikely in this case to be a safe assumption to delete the old steps, so better to leave them clearly marked as OLD, NOT CURRENTLY REPRODUCING. All I can say is that so far, I don't see or hear the problem on Windows 10 in HEAD with default settings.
*** STEPS UPDATED *** I recreated the old steps, as B1-B4 updating them to the new bindings. I've also, per comment #23 marked this bug as Moonphase. It was previously marked as repeatable. Note that I corrected some logic in record-append for Dark Audacity, since DA uses it by default, and Audacity has that new logic. So in particular the selection of tracks to record to is more robust than it was. I've left the steps A1-A4 in for now, but I am pretty sure they are fixed, as they were a repeatable problem and don't occur for me now. Perhaps when it is confirmed A1-A4 are fixed they can be removed.
(In reply to James Crook from comment #24) > I recreated the old steps, as B1-B4 updating them to the new bindings. > I've also, per comment #23 marked this bug as Moonphase. It was previously > marked as repeatable. Thank you. A question here is that we don't have a good way to track that it has reasonably high priority to test despite moonphase rating. I added a "moo-test-fixed" keyword which might help. Also when Steve and I tested this at the time, we found many weirdnesses and bugs with Append Record which I don't think ever got on Bugzilla. We agreed it would be a very time consuming matter to discuss them, but now Append Record is default. This would be another reason to look at the actual behaviour again after the improvements we've had.
*** STEPS UPDATED *** Removed the 'A' steps from Steps to reproduce. They HAVE been fixed. I've left the B steps in place, removed the keyword 'moo-test-fixed' and instead promoted this from P3 to P2 to reflect Gale's wish in comment #25 that it be tested thoroughly. I also updated the release notes, now that record append is the default. Marked DEVEL FIX MADE based on my comment #22 The 'A' steps removed were: "In a debug build: A1. Open Audacity A2. Add new track A3. Click in the track at time 1.0 A4. Press R (for append record) Audacity shows an assert dialog. envelope.cpp(1014) "when <= (mTrackLen)" These 'A steps are believed fixed. They were repeatable."
Well I've tested the B steps several times - today on the 20Aug17 builds on macOS Sierra 10.12.6 and W10 - and on eralier alpha builds and I can't get this moonphase bug to appear.
I banged on this a bit today on Mac and can't get it to happen. I changed the release note to make it clear that the sound heard is audio previously recorded on the track being appended to, and that the problem is rare.
I've been trying to reproduce this today, and not been able to. Also, I don't recall the last time I experienced this bug, but it was a long time ago.
3x reports of 'works for me', plus work done to address the original headline complaint - I'm going to be bold and mark this as RESOLVED WORKSFORME. We can always reopen or open a new bug if we have reports of it.
