Best practice for factory pattern in c++?

I’m implementing a simple data reader to get image sequences from either a pre-recorded video file (In my case ROS bag file), or a camera device (In my case Intel Realsense). Basically I intend for users to use it in the following way:

  1. When a non-empty file path is provided, the reader reads from the video file; otherwise if an empty path is specified, the reader reads from the camera device. (For now in case of any failure, throwing exceptions and terminating the program will be fine.)

  2. The user iteratively calls bool get_next_frame(Frame&) to get all the frames, until reaching the end of the video, upon which the function returns false.

I’m trying to implement the above-mentioned functionality using the factory pattern, in C++ as follows:

class DataReader;
typedef std::shared_ptr<DataReader> datareader_ptr;

class DataReader {
  friend datareader_ptr make_data_reader(const std::string& ros_bag_path);
public:
  virtual bool get_next_frameset(rs2::frameset& frames)=0;
  virtual void finalize() {
    pipe_.stop();
  }
  virtual ~DataReader() {
    std::cout << "dtor for DataReader called." << std::endl;
    finalize();
  }
protected:
  DataReader(rs2::config cfg, rs2::pipeline pipe, rs2::pipeline_profile pipeline_profile)
      :cfg_(cfg),pipe_(pipe),pipeline_profile_(pipeline_profile) {}
  rs2::config cfg_;
  rs2::pipeline_profile pipeline_profile_;
  rs2::pipeline pipe_;
};

class RosBagDataReader : public DataReader {
  friend datareader_ptr make_data_reader(const std::string& ros_bag_path);
public:
  virtual bool get_next_frameset(rs2::frameset& frames) {
    frames = pipe_.wait_for_frames();
    return true;
  }
  virtual ~RosBagDataReader() {}
protected:
  RosBagDataReader(rs2::config cfg, rs2::pipeline pipe, rs2::pipeline_profile pipeline_profile)
      :DataReader(cfg, pipe, pipeline_profile) {}
};

class CameraDataReader : public DataReader {
  friend datareader_ptr make_data_reader(const std::string& ros_bag_path);
public:
  virtual bool get_next_frameset(rs2::frameset& frames) {
    frames = pipe_.wait_for_frames();
    return true;
  }
  virtual ~CameraDataReader() {}
protected:
  CameraDataReader(rs2::config cfg, rs2::pipeline pipe, rs2::pipeline_profile pipeline_profile)
      :DataReader(cfg, pipe, pipeline_profile) {}
};

datareader_ptr make_data_reader(const std::string& ros_bag_path) {
  rs2::config cfg;
  rs2::pipeline pipe;
  if (ros_bag_path=="") {
    rs2::pipeline_profile profile = pipe.start();
    return datareader_ptr(new CameraDataReader(cfg, pipe, profile));
  } else {
    // from ros bag file
    cfg.enable_device_from_file(ros_bag_path, false);
    rs2::pipeline_profile profile = pipe.start(cfg); //File will be opened in read mode at this point
    rs2::playback play(profile.get_device());
    play.set_real_time(false);
    return datareader_ptr(new RosBagDataReader(cfg, pipe, profile));
  }
}

However, I’m really not sure whether this is the best practice. More specifically,

  1. Is the usage of factory pattern here justified ?
  2. Is my implementation of factory pattern correct ? Or are there any pitfalls in the code ?

For example, I used friend function to access the protected constructors in all the classes, is this appropriate or there are better ways to do this?

Another example, protected constructor is used so that the users may not directly construct the readers, but only through the factory function. Is this done correctly?

Thanks in advance!