[asterisk-dev] Manager event for early media

[Russell Bryant]

On 04/15/2011 08:04 AM, Olle E. Johansson wrote:
> Running GREP on the source code I realize that the AST_STATE_MUTE value is not used anywhere... Ouch.

> agave:asterisk-trunk olle$ grep AST_STATE_MUTE */*.c
> agave:asterisk-trunk olle$ grep AST_STATE_MUTE include/asterisk/*
> include/asterisk/channelstate.h:	AST_STATE_MUTE = (1 << 16),	/*!< Do not transmit voice data */
> agave:asterisk-trunk olle$ 
> 
> 
> So I guess we have an open field here. The first question is: Would PROGRESS really be a new state or just an attribute to a state? Are we in PROGRESS STATE as opposed to RINGING or is it just a variant of RINGING?
> 
> Just to give other examples: HOLD is not a channel state, it's an extension state. The call is still in UP state. 
> 
> If it's a new state, we need to make sure that all channel state changes in the code can handle the new state properly. 
> If it's an attribute, like MUTE, we need to make sure that every part of the code that handles ast_channel_state will not bother with these codes. AST_STATE_RINGING | AST_STATE_PROGRESS should be the same as RINGING for the parts of the code that doesn't care if we have early media or not.
> 
> MUTE should propably be removed from ast_channel_states since we have HOLD as an extension state.

res/snmp/agent.c reads it, but no code sets it.  It can be removed.  I
can't remember it ever being used.  I bet it has been there since the
dawn of (Asterisk) time and whoever knew what it was used for has since
forgotten.  :-)

There is a whole bunch of code that reads the state directly.  If any
flags were used, a bunch of stuff would break.  I think we should remove
AST_STATE_MUTE and not use any flags in that field.

Good point about call hold.  That could have been a channel state, I
guess.  I don't know what the reasoning is there ... I think the
introduction of HOLD was before my time.

Early media is a generic enough telephony concept that I do believe that
it makes sense to capture it on ast_channel.  I can't think of a better
place for it than channel state.

Regarding how to deal with code that wants to know if something is
ringing, you could hide the logic a bit ...

Change:

    if (chan->_state == AST_STATE_RINGING) {

    }

to:

    if (ast_state_ringing(chan)) {

    }

and implement it as something like:

    int ast_state_ringing(struct ast_channel *chan)
    {
        switch (chan->_state) {
        case AST_STATE_RINGING:
        case AST_STATE_EARLYMEDIA:
            return 1;
        default:
            return 0;
        }
    }

-- 
Russell Bryant
Digium, Inc.   |   Engineering Manager, Open Source Software
445 Jan Davis Drive NW    -     Huntsville, AL 35806  -  USA
www.digium.com  -=-  www.asterisk.org -=- blogs.asterisk.org