Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect selection of notes when playing part of a multi-voice tune #77

Open
revad opened this issue Aug 18, 2023 · 24 comments
Open

Incorrect selection of notes when playing part of a multi-voice tune #77

revad opened this issue Aug 18, 2023 · 24 comments

Comments

@revad
Copy link

revad commented Aug 18, 2023

I describe my setup in #76

My main use of EasyABC is editing 2-voice tunes to fit the limited compass of my duet concertina. (Melody is played with the right hand and accompaniment with the left.) I often have to modify the accompaniment. (There are fewer buttons on the left.) So playing a section of a 2-voice tune over and over again is important to me.

On a single voice tune I can select a passage on a stave (staff) with the mouse. The selected notes turn orange and I can play it. This also works on a 2-voice (2-stave) tune provided I select a whole line or several whole lines. But I cannot select a rectangular section - say 2 bars of both staves: it will select the remainder of the top stave and the earlier part of the bottom stave. It will correctly play the notes it has selected, but they will be out of sync.

For example, the tune A Bruxa (Bm) of which this is the start:

%Rude Mechanicals tune library: www.rudemex.co.uk
%Chords and arrangements by the Rude Mechanicals unless otherwise acknowledged
X:1
T:A Bruxa
C:Anton Seoane
M:3/4%Meter
L:1/8%
K:Bm
%%score [1|2]
V:1 gchord=down
X"Bm"fd cB cd |f2 d2 Be |"Em"dc G2 d2 |"F#"c6 |
"Bm"fd cB cd |f2 d2 Be |"Em"dc G2 c2 ||1"Bm"B6 :|2B6 ||
V:2
XB2 EG AB |dc B2 F2 |B2 E4 |F2 ^A3 e |
dB F2 EF/2E/2 |D3/2C/2 B,4 |E3 A3 ||EF B,2 DF :|GF ED B,C ||

If, with the mouse, I draw a rectangle around bars 2 and 3 of the first pair of staves, it selects - and will play - bar 4 of the first voice and bar 1 of the second. Screenshot:
Screenshot_2023-08-18_15-02-06_a
Which sounds like this:
https://github.com/jwdj/EasyABC/assets/3061165/20ec3f47-aa96-438b-9edb-529d73a32939

It would also be nice if Follow Score worked when playing a selection - using a second colour.

@revad
Copy link
Author

revad commented Aug 24, 2023

This can be fixed by deleting these lines in function select_notes() in svgrenderer.py

        if selected_indices:
            selected_indices = set(range(min(selected_indices), max(selected_indices) + 1))

The intention appears to be to sort the set's insertion order, which could be done, but from my limited testing appears to be unnecessary.

@revad revad changed the title Limited ability to select and play part of a multi-voice tune Incorrect selection of notes when playing part of a multi-voice tune Aug 24, 2023
@revad
Copy link
Author

revad commented Oct 21, 2023

Here's a video showing muti-voice selection working:
simplescreenrecorder-2023-08-28_17.08.15.webm

@topchyan
Copy link

This can be fixed by deleting these lines in function select_notes() in svgrenderer.py

        if selected_indices:
            selected_indices = set(range(min(selected_indices), max(selected_indices) + 1))

The intention appears to be to sort the set's insertion order, which could be done, but from my limited testing appears to be unnecessary.

I tried this method, but playback on my install just stopped completely, so I returned these settings back.

@revad
Copy link
Author

revad commented Oct 23, 2023

That's interesting. It works for me on Linux with Python 3.6 and on Windows with Python 3.8. It works for someone else on Windows and Python 3.9.
1 What OS? (And distro if Linux.)
2 What version of Python?
3 What version of wxPython?
4 Does it use fluidsynth for playback (messages on console). (I haven't got fluidsynth to work at all on Linux, it's using Timidity.)
5 Is it a current copy of the master branch?
6 Does selecting and playing work for you without my fix - but with additional notes selected as in my original post image and sound sample?

@revad
Copy link
Author

revad commented Oct 23, 2023

I took another look at that function in svgrenderer.py Here it is (unaltered - my line numbers):

1    def select_notes(self, selection_rect):
2         in_selection = selection_rect.Contains
3         selected_offsets = set((abc_row, abc_col) for (x, y, abc_row, abc_col, desc) in self.notes if in_selection((x, y)))
4         list_of_sets = [self.indices_per_row_col[row][col] for row, col in selected_offsets]
5         selected_indices = set().union(*list_of_sets)
6         if selected_indices:
7             selected_indices = set(range(min(selected_indices), max(selected_indices) + 1))
8        self.selected_indices = selected_indices
9        return selected_indices

2-5 constructs a set (selected_indices) of references to the notes in the ABC which are within the selected rectangle - bars 2 and 3 in my original image.

7 changes that to a set of references to all notes starting at the first (top left) of those and ending at the last (bottom right) even if the notes were not within the rectangle. Which is the fault I described.

What was 7 supposed to do? I think the intention was to include notes which were accidentally outside the rectangle, perhaps because they were above or below the staff. Which works well for a single staff:

Screenshot_2023-10-23_17-22-47

But doesn't work across several staffs.

It's a good idea to include these 'above or below the rectangle' notes. It makes it easier to draw and missing notes would mess up the tune. But it needs to be done per-staff.

@revad
Copy link
Author

revad commented Mar 13, 2024

@topchyan You posted on abcusers.io that my fix doesn't work for you:
https://groups.io/g/abcusers/topic/where_is_easyabc_version/101170507
Does it still not work?

After @markblinkhorn 's fluidsynth fix it would be good to get all these changes accepted.

@topchyan
Copy link

Does it still not work?

I just tried it with my pyenv configuration (python3.8.18) and it works perfectly: plays just the right fragment and nothing else.

image

@revad
Copy link
Author

revad commented Mar 14, 2024

The original code works with a single voice, or a whole line like your example.
Does it work with a multi-voice tune? Does it work when you select just 2 or 3 bars?

@topchyan
Copy link

The original code works with a single voice, or a whole line like your example. Does it work with a multi-voice tune? Does it work when you select just 2 or 3 bars?

No, you're right, @revad, it doesn't work on multi selection like you describe: renders a few notes here and there very inconsistently.

I thought the single-voice scenario also had issues, so I was happy to see it working at least now under pyenv.

@revad
Copy link
Author

revad commented Mar 15, 2024

All my work on this issue to date was done with a snapshot of master from 8/23 running on Python 3.6 before fluidsynth was fixed. I'm now running Python 3.10 as I describe in #79 and #88 and I set out to reimplement my solution to this issue.

Clicking on the score panel blanks it with this error:

  File "/home/david/projects/python/easyABC/easyabc_git_1.3.8.7_240312/easyabc/music_score_panel.py", line 260, in draw_drag_rect
    dc.SetPen( dc.CreatePen(wx.Pen(wx_colour('black'), 1.0, style=wx.DOT )) )
TypeError: Pen(): arguments did not match any overloaded call:
  overload 1: too many arguments
  overload 2: argument 1 has unexpected type 'Colour'
  overload 3: argument 2 has unexpected type 'float'
  overload 4: argument 1 has unexpected type 'Colour'

This change in music_score_panel.py fixes that (arg 1 float > integer):

#             dc.SetPen( dc.CreatePen(wx.Pen(wx_colour('black'), 1.0, style=wx.DOT )) )
             dc.SetPen( dc.CreatePen(wx.Pen(wx_colour('black'), 1, style=wx.DOT )) )

I can now drag a rectangle, as in my video above, but it does not apparently select any notes - they do not change colour anyway. Pressing play plays the whole tune, as if nothing were selected.

I suspect this is a change in wxpython version rather than python itself.
Python 3.10: '4.2.1 gtk3 (phoenix) wxWidgets 3.2.2.1'
Python 3.6: '4.1.1 gtk3 (phoenix) wxWidgets 3.1.5'

@topchyan You wrote above that you could select and play a line of a single-voice tune in Python 3.8. Which version of wxpython are you using? [You can check with 'import wx' then 'wx.version()']

@markblinkhorn Can you or anyone else using wx 4.2 select notes - or play them?

@markblinkhorn
Copy link

markblinkhorn commented Mar 15, 2024

@revad All that happens when I drag a selection rectangle over a bunch of notes is a yellow box which disappears when I release the mouse button. No notes are selected. The tune plays from the start when I hit the play button.. It doesn't matter whether it is single or multivoice tune. Linux or Windows, both do the same. Here's the fun part; it doesn't throw any errors in the underlying Python window.

Edited for completeness:

C:\Windows\System32>python
Python 3.12.2 (tags/v3.12.2:6abddd9, Feb  6 2024, 21:26:36) [MSC v.1937 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import wx
>>> wx.version
<function version at 0x000001BF71196700>
>>> wx.version()
'4.2.1 msw (phoenix) wxWidgets 3.2.2.1'
>>>
~ $ python
Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import wx
>>> wx.version()
'4.2.0 gtk3 (phoenix) wxWidgets 3.2.2'
>>>

@topchyan
Copy link

@topchyan You wrote above that you could select and play a line of a single-voice tune in Python 3.8. Which version of wxpython are you using? [You can check with 'import wx' then 'wx.version()']

$ python3
Python 3.8.18 (default, Mar  7 2024, 19:01:52) 
[GCC 12.2.0] on linux
>>> import wx
>>> wx.version()
'4.2.1 gtk3 (phoenix) wxWidgets 3.2.2.1'

@markblinkhorn
Copy link

An observation - I'm not sure whether it will help anyone or not. The following code fragment does not produce any output when notes are drag selected:

    def select_notes(self, selection_rect):
        in_selection = selection_rect.Contains
        selected_offsets = set((abc_row, abc_col) for (x, y, abc_row, abc_col, desc) in self.notes if in_selection((x, y)))
        if selected_offsets:
            print("Hello World!")
        list_of_sets = [self.indices_per_row_col[row][col] for row, col in selected_offsets]
        selected_indices = set().union(*list_of_sets)
        if selected_indices:
            selected_indices = set(range(min(selected_indices), max(selected_indices) + 1))
        self.selected_indices = selected_indices
        return selected_indices

but the following code fragment does indeed say hello:

    def select_notes(self, selection_rect):
        in_selection = selection_rect.Contains
        selected_offsets = set((abc_row, abc_col) for (x, y, abc_row, abc_col, desc) in self.notes if in_selection((x, y)))
        if in_selection:
            print("Hello World!")
        list_of_sets = [self.indices_per_row_col[row][col] for row, col in selected_offsets]
        selected_indices = set().union(*list_of_sets)
        if selected_indices:
            selected_indices = set(range(min(selected_indices), max(selected_indices) + 1))
        self.selected_indices = selected_indices
        return selected_indices

This would imply that the set statement at line 3 is returning an empty set. I'm not familiar enough with the coding logic to take it much further, but someone esle might...

@revad
Copy link
Author

revad commented Mar 16, 2024

'selection_rect' is an instance of wx.Rect, and Contains() is one of its methods, which returns a bool:
https://docs.wxpython.org/wx.Rect.html
So 'in_selection' without any arguments is undefined, I would have thought, and might be true or false - though I'm slightly surprised it's not false!

I'm glad that @topchyan reports that selection works on python 3.8 with wx 4.2.1. I'll look at changes in python 3.9 and 3.10. I notice that 3.9 is "the last version providing those Python 2 backward compatibility layers..."
https://docs.python.org/3/whatsnew/3.9.html

@markblinkhorn
Copy link

@revad Ah, so a pointer to the .Contains method then - I can see why it isn't false. You are probably on to something with the Python versions. Shame it took so long to deprecate Python2, it can be a real nightmare spotting all of the needed code changes. Good luck!
Mark

@markblinkhorn
Copy link

@revad Did you know x and y are floats?
This works:

        selected_offsets = set((abc_row, abc_col) for (x, y, abc_row, abc_col, desc) in self.notes if in_selection((int(x), int(y))))

@revad
Copy link
Author

revad commented Mar 16, 2024

Yes, I just discovered that!

notes: [(111.1, 104.0, 10, 2, ('N', 10, 2, 104.6, 73.0, 13.0, 37.0)), ...
selection: (100, 91, 0, 0)

I was wondering whether notes should become integer throughout - rather a wide-reaching change I suspect.

@markblinkhorn
Copy link

I wondered that too - it would fix the problem at source, which is always preferable. As you say, a wide-reaching change...

@revad
Copy link
Author

revad commented Mar 16, 2024

So my personal copy of that function now looks like this:

#DR1 Multi-voice selection - issue 77
#DR2 notes x,y are floats, wx selected notes are integers - also issue 77

    def select_notes(self, selection_rect):
        in_selection = selection_rect.Contains
#DR2        selected_offsets = set((abc_row, abc_col) for (int(x), int(y), int(abc_row), int(abc_col), desc) in self.notes if in_selection((x, y)))
        selected_offsets = set((abc_row, abc_col) for (x, y, abc_row, abc_col, desc) in self.notes if in_selection((int(x), int(y))))
        list_of_sets = [self.indices_per_row_col[row][col] for row, col in selected_offsets]
        selected_indices = set().union(*list_of_sets)
#DR1        if selected_indices:
#DR1            selected_indices = set(range(min(selected_indices), max(selected_indices), + 1))       
        self.selected_indices = selected_indices
        return selected_indices

I notice that if the selection only contains two notes, instead of playing the selection it plays the whole tune. Odd, but OK I suppose.

Thanks, Mark.

@markblinkhorn
Copy link

This matches my version too, however, I find it only plays Voice 1 if I select a segment of a multi-voice tune. Which, I think, leaves us with a problem similar to the one you originally posted back in August...

@revad
Copy link
Author

revad commented Mar 16, 2024

The original problem, illustrated by the image and soundclip in the OP, was that it would not play a segment of a mult-voice tune: it selected notes outside the box. That's change #DR1

I can now play sections of single or multi-voice tunes, so long as they are in a single box. At least I can in 3.10 (and 3.6 in fact).

Maybe another change in 3.11? Or something about your ABC?

@markblinkhorn
Copy link

markblinkhorn commented Mar 16, 2024

It's my ABC code - I can live with that ;) Your OP example does play a multi-voice segment correctly. I haven't worked out why my example doesn't work yet though...

Update: The example I was using has chords in Voice 2. Changing these to single notes allows them to play.

The code set as we have it currently isnt handling chords correctly. I'm sure I have read something about this previously, somewhere...

@revad
Copy link
Author

revad commented Mar 17, 2024

If the selection includes a chord, it will not play. Function select_notes() returns a set of indexes correctly but the player ignores the chord. This is true of single-voice tunes too.

Example:
Selected: bar 2 containing 1 single note, a 2 note chord, 2 single notes
list_of_sets: [{4}, {7}, {5, 6}, {8}]
selected_indices: {4, 5, 6, 7, 8}
actually plays: 4, 7, 8

I tried converting the chord into a single note, so at least it would keep time, by adding this:

        for x in  list_of_sets:
          if len(x) > 1:
            for y in x:
              z = y
              x.discard(y)
            x.add(z)

It correctly selects one note of the chord - but still doesn't play it.

A comment in easy_abc.txt says:
GetAbcToPlay is called to extract the tune from the editor. (The function provides an option of playing only selected notes.)
That's the place to look for a solution. I have seen comments that playing a selection ceased to work some time ago; whether it ever played chords I do not know.

Fortunately, I can live with this limitation.

@markblinkhorn
Copy link

Coincidentally, I am reading GetABCToPlay right now. I too could probably live with this limitation (now that I have recognised it as such) but, for now, my interest is piqued. I'll open a new discussion on this subject IF I make any progress ;)
Thanks for your input.
Mark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants