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

Gstreamer plugin running in wayland with surfman 0.2 #25334

Merged
merged 6 commits into from Dec 20, 2019

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Dec 18, 2019

Tidying up the gstreamer plugin. The plugin now:

  • uses surfman 0.2
  • runs in wayland (but can't render WebGL content yet)
  • gets its GL configuration from gstreamer
  • uses GLsync if needed

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #24843
  • These changes do not require tests because
@highfive
Copy link

highfive commented Dec 18, 2019

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@asajeffrey asajeffrey force-pushed the asajeffrey:gstplugins-misc-tidying-up branch from 228858f to aca438a Dec 18, 2019
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 18, 2019

Oops, should have squashed before submitting the PR!

@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 18, 2019

@highfive highfive assigned Manishearth and unassigned SimonSapin Dec 18, 2019
@asajeffrey asajeffrey changed the title Gstplugins misc tidying up Gstreamer plugin running in wayland with surfman 0.2 Dec 19, 2019
@@ -31,9 +31,9 @@ libservo = {path = "../../components/servo"}
servo-media = {git = "https://github.com/servo/media"}
sparkle = "0.1"
# NOTE: the sm-angle-default feature only enables angle on windows, not other platforms!

This comment has been minimized.

Copy link
@jdm

jdm Dec 20, 2019

Member

Remove this note now that it no longer applies?

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Dec 20, 2019

Author Member

Done.

let sync_meta =
unsafe { gst_buffer_get_meta(gst_buffer.0, sync_meta_type) } as *mut GstGLSyncMeta;
if !sync_meta.is_null() {
unsafe { gst_gl_sync_meta_set_sync_point(sync_meta, gl_memory.mem.context) };
Copy link
Member Author

asajeffrey left a comment

Fixed.

@@ -31,9 +31,9 @@ libservo = {path = "../../components/servo"}
servo-media = {git = "https://github.com/servo/media"}
sparkle = "0.1"
# NOTE: the sm-angle-default feature only enables angle on windows, not other platforms!

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Dec 20, 2019

Author Member

Done.

let sync_meta =
unsafe { gst_buffer_get_meta(gst_buffer.0, sync_meta_type) } as *mut GstGLSyncMeta;
if !sync_meta.is_null() {
unsafe { gst_gl_sync_meta_set_sync_point(sync_meta, gl_memory.mem.context) };

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Dec 20, 2019

Author Member

Done.

@jdm
Copy link
Member

jdm commented Dec 20, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

📌 Commit 14e0c6a has been approved by jdm

@highfive highfive assigned jdm and unassigned Manishearth Dec 20, 2019
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

Testing commit 14e0c6a with merge b59003f...

bors-servo added a commit that referenced this pull request Dec 20, 2019
Gstreamer plugin running in wayland with surfman 0.2

<!-- Please describe your changes on the following line: -->

Tidying up the gstreamer plugin. The plugin now:

* uses surfman 0.2
* runs in wayland (but can't render WebGL content yet)
* gets its GL configuration from gstreamer
* uses GLsync if needed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24843
- [X] These changes do not require tests because

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 20, 2019

Windows plugin build is broken:

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:103:17
    |
103 | type Context = <Device as DeviceAPI>::Context;
    |                 ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:104:20
    |
104 | type Connection = <Device as DeviceAPI>::Connection;
    |                    ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:105:23
    |
105 | type NativeContext = <Device as DeviceAPI>::NativeContext;
    |                       ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:138:13
    |
138 |     device: Device,
    |             ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:140:27
    |
140 |     swap_chain: SwapChain<Device>,
    |                           ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:168:35
    |
168 |     GetSwapChain(Sender<SwapChain<Device>>),
    |                                   ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:182:27
    |
182 |     swap_chain: SwapChain<Device>,
    |                           ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:188:13
    |
188 |     device: Device,
    |             ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0412]: cannot find type `Device` in this scope
   --> ports\gstplugin\servowebsrc.rs:292:27
    |
292 |     swap_chain: SwapChain<Device>,
    |                           ^^^^^^ not found in this scope
    |
help: possible candidates are found in other modules, you can import them into scope
    |
5   | use gstreamer::Device;
    |
5   | use surfman::Device;
    |
5   | use surfman::device::Device;
    |
5   | use surfman::platform::default::device::Device;
    |
      and 1 other candidate

error[E0282]: type annotations needed for `std::sync::Mutex<std::option::Option<T>>`
   --> ports\gstplugin\servowebsrc.rs:497:37
    |
497 |         let connection = Mutex::new(None);
    |             ----------              ^^^^ cannot infer type for `T`
    |             |
    |             consider giving `connection` the explicit type `std::sync::Mutex<std::option::Option<T>>`, where the type parameter `T` is specified

error: unused import: `surfman_chains_api::SwapChainAPI`
  --> ports\gstplugin\servowebsrc.rs:95:5
   |
95 | use surfman_chains_api::SwapChainAPI;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `-D unused-imports` implied by `-D warnings`

error: aborting due to 11 previous errors
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 20, 2019

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

📌 Commit b5943f5 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

Testing commit b5943f5 with merge b638f56...

bors-servo added a commit that referenced this pull request Dec 20, 2019
Gstreamer plugin running in wayland with surfman 0.2

<!-- Please describe your changes on the following line: -->

Tidying up the gstreamer plugin. The plugin now:

* uses surfman 0.2
* runs in wayland (but can't render WebGL content yet)
* gets its GL configuration from gstreamer
* uses GLsync if needed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24843
- [X] These changes do not require tests because

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 20, 2019

Filed #25353, windows support will have to wait for me having a windows machine to test on.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 20, 2019

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

Testing commit b5943f5 with merge 62899c0...

bors-servo added a commit that referenced this pull request Dec 20, 2019
Gstreamer plugin running in wayland with surfman 0.2

<!-- Please describe your changes on the following line: -->

Tidying up the gstreamer plugin. The plugin now:

* uses surfman 0.2
* runs in wayland (but can't render WebGL content yet)
* gets its GL configuration from gstreamer
* uses GLsync if needed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24843
- [X] These changes do not require tests because

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 20, 2019

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 62899c0 to master...

@bors-servo bors-servo merged commit b5943f5 into servo:master Dec 20, 2019
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
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.

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