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

Add an opcode for note names, and API bindings for note and cc names #174

Merged
merged 13 commits into from
Apr 18, 2020

Conversation

paulfd
Copy link
Member

@paulfd paulfd commented Apr 8, 2020

@jpcima I'm not super sure about the C API design rules here
@redtide I added an @version tag, is this OK? We'll need to fix it before the release!
@alcomposer you just need to integrate your note name midnam in Ardour now 🙂

  • Add and track switch labels and currently switched-on regions

Closes #154

@paulfd paulfd requested review from jpcima and redtide April 8, 2020 11:38
Copy link
Member

@redtide redtide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to me!

@jpcima
Copy link
Collaborator

jpcima commented Apr 8, 2020

@jpcima
Copy link
Collaborator

jpcima commented Apr 11, 2020

@returns [...] or NULL if the index is out of bounds.

I prefer to avoid specifying this part. The index must be in bounds.

@paulfd paulfd linked an issue Apr 13, 2020 that may be closed by this pull request
@jpcima
Copy link
Collaborator

jpcima commented Apr 14, 2020

This might be an incomplete definition of the NoteNameList.

An example from Motif-XF-8

                <ChannelNameSet Name="CP ChannelSet 1" >
                        <PatchBank Name="GM Kit" >
[...]
                                <UsesPatchNameList Name="GM Kit" />
                        </PatchBank>
[...]
                </ChannelNameSet>
[...]
                <PatchNameList Name="GM Kit" >
                        <Patch Number="" Name="Stereo GM Kit" ProgramChange="0" >
                                <UsesNoteNameList Name="SFX Kit" />
                        </Patch>
                </PatchNameList>
[...]
                <NoteNameList Name="SFX Kit" >

@jpcima
Copy link
Collaborator

jpcima commented Apr 15, 2020

I've added following changes

  • rebased on branch develop to eliminate the conflict
  • added the last commit which renames label_note to label_key, and associated API and comments
    it's to make it consistent with SFZ terminology in existing opcodes (eg. key, pitch_keycenter, ..)
    @paulfd OK with it?

@paulfd
Copy link
Member Author

paulfd commented Apr 15, 2020

Yes no problem, it is more coherent.

@alcomposer I added the keyswitch label if you want to try with your branch!

@jpcima
Copy link
Collaborator

jpcima commented Apr 18, 2020

With the last addition, do we run the risk that the key could appear multiple times in the list?

@paulfd
Copy link
Member Author

paulfd commented Apr 18, 2020

Yes, is it a problem? IIRC the DTD does not enforce unique values for this attribute. We run this risk for repeated label_key or label_cc too btw.

@jpcima
Copy link
Collaborator

jpcima commented Apr 18, 2020

Then it's no issue, let's validate in Ardour6 and this should be good

@paulfd
Copy link
Member Author

paulfd commented Apr 18, 2020

cf https://www.midi.org/dtds/MIDINameDocument10.dtd.html

Note that specifying unique attributes in XML is a bit involved I think, so it might be that they did not want to bother specifying it. However, we may be able to leave it to the host to choose what to do with duplicated notes (e.g. take the last, or take them all). In a way we're passing on the ambiguity from the sfz without committing to a resolution.

@paulfd paulfd merged commit 0bf02ac into sfztools:develop Apr 18, 2020
@paulfd paulfd deleted the note-names branch January 21, 2022 10:24
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

Successfully merging this pull request may close these issues.

Add label_noteNN and bindings from the APIs for these and label_ccNN
3 participants