[x265] [PATCH] Merged TEncCfg into Encoder. A few VUI tweaks and fixes

[Steve Borho]

On Thu, Mar 6, 2014 at 7:09 PM, dave <dtyx265 at gmail.com> wrote:
> Steve, just looking for some clarification here...
>
>
> diff -r 6662df480e39 -r 56fa912d6e7c source/Lib/TLibEncoder/TEncCu.h
> --- a/source/Lib/TLibEncoder/TEncCu.h   Mon Mar 03 11:28:22 2014 +0530
> +++ b/source/Lib/TLibEncoder/TEncCu.h   Mon Mar 03 14:24:34 2014 -0800
> @@ -111,7 +111,7 @@
>      TComYuv**    m_bestMergeRecoYuv;
>      TComYuv**    m_origYuv;     ///< Original Yuv at each depth
>
> -    TEncCfg*     m_cfg;
> +    Encoder*     m_cfg;
>
>> It's a bit of a layering violation to have a top-level encoder pointer
>> way down here in the guts.  But this can be cleaned up later.
>>
>> diff -r 6662df480e39 -r 56fa912d6e7c source/encoder/dpb.h
>> --- a/source/encoder/dpb.h      Mon Mar 03 11:28:22 2014 +0530
>> +++ b/source/encoder/dpb.h      Mon Mar 03 14:24:34 2014 -0800
>> @@ -32,7 +32,7 @@
>>   class FrameEncoder;
>>   class TComPic;
>>   class TComSlice;
>> -class TEncCfg;
>> +class Encoder;
>>
>>   class DPB
>>   {
>> @@ -41,18 +41,18 @@
>>       int                m_lastIDR;
>>       int                m_pocCRA;
>>       bool               m_bRefreshPending;
>> -    TEncCfg*           m_cfg;
>> +    Encoder*           m_cfg;
>> Can we just pass a param to dpb?
>
> I just sent a patch for this one.  After altering lowres, DPB only uses
> bOpenGOP so it could either be made a member of DPB or passed instead of
> x265_param*.  I thought in the future x265_param* would be more useful but
> developers more familiar with DPB and h265 know better them what's best.  I
> can submit a patch for whatever is best.

passing bOpenGOP to the constructor would be cleaner I suppose.

>> diff -r 6662df480e39 -r 56fa912d6e7c source/encoder/framefilter.h
>> --- a/source/encoder/framefilter.h      Mon Mar 03 11:28:22 2014 +0530
>> +++ b/source/encoder/framefilter.h      Mon Mar 03 14:24:34 2014 -0800
>> @@ -57,7 +57,7 @@
>>   protected:
>>
>>       Encoder*                    m_top;
>>
>> -    TEncCfg*                    m_cfg;
>> +    Encoder*                    m_cfg;
>> ditto here and in ratecontrol.cpp and slicetype.cpp.  it would be
>> greatly preferred to just pass param to these files instead of Encoder
>>
> From your "layering violation" comment, I guess you would like TEncCu added
> to this list.

Yes

> Do you want Encoder* m_cfg/m_top replaced with x265_param* m_param as a
> member or would you prefer x265_param* passed as an argument to the methods
> of ratecontrol.cpp, slicetype.cpp, framefilter and TEncCu...?

Giving them all const x265_param * member variables is probably better
than adding function arguments.

-- 
Steve Borho