Re: [Audacity-devel] [Audacity-quality] Align tracks end to end

[Martyn S. <mar...@gm...>]

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