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