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 should use GLMemory #25048

Merged
merged 2 commits into from Dec 6, 2019
Merged

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Dec 3, 2019

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #24831
  • These changes do not require tests because it's an embedding perf issue
@highfive
Copy link

highfive commented Dec 3, 2019

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@jdm
Copy link
Member

jdm commented Dec 4, 2019

r? @ferjm

@highfive highfive assigned ferjm and unassigned SimonSapin Dec 4, 2019
@ferjm
ferjm approved these changes Dec 4, 2019
Copy link
Member

ferjm left a comment

As someone who's been fighting with GStreamer for a while, I'm trully amazed by how fast you got all this to work :)

CoreError::Failed,
["Failed to map output buffer writable"]
);
let memory = unsafe { memory.into_ptr() };

This comment has been minimized.

Copy link
@ferjm

ferjm Dec 4, 2019

Member

Regarding #24831 (comment) did you finally try with as_ptr?

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Dec 5, 2019

Author Member

I've been focusing on the issue with buffer allocation rather than the space leak atm. I got buffer allocation to work over in my playground plugin, I'm going to port it here, and look to see if this makes the space leak magically go away. asajeffrey/my-gst-plugin@14f6657

Copy link
Member Author

asajeffrey left a comment

Thanks!

@ferjm
Copy link
Member

ferjm commented Dec 5, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

📌 Commit e53bcfe has been approved by ferjm

@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 5, 2019

I filed the follow-up issues.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

Testing commit e53bcfe with merge 0d2503e...

bors-servo added a commit that referenced this pull request Dec 5, 2019
GStreamer plugin should use GLMemory

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

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.

---
<!-- 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 #24831
- [x] These changes do not require tests because it's an embedding perf issue

<!-- 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 5, 2019

💔 Test failed - status-taskcluster

@asajeffrey asajeffrey force-pushed the asajeffrey:gstplugin-glmemory branch from e53bcfe to 06a10c5 Dec 5, 2019
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 5, 2019

Oops, broke the windows build. @bors-servo r=ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

📌 Commit 06a10c5 has been approved by ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

Testing commit 06a10c5 with merge b4db5c4...

bors-servo added a commit that referenced this pull request Dec 5, 2019
GStreamer plugin should use GLMemory

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

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.

---
<!-- 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 #24831
- [x] These changes do not require tests because it's an embedding perf issue

<!-- 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 5, 2019

💔 Test failed - status-taskcluster

@asajeffrey asajeffrey mentioned this pull request Dec 5, 2019
4 of 4 tasks complete
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 5, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

Testing commit 06a10c5 with merge 38eb89c...

bors-servo added a commit that referenced this pull request Dec 5, 2019
GStreamer plugin should use GLMemory

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

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.

---
<!-- 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 #24831
- [x] These changes do not require tests because it's an embedding perf issue

<!-- 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 5, 2019

💔 Test failed - status-taskcluster

@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 5, 2019

Lets see if disabling the gstreamer media stack in windows does the job. (If it does, I'll file a follow-up to re-enable it). @bors-servo try=windows

@bors-servo
Copy link
Contributor

bors-servo commented Dec 5, 2019

Trying commit c69360f with merge 78120f1...

bors-servo added a commit that referenced this pull request Dec 5, 2019
GStreamer plugin should use GLMemory

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

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.

---
<!-- 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 #24831
- [x] These changes do not require tests because it's an embedding perf issue

<!-- 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 6, 2019

☀️ Test successful - status-taskcluster
State: approved= try=True

@asajeffrey asajeffrey force-pushed the asajeffrey:gstplugin-glmemory branch from c69360f to 445ee9b Dec 6, 2019
@asajeffrey
Copy link
Member Author

asajeffrey commented Dec 6, 2019

Filed #25189. @bors-servo r=ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 6, 2019

📌 Commit 445ee9b has been approved by ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Dec 6, 2019

Testing commit 445ee9b with merge 2d2cd2b...

bors-servo added a commit that referenced this pull request Dec 6, 2019
GStreamer plugin should use GLMemory

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

Get the gstreamer servosrc plugin to generate frames in GLMemory rather than main memory.

---
<!-- 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 #24831
- [x] These changes do not require tests because it's an embedding perf issue

<!-- 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 6, 2019

☀️ Test successful - status-taskcluster
Approved by: ferjm
Pushing 2d2cd2b to master...

@bors-servo bors-servo merged commit 445ee9b into servo:master Dec 6, 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.