[Asterisk-code-review] app stream echo: Added a multi-stream echo application (asterisk[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/5629 )

Change subject: app_stream_echo: Added a multi-stream echo application
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.asterisk.org/#/c/5629/2/apps/app_stream_echo.c
File apps/app_stream_echo.c:

PS2, Line 177: 			if (f->subclass.integer == AST_CONTROL_VIDUPDATE && !update_sent) {
             : 				if (stream_echo_write(chan, f, one_to_one, type)) {
             : 					ast_frfree(f);
             : 					return -1;
             : 				}
             : 				update_sent = 1;
> Maybe not? I pulled this behavior straight from app_echo. I tend to agree w
Update! Commit cc86329228a9e27add4d9962b43d37ac745abca8 added this functionality in because this, and I quote, "can cause a Kafkaesque nightmare if two or more Local channels are in a bridge together where their ends are in app_echo". See commit message for more details if needed.


Line 199: 		    stream_echo_write(chan, f, one_to_one, type)) {
> Again, this behavior was pulled from app_echo. I'm fine though with it not 
After doing a blame and looking through the log I'm still not sure if there is a good reason why it exits on a single write failure except that if an error occurs just bail out. Seeing as there haven't been any complaints (that I know of) with this with regards to app_echo I think I'll just leave it as is too.

That being said if you or anyone else feels strongly about it I'd also be fine with removing the frame write as a failure condition (or at least for some number of write failures).


-- 
To view, visit https://gerrit.asterisk.org/5629
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I254144486734178e196c7f590a26ffc13543ff2c
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes