Skip to content

libvisual-plugins: Upgrade to GStreamer 1.x (fixes #95) #133

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.

Sign up for GitHub

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

Jump to bottom
Merged
hartwork merged 6 commits into from
Jan 20, 2023
Merged

libvisual-plugins: Upgrade to GStreamer 1.x (fixes #95) #133

hartwork merged 6 commits into from
Jan 20, 2023

Conversation

@hartwork
Copy link
Member

@hartwork hartwork commented Nov 30, 2022 •
edited
Loading

Fixes #95

@hartwork hartwork added this to the 0.5.0 milestone Nov 30, 2022
@hartwork hartwork changed the title Plugins (gstreamer): Upgrade to GStreamer 1.x libvisual-plugins: Upgrade to GStreamer 1.x Nov 30, 2022
@hartwork hartwork changed the title libvisual-plugins: Upgrade to GStreamer 1.x libvisual-plugins: Upgrade to GStreamer 1.x (fixes #95) Nov 30, 2022
@hartwork hartwork force-pushed the gstreamer-plugin-upgrade-gstreamer-1-x branch 3 times, most recently from 1c6287e to 103b3d2 Compare December 2, 2022 01:30
@kaixiong
Copy link
Member

kaixiong commented Jan 12, 2023

@hartwork, what remains to be done for the version upgrade?

@hartwork
Copy link
Member Author

hartwork commented Jan 12, 2023

@hartwork, what remains to be done for the version upgrade?

@kaixiong I'm not sure. I don't have a way of testing the gstreamer input plugin yet and also I haven't looked into gstreamer much yet, neither from a user nor from an API point of view. It's not trivial, so finishing this pull request is not within reach of a few days for me. Happy to discuss options, maybe off GitHub.

@hartwork hartwork force-pushed the gstreamer-plugin-upgrade-gstreamer-1-x branch 4 times, most recently from 8cdfa8a to f186701 Compare January 18, 2023 18:44
@hartwork hartwork marked this pull request as ready for review January 19, 2023 14:21
@hartwork hartwork requested a review from kaixiong January 19, 2023 14:21
@hartwork
Plugins (gstreamer): Upgrade to GStreamer 1.x
60209a8 
Joined forces with Chong Kai Xiong (@kaixiong) here, thank you!
@hartwork
Plugins (gstreamer): Split up method handle_bus_message
29992f3 
.. and limit logging to types end-of-stream and errors.

As suggested by Kai Xiong.
@hartwork hartwork force-pushed the gstreamer-plugin-upgrade-gstreamer-1-x branch from f186701 to 183a3b5 Compare January 20, 2023 19:25
@hartwork
Copy link
Member Author

hartwork commented Jan 20, 2023 •
edited
Loading

@kaixiong ready for review now. What I changed in the meantime, as discussed:

  • Fix check for GStreamer buffer size
  • Add comment on auto-calculated pitch
  • Rename method handle_sink_data to handle_sink_handoff
  • Enrich message "Failed to ready pipeline"
  • Document pipeline launch string in detail

kaixiong
kaixiong requested changes Jan 20, 2023
View reviewed changes
Copy link
Member

@kaixiong kaixiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are nearly there!

libvisual-plugins/plugins/actor/gstreamer/actor_gstreamer.c Outdated
// - element "filesrc" reads a video file from disk
// - element "decodebin" detects the video file's encoding
// and serves raw pixel data
// - element "videoconvert" does colorspace conversion from given to wanted
Copy link
Member

@kaixiong kaixiong Jan 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be more accurate to say videoconvert performs video format conversion. After all, it could convert RGB to BGR - the colour channel order changes but the colourspace remains RGB.

Copy link
Member Author

@hartwork hartwork Jan 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was inspired by gst-inspect-1.0 output where it says "Converts video from one colorspace to another". You have a point though, pushed.

Copy link
Member

@kaixiong kaixiong Jan 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hartwork, that's odd. I got the language from videoconvert's online documentation here.

Copy link
Member Author

@hartwork hartwork Jan 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaixiong the object hierachy slighty differs over here and also the upper limit to width and height:

# gst-inspect-1.0 videoconvert
Factory Details:
  Rank                     none (0)
  Long-name                Colorspace converter
  Klass                    Filter/Converter/Video
  Description              Converts video from one colorspace to another
  Author                   GStreamer maintainers <[email protected]>

Plugin Details:
  Name                     videoconvert
  Description              Colorspace conversion
  Filename                 /usr/lib64/gstreamer-1.0/libgstvideoconvert.so
  Version                  1.20.5
  License                  LGPL
  Source module            gst-plugins-base
  Source release date      2022-12-19
  Binary package           Gentoo GStreamer ebuild
  Origin URL               https://www.gentoo.org

GObject
 +----GInitiallyUnowned
       +----GstObject
             +----GstElement
                   +----GstBaseTransform
                         +----GstVideoFilter
                               +----GstVideoConvert

[..]

@hartwork hartwork force-pushed the gstreamer-plugin-upgrade-gstreamer-1-x branch from 183a3b5 to b253f44 Compare January 20, 2023 20:40
@hartwork hartwork requested a review from kaixiong January 20, 2023 20:40
kaixiong
kaixiong approved these changes Jan 20, 2023
View reviewed changes
Copy link
Member

@kaixiong kaixiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay this is it :)

@hartwork
Copy link
Member Author

hartwork commented Jan 20, 2023

@kaixiong thanks for the review! 👍

@hartwork hartwork merged commit d71a9e8 into master Jan 20, 2023
@hartwork hartwork deleted the gstreamer-plugin-upgrade-gstreamer-1-x branch January 20, 2023 21:22
@hartwork hartwork mentioned this pull request Jan 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaixiong kaixiong kaixiong approved these changes

Assignees

No one assigned

Labels

enhancement porting: done

Projects

None yet

Milestone

0.5.0_alpha1

Development

Successfully merging this pull request may close these issues.

gstreamer: Needs upgrade to gstreamer 1.x

3 participants

@hartwork @kaixiong