[x265] [PATCH] Make FrameEncoder partially virtual so it can be overloaded

[Nicolas Morey-Chaisemartin]

On 02/02/2015 06:27 PM, Steve Borho wrote:
> On 02/02, Nicolas Morey-Chaisemartin wrote:
>> # HG changeset patch
>> # User Nicolas Morey-Chaisemartin <nmorey at kalray.eu>
>> # 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);
     }

Not the prettiest but I haven't had the time to make all this more generic and elegant without too much impact on the code.