-
Notifications
You must be signed in to change notification settings - Fork 1.1k
func_channel: Allow manually changing the audio format during a call. #1504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
cherry-pick-to: 23 |
} | ||
|
||
ast_channel_lock(chan); | ||
ast_channel_nativeformats_set(chan, cap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nativeformats alongside the raw format variants are between the channel driver implementation and the core. They are NOT something to be set arbitrarily like this in the CHANNEL function for all channels.
The non-read and write formats are fine to set as they are between what is reading frames from the channel or writing frames, and the core.
Something for Local channels specifically would be fine, because Asterisk itself has full control and they are internal. The raw variants would also need to be set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started with that, but this didn't work for me until I also set the native formats. Open to a workaround for that though, this is just what worked for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I should clarify I have been using this on Local channels only so far
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well for non-Local channels setting nativeformats is an absolute no.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would make more sense to skip certain logic for non-Local channels, or just restrict the capability entirely to Local channels?
Based on what you've said, I'm not sure how much sense it makes for non-Locals, and although I don't have a use case for non-Locals, I wonder if there are possible use cases someone might have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and Local channels complicate things because there can be two of them, and they may expect that each one matches the other so internally within the Local/unreal implementation frames can pass between them without transcoding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that helps a little. I try to visualize how the frames are flowing and where the transcoding is but with Local channels and multiple formats involved it gets a little complicated.
For my particular use case, the way the code is now seems to be the only way it will work. If you think the raw formats should be set, maybe it makes sense to break this out into multiple set operations using the channel function, since it sounds like depending on use case the user may or may not want to set the raw formats, just the native and non-raw. It seems I need to leave the raw alone.
I wanted to put it in one function call though because I know the three different formats are confusing and requiring the user to choose which ones get set and which don't will probably confuse people even more, but maybe that's the price of the flexibility needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's "working" because by not touching raw you are ensuring that the formats that get passed around inside Local/unreal stuff remains what is expected, however the raw formats are derived from native formats. So if nativeformats is set to g722, but this option sets the raw ones to ulaw - you've now fundamentally violated the contract though it just happens to work for your usage.
So I guess the question is... why are you setting native formats?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess the question is... why are you setting native formats?
Because it didn't actually work (do anything) unless I did.
If I just set the non-raw formats, reading the audio format using CHANNEL, it said ulaw instead of what it was before (lpc10), but the audio sent to the SIP trunk from the channel sounded LPC. (This is a good example, since you can easily hear if something sounds "wrong" somewhere.). This audio in question is audio being generated on the local channel itself.
Setting native formats was sufficient to ensure the audio on the channel that reached the SIP trunk was truly ulaw and not transcoded after it was already too late, i.e. I believe what this did is force the transcoding to happen "earlier" in the channel sequence going towards the SIP trunk, ensuring audio generated on the channel was post-transcoding and already native ulaw at that point. Not sure if that's correct, it's just what it appeared from observation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really sounds as if you've just found a combination that violates the contract but stuff compensates/is broken in such a way that it works for you and what you want.
This needs to go back to the drawing board to understand the full flow and interaction for Local channels while respecting the contract of nativeformats and raw formats.
Workflow PRCheck completed successfully |
Currently, the APIs to change channel formats during a call are not exposed, but it may be necessary to manually change the format for certain things to work, so add the ability to change the format. Normally, this should not be necessary as formats are transcoded as needed, but it may be necessary to control when this happens. As an example: IAX [lpc] <-> Local [lpc] <-> Local [lpc] <-> PJSIP [ulaw] While channel drivers will transcode automatically on the last Local -> PJSIP hop in this scenario, this may be too late if audio from the Local channel itself needs to be in ulaw (and normally, there is no reason to transcode between Local channels). By setting CHANNEL(audionativeformat)=ulaw in the appropriate place, it becomes: IAX [lpc] <-> Local [lpc] <-> Local [ulaw] <-> PJSIP [ulaw] The transcoding happens automatically in the desired place. Resolves: asterisk#1503 UserNote: Writing CHANNEL(audionativeformat) can now manually set the channel format during a call if needed.
cbadf83
to
ffe490c
Compare
Workflow PRCheck completed successfully |
Currently, the APIs to change channel formats during a call are not exposed, but it may be necessary to manually change the format for certain things to work, so add the ability to change the format.
Normally, this should not be necessary as formats are transcoded as needed, but it may be necessary to control when this happens.
As an example:
IAX [lpc] <-> Local [lpc] <-> Local [lpc] <-> PJSIP [ulaw]
While channel drivers will transcode automatically on the last Local -> PJSIP hop in this scenario, this may be too late if audio from the Local channel itself needs to be in ulaw (and normally, there is no reason to transcode between Local channels). By setting CHANNEL(audionativeformat)=ulaw in the appropriate place, it becomes:
IAX [lpc] <-> Local [lpc] <-> Local [ulaw] <-> PJSIP [ulaw]
The transcoding happens automatically in the desired place.
Resolves: #1503
UserNote: Writing CHANNEL(audionativeformat) can now manually set the channel format during a call if needed.