Opened 12 years ago

Closed 11 years ago

#1082 closed defect (fixed)

Missing left shift on offset in merge evaluation in SAO

Reported by: karlsharman Owned by:
Priority: minor Milestone: HM-12.1
Component: HM Version: HM-8.0
Keywords: Cc: fbossen, ksuehring, davidf, jct-vc@…

Description

merge_iOffset in TEncSampleAdaptiveOffset.cpp needs to be shifted left by m_auiSaoBitIncrease[channel] in saoComponentParamDist and sao2ChromaParamDist.

Change History (4)

comment:1 Changed 12 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

comment:2 Changed 12 years ago by davidf

I can confirm that this is an issue. It is related to contributions L0189 which reports a loss of expected benefit when coding an 8bit sequence at 10, 12 and 14 bits; and to J0335 which purportedly resolved the issue by adjusting the inverse quantisation of the luma SAO offset.

With SAO off the benefits progress as follows (to reduce clutter, i shall just show a single sequence). Anchor = --SAO=0 --InternalBitDepth=8

config     sequence                      Bitrate-PSNR(Y')  Bitrate-PSNR(Cb)  Bitrate-PSNR(Cr)
ld_main10  ParkScene_1920x1080_24        -2.22             -4.38             -6.98
ld_main12  ParkScene_1920x1080_24        -2.2              -4.69             -7.62
ld_main14  ParkScene_1920x1080_24        -2.26             -4.65             -8.35

With SAO on, we find: Anchor = --SAO=1 --InternalBitDepth=8

config     sequence                      Bitrate-PSNR(Y')  Bitrate-PSNR(Cb)  Bitrate-PSNR(Cr)
ld_main10  ParkScene_1920x1080_24        -1.73             -3.53             -6.27
ld_main12  ParkScene_1920x1080_24        -0.3              -3.31             -6.41
ld_main14  ParkScene_1920x1080_24        0.08              -3.17             -6.62

NB, to address a question that has been asked before, in all cases, performance is higher when comparing --SAO=1 against --SAO=0 at the same internal bitdepth.

Previous bitstream examinations by Karl using the instrumented HM+Rext decoder have shown that the cause in loss of expected benefit occurs because SAO starts failing to merge with previous parameter sets.

Investigation has shown that in the vanilla HM, when computing the distortion for the merge decision, the quantised offset value is used instead of the inverse quantised one. This obviously has no effect for 8 and 10bit configurations as there is no quantisation. However for 12 and 14bit there is an effect. This seems to have been introduced in J0355 r2614.

After applying the fix in the enclosed patch, the performance becomes: Anchor = --SAO=1 --InternalBitDepth=8 (no-fix)

config     sequence                      Bitrate-PSNR(Y')  Bitrate-PSNR(Cb)  Bitrate-PSNR(Cr)
ld_main10  BQTerrace_1920x1080_60        -1.4              -6.37             -11.44
ld_main10  BasketballDrive_1920x1080_50  -3.21             -7.4              -4.41
ld_main10  Cactus_1920x1080_50           -1.95             -1.88             -2.23
ld_main10  Kimono1_1920x1080_24          -3.67             -5.42             -6.55
ld_main10  ParkScene_1920x1080_24        -1.73             -3.53             -6.27

config     sequence                      Bitrate-PSNR(Y')  Bitrate-PSNR(Cb)  Bitrate-PSNR(Cr)
ld_main12  BQTerrace_1920x1080_60        -1.61             -5.42             -9.97
ld_main12  BasketballDrive_1920x1080_50  -3.44             -7.16             -5.14
ld_main12  Cactus_1920x1080_50           -2.05             -1.48             -2.37
ld_main12  Kimono1_1920x1080_24          -3.79             -5.15             -7.04
ld_main12  ParkScene_1920x1080_24        -1.74             -3.7              -6.07

config     sequence                      Bitrate-PSNR(Y')  Bitrate-PSNR(Cb)  Bitrate-PSNR(Cr)
ld_main14  BQTerrace_1920x1080_60        -1.46             -4.9              -10.15
ld_main14  BasketballDrive_1920x1080_50  -3.45             -6.63             -4.84
ld_main14  Cactus_1920x1080_50           -2.02             -1.75             -2.15
ld_main14  Kimono1_1920x1080_24          -3.88             -4.69             -6.63
ld_main14  ParkScene_1920x1080_24        -1.69             -3.69             -7.26

The following patch addresses the issue in the RExt branch:

  • source/Lib/TLibEncoder/TEncSampleAdaptiveOffset.cpp 

    commit 3e1e829b2232536db925a3500695c15a9c0b721b
Author: David Flynn <dflynn@rim.com>
Date:   Thu May 2 01:20:19 2013 -0400

    WIP: possible SAO merge distortion cost fix

diff --git a/source/Lib/TLibEncoder/TEncSampleAdaptiveOffset.cpp b/source/Lib/TLibEncoder/TEncSampleAdaptiveOffset.cpp
index bfa615f..fcd95e1 100644
    a b Void TEncSampleAdaptiveOffset::saoComponentParamDist(Int allowMergeLeft, Int all 
    24232423        Int   merge_iOffset;
    24242424        for(classIdx = 0; classIdx < m_iNumClass[typeIdx]; classIdx++)
    24252425        {
    2426           merge_iOffset = saoLcuParamNeighbor->offset[classIdx];
     2426          Int saoBitIncrease = m_auiSaoBitIncrease[isLuma(yCbCr)?0:1];
     2427          merge_iOffset = saoLcuParamNeighbor->offset[classIdx] << saoBitIncrease;
    24272428          estDist   += estSaoDist(m_iCount [yCbCr][typeIdx][classIdx+mergeBandPosition+1], merge_iOffset, m_iOffsetOrg[yCbCr][typeIdx][classIdx+mergeBandPosition+1],  shift);
    24282429        }
    24292430      }
    … … Void TEncSampleAdaptiveOffset::sao2ChromaParamDist(Int allowMergeLeft, Int allow 
    26432644          Int   merge_iOffset;
    26442645          for(classIdx = 0; classIdx < m_iNumClass[typeIdx]; classIdx++)
    26452646          {
    2646             merge_iOffset = saoLcuParamNeighbor[compIdx]->offset[classIdx];
     2647            Int saoBitIncrease = m_auiSaoBitIncrease[1];
     2648            merge_iOffset = saoLcuParamNeighbor[compIdx]->offset[classIdx] << saoBitIncrease;
    26472649            estDist[compIdx]   += estSaoDist(m_iCount [compIdx+1][typeIdx][classIdx+mergeBandPosition+1], merge_iOffset, m_iOffsetOrg[compIdx+1][typeIdx][classIdx+mergeBandPosition+1],  shift);
    26482650          }
    26492651        }

comment:3 Changed 11 years ago by karlsharman

Fixed in HM12.1 due to reworking of SAO.

comment:4 Changed 11 years ago by davidf

  • Milestone changed from HM-11.0 to HM-12.1
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.

This list contains all users that will be notified about changes made to this ticket.

These roles will be notified: Reporter, Owner, Subscriber, Participant

  • David Flynn(Subscriber, Participant)
  • Frank Bossen(Subscriber)
  • jct-vc@…(Subscriber)
  • Karl Sharman(Reporter, Participant)
  • karl.sharman@…(Always)
  • Karsten Suehring(Subscriber, Always)