[vlc-devel] [PATCH 3/3] Use CCT to format subtitiles text properly.

[Laurent Aimar]

Hi,

On Fri, Jun 03, 2011 at 12:27:03AM -0700, Tomer Barletz wrote:
> +#define GSI_BLOCK_SIZE 1024
> +
> +typedef enum {
> +    CCT_ISO_6937_2 = 0x3030, CCT_BEGIN = CCT_ISO_6937_2,
> +    CCT_ISO_8859_5 = 0x3031,
> +    CCT_ISO_8859_6 = 0x3032,
> +    CCT_ISO_8859_7 = 0x3033,
> +    CCT_ISO_8859_8 = 0x3034, CCT_END = CCT_ISO_8859_8
> +} cct_number_value_t;
> +
> +typedef struct {
> +    cct_number_value_t value;
> +    const char *str;
> +} cct_number_t;
> +
>  struct decoder_sys_t {
> -    int dummy;
> +    cct_number_value_t cct;
>  };
>  
> -static char *ParseText(uint8_t *data, int size)
> +static cct_number_t cct_nums[] = { {CCT_ISO_6937_2, "ISO_6937-2"},
> +                                   {CCT_ISO_8859_5, "ISO_8859-5"},
> +                                   {CCT_ISO_8859_6, "ISO_8859-6"},
> +                                   {CCT_ISO_8859_7, "ISO_8859-7"},
> +                                   {CCT_ISO_8859_8, "ISO_8859-8"} };
> +
> +
> +static char *ParseText(uint8_t *data, int size, const char *charset)
>  {
>      char *text = strdup("");
>      int  text_size = 0;
> @@ -68,7 +91,8 @@ static char *ParseText(uint8_t *data, int size)
>  
>          char tmp[16] = "";
>          char *t = tmp;
> -        if (code >= 0x20 && code <= 0x7f)
> +        if ((code >= 0x20 && code <= 0x7e) ||
> +            (code >= 0xa0 && code <= 0xff) )
>              snprintf(tmp, sizeof(tmp), "%c", code);
>  #if 0
>          else if (code == 0x80)
> @@ -79,9 +103,9 @@ static char *ParseText(uint8_t *data, int size)
>              snprintf(tmp, sizeof(tmp), "<u>");
>          else if (code == 0x83)
>              snprintf(tmp, sizeof(tmp), "</u>");
> -#endif
>          else if (code == 0x8a)
>              snprintf(tmp, sizeof(tmp), "\n");
 Why do you disable this code mapping?
> +#endif
>          else {
>              fprintf(stderr, "--> %2.2x\n", code);
>              t = NULL;
> @@ -97,7 +121,7 @@ static char *ParseText(uint8_t *data, int size)
>          text_size += t_size;
>          text[text_size]   = '\0';
>      }
> -    return text;
> +    return FromCharset(charset, text, text_size);
>  }
>  
>  static subpicture_t *Decode(decoder_t *dec, block_t **block)
> @@ -138,8 +162,11 @@ static subpicture_t *Decode(decoder_t *dec, block_t **block)
>      video_format_Clean(&fmt);
>  
>      if (sub->p_region) {
> -        sub->p_region->psz_text = ParseText(payload, payload_size);
> +        sub->p_region->psz_text = ParseText(payload,
> +                                            payload_size,
> +                                            cct_nums[dec->p_sys->cct - CCT_BEGIN].str);
>          sub->p_region->psz_html = NULL;
> +        sub->p_region->i_align = SUBPICTURE_ALIGN_BOTTOM;
 It doesn't really belong to this patch but not really important.
Using
SUBPICTURE_ALIGN_BOTTOM | var_InheritInteger(dec, "subsdec-align");
would be better as it would make it behave like the other subtitles decoder.

>      }
>  
>      free(payload);
> @@ -149,6 +176,30 @@ exit:
>      return sub;
>  }
>  
> +static int ExtractCCT(const es_format_t *fmt, decoder_sys_t *sys)
> +{
> +    uint8_t *header = fmt->p_extra;
> +    if (!header) {
> +        fprintf(stderr, "NULL EBU header (GSI block)\n");
> +        return VLC_EGENERIC;
> +    }
> +
> +    if (GSI_BLOCK_SIZE != fmt->i_extra) {
> +        fprintf(stderr, "EBU header is not in expected size (%d)\n", fmt->i_extra);
> +        return VLC_EGENERIC;
> +    }
> +
> +    int cct = (header[12] << 8) | header[13];
> +    if (CCT_BEGIN > cct || CCT_END < cct) {
> +        fprintf(stderr, "EBU header contains illegal CCT (0x%x)\n", cct);
> +        return VLC_EGENERIC;
> +    }
> +
> +    sys->cct = cct;
> +
> +    return VLC_SUCCESS;
> +}
 Do not use fprintf. I think here using msg_Err is what you want.
> +
>  static int Open(vlc_object_t *object)
>  {
>      decoder_t *dec = (decoder_t*)object;
> @@ -157,6 +208,14 @@ static int Open(vlc_object_t *object)
>          return VLC_EGENERIC;
>  
>      decoder_sys_t *sys = malloc(sizeof(*sys));
> +    if (!sys)
> +        return VLC_ENOMEM;
> +
> +    int rc = ExtractCCT(&dec->fmt_in, sys);
> +    if (VLC_SUCCESS != rc)
> +        return rc;
 You have a memory leak in this case (sys).
> +
> +    msg_Info(dec, "CCT=0x%x", sys->cct);
 I don't think it deserves a msg_Info(), msg_Dbg() is probably better.

-- 
fenrir