[FFmpeg-devel] [PATCH] Add single stream LATM/LOAS decoder

Janne Grunau janne-ffmpeg
Mon Oct 18 15:59:47 CEST 2010


On Mon, Oct 18, 2010 at 02:56:36PM +0200, Michael Niedermayer wrote:
> On Sun, Oct 17, 2010 at 12:47:54PM +0200, Janne Grunau wrote:
> > The decoder is basicly just a wrapper around the AAC decoder.
> > based on patch by Paul Kendall { paul <?t> kcbbs gen nz }
> > ---
> >  Changelog                |    1 +
> >  configure                |    1 +
> >  libavcodec/Makefile      |    2 +
> >  libavcodec/aaclatmdec.c  |  438 ++++++++++++++++++++++++++++++++++++++++++++++
> >  libavcodec/allcodecs.c   |    2 +
> >  libavcodec/avcodec.h     |    3 +-
> >  libavcodec/latm_parser.c |  118 +++++++++++++
> >  7 files changed, 564 insertions(+), 1 deletions(-)
> >  create mode 100644 libavcodec/aaclatmdec.c
> >  create mode 100644 libavcodec/latm_parser.c
> > 
> > diff --git a/Changelog b/Changelog
> > index 76d6b8b..1abc19d 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -46,6 +46,7 @@ version <next>:
> >  - RTP depacketization of the X-QT QuickTime format
> >  - SAP (Session Announcement Protocol, RFC 2974) muxer and demuxer
> >  - cropdetect filter
> > +- single stream LATM/LOAS decoder
> >  
> >  
> >  version 0.6:
> > diff --git a/configure b/configure
> > index 0e6e439..f3e65d4 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1187,6 +1187,7 @@ rdft_select="fft"
> >  # decoders / encoders / hardware accelerators
> >  aac_decoder_select="mdct rdft"
> >  aac_encoder_select="mdct"
> > +aac_latm_decoder_select="aac_decoder"
> >  ac3_decoder_select="mdct ac3_parser"
> >  alac_encoder_select="lpc"
> >  amrnb_decoder_select="lsp"
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index 385ae02..bd5f041 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -52,6 +52,7 @@ OBJS-$(CONFIG_AAC_ENCODER)             += aacenc.o aaccoder.o    \
> >                                            aacpsy.o aactab.o      \
> >                                            psymodel.o iirfilter.o \
> >                                            mpeg4audio.o
> > +OBJS-$(CONFIG_AAC_LATM_DECODER)        += aaclatmdec.o
> >  OBJS-$(CONFIG_AASC_DECODER)            += aasc.o msrledec.o
> >  OBJS-$(CONFIG_AC3_DECODER)             += ac3dec.o ac3dec_data.o ac3.o
> >  OBJS-$(CONFIG_AC3_ENCODER)             += ac3enc.o ac3tab.o ac3.o
> > @@ -576,6 +577,7 @@ OBJS-$(CONFIG_H264_PARSER)             += h264_parser.o h264.o            \
> >                                            h264_loopfilter.o h264_cabac.o \
> >                                            h264_cavlc.o h264_ps.o \
> >                                            mpegvideo.o error_resilience.o
> > +OBJS-$(CONFIG_AAC_LATM_PARSER)         += latm_parser.o
> >  OBJS-$(CONFIG_MJPEG_PARSER)            += mjpeg_parser.o
> >  OBJS-$(CONFIG_MLP_PARSER)              += mlp_parser.o mlp.o
> >  OBJS-$(CONFIG_MPEG4VIDEO_PARSER)       += mpeg4video_parser.o h263.o \
> > diff --git a/libavcodec/aaclatmdec.c b/libavcodec/aaclatmdec.c
> > new file mode 100644
> > index 0000000..06f3a35
> > --- /dev/null
> > +++ b/libavcodec/aaclatmdec.c
> > @@ -0,0 +1,438 @@
> > +/*
> > + * AAC LATM decoder
> > + * Copyright (c) 2008-2010 Paul Kendall <paul at kcbbs.gen.nz>
> > + * Copyright (c) 2010      Janne Grunau <janne-ffmpeg at jannau.net>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +/**
> > + * @file
> > + * AAC LATM decoder
> > + * @author Paul Kendall <paul at kcbbs.gen.nz>
> > + * @author Janne Grunau <janne-ffmpeg at jannau.net>
> > + */
> > +
> > +/*
> > +    Note: This decoder filter is intended to decode LATM streams transferred
> > +    in MPEG transport streams which only contain one program.
> > +    To do a more complex LATM demuxing a separate LATM demuxer should be used.
> > +*/
> > +
> > +#include "get_bits.h"
> > +#include "dsputil.h"
> > +
> > +#include "aac.h"
> > +#include "aacdectab.h"
> > +#include "mpeg4audio.h"
> > +
> > +#include "libavutil/avassert.h"
> > +
> 
> > +#define LOAS_SYNC_WORD   0x2b7       // 11 bits
> 
> doxy
> 

done

> > +#define MAX_SIZE         8*1024
> > +
> > +struct LATMContext
> > +{
> > +    AACContext      aac_ctx;
> > +    AVCodec        *aac_codec;
> 
> > +    uint8_t         initialized;
> 
> why uint8_t ?

changed to int
 
> > +
> > +    // parser data
> > +    uint8_t         audio_mux_version_A;
> > +    uint8_t         same_time_framing;
> > +    uint8_t         frame_length_type;
> > +    uint32_t        frame_length;
> 
> maybe a bit of documentation would be a good idea

done

> > +};
> > +
> > +static inline int64_t latm_get_value(GetBitContext *b)
> > +{
> > +    uint8_t bytesForValue = get_bits(b, 2);
> 
> why uint8_t ?

chnaged to int
 
> > +    int64_t value = 0;
> 
> inst unsigned int enough?

yes, changed

> > +    int i;
> > +    for (i=0; i<=bytesForValue; i++) {
> > +        value <<= 8;
> > +        value |= get_bits(b, 8);
> > +    }
> > +    return value;
> > +}
> > +
> > +// copied from libavcodec/mpeg4audio.c
> > +static av_always_inline unsigned int copy_bits(PutBitContext *pb,
> > +                                               GetBitContext *gb, int bits)
> > +{
> > +    unsigned int el = get_bits(gb, bits);
> > +    put_bits(pb, bits, el);
> > +    return el;
> > +}
> > +
> 
> > +static void latm_read_ga_specific_config(int audio_object_type,
> > +                                         MPEG4AudioConfig *c,
> > +                                         GetBitContext *gb, PutBitContext *pb)
> > +{
> > +    int ext_flag;
> > +
> > +    copy_bits(pb, gb, 1);                        // framelen_flag
> > +    if (copy_bits(pb, gb, 1))                    // depends_on_coder
> > +        copy_bits(pb, gb, 14);                   // delay
> > +    ext_flag = copy_bits(pb, gb, 1);
> > +
> > +    if (!c->chan_config)
> > +        ff_copy_pce_data(pb, gb);                // program_config_element
> > +
> > +    if (audio_object_type == AOT_AAC_SCALABLE ||
> > +        audio_object_type == AOT_ER_AAC_SCALABLE)
> > +        copy_bits(pb, gb, 3);                    // layer number
> > +
> > +    if (!ext_flag)
> > +        return;
> > +
> > +    if (audio_object_type == AOT_ER_BSAC) {
> > +        copy_bits(pb, gb, 5);                    // numOfSubFrame
> > +        copy_bits(pb, gb, 11);                   // layer_length
> > +    } else if (audio_object_type == AOT_ER_AAC_LC ||
> > +               audio_object_type == AOT_ER_AAC_LTP ||
> > +               audio_object_type == AOT_ER_AAC_SCALABLE ||
> > +               audio_object_type == AOT_ER_AAC_LD)
> > +        copy_bits(pb, gb, 3);                    // stuff
> > +    copy_bits(pb, gb, 1);                        // extflag3
> > +}
> 
> isnt it easier to parse the whole normally (no copy) and then after parsing
> meassure how much was read and copy that amount ?

Yes, makes more sense, it was copied from the demuxer without rethinking
the approach.

> also it does not look like all uses of copy_bits are speed relevant as they
> write in extradata (a one time thing ...) thus marking it always_inline
> seems a bad idea

unfortunately not a one time thing. the multiplex repeats the config for
random access and nothing prevents it from changing. Not that I would
expect it to change regularly in real streams.

> > +static int latm_decode_audio_specific_config(struct LATMContext *latmctx,
> > +                                             GetBitContext *gb)
> > +{
> > +    PutBitContext pb;
> > +    int32_t esize, bits_consumed;
> > +    uint8_t extradata[32+FF_INPUT_BUFFER_PADDING_SIZE];
> > +    AVCodecContext *avctx = latmctx->aac_ctx.avctx;
> > +
> > +    init_put_bits(&pb, extradata, 32 * 8);
> > +
> > +    bits_consumed = latm_read_audio_specific_config(gb, &pb);
> 
> that feels like asking for a stack overflow (if one copies more in on of these
> functions), besides that, it can crash due
> to insufficent alignment on some cpus with some bit writers
> 
> and 32 should be a named constant and it should be documented how much each
> function is allowed to write.

changed to parse the config first and count consumed bits, av_malloc the
right amount of extradata and copy then.
 
> > +
> > +    if (bits_consumed < 0)
> > +        return AVERROR_INVALIDDATA;
> > +
> > +    esize = (bits_consumed+7) / 8;
> > +
> > +    if (avctx->extradata_size != esize) {
> > +        av_free(avctx->extradata);
> > +        avctx->extradata = av_malloc(esize + FF_INPUT_BUFFER_PADDING_SIZE);
> > +        if (!avctx->extradata)
> > +            return AVERROR(ENOMEM);
> > +
> > +        avctx->extradata_size = esize;
> > +        memcpy(avctx->extradata, extradata, esize);
> > +        memset(avctx->extradata+esize, 0, FF_INPUT_BUFFER_PADDING_SIZE);
> > +    }
> 
> it seems easier to alloc just once and write into this, the 15 extra bytes
> allocated surely are smaller than the 2 stage alloc + copy

yes, changed.

> > +static int latm_decode_frame(AVCodecContext *avctx, void *out, int *out_size,
> > +                             AVPacket *avpkt)
> > +{
> > +    struct LATMContext *latmctx = avctx->priv_data;
> > +    uint8_t            *tmp, tmpbuf[MAX_SIZE];
> > +    int                 ret, bufsize = MAX_SIZE;
> > +    GetBitContext       gb;
> > +
> > +    if(avpkt->size == 0)
> > +        return 0;
> > +
> > +    init_get_bits(&gb, avpkt->data, avpkt->size * 8);
> > +
> > +    ret = read_audio_sync_stream(latmctx, &gb, avpkt->size, tmpbuf, &bufsize);
> > +    if (ret < 0)
> > +        return ret;
> > +
> > +    if (!latmctx->initialized) {
> > +        if (!avctx->extradata) {
> > +            *out_size = 0;
> > +            return avpkt->size;
> > +        } else {
> 
> > +            av_assert0(latmctx->aac_codec->init);
> > +            ret = latmctx->aac_codec->init(avctx);
> 
> that assert is pointless, abort or null pointer exception is pretty much equally
> easy to debug

switched to using the functions from aacdec.c directly. exporting the
decoding function was required for avoiding aac bitstream copying. init
and close are exported too.

> > +            if (ret < 0)
> > +                return ret;
> > +            latmctx->initialized = 1;
> > +        }
> > +    }
> > +
> > +    tmp         = avpkt->data;
> > +    avpkt->data = tmpbuf;
> > +    avpkt->size = bufsize;
> > +
> > +    av_assert0(latmctx->aac_codec->decode);
> > +    ret = latmctx->aac_codec->decode(avctx, out, out_size, avpkt);
> > +    avpkt->data = tmp;
> > +    return ret;
> > +}
> > +
> 
> 
> > +static int latm_decode_init(AVCodecContext *avctx)
> > +{
> > +    struct LATMContext *latmctx = avctx->priv_data;
> > +    int ret;
> > +
> > +    latmctx->aac_codec = avcodec_find_decoder_by_name("aac");
> 
> that seems unneeded, the AVCodec can be accessed directly

see above

thanks for the review. I'll resend the patch later today.

Janne



More information about the ffmpeg-devel mailing list