[FFmpeg-devel] [PATCH 2/2] Adding closed caption decoder

Anshul anshul.ffmpeg at gmail.com
Sun Jan 4 15:51:51 CET 2015


On 01/03/2015 08:40 PM, Michael Niedermayer wrote:
> On Sat, Jan 03, 2015 at 12:57:04PM +0530, Anshul wrote:
>> On 01/03/2015 01:42 AM, Michael Niedermayer wrote:
>>> On Wed, Dec 31, 2014 at 07:09:33PM +0530, Anshul wrote:
> [..]
>>  Makefile       |    1 
>>  allcodecs.c    |    1 
>>  ccaption_dec.c |  361 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 363 insertions(+)
>> 54d4896ef8724994e1022eec6a9c79d0cddec29d  0001-Adding-Closed-caption-Support.patch
>> From 17a564409b84fc18293833cc3f2151792209bb8b Mon Sep 17 00:00:00 2001
>> From: Anshul Maheshwari <anshul.ffmpeg at gmail.com>
>> Date: Sat, 3 Jan 2015 12:40:35 +0530
>> Subject: [PATCH 1/2] Adding Closed caption Support
>>
>> Signed-off-by: Anshul Maheshwari <anshul.ffmpeg at gmail.com>
>>
>> To test Closed caption use following command
>> /ffmpeg -f lavfi -i "movie=/home/a141982112/test_videos/Starship_Troopers.vob[out0+subcc]" -map s some.srt
>> ---
>>  libavcodec/Makefile       |   1 +
>>  libavcodec/allcodecs.c    |   1 +
>>  libavcodec/ccaption_dec.c | 361 ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 363 insertions(+)
>>  create mode 100644 libavcodec/ccaption_dec.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 107661b..33051c4 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -173,6 +173,7 @@ OBJS-$(CONFIG_BRENDER_PIX_DECODER)     += brenderpix.o
>>  OBJS-$(CONFIG_C93_DECODER)             += c93.o
>>  OBJS-$(CONFIG_CAVS_DECODER)            += cavs.o cavsdec.o cavsdsp.o \
>>                                            cavsdata.o mpeg12data.o
>> +OBJS-$(CONFIG_CCAPTION_DECODER)        += ccaption_dec.o
>>  OBJS-$(CONFIG_CDGRAPHICS_DECODER)      += cdgraphics.o
>>  OBJS-$(CONFIG_CDXL_DECODER)            += cdxl.o
>>  OBJS-$(CONFIG_CINEPAK_DECODER)         += cinepak.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 8ceee2f..ef77dec 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -481,6 +481,7 @@ void avcodec_register_all(void)
>>      /* subtitles */
>>      REGISTER_ENCDEC (SSA,               ssa);
>>      REGISTER_ENCDEC (ASS,               ass);
>> +    REGISTER_DECODER(CCAPTION,          ccaption);
>>      REGISTER_ENCDEC (DVBSUB,            dvbsub);
>>      REGISTER_ENCDEC (DVDSUB,            dvdsub);
>>      REGISTER_DECODER(JACOSUB,           jacosub);
>> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
>> new file mode 100644
>> index 0000000..d351efe
>> --- /dev/null
>> +++ b/libavcodec/ccaption_dec.c
>> @@ -0,0 +1,361 @@
>> +/*
>> + * Closed Caption Decoding
>> + * Copyright (c) 2014 Anshul Maheshwari
>> + *
>> + * 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
>> + */
>> +
>> +#include "avcodec.h"
>> +#include "ass.h"
>> +#include "libavutil/opt.h"
>> +
>> +#undef CHAR_DEBUG
>> +#define SCREEN_ROWS 15
>> +#define SCREEN_COLUMNS 32
>> +
>> +#define SET_FLAG(var, val) ( var |= ( 1 << (val) ) )
>> +#define UNSET_FLAG(var, val) ( var &=  ~( 1 << (val)) )
>> +#define CHECK_FLAG(var, val) ( (var) & (1 << (val) ) )
>> +
>> +enum cc_mode {
>> +    CCMODE_POPON,
>> +    CCMODE_PAINTON,
>> +    CCMODE_ROLLUP_2,
>> +    CCMODE_ROLLUP_3,
>> +    CCMODE_ROLLUP_4,
>> +    CCMODE_TEXT,
>> +};
>> +
>> +struct Screen {
>> +    /* +1 is used to compensate null character of string */
>> +    uint8_t characters[SCREEN_ROWS][SCREEN_COLUMNS+1];
>> +    /*
>> +     * Bitmask of used rows; if a bit is not set, the
>> +     * corresponding row is not used.
>> +     * for setting row 1  use row | (0 << 1)
>> +     * for setting row 15 use row | (1 << 14)
>> +     */
>> +    int16_t  row_used;
> you can use an array here, this would simplify the code and also
> avoid the *_FLAG macros
>
>
to check whether any row is used or not, It will have for loop for 15 rows,
now row_used can be used directly in if and for loop to see whether any
row is used or not.

In ccextractor we use array for it, but its more complicated.
This version of closed caption decoder is not full fledged, we will need
to use row_used in many more commands.
>> +};
>> +
>> +
>> +typedef struct CCaptionSubContext {
>> +    AVClass *class;
>> +    int parity_table[256];
> this can be a static uint8_t table
>
I don't think static variable in structure are allowed in c language
that is cpp thing.

If you meant to remove that table from structure, then too its
not efficient, we have to make parity table every time decode
function is called.
>> +    int row_cnt;
>> +    struct Screen screen[2];
>> +    int active_screen;
>> +    uint8_t cursor_row;
>> +    uint8_t cursor_column;
>> +    AVBPrint buffer;
>> +    int erase_display_memory;
>> +    int rollup;
>> +    enum  cc_mode mode;
>> +    int64_t start_time;
>> +    /* visible screen time */
>> +    int64_t startv_time;
>> +    int64_t end_time;
>> +    char prev_cmd[2];
>> +    /* buffer to store pkt data */
>> +    AVBufferRef *pktbuf;
> as you memcopy the data each time, theres no need for a AVBufferRef
> a simple uint8_t * would do the same
> but i think not even that is needed,
> all uses of the data go through process_cc608() it would be
> very simply to strip one bit in the arguments there, so no rewriting
> of the table would be needed
>
> [...]
cant do that, for error resistance we need to escape 1st byte
if parity does not match, for escaping I write 0x7f instead of
whatever data is. Some closed caption insert-er don't care much for parity
when they are not using the data.
 
I was using AVBufferRef instead of uint8_t * , so that I don't have to
take care for length,
and length and data are in one context. and there is already lot of
error handling is
done while realloc, means I don't have to copy buffer pointer somewhere,
if realloc fails.
and in future if someone want to make data channel 1  and data channel 2
to be processed
in parallel,  then he may use reference thing too.
still its my opinion, if you think uint8_t will have better performance,
I will change it to that.

>> +static void handle_char(CCaptionSubContext *ctx, char hi, char lo, int64_t pts)
>> +{
>> +    struct Screen *screen = get_writing_screen(ctx);
>> +    char *row = screen->characters[ctx->cursor_row] + ctx->cursor_column;
>> +
>> +    SET_FLAG(screen->row_used,ctx->cursor_row);
>> +
>> +    *row++ = hi;
>> +    ctx->cursor_column++;
>> +    if(lo) {
>> +        *row++ = lo;
>> +        ctx->cursor_column++;
>> +    }
>> +    *row = 0;
> this code appears to lack validity checks on the column index
Added in todo list, will do it while implementing backspace.
>
>> +    /* reset prev command since character can repeat */
>> +    ctx->prev_cmd[0] = 0;
>> +    ctx->prev_cmd[1] = 0;
>> +#ifdef CHAR_DEBUG
>> +    av_log(ctx, AV_LOG_DEBUG,"(%c,%c)\n",hi,lo);
>> +#endif
>> +}
>> +static int process_cc608(CCaptionSubContext *ctx, int64_t pts, uint8_t hi, uint8_t lo)
>> +{
>> +    int ret = 0;
>> +#define COR3(var, with1, with2, with3)  ( (var) == (with1) ||  (var) == (with2) || (var) == (with3) )
>> +    if ( hi == ctx->prev_cmd[0] && lo == ctx->prev_cmd[1]) {
>> +    /* ignore redundant command */
>> +    } else if ( (hi == 0x10 && (lo >= 0x40 || lo <= 0x5f)) ||
>> +              ( (hi >= 0x11 && hi <= 0x17) && (lo >= 0x40 && lo <= 0x7f) ) ) {
>> +        handle_pac(ctx, hi, lo);
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x20 ) {
>> +    /* resume caption loading */
>> +        ctx->mode = CCMODE_POPON;
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x25 ) {
>> +        ctx->rollup = 2;
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x26 ) {
>> +        ctx->rollup = 3;
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x27 ) {
>> +        ctx->rollup = 4;
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x29 ) {
>> +    /* resume direct captioning */
>> +        ctx->mode = CCMODE_PAINTON;
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2C ) {
>> +    /* erase display memory */
>> +        ret = handle_edm(ctx, pts);
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2D ) {
>> +    /* carriage return */
>> +        ctx->row_cnt++;
>> +        if(ctx->row_cnt == ctx->rollup) {
>> +            ctx->row_cnt = 0;
> this check appears insufficient to prevent the index fro getting
> out of array
>
> rollup can be decresed below row_cnt if iam not missing something
>
done, changed to >=
>> +            ret = handle_edm(ctx, pts);
>> +            ctx->active_screen = !ctx->active_screen;
>> +        }
>> +    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2F ) {
>> +    /* end of caption */
>> +        ret = handle_eoc(ctx, pts);
>> +    } else if (hi>=0x20) {
>> +    /* Standard characters (always in pairs) */
>> +        handle_char(ctx, hi, lo, pts);
>> +    } else {
>> +    /* Ignoring all other non data code */
>> +    }
>> +
>> +    /* set prev command */
>> +     ctx->prev_cmd[0] = hi;
>> +     ctx->prev_cmd[1] = lo;
>> +
>> +#undef COR3
>> +    return ret;
>> +
>> +}
>> +static int decode(AVCodecContext *avctx, void *data, int *got_sub, AVPacket *avpkt)
>> +{
>> +    CCaptionSubContext *ctx = avctx->priv_data;
>> +    AVSubtitle *sub = data;
>> +    uint8_t *bptr = NULL;
>> +    int len = avpkt->size;
>> +    int ret = 0;
>> +    int i;
>> +
>> +    if ( ctx->pktbuf->size < len) {
>> +        ret = av_buffer_realloc(&ctx->pktbuf, len);
>> +        if(ret)
>> +            len = ctx->pktbuf->size;
>> +    }
> [...]
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I have attached  another patch set.

-Anshul Maheswari
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Adding-Closed-caption-Support.patch
Type: text/x-patch
Size: 13187 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150104/c4549bd7/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Adding-Closed-caption-accessories.patch
Type: text/x-patch
Size: 1634 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150104/c4549bd7/attachment-0001.bin>


More information about the ffmpeg-devel mailing list