Hi Sakari,

On 10/03/2013 08:29 AM, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thursday 03 October 2013 02:17:50 Sakari Ailus wrote:
>> Pads that set this flag must be connected by an active link for the entity
>> to stream.
>>
>> Signed-off-by: Sakari Ailus <sakari.ai...@iki.fi>
>> Acked-by: Sylwester Nawrocki <sylvester.nawro...@gmail.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Looks good, I would just like to ask for changing my e-mail address on those
patches to s.nawro...@samsung.com, sorry for not mentioning it earlier. 
Just one doubt below regarding name of the flag.

>> ---
>>  Documentation/DocBook/media/v4l/media-ioc-enum-links.xml |   10 ++++++++++
>>  include/uapi/linux/media.h                               |    1 +
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml index
>> 355df43..e357dc9 100644
>> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
>> @@ -134,6 +134,16 @@
>>          <entry>Output pad, relative to the entity. Output pads source data
>>          and are origins of links.</entry>
>>        </row>
>> +      <row>
>> +        <entry><constant>MEDIA_PAD_FL_MUST_CONNECT</constant></entry>
>> +        <entry>If this flag is set and the pad is linked to any other
>> +        pad, then at least one of those links must be enabled for the
>> +        entity to be able to stream. There could be temporary reasons
>> +        (e.g. device configuration dependent) for the pad to need
>> +        enabled links even when this flag isn't set; the absence of the
>> +        flag doesn't imply there is none. The flag has no effect on pads
>> +        without connected links.</entry>

Probably MEDIA_PAD_FL_MUST_CONNECT name is fine, but isn't it more something
like MEDIA_PAD_FL_NEED_ACTIVE_LINK ? Or presumably MEDIA_PAD_FL_MUST_CONNECT
just doesn't make sense on pads without connected links and should never be
set on such pads ? From the last sentence it feels the situation where a pad
without a connected link has this flags set is allowed and a valid 
configuration.

Perhaps the last sentence should be something like:

"The flag should not be used on pads without connected links and has no effect
on such pads." 

Regards,
Sylwester

>> +      </row>
>>      </tbody>
>>        </tgroup>
>>      </table>
>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>> index ed49574..d847c76 100644
>> --- a/include/uapi/linux/media.h
>> +++ b/include/uapi/linux/media.h
>> @@ -98,6 +98,7 @@ struct media_entity_desc {
>>
>>  #define MEDIA_PAD_FL_SINK           (1 << 0)
>>  #define MEDIA_PAD_FL_SOURCE         (1 << 1)
>> +#define MEDIA_PAD_FL_MUST_CONNECT   (1 << 2)
>>
>>  struct media_pad_desc {
>>      __u32 entity;           /* entity ID */


-- 
Sylwester Nawrocki
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to