From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Sakari Ailus <sakari.ailus@iki.fi>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>,
linux-media@vger.kernel.org, a.hajda@samsung.com
Subject: Re: [PATCH v2 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Date: Fri, 04 Oct 2013 00:13:54 +0200 [thread overview]
Message-ID: <524DEC22.5090107@gmail.com> (raw)
In-Reply-To: <1797993.sG828KdLkP@avalon>
Hi,
On 10/03/2013 11:40 AM, Laurent Pinchart wrote:
>>>>> 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.
>
> If I'm not mistaken, that's a valid configuration. The flag merely says that,
> if a pad has any link, then one of them must be active (Sakari, please correct
> me if I'm wrong).
It may be valid but it just sounds odd to me. Since the pad without a link
cannot be connected to anything, how could setting MUST_CONNECT flag on
it be
logical ? :) I think it's more about name of the flag, since its semantics
seems very well described.
>>> > > 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."
>> >
>> > Hmm. Good question. My assumption was that such cases might appear when an
>> > external entity could be connected to the pad, but receivers typically have
>> > just a single pad. So streaming would be out of question in such case
>> > anyway. But my thought was that we shouldn't burden drivers by forcing them
>> > to unset the flag if there happens to be nothing connected to an entity.
>> >
>> > How about just that I remove the last sentence, and s/ and the pad is linked
>> > to any other pad, then at least one of those links must be enabled/, the
>> > pad must be connected by an enabled link/?:-)
I guess removing the last sentence could be enough, IMHO it's pretty
clear also
without this sentence the MEDIA_PAD_FL_MUST_CONNECT flag is meaningless
on a pad
without links.
>> > The purpose is two-fold: to allow automated pipeline validation and also
>> > hint the user what are the conditions for that configuration.
Regards,
Sylwester
next prev parent reply other threads:[~2013-10-03 15:14 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-02 23:17 [PATCH v2 0/4] Add MEDIA_PAD_FL_MUST_CONNECT pad flag, check it Sakari Ailus
2013-10-02 23:17 ` [PATCH v2 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT Sakari Ailus
2013-10-02 23:29 ` Laurent Pinchart
2013-10-03 2:16 ` Sylwester Nawrocki
2013-10-03 8:43 ` Sakari Ailus
2013-10-03 9:40 ` Laurent Pinchart
2013-10-03 22:13 ` Sylwester Nawrocki [this message]
2013-10-13 10:58 ` [PATCH v2.1 " Sakari Ailus
2013-10-13 11:03 ` Sakari Ailus
2013-10-15 15:22 ` Laurent Pinchart
2013-10-15 21:41 ` Sakari Ailus
2013-10-15 21:46 ` [PATCH v2.2 " Sakari Ailus
2013-10-31 14:45 ` Laurent Pinchart
2013-10-13 11:00 ` [PATCH v2.1 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag Sakari Ailus
2013-10-02 23:17 ` [PATCH v2 " Sakari Ailus
2013-10-15 21:48 ` [PATCH v2.2 " Sakari Ailus
2013-10-31 14:46 ` Laurent Pinchart
2013-10-02 23:17 ` [PATCH v2 3/4] omap3isp: Mark which pads must connect Sakari Ailus
2013-10-02 23:17 ` [PATCH v2 4/4] omap3isp: Add resizer data rate configuration to resizer_link_validate Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=524DEC22.5090107@gmail.com \
--to=sylvester.nawrocki@gmail.com \
--cc=a.hajda@samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=s.nawrocki@samsung.com \
--cc=sakari.ailus@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).