onvif: Switch from minidom to xmltree for parsing ONVIF timed metadata

minidom doesn't handle various valid but suboptimal XML documents.

4 files changed, 291 insertions, 247 deletions

diff --git a/net/onvif/Cargo.toml b/net/onvif/Cargo.toml
index 39f64977..4cb91d32 100644
--- a/net/onvif/Cargo.toml
+++ b/net/onvif/Cargo.toml
@@ -15,11 +15,11 @@ gst-base = { package = "gstreamer-base", git = "https://gitlab.freedesktop.org/g
 gst-video = { package = "gstreamer-video", git = "https://gitlab.freedesktop.org/gstreamer/gstreamer-rs", features = ["v1_20"] }
 once_cell = "1.0"
 xmlparser = "0.13"
-minidom = "0.15"
 chrono = { version = "0.4", default-features = false }
 cairo-rs = { git = "https://github.com/gtk-rs/gtk-rs-core", features=["use_glib"] }
 pango = { git = "https://github.com/gtk-rs/gtk-rs-core" }
 pangocairo = { git = "https://github.com/gtk-rs/gtk-rs-core" }
+xmltree = "0.10"
 
 [lib]
 name = "gstrsonvif"
diff --git a/net/onvif/src/lib.rs b/net/onvif/src/lib.rs
index 871d6a94..3bda1cb7 100644
--- a/net/onvif/src/lib.rs
+++ b/net/onvif/src/lib.rs
@@ -21,6 +21,9 @@ mod onvifmetadataoverlay;
 mod onvifmetadataparse;
 mod onvifmetadatapay;
 
+// ONVIF Timed Metadata schema
+pub(crate) const ONVIF_METADATA_SCHEMA: &str = "http://www.onvif.org/ver10/schema";
+
 // Offset in nanoseconds from midnight 01-01-1900 (prime epoch) to
 // midnight 01-01-1970 (UNIX epoch)
 pub(crate) const PRIME_EPOCH_OFFSET: gst::ClockTime = gst::ClockTime::from_seconds(2_208_988_800);
@@ -43,7 +46,7 @@ pub(crate) fn lookup_reference_timestamp(buffer: &gst::Buffer) -> Option<gst::Cl
     None
 }
 
-pub(crate) fn xml_from_buffer(buffer: &gst::Buffer) -> Result<minidom::Element, gst::ErrorMessage> {
+pub(crate) fn xml_from_buffer(buffer: &gst::Buffer) -> Result<xmltree::Element, gst::ErrorMessage> {
     let map = buffer.map_readable().map_err(|_| {
         gst::error_msg!(gst::ResourceError::Read, ["Failed to map buffer readable"])
     })?;
@@ -55,7 +58,7 @@ pub(crate) fn xml_from_buffer(buffer: &gst::Buffer) -> Result<minidom::Element,
         )
     })?;
 
-    let root = utf8.parse::<minidom::Element>().map_err(|err| {
+    let root = xmltree::Element::parse(std::io::Cursor::new(utf8)).map_err(|err| {
         gst::error_msg!(
             gst::ResourceError::Read,
             ["Failed to parse buffer as XML: {}", err]
@@ -66,40 +69,45 @@ pub(crate) fn xml_from_buffer(buffer: &gst::Buffer) -> Result<minidom::Element,
 }
 
 pub(crate) fn iterate_video_analytics_frames(
-    root: &minidom::Element,
+    root: &xmltree::Element,
 ) -> impl Iterator<
-    Item = Result<(chrono::DateTime<chrono::FixedOffset>, &minidom::Element), gst::ErrorMessage>,
+    Item = Result<(chrono::DateTime<chrono::FixedOffset>, &xmltree::Element), gst::ErrorMessage>,
 > {
-    root.get_child("VideoAnalytics", "http://www.onvif.org/ver10/schema")
+    root.get_child(("VideoAnalytics", ONVIF_METADATA_SCHEMA))
         .map(|analytics| {
-            analytics.children().filter_map(|el| {
-                // We are only interested in associating Frame metadata with video frames
-                if el.is("Frame", "http://www.onvif.org/ver10/schema") {
-                    let timestamp = match el.attr("UtcTime") {
-                        Some(timestamp) => timestamp,
-                        None => {
-                            return Some(Err(gst::error_msg!(
-                                gst::ResourceError::Read,
-                                ["Frame element has no UtcTime attribute"]
-                            )));
-                        }
-                    };
-
-                    let dt = match chrono::DateTime::parse_from_rfc3339(timestamp) {
-                        Ok(dt) => dt,
-                        Err(err) => {
-                            return Some(Err(gst::error_msg!(
-                                gst::ResourceError::Read,
-                                ["Failed to parse UtcTime {}: {}", timestamp, err]
-                            )));
-                        }
-                    };
-
-                    Some(Ok((dt, el)))
-                } else {
-                    None
-                }
-            })
+            analytics
+                .children
+                .iter()
+                .filter_map(|n| n.as_element())
+                .filter_map(|el| {
+                    // We are only interested in associating Frame metadata with video frames
+                    if el.name == "Frame" && el.namespace.as_deref() == Some(ONVIF_METADATA_SCHEMA)
+                    {
+                        let timestamp = match el.attributes.get("UtcTime") {
+                            Some(timestamp) => timestamp,
+                            None => {
+                                return Some(Err(gst::error_msg!(
+                                    gst::ResourceError::Read,
+                                    ["Frame element has no UtcTime attribute"]
+                                )));
+                            }
+                        };
+
+                        let dt = match chrono::DateTime::parse_from_rfc3339(timestamp) {
+                            Ok(dt) => dt,
+                            Err(err) => {
+                                return Some(Err(gst::error_msg!(
+                                    gst::ResourceError::Read,
+                                    ["Failed to parse UtcTime {}: {}", timestamp, err]
+                                )));
+                            }
+                        };
+
+                        Some(Ok((dt, el)))
+                    } else {
+                        None
+                    }
+                })
         })
         .into_iter()
         .flatten()
diff --git a/net/onvif/src/onvifmetadataoverlay/imp.rs b/net/onvif/src/onvifmetadataoverlay/imp.rs
index 5217a68b..df46e2b5 100644
--- a/net/onvif/src/onvifmetadataoverlay/imp.rs
+++ b/net/onvif/src/onvifmetadataoverlay/imp.rs
@@ -9,8 +9,6 @@ use once_cell::sync::Lazy;
 use std::collections::HashSet;
 use std::sync::Mutex;
 
-use minidom::Element;
-
 static CAT: Lazy<gst::DebugCategory> = Lazy::new(|| {
     gst::DebugCategory::new(
         "onvifmetadataoverlay",
@@ -445,212 +443,231 @@ impl OnvifMetadataOverlay {
                         gst::FlowError::Error
                     })?;
 
-                    let root = utf8.parse::<Element>().map_err(|err| {
-                        gst::element_imp_error!(
-                            self,
-                            gst::ResourceError::Read,
-                            ["Failed to parse buffer as XML: {}", err]
-                        );
+                    let root =
+                        xmltree::Element::parse(std::io::Cursor::new(utf8)).map_err(|err| {
+                            gst::element_imp_error!(
+                                self,
+                                gst::ResourceError::Read,
+                                ["Failed to parse buffer as XML: {}", err]
+                            );
 
-                        gst::FlowError::Error
-                    })?;
+                            gst::FlowError::Error
+                        })?;
 
                     for object in root
-                        .get_child("VideoAnalytics", "http://www.onvif.org/ver10/schema")
-                        .map(|el| el.children().into_iter().collect())
-                        .unwrap_or_else(Vec::new)
+                        .get_child(("VideoAnalytics", crate::ONVIF_METADATA_SCHEMA))
+                        .map(|e| e.children.iter().filter_map(|n| n.as_element()))
+                        .into_iter()
+                        .flatten()
                     {
-                        if object.is("Frame", "http://www.onvif.org/ver10/schema") {
-                            for object in object.children() {
-                                if object.is("Object", "http://www.onvif.org/ver10/schema") {
-                                    gst::trace!(CAT, imp: self, "Handling object {:?}", object);
-
-                                    let object_id = match object.attr("ObjectId") {
-                                        Some(id) => id.to_string(),
-                                        None => {
-                                            gst::warning!(
-                                                CAT,
-                                                imp: self,
-                                                "XML Object with no ObjectId"
-                                            );
-                                            continue;
-                                        }
-                                    };
-
-                                    if !object_ids.insert(object_id.clone()) {
-                                        gst::debug!(
+                        if object.name == "Frame"
+                            && object.namespace.as_deref() == Some(crate::ONVIF_METADATA_SCHEMA)
+                        {
+                            for object in object
+                                .children
+                                .iter()
+                                .filter_map(|n| n.as_element())
+                                .filter(|e| {
+                                    e.name == "Object"
+                                        && e.namespace.as_deref()
+                                            == Some(crate::ONVIF_METADATA_SCHEMA)
+                                })
+                            {
+                                gst::trace!(CAT, imp: self, "Handling object {:?}", object);
+
+                                let object_id = match object.attributes.get("ObjectId") {
+                                    Some(id) => id.to_string(),
+                                    None => {
+                                        gst::warning!(
                                             CAT,
-                                            "Skipping older version of object {}",
-                                            object_id
+                                            imp: self,
+                                            "XML Object with no ObjectId"
                                         );
                                         continue;
                                     }
+                                };
+
+                                if !object_ids.insert(object_id.clone()) {
+                                    gst::debug!(
+                                        CAT,
+                                        "Skipping older version of object {}",
+                                        object_id
+                                    );
+                                    continue;
+                                }
 
-                                    let appearance = match object.get_child(
-                                        "Appearance",
-                                        "http://www.onvif.org/ver10/schema",
-                                    ) {
-                                        Some(appearance) => appearance,
-                                        None => continue,
-                                    };
-
-                                    let shape = match appearance
-                                        .get_child("Shape", "http://www.onvif.org/ver10/schema")
-                                    {
-                                        Some(shape) => shape,
-                                        None => continue,
-                                    };
-
-                                    let tag = appearance
-                                        .get_child("Class", "http://www.onvif.org/ver10/schema")
-                                        .and_then(|class| {
-                                            class.get_child(
-                                                "Type",
-                                                "http://www.onvif.org/ver10/schema",
-                                            )
-                                        })
-                                        .map(|t| t.text());
-
-                                    let bbox = match shape.get_child(
-                                        "BoundingBox",
-                                        "http://www.onvif.org/ver10/schema",
-                                    ) {
-                                        Some(bbox) => bbox,
-                                        None => {
-                                            gst::warning!(
-                                                CAT,
-                                                imp: self,
-                                                "XML Shape with no BoundingBox"
-                                            );
-                                            continue;
-                                        }
-                                    };
-
-                                    let left: f64 =
-                                        match bbox.attr("left").and_then(|val| val.parse().ok()) {
-                                            Some(val) => val,
-                                            None => {
-                                                gst::warning!(
-                                                    CAT,
-                                                    imp: self,
-                                                    "BoundingBox with no left attribute"
-                                                );
-                                                continue;
-                                            }
-                                        };
-
-                                    let right: f64 =
-                                        match bbox.attr("right").and_then(|val| val.parse().ok()) {
-                                            Some(val) => val,
-                                            None => {
-                                                gst::warning!(
-                                                    CAT,
-                                                    imp: self,
-                                                    "BoundingBox with no right attribute"
-                                                );
-                                                continue;
-                                            }
-                                        };
-
-                                    let top: f64 =
-                                        match bbox.attr("top").and_then(|val| val.parse().ok()) {
-                                            Some(val) => val,
-                                            None => {
-                                                gst::warning!(
-                                                    CAT,
-                                                    imp: self,
-                                                    "BoundingBox with no top attribute"
-                                                );
-                                                continue;
-                                            }
-                                        };
-
-                                    let bottom: f64 = match bbox
-                                        .attr("bottom")
-                                        .and_then(|val| val.parse().ok())
-                                    {
-                                        Some(val) => val,
-                                        None => {
-                                            gst::warning!(
-                                                CAT,
-                                                imp: self,
-                                                "BoundingBox with no bottom attribute"
-                                            );
-                                            continue;
-                                        }
-                                    };
+                                let appearance = match object
+                                    .get_child(("Appearance", crate::ONVIF_METADATA_SCHEMA))
+                                {
+                                    Some(appearance) => appearance,
+                                    None => continue,
+                                };
+
+                                let shape = match appearance
+                                    .get_child(("Shape", crate::ONVIF_METADATA_SCHEMA))
+                                {
+                                    Some(shape) => shape,
+                                    None => continue,
+                                };
+
+                                let tag = appearance
+                                    .get_child(("Class", crate::ONVIF_METADATA_SCHEMA))
+                                    .and_then(|class| {
+                                        class.get_child(("Type", crate::ONVIF_METADATA_SCHEMA))
+                                    })
+                                    .and_then(|t| t.get_text())
+                                    .map(|t| t.into_owned());
+
+                                let bbox = match shape
+                                    .get_child(("BoundingBox", crate::ONVIF_METADATA_SCHEMA))
+                                {
+                                    Some(bbox) => bbox,
+                                    None => {
+                                        gst::warning!(
+                                            CAT,
+                                            imp: self,
+                                            "XML Shape with no BoundingBox"
+                                        );
+                                        continue;
+                                    }
+                                };
+
+                                let left: f64 = match bbox
+                                    .attributes
+                                    .get("left")
+                                    .and_then(|val| val.parse().ok())
+                                {
+                                    Some(val) => val,
+                                    None => {
+                                        gst::warning!(
+                                            CAT,
+                                            imp: self,
+                                            "BoundingBox with no left attribute"
+                                        );
+                                        continue;
+                                    }
+                                };
+
+                                let right: f64 = match bbox
+                                    .attributes
+                                    .get("right")
+                                    .and_then(|val| val.parse().ok())
+                                {
+                                    Some(val) => val,
+                                    None => {
+                                        gst::warning!(
+                                            CAT,
+                                            imp: self,
+                                            "BoundingBox with no right attribute"
+                                        );
+                                        continue;
+                                    }
+                                };
+
+                                let top: f64 = match bbox
+                                    .attributes
+                                    .get("top")
+                                    .and_then(|val| val.parse().ok())
+                                {
+                                    Some(val) => val,
+                                    None => {
+                                        gst::warning!(
+                                            CAT,
+                                            imp: self,
+                                            "BoundingBox with no top attribute"
+                                        );
+                                        continue;
+                                    }
+                                };
+
+                                let bottom: f64 = match bbox
+                                    .attributes
+                                    .get("bottom")
+                                    .and_then(|val| val.parse().ok())
+                                {
+                                    Some(val) => val,
+                                    None => {
+                                        gst::warning!(
+                                            CAT,
+                                            imp: self,
+                                            "BoundingBox with no bottom attribute"
+                                        );
+                                        continue;
+                                    }
+                                };
 
-                                    let x1 = width / 2 + ((left * (width / 2) as f64) as i32);
-                                    let y1 = height / 2 - ((top * (height / 2) as f64) as i32);
-                                    let x2 = width / 2 + ((right * (width / 2) as f64) as i32);
-                                    let y2 = height / 2 - ((bottom * (height / 2) as f64) as i32);
+                                let x1 = width / 2 + ((left * (width / 2) as f64) as i32);
+                                let y1 = height / 2 - ((top * (height / 2) as f64) as i32);
+                                let x2 = width / 2 + ((right * (width / 2) as f64) as i32);
+                                let y2 = height / 2 - ((bottom * (height / 2) as f64) as i32);
 
-                                    let w = (x2 - x1) as u32;
-                                    let h = (y2 - y1) as u32;
+                                let w = (x2 - x1) as u32;
+                                let h = (y2 - y1) as u32;
 
-                                    let mut points = vec![];
+                                let mut points = vec![];
 
-                                    if let Some(polygon) = shape
-                                        .get_child("Polygon", "http://www.onvif.org/ver10/schema")
+                                if let Some(polygon) =
+                                    shape.get_child(("Polygon", crate::ONVIF_METADATA_SCHEMA))
+                                {
+                                    for point in
+                                        polygon.children.iter().filter_map(|n| n.as_element())
                                     {
-                                        for point in polygon.children() {
-                                            if point
-                                                .is("Point", "http://www.onvif.org/ver10/schema")
+                                        if point.name == "Point"
+                                            && point.namespace.as_deref()
+                                                == Some(crate::ONVIF_METADATA_SCHEMA)
+                                        {
+                                            let px: f64 = match point
+                                                .attributes
+                                                .get("x")
+                                                .and_then(|val| val.parse().ok())
+                                            {
+                                                Some(val) => val,
+                                                None => {
+                                                    gst::warning!(
+                                                        CAT,
+                                                        imp: self,
+                                                        "Point with no x attribute"
+                                                    );
+                                                    continue;
+                                                }
+                                            };
+
+                                            let py: f64 = match point
+                                                .attributes
+                                                .get("y")
+                                                .and_then(|val| val.parse().ok())
                                             {
-                                                let px: f64 = match point
-                                                    .attr("x")
-                                                    .and_then(|val| val.parse().ok())
-                                                {
-                                                    Some(val) => val,
-                                                    None => {
-                                                        gst::warning!(
-                                                            CAT,
-                                                            imp: self,
-                                                            "Point with no x attribute"
-                                                        );
-                                                        continue;
-                                                    }
-                                                };
-
-                                                let py: f64 = match point
-                                                    .attr("y")
-                                                    .and_then(|val| val.parse().ok())
-                                                {
-                                                    Some(val) => val,
-                                                    None => {
-                                                        gst::warning!(
-                                                            CAT,
-                                                            imp: self,
-                                                            "Point with no y attribute"
-                                                        );
-                                                        continue;
-                                                    }
-                                                };
-
-                                                let px =
-                                                    width / 2 + ((px * (width / 2) as f64) as i32);
-                                                let px =
-                                                    (px as u32).saturating_sub(x1 as u32).min(w);
-
-                                                let py = height / 2
-                                                    - ((py * (height / 2) as f64) as i32);
-                                                let py =
-                                                    (py as u32).saturating_sub(y1 as u32).min(h);
-
-                                                points.push(Point { x: px, y: py });
-                                            }
+                                                Some(val) => val,
+                                                None => {
+                                                    gst::warning!(
+                                                        CAT,
+                                                        imp: self,
+                                                        "Point with no y attribute"
+                                                    );
+                                                    continue;
+                                                }
+                                            };
+
+                                            let px = width / 2 + ((px * (width / 2) as f64) as i32);
+                                            let px = (px as u32).saturating_sub(x1 as u32).min(w);
+
+                                            let py =
+                                                height / 2 - ((py * (height / 2) as f64) as i32);
+                                            let py = (py as u32).saturating_sub(y1 as u32).min(h);
+
+                                            points.push(Point { x: px, y: py });
                                         }
                                     }
-
-                                    shapes.push(Shape {
-                                        x: x1 as u32,
-                                        y: y1 as u32,
-                                        width: w,
-                                        height: h,
-                                        points,
-                                        tag,
-                                    });
                                 }
+
+                                shapes.push(Shape {
+                                    x: x1 as u32,
+                                    y: y1 as u32,
+                                    width: w,
+                                    height: h,
+                                    points,
+                                    tag,
+                                });
                             }
                         }
                     }
diff --git a/net/onvif/src/onvifmetadataparse/imp.rs b/net/onvif/src/onvifmetadataparse/imp.rs
index ba3600ca..d69ca15e 100644
--- a/net/onvif/src/onvifmetadataparse/imp.rs
+++ b/net/onvif/src/onvifmetadataparse/imp.rs
@@ -76,18 +76,19 @@ impl Default for Settings {
 
 #[derive(Debug)]
 struct Frame {
-    video_analytics: minidom::Element,
-    other_elements: Vec<minidom::Element>,
+    video_analytics: xmltree::Element,
+    other_elements: Vec<xmltree::Element>,
     events: Vec<gst::Event>,
 }
 
 impl Default for Frame {
     fn default() -> Self {
+        let mut video_analytics = xmltree::Element::new("VideoAnalytics");
+        video_analytics.namespace = Some(String::from(crate::ONVIF_METADATA_SCHEMA));
+        video_analytics.prefix = Some(String::from("tt"));
+
         Frame {
-            video_analytics: minidom::Element::bare(
-                "VideoAnalytics",
-                "http://www.onvif.org/ver10/schema",
-            ),
+            video_analytics,
             other_elements: Vec::new(),
             events: Vec::new(),
         }
@@ -372,22 +373,32 @@ impl OnvifMetadataParse {
                     .entry(dt_unix_ns)
                     .or_insert_with(Frame::default);
 
-                frame.video_analytics.append_child(el.clone());
+                frame
+                    .video_analytics
+                    .children
+                    .push(xmltree::XMLNode::Element(el.clone()));
             }
 
             let utc_time = running_time_to_utc_time(utc_time_running_time_mapping, running_time)
                 .unwrap_or(gst::ClockTime::ZERO);
 
-            for child in root.children() {
+            for child in root.children.iter().filter_map(|n| n.as_element()) {
                 let frame = queued_frames.entry(utc_time).or_insert_with(Frame::default);
 
-                if child.is("VideoAnalytics", "http://www.onvif.org/ver10/schema") {
-                    for subchild in child.children() {
-                        if subchild.is("Frame", "http://www.onvif.org/ver10/schema") {
+                if child.name == "VideoAnalytics"
+                    && child.namespace.as_deref() == Some(crate::ONVIF_METADATA_SCHEMA)
+                {
+                    for subchild in child.children.iter().filter_map(|n| n.as_element()) {
+                        if subchild.name == "Frame"
+                            && subchild.namespace.as_deref() == Some(crate::ONVIF_METADATA_SCHEMA)
+                        {
                             continue;
                         }
 
-                        frame.video_analytics.append_child(subchild.clone());
+                        frame
+                            .video_analytics
+                            .children
+                            .push(xmltree::XMLNode::Element(subchild.clone()));
                     }
                 } else {
                     frame.other_elements.push(child.clone());
@@ -678,8 +689,7 @@ impl OnvifMetadataParse {
                     }
                 };
 
-            if frame.video_analytics.children().next().is_none() && frame.other_elements.is_empty()
-            {
+            if frame.video_analytics.children.is_empty() && frame.other_elements.is_empty() {
                 // Generate a gap event if there's no actual data for this time
                 if !had_events {
                     data.push(BufferOrEvent::Event(
@@ -753,21 +763,30 @@ impl OnvifMetadataParse {
                 frame_pts
             );
 
-            let mut xml =
-                minidom::Element::builder("MetadataStream", "http://www.onvif.org/ver10/schema")
-                    .prefix(Some("tt".into()), "http://www.onvif.org/ver10/schema")
-                    .unwrap()
-                    .build();
+            let mut xml = xmltree::Element::new("MetadataStream");
+            xml.namespaces
+                .get_or_insert_with(|| xmltree::Namespace(Default::default()))
+                .put("tt", crate::ONVIF_METADATA_SCHEMA);
+            xml.namespace = Some(String::from(crate::ONVIF_METADATA_SCHEMA));
+            xml.prefix = Some(String::from("tt"));
 
-            if video_analytics.children().next().is_some() {
-                xml.append_child(video_analytics);
+            if !video_analytics.children.is_empty() {
+                xml.children
+                    .push(xmltree::XMLNode::Element(video_analytics));
             }
             for child in other_elements {
-                xml.append_child(child);
+                xml.children.push(xmltree::XMLNode::Element(child));
             }
 
             let mut vec = Vec::new();
-            if let Err(err) = xml.write_to_decl(&mut vec) {
+            if let Err(err) = xml.write_with_config(
+                &mut vec,
+                xmltree::EmitterConfig {
+                    write_document_declaration: false,
+                    perform_indent: true,
+                    ..xmltree::EmitterConfig::default()
+                },
+            ) {
                 gst::error!(CAT, imp: self, "Can't serialize XML element: {}", err);
                 for event in eos_events {
                     data.push(BufferOrEvent::Event(event));