gstreamer-java / gst1-java-core

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

Add VideoMeta #228

Closed lafoletc closed 2 years ago

lafoletc commented 3 years ago

Add a new meta to get video informations for each planes of the buffer

neilcsmith-net commented 3 years ago

Thanks for looking at this - was on my list. Removing need for GType in AllocationQuery is the main problem here, and not sure how best to achieve that - should be Meta.API but need to look at how we map across without package access.

Re-running tests - not sure if failure was a problem that needs looking at.

Once done, ideally this would be squashed and force pushed into the PR branch.

neilcsmith-net commented 3 years ago

OK, test failure on 1.16 seems a glitch - rerun twice.

lafoletc commented 3 years ago

When I run PadTest in master branch, I have the following traces :

(test:5311): GStreamer-CRITICAL : 21:41:43.249: gst_mini_object_unref: assertion 'GST_MINI_OBJECT_REFCOUNT_VALUE (mini_object) > 0' failed**

lafoletc commented 3 years ago

For the field GstVideoMetaStruct.offset, is "long" type compatible for 32 and 64 bits ?

neilcsmith-net commented 3 years ago

For the field GstVideoMetaStruct.offset, is "long" type compatible for 32 and 64 bits ?

No, definitely not. Size is important as it's used for structure size calculation.

Firstly, when adding structs, it's useful to have a link to the source in a doc section above the code - eg. to https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/blob/master/gst-libs/gst/video/gstvideometa.h#L78

Mapping gsize is an awkward one - we used to use NativeLong in a few places - that's also wrong - see discussion at https://groups.google.com/g/jna-users/c/RJjcN5sw_po/m/t282zYqiBgAJ

So far, the only accurate mapping is probably GType, which we shouldn't use! Maybe we need to have a GSize as well - IntegerType subclass similarly sized to Native.SIZE_T_SIZE?

Thanks for other changes - will take a look ASAP.

neilcsmith-net commented 3 years ago

@lafoletc any news on remaining changes for this? I'll be releasing 1.4 soon - would be great to merge this if ready. Thanks!

neilcsmith-net commented 3 years ago

@lafoletc OK, moving this to milestone 1.5.