[x265] [PATCH] modify for BFrames in ABR , clean up code

[Aarthi Priya Thirumalai]

Thanks for comments. Noted. Adding the correction suggested and a few more
tweaks in the next commit.

--Aarthi


On Thu, Sep 12, 2013 at 3:28 PM, Derek Buitenhuis <
derek.buitenhuis at gmail.com> wrote:

> On 9/11/2013 5:31 PM, aarthi at multicorewareinc.com wrote:
> > # HG changeset patch
> > # User Aarthi Thirumalai
> > # Date 1378916973 -19800
> > #      Wed Sep 11 21:59:33 2013 +0530
> > # Node ID 823b35128a7dcbd77f0d4c274a285d63deba7c5a
> > # Parent  a6bf1f10e820da79da76b4649eaae11b6a030521
> > modify for BFrames in ABR , clean up code
>
> Perhaps:
>
>     Improve bframe ratecontrol logic
>
>     This should improve ABR results by <...>.
>
> Where <...> is an explanation on how it improves it. I understand
> it's mostly tweaking magic numbers.
>
> > +        TComSlice* prevRefSlice = curFrame->getRefPic(REF_PIC_LIST_0,
> 0)->getSlice();
> > +        TComSlice* nextRefSlice = curFrame->getRefPic(REF_PIC_LIST_1,
> 0)->getSlice();
> > +        int i0 = prevRefSlice->getSliceType() == I_SLICE;
> > +        int i1 = nextRefSlice->getSliceType() == I_SLICE;
>
> bool? Or do we intentionally stick to ints to make a C port
> easier later on?
>
> > +        if (prevRefSlice->getSliceType()  == B_SLICE &&
> prevRefSlice->isReferenced())
> > +            q0 -= pbOffset / 2;
> > +        if (nextRefSlice->getSliceType()  == B_SLICE &&
> nextRefSlice->isReferenced())
> > +            q1 -= pbOffset / 2;
>
> Accidental extra whitespace before the ==.
>
> > -        lastQScaleFor[P_SLICE] = lastQScale = qScale/pbFactor;
>
> Can you document the reason for removing this in the commit message?
>
> > -            cplxrSum +=  1.1 *bits * qp2qScale(rce->qpaRc) /
> rce->lastRceq;
> > +            /* The factor 1.5 is to tune up the actual bits  otherwise
> the cplxrSum is scaled too low
> > +             * to improve short term compensation for next frame. */
> > +            cplxrSum +=  1.5 * bits * qp2qScale(rce->qpaRc) /
> rce->lastRceq;
>
> Extra space after 'bits', where a comma should be.
>
> Also, s/the cplxrSum/cplxrSum/.
>
> > -            cplxrSum += bits * qp2qScale(rce->qpaRc) / (rce->lastRceq *
> fabs(0.5 * pbFactor));
> > +            cplxrSum += bits * qp2qScale(rce->qpaRc) / (rce->lastRceq *
> fabs(pbFactor));
>
> I assume this is related to the above tweak, i.e. a natrual consequence of
> increasing the coefficient cplxrSum uses?
>
> Technically, seems OK.
>
> - Derek
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20130912/947e9bad/attachment-0001.html>