On 01/06/2013 01:53 PM, Hans Verkuil wrote:
Tomasz Stanislawski (1):
s5p-tv: mixer: fix handling of VIDIOC_S_FMT
I'll drop this one for now. Devin raised a point: such changes would break
existing applications.
So, we'll need to revisit this topic before changing the drivers.
Btw, I failed to find the corresponding patch at patchwork:
http://patchwork.linuxtv.org/project/linux-media/list/?state=*&q=VIDIOC_S_FMT
So, its status update may be wrong after flushing your pwclient commands.
Hmm, I got this patch from Tomasz by e-mail and added it to the pull
request.
I think it wasn't sent to the mailing list, but I noticed it only after
sending you the pull requests, when was preparing the pwclient commands.
I've just posted it now, sorry. The link is here:
http://patchwork.linuxtv.org/patch/16143
Tomasz created this patch specifically for the purpose of format negotiation
in video pipeline in the application we used to test various scenarios with
DMABUF. I agree this patch has a potential of breaking buggy user space
applications. I can't see other solution for it right now, there seems even
to be no possibility to return some flag in VIDIOC_S_FMT indicating that
format has been modified and is valid, when -EINVAL was returned. This
sounds
ugly anyway, but could ensure backward compatibility for applications that
exppect EINVAL when format has been changed. BTW, I wonder if it is only
fourcc,
or other format parameters as well - like width, height, some applications
expect to get EINVAL when those have changed.
The patch makes the driver compliant to v4l-compilance, as its behavior asks
for such change, after some discussions we had this year in San Diego. At that
time, we all believed that such change were safe.
However, we can't do it like proposed there (and on other patches from Hans).
The fact is that tvtime and mythtv applications (maybe more) will fail
if the returned format is different than the requested ones, as they
don't check for the returned value.
As no regressions on userspace are allowed, we need to re-discuss this issue.
While this doesn't happen, I'll postpone such patches.
This is a video output device. So this patch will never affect tvtime/mythtv/etc.
I have no problem with this change being merged.
TBH, I very much doubt anyone would complain in case of this driver. I'm not
certain if there is complete support for even one board in the mainline
kernel,
likely only Origen A. AFAIK most applications use either Exynos DRM driver,
that has support for all features available in s5p-tv driver, or
framebuffer
emulation on top of v4l2 output interface (there were in the past RFC
patches
posted for vb2 adding FB emulation) is used. Although I agree with Mauro in
principle, I think chances of above patch causing any trouble to anyone are
close to zero.
--
Regards,
Sylwester
--
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
