MantisBT

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002628ardourbugspublic2009-04-09 05:522011-09-28 09:22
Reportercolinf 
Assigned Tocth103 
PrioritynormalSeveritycrashReproducibilitysometimes
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionSVN/2.0-ongoing 
Target Version2.8.12Fixed in Version2.8.12 
Summary0002628: [PATCH] incorrect crossfades created, causes crash
DescriptionYesterday, for the first time ever, Ardour crashed while I was actually recording a take. It did it three times, at exactly the same point in the same session.

It was reproducible enough for me to get it to crash in the debugger as well, which I did another three times. The backtraces are attached, but they don't point conclusively to anything that I can see.

Anyway, I'll upload the session file too. To reproduce:
 - open record-crash.ardour
 - record-enable 'josie whistle' track (probably other tracks too, but I haven't tried yet)
 - start recording
 - when the transport reaches 00:02:56:27 or thereabouts, Ardour SEGVs.
TagsNo tags attached.
Attached Fileslog file icon hare-crash-while-recording-1.log [^] (9,730 bytes) 2009-04-09 05:53
log file icon hare-crash-while-recording-2.log [^] (7,991 bytes) 2009-04-09 05:53
log file icon hare-crash-while-recording-3.log [^] (12,576 bytes) 2009-04-09 05:54
? file icon record-crash.ardour [^] (368,667 bytes) 2009-04-09 05:54
? file icon ardour-crossfade-crash.ardour [^] (43,963 bytes) 2009-04-09 08:01
log file icon valgrind.log [^] (198,825 bytes) 2009-04-09 12:38
patch file icon check-for-bogus-crossfades.patch [^] (2,278 bytes) 2009-04-12 11:00 [Show Content]
patch file icon assert-read_at-is-in-range.patch [^] (781 bytes) 2011-07-29 11:44 [Show Content]

- Relationships

-  Notes
(0005868)
colinf (updater)
2009-04-09 07:14

Actually, it doesn't have to be recording to crash with this session: I just tried exporting to .wav and it fell over at (as far as I can see) the same place.

Rebuilding with scons DEBUG=1 now to see if I can get any better backtraces...
(0005869)
colinf (updater)
2009-04-09 08:01

I've narrowed down the cause of the crash in this session to something on the concertina track: probably the crossfade. Removing all of the other tracks & busses and exporting causes a crash in exactly the same place.

Uploading a stripped-down session file in a moment...
(0005870)
colinf (updater)
2009-04-09 09:31

This now smells like a memory corruption bug to me: the time of the crash seems to move around as I make small changes to the session, and Ardour also sometimes crashes on positioning the play-head, saving etc. after loading the session.
(0005873)
colinf (updater)
2009-04-09 13:07

Hah, caught it with arval.

Attached valgrind.log from exporting a file from a session which consistently crashes. It looks like the crossfade buffer is being overrun by 4 bytes.

Could this be related to the fix for #1737?
(0005879)
colinf (updater)
2009-04-10 03:35

I tried reverted the changes to crossfade.cc from svn r4720, which were the fix for bug #1737, but it appears to make no difference, so my hunch about that fix was wrong.
(0005892)
colinf (updater)
2009-04-12 10:58

It looks like spurious Crossfade objects sometimes get created for two regions where the end of the first lies outside the second, and/or the beginning of the second lies outside the first. When this happens, read_at() can be called on one of the regions with nonsensical parameters, leading to it overrunning its buf parameter, with predictably undesirable results.

I've somehow managed to create several sessions now with this error, though I don't have a simple recipe to reproduce it yet. It seems to only happen when there is a full crossfade in a track.

Anyway, I'm attaching a patch that checks that the regions cover the crossfade before rendering it: obviously this doesn't address the root bug, but it at least lets me open & use the sessions where this has happened without crashing Ardour.

The patch also adds some checks on the return value of sscanf in the Crossfade::set_state (const XMLNode& node) function, to avoid valgrind warning about "Conditional jump or move depends on uninitialised value(s)". I don't believe this is actually related to the bug, but it's probably a worthwhile improvement anyway.
(0005975)
colinf (updater)
2009-05-05 11:49

I'm seeing 'bogus' Crossfade objects in a lot of my session files with check-for-bogus-crossfades.patch applied: I think that setting layering mode to 'Most Recently Added is Higher' may be the cause.
(0009017)
colinf (updater)
2010-09-08 05:11

Finally found a simple recipe that seems to reproduce this:

* record a region
* select and copy a shorter section of the region with the range tool
* paste the copy over and completely within the original region
* split the original region somewhere underneath the new region

Bam! With my patch applied, I then get the "in region is outside crossfade: Audio 1-1.2 => Audio 1-1.3" message. I presume without this patch I'd see the original crash.

P.S.: Is there any way I can edit the bug description here? I'd like to give this one a better description now I've tracked it down.
(0009156)
paul (administrator)
2010-09-24 05:36

how do you split the original region when its under the new region? when i tried this with all 3 layering models by putting the original region on top explicitly, your patch's message was not triggered ....
(0009161)
colinf (updater)
2010-09-25 03:16
edited on: 2010-09-25 04:22

I'm not at my computer at the moment, so I might have forgotten something, but AFAIR what I did was:

* layering mode "most recently added is higher"
* edit point is playhead
* link region/track selection is off
* short crossfades
* create two regions as above
* click on exposed part of lower region to select it
* position playhead within upper region
* press 'S' to split lower (selected) region

I think the crash (or error output) happens as the butler pre-reads audio from disc, so if the bogus crossfade is created more than 5 seconds from the playhead you won't see the error until you roll the transport past it. As far as I can tell, the crossfade is created at the end of the upper region: I only tried the recipe above with quite short (<5 sec) regions.

(0009171)
paul (administrator)
2010-09-26 07:31

i've been able to generate the error messages, but i'm not sure you understood the semantics for ::read_at(). its not required that the crossfade be within the time range for which ::read_at() is called, so there is nothing wrong with, for example, this case:

in region is outside crossfade: Audio 1-2.4 => Audio 1-2.5 read at 68900 + 33386 ( 102285 ) in = 0 ... 50795

(i added some extra output). the "in" region covers the range 0..50795. The ::read_at() range is 68900 .. 102285 ... the crossfade had ::read_at() called upon it, and should have just handled this by doing nothing (since it doesn't cover the read_at range at all.
(0009172)
paul (administrator)
2010-09-26 07:32

that said, the code in AudioPlaylist::read looks like this:

    for (Crossfades::iterator i = _crossfades.begin(); i != _crossfades.end(); ++i) {
        if ((*i)->coverage (start, end) != OverlapNone) {
            relevant_xfades[(*i)->upper_layer()].push_back (*i);
        }
    }


so one would not expect an xfade that doesn't exist within the read range to be actually read.

more investigation required ....
(0009173)
paul (administrator)
2010-09-26 08:10

pretty sure from using valgrind that this is a red herring. there is memory corruption,however. looking at that now.
(0009174)
paul (administrator)
2010-09-26 08:54

my ability to reproduce the valgrind errors has gone ...
(0009177)
colinf (updater)
2010-09-27 08:42
edited on: 2010-09-27 13:01

AudioRegion::read_at() (in audioregion.cc line 543) says:
    /* precondition: caller has verified that we cover the desired section */

and it was in the calls to _out->read_at() or _in->read_at() in Crossfade::read_at() that I saw the crash when that precondition wasn't satisfied, so I think that checking that the crossfade overlaps the in and out regions is probably the right thing.

[EDIT] Sorry, what I meant to say was that the memory corruption that eventually caused the crash for me happened in the calls to _in->read_at() or _out->read_at(). It was over a year ago...

(0011234)
colinf (updater)
2011-07-29 11:40
edited on: 2011-07-29 12:03

I still have the changes attached here as check-for-bogus-crossfades.patch in my working copy of A2, and I still see the messages they output from time to time. I know the patch isn't a fix, but it did prevent ardour crashing in these circumstances in my experience, so is it worth considering something like this for 2.8.12?

Obviously fixing whatever causes the bogus crossfades in the first place would be ideal, but it would probably take me a very long time, and it's also not enough to avoid problems with saved sessions containing such bogus crossfades.

(0011236)
colinf (updater)
2011-07-29 12:02

I made a patch to test my understanding of what's actually going wrong here, attached as assert-read_at-is-in-range.patch. It's not meant to be applied. Please ignore the (unrelated) second hunk in there, which is left-over from looking into something unrelated: sorry!

However, with the assert in the assert in libs/ardour/audioregion.cc in place and removing the changes from libs/ardour/crossfade.cc, following the recipe above triggers the assertion.

So, my questions are:

 - is that assert a valid expression of the precondition in the comment above?
 - if it is, what are the consequences of violating that precondition?
 - without the assert, I see messages like "SndFileSource: @ 406765 could not read 319251 within udio 1-1.wav (No Error.) (len = 726016)". There's no 'udio 1-1.wav' in my test session, but there is an 'Audio 1-1.wav'. Is that a sign that something has scribbled on memory it shouldn't, or is it to be expected?
(0011525)
colinf (updater)
2011-09-15 07:40

Looks like the last two (valgrind warning fix) hunks of check-for-bogus-crossfades.patch got applied ages ago already: thanks!

However, I can still reliably provoke a crash with a2.8.12-beta and the record-crash.ardour session file attached above (it actually now trips an assert() rather than a SEGV, but it's still a repeatable crash: I can upload a backtrace if anyone wants). Applying the first hunk of check-for-bogus-crossfades.patch reliably prevents the crash for me.

Any chance of this being fixed in 2.8.12?
(0011558)
paul (administrator)
2011-09-21 10:50

first chunk committed as rev 10111
(0011593)
cth103 (administrator)
2011-09-28 04:35

I believe this was all resolved in 2.8.12.
(0011604)
colinf (updater)
2011-09-28 09:22

Crash fixed in 2.8.12. Thank you!

- Issue History
Date Modified Username Field Change
2009-04-09 05:52 colinf New Issue
2009-04-09 05:53 colinf File Added: hare-crash-while-recording-1.log
2009-04-09 05:53 colinf File Added: hare-crash-while-recording-2.log
2009-04-09 05:54 colinf File Added: hare-crash-while-recording-3.log
2009-04-09 05:54 colinf File Added: record-crash.ardour
2009-04-09 07:14 colinf Note Added: 0005868
2009-04-09 08:01 colinf Note Added: 0005869
2009-04-09 08:01 colinf File Added: ardour-crossfade-crash.ardour
2009-04-09 09:31 colinf Note Added: 0005870
2009-04-09 12:38 colinf File Added: valgrind.log
2009-04-09 13:07 colinf Note Added: 0005873
2009-04-10 03:35 colinf Note Added: 0005879
2009-04-12 10:58 colinf Note Added: 0005892
2009-04-12 11:00 colinf File Added: check-for-bogus-crossfades.patch
2009-05-05 11:49 colinf Note Added: 0005975
2010-04-24 03:28 cth103 Category bugs => bugs2
2010-04-24 03:31 cth103 Category bugs2 => bugs
2010-09-08 05:11 colinf Note Added: 0009017
2010-09-20 14:14 cth103 cost => 0.00
2010-09-20 14:14 cth103 Target Version => 2.8.12
2010-09-20 14:14 cth103 Summary Crash while recording (SEGV) => [PATCH] Crash while recording (SEGV)
2010-09-24 05:29 paul Summary [PATCH] Crash while recording (SEGV) => [PATCH] incorrect crossfades created, causes crash
2010-09-24 05:36 paul Note Added: 0009156
2010-09-25 03:16 colinf Note Added: 0009161
2010-09-25 04:22 colinf Note Edited: 0009161
2010-09-26 07:31 paul Note Added: 0009171
2010-09-26 07:32 paul Note Added: 0009172
2010-09-26 08:10 paul Note Added: 0009173
2010-09-26 08:54 paul Note Added: 0009174
2010-09-27 08:42 colinf Note Added: 0009177
2010-09-27 13:01 colinf Note Edited: 0009177
2011-07-29 11:40 colinf Note Added: 0011234
2011-07-29 11:44 colinf File Added: assert-read_at-is-in-range.patch
2011-07-29 12:02 colinf Note Added: 0011236
2011-07-29 12:03 colinf Note Edited: 0011234
2011-09-15 07:40 colinf Note Added: 0011525
2011-09-21 10:50 paul Note Added: 0011558
2011-09-28 04:35 cth103 Note Added: 0011593
2011-09-28 04:35 cth103 Status new => resolved
2011-09-28 04:35 cth103 Fixed in Version => 2.8.12
2011-09-28 04:35 cth103 Resolution open => fixed
2011-09-28 04:35 cth103 Assigned To => cth103
2011-09-28 09:22 colinf Note Added: 0011604
2011-09-28 09:22 colinf Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker