Added precompiler check for installed gstreamer version > 1.11.1 & Mpeg-streamer example

dturing / node-gstreamer-superficial

Superficial gstreamer binding

MIT License

127 stars 45 forks source link

Added precompiler check for installed gstreamer version > 1.11.1 & Mpeg-streamer example #7

Closed gregfutia closed 7 years ago

gregfutia commented 7 years ago

Used precompiler to conditionally add latency functionality. Changes check if gstreamer version 1.11.1 greater than 1.11.1 where gstreamer where gst_pipeline_get_latency and gst_pipeline_set_latency and functions are first added to the tree before compiling them in. Current Debian stable (Jessie) gstreamer version is 1.4.4 and doesn't have these functions defined in gstpipline.h .

Adding these flags got this to build on Debian Jessie 8.6 running on a Beaglebone Black. I checked for a backport but Debian sid & testing are at 1.10 right now with 1.11 in experimental. It might be a while before 1.11 gets to Debian.

dturing commented 7 years ago

Thanks @CTCHunter1 , but 1.11 is an unstable branch of gstreamer; i'd rather not have this until its stabilized as 1.12...

gregfutia commented 7 years ago

My patch is to keep node-gyp rebuild from failing on systems that dong have gstreamer 1.11.1 or newer which is my system. I didn't realize 1.11 is considered unstable. Otherwise the code those #if surround would need to removed.

gregfutia commented 7 years ago

I added a second example for streaming similar to mpeg-streamer.

dturing commented 7 years ago

Thank you for contributing! I do like your mjpeg example and will be happy to accept a PR for it, but i have a few issues:

please check your git username/email. you're committing as "root@cytobone.localdomain"
use feature branches for things that are really separate. the example has no relation to the pipeline functions you added (or the version check)
read up on your MPEG vs MJPEG. AFAICS, your example does mjpeg, which really isnt like mpeg at all.

I'm not opposed at all to adding the pipeline delay/latency functions; but they seem to be available since gstreamer 1.6, and that seems sufficiently old to just require gst>=1.6, instead of adding compile-time checks; so that should be a branch/PR separate from the mjpeg example.

to be perfectly pedantic, review your commit messages too. if you're adding functions to gst.Pipeline, that's the main thing to write about, not really the gstreamer version currently supported by debian-stable ;)

gregfutia commented 7 years ago

Just to be clear, I wasn't the person who added the additional pipeline functions. I just noticed them as new when the npm install worked but the build from gitthub failed (not the same packages). I only added the compiler flags to get it to build on my system.

When, I figured out those functions weren't in my installed version I went to the gstreamer repository to try to determine when they were added. That's where I came up with the 1.11.1. If it's really 1.6 I wouldn't pull those flags into the master. Like you say a branch maybe for older gstreamer support. I only added the compiler flags for reverse comparability not the functions themselves, hence the commit comment about what' its compatible with.

I'll take a look at those 3 issues you talk mentioned. I didn't realize the importance of branches until I made the commit of the example after my pull request. I will see if I can create branches for both those things and get them pulled.