Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to gstreamer-rs 0.14 #272

Merged
merged 1 commit into from Jun 26, 2019
Merged

Update to gstreamer-rs 0.14 #272

merged 1 commit into from Jun 26, 2019

Conversation

@ceyusa
Copy link
Contributor

ceyusa commented Jun 26, 2019

No description provided.

@@ -220,7 +218,7 @@ impl AudioDecoder for GStreamerAudioDecoder {
})?;

for position in positions.iter() {
let buffer = buffer.clone();
let buffer = buffer.to_owned();

This comment has been minimized.

@sdroege

sdroege Jun 26, 2019

Contributor

Why is this necessary, a whole buffer copy instead of just getting a new reference?

This comment has been minimized.

@sdroege

sdroege Jun 26, 2019

Contributor

There's Sample::get_buffer_owned() if that's the issue here, which allows to do this without a copy of the buffer. See https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/issues/204 for the background.

@@ -96,9 +96,9 @@ impl GStreamerRender {
}

pub fn get_frame_from_sample(&self, sample: &gst::Sample) -> Result<Frame, ()> {
let buffer = sample.get_buffer().ok_or_else(|| ())?;
let buffer = sample.get_buffer().ok_or_else(|| ())?.to_owned();

This comment has been minimized.

@sdroege

sdroege Jun 26, 2019

Contributor

Same question here

@sdroege
Copy link
Contributor

sdroege commented Jun 26, 2019

Generally it looks like the update improved the code a bit and made it nicer to read. Or what do you think? :)

@ceyusa ceyusa force-pushed the ceyusa:gstreamer-rs-0.14 branch from ce639f8 to bddabeb Jun 26, 2019
@ceyusa
Copy link
Contributor Author

ceyusa commented Jun 26, 2019

Generally it looks like the update improved the code a bit and made it nicer to read. Or what do you think? :)

Agree, it feels more rusty, in the sense of the language, of course :)

Thanks @sdroege, specially for the get_buffer_owned() hint!

@Manishearth
Copy link
Member

Manishearth commented Jun 26, 2019

@bors-servo r+

Should also fix the unsafe code used in webrtcbin for https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/issues/189

@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2019

📌 Commit bddabeb has been approved by Manishearth

bors-servo added a commit that referenced this pull request Jun 26, 2019
Update to gstreamer-rs 0.14
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2019

Testing commit bddabeb with merge 89c48f7...

@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2019

☀️ Test successful - checks-travis
Approved by: Manishearth
Pushing 89c48f7 to master...

@bors-servo bors-servo merged commit bddabeb into servo:master Jun 26, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@ceyusa ceyusa deleted the ceyusa:gstreamer-rs-0.14 branch Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.