cyrusbehr / tensorrt-cpp-api

TensorRT C++ API Tutorial
MIT License
596 stars 74 forks source link

Introduce IEngine Interface to Decouple Implementation from Declaration #69

Closed thomaskleiven closed 3 months ago

thomaskleiven commented 4 months ago

What about introducing an interface class IEngine to separate the interface from the implementation in the current Engine class template?

Description: Introducing an IEngine interface will separate the interface from the implementation, providing:

Proposed Changes:

Could be something like this:

template <typename T>
class IEngine {
public:
    virtual ~IEngine() = default;
    virtual bool buildLoadNetwork(std::string onnxModelPath, const std::array<float, 3> &subVals, const std::array<float, 3> &divVals, bool normalize) = 0;
    virtual bool loadNetwork(std::string trtModelPath, const std::array<float, 3> &subVals, const std::array<float, 3> &divVals, bool normalize) = 0;
    virtual bool runInference(const std::vector<std::vector<cv::cuda::GpuMat>> &inputs, std::vector<std::vector<std::vector<T>>> &featureVectors) = 0;
    virtual const std::vector<nvinfer1::Dims3>& getInputDims() const = 0;
    virtual const std::vector<nvinfer1::Dims>& getOutputDims() const = 0;
};
cyrusbehr commented 4 months ago

Yeah I think that's a good idea. I do admit things got fairly messy once I added the class template parameter, because then it meant that I had to write all the template method implementations in the header which makes it harder to read.

thomaskleiven commented 4 months ago

I agree, it's a nice addition, but it does make the code harder to read and maintain. Before creating the IEngine interface, should we move the template functions to engine.inl for inline implementations? This could improve readability and maintainability, making it easier to add the IEngine in a subsequent step.