Simplify Foreign Audio Search code:

- move all subtitle hit counting to the decoders instead of reader
---> allows us to count actual subtitles rather than just packets

- always count subtitles, even when not doing a scan (may be useful in the future)

Miscellaneous improvements:

- always insert select_subtitle at the head of the output subtitle list, to make it less likely to be dropped

- when multiple subtitle tracks have forced hits, pick the track with the fewest forced hits
---> Foreign Audio Search should now work with Star Wars on Blu-ray

- logging improvements (more readable, and log job->select_subtitle configuration - Forced Only vs. All, Render vs. Passthrough)



git-svn-id: svn://svn.handbrake.fr/HandBrake/trunk@4622 b64f7644-9d1e-0410-96f1-a4d463321fa5

4 files changed, 98 insertions, 102 deletions

diff --git a/libhb/decpgssub.c b/libhb/decpgssub.c
index 1b0f7e4cd..77b90ad03 100644
--- a/libhb/decpgssub.c
+++ b/libhb/decpgssub.c
@@ -111,32 +111,30 @@ static int decsubWork( hb_work_object_t * w, hb_buffer_t ** buf_in,
             avp.size = 0;
         }
 
+        if (has_subtitle && subtitle.num_rects > 0)
+        {
+            w->subtitle->hits++;
+            if (subtitle.forced)
+                w->subtitle->forced_hits++;
+        }
+
         // The sub is "usable" if:
         // 1. libav returned a sub AND
-        // 2. we are scanning for foreign audio subs OR
+        // 2. we are not scanning for foreign audio subs AND
         // 3. we want all subs (e.g. not forced) OR
         // 4. the sub is forced and we only want forced OR
         // 5. subtitle clears the previous sub (these are never forced)
         //    subtitle.num_rects == 0
-        uint8_t useable_sub =
-                    pv->job->indepth_scan ||
-                    subtitle.num_rects == 0 ||
-                    !w->subtitle->config.force ||
-                    ( w->subtitle->config.force && subtitle.forced );
+        uint8_t useable_sub = has_subtitle && !pv->job->indepth_scan &&
+                              ( !subtitle.num_rects || !w->subtitle->config.force ||
+                                ( w->subtitle->config.force && subtitle.forced ) );
 
-        if (has_subtitle && useable_sub)
+        if (useable_sub)
         {
             int64_t pts = av_rescale(subtitle.pts, 90000, AV_TIME_BASE);
             hb_buffer_t * out = NULL;
 
-            if( pv->job->indepth_scan )
-            {
-                if (subtitle.forced)
-                {
-                    w->subtitle->forced_hits++;
-                }
-            }
-            else if ( w->subtitle->config.dest == PASSTHRUSUB &&
+            if ( w->subtitle->config.dest == PASSTHRUSUB &&
                  hb_subtitle_can_pass( PGSSUB, pv->job->mux ) )
             {
                 out = pv->list_pass_buffer;
diff --git a/libhb/decvobsub.c b/libhb/decvobsub.c
index 43f2aefd9..9a9b50a19 100644
--- a/libhb/decvobsub.c
+++ b/libhb/decvobsub.c
@@ -259,28 +259,14 @@ static void ParseControls( hb_work_object_t * w )
                 case 0x00: // 0x00 - FSTA_DSP - Forced Start Display, no arguments
                     pv->pts_start = pv->pts + date * 1024;
                     pv->pts_forced = 1;
-
-                    /*
-                     * If we are doing a subtitle scan then note down
-                     */
-                    if( job->indepth_scan )
-                    {
-                        for( n=0; n < hb_list_count(title->list_subtitle); n++ )
-                        {
-                            subtitle = hb_list_item( title->list_subtitle, n);
-                            if( pv->stream_id == subtitle->id ) {
-                                /*
-                                 * A hit, count it.
-                                 */
-                                subtitle->forced_hits++;
-                            }
-                        }
-                    }
+                    w->subtitle->hits++;
+                    w->subtitle->forced_hits++;
                     break;
 
                 case 0x01: // 0x01 - STA_DSP - Start Display, no arguments
                     pv->pts_start = pv->pts + date * 1024;
                     pv->pts_forced  = 0;
+                    w->subtitle->hits++;
                     break;
 
                 case 0x02: // 0x02 - STP_DSP - Stop Display, no arguments
diff --git a/libhb/reader.c b/libhb/reader.c
index 40756bc67..1807c0aac 100644
--- a/libhb/reader.c
+++ b/libhb/reader.c
@@ -741,22 +741,15 @@ static hb_fifo_t ** GetFifoForId( hb_job_t * job, int id )
         }
     }
 
-    n = 0;
     count = hb_list_count( title->list_subtitle );
     count = count > 99 ? 99 : count;
-    for( i=0; i < count; i++ ) {
+    for( i = n = 0; i < count; i++ )
+    {
         subtitle =  hb_list_item( title->list_subtitle, i );
-        if (id == subtitle->id) {
-            subtitle->hits++;
-            if( !job->indepth_scan || job->select_subtitle_config.force )
-            {
-                /*
-                 * Pass the subtitles to be processed if we are not scanning, or if
-                 * we are scanning and looking for forced subs, then pass them up
-                 * to decode whether the sub is a forced one.
-                 */
-                fifos[n++] = subtitle->fifo_in;
-            }
+        if (id == subtitle->id)
+        {
+            /* pass the subtitles to be processed */
+            fifos[n++] = subtitle->fifo_in;
         }
     }
     if ( n != 0 )
@@ -766,8 +759,7 @@ static hb_fifo_t ** GetFifoForId( hb_job_t * job, int id )
     
     if( !job->indepth_scan )
     {
-        n = 0;
-        for( i = 0; i < hb_list_count( title->list_audio ); i++ )
+        for( i = n = 0; i < hb_list_count( title->list_audio ); i++ )
         {
             audio = hb_list_item( title->list_audio, i );
             if( id == audio->id )
diff --git a/libhb/work.c b/libhb/work.c
index eab4db821..39c5a0dab 100644
--- a/libhb/work.c
+++ b/libhb/work.c
@@ -380,30 +380,42 @@ void hb_display_job_info( hb_job_t * job )
         }
     }
 
+    if( job->indepth_scan )
+    {
+        hb_log( " * Foreign Audio Search: %s%s%s",
+                job->select_subtitle_config.dest == RENDERSUB ? "Render/Burn-in" : "Passthrough",
+                job->select_subtitle_config.force ? ", Forced Only" : "",
+                job->select_subtitle_config.default_track ? ", Default" : "" );
+    }
+
     for( i=0; i < hb_list_count( title->list_subtitle ); i++ )
     {
         subtitle =  hb_list_item( title->list_subtitle, i );
 
         if( subtitle )
         {
-            if( subtitle->source == SRTSUB )
+            if( job->indepth_scan )
+            {
+                hb_log( "   + subtitle, %s (track %d, id 0x%x) %s [%s]",
+                        subtitle->lang, subtitle->track, subtitle->id,
+                        subtitle->format == PICTURESUB ? "Picture" : "Text",
+                        hb_subsource_name( subtitle->source ) );
+            }
+            else if( subtitle->source == SRTSUB )
             {
                 /* For SRT, print offset and charset too */
-                hb_log( " * subtitle track %i, %s (id 0x%x) %s [%s] -> %s%s, offset: %"PRId64", charset: %s",
-                        subtitle->out_track, subtitle->lang, subtitle->id,
-                        "Text", "SRT", "Pass-Through",
+                hb_log( " * subtitle track %i, %s (track %d, id 0x%x) Text [SRT] -> %s%s, offset: %"PRId64", charset: %s",
+                        subtitle->out_track, subtitle->lang, subtitle->track, subtitle->id, "Passthrough",
                         subtitle->config.default_track ? ", Default" : "",
                         subtitle->config.offset, subtitle->config.src_codeset );
             }
             else
             {
-                hb_log( " * subtitle track %i, %s (id 0x%x) %s [%s] -> %s%s%s",
-                        subtitle->out_track, subtitle->lang, subtitle->id,
+                hb_log( " * subtitle track %i, %s (track %d, id 0x%x) %s [%s] -> %s%s%s",
+                        subtitle->out_track, subtitle->lang, subtitle->track, subtitle->id,
                         subtitle->format == PICTURESUB ? "Picture" : "Text",
                         hb_subsource_name( subtitle->source ),
-                        job->indepth_scan ? "Foreign Audio Search" :
-                            subtitle->config.dest == RENDERSUB ?
-                                "Render/Burn in" : "Pass-Through",
+                        subtitle->config.dest == RENDERSUB ? "Render/Burn-in" : "Passthrough",
                         subtitle->config.force ? ", Forced Only" : "",
                         subtitle->config.default_track ? ", Default" : "" );
             }
@@ -555,9 +567,10 @@ static void do_job( hb_job_t * job )
     hb_subtitle_t * subtitle;
     unsigned int subtitle_highest = 0;
     unsigned int subtitle_highest_id = 0;
-    unsigned int subtitle_lowest = -1;
+    unsigned int subtitle_lowest = 0;
     unsigned int subtitle_lowest_id = 0;
     unsigned int subtitle_forced_id = 0;
+    unsigned int subtitle_forced_hits = 0;
     unsigned int subtitle_hit = 0;
 
     title = job->title;
@@ -583,16 +596,14 @@ static void do_job( hb_job_t * job )
          * Disable forced subtitles if we didn't find any in the scan
          * so that we display normal subtitles instead.
          */
-        if( interjob->select_subtitle->config.force && 
+        if( interjob->select_subtitle->config.force &&
             interjob->select_subtitle->forced_hits == 0 )
         {
             interjob->select_subtitle->config.force = 0;
         }
-        for( i=0; i < hb_list_count(title->list_subtitle); i++ )
+        for( i = 0; i < hb_list_count(title->list_subtitle); )
         {
-            subtitle =  hb_list_item( title->list_subtitle, i );
-
-            if( subtitle )
+            if( ( subtitle = hb_list_item( title->list_subtitle, i ) ) )
             {
                 /*
                 * Remove the scanned subtitle from the subtitle list if
@@ -600,30 +611,37 @@ static void do_job( hb_job_t * job )
                 * or an emty track (forced and no forced hits).
                 */
                 if( ( interjob->select_subtitle->id == subtitle->id ) &&
-                    ( ( interjob->select_subtitle->forced_hits == 0 &&
-                        subtitle->config.force ) ||
-                    ( subtitle->config.force == interjob->select_subtitle->config.force ) ) )
+                    ( ( subtitle->config.force && interjob->select_subtitle->forced_hits == 0 ) ||
+                      ( subtitle->config.force == interjob->select_subtitle->config.force ) ) )
                 {
-                    *subtitle = *(interjob->select_subtitle);
-                    free( interjob->select_subtitle );
-                    interjob->select_subtitle = NULL;
-                    break;
+                    hb_list_rem( title->list_subtitle, subtitle );
+                    free( subtitle );
+                    continue;
                 }
+                /* Adjust output track number, in case we removed one.
+                 * Output tracks sadly still need to be in sequential order.
+                 * Note: out.track starts at 1, i starts at 0, and track 1 is
+                 *       interjob->select_subtitle */
+                subtitle->out_track = ++i + 1;
+            }
+            else
+            {
+                // avoid infinite loop is subtitle == NULL
+                i++;
             }
         }
 
-        if( interjob->select_subtitle )
-        {
-            /*
-             * Its not in the existing list
-             *
-             * Must be second pass of a two pass with subtitle scan enabled, so
-             * add the subtitle that we found on the first pass for use in this
-             * pass.
-             */
-            hb_list_add( title->list_subtitle, interjob->select_subtitle );
-            interjob->select_subtitle = NULL;
-        }
+        /*
+         * Add the subtitle that we found on the first pass for use in this
+         * pass.
+         *
+         * Make sure it's the first subtitle in the list so that is is the
+         * first burned subtitle (explicitly or after sanitizing) - which
+         * should ensures that it doesn't get dropped.
+         */
+        interjob->select_subtitle->out_track = 1;
+        hb_list_insert( title->list_subtitle, 0, interjob->select_subtitle );
+        interjob->select_subtitle = NULL;
     }
 
     if ( !job->indepth_scan )
@@ -1401,31 +1419,33 @@ cleanup:
         {
             subtitle =  hb_list_item( title->list_subtitle, i );
 
-            hb_log( "Subtitle stream 0x%x '%s': %d hits (%d forced)",
-                    subtitle->id, subtitle->lang, subtitle->hits,
-                    subtitle->forced_hits );
+            hb_log( "Subtitle track %d (id 0x%x) '%s': %d hits (%d forced)",
+                    subtitle->track, subtitle->id, subtitle->lang,
+                    subtitle->hits, subtitle->forced_hits );
 
             if( subtitle->hits == 0 )
                 continue;
 
-            if( subtitle->hits > subtitle_highest )
+            if( subtitle_highest < subtitle->hits )
             {
                 subtitle_highest = subtitle->hits;
                 subtitle_highest_id = subtitle->id;
             }
 
-            if( subtitle->hits < subtitle_lowest )
+            if( subtitle_lowest == 0 ||
+                subtitle_lowest > subtitle->hits )
             {
                 subtitle_lowest = subtitle->hits;
                 subtitle_lowest_id = subtitle->id;
             }
 
-            if( subtitle->forced_hits > 0 )
+            // pick the track with fewest forced hits
+            if( subtitle->forced_hits > 0 &&
+                ( subtitle_forced_hits == 0 ||
+                  subtitle_forced_hits > subtitle->forced_hits ) )
             {
-                if( subtitle_forced_id == 0 )
-                {
-                    subtitle_forced_id = subtitle->id;
-                }
+                subtitle_forced_id = subtitle->id;
+                subtitle_forced_hits = subtitle->forced_hits;
             }
         }
 
@@ -1437,9 +1457,11 @@ cleanup:
              * then select it in preference to the lowest.
              */
             subtitle_hit = subtitle_forced_id;
-            hb_log("Found a subtitle candidate id 0x%x (contains forced subs)",
-                   subtitle_hit);
-        } else if( subtitle_lowest < subtitle_highest )
+            hb_log( "Found a subtitle candidate with id 0x%x (contains forced subs)",
+                    subtitle_hit );
+        }
+        else if( subtitle_lowest > 0 &&
+                 subtitle_lowest < ( subtitle_highest * 0.1 ) )
         {
             /*
              * OK we have more than one, and the lowest is lower,
@@ -1448,14 +1470,12 @@ cleanup:
              *
              * Let's say 10% as a default.
              */
-            if( subtitle_lowest < ( subtitle_highest * 0.1 ) )
-            {
-                subtitle_hit = subtitle_lowest_id;
-                hb_log( "Found a subtitle candidate id 0x%x",
-                        subtitle_hit );
-            } else {
-                hb_log( "No candidate subtitle detected during subtitle-scan");
-            }
+            subtitle_hit = subtitle_lowest_id;
+            hb_log( "Found a subtitle candidate with id 0x%x", subtitle_hit );
+        }
+        else
+        {
+            hb_log( "No candidate detected during subtitle scan" );
         }
     }