GNOME Bugzilla – Bug 726104
Proposal for a v4l2scale element
Last modified: 2015-01-14 14:20:14 UTC
This patch is a proposal of a scaler for V4L2 based drivers. The v4l2scale element will drive a V4L2 M2M device capable of capturing images and output them rescaled. The new dimensions would be set via a caps filter (just as videoscale does). In case no caps filter is defined, the element is used in passthrough mode. Can someone please look at it and make some suggestions?
Created attachment 271520 [details] [review] V4l2 scaler
Interesting, I've been working on a color transform element in the last few days, with the goal to support multiple transforms like scaling. I'll have a look first and come back with how we should integrated the two effort.
Review of attachment 271520 [details] [review]: Ok, on my side, what I've been working on is a more generalized. This implementation only has support for 1 scaler. Also, many platform have multi-purpose transformation. I'll keep that here as a reference, but I'd prefer having a transform framework from which we would correctly detect devices based on a supported list, and flag the feature (like transform, scaling, colorbalance, etc) that are supported. ::: sys/v4l2/gstv4l2scale.c @@ +205,3 @@ + } + i++; + } while (v4l2object->videodev == NULL && i < MAX_VIDEO_DEVICES); MAX being 10, on my HW the scaler is found at 10, 12, 14 and 16. I want a generalized way to probe HW, and remove the overhead (something that works too even if dev are renamed, so udev should be the way), since atm each new class do a full probing again and gain. @@ +536,3 @@ + + LOG_CAPS (v4l2scale, incaps); + LOG_CAPS (v4l2scale, outcaps); Any reason to have customer logging ? @@ +539,3 @@ + + sinkObj = v4l2scale->v4l2sinkobject; + srcObj = v4l2scale->v4l2srcobject; Coding style is sink_obj, not camel. @@ +589,3 @@ + +static gboolean +gst_v4l2scale_decide_allocation (GstBaseTransform * trans, GstQuery * query) These need to be generalized, see propose_allocation.
Review of attachment 271520 [details] [review]: ::: sys/v4l2/gstv4l2scale.c @@ +536,3 @@ + + LOG_CAPS (v4l2scale, incaps); + LOG_CAPS (v4l2scale, outcaps); No, there is no particular reason for having these logs. I guess they where useful when debugging. @@ +539,3 @@ + + sinkObj = v4l2scale->v4l2sinkobject; + srcObj = v4l2scale->v4l2srcobject; Ok, I will update my code. @@ +589,3 @@ + +static gboolean +gst_v4l2scale_decide_allocation (GstBaseTransform * trans, GstQuery * query) What do you mean by "generalized"? And why should I see propose_allocation?
Review of attachment 271520 [details] [review]: Note that I might not merge this element, as I'm currently writing v42transform, with the goal to support color transform/scaling/colorbalance/cropping and be able to instantiate more then 1 element. Though your help in testing it will be appreciated as I don't own that many HW with such v4l2 drivers. ::: sys/v4l2/gstv4l2scale.c @@ +536,3 @@ + + LOG_CAPS (v4l2scale, incaps); + LOG_CAPS (v4l2scale, outcaps); I mean custom, in the sense not using the usual macro. Though I haven't check the definition of LOG_CAPS. @@ +589,3 @@ + +static gboolean +gst_v4l2scale_decide_allocation (GstBaseTransform * trans, GstQuery * query) Something that works for other platform then yours, and that scan the HW once, along with scanning the decoders.
I'd like to evolve gstv4l2transform into supporting more scenarios rather then having a new element. Since most HW converter doe multiple things. I'd propose adding flags in the class, and a white list that would set these flags. As I didn't find anytrick to figure-out what a M2M is actually use for (generically).
Comment on attachment 271520 [details] [review] V4l2 scaler Shouldn't be hard to update the existing gstv4l2transform element to support scaling / aspect ratio handling. We need to edit the probing a bit, so we can opt in at runtime the features we support.
Someone proposed a patch against v4l2tansform. Marking this one as duplicate to keep the other patch. *** This bug has been marked as a duplicate of bug 742917 ***