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 all 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
45 changes: 45 additions & 0 deletions pjmedia/include/pjmedia/conference.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,10 @@ PJ_DECL(unsigned) pjmedia_conf_get_connect_count(pjmedia_conf *conf);
* should not assume that the port will no longer receive/send audio frame
* after this function has returned.
*
* If the port uses any app's resources, application should maintain
* the resources validity until the port is completely removed. Application
* can schedule the resource release via #pjmedia_conf_add_destroy_handler().
*
* @param conf The conference bridge.
* @param slot The port index to be removed.
*
Expand Down Expand Up @@ -602,6 +606,47 @@ PJ_DECL(pj_status_t) pjmedia_conf_adjust_conn_level( pjmedia_conf *conf,
int adj_level );


/**
* Add port destructor handler.
*
* Application can use this function to schedule resource release.
* Note that application cannot release any app's resources used by the port,
* e.g: memory pool, database connection, immediately after removing the port
* from the conference bridge as port removal is asynchronous.
*
* Usually this function is called after adding the port to the conference
* bridge.
*
* @param conf The conference bridge.
* @param slot The port slot index.
* @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_conf_add_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler);


/**
* Remove previously registered destructor handler.
*
* @param conf The conference bridge.
* @param slot The port slot index.
* @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_conf_del_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler);


PJ_END_DECL

Expand Down
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
49 changes: 46 additions & 3 deletions pjmedia/include/pjmedia/vid_conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ PJ_DECL(unsigned) pjmedia_vid_conf_get_port_count(pjmedia_vid_conf *vid_conf);
/**
* Enumerate occupied slots in the video conference bridge.
*
* @param vid_conf The video conference bridge.
* @param vid_conf The video conference bridge.
* @param slots Array of slot to be filled in.
* @param count On input, specifies the maximum number of slot
* in the array. On return, it will be filled with
Expand Down Expand Up @@ -253,7 +253,7 @@ PJ_DECL(pj_status_t) pjmedia_vid_conf_get_port_info(
* Enable unidirectional video flow from the specified source slot to
* the specified sink slot.
*
* @param vid_conf The video conference bridge.
* @param vid_conf The video conference bridge.
* @param src_slot Source slot.
* @param sink_slot Sink slot.
* @param opt The option, for future use, currently this must
Expand All @@ -272,7 +272,7 @@ PJ_DECL(pj_status_t) pjmedia_vid_conf_connect_port(
* Disconnect unidirectional video flow from the specified source to
* the specified sink slot.
*
* @param vid_conf The video conference bridge.
* @param vid_conf The video conference bridge.
* @param src_slot Source slot.
* @param sink_slot Sink slot.
*
Expand Down Expand Up @@ -301,6 +301,49 @@ PJ_DECL(pj_status_t) pjmedia_vid_conf_update_port(pjmedia_vid_conf *vid_conf,
unsigned slot);



/**
* Add port destructor handler.
*
* Application can use this function to schedule resource release.
* Note that application cannot release any app's resources used by the port,
* e.g: memory pool, database connection, immediately after removing the port
* from the conference bridge as port removal is asynchronous.
*
* Usually this function is called after adding the port to the conference
* bridge.
*
* @param vid_conf The video conference bridge.
* @param slot The port slot index.
* @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_vid_conf_add_destroy_handler(
pjmedia_vid_conf* vid_conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler);


/**
* Remove previously registered destructor handler.
*
* @param conf The video conference bridge.
* @param slot The port slot index.
* @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_vid_conf_del_destroy_handler(
pjmedia_vid_conf* vid_conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler);


PJ_END_DECL

/**
Expand Down
37 changes: 37 additions & 0 deletions pjmedia/src/pjmedia/conf_switch.c
Original file line number Diff line number Diff line change
Expand Up @@ -1585,4 +1585,41 @@ static pj_status_t put_frame(pjmedia_port *this_port,
return PJ_SUCCESS;
}


/*
* Add destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_conf_add_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler)
{
PJ_UNUSED_ARG(conf);
PJ_UNUSED_ARG(slot);
PJ_UNUSED_ARG(member);
PJ_UNUSED_ARG(handler);

return PJ_ENOTSUP;
}


/*
* Remove previously registered destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_conf_del_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler)
{
PJ_UNUSED_ARG(conf);
PJ_UNUSED_ARG(slot);
PJ_UNUSED_ARG(member);
PJ_UNUSED_ARG(handler);

return PJ_ENOTSUP;
}


#endif
67 changes: 64 additions & 3 deletions pjmedia/src/pjmedia/conference.c
Original file line number Diff line number Diff line change
Expand Up @@ -1413,11 +1413,11 @@ static void op_disconnect_ports(pjmedia_conf *conf,
/* Disconnect source -> sink */
if (src_port && dst_port) {
/* Check if connection has been made */
for (i=0; i<src_port->listener_cnt; ++i) {
for (i=0; i<(int)src_port->listener_cnt; ++i) {
if (src_port->listener_slots[i] == sink_slot)
break;
}
if (i == src_port->listener_cnt) {
if (i == (int)src_port->listener_cnt) {
PJ_LOG(3,(THIS_FILE, "Ports connection %d->%d does not exist",
src_slot, sink_slot));
return;
Expand Down Expand Up @@ -1458,7 +1458,7 @@ static void op_disconnect_ports(pjmedia_conf *conf,
(int)dst_port->name.slen,
dst_port->name.ptr));

for (i=0; i<conf->max_ports; ++i) {
for (i=0; i<(int)conf->max_ports; ++i) {
int j;

src_port = conf->ports[i];
Expand Down Expand Up @@ -2806,4 +2806,65 @@ static pj_status_t put_frame(pjmedia_port *this_port,
return status;
}


/*
* Add destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_conf_add_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler)
{
struct conf_port *cport;
pj_grp_lock_t *grp_lock;

PJ_ASSERT_RETURN(conf && handler && slot < conf->max_ports, PJ_EINVAL);

pj_mutex_lock(conf->mutex);

/* Port must be valid and has group lock. */
cport = conf->ports[slot];
if (!cport || !cport->port || !cport->port->grp_lock) {
pj_mutex_unlock(conf->mutex);
return cport? PJ_EINVALIDOP : PJ_EINVAL;
}
grp_lock = cport->port->grp_lock;

pj_mutex_unlock(conf->mutex);

return pj_grp_lock_add_handler(grp_lock, NULL, member, handler);
}


/*
* Remove previously registered destructor handler.
*/
PJ_DEF(pj_status_t) pjmedia_conf_del_destroy_handler(
pjmedia_conf* conf,
unsigned slot,
void* member,
pj_grp_lock_handler handler)
{
struct conf_port* cport;
pj_grp_lock_t* grp_lock;

PJ_ASSERT_RETURN(conf && handler && slot < conf->max_ports, PJ_EINVAL);

pj_mutex_lock(conf->mutex);

/* Port must be valid and has group lock. */
cport = conf->ports[slot];
if (!cport || !cport->port || !cport->port->grp_lock) {
pj_mutex_unlock(conf->mutex);
return cport ? PJ_EINVALIDOP : PJ_EINVAL;
}
grp_lock = cport->port->grp_lock;

pj_mutex_unlock(conf->mutex);

return pj_grp_lock_del_handler(grp_lock, member, handler);
}


#endif
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
Loading