AudioBridge-stop_all_files #3392
Conversation
src/plugins/janus_audiobridge.c
Outdated
| "room" : <unique numeric ID, same as request>, | ||
| "file_id_list" : [ | ||
| // Array of file_Id: "<unique string ID of the now interrupted announcement>"" | ||
| ] |
There was a problem hiding this comment.
Indentation seems broken here (there's one extra tab).
src/plugins/janus_audiobridge.c
Outdated
| goto prepare_response; | ||
| } | ||
|
|
||
| /* Get list of started announcements and send a stop announcement notification*/ |
There was a problem hiding this comment.
There's a missing space at the end of the sentence (the asterisk of the comment is right next to the last word).
| janus_refcount_decrease(&p->ref); | ||
| } | ||
|
|
||
| /* Get rid of the announcements */ |
There was a problem hiding this comment.
I don't understand why this is a separate block. You were already looping on all announcements before, so that's where you can populate the array too.
There was a problem hiding this comment.
I don't like either but since we are removing an element (g_hash_table_remove) inside audiobridge->anncs inside the loop, while loop above crashes with g_hash_table_iter_next: assertion 'ri->version' == ri->hash_table->version.... error.
Sorry, I'm not expert on C and hash_table, I'm open other suggestions
I can copy audiobridge->anncs into another hashtable changing the ref and while looping the new hashtable
src/plugins/janus_audiobridge.c
Outdated
| } | ||
|
|
||
| /* Get rid of the announcements */ | ||
| json_t *listAnncRemoved = json_array(); |
There was a problem hiding this comment.
Code style: we don't use camel case for variables.
src/plugins/janus_audiobridge.c
Outdated
|
|
||
| /* Get rid of the announcements */ | ||
| json_t *listAnncRemoved = json_array(); | ||
| for (GList *l = items_to_remove; l!= NULL; l = l->next) { |
There was a problem hiding this comment.
Code style: don't define variables in the for itself. Besides, please don't add an extra space between directive and bracket (it's for(, not for ().
src/plugins/janus_audiobridge.c
Outdated
| for (GList *l = items_to_remove; l!= NULL; l = l->next) { | ||
| json_t *file_id = json_string(l->data); | ||
| if( g_hash_table_remove(audiobridge->anncs, l->data)) | ||
| { |
There was a problem hiding this comment.
Code style: same for the if, there's an extra space that shouldn't be there before g_hash_table_remove.
The bracket of the check should be on the same line as the check, not below as you did here (if(..) {)
Audio bridge list announcements
|
Did you mean to base this upon your "listannouncements" branch? I see changes coming from that PR too, in this branch. |
|
@keremcadirci please address my comment above, since it introduced a conflict when I merged your other PR. |
Requirement:
One or more announcement from file can be played by an admin in the audiobridge room
An admin without knowing
file_ids may want to stop all announcements.Currently we need to know all
file_ids and we need call manystop_fileapi to stop all annoucements.Proposol
A new
stop_all_filesapi that will stop all playing announcements.A
announcement-stoppedevent will be fired for all stopped announcementsstop_all_filesapi will return stoppedfile_idlist