[Bug 747216] applemedia: implement GstAppleCoreVideoMemory

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=747216

--- Comment #30 from Ilya Konstantinov <ilya.konstantinov at gmail.com> ---
(In reply to Nicolas Dufresne (stormer) from comment #29)
> +  gst_apple_core_video_memory_init ();

I'd have a look at how other allocators work. I copied it from GstGLMemory
which, I understand, is not the most shining example of how to do things.

> +  GST_APPLE_CORE_VIDEO_MEMORY_LOCKED_READONLY,
> +  GST_APPLE_CORE_VIDEO_MEMORY_LOCKED_READ_WRITE
> +} GstAppleCoreVideoLockState;
> 
> Isn't this duplicating GstLockFlags ?

As comment 12 explains, locking is per-whole-CVPixelBuffer while individual
Gst*Memory objects are per-plane.

e.g.
1. Plane 1 mmap -> lock the whole CVPixelBuffer
2. Plane 2 mmap -> CVPixelBuffer already locked
3. Plane 1 unmap -> shouldn't unlock CVPixelBuffer yet
4. Plane 2 unmap -> unlock CVPixelBuffer

Therefore, I must remember, per-whole-CVPixelBuffer, whether it's locked and
how.

> @@ +53,3 @@
> +   * or for reading and writing, as reflected in @lock_state. */
> +  guint lock_count;
> +} GstAppleCoreVideoPixelBuffer;
> 
> Why not implementing a GstMiniObject, which already handle the this locking ?

If GstMiniObject maps cleanly to what I'm doing here, then sure, why not.

> @@ +60,3 @@
> + * Indicates a non-planar pixel buffer.
> + */
> +#define GST_APPLE_CORE_VIDEO_NO_PLANE ((size_t)-1)
> 
> non-planar and NO_PLANE does not fully sound the same to me, can we use a
> better name ? (I could propose GST_APPLE_CORE_VIDEO_HAS_ONE_PLANE)

It is passed as an argument when a plane is expected, i.e.
gst_apple_core_video_memory_new_wrapped (gpixbuf,
GST_APPLE_CORE_VIDEO_NO_PLANE, size))

It's like:
Q - Excuse me, what plane are you passing me? 1st? 2nd?
A - I'm passing you a special plane -- a "no-plane".

Also, ..._HAS_ONE_PLANE != non-planar.

> @@ +75,3 @@
> +  GstAppleCoreVideoPixelBuffer *gpixbuf;
> +  size_t plane;
> +} GstAppleCoreVideoMemory;
> 
> In fact, I don't understand why this double wrapping.

Because Gst*Memory is per-plane, see comment 12.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.