On 02/03, Nicolas Morey-Chaisemartin wrote: > > On 02/02/2015 06:27 PM, Steve Borho wrote: > >On 02/02, Nicolas Morey-Chaisemartin wrote: > >># HG changeset patch > >># User Nicolas Morey-Chaisemartin <[email protected]> > >># Date 1414489826 -3600 > >># Tue Oct 28 10:50:26 2014 +0100 > >> > >>Make FrameEncoder partially virtual so it can be overloaded > >> > >>FrameEncoder is a logical place to use a hardware accelerator or an > >>alternative core encoder. > >>By making a few function virtual, it can be easily replaced by an > >>inheriting class, transparently for the Encoder. > >>This way all the RC/interaction code can stay within the FrameEncoder class > >>and reduce conflicts in later updates. > >I'm mostly fine with adding virtual declarations for these functions, > >but I'm curious why this requires the frame encoders to be allocated in > >seperate new() calls? This is accounting for most of the diffs. > > > >Somewhat related to this, I made a patch a couple of days ago that has > >yet to be pushed which added a hard-limit on the number of frame > >encoders (to 16). With this you could avoid mallocing the array of > >pointers, and just declare the max-sized array in the top level Encoder > >class (if individual pointers are required). > > > > > This is required to handle multiple object sizes. > Because Encoder sees FrameEncoder has an interface, it has no real idea what > the exact size of object implementing the interface is (except at the moment > of allocation). > This means that every class implementing FrameEncoder interface must have the > exact same size and thus no extra member/virtual functions... > Otherwise, you're sure to get a SEGV somewhere in there. > > With my proposal, the only requirement to switch to a new FrameEncoder is to > add something like this for the instantiation > for (int i = 0; i < m_param->frameNumThreads; i++) > { > if (m_param->accelerationType != X265_ACCEL_NONE) > m_frameEncoder[i] = new FrameEncoderKalray; > else > m_frameEncoder[i] = new FrameEncoder; > > m_frameEncoder[i]->setThreadPool(m_threadPool); > }
m_frameEncoder = new FrameEncoderKalray[m_param->frameNumThreads] should still work unless you plan to use a mixture of FrameEncoder and FrameEncoderKalray. Either way, can you rebase your change to the new tip and use FrameEncoder* m_frameEncoder[X265_MAX_FRAME_THREADS] in the top-level Encoder? -- Steve Borho _______________________________________________ x265-devel mailing list [email protected] https://mailman.videolan.org/listinfo/x265-devel
