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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.