[asterisk-bugs] [JIRA] (ASTERISK-21981) Pass-through support for Opus and VP8 formats

Wed Jul 10 04:35:04 CDT 2013

    [ https://issues.asterisk.org/jira/browse/ASTERISK-21981?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=207891#comment-207891 ] 

Lorenzo Miniero commented on ASTERISK-21981:
--------------------------------------------

Hi Matt,

about your points:

{quote}
1. I don't see a format_opus or a format_vp8 in the patch. Do those exist? If so, you have to svn add them in order for them to get included in the diff.
{quote}

There are no format files for either of them as of now. Those would apply to support for .opus and raw .vp8 files, wouldn't they? Opus files are basically .ogg files that contain Opus frames rather than Vorbis ones, and so to support them I guess the existing format_ogg_vorbis one could be a good starting point.

By the way, what do you mean by "svn add"? As of now I'm not aware of any SVN folder for this work, I just uploaded the patch.

{quote}
2. Remove WARNING messages that aren't warnings and commented out code. If things need a TODO that's fine, but commented out code without context as a tendency to create bugs. Example:
{quote}

If you mean the commented ast_rtcp_send_h261fur(p->vrtp) in the code, that was already there in the original chan_sip.c. I only added an if to send a RTCP FIR in the VP8 case, and do what was already there if not. I agree about the WARNING messages, I had them there only because they made it easier for me to debug.

{quote}
3. Parsing out Opus parameters in the SDP is fine, but WARNING on each one is probably not a good thing :-)
{quote}

Yes, you're right, as I said for the previous point it was just an easy way for me to immediately visualize what was happening and check I was doing the right thing. Considering SDP parsing should move somewhere else, as you suggested, they can probably be completely removed. 

{quote}
4. SDP attribute parsing for specific codecs actually shouldn't be done in chan_sip. In Asterisk 10 (and more so in Asterisk 11), we added the ability for specific resource modules to provide contextual parsing of SDP parameters to customize handling of codecs. This is done for H.263 in res_format_attr_h263, H.264 in res_format_attr_h264, SILK in res_format_attr_silk, etc. I'd imagine you would want to put this code in a res_format_attr_opus, then register the module as an attribute format handler/provider. The SDP parsing in chan_sip will automatically find and call the callbacks to parse the SDP and pass the information to a codec module. This also reduced code duplication in other channel drivers, as they don't have re-write SDP parsing or codec manipulation.
5. This also applies to SDP generation - a specific format module can add things to an SDP. For an example, see silk_sdp_generate in res_format_attr_silk.
{quote}

I didn't know about this, thanks for the explaination! I'll go through the files you mention and prepare a resource module for Opus. I guess the same won't be needed for VP8, since as you said the RTCP FIR negotiation shouldn't be VP8 specific.

{quote}
6. I am concerned about this:
{quote}

Yes, I guessed so. As I said, I don't think it will be needed for passthrough purposes, as from an RTP point of view, what Asterisk sees passing by are always 48 kHz frames, even when they're not. I needed something like this when transcoding, as the actual framerate was important there. 

{quote}
7. Sending a fast picture update over RTCP doesn't have to be limited to just VP8.
{quote}

Ok, then I guess we can negotiate the parameter via SDP and, in case our peer supports it, send a keyframe request using RTCP FIR instead of a SIP INFO. This could be the "configurable" you mentioned at the end of your coomment. Would such a negotiation need to be defined somewhere outside of chan_sip as well, or considering it would be codec agnostic can it be defined there instead?

> Pass-through support for Opus and VP8 formats
> ---------------------------------------------
>
>                 Key: ASTERISK-21981
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-21981
>             Project: Asterisk
>          Issue Type: New Feature
>      Security Level: None
>          Components: Formats/General
>            Reporter: Tzafrir Cohen
>         Attachments: asterisk_opus+vp8_passthrough.patch
>
>
> Following discussions on the asterisk-dev mailing list, I create a bug for Lorenzo Miniero to merge the parts of his WebRTC codecs patch that can be merged.
> See:
> http://lists.digium.com/pipermail/asterisk-dev/2013-May/060388.html
> http://lists.digium.com/pipermail/asterisk-dev/2013-May/060419.html
> (If this is a duplicate: sorry for the noise. I failed to find it)
> Including this patch should allow for a review in time before the Asterisk 12 deadline. I marked this as "major" as I think this is an important feature missing in Asterisk right now. Supporting the formats is the least we can do.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.asterisk.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira