put _wrap_port_ptr calls in try statment, to avoid errors when port is nor audio nor midi

[Houston4444]

catching a redondant AssertionError in RaySession: Houston4444/RaySession#269, I saw that, for a reason I ignore now, some ports are created and destroyed quickly by KDE Plasma under Wayland. These ports are not audio nor midi, they are of type 'other'. I am ok to find it strange, but I don't think that anything prevent this in JACK (here it is PipeWire).

So, the idea of this PR is to not display the error when a port has other type than _AUDIO or _MIDI, because it is not forbidden by the JACK API.

Another possibility would be to create another class OtherPort (and OwnOtherPort).

[mgeier]

Thanks for this PR, and sorry for my late response.

I agree that this is a problem that should be solved, but I'm not sure if catching an AssertionError is the right thing to do.

An assertion happens only when there is a bug, and I see only two ways to proceed: Either the bug should be fixed (and the assert stays), or the assert should be removed (in this case, having the assert was the bug in the first place).

Catching an AssertionError didn't ever cross my mind, it seems wrong.

Creating an OtherPort and OwnOtherPort is an option, but is there something meaningful that could be done with it? What methods would it have?

Another option would probably be to create an UnknownPort (probably without any methods?) that's created whenever the type is unknown. Maybe an OwnUnknownPort is not needed, then.

What do you think about that?

[Houston4444]

You are right, we can consider that assert False line is the bug.

I like the idea of an UnknownPort class, but I think it should contain the same methods than AudioPort or MidiPort, plus a type property, returning a str with the decoded value of _lib.jack_port_type(). I say this because finally, AUDIO and MIDI ports are just conventions, an user could use another port type to communicate between its programs, it is not forbidden. It would be logical in this case that type property of Port class return decode(_AUDIO) and type property of MidiPort return decode(_MIDI).

The problem is that it breaks this lib API which only provide AUDIO or MIDI ports (for example in port register callback). A solution to solve this is to add a keyword argument (even_unknown=False ?) to port_register_callback to allow callbacks from UnknownPort, and also add the keyword argument is_unknown=False to the method get_ports() of Client class.

Then the _wrap_port_ptr() method should not raise exception but return an UnknownPort instance in this case.

How does it sounds ?

[mgeier]

I like the idea of an UnknownPort class, but I think it should contain the same methods than AudioPort or MidiPort,

OK, I guess we can also provide those methods. Maybe in this case we can even use Port as a base class?

Would it make sense to have an OwnUnknownPort as well, then?

But probably without get_buffer() and get_array() (just like OwnMidiPort)?

plus a type property, returning a str with the decoded value of _lib.jack_port_type().

Yes, that makes sense.

The problem is that it breaks this lib API which only provide AUDIO or MIDI ports (for example in port register callback).

They currently only provide AUDIO or MIDI because the code is not aware of any other types.

But why should it not provide additional port types once they exist?

A solution to solve this is to add a keyword argument (even_unknown=False ?) to port_register_callback to allow callbacks from UnknownPort,

Why not allow all callbacks unconditionally?

and also add the keyword argument is_unknown=False to the method get_ports() of Client class.

Yes, I think this would make sense.

Then the _wrap_port_ptr() method should not raise exception but return an UnknownPort instance in this case.

How does it sounds ?

Sounds great!

[Houston4444]

They currently only provide AUDIO or MIDI because the code is not aware of any other types.
But why should it not provide additional port types once they exist?

I think to the case someone has written a such thing in the port register callback

if port.is_audio:
    do_stuff_with_audio_port(port)
else:
    # because in the current code, a port is audio or midi, it is a midi port
    do_stuff_with_midi_port(port)

but maybe we can consider it is not a big deal.

FYI there are also 3 other default types added with pipewire : https://gitlab.com/pipewire/pipewire/-/blob/master/pipewire-jack/src/pipewire-jack-extensions.h?ref_type=heads .

JACK_DEFAULT_VIDEO_TYPE "32 bit float RGBA video"
JACK_DEFAULT_OSC_TYPE "8 bit raw OSC"
JACK_DEFAULT_UMP_TYPE "32 bit raw UMP"

It's not a big deal if we don't implement this, user will achieve to consider them with the Port.type property.

If you see no objection, I can work on another PR managing UnknownPort this week-end !

[mgeier]

Yes, I think we should consider this a breaking change.

We should increase the version number to 0.6 and mention this in the release notes.

We'll see if people complain about this, but I'm willing to take the risk.

FYI there are also 3 other default types added with pipewire [...] It's not a big deal if we don't implement this

I agree. For now, any new types will be "unknown". But if anyone needs special support for new types, we can add more port classes (in future PRs).

If you see no objection, I can work on another PR managing UnknownPort this week-end !

No objections!