Re: [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 
> 
> ---
>  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 

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

[Clément Bœsch]

On Tue, Jan 05, 2016 at 09:35:41PM +0100, Clément Bœsch wrote:
> On Mon, Jan 04, 2016 at 07:28:03PM -0800, Aman Gupta wrote:
> > From: Aman Gupta 
[...]
> >  } 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);
[...]
> 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.
> 

To expand on this, the first condition would look something like this:

if ((cmd & 0x7f60) == 0x1040 || (cmd & 0x7940) == 0x1140)

with cmd = bptr[i+1]<<8 | bptr[i+2]

(Note: cmd is assumed to be already masked with a 0x7f7f)

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Aman Gupta]

From: Aman Gupta 

---
 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;
 }
 
-- 
2.5.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel