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

Negotiate GL textures and render them in player example #182

Merged
merged 15 commits into from Feb 21, 2019

Conversation

Projects
None yet
6 participants
@ceyusa
Copy link
Contributor

ceyusa commented Jan 23, 2019

This PR goes above PR #196

It needs to update the CI GStreamer setup, because it requires a new version of glib and gstreamer-gl library.

It contains

  • Port to webrender 0.58
  • Port to gstreamer git head version (0.13.0) -- this implied a complete rewrite of servosrc
  • Add a new trait in Frame to hold a reference to the backend's buffer
  • Update glutin version tu current git head
  • Add a method to set the GL parameters in the backend's player
  • Use glsinkbin in gstreamer player in GL parameters are negotiated
  • A lot of changes in example player to render gl textures is --gl argument is passed

Fixes: #166

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch 2 times, most recently from 8914c53 to 250e0a8 Jan 23, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

ceyusa commented Jan 28, 2019

Right now, WebRTC GStreamer backend doesn't compile: there are multiple errors of this type

error[E0623]: lifetime mismatch                                                                                                                                                                                    
  --> backends/gstreamer/src/media_capture.rs:20:36                                                                                                                                                                
   |                                                                                                                                                                                                               
17 |         builder: gst::caps::Builder,                                                                                                                                                                          
   |                  ------------------ this parameter and the return type are declared with different lifetimes...                                                                                               
18 |     ) -> Option<gst::caps::Builder> {                                                                                                                                                                         
   |          --------------------------                                                                                                                                                                           
19 |         match self {                                                                                                                                                                                          
20 |             Constrain::Value(v) => Some(builder.field(name, v)),                                                                                                                                              
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...but data from `builder` is returned here                                                                                                   
                                                                                                                     

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch 6 times, most recently from ec686bc to 503940a Jan 28, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

ceyusa commented Feb 5, 2019

Last Friday I confirmed my suspicion that the media's frame should keep a living reference to the associated GStreamer buffer, otherwise the buffer gets unreffed and the texture turns invalid.

Since media's frame is backend's agnostic, it would be hard to keep a reference to a opaque structure, so I tried out to turn frame into a trait.

But soon I got hit by FrameRenderer trait, which function render() receives a Frame, which now turned into a trait, FrameRenderer violates the rules of trait object safety. And I'm kind of trapped there.

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch from f392196 to 46480aa Feb 8, 2019

@ceyusa ceyusa changed the title WIP: GL textures Negotiate GL textures and render them in player example Feb 8, 2019

@ceyusa ceyusa referenced this pull request Feb 8, 2019

Closed

Update TravisCI for Xenial with Mesa fo GL #197

0 of 3 tasks complete

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch 4 times, most recently from 8da43f7 to 36c0488 Feb 11, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

ceyusa commented Feb 13, 2019

@ferjm
Copy link
Member

ferjm left a comment

Nice! Thank you so much for doing this work.

As mentioned on IRC, we need to figure out the codepath for OSX before being able to merge this.

I'd love to hear @sdroege final feedback as well :)

Show resolved Hide resolved backends/gstreamer/src/source.rs Outdated
Show resolved Hide resolved backends/gstreamer/src/player.rs Outdated
Show resolved Hide resolved backends/gstreamer/src/player.rs Outdated
Show resolved Hide resolved backends/gstreamer/src/player.rs Outdated
@sdroege

This comment has been minimized.

Copy link
Contributor

sdroege commented Feb 14, 2019

I'd love to hear @sdroege final feedback as well :)

Won't get to that today but I'll put it on my list.

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch 2 times, most recently from 5fd9fb1 to 67ec36c Feb 14, 2019

Show resolved Hide resolved backends/gstreamer/Cargo.toml Outdated
@sdroege

This comment has been minimized.

Copy link
Contributor

sdroege commented Feb 20, 2019

Use expect() for programmig errors

Also typo in this commit message, it's missing a "n" in "programming" :)

ceyusa added some commits Jan 9, 2019

Port to git version of gstreamer (0.13.0)
This implies

* to use glib::GString for media_info data
* gst::Buffer::from_slice() doesn't fail
* New returns as Result simplifyng code
* Rewrite ServoSrc element using subclassing
* add life times to media_capture caps builder
* enable NLL for gstreamer crate
Use glutin git version
Since the current unreleased version has merged the required API to
get the used EGL Display.
Add Buffer trait to Frame
And implement it in GStreamer backend. The purpose of this trait is to
keep alive the reference of the backend's buffer while it is used.
Use expect() for programming errors
For programming errors it is better to panic using expect(). This
patch does this for GObject's set_property and get_property.
Handle --gl argument in example
The purpose of this argument is to select if the user wants to
render textures or images.

*This is just a mock up*, textures are not handled yet.
Set app's GL context to player
Move upper the player_wrapper instantiation in order to set the gl
usage as soon as possible.
Use glsinkbin and handle bus sync messages
If gl display and context are set, the glsinkbin is used,
otherwise only appsink.

Now GL textures are negotiated and rendered.
Enable player example to render textures
If buffer contains a GL texture it will return the texture id

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch from bc125a6 to 244058e Feb 20, 2019

@ceyusa ceyusa force-pushed the ceyusa:gl-textures branch from 244058e to 6a3be48 Feb 20, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

ceyusa commented Feb 20, 2019

I guess all (or most of) the issues were solved :)

@sdroege

This comment has been minimized.

Copy link
Contributor

sdroege commented Feb 20, 2019

I guess all (or most of) the issues were solved :)

🎊 Nothing else from my side, I guess :)

@ferjm

This comment has been minimized.

Copy link
Member

ferjm commented Feb 21, 2019

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Feb 21, 2019

@bors-servo r=ferjm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 21, 2019

📌 Commit 6a3be48 has been approved by ferjm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 21, 2019

⌛️ Testing commit 6a3be48 with merge 888668f...

bors-servo added a commit that referenced this pull request Feb 21, 2019

Auto merge of #182 - ceyusa:gl-textures, r=ferjm
Negotiate  GL textures and render them in player example

This PR goes above PR #196

It needs to update the CI GStreamer setup, because it requires  a new version of glib and gstreamer-gl library.

It contains

* Port to webrender 0.58
* Port to gstreamer git head version (0.13.0) -- this implied a complete rewrite of servosrc
* Add a new trait in Frame to hold a reference to the backend's buffer
* Update glutin version tu current git head
* Add a method to set the GL parameters in the backend's player
* Use glsinkbin in gstreamer player in GL parameters are negotiated
* A lot of changes in example player to render gl textures is --gl argument is passed

Fixes: #166
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 21, 2019

☀️ Test successful - checks-travis
Approved by: ferjm
Pushing 888668f to master...

@bors-servo bors-servo merged commit 6a3be48 into servo:master Feb 21, 2019

1 check passed

homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.