Re: [Audacity-devel] more bugzilla review

On Dec 21, 2003, at 4:09 PM, Vaughan Johnson wrote:
> * bug 69 (LADSPA effects plugin menus not greyed out.): Don't know who
> fixed it, but it's correct in 1.2.0pre3, so I marked it fixed.
>
It's possible that this is still a bug on Linux, but in that case it's a
GTK bug, so pretty much out of our hands. Either way we should
>
> * bug 72 (Preview only lasts a couple seconds): Well, actually it's
> not a bug, it's a design decision. Dominic chose 3 seconds. Tony also
> discussed this and wanted looping on the selection (like Reuben
> Martin, the reporter of this bug), plus a bypass button or checkbox. I
> think we decided that will wait for post-1.2.0, right?
My personal preference is to keep Bugzilla free of feature requests.
> * bug 73 (Detect audio CD tracks on Windows:
> Win32 presents audio CDs as mounted volumes, with a pseudo-WAV file
> visible for each track. When a user tries to open one of these audio
> CD tracks in Audacity, it produces a cryptic error message ("riff file
> does not specify 'wave' as type").):
> I don't see that behavior on Win2K Pro or Win98 SE. The files on an
> audio CD have a .cda extension, and trying to import them in Audacity
> 1.2.0pre3 brings up an alert: "Audacity did not recognize the type of
> this file...". So, Matt (reporter), is it fixed, or do I not
> understand?
It would be best if Audacity specifically recognized ".cda" files and
brought up a different error message that explains that CD audio files
must be ripped.
> * bug 103 ("Space" for playing doesn't always work): I think I used to
> experience this, but I can't replicate it with Audacity 1.2.0pre3 on
> Win98 SE. Markus (reporter), are you seeing this?
Leave this one open; it's happened to me every now and then. Perhaps
after you close a dialog box. It's possible that it always works on
Windows but not always on Linux or Mac OS X.
> * bug 110 (Doesn't ask if you save changes if you close while
> recording:
>
> Open a new Audacity project and click "Record". Then close the window
> or Quit,
> and Audacity closes the window without asking if you want to save
> changes, so
> you could lose everything you've recorded.
>
> ):
> Confirmed. This can be harsh, but do many people run into it?
Shouldn't be too hard to fix.
> * bug 117 (Add "Set selection by time" feature): Is this feature
> request important for 1.2.0, or can it wait?
Again, shouldn't be in Bugzilla, in my opinion.
>
> OK, that's all the open, unassigned bugs that I have comments on.
> Really only 110 seems bad to me. I'll look into it.
This is the piece of code that is NOT getting called (in
TrackPanel.cpp):
// Next, check to see if we were playing or recording
// audio, but now Audio I/O is completely finished.
// The main point of this is to properly push the state
// and flush the tracks once we've completely finished
// recording new state.
if (p->GetAudioIOToken()>0 &&
!gAudioIO->IsAudioTokenActive(p->GetAudioIOToken())) {
if (gAudioIO->GetNumCaptureChannels() > 0) {
// Tracks are buffered during recording. This flushes
// them so that there's nothing left in the append
// buffers.
TrackListIterator iter(mTracks);
for (Track * t = iter.First(); t; t = iter.Next()) {
if (t->GetKind() == Track::Wave) {
((WaveTrack *)t)->Flush();
}
}
MakeParentPushState(_("Recorded Audio"), _("Record"));
}
MakeParentRedrawScrollbars();
p->SetAudioIOToken(0);
p->RedrawProject();
}
The fundamental problem is that the Project never pushes state,
realizing that it's been modified, until it checks to see if recording
has _finished_ during an idle event.
Three ideas for solutions:
* Is it enough to mark a project as Dirty (mDirty = true) when you
start recording?
* Could a project PushState when you start recording, and then
ModifyState when you finish?
* Do an explicit check to see if you're in the middle of recording in
the AudacityProject destructor, and if so, push state, duplicating a
bit of code shown above.
> My constraint: I will have to post Windows builds by Monday night
> (California time) or I won't be able to do it for several days after
> that.
The holidays are busy for many of us; don't worry about it.
- Dominic

Thread view

* bug 69 (LADSPA effects plugin menus not greyed out.): Don't know who
fixed it, but it's correct in 1.2.0pre3, so I marked it fixed.
* bug 72 (Preview only lasts a couple seconds): Well, actually it's not
a bug, it's a design decision. Dominic chose 3 seconds. Tony also
discussed this and wanted looping on the selection (like Reuben Martin,
the reporter of this bug), plus a bypass button or checkbox. I think we
decided that will wait for post-1.2.0, right?
* bug 73 (Detect audio CD tracks on Windows:
Win32 presents audio CDs as mounted volumes, with a pseudo-WAV file
visible for each track. When a user tries to open one of these audio CD
tracks in Audacity, it produces a cryptic error message ("riff file does
not specify 'wave' as type").):
I don't see that behavior on Win2K Pro or Win98 SE. The files on an
audio CD have a .cda extension, and trying to import them in Audacity
1.2.0pre3 brings up an alert: "Audacity did not recognize the type of
this file...". So, Matt (reporter), is it fixed, or do I not understand?
* bug 103 ("Space" for playing doesn't always work): I think I used to
experience this, but I can't replicate it with Audacity 1.2.0pre3 on
Win98 SE. Markus (reporter), are you seeing this?
* bug 110 (Doesn't ask if you save changes if you close while recording:
Open a new Audacity project and click "Record". Then close the window or Quit,
and Audacity closes the window without asking if you want to save changes, so
you could lose everything you've recorded.
):
Confirmed. This can be harsh, but do many people run into it?
* bug 117 (Add "Set selection by time" feature): Is this feature request
important for 1.2.0, or can it wait?
OK, that's all the open, unassigned bugs that I have comments on. Really
only 110 seems bad to me. I'll look into it.
My constraint: I will have to post Windows builds by Monday night
(California time) or I won't be able to do it for several days after that.
-Vaughan

On Dec 21, 2003, at 4:09 PM, Vaughan Johnson wrote:
> * bug 69 (LADSPA effects plugin menus not greyed out.): Don't know who
> fixed it, but it's correct in 1.2.0pre3, so I marked it fixed.
>
It's possible that this is still a bug on Linux, but in that case it's a
GTK bug, so pretty much out of our hands. Either way we should
>
> * bug 72 (Preview only lasts a couple seconds): Well, actually it's
> not a bug, it's a design decision. Dominic chose 3 seconds. Tony also
> discussed this and wanted looping on the selection (like Reuben
> Martin, the reporter of this bug), plus a bypass button or checkbox. I
> think we decided that will wait for post-1.2.0, right?
My personal preference is to keep Bugzilla free of feature requests.
> * bug 73 (Detect audio CD tracks on Windows:
> Win32 presents audio CDs as mounted volumes, with a pseudo-WAV file
> visible for each track. When a user tries to open one of these audio
> CD tracks in Audacity, it produces a cryptic error message ("riff file
> does not specify 'wave' as type").):
> I don't see that behavior on Win2K Pro or Win98 SE. The files on an
> audio CD have a .cda extension, and trying to import them in Audacity
> 1.2.0pre3 brings up an alert: "Audacity did not recognize the type of
> this file...". So, Matt (reporter), is it fixed, or do I not
> understand?
It would be best if Audacity specifically recognized ".cda" files and
brought up a different error message that explains that CD audio files
must be ripped.
> * bug 103 ("Space" for playing doesn't always work): I think I used to
> experience this, but I can't replicate it with Audacity 1.2.0pre3 on
> Win98 SE. Markus (reporter), are you seeing this?
Leave this one open; it's happened to me every now and then. Perhaps
after you close a dialog box. It's possible that it always works on
Windows but not always on Linux or Mac OS X.
> * bug 110 (Doesn't ask if you save changes if you close while
> recording:
>
> Open a new Audacity project and click "Record". Then close the window
> or Quit,
> and Audacity closes the window without asking if you want to save
> changes, so
> you could lose everything you've recorded.
>
> ):
> Confirmed. This can be harsh, but do many people run into it?
Shouldn't be too hard to fix.
> * bug 117 (Add "Set selection by time" feature): Is this feature
> request important for 1.2.0, or can it wait?
Again, shouldn't be in Bugzilla, in my opinion.
>
> OK, that's all the open, unassigned bugs that I have comments on.
> Really only 110 seems bad to me. I'll look into it.
This is the piece of code that is NOT getting called (in
TrackPanel.cpp):
// Next, check to see if we were playing or recording
// audio, but now Audio I/O is completely finished.
// The main point of this is to properly push the state
// and flush the tracks once we've completely finished
// recording new state.
if (p->GetAudioIOToken()>0 &&
!gAudioIO->IsAudioTokenActive(p->GetAudioIOToken())) {
if (gAudioIO->GetNumCaptureChannels() > 0) {
// Tracks are buffered during recording. This flushes
// them so that there's nothing left in the append
// buffers.
TrackListIterator iter(mTracks);
for (Track * t = iter.First(); t; t = iter.Next()) {
if (t->GetKind() == Track::Wave) {
((WaveTrack *)t)->Flush();
}
}
MakeParentPushState(_("Recorded Audio"), _("Record"));
}
MakeParentRedrawScrollbars();
p->SetAudioIOToken(0);
p->RedrawProject();
}
The fundamental problem is that the Project never pushes state,
realizing that it's been modified, until it checks to see if recording
has _finished_ during an idle event.
Three ideas for solutions:
* Is it enough to mark a project as Dirty (mDirty = true) when you
start recording?
* Could a project PushState when you start recording, and then
ModifyState when you finish?
* Do an explicit check to see if you're in the middle of recording in
the AudacityProject destructor, and if so, push state, duplicating a
bit of code shown above.
> My constraint: I will have to post Windows builds by Monday night
> (California time) or I won't be able to do it for several days after
> that.
The holidays are busy for many of us; don't worry about it.
- Dominic

Dominic Mazzoni wrote:
> On Dec 21, 2003, at 4:09 PM, Vaughan Johnson wrote:
>
>> ...* bug 73 (Detect audio CD tracks on Windows:
>> Win32 presents audio CDs as mounted volumes, with a pseudo-WAV file
>> visible for each track. When a user tries to open one of these audio
>> CD tracks in Audacity, it produces a cryptic error message ("riff
>> file does not specify 'wave' as type").):
>> I don't see that behavior on Win2K Pro or Win98 SE. The files on an
>> audio CD have a .cda extension, and trying to import them in Audacity
>> 1.2.0pre3 brings up an alert: "Audacity did not recognize the type of
>> this file...". So, Matt (reporter), is it fixed, or do I not understand?
>
>
> It would be best if Audacity specifically recognized ".cda" files and
> brought up a different error message that explains that CD audio files
> must be ripped.
Okay. I put a hack in Importer::Import() to do this, but should it
instead be an importPluginNode that returns 0? Also, if this extra
verbiage is a translation burden, we can back this out to the
development version.
>> * bug 110 (Doesn't ask if you save changes if you close while recording:
>>
>> Open a new Audacity project and click "Record". Then close the
>> window or Quit,
>> and Audacity closes the window without asking if you want to save
>> changes, so
>> you could lose everything you've recorded.
>>
>> ):
>
> ...
> The fundamental problem is that the Project never pushes state,
> realizing that it's been modified, until it checks to see if recording
> has _finished_ during an idle event.
>
> Three ideas for solutions:
>
> * Is it enough to mark a project as Dirty (mDirty = true) when you
> start recording?
No, that didn't do it.
>
>
> * Could a project PushState when you start recording, and then
> ModifyState when you finish?
I hacked in a PushState (but not the ModifyState), and that makes it ask
whether to save when you click the close box, but it keeps recording! If
you save, it does it successfully, up to some completed chunk, and
closes. I was surprised it didn't crash, but as you said, Dominic, we
need to stop recording before exiting, and presumably before asking
whether to save.
On these first 2 ideas, because mDirty and PushState are private, I just
hacked AudacityProject::SetAudioIOToken(). That's the wrong place to do
it, but for testing worked because the only place I could easily tell
where recording starts is in ControlToolBar::OnRecord(), and it calls
SetAudioIOToken on first successful loop. Is there a better place to
catch that, or will ControlToolBar need state-modifying callbacks a la
TrackPanel (to maintain the privacy restriction)? It doesn't look like
AudacityProject::OnRecord() is called anywhere.
>
>
> * Do an explicit check to see if you're in the middle of recording in
> the AudacityProject destructor, and if so, push state, duplicating a
> bit of code shown above.
I didn't try this one, but does the above give you an indication whether
this is necessary? I didn't yet look into making it stop recording
before presenting the save dialog. Are both necessary?
-Vaughan

Vaughan Johnson wrote:
> Okay. I put a hack in Importer::Import() to do this, but should it
> instead be an importPluginNode that returns 0? Also, if this extra
> verbiage is a translation burden, we can back this out to the
> development version.
My vote would be to go with the hack. Stable releases should be
ugly and they should work. We should solve the problem the "right"
way in the CVS HEAD.
As for the translation, please mark the string as translatable so
that the translation can be added in time for the 1.2.1 release.
>>> * bug 110 (Doesn't ask if you save changes if you close while recording:
>>>
>>> Open a new Audacity project and click "Record". Then close the
>>> window or Quit,
>>> and Audacity closes the window without asking if you want to save
>>> changes, so
>>> you could lose everything you've recorded.
>>>
>> ...
>> The fundamental problem is that the Project never pushes state,
>> realizing that it's been modified, until it checks to see if recording
>> has _finished_ during an idle event.
>>
>> Three ideas for solutions:
>>
>> * Is it enough to mark a project as Dirty (mDirty = true) when you
>> start recording?
>
> No, that didn't do it.
>>
>> * Could a project PushState when you start recording, and then
>> ModifyState when you finish?
>
> I hacked in a PushState (but not the ModifyState), and that makes it ask
> whether to save when you click the close box, but it keeps recording! If
> you save, it does it successfully, up to some completed chunk, and
> closes. I was surprised it didn't crash, but as you said, Dominic, we
> need to stop recording before exiting, and presumably before asking
> whether to save.
>
> On these first 2 ideas, because mDirty and PushState are private, I just
> hacked AudacityProject::SetAudioIOToken(). That's the wrong place to do
> it, but for testing worked because the only place I could easily tell
> where recording starts is in ControlToolBar::OnRecord(), and it calls
> SetAudioIOToken on first successful loop. Is there a better place to
> catch that, or will ControlToolBar need state-modifying callbacks a la
> TrackPanel (to maintain the privacy restriction)? It doesn't look like
> AudacityProject::OnRecord() is called anywhere.
Again, ugly but simple is the key. Ugly, simple, and well-documented.
AudacityProject::OnRecord is called when you press the 'R' key. See
Menus.cpp and the CommandManager class for details. But since the
implementation just calls the control toolbar, you could just patch
ControlToolBar::OnRecord and/or SetAudioIOToken; I don't see a problem
with that. To be safe, be sure to test it using the 'R' key, though...
>> * Do an explicit check to see if you're in the middle of recording in
>> the AudacityProject destructor, and if so, push state, duplicating a
>> bit of code shown above.
>
> I didn't try this one, but does the above give you an indication whether
> this is necessary? I didn't yet look into making it stop recording
> before presenting the save dialog. Are both necessary?
Hmmmm, I guess I'm starting to think that it may be necessary to stop
recording before saving. Even though it worked for you once, it
sounds like a potential race condition to me. It may be necessary to
put up a stopwatch (wxBusyCursor), stop, wait until the token is
returned (which means that the recording has _really_ finished), and
then go on to save, exit, etc.
Note that if you do this, you won't need to add another PushState
anywhere, since this will already be done when AudioIO
_completely_ finishes and zeros out the token.
Yet another idea: modify the CloseWindow and Exit handlers to Veto
the request if you're recording, popping up a wxMessageBox that says
you must stop first. Not ideal, but if everything else gets too
complicated, this would be a better solution than doing nothing.
- Dominic