lkml.org 
[lkml]   [2016]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH] media: s5p-mfc - remove vidioc_g_crop
    From
    Date
    On 07/03/2016 07:30 PM, Nicolas Dufresne wrote:
    > Le dimanche 03 juillet 2016 à 11:43 +0200, Hans Verkuil a écrit :
    >> Hi Nicolas,
    >>
    >> On 07/02/2016 10:29 PM, Nicolas Dufresne wrote:
    >>>
    >>> Le 30 juin 2016 5:35 PM, "Shuah Khan" <shuahkh@osg.samsung.com
    >>> <mailto:shuahkh@osg.samsung.com>> a écrit :
    >>>>
    >>>> Remove vidioc_g_crop() from s5p-mfc decoder. Without its s_crop
    >>>> counterpart
    >>>> g_crop is not useful. Delete it.
    >>>
    >>> G_CROP tell the userspace which portion of the output is to be
    >>> displayed. Example, 1920x1080 inside a buffer of 1920x1088. It can
    >>> be
    >>> implemented using G_SELECTION too, which emulate G_CROP. removing
    >>> this without implementing G_SEKECTION will break certain software
    >>> like
    >>> GStreamer v4l2 based decoder.
    >>
    >> Sorry, but this is not correct.
    >>
    >> G_CROP for VIDEO_OUTPUT returns the output *compose* rectangle, not
    >> the output
    >> crop rectangle.
    >>
    >> Don't blame me, this is how it was defined in V4L2. The problem is
    >> that for video
    >> output (esp. m2m devices) you usually want to set the crop rectangle,
    >> and that's
    >> why the selection API was added so you can unambiguously set the crop
    >> and compose
    >> rectangles for both capture and output.
    >>
    >> Unfortunately, the exynos drivers were written before the
    >> G/S_SELECTION API was
    >> created, and the crop ioctls in the video output drivers actually set
    >> the output
    >> crop rectangle instead of the compose rectangle :-(
    >>
    >> This is a known inconsistency.
    >>
    >> You are right though that we can't remove g_crop here, I had
    >> forgotten about the
    >> buffer padding.
    >>
    >> What should happen here is that g_selection support is added to s5p-
    >> mfc, and
    >> have that return the proper rectangles. The g_crop can be kept, and a
    >> comment
    >> should be added that it returns the wrong thing, but that that is
    >> needed for
    >> backwards compat.

    Thank you both for the review and comments. I wasn't entirely sure
    about removing g-crop, hence this RFC patch. I will add g_selection
    and the comment to g_crop about returning incorrect info.

    thanks,
    -- Shuah

    >>
    >> The gstreamer code should use g/s_selection if available. It should
    >> check how it
    >> is using g/s_crop for video output devices today and remember that
    >> for output
    >> devices g/s_crop is really g/s_compose, except for the exynos
    >> drivers.
    >
    > This is already the case. There is other non-mainline driver that do
    > like exynos (I have been told).
    >
    > https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/sys?id=7
    > 4f020fd2f1dc645efe35a7ba1f951f9c5ee7c4c
    >
    >>
    >> It's why I recommend the selection API since it doesn't have these
    >> problems.
    >>
    >> I think I should do another push towards implementing the selection
    >> API in all
    >> drivers. There aren't many left.
    >>
    >> Regards,
    >>
    >> Hans

    \
     
     \ /
      Last update: 2016-07-05 22:21    [W:4.101 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site