On Mon, Mar 17, 2014 at 6:26 PM, dave <[email protected]> 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 _______________________________________________ x265-devel mailing list [email protected] https://mailman.videolan.org/listinfo/x265-devel
