Re: [PATCH 3/6] media: v4l: subdev: Rename sub-device state information access functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 13, 2023 at 11:33:42AM +0000, Sakari Ailus wrote:
> On Fri, Oct 13, 2023 at 02:23:53PM +0300, Laurent Pinchart wrote:
> > On Fri, Oct 13, 2023 at 11:09:36AM +0000, Sakari Ailus wrote:
> > > On Fri, Oct 13, 2023 at 02:04:39PM +0300, Laurent Pinchart wrote:
> > > > On Fri, Oct 13, 2023 at 01:44:21PM +0300, Sakari Ailus wrote:
> > > > > Rename the sub-devices state information access functions, removing
> > > > > "_state" and "_stream" from them. This makes them shorter and so more
> > > > > convenient to use. No other functions will be needed to access this
> > > > > information.
> > > > 
> > > > The new names are too generic, and thus confusing. For instance,
> > > > v4l2_subdev_get_format() is way too close to v4l2_subdev_get_fmt(). I'm
> > > > fine dropping "_stream", but I would like to keep "_state".
> > > > 
> > > 
> > > My intention was actually to rename v4l2_subdev_get_fmt() later on: it's
> > > used in an ops struct, almost uniquely, so its name can be longer without
> > > it being a nuisance. I can include this in the same set.
> > 
> > No objection, as long as the new name is clear.
> > 
> > > The reason for using a shorter names such as v4l2_subdev_get_format() is
> > > that they're nicer to use in the code. The function names we've added
> > > recently are often exceedingly long. There's hardly room for confusion in
> > > this case either: these functions will remain as the only interface to
> > > access information in sub-device state.
> > 
> > I agree that long names are not nice, but too short names end up not
> > being descriptive enough. As these functions operate on a state, I'd
> > like to keep that information in the name to differenciate them from
> > functions operating on the subdev, and use the same state-aware prefix
> > for all similar functions (I expect we'll get more of them). If you can
> > find a good short form for the v4l2_subdev_state_ prefix that we can use
> > through the code base, that would be fine too. And before you ask
> > v4l2_sd_st_ is not good :-)
> 
> What would you say about v4l2_subdev_state_format() etc.? It's a function
> to obtain a pointer to the format on a given pad(/stream) and "get"
> typically isn't a part of the function name in other similar cases.

I like it better than v4l2_subdev_get_format() for sure, but I think the
_get here still looks a bit better. v4l2_subdev_state_get_format() would
already be shorter than the existing function name, do we need to
shorten it even further ?

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux