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

[Ganesh Ajjanagadde]

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.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The real ebay dictionary, page 2
> "100% positive feedback" - "All either got their money back or didnt complain"
> "Best seller ever, very honest" - "Seller refunded buyer after failed scam"
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>