Re: [Audacity-devel] [Audacity-quality] Align tracks end to end
A free multi-track audio editor and recorder
Brought to you by:
aosiniao
From: Martyn S. <mar...@gm...> - 2013-08-13 23:37:31
|
Hi I just made a comment on bugzilla 530 that appears to fix the export of keyboard prefs here. Does that help? I appear to be able to set keys for the Align things. And Amplify. Also, in the keyboard pref I can't help but think that we could easily write Align:Align with Zero and AlignMove:Align with Zero etc. like we do in the xml export file, so users aren't confused, but I haven't implemented that here. Should I? HTH Martyn On 12/08/2013 15:09, Steve the Fiddle wrote: > On 11 August 2013 01:41, Vaughan Johnson <va...@au...> wrote: >> Thanks for the reminder. I thought another TL had committed to reviewing >> this. >> >> On a brief review today, I think it's not a great idea to re-architect >> it so much. The alignLabels array was a good thing, more succinct code. >> Your patch has instead a one-line method for each former index. > > I agree that alignLabels array is much more succinct, but > unfortunately it does not work with the current keyboard preferences > export/import.(bug 530) It has been broken for over a year. > > The choices facing me were: > * Fix bug 530 before adding the new feature - best option but > unfortunately I don't know how to do that. > * Add a new feature on top of the existing bug. > * Implement the new feature with the more verbose, but tried, tested > and working method used elsewhere in Audacity. > * Wait until some unknown and probably distant point in the future > when bug 530 is fixed. > > Of these, the "tried, tested and working" approach looks like the best > option to me. > > I'm not keen on adding this feature on top of bug 530. Doing so is > arguably no worse than now and will greatly simplify the patch, but is > there any guarantee that the alignLabels array will work properly even > when bug 530 has been fixed? I think it is a very good bet that this > more verbose method will continue to work because it is so widely used > elsewhere. > > The other disadvantage of the alignLabels array is that it causes > duplicate entries in the keyboard preferences and there is no > distinction in the undo history between items from the "Align" menu > and the "Align and Move" menu. The duplicate keyboard preferences are > the greater problem as users cannot tell which command they are > setting a shortcut for. > > >> >> There are also many mods in this outside 'Align' (e.g., USE_MIDI, >> default flags, etc). It's much easier to review code submissions if they >> are focused on one thing at a time. Or is some of that because this >> patch is out of date? > > Changes to the flags are because the current flags are wrong. As the > align menu has been rewritten in this patch I see no point in > repeating the previous errors and then submitting additional patches > to fix them. Similarly for "#if defined(USE_MIDI)". > > If preferred I can submit a new patch which *only* adds "Align End to > End", but then there will still be multiple problems that are fixed by > the current patch. > > Namely: > * Duplicate command names in keyboard preferences with no indication > to users which are which. > * Menu commands become extremely wide if keyboard shortcuts are set. > * Keyboard shortcuts can not be exported/imported. > * Inconsistent naming between command names, functions and enums. > * "Align" and "Align and Move" both appear the same in the Undo history. > * Align commands are "active" even when there is no audio to align. > * "Align Tracks Together" may push audio entirely behind time zero. > * "Sync Lock" is incorrectly disabled during Play/Record. > * "Align Tracks Together" causes nonsensical changes to label tracks. > * Incorrect handling of stereo tracks if channels are different lengths. > * "Align with Cursor" is a duplicate of "Align with Selection Start" > * The underlined "g" for "Align Together" is not visible. > * "Align Tracks Together" fails if there is a start time before zero. > > not to mention the weeks of negotiations over menu names and the > correct behaviour for each Align option. > > I'm aware that it is not a small patch, but there are a lot of > problems in this part of the Menus code which would require an awful > lot of patches to fix them one by one and each patch would need to > work around the other problems. > > > >> >> I appreciate that you probably feel frustrated that it wasn't responded >> to sufficiently, and timely. I would, if I were you. But it needs >> updating to current code base, at least. And please let me know if the >> above makes sense to you. > > Attached is a refreshed version of AlignTracks2.patch, now called > AlignTracks2b.patch. > I have also attached the complete patched Menus.cpp and Menus.h files > in case they are easier to review. > > Steve > > >> >> Thanks! >> >> - Vaughan >> >> >> >> >> On 8/7/2013 5:05 PM, Steve the Fiddle wrote: >>> I was just reminded about this by a feature request on the forum. >>> >>> Steve >>> >>> On 28 June 2013 10:24, Steve the Fiddle <ste...@gm...> wrote: >>>> On 28 June 2013 05:15, Vaughan Johnson <va...@au...> wrote: >>>>> I haven't been in on this discussion, or looked at this patch >>>>> extensively, but why "Move" at the end of the new method names, when >>>>> they start with "Align", a verb that means move? >>>> >>>> So as to differentiate between the commands in "Tracks > Align Tracks" >>>> and those in "Tracks > Align and Move Cursor". >>>> >>>> Currently in the keyboard preferences, the following command names occur twice: >>>> Align with zero >>>> Align with cursor >>>> Align with selection start >>>> Align with selection end >>>> Align end with cursor >>>> Align end with selection start >>>> Align end with selection end >>>> >>>> One instance is for "Tracks > Align Tracks" and the other is for >>>> "Tracks > Align and Move Cursor". The appended word "Move" indicates >>>> to the user which ones are which. >>>> >>>> Steve >>>> >>>> >>>>> >>>>> - V >>>>> >>>>> On 6/9/2013 12:39 PM, Steve the Fiddle wrote: >>>>>> Hopefully this is the finished version of this patch. >>>>>> As per the decision on -quality I've also added the >>>>>> WaveTracksExistFlag to the "Labeled Audio" commands. >>>>>> >>>>>> Steve >>>>>> >>>>>> On 3 June 2013 21:51, Steve the Fiddle <ste...@gm...> wrote: >>>>>>> On 1 June 2013 04:59, Gale Andrews <ga...@au...> wrote: >>>>>>>> >>>>>>>> | From Steve the Fiddle <ste...@gm...> >>>>>>>> | Sat, 1 Jun 2013 02:40:31 +0100 >>>>>>>> | Subject: [Audacity-quality] [Audacity-devel] Align tracks end to end >>>>>>>>> On 1 June 2013 00:52, Gale Andrews <ga...@au...> wrote: >>>>>>>>>> >>>>>>>>>> | From Steve the Fiddle <ste...@gm...> >>>>>>>>>> | Fri, 31 May 2013 15:08:25 +0100 >>>>>>>>>> | Subject: [Audacity-devel] Align tracks end to end >>>>>>>>>> >>>>>>>>>> Thanks, Steve. I built it on Win 7 x64 and did some tests. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> The attached patch adds a new "Align End to End" command to the "Align >>>>>>>>>>> Tracks" menu. Selected Audio and Note tracks are moved end to end, >>>>>>>>>>> starting from the position of the first selected track. This command >>>>>>>>>>> overrides Sync Lock. >>>>>>>>>>> >>>>>>>>>>> After much discussion on the -QA list it also addresses a number of >>>>>>>>>>> other issues: >>>>>>>>>>> >>>>>>>>>>> * The menu items in "Align Tracks" and "Align and Move Cursor" menus >>>>>>>>>>> now have unique, but closely related names. >>>>>>>>>> >>>>>>>>>> Would the "Align Tracks & Move Selection" commands be better as >>>>>>>>>> "Align Start to Zero & Move" and so on, rather than "Move /Align >>>>>>>>>> Start to Zero"? After all, we are not moving the selection or cursor >>>>>>>>>> to zero. >>>>>>>>> >>>>>>>>> I've tested this on a screen resolution of 800 x 600. >>>>>>>>> >>>>>>>>> With a shortcut: >>>>>>>>> "Align Tracks & Move Selection > Move/Align Start to Selection Start Alt+Ctrl+S" >>>>>>>>> the sub-menu only just fists on screen without obscuring the text of >>>>>>>>> the Align menu. >>>>>>>>> >>>>>>>>> With a shortcut: >>>>>>>>> "Align Tracks & Move Selection > Move/Align Start to Selection Start >>>>>>>>> Shift+Ctrl+S" >>>>>>>>> the sub-menu overlaps the parent menu chopping off the "n" from "Selection" >>>>>>>>> >>>>>>>>> Similarly, >>>>>>>>> "Move/Aligned start to selection start" >>>>>>>>> will only just fit in the Undo history window without resizing the window. >>>>>>>>> >>>>>>>>> I think we are at the absolute maximum length for the menu labels >>>>>>>>> without making it look crap. >>>>>>>> >>>>>>>> My position has always been that it would be much better (if >>>>>>>> possible) not to have unique names in the sub-menus for "Align >>>>>>>> Tracks" and "Align Tracks & Move Selection" (but to do so in the >>>>>>>> Prefs). That sub-menu is being lengthened rather than only fixing >>>>>>>> the Prefs. >>>>>>> >>>>>>> I agree, but currently that is not an option. >>>>>>> >>>>>>> The menu labels (menu text) is what is shown in Keyboard Preferences. >>>>>>> If there are two identical menu labels, then there are two keyboard >>>>>>> preferences with identical text for different commands. >>>>>>> >>>>>>>> >>>>>>>> On 800x600 on Windows 7 (normal DPI) your examples above fit, >>>>>>>> but Linux often has larger default fonts. >>>>>>>> >>>>>>>> I guess you are saying that "<Align command> & Move" (spaces >>>>>>>> either side of "&") may overflow with a long custom binding at >>>>>>>> 800x600, but "Move/<Align command> (no space either side of "/") >>>>>>>> is less risk. >>>>>>>> >>>>>>>> If so, then even "<Align command>/Move" (no space either side >>>>>>>> of "/") would be much clearer IMHO. >>>>>>> >>>>>>> OK I can go with <Align command>/Move >>>>>>> To keep the command names, menu labels, function names and enumerator >>>>>>> names consistent requires changes in a lot of places so I'll not be >>>>>>> changing this again (but it's open source so of course others can >>>>>>> change it). >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>>> Perhaps it is easier to pick out "Move/" in Keyboard Prefs, but I don't >>>>>>>>>> think this outweighs the somewhat confusing command names in the >>>>>>>>>> menus. >>>>>>>>>> >>>>>>>>>> Would "and" be better than "&"? We don't have "Mix & Render". >>>>>>>>> >>>>>>>>> No, but >>>>>>>>> "Mix and Render" >>>>>>>>> is significantly shorter than >>>>>>>>> "Align Tracks & Move Selection" >>>>>>>>> and doesn't have a sub menu. >>>>>>>>> >>>>>>>>> "Align Tracks and Move Selection > Align Start to Selection Start and >>>>>>>>> Move Shift+Ctrl+S" >>>>>>>>> is far too long in my opinion. >>>>>>>> >>>>>>>> It's a pity we have to become inconsistent in this way due to >>>>>>>> practical considerations. >>>>>>>> >>>>>>>> Perhaps then it should be "Align Tracks/Move Selection"? >>>>>>> >>>>>>> OK I can go with that too. >>>>>>> >>>>>>>> >>>>>>>> Though this is much less important than where "Move" goes >>>>>>>> in the sub-menu. I won't say any more about it. >>>>>>>> >>>>>>>> >>>>>>>>>>> * The names of the commands in the "Edit > Labelled Audio" menu have >>>>>>>>>>> been made more descriptive and distinct from the names of commands in >>>>>>>>>>> "Edit > Remove Audio or Labels". >>>>>>>>>> >>>>>>>>>> I think it's a little unfortunate we need to bloat that sub-menu with >>>>>>>>>> repetition of "Labeled Audio" >>>>>>> >>>>>>> As above, we don't really have any other option at present. >>>>>>> >>>>>>> I would hope that eventually we will have more than one level in the >>>>>>> XML file so that menus, sub-menus and sub-sub-menus are hierarchical >>>>>>> in the XML but there's no sign of that yet. >>>>>>> >>>>>>> >>>>>>>>> >>>>>>>>> That seems rather contrary considering your opposition to removing the >>>>>>>>> word "Align" from the "Align" sub-menus. >>>>>>>> >>>>>>>> Considering your diatribe against dyslexic menus the sub-menu >>>>>>>> you are proposing for Labeled Audio seems rather contrary :=) >>>>>>>> >>>>>>>> For me it's horses for courses. The "Labeled Audio" menu is short >>>>>>>> and clear enough and the items in its sub-menu are the same as >>>>>>>> in the "Remove Audio or Labels" sub-menu. >>>>>>> >>>>>>> but the "Keyboard Preferences > Edit" we have all of the following >>>>>>> appearing twice: >>>>>>> >>>>>>> Cut, >>>>>>> Delete, >>>>>>> Split Cut, >>>>>>> Split Delete, >>>>>>> Split, >>>>>>> Silence Audio, >>>>>>> Copy, >>>>>>> Join, >>>>>>> Detach at Silences >>>>>>> >>>>>>> Usability is a practical consideration. >>>>>>> >>>>>>> The purpose of adding "Labeled Audio" to one set of commands is so >>>>>>> that the user can see which set of commands in Keyboard Preferences >>>>>>> are for labelled audio. >>>>>>> >>>>>>>> >>>>>>>> The two Align commands and the commands in their submenus >>>>>>>> are much harder to understand. Losing "Align" is OK for the >>>>>>>> "Align Tracks" commands but I think you agree this is very >>>>>>>> problematic for the "Align Tracks & Move Selection" commands. >>>>>>>> >>>>>>>> As long as we keep "Align" then if it really helps I could live with >>>>>>>> removing all the "Start" instances from the submenus. Then >>>>>>>> "Align End to Selection End" would be the longest. But we say >>>>>>>> "Selection Start" elsewhere in menus. >>>>>>>> >>>>>>>> BTW were we not having default shortcuts in the Align Tracks >>>>>>>> Menu: >>>>>>> >>>>>>> Personally I think that we have more than enough pre-defined shortcuts. >>>>>>> >>>>>>> The way that I have rewritten the menus does support default shortcuts >>>>>>> being added, but until we have a more convenient way for users to >>>>>>> reassign keyboard combinations I am not willing to add more default >>>>>>> keyboard shortcuts. Of course this does not prevent someone else >>>>>>> adding them if they can reach consensus about doing so and which keys >>>>>>> to assign. >>>>>>> >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> CTRL+ SHIFT + O (letter) for Align Start to Zero >>>>>>>> >>>>>>>> CTRL + SHIFT + D for Align End to End >>>>>>>> >>>>>>>> And we had CTRL + SHIFT + R suggested for Align Start to >>>>>>>> Cursor but it is now called "Align Start to Selection Start" >>>>>>>> and could even be "Align to Selection". >>>>>>>> >>>>>>>> CTRL + SHIFT + S is available but perhaps a bit like a save >>>>>>>> shortcut. Most other reasonable candidates are taken >>>>>>>> so perhaps CTRL + SHIFT + R is still OK if it is not renamed? >>>>>>>> >>>>>>>> >>>>>>>>>> - I doubt we would have done it if we >>>>>>>>>> could have had the "Labeled Audio" menu string in the Keyboard Prefs >>>>>>>>>> in a similar fashion that we did in 2.0.0. >>>>>>>>>> >>>>>>>>>> Do you still see this as a temporary fix as per the analysis of the >>>>>>>>>> problem that you sent to me? Did you post that to anyone else, or >>>>>>>>>> should you perhaps post it on -devel? >>>>>>>>> >>>>>>>>> It depends on what James is planning to do about bug 530. >>>>>>>> >>>>>>>> It would be good to have that bug 530 fixed though I understand >>>>>>>> the code is tangled. :=) >>>>>>>> >>>>>>>> >>>>>>>>>>> * Keyboard shortcuts for the Align commands can now be set, exported >>>>>>>>>>> and imported via the keyboard preferences. >>>>>>>>>> >>>>>>>>>> On my tests this works fine, except that on two occasions out >>>>>>>>>> of four, when I set custom "Labeled Audio", "Align" and "Save >>>>>>>>>> Project As" shortcuts, exported them, defaulted shortcuts then >>>>>>>>>> imported the custom shortcuts, the "Labeled Audio" and "Align" >>>>>>>>>> shortcuts worked but were not in the menus until I restarted >>>>>>>>>> Audacity. The "Save Project As" shortcut was in the menu each >>>>>>>>>> time. >>>>>>>>>> >>>>>>>>>> Is there something that needs to refreshed or flushed? >>>>>>>>> >>>>>>>>> I'm not able to reproduce that. They work every time here. >>>>>>>>> Is that just on Windows? >>>>>>>> >>>>>>>> I only tested on Windows so far. >>>>>>>> >>>>>>>> >>>>>>>>> Does the problem occur with other sub-menus? >>>>>>>> >>>>>>>> I tried it four times in Edit > Move Cursor > to Selection Start >>>>>>>> without an occurrence. On one occasion the Align End to End >>>>>>>> shortcut failed to show in the menu until restart. >>>>>>>> >>>>>>>> >>>>>>>>>>> * The Align commands are greyed out if there are no Audio or Note >>>>>>>>>>> tracks in the project. >>>>>>>>>> >>>>>>>>>> I cannot make that work here with a project containing only one >>>>>>>>>> unselected label track, unless if I turn "Select all... if none" off in >>>>>>>>>> Prefs. >>>>>>>>> >>>>>>>>> They are greyed out if there are NO tracks in the project. I think >>>>>>>>> that this is the best that we can do given the way that greying out >>>>>>>>> commands work. >>>>>>>> >>>>>>>> I can only go on what you said: >>>>>>>> >>>>>>>> "The Align commands are greyed out if there are no Audio or Note >>>>>>>> tracks in the project." >>>>>>>> >>>>>>>> To be honest I was a bit surprised you had added that flexibility >>>>>>>> so I tried it =). >>>>>>>> >>>>>>>> >>>>>>>>> Greying out of menu options is done by matching "flags". There isn't a >>>>>>>>> flag for "more than one track", so just as "Auto Duck" can't be greyed >>>>>>>>> out if there is only one track selected, the same is true for "Align". >>>>>>>>> For the "Align" commands the ability to grey out commands is more >>>>>>>>> limited because the commands work on both Audio and Note tracks, and >>>>>>>>> for most of them Label tracks as well, so we are very limited in how >>>>>>>>> much logic can be applied here. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Even with that Pref off, a project containing only one selected >>>>>>>>>> label track makes the Align commands appear active. Should >>>>>>>>>> the Align commands not be greyed in that case? >>>>>>>>> >>>>>>>>> As above, I don't think that menus allow complex logic for greying out >>>>>>>>> - it's not done anywhere else in Audacity. >>>>>>>> >>>>>>>> The "Labeled Audio" sub-menu is greyed if there are no label >>>>>>>> tracks selected. >>>>>>> >>>>>>> Logically "Labelled Audio" should also be greyed out if there is no audio track. >>>>>>> I'd like to deal with that separately as it is not completely straightforward. >>>>>>> >>>>>>> Steve >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> But none of this is an argument against your current patch if you >>>>>>>> were not intending extra flexibility. >>>>>>>> >>>>>>>> >>>>>>>>>> Note that if you try Align End to End with a selected label track >>>>>>>>>> containing some labels, this pushes the last label to the end of >>>>>>>>>> the track, and so can change the zoom level. >>>>>>>>> >>>>>>>>> That's what "Fit in Window" does. >>>>>>>>> >>>>>>>>> In most real life cases the end-to-end aligned tracks are likely to be >>>>>>>>> much longer than any label tracks in the project, but if you put a >>>>>>>>> label well beyond the end of the audio, "Fit in Window" will zoom out >>>>>>>>> so that the label is in the window. >>>>>>>> >>>>>>>> Yes, I'm fine with the zoom change after aligning audio tracks >>>>>>>> end to end (whether label tracks are present or not). >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Gale >>>>>>>> >>>>>>>> >>>>>>>>>> Should not Align End to End and Align Together be greyed if >>>>>>>>>> there is only one selected audio or note track? >>>>>>>>> >>>>>>>>> As above, there is no flag for "more than one track". >>>>>>>>> >>>>>>>>>> >>>>>>>>>> What should happen if you Align Start to Zero with offset audio >>>>>>>>>> tracks and a selected label is before the start of those tracks >>>>>>>>>> (but currently on screen)? The label seems to disappear, but is >>>>>>>>>> exported. >>>>>>>>> >>>>>>>>> That's what should happen. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Should there be <--- Arrows on the left edge of the label track? >>>>>>>>> >>>>>>>>> That would probably be desirable, but we don't currently support <- >>>>>>>>> arrows in label tracks. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thanks for giving it a good workout Gale. >>>>>>>>> >>>>>>>>> Steve >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Gale >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> * "Sync Lock" menu item is always enabled (previously disabled while >>>>>>>>>>> audio IO was busy). >>>>>>>>>>> >>>>>>>>>>> * "Align Tracks Together" now works even if one or more tracks have >>>>>>>>>>> start times before zero. >>>>>>>>>>> >>>>>>>>>>> Steve >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ------------------------------------------------------------------------------ >>>>>>>> Get 100% visibility into Java/.NET code with AppDynamics Lite! >>>>>>>> It's a free troubleshooting tool designed for production. >>>>>>>> Get down to code-level detail for bottlenecks, with <2% overhead. >>>>>>>> Download for free and get started troubleshooting in minutes. >>>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> audacity-devel mailing list >>>>>>>> aud...@li... >>>>>>>> https://lists.sourceforge.net/lists/listinfo/audacity-devel |