Hi Sakari, On 05/05/2012 03:15 PM, Sakari Ailus wrote: > Sylwester Nawrocki wrote: >> After introduction of the selection API on subdevs we have following sets >> of selection targets: >> >> /dev/v4l-subdev? | /dev/video? >> ------------------------------------------------------------------------- >> V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL | V4L2_SEL_TGT_CROP_ACTIVE >> V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS | V4L2_SEL_TGT_CROP_BOUNDS >> | V4L2_SEL_TGT_CROP_DEFAULT >> | V4L2_SEL_TGT_CROP_PADDED >> V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTUAL | V4L2_SEL_TGT_COMPOSE_ACTIVE >> V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS | V4L2_SEL_TGT_COMPOSE_BOUNDS >> | V4L2_SEL_TGT_COMPOSE_DEFAULT >> | V4L2_SEL_TGT_COMPOSE_PADDED >> >> Although not exactly the same, the meaning of V4L2_SEL_TGT_*_ACTIVE >> and V4L2_SUBDEV_SEL_TGT_*_ACTUAL selection targets is logically the >> same. Different names add to confusion where both APIs are used in >> a single driver or an application. >> Then, rename the V4l2_SEL_TGT_[CROP/COMPOSE]_ACTIVE to >> V4l2_SEL_TGT_[CROP/COMPOSE]_ACTUAL to avoid the API inconsistencies. >> The selections API is experimental, so no any compatibility layer >> is added. The ABI remains unchanged. > > I'm definitely for keeping the two sets of target as uniform as possible. > > I have one question, though: how about dropping the ACTIVE / ACTUAL > altogether? Then we'd have V4L2_SUBDEV_SEL_TGT_CROP and > V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS etc. I feel that the ACTIVE or ACTUAL > doesn't really say anything there, and don't think it's an issue that a > target name is a part of another one. Agreed. I really like the idea of dropping the rather meaningless postfixes. I don't see an issue here with the subdev API, 'crop/compose' and 'crop/compose bounds' would mean exactly what it says. Only V4L2_SEL_TGT_COMPOSE_PADDED stands a bit out. I don't see any driver really using it at the moment though and I recall Tomasz saying it wasn't best idea to add it in first place. So I believe V4L2_SEL_TGT_COMPOSE_PADDED might get removed from the API in future. I'm going to rework the patch, then let's see what's Tomasz's and others' opinion. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
