[asterisk-dev] [Code Review]: Support 'deaf' participants in ConfBridge

[fabled]

> On Jan. 3, 2012, 4:29 p.m., David Vossel wrote:
> > /trunk/main/bridging.c, lines 341-355
> > <https://reviewboard.asterisk.org/r/1645/diff/3/?file=22534#file22534line341>
> >
> >     This will only work if the write format is set to "SLINEAR", otherwise strange things may occur.
> >     
> >     Your best bet here is to first, detect if the write format on the channel is some form of signed linear.  If it is, then create a frame with the correct number of samples for the given length in time of the incoming frame (whatever format it is).
> >     
> >     so.. say the write format is slin32... and length of incoming frame is 40ms... the number of samples needed would be. 32000/(1000/40) = 1280 samples for 40ms of 32khz signed linear.
> >     
> >     The ast_codec_get_samples(), ast_format_rate(), and ast_format_is_slinear() functions will be of use here.
> >     
> >     If the channel's write format is not some form of signed linear, then just return a NULL frame instead of the audio as that is the best we can hope to do.

I thought bridging core did some automatic translations since bridge_softmix can write either slinear or rawwriteformat frames. E.g. if own voice is removed, it's writing slinear, otherwise it writes rawwriteformat via softmix_process_write_audio(). But apparently softmix just sets up the channels translation path so it'd accept both. Will add check to see if rawwriteformat, or the writeformat allows slinear.

However, I would expect the silent frame length to be always correct: my understanding is that frame->samples is the decompressed number of (slinear) samples it represents. Thus we can just use that to build up proper length frames regardless of sampling rate, or if the source frame was not slinear.


> On Jan. 3, 2012, 4:29 p.m., David Vossel wrote:
> > /trunk/main/bridging.c, lines 343-345
> > <https://reviewboard.asterisk.org/r/1645/diff/3/?file=22534#file22534line343>
> >
> >     Is a short always 16 bits?  Use of int_16 might bet better here.

I used 'short' as it's this part of the code is copy from main/channel.c:silence_generator_generate(). 'short' is guaranteed to be 16-bit, but int_16 is more explicit on that.

Would it make sense to merge the code with silence_generator_generate? They could share most of the code, and perhaps even use a global silence buffer too - we'd avoid per-call memset() and large allocations from stack.


- fabled


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1645/#review5098
-----------------------------------------------------------


On Jan. 3, 2012, 8:36 a.m., fabled wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1645/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2012, 8:36 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Support 'deaf' participants in ConfBridge
> 
> 
> This addresses bug ASTERISK-19109.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19109
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 349410 
>   /trunk/apps/app_confbridge.c 349410 
>   /trunk/apps/confbridge/conf_config_parser.c 349410 
>   /trunk/apps/confbridge/include/confbridge.h 349410 
>   /trunk/bridges/bridge_multiplexed.c 349410 
>   /trunk/bridges/bridge_simple.c 349410 
>   /trunk/bridges/bridge_softmix.c 349410 
>   /trunk/configs/confbridge.conf.sample 349410 
>   /trunk/include/asterisk/bridging_features.h 349410 
>   /trunk/include/asterisk/bridging_technology.h 349410 
>   /trunk/main/bridging.c 349410 
> 
> Diff: https://reviewboard.asterisk.org/r/1645/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> fabled
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120104/37172d13/attachment.htm>