[FFmpeg-devel] [PATCH 3/3] libavcodec/ccaption_dec: rewrite packet handler as case statement; remove COR3 macro

[Clément Bœsch]

On Mon, Jan 04, 2016 at 07:28:03PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman at tmm1.net>
> 
> ---
>  libavcodec/ccaption_dec.c | 92 +++++++++++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index 96f0ccf..788e96a 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -453,54 +453,69 @@ static void handle_char(CCaptionSubContext *ctx, char hi, char lo, int64_t pts)
>  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 */
> +        /* 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 ( ( hi == 0x11 && lo >= 0x20 && lo <= 0x2f ) ||
>                  ( hi == 0x17 && lo >= 0x2e && lo <= 0x2f) ) {
>          handle_textattr(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 == 0x24 ) {
> -        handle_delete_end_of_row(ctx, hi, lo);
> -    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x25 ) {
> -        ctx->rollup = 2;
> -        ctx->mode = CCMODE_ROLLUP_2;
> -    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x26 ) {
> -        ctx->rollup = 3;
> -        ctx->mode = CCMODE_ROLLUP_3;
> -    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x27 ) {
> -        ctx->rollup = 4;
> -        ctx->mode = CCMODE_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 == 0x2B ) {
> -    /* resume text display */
> -        ctx->mode = CCMODE_TEXT;
> -    } 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 */
> -        ff_dlog(ctx, "carriage return\n");
> -        reap_screen(ctx, pts);
> -        roll_up(ctx);
> -        ctx->screen_changed = 1;
> -        ctx->cursor_column = 0;
> -    } else if ( COR3(hi, 0x14, 0x15, 0x1C) && lo == 0x2F ) {
> -    /* end of caption */
> -        ff_dlog(ctx, "handle_eoc\n");
> -        ret = handle_eoc(ctx, pts);
> +    } else if (hi == 0x14 || hi == 0x15 || hi == 0x1c) {
> +        switch (lo) {
> +        case 0x20:
> +            /* resume caption loading */
> +            ctx->mode = CCMODE_POPON;
> +            break;
> +        case 0x24:
> +            handle_delete_end_of_row(ctx, hi, lo);
> +            break;
> +        case 0x25:
> +            ctx->rollup = 2;
> +            ctx->mode = CCMODE_ROLLUP_2;
> +            break;
> +        case 0x26:
> +            ctx->rollup = 3;
> +            ctx->mode = CCMODE_ROLLUP_3;
> +            break;
> +        case 0x27:
> +            ctx->rollup = 4;
> +            ctx->mode = CCMODE_ROLLUP_4;
> +            break;
> +        case 0x29:
> +            /* resume direct captioning */
> +            ctx->mode = CCMODE_PAINTON;
> +            break;
> +        case 0x2b:
> +            /* resume text display */
> +            ctx->mode = CCMODE_TEXT;
> +            break;
> +        case 0x2c:
> +            /* erase display memory */
> +            ret = handle_edm(ctx, pts);
> +            break;
> +        case 0x2d:
> +            /* carriage return */
> +            ff_dlog(ctx, "carriage return\n");
> +            reap_screen(ctx, pts);
> +            roll_up(ctx);
> +            ctx->screen_changed = 1;
> +            ctx->cursor_column = 0;
> +            break;
> +        case 0x2f:
> +            /* end of caption */
> +            ff_dlog(ctx, "handle_eoc\n");
> +            ret = handle_eoc(ctx, pts);
> +            break;
> +        default:
> +            ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
> +            break;
> +        }
>      } else if (hi>=0x20) {
> -    /* Standard characters (always in pairs) */
> +        /* Standard characters (always in pairs) */
>          handle_char(ctx, hi, lo, pts);
>      } else {
> -    /* Ignoring all other non data code */
> +        /* Ignoring all other non data code */
>          ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
>      }
>  
> @@ -508,7 +523,6 @@ static int process_cc608(CCaptionSubContext *ctx, int64_t pts, uint8_t hi, uint8
>      ctx->prev_cmd[0] = hi;
>      ctx->prev_cmd[1] = lo;
>  
> -#undef COR3
>      return ret;
>  }
>  

Looks OK, but the command handling really is clumsy in that decoder. I'm
pretty sure we can do something better by handling the command as a
uint16.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160105/8a1f820e/attachment.sig>