[Mesa-dev] [PATCH 2/2] gallium: don't use enum bitfields in p_video_state.h

[younes.m at gmail.com]

On , Christian König <deathsimple at vodafone.de> wrote:
> Am Donnerstag, den 14.07.2011, 15:04 +0200 schrieb Michel Dänzer:

> > On Mit, 2011-07-13 at 16:28 -0600, Brian Paul wrote:

> > > Silences many warnings about "type of bit-field 'field_select' is a

> > > GCC extension".

> > > ---

> > > src/gallium/include/pipe/p_video_state.h | 8 ++++----

> > > 1 files changed, 4 insertions(+), 4 deletions(-)

> > >

> > > diff --git  
> a/src/gallium/include/pipe/p_video_state.hb/src/gallium/include/pipe/p_video_state.h

> > > index 5b47b15..1f42ac5 100644

> > > --- a/src/gallium/include/pipe/p_video_state.h

> > > +++ b/src/gallium/include/pipe/p_video_state.h

> > > @@ -49,8 +49,8 @@ struct pipe_motionvector

> > > {

> > > struct {

> > > signed x:16, y:16;

> > > - enum pipe_video_field_select field_select:16;

> > > - enum pipe_video_mv_weight weight:16;

> > > + unsigned field_select:16; /**
> > > + unsigned weight:16; /**
> > > } top, bottom;

> > > };

> > >

> > > @@ -58,8 +58,8 @@ struct pipe_motionvector

> > > struct pipe_ycbcr_block

> > > {

> > > unsigned x:8, y:8;

> > > - enum pipe_mpeg12_dct_intra intra:8;

> > > - enum pipe_mpeg12_dct_type coding:8;

> > > + unsigned intra:8; /**
> > > + unsigned coding:8; /**
> > > };

> > >

> > > struct pipe_picture_desc

> >

> > Out of curiosity, what's the benefit of using bitfields here at all, as

> > opposed to fixed size integer types?

> >

> > IME bitfields are generally best avoided if possible.

> There is no real benefit, those are there just for historical reasons. I

> started with and interface that looked some like this:



> app copies info into XvMC structure->XvMC state tracker copies into

> those pipe structures->driver copies them into vertex buffers.



> That were a bit too much copy things around for my taste, so I changed

> the driver interface to directly export pointers to the vertex buffers

> with types of pipe_motionvector/pipe_ycbcr_block and was so happy about

> the speed improvements that I forgotten to clean that up.



> I just put it onto my todo list, but not as prio 1. I've anybody has

> time to spare feel free to clean it up.

I wasn't going to bring this up until I had all the hardware decoding bits  
sorted out, however since you mention it, "vertex buffers" make no sense in  
a video decoding interface. You can't assume that a driver can provide such  
a buffer for motion vectors. Also, you moved the buffering policy out into  
the state tracker, which means that every driver will have to buffer an  
entire frame's worth of data even if the hardware doesn't support such  
things. I've backed out of some of these things in my local tree for my own  
purposes, but they'll have to be fixed properly eventually.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110714/9c5ec418/attachment-0001.htm>