- change aspect from a scaled int to a double so we can handle the wider

   range of aspect ratios we get from ffmpeg files.

 - add container_aspect to title struct (always zero except for DVDs
   when it's the aspect from the VTSI). To handle broken French DVDs,
   make HB complain & use the container aspect if it's different from
   the aspect computed from the video PAR.

 - fix ScanFunc's job template init so that it doesn't think the only
   legal aspect ratios are 16:9 & 4:3.

 - hb_reduce wouldn't reduce any fraction where both terms were equal and
   prime (e.g., 2/2, 3/3, 5/5, etc. would not become 1/1). Recoded it using
   Euclid's Algorithm so it always works and is faster.


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

10 files changed, 129 insertions, 84 deletions

diff --git a/libhb/common.c b/libhb/common.c
index c625bec9b..c8cc8dddc 100644
--- a/libhb/common.c
+++ b/libhb/common.c
@@ -82,18 +82,26 @@ const char * hb_mixdown_get_short_name_from_mixdown( int amixdown )
  *********************************************************************/
 void hb_reduce( int *x, int *y, int num, int den )
 {
-    int lower = MIN( num, den );
-    int i;
-    *x = num;
-    *y = den;
-    for( i = lower - 1; i > 1; --i )
+    // find the greatest common divisor of num & den by Euclid's algorithm
+    int n = num, d = den;
+    while ( d )
     {
-        if( ( num % i == 0 ) && ( den % i == 0 ) )
-        {
-            *x = num / i;
-            *y = den / i;
-            break;
-        }
+        int t = d;
+        d = n % d;
+        n = t;
+    }
+
+    // at this point n is the gcd. if it's non-zero remove it from num
+    // and den. Otherwise just return the original values.
+    if ( n )
+    {
+        *x = num / n;
+        *y = den / n;
+    }
+    else
+    {
+        *x = num;
+        *y = den;
     }
 }
 
@@ -143,22 +151,18 @@ void hb_fix_aspect( hb_job_t * job, int keep )
         }
     }
 
+    double par = (double)title->width / ( (double)title->height * title->aspect );
+    double cropped_sar = (double)( title->height - job->crop[0] - job->crop[1] ) /
+                         (double)(title->width - job->crop[2] - job->crop[3] );
+    double ar = par * cropped_sar;
     if( keep == HB_KEEP_WIDTH )
     {
-        job->height = MULTIPLE_16(
-            (uint64_t) job->width * title->width * HB_ASPECT_BASE *
-              ( title->height - job->crop[0] - job->crop[1] ) /
-            ( (uint64_t) title->height * title->aspect *
-              ( title->width - job->crop[2] - job->crop[3] ) ) );
+        job->height = MULTIPLE_16( (uint64_t)( (double)job->width * ar ) );
         job->height = MAX( 16, job->height );
     }
     else
     {
-        job->width = MULTIPLE_16(
-            (uint64_t) job->height * title->height * title->aspect *
-              ( title->width - job->crop[2] - job->crop[3] ) /
-            ( (uint64_t) title->width * HB_ASPECT_BASE *
-              ( title->height - job->crop[0] - job->crop[1] ) ) );
+        job->width = MULTIPLE_16( (uint64_t)( (double)job->height / ar ) );
         job->width = MAX( 16, job->width );
     }
 }
diff --git a/libhb/common.h b/libhb/common.h
index 381b32d07..018055894 100644
--- a/libhb/common.h
+++ b/libhb/common.h
@@ -93,7 +93,6 @@ struct hb_mixdown_s
     int    amixdown;
 };
 
-#define HB_ASPECT_BASE 9
 #define HB_VIDEO_RATE_BASE   27000000
 
 extern hb_rate_t    hb_video_rates[];
@@ -439,9 +438,10 @@ struct hb_title_s
     /* Exact duration (in 1/90000s) */
     uint64_t    duration;
 
+    double      aspect;             // aspect ratio for the title's video
+    double      container_aspect;   // aspect ratio from container (0 if none)
     int         width;
     int         height;
-    int         aspect;
     int         pixel_aspect_width;
     int         pixel_aspect_height;
     int         rate;
diff --git a/libhb/decavcodec.c b/libhb/decavcodec.c
index 2ad3993a0..a945680e0 100644
--- a/libhb/decavcodec.c
+++ b/libhb/decavcodec.c
@@ -547,12 +547,11 @@ static int decavcodecvInfo( hb_work_object_t *w, hb_work_info_t *info )
         /* ffmpeg returns the Pixel Aspect Ratio (PAR). Handbrake wants the
          * Display Aspect Ratio so we convert by scaling by the Storage
          * Aspect Ratio (w/h). We do the calc in floating point to get the
-         * rounding right. We round in the second decimal digit because we
-         * scale the (integer) aspect by 9 to preserve the 1st digit.  */
-        info->aspect = ( (double)info->pixel_aspect_width * 
-                         (double)context->width /
-                         (double)info->pixel_aspect_height /
-                         (double)context->height + 0.05 ) * HB_ASPECT_BASE;
+         * rounding right. */
+        info->aspect = (double)info->pixel_aspect_width * 
+                       (double)context->width /
+                       (double)info->pixel_aspect_height /
+                       (double)context->height;
 
         info->profile = context->profile;
         info->level = context->level;
diff --git a/libhb/decmpeg2.c b/libhb/decmpeg2.c
index bccbd065b..4972d0468 100644
--- a/libhb/decmpeg2.c
+++ b/libhb/decmpeg2.c
@@ -39,7 +39,7 @@ typedef struct hb_libmpeg2_s
     int                  width;
     int                  height;
     int                  rate;
-    int                  aspect_ratio;
+    double               aspect_ratio;
     int                  got_iframe;        /* set when we get our first iframe */
     int                  look_for_iframe;   /* need an iframe to add chap break */
     int                  look_for_break;    /* need gop start to add chap break */
@@ -109,13 +109,10 @@ static int hb_libmpeg2_decode( hb_libmpeg2_t * m, hb_buffer_t * buf_es,
                      * it keeps the pixel width & height that would cause
                      * the storage width & height to come out in the correct
                      * aspect ratio. Convert these back to aspect ratio.
-                     * We do the calc in floating point to get the rounding right.
-                     * We round in the second decimal digit because we scale
-                     * the (integer) aspect by 9 to preserve the 1st digit.
                      */
                     double ar_numer = m->width * m->info->sequence->pixel_width;
                     double ar_denom = m->height * m->info->sequence->pixel_height;
-                    m->aspect_ratio = ( ar_numer / ar_denom + .05 ) * HB_ASPECT_BASE;
+                    m->aspect_ratio = ar_numer / ar_denom;
                 }
             }
         }
@@ -431,7 +428,7 @@ static int decmpeg2Info( hb_work_object_t *w, hb_work_info_t *info )
         info->height = m->height;
         info->pixel_aspect_width = m->info->sequence->pixel_width;
         info->pixel_aspect_height = m->info->sequence->pixel_height;
-        info->aspect = (double)m->aspect_ratio;
+        info->aspect = m->aspect_ratio;
 
         // if the frame is progressive & NTSC DVD height report it as 23.976 FPS
         // so that scan can autodetect NTSC film
diff --git a/libhb/dvd.c b/libhb/dvd.c
index 3cfc1b91e..c21a24589 100644
--- a/libhb/dvd.c
+++ b/libhb/dvd.c
@@ -488,17 +488,17 @@ hb_title_t * hb_dvd_title_scan( hb_dvd_t * d, int t )
     switch( vts->vtsi_mat->vts_video_attr.display_aspect_ratio )
     {
         case 0:
-            title->aspect = HB_ASPECT_BASE * 4 / 3;
+            title->container_aspect = 4. / 3.;
             break;
         case 3:
-            title->aspect = HB_ASPECT_BASE * 16 / 9;
+            title->container_aspect = 16. / 9.;
             break;
         default:
             hb_log( "scan: unknown aspect" );
             goto fail;
     }
 
-    hb_log( "scan: aspect = %d", title->aspect );
+    hb_log( "scan: aspect = %g", title->aspect );
 
     /* This title is ok so far */
     goto cleanup;
diff --git a/libhb/hb.c b/libhb/hb.c
index b91bddf27..0e0bddb3c 100644
--- a/libhb/hb.c
+++ b/libhb/hb.c
@@ -567,11 +567,11 @@ void hb_set_anamorphic_size( hb_job_t * job,
     hb_title_t * title = job->title;
     int cropped_width = title->width - job->crop[2] - job->crop[3] ;
     int cropped_height = title->height - job->crop[0] - job->crop[1] ;
-    int storage_aspect = cropped_width * 10000 / cropped_height;
+    double storage_aspect = (double)cropped_width / (double)cropped_height;
     int width = job->width;
     int height; // Gets set later, ignore user value
     int mod = job->modulus;
-    int aspect = title->aspect;
+    double aspect = title->aspect;
 
     /* Gotta handle bounding dimensions differently
        than for non-anamorphic encodes:
@@ -585,13 +585,14 @@ void hb_set_anamorphic_size( hb_job_t * job,
     if ( job->maxWidth && (job->maxWidth < job->width) )
             width = job->maxWidth;
 
-    if ( job->maxHeight && (job->maxHeight < (width / storage_aspect * 10000)) )
+    height = (double)width / storage_aspect;
+    if ( job->maxHeight && (job->maxHeight < height) )
     {
         height = job->maxHeight;
     }
     else
     {
-        height = width * 10000 / storage_aspect;
+        height = (double)width / storage_aspect;
     }
 
 
@@ -659,19 +660,24 @@ void hb_set_anamorphic_size( hb_job_t * job,
     
     /* If a source was really 704*480 and hard matted with cropping
        to 720*480, replace the PAR values with the ITU broadcast ones. */
-    if (cropped_width <= 706)
+    if (title->width == 720 && cropped_width <= 706)
     {
+        // convert aspect to a scaled integer so we can test for 16:9 & 4:3
+        // aspect ratios ignoring insignificant differences in the LSBs of
+        // the floating point representation.
+        int iaspect = aspect * 9.;
+
         /* Handle ITU PARs */
         if (title->height == 480)
         {
             /* It's NTSC */
-            if (aspect == 16)
+            if (iaspect == 16)
             {
                 /* It's widescreen */
                 pixel_aspect_width = 40;
                 pixel_aspect_height = 33;
             }
-            else if (aspect == 12)
+            else if (iaspect == 12)
             {
                 /* It's 4:3 */
                 pixel_aspect_width = 10;
@@ -681,13 +687,13 @@ void hb_set_anamorphic_size( hb_job_t * job,
         else if (title->height == 576)
         {
             /* It's PAL */
-            if(aspect == 16)
+            if(iaspect == 16)
             {
                 /* It's widescreen */
                 pixel_aspect_width = 16;
                 pixel_aspect_height = 11;
             }
-            else if (aspect == 12)
+            else if (iaspect == 12)
             {
                 /* It's 4:3 */
                 pixel_aspect_width = 12;
@@ -697,7 +703,8 @@ void hb_set_anamorphic_size( hb_job_t * job,
     }
 
     /* Figure out what dimensions the source would display at. */
-    int source_display_width = cropped_width * ((float)pixel_aspect_width / (float)pixel_aspect_height) ;
+    int source_display_width = cropped_width * (double)pixel_aspect_width /
+                               (double)pixel_aspect_height ;
 
     /* The film AR is the source's display width / cropped source height.
        The output display width is the output height * film AR.
@@ -705,16 +712,14 @@ void hb_set_anamorphic_size( hb_job_t * job,
     pixel_aspect_width = height * source_display_width / cropped_height;
     pixel_aspect_height = width;
 
-    /* While x264 is smart enough to reduce fractions on its own, libavcodec
-       needs some help with the math, so lose superfluous factors.            */
-    hb_reduce( &pixel_aspect_width, &pixel_aspect_height,
-               pixel_aspect_width, pixel_aspect_height );
-
     /* Pass the results back to the caller */
     *output_width = width;
     *output_height = height;
-    *output_par_width = pixel_aspect_width;
-    *output_par_height = pixel_aspect_height;
+
+    /* While x264 is smart enough to reduce fractions on its own, libavcodec
+       needs some help with the math, so lose superfluous factors.            */
+    hb_reduce( output_par_width, output_par_height,
+               pixel_aspect_width, pixel_aspect_height );
 }
 
 /**
@@ -723,14 +728,14 @@ void hb_set_anamorphic_size( hb_job_t * job,
  * @param aspect Desired aspect ratio. Value of -1 uses title aspect.
  * @param pixels Maximum desired pixel count.
  */
-void hb_set_size( hb_job_t * job, int aspect, int pixels )
+void hb_set_size( hb_job_t * job, double aspect, int pixels )
 {
     hb_title_t * title = job->title;
 
     int croppedWidth  = title->width - title->crop[2] - title->crop[3];
     int croppedHeight = title->height - title->crop[0] - title->crop[1];
-    int croppedAspect = title->aspect * title->height * croppedWidth /
-                            croppedHeight / title->width;
+    double croppedAspect = title->aspect * title->height * croppedWidth /
+                           croppedHeight / title->width;
     int addCrop;
     int i, w, h;
 
@@ -795,7 +800,7 @@ void hb_set_size( hb_job_t * job, int aspect, int pixels )
     for( i = 0;; i++ )
     {
         w = 16 * i;
-        h = MULTIPLE_16( w * HB_ASPECT_BASE / aspect );
+        h = MULTIPLE_16( (int)( (double)w / aspect ) );
         if( w * h > pixels )
         {
             break;
@@ -803,7 +808,7 @@ void hb_set_size( hb_job_t * job, int aspect, int pixels )
     }
     i--;
     job->width  = 16 * i;
-    job->height = MULTIPLE_16( 16 * i * HB_ASPECT_BASE / aspect );
+    job->height = MULTIPLE_16( (int)( (double)job->width / aspect ) );
 }
 
 /**
diff --git a/libhb/hb.h b/libhb/hb.h
index a47e4e2e5..8bfd9a144 100644
--- a/libhb/hb.h
+++ b/libhb/hb.h
@@ -90,7 +90,7 @@ int hb_detect_comb( hb_buffer_t * buf, int width, int height, int color_equal, i
 
 void          hb_get_preview( hb_handle_t *, hb_title_t *, int,
                               uint8_t * );
-void          hb_set_size( hb_job_t *, int ratio, int pixels );
+void          hb_set_size( hb_job_t *, double ratio, int pixels );
 void          hb_set_anamorphic_size( hb_job_t *,
                 int *output_width, int *output_height,
                 int *output_par_width, int *output_par_height);
diff --git a/libhb/scan.c b/libhb/scan.c
index 1120d883d..5cc0b414a 100644
--- a/libhb/scan.c
+++ b/libhb/scan.c
@@ -26,18 +26,15 @@ static int  DecodePreviews( hb_scan_t *, hb_title_t * title );
 static void LookForAudio( hb_title_t * title, hb_buffer_t * b );
 static int  AllAudioOK( hb_title_t * title );
 
-static const char *aspect_to_string( int aspect )
+static const char *aspect_to_string( double aspect )
 {
-    switch ( aspect )
+    switch ( (int)(aspect * 9.) )
     {
-        case HB_ASPECT_BASE * 1 / 1:    return "1:1";
-        case HB_ASPECT_BASE * 4 / 3:    return "4:3";
-        case HB_ASPECT_BASE * 16 / 9:   return "16:9";
-        case HB_ASPECT_BASE * 221 / 100:   return "2.21:1";
+        case 9 * 4 / 3:    return "4:3";
+        case 9 * 16 / 9:   return "16:9";
     }
     static char arstr[32];
-    double a = (double)aspect / HB_ASPECT_BASE;
-    sprintf( arstr, aspect >= 1.? "%.2f:1" : "1:%.2f", a );
+    sprintf( arstr, aspect >= 1.? "%.2f:1" : "1:%.2f", aspect );
     return arstr;
 }
 
@@ -198,15 +195,11 @@ static void ScanFunc( void * _data )
             job->pixel_aspect_height = title->pixel_aspect_height;
         }
 
-        if( title->aspect == 16 && !job->pixel_aspect_width && !job->pixel_aspect_height)
+        if( title->aspect != 0 && title->aspect != 1. &&
+            !job->pixel_aspect_width && !job->pixel_aspect_height)
         {
             hb_reduce( &job->pixel_aspect_width, &job->pixel_aspect_height,
-                       16 * title->height, 9 * title->width );
-        }
-        else if( !job->pixel_aspect_width && !job->pixel_aspect_height )
-        {
-            hb_reduce( &job->pixel_aspect_width, &job->pixel_aspect_height,
-                       4 * title->height, 3 * title->width );
+                       (int)(title->aspect * title->height), title->width );
         }
 
         job->width = title->width - job->crop[2] - job->crop[3];
@@ -670,16 +663,29 @@ skip_preview:
 
         // compute the aspect ratio based on the storage dimensions and the
         // pixel aspect ratio (if supplied) or just storage dimensions if no PAR.
-        title->aspect = ( (double)title->width / (double)title->height + 0.05 ) *
-                        HB_ASPECT_BASE;
-
-        double aspect = (double)title->width / (double)title->height;
+        title->aspect = (double)title->width / (double)title->height;
         if( title->pixel_aspect_width && title->pixel_aspect_height )
         {
-            aspect *= (double)title->pixel_aspect_width /
-                      (double)title->pixel_aspect_height;
+            title->aspect *= (double)title->pixel_aspect_width /
+                             (double)title->pixel_aspect_height;
+
+            // For unknown reasons some French PAL DVDs put the original
+            // content's aspect ratio into the mpeg PAR even though it's
+            // the wrong PAR for the DVD. Apparently they rely on the fact
+            // that DVD players ignore the content PAR and just use the
+            // aspect ratio from the DVD metadata. So, if the aspect computed
+            // from the PAR is different from the container's aspect we use
+            // the container's aspect & recompute the PAR from it.
+            if( title->container_aspect && title->aspect != title->container_aspect )
+            {
+                hb_log("scan: content PAR gives wrong aspect %.2f; "
+                       "using container aspect %.2f", title->aspect,
+                       title->container_aspect );
+                title->aspect = title->container_aspect;
+                hb_reduce( &title->pixel_aspect_width, &title->pixel_aspect_height,
+                           (int)(title->aspect * title->height), title->width );
+            }
         }
-        title->aspect = ( aspect + 0.05 ) * HB_ASPECT_BASE;
 
         // don't try to crop unless we got at least 3 previews
         if ( crops->n > 2 )
diff --git a/scripts/tst.aspect b/scripts/tst.aspect
new file mode 100755
index 000000000..64d58d7b7
--- /dev/null
+++ b/scripts/tst.aspect
@@ -0,0 +1,34 @@
+#!/bin/tcsh
+#
+# generate aspect ratio & cropping regression test data
+# from a set of HandBrake input files
+#
+# usage: tst.aspect [file ...]
+#
+# if no file names are supplied a default set of inputs is used (see the
+# variable 'inputs' below). Each file is encoded multiple times using
+# different options each time. The options to use are the elements of
+# the 'options' variable below.
+#
+# One line is printed for each HB run. It has the input dimensions,
+# output dimensions, crop, PAR, filename & options for the encode.
+# Since PAR is only output for anamorphic encodes, an omitted PAR
+# is indicated by "1:1" (to distinguish it from the explicit PAR "1/1").
+
+set options=('-w 480' '-l 368' '-p' '-P')
+
+if ($#argv) then
+    set inputs=($argv:q)
+else
+    set inputs=(~/Movies/DVD/* ~/tst/*.{ts,mpg,mkv,avi,vob})
+endif
+
+foreach i ($inputs:q)
+    foreach o ($options:q)
+        (sleep 5; echo q) | ./HandBrakeCLI -v -L -i "$i" -o /dev/null -f mp4 -e x264 $o |& \
+        awk -v fnm="$i" -v opts="$o" '/ storage dimensions: / { dimen = $5 "*" $7 " -> " $9 "*" $11 " " $13 }\
+        $3=="dimensions:" { dimen = $4 "*" $6 " -> " $8 "*" $10 " " $12 }\
+        / pixel aspect ratio: / { par=$6 "/" $8 }\
+        /encx264: opening libx264/ { if(! par) par="1:1";printf "%s %s  %s %s\n", dimen, par, fnm, opts }'
+    end
+end
diff --git a/test/test.c b/test/test.c
index fec091a93..3fd9d0e62 100644
--- a/test/test.c
+++ b/test/test.c
@@ -310,7 +310,7 @@ static void PrintTitleInfo( hb_title_t * title )
              title->hours, title->minutes, title->seconds );
     fprintf( stderr, "  + size: %dx%d, aspect: %.2f, %.3f fps\n",
              title->width, title->height,
-             (float) title->aspect / HB_ASPECT_BASE,
+             (float) title->aspect,
              (float) title->rate / title->rate_base );
     fprintf( stderr, "  + autocrop: %d/%d/%d/%d\n", title->crop[0],
              title->crop[1], title->crop[2], title->crop[3] );