Hi, 2012/7/10 ykzhao <[email protected]>: > On Sun, 2012-07-08 at 23:43 -0600, Gwenole Beauchesne wrote: >> Hi, >> >> 2012/7/9 ykzhao <[email protected]>: >> > On Sun, 2012-07-08 at 23:06 -0600, Gwenole Beauchesne wrote: >> >> 2012/7/9 ykzhao <[email protected]>: >> >> > On Sun, 2012-07-08 at 22:28 -0600, Gwenole Beauchesne wrote: >> >> >> > There is no problem when the variable of dri_state is renamed to >> >> >> > drm_state. >> >> >> > But it seems a little confusing as the patch 2 already defines the >> >> >> > structure type of drm_state. >> >> >> >> >> >> Both patches could be merged altogether, if this is what you suggest. >> >> > >> >> > No. What I mean is that the name seems a little confusing. And the >> >> > readability of ctx->drm_state is decreased as the patch 2 defines the >> >> > data structure of drm_state. >> >> >> >> What are you really talking about? And what's your suggestion then? >> > >> > It is OK for me to rename the "dri_state" to drm_state. But to keep the >> > readability of "ctx->dri_state/drm_state", I suggest that the drm_state >> > defined in patch 2 can use another name. (For example: drm_base_state) >> >> I am sorry but I still don't see your point and how this affects >> readability. drm_state is the base class for dri_state, which is a >> DRI-authenticated drm_state + data for its housekeeping. It's pretty >> clear already. Your suggestion actually decreases readability when it >> comes to the VA/DRM case whereby you don't care of extra data, so the >> base drm_state is already sufficient. > > Of course your patch is ok. The readability I mentioned is the > following: > >The drm_state is defined in the structure of VADriverContext, > which is parsed as struct dri_state. > >The patch 2 defines the structure of struct drm_state. > In such case maybe the "drm_state" defined in dri_state will be > misregarded as the type of "struct drm_state".
In VA/X11 implementation, you can obviously use VADriverContext.drm_state as a struct drm_state or a struct dri_state. So, that's still valid, and desired for common code. However, the most relevant object there remains a struct dri_state, which is allocated on the libVA side, so there is no further ambiguity either. I believe we could also document further the structures. Let me send another iteration. > Not sure whether the below is helpful to improve the readability? > a. the struct drm_state in patch 2 is renamed as drm_base_state > b. add one typedef definition to define the struct dri_state as > drm_state. The problem with (b) is that you see lots of dri related definitions, and renaming it to drm_state could be confusing. Regards, Gwenole. _______________________________________________ Libva mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libva
