[Freeswitch-dev] mod_portaudio multiple endpoints support

[Moises Silva]

Hello Mitch,

Thanks for taking your time to review this feature and share your thoughts.

On Thu, May 26, 2011 at 1:37 PM, Mitch Capper <mitch.capper at gmail.com>wrote:

> Hi Moy,
> First of all I wanted to thank you and Comrex for the new improvements
> and code you have done I think a lot of people will find these
> features very useful!
>

You're certainly welcomed, it is nice to work with companies like Comrex
that realize is better to contribute and be part of the community.


> Thanks for clarifying some things on IRC.   The changes look good my
> main concerns with the current code are how well it reacts to dynamic
> changes (usb devices plugged or unplugged)  and the fact all streams
> and endpoints have to be specified in the xml config itself.
>

Where else should be specified if not in the XML config? doing it
dynamically would have created extra complexity and I did not feel added
much value for the current user.


> I am not sure if there is a large reason to have endpoints that are
> defined separately from streams.  An endpoint is just a name, a the
> indev, the outdev, and the channel for each.  Streams already define
> an indev and outdev which makes things a bit confusing.   If channel
> selection was moved to the stream definition it could all be defined
> there.    The only downside to this that I see is when you have an
> audio device with multiple channels you are going to end up
> duplicating the stream section partially for each but I am not sure if
> that is enough to warrant to require the double configuration of an
> audio device before you can start using it.   I would say that if the
> device selection was expanded so that rather than just a device ID
> /name to also include the channel I think it would be an improved
> interface.
>

Well, you have also the sampling rate and IO sizes (codec-ms) which belong
to the stream. The main reason for the separation is that they are meant to
be used separate. You can define 10 different endpoints using the same
stream. As long as they don't conflict in the channel they use and the
direction. You can define an endpoint that only uses the input of a given
stream and channel, and does not use the output, and another endpoint doing
the opposite, both at the same time having a call using the same stream.


> Short of that I think before it is merged into trunk the changes
> should be merged in a bit better with the existing commands and the
> existing stream code should be merged with the shared stream code as
> right now some functionality is being duplicated.   While the way the
> changes were made looks like relatively few new code paths are
> followed if you don't use the new shared streams/endpoints, it makes
> for a confusing interface for someone trying to figure out how things
> work.
>

The decision here had to do mostly with backwards compatibility and the
amount of things I'd most likely break if I touched the original code paths
and the amount of testing required to make sure the previous functionality
wasn't affected. I'd rather slowly start moving chunks from the old code
compatible with the new one as the need arises.


> My suggested list of changes would be:
> -Merge the existing stream code with the new stream code, this should
> eliminate some of the redundancies and allow the new code to be used
> everywhere, streams would be expanded to have the naming, channels,
> rates, and other features that shared streams allow
>

I don't think I'd be able to do that at this point.


> -Ability to create streams through the API (with the merge of the
> above code its just expanding the API to take the additional params)
> -Remove endpoints and allow bridging directly to a stream by using the
> stream's name
>

For the reasons I explained before (the same stream being used by multiple
endpoints), I can't see how this would work.


> -Do not validate streams until you go to use them, right now if a
> device is not ready when the xml config is read you can't use it


I like failing early, is easier to spot mistakes in the configuration. I
understand this might be a problem if you're planing to plug/unplug USB
devices, but that is not our case yet :-)


> -pa answer, pa call, pa play  are expanded to optionally take the
> streamname to act on
> -pa switchstream takes the call to switch the stream on
> -pa switch this is a tougher one but would need to figure something else
> out


pa answer, pa call are replaced by uuid_outgoing_answer, originate from the
command line respectively.


> I think it will probably be a decent bit of work to do these things
> but should allow for the great new features you have added without
> causing confusion for other users.


The current state of things is exactly to avoid confusing existing users.
They can go ahead and keep being happy with their current monodevice
configuration and usage. If, and only if they want to move to multi devices
they will have to learn something new. I found too complicated expanding the
current API's to support the new architecture without changing the usage of
the commands.


> By all means I am not asking you
> do this work any time someone contributes code back to FS its great
> and something much appreciated and certainly others can work to fully
> integrate it into freeswitch.  I figured I would just give some other
> feedback about it as you asked for comments.


Your input is very much appreciated. Although I can certainly tell you,
given that my own needs and Comrex needs are completely fulfilled with the
current feature set, I am looking mostly for bugs or small improvements.
Most the things you described make the interface nicer and consistent, but
will require a reasonable amount of work and a high chance of breaking
existing users (either monodevice users or Comrex themselves).

Once the code is merged is easier to start improving one thing at the time.

-
Moy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.freeswitch.org/pipermail/freeswitch-dev/attachments/20110526/abf7cbe8/attachment-0001.html