Add ptz to ndi source #885
Conversation
exeldro
commented
May 9, 2023
- removed OBS_PROPERTIES_DEFER_UPDATE to update the camera as soon as property is changed
- add check if ndi_name or bandwidth changed to recreate ndi_receiver
- hide source in audio mixer when no audio is selected
- add checkable ptz group to properties to control the pan tilt and zoom with sliders
|
@exeldro I can't explain how I didn't see this before. |
|
@paulpv after this I added hotkeys, see: |
paulpv
left a comment
There was a problem hiding this comment.
Still looking through the other charges...
| { | ||
| if (!obs_data_get_bool(settings, PROP_PTZ)) | ||
| return; | ||
| float pan = (float)obs_data_get_double(settings, PROP_PAN); |
There was a problem hiding this comment.
Should there be any range checks here?
Will NDI ptz accept bad values and not bork?
There was a problem hiding this comment.
the min and max are set in the properties, but we can add an extra check here
| float pan = (float)obs_data_get_double(settings, PROP_PAN); | ||
| float tilt = (float)obs_data_get_double(settings, PROP_TILT); | ||
| ndiLib->recv_ptz_pan_tilt(s->ndi_receiver, pan, tilt); | ||
| float zoom = (float)obs_data_get_double(settings, PROP_ZOOM); |
There was a problem hiding this comment.
Should there be any range checks here?
Will NDI ptz accept bad values and not bork?
|
|
||
| void ndi_source_ptz(struct ndi_source *s, obs_data_t *settings) | ||
| { | ||
| if (!obs_data_get_bool(settings, PROP_PTZ)) |
There was a problem hiding this comment.
Should the code also test for recv_ptz_is_supported?
Sorry, acronymnesia: I was confusing Advanced KVM ability with Standard PTZ ability. |
paulpv
left a comment
There was a problem hiding this comment.
Looks great.
I made a few changes in the develop branch obs-ndi-source.cpp, so I'll take care of porting your changes from master to develop after I merge your master based changes to master.
| obs_properties_add_bool(props, PROP_AUDIO, | ||
| obs_module_text("NDIPlugin.SourceProps.Audio")); | ||
|
|
||
| obs_properties_t *group = obs_properties_create(); |
| { | ||
| if (!obs_data_get_bool(settings, PROP_PTZ)) | ||
| return; | ||
| float pan = (float)obs_data_get_double(settings, PROP_PAN); |
| hwAccelMetadata.p_data = | ||
| hwAccelEnabled | ||
| ? (char *)"<ndi_hwaccel enabled=\"true\"/>" | ||
| : (char *)"<ndi_hwaccel enabled=\"false\"/>"; |
| ndiLib->recv_destroy(s->ndi_receiver); | ||
|
|
||
| NDIlib_recv_create_v3_t recv_desc; | ||
| recv_desc.source_to_connect_to.p_ndi_name = ndi_name; |
|
@exeldro Did you see any actual benefit to only reinitiating the ndi_receiver if the name, bandwidth, or hwacceleration changed? The old code always reinitialized the ndi_receiver, and I am thinking of moving back to that unless you had an obviously good reason in your code. |
|
@paulpv reinitiating the ndi_receiver is visible in the output, so if the user wants to do live changes viewers will see interruptions |
Shouldn't almost all of these options result in the preview resetting? Do you think that only name, bandwidth, and hwaccel are the only properties that should reset the preview? |
|
OH! I see how that could be a big issue with the PTZ preview! |
|
hwaccel does not need to reset because recv_send_metadata can be used to change that live and all options after that only change things local in obs so no need to reset the ndi reciever for that |