[x265] H265 classes in TComSlice.h

[Steve Borho]

On Mon, Mar 17, 2014 at 6:26 PM, dave <dtyx265 at gmail.com> wrote:
> Hi All,
>
> I have been working on a file, h265.h, of classes to replace the H265
> classes in TComSlice.h.  I would like some feedback on a few things before I
> submit it for review.
>
> First, a decent number of fields either have default values or are optional
> and are not currently added to encoded video generated by x265.  I am
> duplicating the functionality of what is in TComSlice.h while cutting out
> its cruft with the goal of not breaking current x265 code.  Please remember
> this as some of the code that I will be submitting probably doesn't look to
> good as it is meant to replace HM code without breaking x265 code.  As
> functionality is added to x265 it should be easier to work with the unused
> fields.
>
> Some of the TComSlice.h H265 classes have fields that are not part of their
> equivalent H265 data structures but are used in the encoding process.  Some
> are duplicates of what is in Encoder and could be used from there.  Others
> could be moved somewhere else.  Or they could just be used as they currently
> are.  My preference would be for H265 classes to only have H265 fields.
>
> Generally, I have been naming/renaming fields after what they are in the
> H265 while adding x265 naming convention standards.  Basically, m_<H265
> field>.
>
> Some fields are obviously booleans and have the form <H265 field>Flag which
> makes adding the boolean m_b seem a bit redundant.  I could drop the b or
> Flag, my preference is to drop the b.

This might be ok for these classes that describe NALs since the bool
is actually describing a single bit flag, and the H265 names generally
have Flag in them.

> Some fields had to be renamed because the HM code called them something
> slightly different.  Most of the time this renaming simply shortened the
> name in some way, a few almost seem to be obfuscating something.  Generally
> I prefer to stick to the H265 standard for clarity.
>
> Having said that, not all of the H265 names are all that clear to begin
> with.  fpsNum and fpsDenom being one example.  My preference would be to use
> H265 names but if unclear, then document in comments a more clear
> definition.
>
>    /* Numerator of frame rate(fpsNum) */
>     uint32_t  numUnitsInTick;
>
>    /* Denominator of frame rate(fpsDenom) */
>     uint32_t  timeScale;

This all sounds pretty reasonable but I think in this particular
instance the H265 name is more correct.  The timeScale we signal
*could* in theory be different than fpsDenom.  In fact I think x264
does this whenever variable frame rate is in use.  So keeping the H265
name is a good rule of thumb.

> The obvious alternative would be to have the documentation contain the H265
> name and use the alternative field name for the variable.
>
> I debated putting h265.h in source/common or source/encoder. Currently I
> have it in common.

The rough equivalent header in x264 is common/set.h

> Currently, I have classes for VPS, SPS, PPS, VUI, HRD, Window, TimingInfo,
> SubLayerOrderingInfo and HrdSubLayerInfo.
>
> Todo classes are PTL(ProfileTierLevel), ScalingList and RPSList.  If
> anything else needs to be added let me know.
>
> The TComSlice class looks pretty big.  I haven't had a chance to go over it
> yet but if it isn't mostly cruft then it probably should have its own file
> or perhaps be merged into a current x265 file.

The function which encodes a slice header isn't terribly big so I
imagine a large amount of encoder functionality has been grafted onto
TComSlice.

-- 
Steve Borho