[Libva] [PATCH v2 3/5] API: backend: rename dri_state to drm_state.

ykzhao yakui.zhao at intel.com
Mon Jul 9 18:08:16 PDT 2012


On Sun, 2012-07-08 at 23:43 -0600, Gwenole Beauchesne wrote:
> Hi,
> 
> 2012/7/9 ykzhao <yakui.zhao at intel.com>:
> > On Sun, 2012-07-08 at 23:06 -0600, Gwenole Beauchesne wrote:
> >> 2012/7/9 ykzhao <yakui.zhao at intel.com>:
> >> > 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".

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.

Of course the above is my point. If you think that it is not very
reasonable, please ignore it.

Thanks.
    Yakui



More information about the Libva mailing list