Audacity Forum

Multiple small commits that fix issues independently are preferred over big commits that make multiple improvements and fixes.

I'm not sure if I always agree in the case of external libraries (lib-src) like sbsms. When I was first notified of timestretch issues in audacity a few months ago, I submitted a partial bugfix to lib-src/sbsms that was not part of a library release, and somebody called me on it. I agreed that it would be best in principle to submit changes to lib-src that atomically synchronize with a library release - otherwise maintaining two copies of the library is troublesome. Since then I've been working on the next release with that in mind, and it unexpectedly turned in to a major rewrite.

I suppose that since lib-src/sbsms is already out of sync with a release, it can't hurt to fix what's there in the meantime, and then subsequently keep them in sync, especially since, as pointed out, the actual fix for bug 172 is relatively straightforward. I agree that this is the most sensible thing to do at this point, regardless of how close I want to think I am to a shiny new sbsms release, so I'll suck it up and dust off an old version that resolves the bug with minimal changes and submit that before touching the new stuff any more. It should be noted that there are known problems and limitations with what I will submit.

Thanks for the reply. I was wrong by saying "check in whatever you got once it works". And also, you are correct that lib-src is not a working repository for libsms when you have one on sf.

For lib-src we don't want to create a bunch of new but intermediate features/code that doesn't have an upstream version.
But for P2 bugfixes we still want them fixed. The email I think you are referring to from Al was worried about the former.

When dealing with bugs for lib-src mutiple patches are still good if they deal with individual bugs so when they are looked at upstream they can be reviewed by functionality and applied faster. Just, we need to keep track of these changes as in lib-src/audacity-patches.txt does (I think this is what Al was saying.)

So, if you can you should commit a fix/patch that deals with just this bug and no midway enhancements. Since you are maintainer you can apply it both at the same time and there will be no future problems - just remove the patch when you put the next version of libsms in.
If you think you are close and can wrap the new version up in three weeks (by october 31,) then maybe it's worth waiting. But otherwise I think its good to just patch and move on because I'm sure we are getting annoying by now

Hi Clayton,

Is there any ETA on just committing a decent version without the previous crash and with Bug 172 fixed? Or is a new version of the library really imminent now?

Thanks


Gale

Hi Clayton,

We've decided to set a deadline of December 15th on this so we can move forward. If you genuinely had a complete commit of a new version say on the 16th and we knew that would happen, obviously we would wait, but beyond that I am expecting this P2 to be fixed one way or the other by mid December.

Thanks


Gale

In all likelihood I will submit the new imminent release (I have not been idle but am still reluctant to wrap up). I will take this deadline seriously regardless.

otey wrote:In all likelihood I will submit the new imminent release (I have not been idle but am still reluctant to wrap up). I will take this deadline seriously regardless.

Just to keep things up to date, Michael has done a hack fix to potentially fix Bug 172 - see r10820. Not tested yet by anyone except Michael.



Gale

Thanks Gale and Michael. My academic quarter just ended so the previous week was unavailable, but I'm almost there now. Can you give me a sense of how firm the deadline is, in case I drift a couple half-days?

otey wrote:Thanks Gale and Michael. My academic quarter just ended so the previous week was unavailable, but I'm almost there now. Can you give me a sense of how firm the deadline is, in case I drift a couple half-days?

Deadline has passed and the fix has been committed, in that sense. However the patch Michael applied is here so just check in the new sbsms and update lib-srcaudacity-patches.txt when ready , but remember to revert the patch as well.


Thanks


Gale

Will do. I'm glad the patch looks so simple. Thanks again, Michael. It should last a couple days until the interface is smoothed out. I'm going to drop the ball on stereo phase tracking so I don't end up slipping a week, and because I don't actually know the proper way to do it yet, but nobody directly asked for that anyhow. That will naturally follow in a libsbsms-2.1 release.

Some points come to mind before I submit:

1) Does anybody have any additional advice for how to notate my changes in lib-src/audacity-patches.txt? I'm essentlally completely swapping out libsbsms-1.7.0 for 2.0.0 - diffs will be pretty much meaningless. I think I'll erase the previous patch history, since the relevant changes have been incorporated in 2.0.0.

2) There are potential configure/build issues, since I now do things like test for SSE instructions at configure time. On a related note, my mixed radix fft implementation could easily be used in place of the existing audacity code, used by the spectrogram, for example. I often get annoyed when waiting for the spectrogram to render - my code may be fast enough to allow for a responsive scale slider which finely adjusts the FFT size/step so you can focus properly. It's not quite FFTW, but if I recall correctly, my imp takes less than a factor of 2 more cpu time, and my code is in a single template header ~800 lines.

3) I have a configure-time multithreaded implementation which spawns about a dozen serial threads. Should I enable this by default? The multithreaded implementation is slower than the single-thread implementation on a single core, but works quite well on 2-4 cores. I suppose I could look into configure time tests for multiple cores, but that doesn't seem like it's been done portably in autoconf m4 yet.

4) I've added more widgets to the SBSMSEffectDialog which allow the user to control the interpolation in the rate/pitch slides. I would like feedback.

Hi Clayton,

I would post your points/questions to -devel list so that more people can see them. If you post to -devel using the e-mail address you registered on this Forum, it will go through automatically.

I would assume in audacity-patches.txt you want to state the "Version in Audacity SVN" as 2.0.0 (it says "??" now), and "Patches" will be "none" (so erase the patch history), unless you are noting new modifications to e.g. Audacity makefiles to allow for compile-time options. It might be good to give a one sentence summary of improvements in 2.0.0 as against 1.7.0.

My 2p would be that as most people won't be on multiple core machines, you shouldn't turn multi-threading on globally if it's slower on single core. I think Michael has most knowledge about threading issues.

When your new library is committed I'll be glad to comment about the new widgets. One point Michael made (as others do) is that an envelope GUI might be useful longer term so that there are more than just the start and end control points. Probably that should be after 2.0. That said, I think the interface needs to be kept fairly simple/acessible to screen readers as it is used as a generally more satisfactory Effect > Change Tempo and Chenge Pitch.



Gale