[libcamera-devel] [PATCH] libcamera: media_device: Documentation fix

[Umang Jain]

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 at 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