Implement Dialog Event subscription (client only) #3754
Conversation
Implement busy line field implementation
|
|
|
This is a rather extensive PR, so we may need some time to review it.
|
Without deletion {} in "struct pjdialog_info_op_desc pjdialog_info_op" compiler fails wiht error C2078.
|
Hi @sauwming! |
|
Thank you for the patch, appreciate it. Coincidentally, there are some of our users who recently inquired about this feature, so this could be useful. I tried it here and have to fix adding the declaration of So I guess what's missing here are some test scenarios for us to verify that the offered features and functionalities are indeed working as expected. For example: |
|
Hi @sauwming! | sleep MS echo [0|1|txt] | Enter buddy's URI: (empty to cancel): sip:[email protected] --end msg-- --end msg-- --end msg-- --end msg-- --end msg-- --end msg-- |
|
I am using Kamailio for testing and after following https://kb.asipto.com/kamailio:presence:k43-blf, now it can work correctly. Thx for the pointer. |
sauwming
changed the title
* Add callbacks description. * Sort includes alphabetically. * Apply stylistic formating.
|
To update you, we will review this during our internal team meeting next week. Note: the Windows build failed. |
|
@sauwming Thanks for your feedback. |
|
I was referring to your description which mentions that there is not a specific RFC dedicated to the Busy Lamp Field (BLF), and that the actual implementation of BLF can vary between different VoIP systems and devices. |
|
What has been done:
Ready for review, @nanangizz and @goodicus . Please let me know if there are any changes that need to be incorporated. |
|
Hi @sauwming!
...
Choices: |
|
I previously tested it using |
|
@sauwming Everything is fine for me. |
|
I still can't test this past the initial SUBSCRIBE. Since this doesn't have any server implementation, I tried testing against various other SIP phones such as Linphone, Zoiper, but so far I had no luck. |
pjsip/src/pjsip-simple/dlg_event.c
Outdated
| if (dlgev->tmp_status._is_valid) { | ||
| PJ_ASSERT_RETURN(dlgev->tmp_pool!=NULL, PJSIP_SIMPLE_ENOPRESENCE); | ||
| pj_memcpy(status, &dlgev->tmp_status, sizeof(pjsip_dlg_event_status)); | ||
| } else { | ||
| PJ_ASSERT_RETURN(dlgev->status_pool!=NULL, PJSIP_SIMPLE_ENOPRESENCE); | ||
| pj_memcpy(status, &dlgev->status, sizeof(pjsip_dlg_event_status)); | ||
| } |
There was a problem hiding this comment.
Protect access to dlgev states with lock?
There was a problem hiding this comment.
I have modified everything except these two.
Note that the code here is copied from presence, so if it requires lock here, it probably needs one there too, and both don't have their own lock at the moment.
There was a problem hiding this comment.
This is one possible race condition scenario:
- Thread 1: executes
pjsip_dlg_event_get_status(),dlgev->tmp_status._is_validis evaluated as true, but beforememcpy(status, tmp_status, ..), context switch happens. - Thread 2: executes
dlg_event_process_rx_notify()in this line:dlgev->tmp_status._is_valid = PJ_FALSE; pj_pool_reset(dlgev->tmp_pool); - Back to Thread 1: it copies from
dlgev->tmp_statuswhose pool has just been reset (not swapped due top_st_codeis not 2xx).
This may be a corner case as usually NOTIFY will be responded with 200.
There was a problem hiding this comment.
OK, so how do we proceed here? I only add lock for dlg event and create the fix for presence in a separate PR?
|
Modifications in the latest commit:
|
sauwming
changed the title
- wrong callback on event: TSX_STATE when using dialog-event subscription - wrong method called to re-subscribe from timer callback
Implement Dialog Event subscription (client only) for dialog monitoring purpose, which can be useful for features such as BLF (Busy Lamp Field) that displays the state of the calls associated with the Sip extension.
The most important RFCs related to this are: