video_filter/canvas: various aspect-ratio related fixes

video_filter/canvas: various aspect-ratio related fixes

 - Remove silly defaulting to 4:3.  Instead, compute PAR based
   upon SAR of input video and canvas size.
 - Update documentation to remove ambiguity over what is meant
   by aspect-ratio
 - Fix subpicture calculations to
    (a) be much simpler,
    (b) correctly calculate width and height
Signed-off-by: Ilkka Ollakka <ileoo@videolan.org>
(cherry picked from commit c2752f81)

modules/video_filter/canvas.c

@@ -44,16 +44,54 @@ static void Destroy( vlc_object_t * );
 static picture_t *Filter( filter_t *, picture_t * );
 static int alloc_init( filter_t *, void * );
 
-#define WIDTH_TEXT N_( "Image width" )
+/* This module effectively implements a form of picture-in-picture.
+ *  - The outer picture is called the canvas.
+ *  - The innter picture is callsed the subpicture.
+ *
+ * NB, all of the following operatons take into account aspect ratio
+ *
+ * A canvas is of canvas_{width,height}.
+ * In Pad mode:
+ *  - The subpicture is upconverted with a inverse scalefactor of:
+ *     (The size of subpicture's largest dimension)
+ *     --------------------------------------------
+ *     (The size of canvas's equivalent dimension)
+ *
+ *   Ie, The subpicture's largest dimension is made equal to the
+ *   equivalent canvas dimension.
+ *
+ *  - The upconverted subpicture's smallest dimension is then padded
+ *    to make the upconverted subpicture have the same dimensions of
+ *    the canvas.
+ *
+ * In Crop mode:
+ *  - The subpicture is upconverted with an inverse scalefactor of:
+ *     (The size of subpicture's smallest dimension)
+ *     --------------------------------------------
+ *     (The size of canvas's equivalent dimension)
+ *
+ *   Ie, The subpicture's smallest dimension is made equal to the
+ *   equivalent canvas dimension. (The subpicture will then be
+ *   larger than the canvas)
+ *
+ *  - The upconverted subpicture's largest dimension is then cropped
+ *    to make the upconverted subpicture have the same dimensions of
+ *    the canvas.
+ */
+
+/* NB, use of `padd' in this module is a 16-17th Century spelling of `pad' */
+
+#define WIDTH_TEXT N_( "Output width" )
 #define WIDTH_LONGTEXT N_( \
-    "Image width" )
-#define HEIGHT_TEXT N_( "Image height" )
+    "Output (canvas) image width" )
+#define HEIGHT_TEXT N_( "Output height" )
 #define HEIGHT_LONGTEXT N_( \
-    "Image height" )
-#define ASPECT_TEXT N_( "Aspect ratio" )
+    "Output (canvas) image height" )
+#define ASPECT_TEXT N_( "Output picture aspect ratio" )
 #define ASPECT_LONGTEXT N_( \
-    "Set aspect (like 4:3) of the video canvas" )
-#define PADD_TEXT N_( "Padd video" )
+    "Set the canvas' picture aspect ratio. " \
+    "If omitted, the canvas is assumed to have the same SAR as the input." )
+#define PADD_TEXT N_( "Pad video" )
 #define PADD_LONGTEXT N_( \
     "If enabled, video will be padded to fit in canvas after scaling. " \
     "Otherwise, video will be cropped to fix in canvas after scaling." )
@@ -64,7 +102,7 @@ static int alloc_init( filter_t *, void * );
  * Module descriptor
  *****************************************************************************/
 vlc_module_begin ()
-    set_description( N_("Automatically resize and padd a video") )
+    set_description( N_("Automatically resize and pad a video") )
     set_capability( "video filter2", 0 )
     set_callbacks( Activate, Destroy )
 
@@ -76,7 +114,7 @@ vlc_module_begin ()
     add_integer_with_range( CFG_PREFIX "height", 0, 0, INT_MAX, NULL,
                             HEIGHT_TEXT, HEIGHT_LONGTEXT, false )
 
-    add_string( CFG_PREFIX "aspect", "4:3", NULL,
+    add_string( CFG_PREFIX "aspect", NULL, NULL,
                 ASPECT_TEXT, ASPECT_LONGTEXT, false )
 
     add_bool( CFG_PREFIX "padd", true, NULL,
@@ -98,12 +136,13 @@ struct filter_sys_t
 static int Activate( vlc_object_t *p_this )
 {
     filter_t *p_filter = (filter_t *)p_this;
-    unsigned int i_width, i_height;
-    es_format_t fmt;
+    unsigned i_canvas_width; /* width of output canvas */
+    unsigned i_canvas_height; /* height of output canvas */
+    unsigned i_canvas_aspect; /* canvas PictureAspectRatio */
+    es_format_t fmt; /* target format after up/down conversion */
     char psz_croppadd[100];
     int i_padd,i_offset;
     char *psz_aspect, *psz_parser;
-    int i_aspect;
     bool b_padd;
 
     if( !p_filter->b_allow_fmt_out_change )
@@ -121,39 +160,49 @@ static int Activate( vlc_object_t *p_this )
     config_ChainParse( p_filter, CFG_PREFIX, ppsz_filter_options,
                        p_filter->p_cfg );
 
-    i_width = var_CreateGetInteger( p_filter, CFG_PREFIX "width" );
-    i_height = var_CreateGetInteger( p_filter, CFG_PREFIX "height" );
+    i_canvas_width = var_CreateGetInteger( p_filter, CFG_PREFIX "width" );
+    i_canvas_height = var_CreateGetInteger( p_filter, CFG_PREFIX "height" );
 
-    if( i_width == 0 || i_height == 0 )
+    if( i_canvas_width == 0 || i_canvas_height == 0 )
     {
         msg_Err( p_filter, "Width and height options must be set" );
         return VLC_EGENERIC;
     }
 
-    if( i_width & 1 || i_height & 1 )
+    if( i_canvas_width & 1 || i_canvas_height & 1 )
     {
+        /* If this restriction were ever relaxed, it is very important to
+         * get the field polatiry correct */
         msg_Err( p_filter, "Width and height options must be even integers" );
         return VLC_EGENERIC;
     }
 
     psz_aspect = var_CreateGetNonEmptyString( p_filter, CFG_PREFIX "aspect" );
-    if( !psz_aspect )
+    if( psz_aspect )
     {
-        msg_Err( p_filter, "Aspect ratio must be set" );
-        return VLC_EGENERIC;
+        psz_parser = strchr( psz_aspect, ':' );
+        int numerator = atoi( psz_aspect );
+        int denominator = psz_parser ? atoi( psz_parser+1 ) : 0;
+        denominator = denominator == 0 ? 1 : denominator;
+        i_canvas_aspect = numerator * VOUT_ASPECT_FACTOR / denominator;
+        free( psz_aspect );
+
+        if( numerator <= 0 || denominator < 0 )
+        {
+            msg_Err( p_filter, "Aspect ratio must be strictly positive" );
+            return VLC_EGENERIC;
+        }
     }
-    psz_parser = strchr( psz_aspect, ':' );
-    if( psz_parser ) psz_parser++;
-    if( psz_parser && atoi( psz_parser ) > 0 )
-        i_aspect = atoi( psz_aspect ) * VOUT_ASPECT_FACTOR / atoi( psz_parser );
     else
-        i_aspect = atof( psz_aspect ) * VOUT_ASPECT_FACTOR;
-    free( psz_aspect );
-
-    if( i_aspect <= 0 )
     {
-        msg_Err( p_filter, "Aspect ratio must be strictly positive" );
-        return VLC_EGENERIC;
+        /* if there is no user supplied aspect ratio, assume the canvas
+         * has the same sample aspect ratio as the subpicture */
+        /* aspect = subpic_sar * canvas_width / canvas_height
+         *  where subpic_sar = subpic_ph * subpic_par / subpic_pw */
+        i_canvas_aspect = (uint64_t) p_filter->fmt_in.video.i_height
+                        * p_filter->fmt_in.video.i_aspect
+                        * i_canvas_width
+                        / (i_canvas_height * p_filter->fmt_in.video.i_width);
     }
 
     b_padd = var_CreateGetBool( p_filter, CFG_PREFIX "padd" );
@@ -174,36 +223,42 @@ static int Activate( vlc_object_t *p_this )
 
     es_format_Copy( &fmt, &p_filter->fmt_in );
 
-    fmt.video.i_width = i_width;
-    fmt.video.i_height = ( p_filter->fmt_in.video.i_height * i_width )
-                         / p_filter->fmt_in.video.i_width;
-    fmt.video.i_height = ( fmt.video.i_height * i_aspect )
-                         / p_filter->fmt_in.video.i_aspect;
+    /* one dimension will end up with one of the following: */
+    fmt.video.i_width = i_canvas_width;
+    fmt.video.i_height = i_canvas_height;
 
     if( b_padd )
     {
         /* Padd */
-        if( fmt.video.i_height > i_height )
+        if( i_canvas_aspect > p_filter->fmt_in.video.i_aspect )
         {
-            fmt.video.i_height = i_height;
-            fmt.video.i_width = ( p_filter->fmt_in.video.i_width * i_height )
-                                / p_filter->fmt_in.video.i_height;
-            fmt.video.i_width = ( fmt.video.i_width * p_filter->fmt_in.video.i_aspect )
-                                / i_aspect;
+            /* The canvas has a wider aspect than the subpicture:
+             *  ie, pillarbox the [scaled] subpicture */
+            /* The following is derived form:
+             * width = upconverted_subpic_height * subpic_par / canvas_sar
+             *  where canvas_sar = canvas_width / (canvas_height * canvas_par)
+             * then simplify */
+            fmt.video.i_width = i_canvas_width
+                              * p_filter->fmt_in.video.i_aspect
+                              / i_canvas_aspect;
             if( fmt.video.i_width & 1 ) fmt.video.i_width -= 1;
 
-            i_padd = (i_width - fmt.video.i_width) / 2;
+            i_padd = (i_canvas_width - fmt.video.i_width) / 2;
             i_offset = (i_padd & 1);
-            /* Gruik */
             snprintf( psz_croppadd, 100, "croppadd{paddleft=%d,paddright=%d}",
                       i_padd - i_offset, i_padd + i_offset );
         }
         else
         {
+            /* The canvas has a taller aspect than the subpicture:
+             *  ie, letterbox the [scaled] subpicture */
+            fmt.video.i_height = i_canvas_height
+                               * i_canvas_aspect
+                               / p_filter->fmt_in.video.i_aspect;
             if( fmt.video.i_height & 1 ) fmt.video.i_height -= 1;
-            i_padd = (i_height - fmt.video.i_height ) / 2;
+
+            i_padd = (i_canvas_height - fmt.video.i_height ) / 2;
             i_offset = (i_padd & 1);
-            /* Gruik */
             snprintf( psz_croppadd, 100, "croppadd{paddtop=%d,paddbottom=%d}",
                       i_padd - i_offset, i_padd + i_offset );
         }
@@ -211,32 +266,40 @@ static int Activate( vlc_object_t *p_this )
     else
     {
         /* Crop */
-        if( fmt.video.i_height < i_height )
+        if( i_canvas_aspect < p_filter->fmt_in.video.i_aspect )
         {
-            fmt.video.i_height = i_height;
-            fmt.video.i_width = ( p_filter->fmt_in.video.i_width * i_height )
-                                / p_filter->fmt_in.video.i_height;
-            fmt.video.i_width = ( fmt.video.i_width * p_filter->fmt_in.video.i_aspect )
-                                / i_aspect;
+            /* The canvas has a narrower aspect than the subpicture:
+             *  ie, crop the [scaled] subpicture horizontally */
+            fmt.video.i_width = i_canvas_width
+                              * p_filter->fmt_in.video.i_aspect
+                              / i_canvas_aspect;
             if( fmt.video.i_width & 1 ) fmt.video.i_width -= 1;
 
-            i_padd = (fmt.video.i_width - i_width) / 2;
-            i_offset =  (i_padd & 1);
-            /* Gruik */
+            i_padd = (fmt.video.i_width - i_canvas_width) / 2;
+            i_offset = (i_padd & 1);
             snprintf( psz_croppadd, 100, "croppadd{cropleft=%d,cropright=%d}",
                       i_padd - i_offset, i_padd + i_offset );
         }
         else
         {
+            /* The canvas has a shorter aspect than the subpicture:
+             *  ie, crop the [scaled] subpicture vertically */
+            fmt.video.i_height = i_canvas_height
+                               * i_canvas_aspect
+                               / p_filter->fmt_in.video.i_aspect;
             if( fmt.video.i_height & 1 ) fmt.video.i_height -= 1;
-            i_padd = (fmt.video.i_height - i_height) / 2;
+
+            i_padd = (fmt.video.i_height - i_canvas_height) / 2;
             i_offset = (i_padd & 1);
-            /* Gruik */
             snprintf( psz_croppadd, 100, "croppadd{croptop=%d,cropbottom=%d}",
                       i_padd - i_offset, i_padd + i_offset );
         }
     }
 
+    /* xxx, should the clean area include the letter-boxing?
+     *  probably not, as some codecs can make use of that information
+     *  and it should be a scaled version of the input clean area
+     *   -- davidf */
     fmt.video.i_visible_width = fmt.video.i_width;
     fmt.video.i_visible_height = fmt.video.i_height;
 
@@ -249,16 +312,16 @@ static int Activate( vlc_object_t *p_this )
     fmt = *filter_chain_GetFmtOut( p_sys->p_chain );
     es_format_Copy( &p_filter->fmt_out, &fmt );
 
-    p_filter->fmt_out.video.i_aspect = i_aspect * i_width / i_height;
+    p_filter->fmt_out.video.i_aspect = i_canvas_aspect;
 
-    if( p_filter->fmt_out.video.i_width != i_width
-     || p_filter->fmt_out.video.i_height != i_height )
+    if( p_filter->fmt_out.video.i_width != i_canvas_width
+     || p_filter->fmt_out.video.i_height != i_canvas_height )
     {
         msg_Warn( p_filter, "Looks like something went wrong. "
                   "Output size is %dx%d while we asked for %dx%d",
                   p_filter->fmt_out.video.i_width,
                   p_filter->fmt_out.video.i_height,
-                  i_width, i_height );
+                  i_canvas_width, i_canvas_height );
     }
 
     p_filter->pf_video_filter = Filter;