search

gstreamer-java / gst1-java-core

Java bindings for GStreamer 1.x
GNU Lesser General Public License v3.0
194 stars 72 forks source link

Timecode meta #205

Closed ghost closed 4 years ago

ghost commented 4 years ago

I implemented the possibility to get additional metadata from each buffer. Currently, there is only metadata for time codes. In our company, we use these values for navigation in the video (for obtaining exactly the position of showed frame). But there really depends on how the video was created, container type, and codec type. Not all buffers/frames come with attached this kind of metadata. This is the first time when I wrote some binding over C library, so I hope that I understood correctly how to do it and this extension will be useful also for someone else.

neilcsmith-net commented 4 years ago

Thanks for this, and for adding tests!

I'll try and find some free time to review soon. At a glance seems to be in right direction, but can you add links to upstream docs (or code in the case of lowlevel) to the Javadocs as other classes have. This keeps our requirement to document a little lower, and makes for easier review.

Travis tests seem to be failing. Bear in mind that all tests should pass with GStreamer 1.8, currently our baseline. Or are we missing availability of GStreamer elements? Features from higher versions need to be guarded and tests not run. See eg. https://github.com/gstreamer-java/gst1-java-core/blob/master/src/org/freedesktop/gstreamer/Buffer.java#L249 and https://github.com/gstreamer-java/gst1-java-core/blob/master/test/org/freedesktop/gstreamer/PropertyTypeTest.java#L188

ghost commented 4 years ago

I hope that I fixed all your comments

neilcsmith-net commented 4 years ago

Thanks. I'll take a look as soon as I can - probably won't be able to for at least a week.

neilcsmith-net commented 4 years ago

Sorry this is taking so long - few other projects going through releases and hitting some teething problems. Have scheduled some time for next week to fully review. Thanks for your patience!

neilcsmith-net commented 4 years ago

Thanks for looking at this. I still have some issues with it as is. I'm still not sure about the package structure, a few of the method names, the license headers are in the wrong place and mixed up with what should be Javadocs, and I'd still like to get tests working without plugins bad if at all possible.

I'm going to change the base branch for this and merge into a specific branch here to work on together before it hits master.

neilcsmith-net commented 4 years ago

@aveco-devel I've started reviewing through, and working out a task list for merging this code. Unfortunately, I hadn't realised quite how much this was exposing lowlevel package types (eg. GType) and JNA types (eg. Pointer). Neither are allowed in our public API as a rule. Feel free to join in the discussion on the linked thread if you wish, and thanks for your work to date.