> On March 13, 2014, 5:43 p.m., rmudgett wrote: > > You need to add cases to the switches that handle AST_FRAME_BRIDGE_ACTION > > in frame.c and bridge_softmix.c. It is probably best if the > > AST_FRAME_BRIDGE_ACTION_SYNC case is next to the AST_BRIDGE_ACTION case. > > > > The main difficulty with this patch is that you are adding an allocated > > resource to ast_frame's which inherently doesn't support this.
I don't understand the comment about adding an allocated resource to ast_frame. The frame payload is allocated on the stack, but that's the same strategy used for all bridge_playfile payloads right now. The bridge_playfile is memcpy'd into the sync_payload, and the sync_payload's data length is adjusted to account for this. When ast_frdup() copies the frame data to another frame, the entire sync_payload and encased bridge_playfile is copied to the other frame, so I'm not sure what the problem here is. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3338/#review11177 ----------------------------------------------------------- On March 13, 2014, 2:53 p.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3338/ > ----------------------------------------------------------- > > (Updated March 13, 2014, 2:53 p.m.) > > > Review request for Asterisk Developers and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > ARI tests were occasionally showing a channel being stuck forever in Stasis. > After looking at a live system where the problem was occurring, it became > clear that the channel was stuck trying to wait for a playback to finish. > However, it was also clear that the playback had been abandoned long ago. > > When playing back a file to a channel that is in a bridge, the idea was to > queue the playfile action onto the corresponding bridge channel, and then > wait until the ARI custom playback function completed to signal that the > playback had finished. There were several ways that this could fail: > * If the bridge channel were not found in the bridge, then we would never > attempt to queue the playfile action, but we would still try to wait for it > to finish happening. > * If queuing the playfile action did not succeed, then we would still attempt > to wait for the action to occur. > * If the playback action was successfully queued, but the bridge channel were > removed from the bridge before the playfile action could start, then the > playback would never happen, and we'd wait forever. > > The responsibility of knowing whether a playfile action has conpleted or ever > will occur is the bridge's, since ARI can never fully know. With this in > mind, I have created a new bridge channel API call to queue a playfile action > synchronously. The way it is done, synchronous queuing operations could be > created for other bridge actions quite easily. A new frametype, > AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a > synchronization ID and the actual payload of the bridge action that is to be > queued. When the frame is queued, a synchronization structure is created and > added to a linked list. The procedure then blocks until it is told that the > frame has been disposed of. > > When the frame gets freed (which will occur whether the playfile action > succeeds or does not happen), the freer of the frame signals the waiting > procedure that the playfile action has terminated, and control returns to > whoever queued the playfile action in the first place. > > From ARI's perspective, this greatly simplifies its code. Most of the > res_stasis_playback changes are code removal. The bridge_channel changes, > though, are pretty large, which is why I've included Richard on this issue. > > > Diffs > ----- > > /branches/12/res/res_stasis_playback.c 410467 > /branches/12/main/channel.c 410467 > /branches/12/main/bridge_channel.c 410467 > /branches/12/include/asterisk/frame.h 410467 > /branches/12/include/asterisk/bridge_channel.h 410467 > /branches/12/funcs/func_frame_trace.c 410467 > > Diff: https://reviewboard.asterisk.org/r/3338/diff/ > > > Testing > ------- > > Initially, I did some manual playbacks using Swagger-UI to a channel in a > bridge to ensure that the specified file was actually being played as > promised. I also queued up several files in quick succession to ensure that > they played in series and not on top of each other. > > In addition, I have created an automated test that is up for review at /r/3339 > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
