Skip to content

Add Rust client library fro GstD with examples#338

Merged
lmerayo1 merged 30 commits into
developfrom
feature/add-rust-api
May 28, 2026
Merged

Add Rust client library fro GstD with examples#338
lmerayo1 merged 30 commits into
developfrom
feature/add-rust-api

Conversation

@lmerayo1

Copy link
Copy Markdown
Contributor

No description provided.

@lmerayo1 lmerayo1 requested a review from michaelgruner March 27, 2026 21:20
@lmerayo1 lmerayo1 self-assigned this Mar 27, 2026
Comment thread libgstc/rust/gstc/src/json.rs Outdated
@@ -0,0 +1,211 @@
/*
* This file is part of GStreamer Daemon
* Copyright 2015-2022 Ridgerun, LLC (http://www.ridgerun.com)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to:

Copyright 2015-2026 RidgeRun, LLC (http://www.ridgerun.com)

Apply this to all the files in the PR

Comment thread examples/libgstc/meson.build Outdated
install: false)
endforeach

subdir('rust')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a README inside examples/libgstc/rust/ explaining what each example does and how to run them

@lmerayo1 lmerayo1 force-pushed the feature/add-rust-api branch from e9fd940 to 1caf51e Compare March 27, 2026 22:02
println!("Pipeline set to playing!");

println!("Press enter to stop the pipeline...");
let (tx, rx) = mpsc::channel::<()>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly think mpsc is overkill here. I'd recommend a simple Atomic boolean for the exit flag.


let abs_path = PathBuf::from(&video)
.canonicalize()
.unwrap_or_else(|_| PathBuf::from(video));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer borrowing here PathBuf::from(&video)

println!("Pipeline set to playing!");

println!("Press enter to stop the pipeline...");
let (tx, rx) = mpsc::channel::<()>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here

Comment on lines +47 to +58
if bus_message.status == Status::OK {
println!("received!");
} else if bus_message.status == Status::BUS_TIMEOUT {
println!("timeout!");
eprintln!("EOS not received, file may be unreadable");
} else {
println!("error!");
eprintln!(
"An error occurred waiting for EOS: {}",
bus_message.status.0
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer matching here.

@@ -0,0 +1,45 @@
/*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this example name. Maybe wait_on_bus or something like that.

Comment thread libgstc/rust/gstc/src/status.rs Outdated
use std::fmt;

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Status(pub i32);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a struct and not an enum? An enum is the more intuitive option here.

}

pub(crate) struct Transport {
settings: ConnectionSettings,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we make this public so the client can read our settings.

Comment thread libgstc/rust/gstc/src/transport.rs Outdated
Comment on lines +83 to +86
if self.stream.is_none() {
let stream = self.open_socket()?;
self.stream = Some(stream);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, this overprotection is an antipattern. If this happens, this is an error and we don't want to silence it. Why would the stream be closed? If we opened with keep_connection_open the stream should be open, if not, something went wrong and we should react accordingly.

Comment thread libgstc/rust/gstc/src/transport.rs Outdated
Comment on lines +106 to +114
if timeout_ms < 0 {
stream
.set_read_timeout(None)
.map_err(|_| Status::SOCKET_ERROR)?;
} else {
stream
.set_read_timeout(Some(Duration::from_millis(timeout_ms as u64)))
.map_err(|_| Status::SOCKET_ERROR)?;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor this to set the timeout instead of duplicating all the code.

Comment thread libgstc/rust/meson.build Outdated
Comment on lines +1 to +7
rust_enabled = add_languages('rust', required : false)

if rust_enabled
subdir('gstc')
else
message('Rust compiler not found; skipping libgstc Rust API')
endif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the same pattern we use with python.

@lmerayo1 lmerayo1 requested a review from michaelgruner April 13, 2026 21:12

println!("EOS message received!");

client.pipeline_seek("pipe", 1.0, 3, 1, 1, 0, 1, -1)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make local constants for these magic numbers? to give us an idea of what they are.

Comment thread examples/libgstc/rust/meson.build Outdated
@@ -0,0 +1,25 @@
if is_variable('lib_gstc_rust_dep')
rustc = find_program('rustc')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meson supports rust by default. Don't build manually. Add rust as the list of project languages and it will do it on its own.

Comment thread examples/libgstc/rust/meson.build Outdated

foreach ex : rust_examples
executable(ex[0], ex[1],
rust_args : ['--edition=2021'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this do the default options as rust_std=2021


use crate::Status;

pub(crate) fn json_get_int(json: &str, name: &str) -> Result<i32, Status> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vote for using meson 1.11, pip already has meson 1.11.1. But creating a JSON parser from scratch is something i'm not interested in maintaining.

println!("EOS sent!");

print!("Waiting for EOS... ");
let bus_message = client.pipeline_bus_wait("pipe", "eos", 10_000_000_000)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we treat Status::BusTimeout as an error as well. This is counterintuitive., IMO.

match client.pipeline_bus_wait("pipe", "eos", 10_000_000_000) {
    Ok(_bus_message) => {
        println!("received!");
    }
    Err(Status::BusTimeout) => {
        println!("timeout!");
        eprintln!("EOS not received, file may be unreadable");
    }
    Err(status) => {
        println!("error!");
        eprintln!(
            "An error occurred waiting for EOS: {}",
            status.code()
        );
    }
}

Comment thread libgstc/rust/gstc/src/client.rs Outdated
Comment on lines +272 to +281
let status = match json_is_null_field(&raw, "response") {
Ok(is_null) if is_null => Status::BusTimeout,
Ok(_) => Status::Ok,
Err(err) => err,
};

Ok(BusMessage {
status,
raw_response: raw,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's treat BusTimeout as an Err()

Comment thread libgstc/rust/gstc/src/client.rs Outdated
Comment on lines +294 to +301
self.cmd_update(
&format!("/pipelines/{}/bus/types", pipeline_name),
&message_name,
)?;
self.cmd_update(
&format!("/pipelines/{}/bus/timeout", pipeline_name),
&format!("{}", timeout_ns),
)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This values are already written by pipeline_bus_wait(), which you are already calling. Why write them twice?

* Boston, MA 02110-1301, USA.
*/

use gstc_rust::{Client, Status};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop the rust in gstc_rust. Just gstc.

Comment thread libgstc/rust/gstc/src/client.rs Outdated
),
self.default_wait_time_ms()?,
)?;
Ok(())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary.

Comment on lines +401 to +411
fn cmd_send(&self, request: &str) -> Result<(), Status> {
let response = self.send_request(request, self.default_wait_time_ms()?)?;
let code = json_get_int(&response, "code")?;
let status = Status::from_code(code);

if status.is_ok() {
Ok(())
} else {
Err(status)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrite this in terms of cmd_send_get_response

let client = Client::new("127.0.0.1", 5000, -1, true)?;
let output = env::current_dir().map(|dir| dir.join("mp4_recording.mp4"))?;
let output = output.to_string_lossy();
let output = output.replace('\\', "\\\\").replace('"', "\\\"");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be manually escaping. 1) Do we really need to escape? 2) If so, let's use something specific for escaping shell strings, like shell_escape::unix::escape, but not manually.

fn main() -> Result<(), Status> {
let client = Client::new("127.0.0.1", 5000, 5000, false)?;

client.pipeline_create("pipe", "videotestsrc ! fakesink")?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please either use videotestsrc is-live=true or fakesink sync=true otherwise we are going to eat up the processor.

@lmerayo1 lmerayo1 merged commit 1d65181 into develop May 28, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants