On 06/17/2012 11:03 AM, Martin Storsjö wrote:
> The library is 3-clause BSD licensed.
> ---
> Changelog | 1 +
> configure | 6 ++
> doc/general.texi | 10 +++
> libavcodec/Makefile | 2 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/libilbc.c | 194
> ++++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/utils.c | 5 ++
> 8 files changed, 220 insertions(+)
> create mode 100644 libavcodec/libilbc.c
>
> This still lacks a minor bump, I'll add it once the patchset is finalized.
[...]
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -85,6 +85,14 @@ x264 is under the GNU Public License Version 2 or later
> details), you must upgrade Libav's license to GPL in order to use it.
> @end float
>
> +@section libilbc
> +
> +Libav can make use of the libilbc library for iLBC encoding and decoding.
> +
> +Go to @url{https://github.com/dekkers/libilbc} and follow the instructions
> for
> +installing the library. Then pass @code{--enable-libilbc} to configure to
> +enable it.
Maybe also add some introductory info like: "iLBC is a narrowband speech
codec that has been made freely available by Google as part of the
WebRTC project. libilbc is a packaging friendly copy of the iLBC codec."
[...]
> +#include "avcodec.h"
> +#include "libavutil/opt.h"
> +#include "internal.h"
> +#include <ilbc.h>
put the system header before libav headers
[...]
> +static av_cold int ilbc_decode_close(AVCodecContext *avctx)
> +{
> + return 0;
> +}
This function is not required.
> +static int ilbc_decode_frame(AVCodecContext *avctx, void *data,
> + int *got_frame_ptr, AVPacket *avpkt)
> +{
> + const uint8_t *buf = avpkt->data;
> + int buf_size = avpkt->size;
> + ILBCDecContext *s = avctx->priv_data;
align the =
> + int ret;
> +
> + s->frame.nb_samples = s->decoder.blockl;
> + if ((ret = avctx->get_buffer(avctx, &s->frame)) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> + return ret;
> + }
> +
> + if (s->decoder.no_of_bytes > buf_size) {
> + av_log(avctx, AV_LOG_ERROR, "iLBC frame too short (%u, should be
> %u)\n",
> + buf_size, s->decoder.no_of_bytes);
> + return AVERROR_INVALIDDATA;
> + }
I would put the buf_size check before get_buffer().
> +
> + WebRtcIlbcfix_DecodeImpl((WebRtc_Word16*) s->frame.data[0], (const
> WebRtc_UWord16*) buf, &s->decoder, 1);
> +
break the long line.
can this return an error?
[...]
> + if (avctx->sample_rate != 8000) {
> + av_log(avctx, AV_LOG_ERROR, "Only 8000Hz sample rate supported\n");
> + return AVERROR(ENOSYS);
> + }
> +
> + if (avctx->channels != 1) {
> + av_log(avctx, AV_LOG_ERROR, "Only mono supported\n");
> + return AVERROR(ENOSYS);
> + }
why ENOSYS? can iLBC ever be anything other than mono 8kHz? if not,
those should be EINVAL.
> +
> + if ((mode = get_mode(avctx)) < 0)
> + return AVERROR(EINVAL);
Having a private option to set 20ms or 30ms mode could be useful rather
than having to know the corresponding bitrate.
Thanks,
Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
