[x265] [PATCH] motion: set search range correctly for bidir prediction

[Steve Borho]

On 06/07, deepthi at multicorewareinc.com wrote:
> # HG changeset patch
> # User Deepthi Nandakumar <deepthi at multicorewareinc.com>
> # Date 1433696064 -19800
> #      Sun Jun 07 22:24:24 2015 +0530
> # Node ID 45ca4cb5bc769a54630e7715212295cc86768c70
> # Parent  2c5d6a1825389e052badbb46e3b4fdfe3b65aa48
> motion: set search range correctly for bidir prediction
> 
> diff -r 2c5d6a182538 -r 45ca4cb5bc76 source/encoder/analysis.cpp
> --- a/source/encoder/analysis.cpp	Fri Jun 05 08:36:51 2015 -0700
> +++ b/source/encoder/analysis.cpp	Sun Jun 07 22:24:24 2015 +0530
> @@ -1713,17 +1713,15 @@
>      bool bTryZero = bestME[0].mv.notZero() || bestME[1].mv.notZero();
>      if (bTryZero)
>      {
> -        /* Do not try zero MV if unidir motion predictors are beyond
> -         * valid search area */
> -        MV mvmin, mvmax;
> -        int merange = X265_MAX(m_param->sourceWidth, m_param->sourceHeight);
> -        setSearchRange(cu, mvzero, merange, mvmin, mvmax);
> -        mvmax.y += 2; // there is some pad for subpel refine
> -        mvmin <<= 2;
> -        mvmax <<= 2;
> +        /* Do not try zero MV if unidir motion predictors are beyond valid search area */
> +        MV mvmin0, mvmax0, mvmin1, mvmax1;
> +        setSearchRange(cu, bestME[0].mvp, m_param->searchRange, mvmin0, mvmax0);
> +        setSearchRange(cu, bestME[1].mvp, m_param->searchRange, mvmin1, mvmax1);
> +        mvmin0 <<= 2; mvmax0 <<= 2;
> +        mvmin1 <<= 2; mvmax1 <<= 2;
>  
> -        bTryZero &= bestME[0].mvp.checkRange(mvmin, mvmax);
> -        bTryZero &= bestME[1].mvp.checkRange(mvmin, mvmax);
> +        bTryZero &= bestME[0].mvp.checkRange(mvmin0, mvmax0);
> +        bTryZero &= bestME[1].mvp.checkRange(mvmin1, mvmax1);

technically we only need a single mvmin/mvmax range per PU (origin),
which describes the full extent of the available pixels in the reference
frames. I can see why sourceWidth/sourceHeight were used (to get the
range of all possibly available pixels).

param->searchRange is generally meant to be relative from the MVP, but
for bidir we are not actually doing any searching, we just need to know
that the pixels at MVP are available.

But the 'mvmax.y += 2' expression is confounding me. setSearchRange() is
not subtracting 2 from mvmax.y. In frameencoder.cpp the reference row
lag calculation is adding two rows of pixels (for subpel refine steps)
beyond param->searchRange which one might think could be reclaimed here,
but this is definitely *not* the case. The number of available pixels in
the Y direction is always defined by the number of reference lag rows.

This looks like a bug, but I would expect it to have resulted in hash
mismatches, but we haven't been seeing those. Which means we probably
have more rows available than we generally use.

>      }
>      if (bTryZero)
>      {
> diff -r 2c5d6a182538 -r 45ca4cb5bc76 source/encoder/search.cpp
> --- a/source/encoder/search.cpp	Fri Jun 05 08:36:51 2015 -0700
> +++ b/source/encoder/search.cpp	Sun Jun 07 22:24:24 2015 +0530
> @@ -2214,17 +2214,15 @@
>              bool bTryZero = bestME[0].mv.notZero() || bestME[1].mv.notZero();
>              if (bTryZero)
>              {
> -                /* Do not try zero MV if unidir motion predictors are beyond
> -                 * valid search area */
> -                MV mvmin, mvmax;
> -                int merange = X265_MAX(m_param->sourceWidth, m_param->sourceHeight);
> -                setSearchRange(cu, mvzero, merange, mvmin, mvmax);
> -                mvmax.y += 2; // there is some pad for subpel refine
> -                mvmin <<= 2;
> -                mvmax <<= 2;
> -
> -                bTryZero &= bestME[0].mvp.checkRange(mvmin, mvmax);
> -                bTryZero &= bestME[1].mvp.checkRange(mvmin, mvmax);
> +                /* Do not try zero MV if unidir motion predictors are beyond valid search area */
> +                MV mvmin0, mvmax0, mvmin1, mvmax1;
> +                setSearchRange(cu, bestME[0].mvp, m_param->searchRange, mvmin0, mvmax0);
> +                setSearchRange(cu, bestME[1].mvp, m_param->searchRange, mvmin1, mvmax1);
> +                mvmin0 <<= 2; mvmax0 <<= 2;
> +                mvmin1 <<= 2; mvmax1 <<= 2;
> +
> +                bTryZero &= bestME[0].mvp.checkRange(mvmin0, mvmax0);
> +                bTryZero &= bestME[1].mvp.checkRange(mvmin1, mvmax1);

we don't need to re-calculate the same mvmin/mvmax here.

>              }
>              if (bTryZero)
>              {
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho