Re: Fwd: [PATCH 3/6] [media] s5p-mfc: add support for VIDIOC_{G,S}_CROP to encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 17, 2013 at 3:54 PM, Sylwester Nawrocki
<sylvester.nawrocki@xxxxxxxxx> wrote:
> On 10/18/2013 12:25 AM, John Sheu wrote:
>> On Thu, Oct 17, 2013 at 2:46 PM, John Sheu<sheu@xxxxxxxxxx>  wrote:
>>> >  Sweet.  Thanks for spelling things out explicitly like this.  The fact
>>> >  that the CAPTURE and OUTPUT queues "invert" their sense of "crop-ness"
>>> >  when used in a m2m device is definitely all sorts of confusing.
>>
>> Just to double-check: this means that we have another bug.
>>
>> In drivers/media/v4l2-core/v4l2-ioctl.c, in v4l_s_crop and v4l_g_crop,
>> we "simulate" a G_CROP or S_CROP, if the entry point is not defined
>> for that device, by doing the appropriate S_SELECTION or G_SELECTION.
>> Unfortunately then, for M2M this is incorrect then.
>>
>> Am I reading this right?
>
> You are right, John. Firstly a clear specification needs to be written,
> something along the lines of Tomasz's explanation in this thread, once
> all agree to that the ioctl code should be corrected if needed.
>
> It seems this [1] RFC is an answer exactly to your question.
>
> Exact meaning of the selection ioctl is only part of the problem, also
> interaction with VIDIOC_S_FMT is not currently defined in the V4L2 spec.
>
> [1] http://www.spinics.net/lists/linux-media/msg56078.html

I think the "inversion" behavior is confusing and we should remove it
if at all possible.

I took a look through all the drivers in linux-media which implement
S_CROP.  Most of them are either OUTPUT or CAPTURE/OVERLAY-only.  Of
those that aren't:

* drivers/media/pci/zoran/zoran_driver.c : this driver explicitly accepts both
  OUTPUT and CAPTURE queues in S_CROP, but they both configure the same state.
  No functional difference.
* drivers/media/platform/davinci/vpfe_capture.c : this driver doesn't specify
  the queue, but is a CAPTURE-only device.  Probably an (unrelated) bug.
* drivers/media/platform/exynos4-is/fimc-m2m.c : this driver is a m2m driver
  with both OUTPUT and CAPTURE queues.  It has uninverted behavior:
  S_CROP(CAPTURE) -> source
  S_CROP(OUTPUT) -> destination
* drivers/media/platform/s5p-g2d/g2d.c : this driver is a m2m driver with both
  OUTPUT and CAPTURE queues.  It has inverted behavior:
  S_CROP(CAPTURE) -> destination
  S_CROP(OUTPUT) -> source

The last two points above are the most relevant.  So we already have
at least one broken driver, regardless of whether we allow inversion
or not; I'd think this grants us a certain freedom to redefine the
specification to be more logical.  Can we do this please?

-John Sheu
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux