Hi,
On 11/03/2016 12:55 PM, Hans Verkuil wrote:
> On 03/11/16 11:45, Stanimir Varbanov wrote:
>> Hi Hans,
>>
>> On 09/19/2016 01:04 PM, Hans Verkuil wrote:
>>> On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
>>>> This consists of video decoder implementation plus decoder
>>>> controls.
>>>>
>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx>
>>>> ---
>>>> drivers/media/platform/qcom/vidc/vdec.c | 1091
>>>> +++++++++++++++++++++++++
>>>> drivers/media/platform/qcom/vidc/vdec.h | 29 +
>>>> drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +++++
>>>> drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 +
>>>> 4 files changed, 1341 insertions(+)
>>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
>>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
>>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
>>>> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
>>>>
>>
>> <cut>
>>
>>>> +
>>>> +static int
>>>> +vdec_g_selection(struct file *file, void *priv, struct
>>>> v4l2_selection *s)
>>>> +{
>>>> + struct vidc_inst *inst = to_inst(file);
>>>> +
>>>> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>>>> + return -EINVAL;
>>>> +
>>>> + switch (s->target) {
>>>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>>>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>>>> + case V4L2_SEL_TGT_CROP:
>>>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>>>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>>>> + case V4L2_SEL_TGT_COMPOSE:
>>>
>>> This is almost certainly wrong.
>>>
>>> For capture I would expect that you can do compose, but not crop.
>>>
>>> This would likely explain that v4l2-compliance thinks that the driver
>>> can
>>> scale:
>>>
>>> test Scaling: OK
>>>
>>
>> Maybe I need some help to implement correctly g_selection.
>>
>> Lets say that the resolution of the compressed stream is 1280x720, and
>> that resolution is set with s_fmt(OUTPUT queue), then I calculate the
>> output resolution which I will return by g_fmt(CAPTURE queue) and it
>> will be 1280x736 (hardware wants height to be multiple of 32 lines). So
>> the result will be 16 lines of vertical padding which should be exposed
>> to client (think of gstreamer v4l2videodec element) via g_crop
>> (g_selection) as 1280x720 because this is the actual image.
>>
>> So from what I understood while read Selection API, I need to support
>> only composing on CAPTURE queue, no scaling and no cropping.
>>
>> OUTPUT buffer type, data source
>> TGT_CROP_BOUNDS = TGT_CROP_DEFAULT = TGT_CROP = 1280x720
>> TGT_COMPOSE_BOUNDS = TGT_COMPOSE_DEFAULT = TGT_COMPOSE = 1280x720
>
> The output buffer type doesn't do composition, only crop. So you shouldn't
> support the compose targets.
OK I tried to return EINVAL for compose targets for output buffer type
and the result is :
test Cropping: OK
test Composing: OK
test Scaling: OK
Which is odd because now Scaling is supported.
>
>>
>> CAPTURE buffer type, data sink
>> TGT_CROP_BOUNDS = TGT_CROP_DEFAULT = TGT_CROP = EINVAL
>> TGT_COMPOSE_BOUNDS = 1280x736
>> TGT_COMPOSE_DEFAULT = TGT_COMPOSE = 1280x720
>
> This looks good.
>
>>
>> With this logic in g_selection the output of v4l2-compliance test
>> application is:
>> test Cropping: OK
>> test Composing: OK
>> test Scaling: OK (Not Supported)
>>
>> So why v4l2-compliance still thinks that the driver supports Cropping?
>
> Hmm, v4l2-compliance could be improved. The problem is that it doesn't
> show for m2m devices for which side (capture or output) cropping, composing
> or scaling is supported. It does test both sides, but you can't tell from
> the output.
>
> It's a bit confusing in this case.
OK I will incorporate the above g_selection behavior and send a new
version of the patches, I do not waste time on that. The drawback is
that this padding will be displayed as green bar on bottom of the
displayed image, although the gstreamer master branch now use
g_selection so probably it should be fine there.
>
> Regards,
>
> Hans
>
>>
>>>> + break;
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + s->r.top = 0;
>>>> + s->r.left = 0;
>>>> + s->r.width = inst->out_width;
>>>> + s->r.height = inst->out_height;
>>>> +
>>>> + return 0;
>>>> +}
>>
>> <cut>
>>
--
regards,
Stan
--
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
