Is this an appropriate use of abstract classes and inheritance?

Question

This is a simple app that starts the webcam either in color or in grayscale mode depending on what the user wants:

enter image description here

The grayscale camera is implemented via the GrayVideo class and the color camera through ColorVideo. I wanted to have a common show_current_frame for both the ColorVideo and GrayVideo objects. That show_current_frame gets the current frame from the process method and displays it in the GUI. Since the color and the grayscale videos need different frame processing, I needed to have different process methods for ColorVideo and GrayVideo. So, I thought of having a Video class with an abstract process method.

Is this a decent use of abstract classes and inheritance? What other problems and possible improvements do you see in my code? Or is the design wrong in the first place?

import cv2
from abc import ABC, abstractmethod
class Video(ABC):
 def __init__(self, source):
 self.capture = cv2.VideoCapture(source) # If source = 0 the webcam starts
 def get_raw_frame(self):
 # Get a bolean and current frame (numpy array) from the webcam
 ret, frame = self.capture.read()
 if ret:
 return frame
 @abstractmethod
 def process(self):
 # Method to process frames.
 # Method will be overwritten by subclasses
 pass
 def show_current_frame(self):
 # Get processed frame and show it in the GUI
 current_frame = self.process()
 cv2.imshow('Live', current_frame)
 def end(self):
 # Releases webcam and closes GUI window
 self.capture.release()
 cv2.destroyAllWindows()
class ColorVideo(Video):
 def process(self):
 # Since raw frames are in color, there's no processing needed
 return self.get_raw_frame()
class GrayVideo(ColorVideo):
 def process(self):
 # Grayscaling the raw frames
 raw_frame = self.get_raw_frame()
 gray = cv2.cvtColor(raw_frame, cv2.COLOR_BGR2GRAY)
 return gray
user_preference = input('Enter "c" for color, or "g" for grayscale: ')
if user_preference == 'c':
 video = ColorVideo(source=0)
if user_preference == 'g':
 video = GrayVideo(source=0)
while True:
 video.show_current_frame()
 if cv2.waitKey(1) & 0xFF == ord('q'):
 video.end()

Answer 1

Is this a decent use of abstract classes and inheritance?

Yes, it's decent. My personal preference is, rather than using ABC, simply

def process(self):
 # Method to process frames.
 # Method will be overwritten by subclasses
 raise NotImplementedError()

You can also simplify this somewhat by

• Renaming Video to ColorVideo
• Deleting the class that is now called ColorVideo
• Rather than making process abstract, make it "virtual" (in C++ parlance): make it take the contents of what is now get_raw_frame and delete get_raw_frame
• In the child GrayVideo, override process to call its super().process and convert the results

If you're concerned that this more minimal representation is confusing (i.e. why is a GrayVideo a ColorVideo?) then you can keep Video as a conceptually abstract class, and simply

class ColorVideo(Video):
 pass

Other stuff:

• end should actually be the __exit__ of a context manager;
• it looks like your source argument should take a default of 0;
• make a main function called by standard __name__ guard.

• multigoodverse: Cool, thank you! That made it a lot more compact! I never thought of leveraging super that way There's one more aspect I'd like to get your opinion about. I want to draw some rectangles on top of the video frames with cv2.rectangle(self.frame, (x1, y1), (x2, y2), (255, 0, 0), 2). Would you make a new decorator class that "decorates" the ColorVideo frames with rectangles or would you simply create a RectangledVidoe class that inherits from ColorVideo, or other?
• Reinderien: Are the rectangles always on every frame?
• multigoodverse: Yes, they are. In other words, there's a rectangle annotation on the entire video.
• multigoodverse: Alright, I can simply make an AnnotatedVideo class inheriting from ColorVideo and then modify the process method to return the annotated frame. That wouldn't work if the rectangle would be there all the time though. If you have any suggestions for the later (rectangle not all the time in the video) I'm all ears.
• Reinderien: Simply add it as a boolean argument on the constructor, saved to an attribute of the class. If true, make the rectangle.

Answer 2

user input

What happens if the user does not type in c or g?

typing

As a general remark, I would include type annotations, so users of your code, (this includes you in 6 months time) can know what to expect.

docstring

The same goes for a docstring.

You kind of do that already

def get_raw_frame(self):
 # Get a bolean and current frame (numpy array) from the webcam

But if you turn that into a docstring

def get_raw_frame(self):
 """Get a bolean and current frame (numpy array) from the webcam"""

IDE's etc can keep track of this.

inheritance

I would not use inheritance here, but composition. An excellent explanation is given by Brandon Rhodes here

You can define a procotol.

class VideoProcessor(typing.Protocol):
 def process(self, raw_frame:np.ndarray) -> np.ndarray:
 ...

And then give 2 implementations:

class ColorProcessor(VideoProcessor):
 def process(self, raw_frame: np.ndarray) -> np.ndarray:
 """Return the frame untouched."""
 return raw_frame
class GrayProcessor(VideoProcessor):
 def process(self, raw_frame: np.ndarray) -> np.ndarray:
 """Convert the raw frame to grayscale."""
 return cv2.cvtColor(raw_frame, cv2.COLOR_BGR2GRAY)

Then the init and process become something like this:

def __init__(
 self, source: int, processor: VideoProcessor
 ):
 self.processor = processor
 self.capture = cv2.VideoCapture(source) 
 # If source = 0 the webcam starts
def process(self):
 """Let the processor process the raw frame."""
 raw_frame = self.get_raw_frame()
 if raw_frame is not None:
 return self.processor.process(raw_frame)

This way, If you ever want to implement a sepia, or green version, it's just a matter of implementing another Processor.

These processors can also be tested individually, without having to set up a videosource

Hoist the IO

Another thing I would change, is instead of letting the Video class instantiate the connection to the webcam, I would let this be done on a higher level, and have the Video class accept a video source.

Here are 1 2 talks on why you would want to do this. This concept is not limited to python (3)

class VideoSource(typing.Protocol):
 def read(self) -> typing.Tuple[bool, np.ndarray]:
 """Read the current frame.
 Returns a boolean success flag, 
 and the current frame, if successful.
 """
 ...
 def release(self) -> None:
 """Release the connection to the video source."""
 ...
def __init__(
 self, source: VideoSource, processor: VideoProcessor
):
 self.processor = processor
 self.capture = source

This change makes it even easier to test the Video class.

context manager

Turning Video into a context manager is very simple:

def __enter__(self):
 return self
def __exit__(self, type, value, traceback):
 self.end()

Putting it together

if __name__ == "__main__": 
 while True:
 user_preference = input('Enter "c" for color, or "g" for grayscale: ')
 if user_preference in "cg":
 break
 if user_preference == 'c':
 processor = ColorProcessor()
 if user_preference == 'g':
 processor = GrayProcessor()
 source = cv2.VideoCapture(0) 
 with Video(source=source, processor=processor) as video:
 while True:
 video.show_current_frame()
 if cv2.waitKey(1) & 0xFF == ord('q'):
 break

rectangles on the screen

You could even generalize this to have consequent processors, for example if you want to add the rectangles

The Processor itself can be quite simple (I use dataclasses to avoid the boiler plate __init__:

import dataclasses

@dataclasses.dataclass
class RectangleProcessor(VideoProcessor):
 x1: int
 y1: int
 x2: int
 y2: int
 color: typing.Tuple[int, int, int]
 def process(self, raw_frame: np.ndarray) -> np.ndarray:
 return cv2.rectangle(
 raw_frame, (self.x1, self.y1), (self.x2, self.y2), self.color, 2
 )

You can implement a chain of processors very simply:

class Video:
 def __init__(
 self,
 source: VideoSource,
 processors: typing.Optional[typing.Sequence[VideoProcessor]] = None,
 ):
 self.processors = processors
 self.capture = source
 def process(self) -> np.ndarray:
 raw_frame = self.get_raw_frame()
 if self.processors is None:
 return raw_frame
 for processor in self.processors:
 raw_frame = processor.process(raw_frame)
 return raw_frame

This way you can even skip the noop ColorProcessor

if __name__ == "__main__":
 while True:
 user_preference = input('Enter "c" for color, or "g" for grayscale: ')
 if user_preference in "cg":
 break
 while True:
 processors = []
 if user_preference == "g":
 processors.append(GrayProcessor())
 user_preference = input('Do you want to add a rectange [y/N]:')
 if user_preference.lower() == "y":
 processors.append(RectangleProcessor(0, 0, 10, 10, (255, 0, 0)))
 source = cv2.VideoCapture(0)
 with Video(source=source, processors=processors) as video:
 while True:
 video.show_current_frame()
 if cv2.waitKey(1) & 0xFF == ord('q'):
 break

Like this, you can easily add Processors that add timestamps to video's, names to streams, ...

• Grajdeanu Alex: I'd suggest you add a main() function instead of throwing everything in bulk right under the if __name__ == "__main__" guard.