[asterisk-dev] Opus and VP8

Lorenzo Miniero lminiero at gmail.com
Wed Jul 3 06:04:54 CDT 2013


2013/7/1 Matthew Jordan <mjordan at digium.com>

>
> On Mon, Jul 1, 2013 at 8:20 AM, Lorenzo Miniero <lminiero at gmail.com>wrote:
>
>> 2013/7/1 Tzafrir Cohen <tzafrir.cohen at xorcom.com>
>>
>>> On Mon, May 27, 2013 at 09:35:21PM +0200, Lorenzo Miniero wrote:
>>> > 2013/5/27 Tzafrir Cohen <tzafrir.cohen at xorcom.com>
>>> >
>>> > > On Mon, May 27, 2013 at 04:09:08PM +0200, Lorenzo Miniero wrote:
>>> > > > Dear all,
>>> > > >
>>> > > > I've just published the patch on github:
>>> > > >
>>> > > > https://github.com/meetecho/asterisk-opus
>>> > > >
>>> > > > The README should be quite self explainatory, but if you need any
>>> > > > additional info feel free to ask me.
>>> > > > Any feedback will be more than welcome!
>>> > >
>>> > > $ diffstat asterisk-opus/asterisk_opus+vp8.diff
>>> > >  build_tools/menuselect-deps.in |    1
>>> > >  channels/chan_sip.c            |   91 ++++++-
>>> > >  codecs/codec_opus.c            |  529
>>> > > +++++++++++++++++++++++++++++++++++++++++
>>> > >  codecs/ex_opus.h               |   35 ++
>>> > >  configure.ac                   |    3
>>> > >  formats/format_vp8.c           |  195 +++++++++++++++
>>> > >  include/asterisk/format.h      |    4
>>> > >  main/channel.c                 |    2
>>> > >  main/format.c                  |   16 +
>>> > >  main/frame.c                   |   38 ++
>>> > >  main/rtp_engine.c              |    6
>>> > >  makeopts.in                    |    3
>>> > >  res/res_rtp_asterisk.c         |   42 +++
>>> > >  13 files changed, 960 insertions(+), 5 deletions(-)
>>> > >
>>> > > Any problem with including the format parts of this patch into
>>> Asterisk?
>>> > > Asterisk has limited support for H.264 video. I don't suppose
>>> Asterisk
>>> > > comes with a license for a H.264 video playback (let alone encoding).
>>> > >
>>> > > At first glance, the following parts of the patch seem to be related
>>> to
>>> > > formats, rather than codecs:
>>> > >
>>> > >  channels/chan_sip.c            |   91 ++++++-
>>> > >  formats/format_vp8.c           |  195 +++++++++++++++
>>> > >  include/asterisk/format.h      |    4
>>> > >  main/channel.c                 |    2
>>> > >  main/format.c                  |   16 +
>>> > >  main/rtp_engine.c              |    6
>>> > >  res/res_rtp_asterisk.c         |   42 +++
>>> > >
>>> > > Would a patch / review of those parts by Lorenzo be welcomed?
>>> > >
>>> > >
>>> >
>>> > Actually all the files have stuff related to both codecs: so the code
>>> > related to Opus should be stripped from chan_sip, format, rtp_engine,
>>> etc.
>>> > first in order to have them refer to VP8 only. The format_vp8.c file
>>> itself
>>> > is pretty much a clone of the H.264 one, so if this one's ok I guess
>>> that
>>> > one will be too.
>>>
>>> In case I was not clear enough: I believe (and I suppose Matt's answer
>>> clarified it) that those parts of the patch can be merged into Asterisk
>>> as there's no reasonable fear of abusing any patents with them.
>>>
>>> If you agree with me, please follow up with the bug I opened:
>>> https://issues.asterisk.org/jira/browse/ASTERISK-21981
>>> and attach the partial patch.
>>>
>>> The nice thing about it is that it includes almost all of the patching
>>> needed to actual Asterisk code (besides the changes to frame.c . I
>>> wonder if those are actually needed). Apart from that you only have
>>> codec modules and changes to the build system required to build them.
>>>
>>> (if this bug is a duplicate: sorry for the noise. I didn't see any
>>> progress here)
>>>
>>> If this is to be merged before the Asterisk 12 freeze, we don't have
>>> much time.
>>>
>>>
>>
>> The code in frame.c is only needed if Asterisk needs to be aware of how
>> many samples an Opus RTP packet actually includes. Considering the patch
>> would be targeted to only add passthrough support for the codec, I guess
>> the changes in frame.c can be safely ignored.
>>
>> I think format_vp8.c can be removed as well: in fact, VP8 passthrough
>> support does not strictly require that file, which is only needed in case
>> you also want to provide ability to read/write VP8 files (e.g., for
>> announcements), something that code doesn't currently provide anyway (it's
>> just a copy of format_h264.c acting as a placeholder).
>>
>> I'm out of the lab today so I won't be able to upload the partial patch
>> until tomorrow, though, is it OK anyway considering the Asterisk 12 freeze
>> deadline you mentioned?
>>
>>
> I'd put up for review all parts of the code that do not perform
> transcoding. If additional work in frame.c is needed to know how many
> samples *any* RTP packet contains, then I'd include that as well (the fact
> that some codecs support varying sample rates is not strictly unique to any
> one codec). The same could be true for some portion of Asterisk that reacts
> to changes in sample rates. Infrastructure support is entirely appropriate
> for Asterisk 12; the easier it makes it for someone to potentially add
> future enhancements to Asterisk the better.
>
> I'd consider limited support of pass-through for VP8 to be a candidate as
> well.
>
> When you have those portions ready, attach them to the issue Tzafrir has
> made. Feature freeze for Asterisk 12 is July 31st, but all that means is
> that the patch has to be on the issue and posted to Review Board. We can
> help get that second step going once it is on the issue.
>
> Thanks,
>
> Matt
>
>

I prepared the patch but it seems I can't upload it yet. I had to sign the
agreement again, even though I thought I already had it done a long time
ago (but this was before Jira, so something may have changed in the
meanwhile). As soon as I'm accepted I'll post the patch on the issue page.

Lorenzo


> --
> Matthew Jordan
> Digium, Inc. | Engineering Manager
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at: http://digium.com & http://asterisk.org
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130703/ee685c85/attachment.htm>


More information about the asterisk-dev mailing list