Bugs item #2122004, was opened at 2008-09-21 19:19
Message generated for change (Comment added) made by sf-robot
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=104932&aid=2122004&group_id=4932
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: midi
Group: None
>Status: Closed
Resolution: Fixed
Priority: 9
Private: No
Submitted By: Flvio Luiz Schiavoni (schiavoni)
Assigned to: D. Michael McIntyre (dmmcintyr)
Summary: Midi Recording crash
Initial Comment:
I deleted all tracks in a rosegarden project. Then I added one and tried to record a midi. Crashed.
Composition::getTrackById(0) - WARNING - track id not found, this is probably a BUG /home/flavio/downloads/rosegarden-1.7.0/src/base/Composition.cpp:1539
Available track ids are:
60
61
62
63
64
65
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
----------------------------------------------------------------------
>Comment By: SourceForge Robot (sf-robot)
Date: 2008-10-11 02:20
Message:
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-26 23:46
Message:
OK, fair enough on where it goes and why it won't work there. Sorry I
didn't give you the hint about MappedEvent ctors initializing to 0 before
you made the discovery independently.
I've played with all of this new work, and it seems to fix the crashes
without causing weird update problems like my own fixes did, and it all
seems to be good, except for your new implementation of my old tentative
fix for this transposition issue.
I'd say my fix isn't so tentative anymore in terms of the behavior being
acceptable. We play back what you recorded, and adjust the notation so a
real instrument would be able to play the same thing. The keyboard is
always assumed to be in concert pitch.
That will work, but this implementation is broken. I'm investigating, and
will report elsewhere.
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-26 22:42
Message:
The sample file attached to this report seems to work now.
----------------------------------------------------------------------
Comment By: Chris Cannam (cannam)
Date: 2008-09-26 14:01
Message:
Committed (to trunk, rev 9420) what I believe to be a fix to the underlying
problem.
----------------------------------------------------------------------
Comment By: Chris Cannam (cannam)
Date: 2008-09-26 13:02
Message:
Following up on the aside in my previous comment -- yes, the track ID in
MappedEvent is used; it's used by the sequencer when playing events to
determine whether they're supposed to be muted or solo'd. Of course, those
events came from the GUI side so they do have valid track IDs.
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-26 12:47
Message:
One thing I'm trying to get through the morning fog in my head, Julie, is
that if you fiddle around long enough, then the problem becomes ID 1
instead of 0, and then ID 2 instead of 0. It's returning a pointer to a
track that should no longer exist that's the real problem. Not 0
specifically. Ugh. Out the door!
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-26 12:46
Message:
I already fixed that specific problem in less brain damaged code, Julie,
but I don't have time to clean it up and commit it. I overslept rather
badly, and have just a minute before I have to head out the door (with no
breakfast :( )
I'm not entirely sure using 0 for an ID is the exact problem here, because
"if (track)" isn't comparing an ID to an ID, but it' seeing if the pointer
to track n is valid, and the real issue is that Segment::getTrackId() or
whatever is returning a pointer to track 0 if in doubt.
Or something a bit like that. I had my head around it better last night.
I wish I had time to tinker with it right now, to at least clean up my
code, but I guess we just have to risk reinventing the wheel here. None of
it is that big a deal if we end up conflicting over this.
----------------------------------------------------------------------
Comment By: Chris Cannam (cannam)
Date: 2008-09-26 12:44
Message:
OK, the root cause is kind of subtle.
The problem is the line at RosegardenGUIDoc.cpp:1803, where we have:
TrackId tid = (*i)->getTrackId();
Here "i" is an iterator into a MappedComposition, that is, a sequence of
MappedEvents which have come straight from the sequencer driver via shared
memory.
So, MappedEvent has a track ID data member, which is what's being
retrieved here. However, the sequencer side of things doesn't actually
deal in tracks, only in instruments, and this data member has never been
set -- it is always going to have the default value, which is 0.
0 is a valid track ID in most cases -- it's the first track in the default
composition. But if you've deleted the original first track, you won't
have a track 0 at all. Hence the crash. (This also suggests that the
"tentative fix for #1597279" would only work when recording onto the first
track?)
The problem is bigger than just a question of looking up the track in the
wrong place. In fact, at this point in the process, nobody has decided
which track this event will end up on -- that doesn't happen until
insertRecordedEvent is called. One reason for that is that, if your
recording filters are set appropriately, this single event could actually
end up going on to more than one track.
So, it seems the bit of code that fixes recorded pitch according to track
transpose should be moved to insertRecordedEvent or similar, since a single
event may have to be transposed more than one way depending on the target
track.
(I am not sure whether there is any good reason why MappedEvent still has
the track-ID member at all, actually. I can't think how it could be of any
use, since the sequencer would have no reason to read it and no ability to
write it. I'll check it and, if it isn't used, I'll remove it.)
----------------------------------------------------------------------
Comment By: Julie S. (msjulie)
Date: 2008-09-26 11:50
Message:
Michael,
You are getting 0 for pitch when track is null because you are not setting
the pitch
In Rosegarden::RosegardenGUIDoc::insertRecordedMidi
Existing code: (/w your fix)
if (track) {
pitch = (*i)->getPitch() - track->getTranspose();
}
now add:
else {
pitch = (*i)->getPitch();
}
I'll submit the change when I get to the other computer in about an hour.
The real issue, having not looked deeply into it yet, is that we are using
a a track ID of 0. and checking track for 0. I'll look more into this
today, but if that is the case, this is just the tip of the iceberg of
hidden problems. We can't use 0 for a track number and a null pointer.
They can't serve two purposes.
If this is so, we need to start track at track 1 or greater and reserve
track 0 for null pointing. I'll see what I can find today.
Julie S.
----------------------------------------------------------------------
Comment By: Julie S. (msjulie)
Date: 2008-09-26 11:44
Message:
Michael,
You are getting 0 for pitch when track is null because you are not setting
the pitch
In Rosegarden::RosegardenGUIDoc::insertRecordedMidi
Existing code: (/w your fix)
if (track) {
pitch = (*i)->getPitch() - track->getTranspose();
}
now add:
else {
pitch = (*i)->getPitch();
}
I'll submit the change when I get to the other computer in about an hour.
The real issue, having not looked deeply into it yet, is that we are using
a a track ID of 0. and checking track for 0. I'll look more into this
today, but if that is the case, this is just the tip of the iceberg of
hidden problems. We can't use 0 for a track number and a null pointer.
They can't serve two purposes.
If this is so, we need to start track at track 1 or greater and reserve
track 0 for null pointing. I'll see what I can find today.
Julie S.
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-26 00:50
Message:
I can get a crash importing that empty MIDI file, but let's ignore that for
now, because we're not likely to fix that problem. All it really deserves
is a warning that the file you're trying to import is empty, but I don't
know right off where to go put that warning, and I'm not going to spend
time hunting it down.
As to the crash at hand here, I can avoid the crash itself by testing to
make sure my track pointer is pointing at something. I committed that to
trunk/ rev 9418.
Ahem. Then I fixed the bug I just introduced, and committed that in rev
9419.
The track code I'm using is very standard boilerplate stuff, and this
isn't my bug. My code really shouldn't be crashing at all (although I
still should have been testing to make sure the pointer was valid before
accessing what was on the other side of it.)
The real problem here is that something bizarre is happening when the
track with ID 0 disappears.
I'm still trying to resolve the extent of that weirdness over on:
https://sourceforge.net/tracker/index.php?func=detail&aid=2129476&group_id=4932&atid=104932
----------------------------------------------------------------------
Comment By: Flvio Luiz Schiavoni (schiavoni)
Date: 2008-09-25 12:54
Message:
Hi D.
Thanks again. I think I can add a track 0 to all my projects files.
Now that you found the bug (thanks again), let's talk about the no-tracks
Rosegarden file.
You are right, there is no way to delete all tracks, but I did this:
I recorded a midi file with arecordmidi.
I opened it in Rosegarden to see if it was ok.
I had a cable connection problem, so I recorded nothing.
And there is the no-tracks Rosegarden file.
Rosegarden can't add a track to this dummy midi file.
If you save this as a Rosegarden (.rg) file, Rosegarden will crash trying
to open it.
I know that maybe I'm the only Rosegarden user that did this and it
doesn't seem to be a bug to me.
Anyway, I attached the midi file but you can repeat it by your self.
Att
Flavio Schiavoni
P.S.: The brazilian portuguese translation is almost ready.
File Added: teste.midi
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-25 04:37
Message:
This turns out to be a good catch! The fault in reproducing this was
mine.
I got the crash with your file, and then I took another look at everything
and kept narrowing it down. Turns out when I tested this before, I hadn't
realized that I saved my autoload with track 11 selected. When I started
at the top of the screen and deleted everything below the top, I deleted
everything from 5 down to 64, and then deleted up 5, 4, 3, 2, and left
track 1 behind. When I added tracks from there, everything was fine.
If you delete track 1, and then try to record MIDI on any track (such as
the new track 1) then if you actually capture events that need to be added
to a segment (as opposed to an empty recording) then you get this crash
every time.
The key bit of the trace seems to be:
#0 0x08494847 in Rosegarden::RosegardenGUIDoc::insertRecordedMidi
(this=0x89baaf8, mC=@0xbfee81b4)
at rosegarden-1.7.2/src/document/RosegardenGUIDoc.cpp:1805
1805 pitch = (*i)->getPitch() - track->getTranspose();
(gdb) bt
#0 0x08494847 in Rosegarden::RosegardenGUIDoc::insertRecordedMidi
(this=0x89baaf8, mC=@0xbfee81b4)
I'm not sure what to make of the overall problem. It looks like a classic
Track ID issue. I'm too tired to try to sort it out tonight, but it looks
pretty cut and dried, and I guess this is a 1.7.3 release in the making,
because this is pretty nasty. :(
For now, it appears that it is easy to salvage your work. You need to
create a track with the ID 0.
If you are in a big hurry, you can email me any broken files, and I'll
hack them by hand to try to work around this problem. After I change the
first track in your file from ID 63 to ID 0 I seem to be able to continue
normally, although it is very possible I might break something again by
adding or deleting or moving tracks.
I haven't yet determined for sure whether this is a file corruption bug,
but I think it is probably the case that I did something stupid in my code
with perfectly good data. Yes, I recognize the code where the crash is. I
wrote it to fix another bug. How about that. :)
I expect once I fix this bug, all your files will work fine the way they
are right now. If you can wait up to a few days, we'll find out for sure.
I might fix it as soon as tomorrow evening. I don't yet understand exactly
what's wrong, but I understand this kind of problem, and have a general
idea what I probably did wrong. I just need to do a little homework.
After I get some sleep.
For now, I've hacked that one file, and I'm attaching it to this report
for you to try. Maybe you can keep working on this one while I figure out
how to solve the underlying problem.
File Added: hacked.rg
----------------------------------------------------------------------
Comment By: Flvio Luiz Schiavoni (schiavoni)
Date: 2008-09-24 21:13
Message:
Now I attached my project file.
Maybe with this file you can repeat the crash.
Att
Flvio Schiavoni
File Added: vida social.rg
----------------------------------------------------------------------
Comment By: Flvio Luiz Schiavoni (schiavoni)
Date: 2008-09-24 21:11
Message:
Hi D. thanks to the answer
I Updated my Rosegarden to 1.7.2 from the svn repository and the midi
recording is still crashing.
It's not one song, but all songs that I had done the same thing: deleted
all tracks (left one) and recorded a lot of audio stuffs. I also wrote some
instruments tracks and now I can't record midi.
If I import a midi file, I can record a midi track but can't record it if
I use one of my projects files.
I tried with my Yamaha Synth and with the Virtual Midi Keyboard. Both
crashed.
One interesting thing: Rosegarden crashs but rosegardensequencer keep
playing all the stuff.
I attached the full log and one project file.
thanks
Schiavoni
File Added: midicrash.log
----------------------------------------------------------------------
Comment By: D. Michael McIntyre (dmmcintyr)
Date: 2008-09-21 20:34
Message:
I can't repeat this with Rosegarden 1.7.2.
First, it isn't possible to delete all the tracks. If I delete every
track that can be deleted, there is still one left.
Second, if I delete all but this final track, then add another one, I can
record MIDI on either of them without a crash.
I'm sure there was a real crash, but this procedure is not sufficient for
repeating it. Further investigation is required to determine a way to
reproduce the crash.
If you were starting with a pre-existing composition, the source of the
bug could be any number of things. The best place to start would be with
that file. If we can at least get a repeatable crash from that file,
that's something to go on, although it might still be impossible to find
and fix this bug.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=104932&aid=2122004&group_id=4932
|
