[media-ctl PATCH 1/1] libmediactl: Implement MEDIA_ENT_ID_FLAG_NEXT in media_get_entity_by_id()

[media-ctl PATCH 1/1] libmediactl: Implement MEDIA_ENT_ID_FLAG_NEXT in media_get_entity_by_id()

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 src/mediactl.c |    9 +++++++--
 src/mediactl.h |    4 +++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/mediactl.c b/src/mediactl.c
index 5b8c587..f62fcdf 100644
--- a/src/mediactl.c
+++ b/src/mediactl.c
@@ -81,8 +81,13 @@ struct media_entity *media_get_entity_by_id(struct media_device *media,
 	for (i = 0; i < media->entities_count; ++i) {
 		struct media_entity *entity = &media->entities[i];
 
-		if (entity->info.id == id)
-			return entity;
+		if (!(id & MEDIA_ENT_ID_FLAG_NEXT)) {
+			if (entity->info.id == id)
+				return entity;
+		} else {
+			if (entity->info.id >= (id & ~MEDIA_ENT_ID_FLAG_NEXT)
+				return entity;
+		}
 	}
 
 	return NULL;
diff --git a/src/mediactl.h b/src/mediactl.h
index 1b47b7e..4d3892e 100644
--- a/src/mediactl.h
+++ b/src/mediactl.h
@@ -164,7 +164,9 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
  * @param media - media device.
  * @param id - entity ID.
  *
- * Search for an entity with an ID equal to @a id.
+ * Search for an entity with an ID equal to @a id. If id flag
+ * MEDIA_ENT_ID_FLAG_NEXT is present, an entity with ID greater or equal to
+ * @a id will be returned.
  *
  * @return A pointer to the entity if found, or NULL otherwise.
  */
-- 
1.7.2.5

Re: [media-ctl PATCH 1/1] libmediactl: Implement MEDIA_ENT_ID_FLAG_NEXT in media_get_entity_by_id()

[Laurent Pinchart]

Hi Sakari,

Thanks for the patch.

On Saturday 14 January 2012 20:33:36 Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  src/mediactl.c |    9 +++++++--
>  src/mediactl.h |    4 +++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mediactl.c b/src/mediactl.c
> index 5b8c587..f62fcdf 100644
> --- a/src/mediactl.c
> +++ b/src/mediactl.c
> @@ -81,8 +81,13 @@ struct media_entity *media_get_entity_by_id(struct
> media_device *media, for (i = 0; i < media->entities_count; ++i) {
>  		struct media_entity *entity = &media->entities[i];
> 
> -		if (entity->info.id == id)
> -			return entity;
> +		if (!(id & MEDIA_ENT_ID_FLAG_NEXT)) {
> +			if (entity->info.id == id)
> +				return entity;
> +		} else {
> +			if (entity->info.id >= (id & ~MEDIA_ENT_ID_FLAG_NEXT)
> +				return entity;
> +		}

Just one question that hasn't crossed my mind before, why do you need this ? 
If you want to enumerate entities in an application you can just iterate over 
media_device::entities.

>  	}
> 
>  	return NULL;
> diff --git a/src/mediactl.h b/src/mediactl.h
> index 1b47b7e..4d3892e 100644
> --- a/src/mediactl.h
> +++ b/src/mediactl.h
> @@ -164,7 +164,9 @@ struct media_entity *media_get_entity_by_name(struct
> media_device *media, * @param media - media device.
>   * @param id - entity ID.
>   *
> - * Search for an entity with an ID equal to @a id.
> + * Search for an entity with an ID equal to @a id. If id flag
> + * MEDIA_ENT_ID_FLAG_NEXT is present, an entity with ID greater or equal
> to + * @a id will be returned.
>   *
>   * @return A pointer to the entity if found, or NULL otherwise.
>   */

-- 
Regards,

Laurent Pinchart

Re: [media-ctl PATCH 1/1] libmediactl: Implement MEDIA_ENT_ID_FLAG_NEXT in media_get_entity_by_id()

[Sakari Ailus]

Hi Laurent,

Laurent Pinchart wrote:
> On Saturday 14 January 2012 20:33:36 Sakari Ailus wrote:
>> Signed-off-by: Sakari Ailus<sakari.ailus@iki.fi>
>> ---
>>   src/mediactl.c |    9 +++++++--
>>   src/mediactl.h |    4 +++-
>>   2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mediactl.c b/src/mediactl.c
>> index 5b8c587..f62fcdf 100644
>> --- a/src/mediactl.c
>> +++ b/src/mediactl.c
>> @@ -81,8 +81,13 @@ struct media_entity *media_get_entity_by_id(struct
>> media_device *media, for (i = 0; i<  media->entities_count; ++i) {
>>   		struct media_entity *entity =&media->entities[i];
>>
>> -		if (entity->info.id == id)
>> -			return entity;
>> +		if (!(id&  MEDIA_ENT_ID_FLAG_NEXT)) {
>> +			if (entity->info.id == id)
>> +				return entity;
>> +		} else {
>> +			if (entity->info.id>= (id&  ~MEDIA_ENT_ID_FLAG_NEXT)
>> +				return entity;
>> +		}
>
> Just one question that hasn't crossed my mind before, why do you need this ?
> If you want to enumerate entities in an application you can just iterate over
> media_device::entities.

We do have the MEDIA_ENT_ID_FLAG_NEXT flag which is intended to help in 
entity enumeration. Currently the range of entity ids is contiguous in 
all practical implementation but will that always be the case, also in 
the future? A few things might break in the kernel if the range is 
non-contiguous as well, but that's still internal to the kernel.

However, this is a user space library and if this interface change is 
not made, we essentially are making a promise that the entity ranges 
will always be contiguous.

I wouldn't as there's no need to do so.

I you think about programmable hardware, entities there are logical 
rather than physical and their existence may be dependent on multiple 
factors.

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [media-ctl PATCH 1/1] libmediactl: Implement MEDIA_ENT_ID_FLAG_NEXT in media_get_entity_by_id()

[Laurent Pinchart]

Hi Sakari,

On Sunday 15 January 2012 16:40:56 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Saturday 14 January 2012 20:33:36 Sakari Ailus wrote:
> >> Signed-off-by: Sakari Ailus<sakari.ailus@iki.fi>
> >> ---
> >> 
> >>   src/mediactl.c |    9 +++++++--
> >>   src/mediactl.h |    4 +++-
> >>   2 files changed, 10 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/src/mediactl.c b/src/mediactl.c
> >> index 5b8c587..f62fcdf 100644
> >> --- a/src/mediactl.c
> >> +++ b/src/mediactl.c
> >> @@ -81,8 +81,13 @@ struct media_entity *media_get_entity_by_id(struct
> >> media_device *media, for (i = 0; i<  media->entities_count; ++i) {
> >> 
> >>   		struct media_entity *entity =&media->entities[i];
> >> 
> >> -		if (entity->info.id == id)
> >> -			return entity;
> >> +		if (!(id&  MEDIA_ENT_ID_FLAG_NEXT)) {
> >> +			if (entity->info.id == id)
> >> +				return entity;
> >> +		} else {
> >> +			if (entity->info.id>= (id&  ~MEDIA_ENT_ID_FLAG_NEXT)
> >> +				return entity;
> >> +		}
> > 
> > Just one question that hasn't crossed my mind before, why do you need
> > this ? If you want to enumerate entities in an application you can just
> > iterate over media_device::entities.
> 
> We do have the MEDIA_ENT_ID_FLAG_NEXT flag which is intended to help in
> entity enumeration. Currently the range of entity ids is contiguous in
> all practical implementation but will that always be the case, also in
> the future? A few things might break in the kernel if the range is
> non-contiguous as well, but that's still internal to the kernel.
> 
> However, this is a user space library and if this interface change is
> not made, we essentially are making a promise that the entity ranges
> will always be contiguous.

I definitely don't want to make that promise, but what's the point in calling 
media_get_entity_by_id() for entity enumeration instead of iterating over the 
media_device::entities array ?

> I wouldn't as there's no need to do so.
> 
> I you think about programmable hardware, entities there are logical
> rather than physical and their existence may be dependent on multiple
> factors.

-- 
Regards,

Laurent Pinchart