Add Rust GStreamer plugins #291

pedro-alves-ferreira · 2025-12-17T19:25:31Z

This PR adds GStreamer source and sink plugins.

Signed-off-by: Simao Fonseca <simaofonseca@bisect.pt>

Signed-off-by: Simao Fonseca <simao.fonseca@bisect.pt>

There is an indirect dependency that uses the "Apache-2.0 WITH LLVM-exception" license. This commits adds that licensed to the allowed list. Signed-off-by: Pedro Ferreira <pedro@bisect.pt>

jonasohland · 2025-12-18T15:17:03Z

Thanks for all the work! I was able to get a few basic pipelines working without issues. Will do a proper review of the rust code later.

pedro-alves-ferreira · 2025-12-18T15:54:44Z

Thanks for the feedback, @jonasohland. There's still some work to be done around reopening the flow, clean shutdown, etc. but we're almost there.

mlefebvre1

I've only went through half of the PR. But here's my comment so far.

mlefebvre1 · 2025-12-22T15:25:09Z

rust/mxl/src/flow/flowdef.rs

These could be moved and maintained in the mxl crate.

mlefebvre1 · 2025-12-22T15:29:27Z

rust/gst-mxl-rs/src/flowdef.rs

+    pub parents: Vec<String>,
+}
+
+pub struct FlowDef {


Why not use an enum instead? A flow can't be both at the same time.

mlefebvre1 · 2025-12-22T15:29:44Z

rust/gst-mxl-rs/src/flowdef.rs

+    pub format: String,
+    pub tags: HashMap<String, Vec<String>>,
+    pub label: String,
+    pub id: String,


Could use the uuid crate to add validation.

mlefebvre1 · 2025-12-22T15:54:34Z

rust/gst-mxl-rs/src/mxlsrc/imp.rs

+        if let Ok(settings) = self.settings.lock() {
+            match pspec.name() {
+                "video-flow" => settings.video_flow.to_value(),
+                "audio-flow" => settings.video_flow.to_value(),


Should be settings.audio_flow.to_value()

mlefebvre1 · 2025-12-22T16:01:31Z

rust/gst-mxl-rs/src/mxlsrc/imp.rs

+    fn properties() -> &'static [glib::ParamSpec] {
+        static PROPERTIES: LazyLock<Vec<glib::ParamSpec>> = LazyLock::new(|| {
+            vec![
+                glib::ParamSpecString::builder("video-flow")


Should not the attribute be named video-flow-id?
Same applies to audio-flow

mlefebvre1 · 2025-12-22T17:01:49Z

rust/gst-mxl-rs/src/mxlsrc/create_video.rs

+        }
+    }
+
+    trace!("PTS: {:?} GST-CURRENT: {:?}", buffer.pts(), ts_gst);


As a general comment, using structured logging is better for logs processing.

trace!(pts=?buffer.pts(), ts_gst=?ts_gst, buffer=?buffer, "Produced buffer");

mlefebvre1 · 2025-12-22T17:02:54Z

rust/gst-mxl-rs/src/mxlsrc/create_video.rs

+
+    trace!("PTS: {:?} GST-CURRENT: {:?}", buffer.pts(), ts_gst);
+    trace!("Produced buffer {:?}", buffer);
+    if video_state.frame_counter == 0 {


Hmm, can a comment be added here to explain this logic?

mlefebvre1 · 2025-12-22T17:09:09Z

rust/gst-mxl-rs/src/mxlsrc/create_audio.rs

+        .sample_rate()
+        .map_err(|_| gst::FlowError::Error)?;
+
+    let batch_size = DEFAULT_BATCH_SIZE.min(continuous_flow_info.bufferLength / 2);


Do note that reader_info.config.common().max_commit_batch_size_hint() indicates the maximum number of samples a writer can commit at once. This DEFAULT_BATCH_SIZE could be bigger if left unchecked, thus violating that contract.

mlefebvre1 · 2025-12-22T17:22:11Z

rust/gst-mxl-rs/src/mxlsrc/create_audio.rs

+    );
+
+    let mut head = reader_info.runtime.head_index();
+    wait_for_sample(head, batch, audio_state)?;


Any reason why we just don't use samples_reader.get_samples() to wait for samples, use error variants to determine if we are OutOfRangeTooLate or OutOfRangeTooEarly, and act accordingly.

mlefebvre1 · 2025-12-22T17:39:46Z

rust/gst-mxl-rs/src/mxlsrc/create_audio.rs

+
+    let buffer = build_buffer(pts, samples, is_discont, interleaved)?;
+
+    audio_state.batch_counter += 1;


Why do we need to keep synchronized a batch_counter with the index. Unless I'm missing something, we can infer it from the index.

mlefebvre1 · 2025-12-22T18:36:10Z

rust/gst-mxl-rs/src/mxlsrc/create_video.rs

+        end_hms,
+        elapsed_real.as_millis()
+    );
+    let _ = initial_info;


Could be removed

let _ = initial_info; let initial_info = &state.initial_info;

mlefebvre1 · 2025-12-22T18:38:28Z

rust/gst-mxl-rs/src/mxlsrc/create_video.rs

+    let pts = gst::ClockTime::from_nseconds(pts as u64);
+
+    let mut pts = pts + initial_info.gst_time;
+    let _ = initial_info;


Remove let _ = initial_info;
You are already dropping the shared reference when you create the exclusive reference at line 107.

mlefebvre1 · 2025-12-22T19:01:22Z

rust/gst-mxl-rs/src/mxlsink/render_video.rs

+            .map_err(|_| gst::FlowError::Error)?,
+    );
+    let video_state = state.video.as_mut().ok_or(gst::FlowError::Error)?;
+    let gst_time = mxlsink


Can you explain where and the how the clock type is set from this plugin. If it is using GST_CLOCK_TYPE_MONOTONIC, in the rare case of a leap of second, there could be an offset between this plugin producer and a consumer that uses mxl/flow.h (which uses CLOCK_TAI instead).

The plugin does not propose any clock. It uses the pipeline's clock. It is the user's responsibility to set one or to use the default, which is monotonic, as explained in this documentation.
In regards to the leap of second case, it is to my understanding that monotonic clocks won't cause an offset between the producer and consumer, that case occurs in real-time clocks.

mlefebvre1 · 2025-12-22T19:04:14Z

rust/gst-mxl-rs/src/mxlsink/render_video.rs

+                index,
+                current_index,
+                gst_time,
+                if pts > gst_time {


pts.saturating_sub(gst_time)

mlefebvre1 · 2025-12-22T19:10:33Z

rust/gst-mxl-rs/src/mxlsink/render_video.rs

+    access
+        .commit(copy_len as u16)
+        .map_err(|_| gst::FlowError::Error)?;
+    trace!(


Just wondering, is this log still relevant? Why would you need to measure the time it takes copy and commit?

Agreed, this was only useful for an early development bug and probably should’ve been removed sooner.

mlefebvre1 · 2025-12-22T19:12:01Z

rust/gst-mxl-rs/src/mxlsink/render_video.rs

+    let mut copy_len = std::cmp::min(payload.len(), data.len());
+    let commit_time = Instant::now();
+    payload[..copy_len].copy_from_slice(&data[..copy_len]);
+    if copy_len > access.total_slices() as usize {


We're comparing the payload size (in bytes) vs the number of slices. This will always be true ...

If you still want to do the check, you will have to obtain the number of bytes per slice which you can get from the field sliceSizes of DiscreteFlowConfigInfo.

mlefebvre1 · 2025-12-22T19:27:07Z

rust/gst-mxl-rs/src/mxlsink/render_audio.rs

+            .map_err(|_| gst::FlowError::Error)?
+            + initial_info.index;
+
+        if write_index > current_index + buffer_length {


use std::cmp::min instead?

mlefebvre1 · 2025-12-22T19:49:28Z

rust/gst-mxl-rs/src/mxlsink/render_audio.rs

+        write_index, sample_rate.numerator, sample_rate.denominator
+    );
+
+    let max_chunk = (buffer_length / 2) as usize;


I suppose it's not possible to configure a max size for the gstreamer buffers we receive?

It's not possible, you can't restrict the max size of the buffers you receive in GStreamer.

mlefebvre1 · 2025-12-22T19:54:08Z

rust/gst-mxl-rs/src/mxlsink/state.rs

+    state.audio = Some(AudioState {
+        writer,
+        bit_depth,
+        batch_size: (rate as usize / 100),


Using mxl flow_info you should be able to retrieve the max_commit_batch_size_hint instead of arbitrary setting this.

pedro-alves-ferreira · 2025-12-22T19:55:50Z

Thanks, @mlefebvre1! We will review your comments a.s.a.p.

mlefebvre1 · 2025-12-23T02:32:50Z

Thanks, @mlefebvre1! We will review your comments a.s.a.p.

No worries @pedro-alves-ferreira, most of the comments are suggestions aimed at improving code quality and I would be fine with them being addressed after v1.0. You could focus on resolving these comments for v1.0 :
#291 (comment)
#291 (comment)
#291 (comment)
#291 (comment)
#291 (comment)
#291 (comment)

Signed-off-by: Simaofonseca <simao.fonseca@bisect.pt>

Signed-off-by: Simao Fonseca <simao.fonseca@bisect.pt>

jonasohland · 2025-12-24T11:42:03Z

Thanks for the feedback, @jonasohland. There's still some work to be done around reopening the flow, clean shutdown, etc. but we're almost there.

Flow deletion, reopening and those things should be all automatic now that #290 has been merged. In the rust api everything should be RAII now. Hope that helps!

pedro-alves-ferreira · 2025-12-24T11:45:46Z

Thanks for the feedback, @jonasohland. There's still some work to be done around reopening the flow, clean shutdown, etc. but we're almost there.

Flow deletion, reopening and those things should be all automatic now that #290 has been merged. In the rust api everything should be RAII now. Hope that helps!

Yes, that was quite an improvement. Thanks!

pedro-alves-ferreira requested review from a team and vt-tv December 17, 2025 19:25

Simao Fonseca and others added 3 commits December 18, 2025 12:39

gst-plugins: add sinesrc as base for future plugin

601c271

Signed-off-by: Simao Fonseca <simaofonseca@bisect.pt>

gst-plugins: implement MXL GStreamer plugins

5ed8c58

Signed-off-by: Simao Fonseca <simao.fonseca@bisect.pt>

rust: allow apache licenses with llvm exception

ca1eae1

There is an indirect dependency that uses the "Apache-2.0 WITH LLVM-exception" license. This commits adds that licensed to the allowed list. Signed-off-by: Pedro Ferreira <pedro@bisect.pt>

pedro-alves-ferreira force-pushed the feature/add-gst-plugins branch from 87b5249 to ca1eae1 Compare December 18, 2025 12:40

felixpou linked an issue Dec 18, 2025 that may be closed by this pull request

GStreamer/rust modules: video and audio #63

Open

vt-tv requested review from jonasohland and pac-work December 18, 2025 16:30

felixpou mentioned this pull request Dec 22, 2025

GStreamer/rust modules: video and audio #63

Open

felixpou added this to the v1.0 milestone Dec 22, 2025

mlefebvre1 requested changes Dec 22, 2025

View reviewed changes

SimaoPFonseca added 2 commits December 24, 2025 10:50

gst-plugins: add timeout when waiting for producer

f6e32ad

Signed-off-by: Simaofonseca <simao.fonseca@bisect.pt>

gst-plugins: Improved code quality and fixed naming inconsistencies

2d91939

Signed-off-by: Simao Fonseca <simao.fonseca@bisect.pt>


		let buffer = build_buffer(pts, samples, is_discont, interleaved)?;

		audio_state.batch_counter += 1;

Add Rust GStreamer plugins #291

Are you sure you want to change the base?

Add Rust GStreamer plugins #291

Uh oh!

Conversation

pedro-alves-ferreira commented Dec 17, 2025

Uh oh!

jonasohland commented Dec 18, 2025

Uh oh!

pedro-alves-ferreira commented Dec 18, 2025

Uh oh!

mlefebvre1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedro-alves-ferreira commented Dec 22, 2025

Uh oh!

mlefebvre1 commented Dec 23, 2025

Uh oh!

jonasohland commented Dec 24, 2025

Uh oh!

pedro-alves-ferreira commented Dec 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants