| Message ID | 20211118180707.348256-1-umang.jain@ideasonboard.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote: > Fix a couple of sentence formations to clarify the intent. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/media_device.cpp | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index aa93da75..dd5def20 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice) > * instance. > * > * The instance is created with an empty media graph. Before performing any > - * other operation, it must be populate by calling populate(). Instances of > + * other operation, it must be populated by calling populate(). Instances of Oops. Good catch. > * MediaEntity, MediaPad and MediaLink are created to model the media graph, > * and stored in a map indexed by object id. > * > - * The graph is valid once successfully populated, as reported by the isValid() > - * function. It can be queried to list all entities(), or entities can be > - * looked up by name with getEntityByName(). The graph can be traversed from > - * entity to entity through pads and links as exposed by the corresponding > - * classes. > + * The graph is valid once it has been successfully populated, I think the sentence is grammatically correct already. Is it confusing ? > and is reported > + * as valid by the isValid() function. It can be queried to list all entities That doesn't mean the same. The current text meant to say that the successful population of the graph is reported by isValid(). > + * with entities(), or entities can be looked up by name with getEntityByName() > + * function. The graph can be traversed from entity to entity through pads and > + * links as exposed by the corresponding classes. > * > * Media devices can be claimed for exclusive use with acquire(), released with > * release() and tested with busy(). This mechanism is aimed at pipeline
Hi Laurent, On 11/19/21 1:20 AM, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > On Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote: >> Fix a couple of sentence formations to clarify the intent. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> src/libcamera/media_device.cpp | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp >> index aa93da75..dd5def20 100644 >> --- a/src/libcamera/media_device.cpp >> +++ b/src/libcamera/media_device.cpp >> @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice) >> * instance. >> * >> * The instance is created with an empty media graph. Before performing any >> - * other operation, it must be populate by calling populate(). Instances of >> + * other operation, it must be populated by calling populate(). Instances of > Oops. Good catch. > >> * MediaEntity, MediaPad and MediaLink are created to model the media graph, >> * and stored in a map indexed by object id. >> * >> - * The graph is valid once successfully populated, as reported by the isValid() >> - * function. It can be queried to list all entities(), or entities can be >> - * looked up by name with getEntityByName(). The graph can be traversed from >> - * entity to entity through pads and links as exposed by the corresponding >> - * classes. >> + * The graph is valid once it has been successfully populated, > I think the sentence is grammatically correct already. Is it confusing ? It seems confusing to me: Other options were: * The graph is valid once populated successfully, as reported by the isValid() ... But I don't think it goes with the explanation you gave below about isValid() > >> and is reported >> + * as valid by the isValid() function. It can be queried to list all entities > That doesn't mean the same. The current text meant to say that the > successful population of the graph is reported by isValid(). Oh. So I re-read the isValid() to clarify this - /** * \fn MediaDevice::isValid() * \brief Query whether the media graph has been populated and is valid * \return true if the media graph is valid, false otherwise */ It makes sense to me now, but the line above in the para, still seems a bit implicit to bring this meaning there. > >> + * with entities(), or entities can be looked up by name with getEntityByName() >> + * function. The graph can be traversed from entity to entity through pads and >> + * links as exposed by the corresponding classes. >> * >> * Media devices can be claimed for exclusive use with acquire(), released with >> * release() and tested with busy(). This mechanism is aimed at pipeline
Quoting Umang Jain (2021-11-19 05:21:13) > Hi Laurent, > > On 11/19/21 1:20 AM, Laurent Pinchart wrote: > > Hi Umang, > > > > Thank you for the patch. > > > > On Thu, Nov 18, 2021 at 11:37:07PM +0530, Umang Jain wrote: > >> Fix a couple of sentence formations to clarify the intent. > >> > >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > >> --- > >> src/libcamera/media_device.cpp | 12 ++++++------ > >> 1 file changed, 6 insertions(+), 6 deletions(-) > >> > >> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > >> index aa93da75..dd5def20 100644 > >> --- a/src/libcamera/media_device.cpp > >> +++ b/src/libcamera/media_device.cpp > >> @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice) > >> * instance. > >> * > >> * The instance is created with an empty media graph. Before performing any > >> - * other operation, it must be populate by calling populate(). Instances of > >> + * other operation, it must be populated by calling populate(). Instances of > > Oops. Good catch. > > > >> * MediaEntity, MediaPad and MediaLink are created to model the media graph, > >> * and stored in a map indexed by object id. > >> * > >> - * The graph is valid once successfully populated, as reported by the isValid() > >> - * function. It can be queried to list all entities(), or entities can be > >> - * looked up by name with getEntityByName(). The graph can be traversed from > >> - * entity to entity through pads and links as exposed by the corresponding > >> - * classes. > >> + * The graph is valid once it has been successfully populated, > > I think the sentence is grammatically correct already. Is it confusing ? > > Ayyee wrapped lines are making that really hard to interpret. As far as I can tell: Original: The graph is valid once successfully populated, as reported by the isValid() function. Suggested: The graph is valid once it has been successfully populated, and is reported as valid by the isValid() function. diff: A) s/once/once it has been/ B) s/as reported/and is reported/ Diff-A certainly works for me. Diff-B makes it sound like calling isValid() is a requirement to making it valid it, which it isn't. To me a clearer diff would be: B s/as reported/which is reported/ > It seems confusing to me: Other options were: > > * The graph is valid once populated successfully, as reported by > the isValid() ... > > But I don't think it goes with the explanation you gave below about > isValid() > > > >> and is reported > >> + * as valid by the isValid() function. It can be queried to list all entities > > That doesn't mean the same. The current text meant to say that the > > successful population of the graph is reported by isValid(). > > > Oh. So I re-read the isValid() to clarify this - > > /** > * \fn MediaDevice::isValid() > * \brief Query whether the media graph has been populated and > is valid > * \return true if the media graph is valid, false otherwise > */ > > It makes sense to me now, but the line above in the para, still seems a > bit implicit to bring this meaning there. > > > >> + * with entities(), or entities can be looked up by name with getEntityByName() > >> + * function. The graph can be traversed from entity to entity through pads and > >> + * links as exposed by the corresponding classes. > >> * > >> * Media devices can be claimed for exclusive use with acquire(), released with > >> * release() and tested with busy(). This mechanism is aimed at pipeline
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index aa93da75..dd5def20 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -40,15 +40,15 @@ LOG_DEFINE_CATEGORY(MediaDevice) * instance. * * The instance is created with an empty media graph. Before performing any - * other operation, it must be populate by calling populate(). Instances of + * other operation, it must be populated by calling populate(). Instances of * MediaEntity, MediaPad and MediaLink are created to model the media graph, * and stored in a map indexed by object id. * - * The graph is valid once successfully populated, as reported by the isValid() - * function. It can be queried to list all entities(), or entities can be - * looked up by name with getEntityByName(). The graph can be traversed from - * entity to entity through pads and links as exposed by the corresponding - * classes. + * The graph is valid once it has been successfully populated, and is reported + * as valid by the isValid() function. It can be queried to list all entities + * with entities(), or entities can be looked up by name with getEntityByName() + * function. The graph can be traversed from entity to entity through pads and + * links as exposed by the corresponding classes. * * Media devices can be claimed for exclusive use with acquire(), released with * release() and tested with busy(). This mechanism is aimed at pipeline
Fix a couple of sentence formations to clarify the intent. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/media_device.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)