GNOME Bugzilla – Bug 742917
v4l2: Allow scaling in the v4l2*convert element
Last modified: 2015-06-09 22:52:26 UTC
Created attachment 294525 [details] [review] scaling patch * Remove also 'width' and 'height' caps in gst_v4l2_transform_caps_remove_format_info() in order to allow scaling by the underlying v4l2 element. * Choose appropriate width and height values in caps fixation Copied caps fixation code from gstvideoconvert element Example usage: [..] ! video/x-raw,width=320 ! v4l2video1convert ! video/x-raw,width=1280 ! [..]
*** Bug 726104 has been marked as a duplicate of this bug. ***
Thanks Enrico for this patch, I'll have a look as soon as possible. Meanwhile, I have few general question: Is this negotiation code inspired from videoscale element ? Have you taken pixel aspect ratio into account ? Do you support device that cannot scale ? Finally, just mentioning, I had a discussion with Hans and could not conclude if my use of the CROP ioctl was right or not. Ideally we'd be using SELECTION but it was not implemented when I did that work. Have you made some tests with various cropping ? I also notice that you forgot to update the Klass field of this element. The software videoscaler sets: Klass Filter/Converter/Video/Scaler These will eventually be used for auto-plugging elements in the future.
(oh, and I'll do some tests with FIMC driver on Exynos 4)
Interesting, I think I can also do some tests with my HW. By the way, do we have a way to know that device scale other than probing method (like in https://bugzilla.gnome.org/show_bug.cgi?id=727483) ?
(In reply to Nicolas Dufresne (stormer) from comment #2) > > Is this negotiation code inspired from videoscale element ? > Have you taken pixel aspect ratio into account ? Answer is yes, twice. > Do you support device that cannot scale ? > Klass Filter/Converter/Video/Scaler Any update on these ?
I apply the patch and it seems to work well. (In reply to Nicolas Dufresne (stormer) from comment #5) > > Do you support device that cannot scale ? > > Klass Filter/Converter/Video/Scaler > > Any update on these ? I quckly looked at the code and I didn't find such check, so I think the answer is no. However the S_FMT should fail (hopefully).
The problem is that we need to only suggest things that are possible in our transform caps call. Failing trying to do something that is not supported is not acceptable. There could be an element downstream the do scale correctly. If V4L2 don't tell us that, we should at least create a white list.
(In reply to Nicolas Dufresne (stormer) from comment #7) > The problem is that we need to only suggest things that are possible in our > transform caps call. Failing trying to do something that is not supported is > not acceptable. There could be an element downstream the do scale correctly. I'm entirely agree with that. > If V4L2 don't tell us that, we should at least create a white list. There is currently a way to probe if the device can scale or not by checking the crop window against the set format. But I don't really know if it's reliable or not.
(In reply to Aurélien Zanelli from comment #8) > > If V4L2 don't tell us that, we should at least create a white list. > There is currently a way to probe if the device can scale or not by checking > the crop window against the set format. But I don't really know if it's > reliable or not. Oh, and I saw you patches btw, but could not completly make my mind. The crop API being what it is. (and v4l2 people telling me to use SELECTION which they didn't implement ;-P). I guess we need to do some more testing to find the right information.
(In reply to Nicolas Dufresne (stormer) from comment #7) > The problem is that we need to only suggest things that are possible in our > transform caps call. Failing trying to do something that is not supported is > not acceptable. There could be an element downstream the do scale correctly. > > If V4L2 don't tell us that, we should at least create a white list. Yes, afaik there is no straight way to test if a v4l2 m2m device supports scaling. But this is the same for color conversion capability. Thus if we had a device that could scale but not convert color space, the current approach might fail, too. In the current kernel there should be no driver that supports colorspace conversion only. So imho this patch should not make things worse. But yes, in general this is a problem to be solved. I've attached a modified version of the patch that adds the "Scalar" hint.
Created attachment 301509 [details] [review] scaling patch v2
Any comments for this?
I was on holiday. I accept the argument in comment 10. My only concern was around the granularity of the scaler and how do we handle it. Scaler often only support sizes with multiple of 2 or even 16. Also that concern that it may fail randomly since the kernel fixes to round S_FMT properly is still not merged yet. Would it be possible to document you approach to handle a scaler that can only scale with multiple of 2 or 4 ? Though, in general, as long as we have a plan on how we want to handle this, and that the code mostly reflect that, I'm fine.
Review of attachment 301509 [details] [review]: Looks good to me. I didn't tested myself though.
Answering myself: (In reply to Nicolas Dufresne (stormer) from comment #13) > I was on holiday. I accept the argument in comment 10. My only concern was > around the granularity of the scaler and how do we handle it. Scaler often > only support sizes with multiple of 2 or even 16. Also that concern that it > may fail randomly since the kernel fixes to round S_FMT properly is still > not merged yet. > > Would it be possible to document you approach to handle a scaler that can > only scale with multiple of 2 or 4 ? To work properly in generic environment, the drive should implement enumeration of formats. In that context, it will be able to use the step and range. The rest will all be handled correctly automatically.
commit 3067a60d8e76062f42f1b605393ab50be8c9d907