Comment on attachment 248464 [details] [review]
allocators: dmabuf: allow testing allocator type

1) how is

     gst_is_dmabuf_allocator(allocator)

a huge improvement on

    GST_IS_DMABUF_ALLOCATOR (allocator)

?

2) please drop the header changes. They are unrelated, but also make things worse. Headers are not indented with gst-indent, only .c files. The headers are nicely aligned as they are now.

Created attachment 248601 [details] [review]
allocators: dmabuf: allow testing allocator type

if your platform doesn't support mmap GST_IS_DMABUF_ALLOCATOR isn't defined, so I wrote gst_is_dmabuf_allocator for the both cases.

The second patch fix indentations issues in .h file.

So, if that macro and the type and everything does not exist if support for it isn't there, you will have to conditionally compile the code anyway. And if it is available you can just use GST_IS_DMABUF_ALLOCATOR().

I don't see the point of this change :)

For me it is exactly like the function gst_is_dmabuf_memory above it return false if dmabuf isn't implemented.

I would like avoid propagate compilation everywhere: for example in v4l2src it should help us to know if we can use dmabuf ioctl or not.

Then make the macros be defined if dmabuf is not available too, and let them return sensible values for that. There's really not point in adding new API for something that already exists.

Created attachment 249198 [details] [review]
allocators: dmabuf: allow testing allocator type

This version of the patch export the allocator name and not create new functions to test it type.
The example in the commit header has been change to show how use allocator->mem_type field.

Make gst_is_dmabuf_memory use gst_memory_is_type instead of duplicated the code.

commit 84a09348832407a2f7e21dbae8cfa1e834116ccf
Author: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Date:   Mon Jul 15 15:23:17 2013 +0200

    allocators: dmabuf: allow testing allocator type
    
    In decide_allocation function some element may when to test the proposed allocator.
    For example like this:
    if (gst_query_get_n_allocation_params (query) > 0) {
        GstAllocator * allocator;
        GstAllocationParams params;
        gst_query_parse_nth_allocation_param (query, 0, &allocator, &params);
        if (g_strcmp0(allocator->mem_type, GST_ALLOCATOR_DMABUF) == 0)
                GST_DEBUG("got dmabuf allocator");
        else
                GST_DEBUG("got an other allocator");
    }
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703659