[FFmpeg-devel] [PATCH 2/2][RFC] avcodec/g729: add g729_parser

Ganesh Ajjanagadde gajjanag at mit.edu
Tue Aug 11 19:05:40 CEST 2015


On Tue, Aug 11, 2015 at 12:00 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Tue, Aug 11, 2015 at 11:21:22AM -0400, Ganesh Ajjanagadde wrote:
>> On Tue, Aug 11, 2015 at 11:02 AM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Mon, Aug 10, 2015 at 09:51:43PM -0400, Ganesh Ajjanagadde wrote:
>> >> Add trivial g729 parser; fixes Ticket4753
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >> ---
>> >>  libavcodec/Makefile      |  1 +
>> >>  libavcodec/allcodecs.c   |  1 +
>> >>  libavcodec/g729.h        |  4 +++
>> >>  libavcodec/g729_parser.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  4 files changed, 96 insertions(+)
>> >>  create mode 100644 libavcodec/g729_parser.c
>> >>
>> >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> >> index e61b9cd..31b8ceb 100644
>> >> --- a/libavcodec/Makefile
>> >> +++ b/libavcodec/Makefile
>> >> @@ -847,6 +847,7 @@ OBJS-$(CONFIG_DVD_NAV_PARSER)          += dvd_nav_parser.o
>> >>  OBJS-$(CONFIG_DVDSUB_PARSER)           += dvdsub_parser.o
>> >>  OBJS-$(CONFIG_FLAC_PARSER)             += flac_parser.o flacdata.o flac.o \
>> >>                                            vorbis_data.o
>> >> +OBJS-$(CONFIG_G729_PARSER)             += g729_parser.o
>> >>  OBJS-$(CONFIG_GSM_PARSER)              += gsm_parser.o
>> >>  OBJS-$(CONFIG_H261_PARSER)             += h261_parser.o
>> >>  OBJS-$(CONFIG_H263_PARSER)             += h263_parser.o
>> >> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> >> index ed0975e..04a83d4 100644
>> >> --- a/libavcodec/allcodecs.c
>> >> +++ b/libavcodec/allcodecs.c
>> >> @@ -602,6 +602,7 @@ void avcodec_register_all(void)
>> >>      REGISTER_PARSER(DVDSUB,             dvdsub);
>> >>      REGISTER_PARSER(DVD_NAV,            dvd_nav);
>> >>      REGISTER_PARSER(FLAC,               flac);
>> >> +    REGISTER_PARSER(G729,               g729);
>> >>      REGISTER_PARSER(GSM,                gsm);
>> >>      REGISTER_PARSER(H261,               h261);
>> >>      REGISTER_PARSER(H263,               h263);
>> >> diff --git a/libavcodec/g729.h b/libavcodec/g729.h
>> >> index 6168313..7c5f693 100644
>> >> --- a/libavcodec/g729.h
>> >> +++ b/libavcodec/g729.h
>> >> @@ -26,4 +26,8 @@
>> >>   */
>> >>  #define SUBFRAME_SIZE 40
>> >>
>> >> +/* bytes per block */
>> >> +#define G729_8K_BLOCK_SIZE     10
>> >> +#define G729D_6K4_BLOCK_SIZE   8
>> >> +
>> >>  #endif // AVCODEC_G729_H
>> >> diff --git a/libavcodec/g729_parser.c b/libavcodec/g729_parser.c
>> >> new file mode 100644
>> >> index 0000000..203c787
>> >> --- /dev/null
>> >> +++ b/libavcodec/g729_parser.c
>> >> @@ -0,0 +1,90 @@
>> >> +/*
>> >> + * Copyright (c) 2015  Ganesh Ajjanagadde
>> >> + *
>> >> + * 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
>> >> + * G.729 audio parser
>> >> + *
>> >> + * Splits packets into individual blocks.
>> >> + */
>> >> +
>> >> +#include "parser.h"
>> >> +#include "g729.h"
>> >> +#include "g729dec.h"
>> >> +
>> >> +typedef struct G729ParseContext {
>> >> +    ParseContext pc;
>> >> +    int block_size;
>> >> +    int duration;
>> >> +    int remaining;
>> >> +} G729ParseContext;
>> >> +
>> >> +static int g729_parse(AVCodecParserContext *s1, AVCodecContext *avctx,
>> >> +                     const uint8_t **poutbuf, int *poutbuf_size,
>> >> +                     const uint8_t *buf, int buf_size)
>> >> +{
>> >> +    G729ParseContext *s = s1->priv_data;
>> >> +    ParseContext *pc = &s->pc;
>> >
>> >> +    G729Context *ctx = avctx->priv_data;
>> >
>> > this is not safe.
>> > theres no gurantee that there even is a decoder decoding the output
>> > from the parser, there might be none, or 2 or it might be a binary
>> > decoder using a different struct, an application can use anything
>> > or nothing after a parser
>>
>> I could move that into the switch block, and do this only after
>> confirming the CODEC_ID.
>> I do not think this resolves all of the above concerns though.
>> Any ideas as to what can be done?
>> Basic issue pointed out by Paul is that the parser really has no way of knowing
>> whether to split into 8 byte or 10 byte chunks.
>
> the decoder chooses the format based on the buf_size
> the buf_size is what the parser returns
> the parser uses the decoders format
>
> there is no way this can work, each just takes the information from
> the other but neither knows anything in the first place
> even if there was a decoder and even if it was safe to access its
> context from the parser.
>
> The first thing to do is to read any study the specification
> are there any fixed bits or invalid values in the bitstream
> if so they can possibly be used to identify the packet size by
> checking which size leads to packets with no invalid values or bits
> occuring over a long enough period
>
> one also could try to use the bitrate but this too has to be set by
> something (like the user for raw and some header from other containers)

Used this method, see updated patch.
Thanks.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list