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 helper APIs for adding & removing port destroy handler #4244

Merged
merged 3 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions pjmedia/include/pjmedia/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,18 @@ PJ_DECL(pj_status_t) pjmedia_port_destroy( pjmedia_port *port );
* lock by adding the port's destructor to the group lock handler list, and
* increment the reference counter.
*
* This function should only be called by media port implementation. The port
* must have its own pool which will be released in its on_destroy() function.
* Normally this function is only called by media port implementation,
* application should never call this function.
*
* This function will check whether the port implements on_destroy(),
* because when working with conference bridge, a port is required to manage
* its lifetime via group lock (e.g: initialized by this function),
* so basically it must use its own pool and the pool is normally
* released from its on_destroy().
*
* Also note that this function will add a group lock destroy handler
* that calls port's on_destroy(), so port implementation can release
* its resources from on_destroy() as usual.
*
* @param port The pjmedia port to be initialized.
* @param pool The pool, this can be a temporary pool as
Expand Down Expand Up @@ -563,6 +573,46 @@ PJ_DECL(pj_status_t) pjmedia_port_add_ref( pjmedia_port *port );
PJ_DECL(pj_status_t) pjmedia_port_dec_ref( pjmedia_port *port );


/**
* This is a helper function to add port destructor handler.
Copy link
Member

Choose a reason for hiding this comment

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

It's not obvious when users can, or should, call this function? Should we call pjmedia_port_init_grp_lock() first? Or after adding the port to conf?

Copy link
Member Author

@nanangizz nanangizz Jan 7, 2025

Choose a reason for hiding this comment

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

After adding the port to conf (where port is guaranted to have a group lock, i.e: added by conf if not yet). Updated #3928 desc issue 1 point ii. Should it be added to the function docs (also about avoiding premature pool release)?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please.

Adding to the doc will be of great help in the future so we can easily recall how we're supposed to do this.

*
* Application can use this function to schedule its resource release.
* Note that application cannot release the resources related to the port,
* e.g: memory pool (custom ports that do not use its own pool),
* immediately after removing the port from the conference bridge
* as the port removal is done asynchronously.
*
* Usually this function is called after adding the port to the conference
* bridge.
*
* @param port The PJMEDIA port.
* @param member A pointer to be passed to the handler.
* @param handler The destroy handler.
*
* @return PJ_SUCCESS on success.
*/
PJ_DECL(pj_status_t) pjmedia_port_add_destroy_handler(
pjmedia_port* port,
void *member,
pj_grp_lock_handler handler);


/**
* This is a helper function to remove previously registered
* destructor handler.
*
* @param port The PJMEDIA port.
* @param member A pointer to be passed to the handler.
* @param handler The destroy handler.
*
* @return PJ_SUCCESS on success.
*/
PJ_DECL(pj_status_t) pjmedia_port_del_destroy_handler(
pjmedia_port* port,
void *member,
pj_grp_lock_handler handler);


PJ_END_DECL

/**
Expand Down
30 changes: 30 additions & 0 deletions pjmedia/src/pjmedia/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,33 @@ PJ_DEF(pj_status_t) pjmedia_port_dec_ref( pjmedia_port *port )

return pj_grp_lock_dec_ref(port->grp_lock);
}


/**
* Add destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_port_add_destroy_handler(
pjmedia_port* port,
void* member,
pj_grp_lock_handler handler)
{
PJ_ASSERT_RETURN(port && handler, PJ_EINVAL);
PJ_ASSERT_RETURN(port->grp_lock, PJ_EINVALIDOP);

return pj_grp_lock_add_handler(port->grp_lock, NULL, member, handler);
}


/**
* Remove previously registered destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_port_del_destroy_handler(
pjmedia_port* port,
void* member,
pj_grp_lock_handler handler)
{
PJ_ASSERT_RETURN(port && handler, PJ_EINVAL);
PJ_ASSERT_RETURN(port->grp_lock, PJ_EINVALIDOP);

return pj_grp_lock_del_handler(port->grp_lock, member, handler);
}
Loading