After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759481 - Add in another camera support for dma_buf in gst-vaapi
Add in another camera support for dma_buf in gst-vaapi
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-15 10:22 UTC by Lim Siew Hoon
Modified: 2016-02-22 20:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add icamerasrc support in gstreamer-vaapi (1.48 KB, application/mbox)
2015-12-15 10:22 UTC, Lim Siew Hoon
  Details
Add the patches using for support icamerasrc for dmabuf (1.48 KB, patch)
2015-12-28 03:01 UTC, Lim Siew Hoon
none Details | Review
Add icamerasrc support in gstreamer-vaapi (1.54 KB, patch)
2015-12-28 05:56 UTC, Lim Siew Hoon
none Details | Review
Add icamerasrc support in gstreamer-vaapi (1.66 KB, patch)
2016-02-17 08:49 UTC, Lim Siew Hoon
none Details | Review

Description Lim Siew Hoon 2015-12-15 10:22:45 UTC
Created attachment 317407 [details]
Add icamerasrc support in gstreamer-vaapi

static gboolean has_dmabuf_capable_peer (GstVaapiPluginBase * plugin, GstPad * pad) {
     ... ...
     if (GST_IS_PUSH_SRC (element)) {
        element_name = gst_element_get_name (element);
        if (!element_name || sscanf (element_name, "v4l2src%d", &v) != 1)
           break;
    
        v = 0;
        g_object_get (element, "io-mode", &v, NULL);
         is_dmabuf_capable = v == 5;       /* "dmabuf-import" enum value
*/

Current above code implementation is hardcode support for dma-buff v4l2src.
Coming soon there will be another camera plugin need to support dma-buf and using gstreamer-vaapi. 


Attached the code patches.

At the same time, also waiting for verify result from another party.
Comment 1 Lim Siew Hoon 2015-12-28 03:01:10 UTC
Created attachment 317943 [details] [review]
Add the patches using for support icamerasrc for dmabuf

This is final patches, we tested is working. Please review it. 
CC-ing Sreerenj. Thanks.
Comment 2 Lim Siew Hoon 2015-12-28 05:56:29 UTC
Created attachment 317950 [details] [review]
Add icamerasrc support in gstreamer-vaapi

Added in tested-by in commit message.
Comment 3 Víctor Manuel Jáquez Leal 2016-01-14 17:22:12 UTC
I think that rather to commit this (which is based on the hackish function has_dmabuf_capable_peer()) we should rely in the memory:DMABuf capsfeature.

We should work rather on bug 755072
Comment 4 sreerenj 2016-01-18 11:40:09 UTC
(In reply to Lim Siew Hoon from comment #2)
> Created attachment 317950 [details] [review] [review]
> Add icamerasrc support in gstreamer-vaapi
> 
> Added in tested-by in commit message.

- Where can I get the Icamerasrc ? is it open sourced? I can't find it in 01org/github

- please give a description about "icamerasrc" in the commit message.
Comment 5 Lim Siew Hoon 2016-01-26 09:51:18 UTC
Sorry. On hold first. Still pending to the icamerasrc upload to Github. They still haven't upload the code yet. Once they uploaded, will update it.

https://github.com/01org/icamerasrc
Comment 6 sreerenj 2016-01-26 10:36:02 UTC
Review of attachment 317950 [details] [review]:

Few more:
-- indentation is incorrect
-- why unnecessary strlen()?, use hardcoded value in strncmp
Comment 7 Lim Siew Hoon 2016-02-17 08:49:55 UTC
Created attachment 321466 [details] [review]
Add icamerasrc support in gstreamer-vaapi

Attached new rework patch:
1. Added description about "icamerasrc" in commit message.
1. Fix the incorrect indentation 
2. Replace strlen by using hardcoded value in strncmp.

Current the icamerasrc source code already upload in https://github.com/01org/icamerasrc.
Comment 8 sreerenj 2016-02-17 13:54:27 UTC
Pushed, thanks for the patch.

commit ce1ab4673c9dedaa9e9d9ead7ff78e25087ea8d6
Author: Lim Siew Hoon <siew.hoon.lim@intel.com>
Date:   Wed Feb 17 15:40:54 2016 +0200

    Add icamerasrc as dmabuf capable peer element
    
    icamerasrc is another gstreamer plugin using to capture RAW
    frames from camera device. It is based on libcamhal library.
    There are some properties available to control icamera behavior.
    
    Signed-off-by: Lim Siew Hoon <siew.hoon.lim@intel.com>
    Tested & Reviewed: Zhu Haiyang <haiyang.zhu@intel.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759481
    
    Fixme: This is the similar workaround we done for v4l2src.
    The workaround will be removed once we fix #755072
Comment 9 Nicolas Dufresne (ndufresne) 2016-02-22 19:20:17 UTC
Sree, just a reminder that this is terrible hack you have in there. There is no logical reason for a property to be changed from downstream element like this, just like there is no reason for camera to always produce DMABuf when possible. And vaapi should adapt to the received type of buffer rather then enable dmabuf only for camera that is hardcoded in it.
Comment 10 sreerenj 2016-02-22 20:38:58 UTC
(In reply to Nicolas Dufresne (stormer) from comment #9)
> Sree, just a reminder that this is terrible hack you have in there. There is
> no logical reason for a property to be changed from downstream element like
> this, just like there is no reason for camera to always produce DMABuf when
> possible. And vaapi should adapt to the received type of buffer rather then
> enable dmabuf only for camera that is hardcoded in it.

Nicolas, I knew this is a hack.. We did this hack for v4l2src sometimes back to support a customer when gstreamer-vaapi was not the part of upstream :).. There was no proper dmabuf negotiation capabilities at that time too. 
As I mentioned in the commit message we will get-rid of this hack once we have a fix for https://bugzilla.gnome.org/show_bug.cgi?id=755072 .