[PATCH] Print more subtitle info to the Activity Log

[Deleted User 11865]

Currently, for each subtitle, the Activity Log shows info about the source and whether HB is passing through or burning in, and that's it.
It doesn't show whether "Forced Only", "Default" were checked, or the charset when the source is an SRT file.

While the additional information is not absolutely necessary it can still be useful. Here's a patch: http://pastebin.org/223655

Code: Select all

Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 3293)
+++ libhb/work.c	(working copy)
@@ -305,14 +305,19 @@
 
         if( subtitle )
         {
-            hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s ", subtitle->track, subtitle->lang, subtitle->id,
+            hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s%s%s%s%s%s", subtitle->track, subtitle->lang, subtitle->id,
                     subtitle->format == PICTURESUB ? "Picture" : "Text",
                     subtitle->source == VOBSUB ? "VOBSUB" : 
                     subtitle->source == UTF8SUB ? "UTF-8" : 
                     subtitle->source == TX3GSUB ? "TX3G" : 
-                    ((subtitle->source == CC608SUB ||
-                      subtitle->source == CC708SUB) ? "CC" : "SRT"),
-                    subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through");
+                    subtitle->source == CC608SUB || subtitle->source == CC708SUB ? "CC" : "SRT",
+                    subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through",
+                    subtitle->config.force ? ", Forced Only" : "",
+                    subtitle->config.default_track ? ", Default" : "",
+                    /* If we're doing SRT, print the charset in 3 steps for formatting */
+                    subtitle->config.src_codeset && strlen( subtitle->config.src_codeset ) ? " (charset: " : "",
+                    subtitle->config.src_codeset && strlen( subtitle->config.src_codeset ) ? subtitle->config.src_codeset : "",
+                    subtitle->config.src_codeset && strlen( subtitle->config.src_codeset ) ? ")" : "" );
         }
     }
 

"Forced Only" and "Default" were pretty straightforward; I guess the way I check whether a charset has been specified is a bit hacky, but it works in my testing.

[eddyg]

Hi Rodeo,

that looks good. Only comment would be to add a check for source being SRT rather than being the final case, and having a final case being "unknown".

That code is starting to get to the point where if/thens would be more appropriate, but I'm not saying that you should do that, just an aside.

Cheers Ed.

[Deleted User 11865]

Only comment would be to add a check for source being SRT rather than being the final case, and having a final case being "unknown".

Currently that's not possible (there's no "SRTSUB" to check against, otherwise the last 3 lines would have been much simpler and I could have printed the offset for SRT as well).

Not necessarily difficult to add but it would increase the scope of my patch a bit - still, I'll look into it when I have the time.

[eddyg]

Well that was pretty silly of me not to add a source type for SRT.

Oops, thought I did.

Cheers Ed.

[Deleted User 11865]

Wait a minute, "SRTSUB" actually exists, I'm not sure how I missed it. I just assumed there wasn't because the existing work.c code did not check against it.

[Deleted User 11865]

Two new versions of the patch:

Edit 05/24/2010: fix issue in pastebin of second patch, updated patches and pastebins against r3320

http://handbrake.fr/pastebin/pastebin.php?show=1435

Code: Select all

Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 3320)
+++ libhb/work.c	(working copy)
@@ -305,14 +305,26 @@
 
         if( subtitle )
         {
-            hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s ", subtitle->track, subtitle->lang, subtitle->id,
+            //used to store the SRT offset as a C string
+            char offset[12];
+            if( subtitle->source == SRTSUB )
+                snprintf( offset, 11, "%"PRId64"", subtitle->config.offset );
+            
+            hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s%s%s%s%s%s%s", subtitle->track, subtitle->lang, subtitle->id,
                     subtitle->format == PICTURESUB ? "Picture" : "Text",
                     subtitle->source == VOBSUB ? "VOBSUB" : 
+                    subtitle->source == SRTSUB ? "SRT" : 
+                    subtitle->source == CC608SUB || subtitle->source == CC708SUB ? "CC" : 
                     subtitle->source == UTF8SUB ? "UTF-8" : 
-                    subtitle->source == TX3GSUB ? "TX3G" : 
-                    ((subtitle->source == CC608SUB ||
-                      subtitle->source == CC708SUB) ? "CC" : "SRT"),
-                    subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through");
+                    subtitle->source == TX3GSUB ? "TX3G" : "Unknown",
+                    subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through",
+                    subtitle->config.force ? ", Forced Only" : "",
+                    subtitle->config.default_track ? ", Default" : "",
+                    /* For SRT, print offset and charset too */
+                    subtitle->source == SRTSUB ? ", offset: " : "",
+                    subtitle->source == SRTSUB ? offset : "",
+                    subtitle->source == SRTSUB ? ", charset: " : "",
+                    subtitle->source == SRTSUB ? subtitle->config.src_codeset : "" );
         }
     }

http://handbrake.fr/pastebin/pastebin.php?show=1436

Code: Select all

Index: libhb/work.c
===================================================================
--- libhb/work.c	(revision 3320)
+++ libhb/work.c	(working copy)
@@ -305,14 +305,26 @@
 
         if( subtitle )
         {
-            hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s ", subtitle->track, subtitle->lang, subtitle->id,
-                    subtitle->format == PICTURESUB ? "Picture" : "Text",
-                    subtitle->source == VOBSUB ? "VOBSUB" : 
-                    subtitle->source == UTF8SUB ? "UTF-8" : 
-                    subtitle->source == TX3GSUB ? "TX3G" : 
-                    ((subtitle->source == CC608SUB ||
-                      subtitle->source == CC708SUB) ? "CC" : "SRT"),
-                    subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through");
+            if( subtitle->source == SRTSUB )
+            {
+                /* For SRT, print offset and charset too */
+                hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s%s, offset: %"PRId64", charset: %s",
+                        subtitle->track, subtitle->lang, subtitle->id, "Text", "SRT", "Pass-Through",
+                        subtitle->config.default_track ? ", Default" : "",
+                        subtitle->config.offset, subtitle->config.src_codeset );
+            }
+            else
+            {
+                hb_log( " * subtitle track %i, %s (id %x) %s [%s] -> %s%s%s", subtitle->track, subtitle->lang, subtitle->id,
+                        subtitle->format == PICTURESUB ? "Picture" : "Text",
+                        subtitle->source == VOBSUB ? "VOBSUB" : 
+                        subtitle->source == CC608SUB || subtitle->source == CC708SUB ? "CC" : 
+                        subtitle->source == UTF8SUB ? "UTF-8" : 
+                        subtitle->source == TX3GSUB ? "TX3G" : "Unknown",
+                        subtitle->config.dest == RENDERSUB ? "Render/Burn in" : "Pass-Through",
+                        subtitle->config.force ? ", Forced Only" : "",
+                        subtitle->config.default_track ? ", Default" : "" );
+            }
         }
     }

I figured there were enough differences between external (SRT) subs and other subs to make it a distinct case (second patch). Also avoids having to write the int64_t offset to a temporary string to allow for replacing it with "" when the source isn't SRT.

[eddyg]

Looks good to me, thanks Rodeo

[Deleted User 11865]

Minor update of the patches, see 2 posts above.

[Deleted User 11865]

Same patch as above, against r3339:

http://handbrake.fr/pastebin/pastebin.php?show=1444

One change, only print "Forced Only" if the source track is VOBSUB (though this may be better left to the GUI - e.g. hiding the checkbox if the source sub isn't VOBSUB):

http://handbrake.fr/pastebin/pastebin.php?show=1445

I tested on as many sources as possible (DVD, MP4, MKV with a mix of VOBSUB, CC, TX3G, SRT or Text/UTF-8 subtitle tracks) and it works fine. Anyone feel like committing either patch?

[JohnAStebbins]

Committed http://trac.handbrake.fr/changeset/3341
I used the first patch. It's possible that other subtitle types could have forced flags. BD PGS comes to mind. So I think sanitizing that flag should be left to other parts of the code.

[Deleted User 11865]

Thanks.

I used the first patch. It's possible that other subtitle types could have forced flags. BD PGS comes to mind. So I think sanitizing that flag should be left to other parts of the code.

Makes sense. I only posted the second patch because I was experimenting with it.