[x264-devel] [PATCH] fix bug of weighted bipred according to Rec. ITU-T H.264 (10/2016) 8.4.3

[BugMaster]

On Thu, 21 Jan 2021 12:00:25 +0800, Lingjiang Fang wrote:
> On Thu, 21 Jan 2021 00:02:18 +0300
> BugMaster <BugMaster at narod.ru> wrote:

>> On Wed, 20 Jan 2021 10:59:39 +0300, BugMaster wrote:
>> > On Wed, 20 Jan 2021 11:55:14 +0800, Lingjiang Fang wrote:
>> >> On Wed, 20 Jan 2021 00:51:11 +0300
>> >> BugMaster <BugMaster at narod.ru> wrote:
>> 
>> >>> ...
>> >>> Hi.
>> >>> 
>> >>> I understand which part of H.264 spec you tried to fix. But was
>> >>> you able to really trigger this bug (code path) in x264? I in my
>> >>> quick test I couldn't because x264 does not use long-term refs
>> >>> and I don't know when poc0 and poc1 would be equal for B-frame
>> >>> (which frame-types pattern will cause this?).
>> 
>> >> thank you for your reply ^_^
>> >> Yes, we will never trigger this bug under the current logic of
>> >> x264, we notice this because we want do some enhancement to the
>> >> reference management, so we need to fix it first.
>> >> anyway, since the implementation doesn't follow the spec and it's
>> >> easy to fix it, why not ^_^
>> 
>> > Ok, understood. But I have rewritten it another way:
>> > https://code.videolan.org/BugMaster/x264/-/commit/60dc14afb103cbbbdd79d643c4746ca95ba86b22
>> > So check that I didn't miss anything and your enhancement will be ok
>> > with it.
>> 
>> I have created MR with this fix (with small changes):
> thanks, your MR is much more clear
> however, I wonder why you revert the small change of completing the
> comment below :)
> if( td == 0 /* || pic0 or pic1 is a long-term ref */ )

I moved comment about "pic1 is not a long-term ref" lower because
while 8.4.3 talks about both pic0 or pic1 for w0C = 32, w1C = 32 but
8.4.1.2.3 mention only refIdxL0 (pic0) for DistScaleFactor formulas
(8-195) and (8-196). I.e. we need different conditions for
dist_scale_factor_buf and bipred_weight_buf in this case.