----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3046/#review10329 -----------------------------------------------------------
/branches/12/main/framehook.c <https://reviewboard.asterisk.org/r/3046/#comment19729> I'd scrap the goto loop. do { } while (frame != original_frame) does the same without being ew. The other issue here is that two frame hooks could cancel each other out: * original_frame = foo * frame hook A returns converts foo to bar * frame hook A is passed on * frame hook B converts bar to foo * repeat... :-( We could ignore this, but if we ever hit this problem, it will be incredibly ugly. The other option is to clone the framehook list before we start the pass. When a framehook modifies the frame, we remove the framehook from the cloned list and then re-iterate. That guarantees that a framehook is never called twice and prevents this situation. /branches/12/main/framehook.c <https://reviewboard.asterisk.org/r/3046/#comment19728> Blob My only major fear with this change is that it effectively adds a substantial amount of processing on every frame. It does guarantee corectness, but I think it would be worthwhile to take a look at our framehook implementations to ensure that they aren't doing anything beyond the bare minimum for frames that they don't otherwise consume. - Matt Jordan On Dec. 5, 2013, 5:20 p.m., Joshua Colp wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3046/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2013, 5:20 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > Currently when a frame is changed by a frame hook previous hooks aren't aware > of it. This can be a problem when a previous hook reacts to the types of > frames that a subsequent hook produces. An example of this would be the fax > gateway hook and the PJSIP T.38 hook. Since the fax gateway hook injects T.38 > control frames after the T.38 hook, nothing happens. > > The attached change makes it so if a frame hook produces a different frame > the hook iteration loop is restarted, skipping the hook that has produced the > frame. > > > Diffs > ----- > > /branches/12/main/framehook.c 403362 > > Diff: https://reviewboard.asterisk.org/r/3046/diff/ > > > Testing > ------- > > Ran fax tests, confirmed fixed. > > > Thanks, > > Joshua Colp > >
-- _____________________________________________________________________ -- 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