On Sat, 26 Jul 2014 15:06:14 +0100, Kieran Kunhya <kier...@obe.tv> wrote:
> ---
> libavcodec/opus.c | 41 +++++++++++++++----
> libavcodec/opus.h | 11 +++++
> libavcodec/opus_parser.c | 101
> ++++++++++++++++++++++++++++++++++++++++------
> libavcodec/opusdec.c | 4 +-
> libavformat/mpegts.c | 52 +++++++++++++++++++++++-
> 5 files changed, 186 insertions(+), 23 deletions(-)
>
> diff --git a/libavcodec/opus.c b/libavcodec/opus.c
> index 91021ce..e230597 100644
> --- a/libavcodec/opus.c
> +++ b/libavcodec/opus.c
> @@ -290,10 +290,6 @@ av_cold int ff_opus_parse_extradata(AVCodecContext
> *avctx,
> OpusContext *s)
> {
> static const uint8_t default_channel_map[2] = { 0, 1 };
> - uint8_t default_extradata[19] = {
> - 'O', 'p', 'u', 's', 'H', 'e', 'a', 'd',
> - 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> - };
>
> int (*channel_reorder)(int, int) = channel_reorder_unknown;
>
> @@ -308,9 +304,8 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx,
> "Multichannel configuration without extradata.\n");
> return AVERROR(EINVAL);
> }
> - default_extradata[9] = (avctx->channels == 1) ? 1 : 2;
> - extradata = default_extradata;
> - extradata_size = sizeof(default_extradata);
> + extradata = av_opus_default_extradata;
> + extradata_size = sizeof(av_opus_default_extradata);
> } else {
> extradata = avctx->extradata;
> extradata_size = avctx->extradata_size;
> @@ -330,7 +325,7 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx,
>
> avctx->delay = AV_RL16(extradata + 10);
>
> - channels = extradata[9];
> + channels = avctx->extradata ? extradata[9] : (avctx->channels == 1) ? 1
> : 2;
> if (!channels) {
> av_log(avctx, AV_LOG_ERROR, "Zero channel count specified in the
> extadata\n");
> return AVERROR_INVALIDDATA;
> @@ -426,3 +421,33 @@ av_cold int ff_opus_parse_extradata(AVCodecContext
> *avctx,
>
> return 0;
> }
> +
> +uint8_t *ff_parse_opus_ts_header(uint8_t *start, int *payload_len){
This function is now used only in the parser, so it doesn't need to live here.
> + uint8_t *buf = start + 1;
> + int start_trim_flag, end_trim_flag, control_extension_flag,
> control_extension_length, i;
> +
> + start_trim_flag = (buf[0] >> 4) & 1;
> + end_trim_flag = (buf[0] >> 3) & 1;
> + control_extension_flag = (buf[0] >> 2) & 1;
> + buf++;
> +
> + *payload_len = 0;
> + while (buf[0] == 0xff){
> + *payload_len += buf[0];
> + buf++;
> + }
> + *payload_len += buf[0];
> + buf++;
> +
> + if (start_trim_flag)
> + buf += 2;
> + if (end_trim_flag)
> + buf += 2;
> + if (control_extension_flag){
> + control_extension_length = buf[0];
> + for (i = 0; i < control_extension_length; i++)
> + buf++;
> + }
> +
> + return buf;
Unless I'm missing something, there are no overread checks here. This will crash
on truncated input.
> +}
> diff --git a/libavcodec/opus.h b/libavcodec/opus.h
> index c2fac06..16e5b39 100644
> --- a/libavcodec/opus.h
> +++ b/libavcodec/opus.h
> @@ -61,6 +61,15 @@
> #define ROUND_MUL16(a,b) ((MUL16(a, b) + 16384) >> 15)
> #define opus_ilog(i) (av_log2(i) + !!(i))
>
> +#define OPUS_TS_HEADER 0x7FE0 // 0x3ff (11 bits)
> +#define OPUS_TS_MASK 0x7FF0 // top 11 bits
> +
> +static const uint8_t av_opus_default_extradata[30] = {
Drop the av_ prefix, it's to be used only for public stuff
> + 'O', 'p', 'u', 's', 'H', 'e', 'a', 'd',
> + 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +};
> +
> enum OpusMode {
> OPUS_MODE_SILK,
> OPUS_MODE_HYBRID,
> @@ -383,6 +392,8 @@ int ff_opus_parse_packet(OpusPacket *pkt, const uint8_t
> *buf, int buf_size,
>
> int ff_opus_parse_extradata(AVCodecContext *avctx, OpusContext *s);
>
> +uint8_t *ff_parse_opus_ts_header(uint8_t *start, int *payload_len);
> +
> int ff_silk_init(AVCodecContext *avctx, SilkContext **ps, int
> output_channels);
> void ff_silk_free(SilkContext **ps);
> void ff_silk_flush(SilkContext *s);
> diff --git a/libavcodec/opus_parser.c b/libavcodec/opus_parser.c
> index 8a2bc22..356882c 100644
> --- a/libavcodec/opus_parser.c
> +++ b/libavcodec/opus_parser.c
> @@ -27,49 +27,124 @@
>
> #include "avcodec.h"
> #include "opus.h"
> +#include "parser.h"
> +#include "bytestream.h"
>
> typedef struct OpusParseContext {
> OpusContext ctx;
> OpusPacket pkt;
> int extradata_parsed;
> + ParseContext pc;
> + int ts_framing;
> } OpusParseContext;
>
> -static int opus_parse(AVCodecParserContext *ctx, AVCodecContext *avctx,
> - const uint8_t **poutbuf, int *poutbuf_size,
> - const uint8_t *buf, int buf_size)
> +/**
> + * Find the end of the current frame in the bitstream.
> + * @return the position of the first byte of the next frame, or -1
> + */
> +static int opus_find_frame_end(AVCodecParserContext *ctx, AVCodecContext
> *avctx,
> + const uint8_t *buf, int buf_size, int
> *header_len)
> {
> OpusParseContext *s = ctx->priv_data;
> - int ret;
> + ParseContext *pc = &s->pc;
> + int ret, pic_found, i = 0, payload_len = 0;
> + uint8_t *payload;
> + uint32_t state;
> + uint16_t hdr;
> + *header_len = 0;
>
> if (!buf_size)
> return 0;
>
> + pic_found = pc->frame_start_found;
'pic' is a bit strange when talking about audio
> + state = pc->state;
> + payload = buf;
> +
> + /* Check if we're using Opus in MPEG-TS framing */
> + if (!s->ts_framing){
> + hdr = AV_RB16(buf);
This is also missing a buffer size check
> + if((hdr & OPUS_TS_MASK) == OPUS_TS_HEADER)
> + s->ts_framing = 1;
> + }
> +
> + if (s->ts_framing && !pic_found){
> + for (i = 0; i < buf_size-2; i++){
> + state = (state << 8) | payload[i];
> + if((state & OPUS_TS_MASK) == OPUS_TS_HEADER){
> + payload = ff_parse_opus_ts_header(payload, &payload_len);
>
> + *header_len = payload - buf;
> + pic_found = 1;
> + break;
> + }
> + }
> + }
> +
> + if(!s->ts_framing)
> + payload_len = buf_size;
> +
> if (avctx->extradata && !s->extradata_parsed) {
> ret = ff_opus_parse_extradata(avctx, &s->ctx);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR, "Error parsing Ogg extradata.\n");
> - goto fail;
> + return AVERROR_INVALIDDATA;
> }
> av_freep(&s->ctx.channel_maps);
> s->extradata_parsed = 1;
> }
>
> - ret = ff_opus_parse_packet(&s->pkt, buf, buf_size, s->ctx.nb_streams >
> 1);
> - if (ret < 0) {
> - av_log(avctx, AV_LOG_ERROR, "Error parsing Opus packet header.\n");
> - goto fail;
> + if (payload_len <= buf_size){
> + s->ctx.nb_streams = 1;
> + ret = ff_opus_parse_packet(&s->pkt, payload, payload_len,
> s->ctx.nb_streams > 1);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error parsing Opus packet
> header.\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + ctx->duration = s->pkt.frame_count * s->pkt.frame_duration;
> }
>
> - ctx->duration = s->pkt.frame_count * s->pkt.frame_duration;
> + if (s->ts_framing){
> + if(pic_found){
> + if (payload_len + *header_len <= buf_size){
> + pc->frame_start_found = 0;
> + pc->state = -1;
> + return payload_len + *header_len;
> + }
> + }
> +
> + pc->frame_start_found = pic_found;
> + pc->state = state;
> + return END_NOT_FOUND;
> + }
>
> -fail:
> - *poutbuf = buf;
> - *poutbuf_size = buf_size;
> return buf_size;
> }
>
> +static int opus_parse(AVCodecParserContext *ctx, AVCodecContext *avctx,
> + const uint8_t **poutbuf, int *poutbuf_size,
> + const uint8_t *buf, int buf_size)
> +{
> + OpusParseContext *s = ctx->priv_data;
> + ParseContext *pc = &s->pc;
> + int next, header_len;
> +
> + next = opus_find_frame_end(ctx, avctx, buf, buf_size, &header_len);
> +
> + if (s->ts_framing && next != AVERROR_INVALIDDATA &&
> + ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
> + *poutbuf = NULL;
> + *poutbuf_size = 0;
> + return buf_size;
> + }
> +
> + *poutbuf = buf + header_len;
> + *poutbuf_size = buf_size - header_len;
> + return next;
> +}
> +
> AVCodecParser ff_opus_parser = {
> .codec_ids = { AV_CODEC_ID_OPUS },
> .priv_data_size = sizeof(OpusParseContext),
> .parser_parse = opus_parse,
> + .parser_close = ff_parse_close
> };
> diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c
> index bf3a54b..5a5858b 100644
> --- a/libavcodec/opusdec.c
> +++ b/libavcodec/opusdec.c
> @@ -452,10 +452,11 @@ static int opus_decode_packet(AVCodecContext *avctx,
> void *data,
> int coded_samples = 0;
> int decoded_samples = 0;
> int i, ret;
> + OpusPacket *pkt;
>
> /* decode the header of the first sub-packet to find out the sample
> count */
> if (buf) {
> - OpusPacket *pkt = &c->streams[0].packet;
> + pkt = &c->streams[0].packet;
> ret = ff_opus_parse_packet(pkt, buf, buf_size, c->nb_streams > 1);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR, "Error parsing the packet
> header.\n");
> @@ -505,6 +506,7 @@ static int opus_decode_packet(AVCodecContext *avctx, void
> *data,
>
> ret = opus_decode_subpacket(&c->streams[i], buf,
> s->packet.data_size, coded_samples);
> +
> if (ret < 0)
> return ret;
> if (decoded_samples && ret != decoded_samples) {
The changes in this file look like artifacts from an older version
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index fb58e1f..655e3d9 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -28,6 +28,7 @@
> #include "libavutil/opt.h"
> #include "libavcodec/bytestream.h"
> #include "libavcodec/get_bits.h"
> +#include "libavcodec/opus.h"
> #include "avformat.h"
> #include "mpegts.h"
> #include "internal.h"
> @@ -595,6 +596,7 @@ static const StreamType REGD_types[] = {
> { MKTAG('D', 'T', 'S', '3'), AVMEDIA_TYPE_AUDIO, AV_CODEC_ID_DTS },
> { MKTAG('H', 'E', 'V', 'C'), AVMEDIA_TYPE_VIDEO, AV_CODEC_ID_HEVC },
> { MKTAG('V', 'C', '-', '1'), AVMEDIA_TYPE_VIDEO, AV_CODEC_ID_VC1 },
> + { MKTAG('O', 'p', 'u', 's'), AVMEDIA_TYPE_AUDIO, AV_CODEC_ID_OPUS },
> { 0 },
> };
>
> @@ -1296,15 +1298,31 @@ static void m4sl_cb(MpegTSFilter *filter, const
> uint8_t *section,
> av_free(mp4_descr[i].dec_config_descr);
> }
>
> +static const uint8_t opus_coupled_stream_cnt[9] = {
> + 1, 0, 1, 1, 2, 2, 2, 3, 3
> +};
> +
> +static const uint8_t opus_channel_map[8][8] = {
> + { 0 },
> + { 0,1 },
> + { 0,2,1 },
> + { 0,1,2,3 },
> + { 0,4,1,2,3 },
> + { 0,4,1,2,3,5 },
> + { 0,4,1,2,3,5,6 },
> + { 0,6,1,2,3,4,5,7 },
> +};
> +
> int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int
> stream_type,
> const uint8_t **pp, const uint8_t
> *desc_list_end,
> Mp4Descr *mp4_descr, int mp4_descr_count, int
> pid,
> MpegTSContext *ts)
> {
> const uint8_t *desc_end;
> - int desc_len, desc_tag, desc_es_id;
> + int desc_len, desc_tag, desc_es_id, ext_desc_tag, channels;
> char language[252];
> int i;
> + uint8_t channel_config_code;
>
> desc_tag = get8(pp, desc_list_end);
> if (desc_tag < 0)
> @@ -1423,6 +1441,38 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
> AVStream *st, int stream_type
> if (st->codec->codec_id == AV_CODEC_ID_NONE)
> mpegts_find_stream_type(st, st->codec->codec_tag, REGD_types);
> break;
> + case 0x7f: /* DVB extension descriptor */
> + ext_desc_tag = bytestream_get_byte(pp);
Those reads from pp also look unchecked
> + if (st->codec->codec_id == AV_CODEC_ID_OPUS &&
> + ext_desc_tag == 0x80){ /* User defined (provisional Opus) */
> + if (st->codec->extradata)
No need for if, av_free() works fine on NULL
> + av_freep(&st->codec->extradata);
> +
> + st->codec->extradata =
> av_malloc(sizeof(av_opus_default_extradata)
> + + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (st->codec->extradata) {
Seems to me you should instead return an error if malloc fails here
> + st->codec->extradata_size =
> sizeof(av_opus_default_extradata);
> + memcpy(st->codec->extradata, av_opus_default_extradata,
> sizeof(av_opus_default_extradata));
> + }
> + channel_config_code = bytestream_get_byte(pp);
> + if (channel_config_code <= 0x8){
> + st->codec->extradata[9] = channels = channel_config_code ?
> channel_config_code : 2;
> + st->codec->extradata[18] = channels > 2;
> + st->codec->extradata[19] = channel_config_code;
> + if (channel_config_code == 0){ /* Dual Mono */
> + st->codec->extradata[18] = 255; /* Mapping */
> + st->codec->extradata[19] = 2; /* Stream Count */
> + }
> + st->codec->extradata[20] =
> opus_coupled_stream_cnt[channel_config_code];
> + memcpy(&st->codec->extradata[21],
> opus_channel_map[channels-1], channels);
> + } else {
> + avpriv_request_sample(fc, "opus in ts - channel_config_code
> > 0x8");
> + }
> + st->codec->sample_rate = 48000;
> + st->codec->sample_fmt = AV_SAMPLE_FMT_FLTP;
Don't set the sample format, it's not for the demuxer to decide.
The decoder can very well be int.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
