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

GstPlayer GL fixes. #2039

Merged
merged 2 commits into from Oct 21, 2018

Conversation

Projects
None yet
3 participants
@PetrosKataras
Copy link
Contributor

PetrosKataras commented Oct 14, 2018

This PR contains some critical fixes for the case of multiple gst-gl pipelines running in parallel. It should address issues like the one described in the following forum post https://discourse.libcinder.org/t/gstreamer-linux-issue/1312 and also some other memory issues that would manifest deep in the GStreamer pipeline.

A note that these changes render #1947 obsolete. I could open a new PR with an updated patch for Windows once this is merged and assuming there is interest.

@brucelane

This comment has been minimized.

Copy link

brucelane commented Oct 18, 2018

I'm interested in this for Windows, thank you

@richardeakin

This comment has been minimized.

Copy link
Collaborator

richardeakin commented Oct 19, 2018

By obsolete you mean it will be conflicting afterwards? I was under the impression that #1947 was only adding some things and even if gstreamer stays as experimental on windows at the moment, it isn't causing any new issues. If this is correct I say we move towards getting them both merged (and sorry it grew a little stagnant).

@PetrosKataras

This comment has been minimized.

Copy link
Contributor

PetrosKataras commented Oct 20, 2018

Hey Rich, by obsolete I mean that it would be easier/faster to create a new patch that replaces #1947 on top of this PR than resolving conflicts that have been introduced since it was open. There are already some conflicts in #1947 since the optional module ( i.e video, audio etc ) disablement was merged. This PR changes the initialization code for the player in the case of gst-gl and it also streamlines it so that the same approach can be used in all platforms ( in contrast with #1947 where only Windows follows a different initialization path ). My current thinking is that it doesn't worth the time to git-resolve these conflicts and should be easier to just open a new PR which integrates the Windows changes on top of this one.

@richardeakin

This comment has been minimized.

Copy link
Collaborator

richardeakin commented Oct 21, 2018

Ok makes sense to me too, merging.

@richardeakin richardeakin merged commit 71f2f5a into cinder:master Oct 21, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@PetrosKataras PetrosKataras deleted the PetrosKataras:gstplayer-gl-fixes branch Oct 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment