Merge pull request #915 from OpenShot/sentry-invalid-property-data

Prevent crash in ObjectDetection (due to NULL parentClip)

Author: jonoomph

.github/workflows/ci.yml

@@ -11,8 +11,8 @@ jobs:
     strategy:
       matrix:
         sys:
-          - { os: ubuntu-18.04, shell: bash }
           - { os: ubuntu-20.04, shell: bash }
+          - { os: ubuntu-22.04, shell: bash }
           - { os: windows-2022, shell: 'msys2 {0}' }
         compiler:
           - { cc: gcc, cxx: g++ }
@@ -82,13 +82,19 @@ jobs:
             qtbase5-dev qtbase5-dev-tools libqt5svg5-dev \
             libfdk-aac-dev libavcodec-dev libavformat-dev \
             libavutil-dev libswscale-dev libswresample-dev \
-            libzmq3-dev libmagick++-dev libbabl-dev \
+            libzmq3-dev libbabl-dev \
             libopencv-dev libprotobuf-dev protobuf-compiler \
             cargo libomp5 libomp-dev
-          # Install catch2 package from Ubuntu 20.10, since for some reason
-          # even 20.04 only has Catch 1.12.1 available.
-          wget https://launchpad.net/ubuntu/+archive/primary/+files/catch2_2.13.0-1_all.deb
-          sudo dpkg -i catch2_2.13.0-1_all.deb
+
+          # Install catch2 package from source
+          git clone https://github.com/catchorg/Catch2.git
+          pushd Catch2
+          cmake -Bbuild -H. -DBUILD_TESTING=OFF
+          sudo cmake --build build/ --target install
+          popd
+
+          wget https://launchpad.net/ubuntu/+archive/primary/+files/catch2_2.13.8-1_amd64.deb
+          sudo dpkg -i catch2_2.13.8-1_amd64.deb
 
       - name: Install macOS dependencies
         if: ${{ runner.os == 'macos' }}

src/Clip.cpp

@@ -253,19 +253,16 @@ void Clip::AttachToObject(std::string object_id)
 			SetAttachedClip(clipObject);
 		}
 	}
-	return;
 }
 
 // Set the pointer to the trackedObject this clip is attached to
 void Clip::SetAttachedObject(std::shared_ptr<openshot::TrackedObjectBase> trackedObject){
 	parentTrackedObject = trackedObject;
-	return;
 }
 
 // Set the pointer to the clip this clip is attached to
 void Clip::SetAttachedClip(Clip* clipObject){
 	parentClipObject = clipObject;
-	return;
 }
 
 /// Set the current reader
@@ -754,11 +751,8 @@ std::string Clip::PropertiesJSON(int64_t requested_frame) const {
 	root["display"] = add_property_json("Frame Number", display, "int", "", NULL, 0, 3, false, requested_frame);
 	root["mixing"] = add_property_json("Volume Mixing", mixing, "int", "", NULL, 0, 2, false, requested_frame);
 	root["waveform"] = add_property_json("Waveform", waveform, "int", "", NULL, 0, 1, false, requested_frame);
-	if (!parentObjectId.empty()) {
-		root["parentObjectId"] = add_property_json("Parent", 0.0, "string", parentObjectId, NULL, -1, -1, false, requested_frame);
-	} else {
-		root["parentObjectId"] = add_property_json("Parent", 0.0, "string", "", NULL, -1, -1, false, requested_frame);
-	}
+	root["parentObjectId"] = add_property_json("Parent", 0.0, "string", parentObjectId, NULL, -1, -1, false, requested_frame);
+
 	// Add gravity choices (dropdown style)
 	root["gravity"]["choices"].append(add_property_choice_json("Top Left", GRAVITY_TOP_LEFT, gravity));
 	root["gravity"]["choices"].append(add_property_choice_json("Top Center", GRAVITY_TOP, gravity));
@@ -792,7 +786,7 @@ std::string Clip::PropertiesJSON(int64_t requested_frame) const {
 	root["waveform"]["choices"].append(add_property_choice_json("No", false, waveform));
 
 	// Add the parentTrackedObject's properties
-	if (parentTrackedObject)
+	if (parentTrackedObject && parentClipObject)
 	{
 		// Convert Clip's frame position to Timeline's frame position
 		long clip_start_position = round(Position() * info.fps.ToDouble()) + 1;
@@ -1440,14 +1434,14 @@ QTransform Clip::get_transform(std::shared_ptr<Frame> frame, int width, int heig
 
 	// Get the parentTrackedObject properties
 	if (parentTrackedObject){
-
 		// Convert Clip's frame position to Timeline's frame position
 		long clip_start_position = round(Position() * info.fps.ToDouble()) + 1;
 		long clip_start_frame = (Start() * info.fps.ToDouble()) + 1;
 		double timeline_frame_number = frame->number + clip_start_position - clip_start_frame;
 
 		// Get parentTrackedObject's parent clip's properties
-		std::map<std::string, float> trackedObjectParentClipProperties = parentTrackedObject->GetParentClipProperties(timeline_frame_number);
+		std::map<std::string, float> trackedObjectParentClipProperties =
+				parentTrackedObject->GetParentClipProperties(timeline_frame_number);
 
 		// Get the attached object's parent clip's properties
 		if (!trackedObjectParentClipProperties.empty())

src/TrackedObjectBBox.cpp

@@ -235,7 +235,7 @@ void TrackedObjectBBox::ScalePoints(double time_scale){
 // Load the bounding-boxes information from the protobuf file
 bool TrackedObjectBBox::LoadBoxData(std::string inputFilePath)
 {
-    using std::ios;
+	using std::ios;
 
 	// Variable to hold the loaded data
 	pb_tracker::Tracker bboxMessage;
@@ -282,7 +282,7 @@ bool TrackedObjectBBox::LoadBoxData(std::string inputFilePath)
 	if (bboxMessage.has_last_updated())
 	{
 		std::cout << " Loaded Data. Saved Time Stamp: "
-		          << TimeUtil::ToString(bboxMessage.last_updated()) << std::endl;
+				  << TimeUtil::ToString(bboxMessage.last_updated()) << std::endl;
 	}
 
 	// Delete all global objects allocated by libprotobuf.
@@ -381,11 +381,8 @@ void TrackedObjectBBox::SetJsonValue(const Json::Value root)
 		protobufDataPath = root["protobuf_data_path"].asString();
 
 	// Set the id of the child clip
-	if (!root["child_clip_id"].isNull() && root["child_clip_id"].asString() != "" && root["child_clip_id"].asString() != Id()){
-		Clip* parentClip = (Clip *) ParentClip();
-
-		if (root["child_clip_id"].asString() != parentClip->Id())
-			ChildClipId(root["child_clip_id"].asString());
+	if (!root["child_clip_id"].isNull() && root["child_clip_id"].asString() != Id()){
+		ChildClipId(root["child_clip_id"].asString());
 	}
 
 	// Set the Keyframes by the given JSON object

src/TrackedObjectBase.h

@@ -22,8 +22,8 @@
 
 namespace openshot {
 
-    // Forward decls
-    class ClipBase;
+	// Forward decls
+	class ClipBase;
 
 	/**
 	 * @brief This abstract class is the base class of all Tracked Objects.
@@ -50,8 +50,8 @@ namespace openshot {
 		/// Constructor which takes an object ID
 		TrackedObjectBase(std::string _id);
 
-        /// Destructor
-        virtual ~TrackedObjectBase() = default;
+		/// Destructor
+		virtual ~TrackedObjectBase() = default;
 
 		/// Get the id of this object
 		std::string Id() const { return id; }

src/effects/ObjectDetection.cpp

@@ -78,9 +78,9 @@ std::shared_ptr<Frame> ObjectDetection::GetFrame(std::shared_ptr<Frame> frame, i
     }
 
     // Initialize the Qt rectangle that will hold the positions of the bounding-box
-	std::vector<QRectF> boxRects;
-	// Initialize the image of the TrackedObject child clip
-	std::vector<std::shared_ptr<QImage>> childClipImages;
+    std::vector<QRectF> boxRects;
+    // Initialize the image of the TrackedObject child clip
+    std::vector<std::shared_ptr<QImage>> childClipImages;
 
     // Check if track data exists for the requested frame
     if (detectionsData.find(frame_number) != detectionsData.end()) {
@@ -122,15 +122,6 @@ std::shared_ptr<Frame> ObjectDetection::GetFrame(std::shared_ptr<Frame> frame, i
                 std::vector<int> bg_rgba = trackedObject->background.GetColorRGBA(frame_number);
                 float bg_alpha = trackedObject->background_alpha.GetValue(frame_number);
 
-                // Create a rotated rectangle object that holds the bounding box
-                // cv::RotatedRect box ( cv::Point2f( (int)(trackedBox.cx*fw), (int)(trackedBox.cy*fh) ),
-                //					 cv::Size2f( (int)(trackedBox.width*fw), (int)(trackedBox.height*fh) ),
-                //					 (int) (trackedBox.angle) );
-
-                // DrawRectangleRGBA(cv_image, box, bg_rgba, bg_alpha, 1, true);
-                // DrawRectangleRGBA(cv_image, box, stroke_rgba, stroke_alpha, stroke_width, false);
-
-
                 cv::Rect2d box(
                     (int)( (trackedBox.cx-trackedBox.width/2)*fw),
                     (int)( (trackedBox.cy-trackedBox.height/2)*fh),
@@ -160,9 +151,8 @@ std::shared_ptr<Frame> ObjectDetection::GetFrame(std::shared_ptr<Frame> frame, i
                         Clip* childClip = parentTimeline->GetClip(trackedObject->ChildClipId());
 
                         if (childClip){
-                            std::shared_ptr<Frame> f(new Frame(1, frame->GetWidth(), frame->GetHeight(), "#00000000"));
                             // Get the image of the child clip for this frame
-					        std::shared_ptr<Frame> childClipFrame = childClip->GetFrame(f, frame_number);
+                            std::shared_ptr<Frame> childClipFrame = childClip->GetFrame(frame_number);
                             childClipImages.push_back(childClipFrame->GetImage());
 
                             // Set the Qt rectangle with the bounding-box properties
@@ -182,15 +172,15 @@ std::shared_ptr<Frame> ObjectDetection::GetFrame(std::shared_ptr<Frame> frame, i
     // Update Qt image with new Opencv frame
     frame->SetImageCV(cv_image);
 
-	// Set the bounding-box image with the Tracked Object's child clip image
-	if(boxRects.size() > 0){
+    // Set the bounding-box image with the Tracked Object's child clip image
+    if(boxRects.size() > 0){
         // Get the frame image
         QImage frameImage = *(frame->GetImage());
         for(int i; i < boxRects.size();i++){
             // Set a Qt painter to the frame image
             QPainter painter(&frameImage);
             // Draw the child clip image inside the bounding-box
-            painter.drawImage(boxRects[i], *childClipImages[i], QRectF(0, 0, frameImage.size().width(),  frameImage.size().height()));
+            painter.drawImage(boxRects[i], *childClipImages[i]);
         }
         // Set the frame image as the composed image
         frame->AddImage(std::make_shared<QImage>(frameImage));
@@ -362,7 +352,7 @@ bool ObjectDetection::LoadObjDetectdData(std::string inputFilePath){
 
                 std::shared_ptr<TrackedObjectBBox> trackedObjPtr = std::make_shared<TrackedObjectBBox>(trackedObj);
                 ClipBase* parentClip = this->ParentClip();
-	            trackedObjPtr->ParentClip(parentClip);
+                trackedObjPtr->ParentClip(parentClip);
 
                 // Create a temp ID. This ID is necessary to initialize the object_id Json list
                 // this Id will be replaced by the one created in the UI