[vlc-devel] [PATCH] Add option to disable DVD subtitle transparency

[Rémi Denis-Courmont]

Le dimanche 17 janvier 2010 13:33:49 Kaarlo Räihä, vous avez écrit :
> As someone pointed out in forums:
> http://forum.videolan.org/viewtopic.php?f=14&t=70201&p=233859
> 
> Some DVD subtitles have "bad" transparency values. I created a patch to
>  solve this. It adds option that makes subtitle parsing skip
>  transparency/alpha values given in stream and instead use default values
>  which solid ones (no transparency at all).

diff --git a/modules/codec/spudec/parse.c b/modules/codec/spudec/parse.c
index 5c2bc30..6fc4a4d 100644
--- a/modules/codec/spudec/parse.c
+++ b/modules/codec/spudec/parse.c
@@ -293,11 +293,14 @@ static int ParseControlSeq( decoder_t *p_dec, 
subpicture_t *p_spu,
                 return VLC_EGENERIC;
             }
 
-            b_cmd_alpha = true;
-            spu_data_cmd.pi_alpha[3] = (p_sys->buffer[i_index+1]>>4)&0x0f;
-            spu_data_cmd.pi_alpha[2] = (p_sys->buffer[i_index+1])&0x0f;
-            spu_data_cmd.pi_alpha[1] = (p_sys->buffer[i_index+2]>>4)&0x0f;
-            spu_data_cmd.pi_alpha[0] = (p_sys->buffer[i_index+2])&0x0f;
+            if(!p_sys->b_disabletrans)
+            { /* If we want to use original transparency values */
+                b_cmd_alpha = true;
+                spu_data_cmd.pi_alpha[3] = (p_sys-
>buffer[i_index+1]>>4)&0x0f;
+                spu_data_cmd.pi_alpha[2] = (p_sys->buffer[i_index+1])&0x0f;
+                spu_data_cmd.pi_alpha[1] = (p_sys-
>buffer[i_index+2]>>4)&0x0f;
+                spu_data_cmd.pi_alpha[0] = (p_sys->buffer[i_index+2])&0x0f;
+            }

Are those values initialized somewhere?
 
@@ -49,6 +53,9 @@ vlc_module_begin ()
     set_subcategory( SUBCAT_INPUT_SCODEC )
     set_callbacks( DecoderOpen, Close )
 
+    add_bool( "dvdsub-transparency", false, NULL,
+    		  DVDSUBTRANS_DISABLE_TEXT, DVDSUBTRANS_DISABLE_LONGTEXT, true )
+    change_need_restart ()

Why does this need a restart??

@@ -79,6 +86,9 @@ static int DecoderOpen( vlc_object_t *p_this )
     p_dec->p_sys = p_sys = malloc( sizeof( decoder_sys_t ) );
 
     p_sys->b_packetizer = false;
+    p_sys->b_disabletrans = false;
+    if( var_CreateGetBool( p_dec, "dvdsub-transparency" ) )
+        p_sys->b_disabletrans = true;

Why do you create a new object variable? This seems totally contradictory with 
requiring a restart.

diff --git a/modules/codec/spudec/spudec.h b/modules/codec/spudec/spudec.h
index 9e5b5fb..df2fa27 100644
--- a/modules/codec/spudec/spudec.h
+++ b/modules/codec/spudec/spudec.h
@@ -26,6 +26,7 @@
 struct decoder_sys_t
 {
     int b_packetizer;
+    int b_disabletrans;

Why is this not a boolean?

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis