linux-kernel - Re: [PATCH v4 7/8] v4l: Add signal lock status to source change events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b3ef22f9-c048-3c27-1e81-84533728dfb5@gmail.com>
Date:   Mon, 14 Nov 2016 11:32:19 -0800
From:   Steve Longerbeam <slongerbeam@...il.com>
To:     Steve Longerbeam <steve_longerbeam@...tor.com>,
        Hans Verkuil <hverkuil@...all.nl>, lars@...afoo.de
Cc:     mchehab@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@....samsung.com>
Subject: Re: [PATCH v4 7/8] v4l: Add signal lock status to source change
 events



On 11/14/2016 11:19 AM, Steve Longerbeam wrote:
>
>
> On 11/14/2016 03:36 AM, Hans Verkuil wrote:
>> On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
>>> Add a signal lock status change to the source changes bitmask.
>>> This indicates there was a signal lock or unlock event detected
>>> at the input of a video decoder.
>>>
>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@...tor.com>
>>> Cc: Mauro Carvalho Chehab <mchehab@....samsung.com>
>>>
>>> ---
>>>
>>> v4:
>>> - converted to rst from DocBook
>>>
>>> v3: no changes
>>> v2: no changes
>>> ---
>>>   Documentation/media/uapi/v4l/vidioc-dqevent.rst | 9 +++++++++
>>>   Documentation/media/videodev2.h.rst.exceptions  | 1 +
>>>   include/uapi/linux/videodev2.h                  | 1 +
>>>   3 files changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
>>> b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> index 73c0d5b..7d8a053 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> @@ -564,6 +564,15 @@ call.
>>>         an input. This can come from an input connector or from a video
>>>         decoder.
>>>   +    -  .. row 2
>>> +
>>> +       -  ``V4L2_EVENT_SRC_CH_LOCK_STATUS``
>>> +
>>> +       -  0x0002
>>> +
>>> +       -  This event gets triggered when there is a signal lock or
>>> +      unlock detected at the input of a video decoder.
>>> +
>>>     Return Value
>>>   ============
>>> diff --git a/Documentation/media/videodev2.h.rst.exceptions 
>>> b/Documentation/media/videodev2.h.rst.exceptions
>>> index 9bb9a6c..f412cc8 100644
>>> --- a/Documentation/media/videodev2.h.rst.exceptions
>>> +++ b/Documentation/media/videodev2.h.rst.exceptions
>>> @@ -453,6 +453,7 @@ replace define V4L2_EVENT_CTRL_CH_FLAGS 
>>> ctrl-changes-flags
>>>   replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>     replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>> +replace define V4L2_EVENT_SRC_CH_LOCK_STATUS src-changes-flags
>>>     replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ 
>>> v4l2-event-motion-det
>>>   diff --git a/include/uapi/linux/videodev2.h 
>>> b/include/uapi/linux/videodev2.h
>>> index 724f43e..08a153f 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -2078,6 +2078,7 @@ struct v4l2_event_frame_sync {
>>>   };
>>>     #define V4L2_EVENT_SRC_CH_RESOLUTION        (1 << 0)
>>> +#define V4L2_EVENT_SRC_CH_LOCK_STATUS        (1 << 1)
>>>     struct v4l2_event_src_change {
>>>       __u32 changes;
>>>
>> Quoting from an old (July) conversation about this:
>>
>>>>> I'm not entirely sure I like this. Typically losing lock means 
>>>>> that this event
>>>>> is triggered with the V4L2_EVENT_SRC_CH_RESOLUTION flag set, and 
>>>>> userspace has
>>>>> to check the new timings etc., which will fail if there is no lock 
>>>>> anymore.
>>>>>
>>>>> This information is also available through ENUMINPUT.
>>>>>
>>>>> I would need to know more about why you think this is needed, 
>>>>> because I don't
>>>>> see what this adds.
>>> Hi Hans,
>>>
>>> At least on the ADV718x, a source resolution change (from an
>>> autodetected video
>>> standard change) and a signal lock status change are distinct events.
>>> For example
>>> there can be a temporary loss of input signal lock without a change in
>>> detected
>>> input video standard/resolution.
>> OK, but what can the application do with that event? If the glitch 
>> didn't
>> affect the video, then it is pointless.
>
> Hi Hans, that's just it, for i.mx6 it does affect video. On i.mx6 a 
> temporary
> loss of signal from the adv7180 often results in a "split image", or 
> rolling
> image from captured frame to the next, and the only way to recover
> from this failure is to restart the pipeline (stream off -- stream 
> on). So
> the application needs to be informed of this temporary loss of signal
> event in order to restart streaming.
>
>
>>
>> If the lock is lost, then normally you loose video as well. If not, then
>> applications are not interested in the event.
>
> Yes, the lost lock causes a temporary or permanent loss of video,
> but with no other indications from the adv7180 (such as a detected
> video standard change). And on i.mx6, the lost lock often (actually
> usually) causes a split or rolling image.
>

Also it seems the sequence of actions from the application due to a
SRC_CH_LOCK_STATUS event would be in line with the action due to
SRC_CH_RESOLUTION. In other words for SRC_CH_RESOLUTION,
the app requests an update on the detected video standard via
QUERYSTD, and for SRC_CH_LOCK_STATUS, it requests an update
on input status (ENUMINPUTS).

Steve