On Tue, 2009-10-27 at 12:11 -0700, Zack Rusin wrote:
> On Saturday 24 October 2009 19:02:51 Younes Manton wrote:
> > Hi Thomas,
> > 
> > I'd like to make these changes to make it possible to use the DRM
> > state tracker interface with contexts other than pipe_context. Nouveau
> > is the only public driver that does DRI1 from what I can see, but I've
> > been told that there are some closed drivers that also depend on these
> > interfaces and that you look after them.
> > 
> > Added drm_api::create_video_context() to get a pipe_video_context, you
> > can ignore it. Changes to the st_* lock functions are trivial and
> > obvious, Nouveau doesn't use them but presumably anyone who does can
> > return pipe_context->priv just as easily. Changes to
> > dri1_api::front_srf_locked() are too, hopefully your front buffer can
> > be accessed via pipe_screen as well. I didn't change
> > dri1_api::present_locked() because we don't do page flipping yet, but
> > I think that could be done with just a screen as well in Nouveau.
> > Thoughts?
> 
> Younes,
> 
> sorry for the slow response, we've been a bit busy lately. 
> I can't say that I'm a huge fan of this patch. I've been thinking about it 
> lately and essentially I'd like to reiterate what I said initially about the 
> pipe_video_context. While I think it's a great idea, I'm not too excited 
> about 
> the way it looks right now. It boils down to three problems for me:
> 
> 1) The interface itself. In particular:
> 
> void (*clear_surface)(struct pipe_video_context *vpipe,
>                          unsigned x, unsigned y,
>                          unsigned width, unsigned height,
>                          unsigned value,
>                          struct pipe_surface *surface);
> is problematic for two reasons: a) because it's something that looks more 
> like 
> a pipe_context method in itself, b) because it's a combination of a 
> surface_fill and clear interface which we moved away from in master
> 
> void (*render_picture)(struct pipe_video_context     *vpipe,
>                           /*struct pipe_surface         *backround,
>                           struct pipe_video_rect        *backround_area,*/
>                           struct pipe_video_surface     *src_surface,
>                           enum pipe_mpeg12_picture_type picture_type,
>                           /*unsigned                    num_past_surfaces,
>                           struct pipe_video_surface     *past_surfaces,
>                           unsigned                      num_future_surfaces,
>                           struct pipe_video_surface     *future_surfaces,*/
>                           struct pipe_video_rect        *src_area,
>                           struct pipe_surface           *dst_surface,
>                           struct pipe_video_rect        *dst_area,
>                           /*unsigned                      num_layers,
>                           struct pipe_texture           *layers,
>                           struct pipe_video_rect        *layer_src_areas,
>                           struct pipe_video_rect        *layer_dst_areas,*/
>                           struct pipe_fence_handle      **fence);
> 
> has simply way too many arguments, not to mention that over a half is 
> commented out. It's really a common problem for the entire interface, methods 
> are very complex. Gallium deals with it with settable state. Which  brings us 
> to another problem with the interface:
> 
> struct pipe_video_context
> {
>    struct pipe_screen *screen;
>    enum pipe_video_profile profile;
>    enum pipe_video_chroma_format chroma_format;
>    unsigned width;
>    unsigned height; 
>         ... methods follow...
> 
> which means that the interface is both an interface and a state. 
> All of it is very un-gallium like.
> 
> 2) We really need a real world video api implemented with the interface 
> before 
> making it public. So it should be proven that openmax or vdpau can actually 
> be 
> implemented using the interface before making it public.
> 
> 3) There's no hardware implementation for the interface and as far as i can 
> see there's no plans for one. It's really what the interfaces are for, until 
> we have people actually working on a hardware implementation there's no 
> reason 
> for this to be a Gallium interface at all.
> 
> That's why i think the whole code should be an auxiliary module in which case 
> patches like this one wouldn't be even necessary.
> 
> When it comes to interfaces it's a lot harder to remove/significantly change 
> an 
> interface than to add a new one, so we should be extremely careful when 
> adding 
> interfaces and at least try to make sure that for all #1, #2 and #3 are 
> fixable.

Yes. Younes, I really think it is better to crystallize this in a
separate branch until it reaches a point where the interfaces are
crystallized. The master branch is not always perfect, but it should
always near a state were a release can be made, and it seems that the
video interfaces need to mature a little more before that.

Jose


------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to