Fix building argument-less messages

Building an argument-less message such as eos yields an assertion failure due to the inner structure being null. The short term solution consists in checking that the inner `structure` is not `null` before attempting to insert `other_fields`. The consequence is that `others_fields` defined for argument-less messages will be ignored. A correction will be applied when GStreamer 1.14 is released thank to the introduction of `gst_message_writable_structure` (see https://bugzilla.gnome.org/show_bug.cgi?id=792928). Due to the dependency on GStreamer 1.14, the correction will be only available under the activation of a feature "v1_14".

Events are not affected as the build method the availability of `gst_event_writable_structure` and this function "will never return NULL". However, we can avoid a `structure` allocation for argument-less messages without `other_fields`.
This commit is contained in:
fengalin 2018-01-27 12:52:03 +01:00 committed by Sebastian Dröge
parent 7fe2216daf
commit 658be807c1
2 changed files with 95 additions and 7 deletions

View file

@ -895,7 +895,7 @@ macro_rules! event_builder_generic_impl {
ffi::gst_event_set_running_time_offset(event, running_time_offset);
}
{
if !self.other_fields.is_empty() {
let s = StructureRef::from_glib_borrow_mut(
ffi::gst_event_writable_structure(event)
);
@ -1667,3 +1667,49 @@ impl<'a> CustomBothOobBuilder<'a> {
ev
});
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_simple() {
::init().unwrap();
// Event without arguments
let flush_start_evt = Event::new_flush_start().build();
match flush_start_evt.view() {
EventView::FlushStart(flush_start_evt) => {
assert!(flush_start_evt.0.get_structure().is_none());
},
_ => panic!("flush_start_evt.view() is not an EventView::FlushStart(_)"),
}
let flush_start_evt = Event::new_flush_start()
.other_fields(&[("extra-field", &true)])
.build();
match flush_start_evt.view() {
EventView::FlushStart(flush_start_evt) => {
assert!(flush_start_evt.0.get_structure().is_some());
if let Some(other_fields) = flush_start_evt.0.get_structure() {
assert!(other_fields.has_field("extra-field"));
}
},
_ => panic!("flush_start_evt.view() is not an EventView::FlushStart(_)"),
}
// Event with arguments
let flush_stop_evt = Event::new_flush_stop(true)
.other_fields(&[("extra-field", &true)])
.build();
match flush_stop_evt.view() {
EventView::FlushStop(flush_stop_evt) => {
assert_eq!(flush_stop_evt.get_reset_time(), true);
assert!(flush_stop_evt.0.get_structure().is_some());
if let Some(other_fields) = flush_stop_evt.0.get_structure() {
assert!(other_fields.has_field("extra-field"));
}
}
_ => panic!("flush_stop_evt.view() is not an EventView::FlushStop(_)"),
}
}
}

View file

@ -1165,6 +1165,8 @@ macro_rules! message_builder_generic_impl {
}
}
// Warning: other_fields are ignored with argument-less messages
// until GStreamer 1.14 is released
pub fn other_fields(self, other_fields: &[(&'a str, &'a ToSendValue)]) -> Self {
Self {
other_fields: self.other_fields.iter().cloned()
@ -1183,13 +1185,19 @@ macro_rules! message_builder_generic_impl {
ffi::gst_message_set_seqnum(msg, seqnum.to_glib());
}
{
let s = StructureRef::from_glib_borrow_mut(
ffi::gst_message_get_structure(msg) as *mut _
);
if !self.other_fields.is_empty() {
// issue with argument-less messages. We need the function
// ffi::gst_message_writable_structure to sort this out
// and this function will be available in GStreamer 1.14
// See https://github.com/sdroege/gstreamer-rs/pull/75
// and https://bugzilla.gnome.org/show_bug.cgi?id=792928
let structure = ffi::gst_message_get_structure(msg);
if !structure.is_null() {
let structure = StructureRef::from_glib_borrow_mut(structure as *mut _);
for (k, v) in self.other_fields {
s.set_value(k, v.to_send_value());
structure.set_value(k, v.to_send_value());
}
}
}
@ -2442,3 +2450,37 @@ impl<'a> RedirectBuilder<'a> {
msg
});
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_simple() {
::init().unwrap();
// Message without arguments
let eos_msg = Message::new_eos().build();
match eos_msg.view() {
MessageView::Eos(eos_msg) => {
assert!(eos_msg.0.get_structure().is_none());
},
_ => panic!("eos_msg.view() is not a MessageView::Eos(_)"),
}
// Note: can't define other_fields for argument-less messages before GStreamer 1.14
// Message with arguments
let buffering_msg = Message::new_buffering(42)
.other_fields(&[("extra-field", &true)])
.build();
match buffering_msg.view() {
MessageView::Buffering(buffering_msg) => {
assert_eq!(buffering_msg.get_percent(), 42);
assert!(buffering_msg.0.get_structure().is_some());
assert!(buffering_msg.0.get_structure().unwrap().has_field("extra-field"));
}
_ => panic!("buffering_msg.view() is not a MessageView::Buffering(_)"),
}
}
}