[x265] [PATCH] cli: refactor handling of input file parameters

[Steve Borho]

On Tue, Feb 25, 2014 at 8:57 AM, dave <dtyx265 at gmail.com> wrote:
> On 02/24/2014 09:03 PM, Steve Borho wrote:
>
> # HG changeset patch
> # User Steve Borho <steve at borho.org>
> # Date 1393303499 21600
> #      Mon Feb 24 22:44:59 2014 -0600
> # Node ID dfc6889247da48d658dfe124d5dcda817b3447b3
> # Parent  6214d2609ea806408ed18d92f5f95acad8c8c5d1
> cli: refactor handling of input file parameters
>
> * allow y4m header to provide aspect ratio
> * allow user to override Y4M aspect ratio or frame rate
> * allow user to provide data possibly missing from Y4M header
> * do not clamp framesToBeEncoded to predicted file size (stop at EOF)
>
> diff -r 6214d2609ea8 -r dfc6889247da source/common/param.cpp
> --- a/source/common/param.cpp Mon Feb 24 22:45:15 2014 -0600
> +++ b/source/common/param.cpp Mon Feb 24 22:44:59 2014 -0600
> @@ -556,10 +556,10 @@
>      }
>      OPT("extended-sar")
>      {
> -        p->bEnableVuiParametersPresentFlag = 1;
> -        p->bEnableAspectRatioIdc = 1;
> -        p->aspectRatioIdc = X265_EXTENDED_SAR;
> -        bError |= sscanf(value, "%dx%d", &p->sarWidth, &p->sarHeight) != 2;
> +        int width, height;
> +        bError |= sscanf(value, "%dx%d", &width, &height) != 2;
> +        if (!bError)
> +            setParamAspectRatio(p, width, height);
>      }
>      OPT("overscan")
>      {
> @@ -751,6 +751,44 @@
>  namespace x265 {
>  // internal encoder functions
>
> +void setParamAspectRatio(x265_param *p, int width, int height)
> +{
> +    const int fixedRatios[][2] =
> +    {
> +        { 1,  1 },
> +        { 12, 11 },
> +        { 10, 11 },
> +        { 16, 11 },
> +        { 40, 33 },
> +        { 24, 11 },
> +        { 20, 11 },
> +        { 32, 11 },
> +        { 80, 33 },
> +        { 18, 11 },
> +        { 15, 11 },
> +        { 64, 33 },
> +        { 160, 99 },
> +        { 4, 3 },
> +        { 3, 2 },
> +        { 2, 1 },
> +
> +        { 0, 0 }
> +    };
> +    p->bEnableVuiParametersPresentFlag = 1;
> +    p->bEnableAspectRatioIdc = 1;
> +    p->aspectRatioIdc = X265_EXTENDED_SAR;
> +    p->sarWidth = width;
> +    p->sarHeight = height;
> +    for (int i = 0; fixedRatios[i][0]; i++)
> +    {
> +        if (width == fixedRatios[i][0] && height == fixedRatios[i][1])
> +        {
> +            p->aspectRatioIdc = i + 1;
> +            return;
> +        }
> +    }
> +}
> +
>
> What's wrong with using parseName for aspectRatio?

I originally had it written this way, it called x265_param_parse(p,
"sar", buf) after sprintf'ing the width and height to a string, but
doing an sprintf there seemed quite ugly.

In the most recent patch, there are set and get methods because the
CLI needs to get the SAR ratio from the param structure multiple times
to merge the values read from the Y4M header and report the results.
Getting the SAR from the string table is also messy, but using this
table makes it straight-forward.

I'll mail the updated patch so you can see what I mean.

-- 
Steve Borho