On Thu, Jun 16, 2011 at 10:14 AM, Jose Fonseca <jfons...@vmware.com> wrote: > > Younes, > > If the thread you quote had been last word on the subject, then then this > thread would have never happened: > > http://www.mail-archive.com/mesa3d-dev@lists.sourceforge.net/msg09601.html > > and this conversation would not be happening neither. > > It's not my intention to make you go around in circles -- that's the result > of lack of consensus. I merely suggested the assimilation of > pipe_video_context by the state tracker as an easy way out, but I'm fine If > you prefer to get HW proof of concept. > > The fact is that there is no pipe_2d_context to date, in any form, and > probably never will, so we can't rely on that as reference to what to do with > pipe_video_context. > > The more I think about it, the more I think that having totally different > kind of actors (pipe_xxx_context) operating on shared resources (like > textures) is not sustainable, even if these pipe_xxx_contexts ultimately > share the same winsys, I still think that having the video interface to be an > extension or inclusive part of pipe_context is preferable, as it is > cleaner/leaner. > > > I've also looked the way DXVA API/DDI operates, and I think that if we change > the interface, such that pipe_screen::video_context_create that takes a pipe > context argument as: > > struct pipe_screen { > ... > > struct pipe_video_context * > (*video_context_create)( struct pipe_screen *, > struct pipe_context *); > ... > }; > > > and enforce this pairing of pipe_video_context <-> pipe_context > > struct pipe_video_context { > struct pipe_context *pipe; > > ... > }; > > then this lets us get the best of all worlds: > > - no need of duplicating pipe_context methods in pipe_video_context -- simply > use the pipe_context methods from the associated pipe_video_context->pipe > > - ties nicely with the pipe_context based implementation of > pipe_video_context in gallium/auxiliary > > - ties nicely with the way DXVA API/DDI works > > > Jose >
I'm not suggesting that there shouldn't be discussion, just that the state tracker approach was where we started more than 2 years ago and going back has no technical benefits. I don't see why it should be set in stone that to do HW decoding you also need a 3D context. Practically speaking, yes, most hardware will actually use the 3D pipeline to do the color conversion and scaling, but in such a case one can create a pipe_context in their pipe_video_context. (This is actually what the nvfx HW decoder did.) However, if I present you with hardware where the 3D and decoding HW have separate command buffers and state and the decoding HW can independently get the final image to a DRI2 buffer, it makes no sense to force the state tracker to create to create a pipe_context and all the associated 3D machinery when it will have no use. I realize the DDI takes a different approach and throws everything under the sun into the device interface (2D, shader 3D, ff 3D, video dec, cryptography, modeset... I can only imagine that decent drivers there instantiate things lazily such that if the runtime never calls say, the 3D functionality at all and they're not needed by whichever functions are being called, then no resources associated with that block are ever initialized). If that's the approach we want to take with pipe_context that's fine, but in my mind I thought people would want to keep separate interfaces separate and not clutter pipe_context with everything. So a question that remains is how much overlap is there between pipe_context and pipe_video_context? As far as duplicate or nearly duplicate functionality goes I see: ::create_sampler_view() - This shouldn't be in the interface. Video decoding shouldn't have to care about "samplers" per se, it's an implementation detail specific to 3D decoding. From what I can see Christian changed it for performance reasons; originally the interface dealt with textures, not samplers, but for 3D decoding we had to keep samplers handy somewhere so I had a hash map that kept track of this. Probably a better interface is to subclass textures for 3D decoding and hang samplers off them, but I don't recall if I considered and rejected this for any particular reason. ::create_surface() - This used to be part of pipe_screen. I don't recall the motivation for making it part of the context, but every driver does the same thing, malloc a driver-specific subclass of pipe_surface and fill it. I would argue that since video decoding deals with planar/mixed surfaces, anything special that has to be done for them doesn't need to clutter up pipe_context, which will likely never have to deal with these types of surfaces. But again, I'm not sure why this is context-specific and why it shouldn't be elsewhere. ::is_format_supported() and ::get_param() - Not in pipe_context but in pipe_screen. I already explained why I thought these should be here. The names might be the same, but the types of formats being queried and the caps being returned won't overlap with what pipe_screen is doing, both because decoding hardware is concerned with a larger set of formats and has unique caps and because potentially those formats and caps are context-dependent. Anyhow, I don't have a problem with following what the DDI does, even though I think separating concerns and encapsulation is better. I also don't see why this thing has to be perfect from the get-go, considering how many times the gallium interface has changed over the years. With the current structure, changing any part of the video interface will have very little impact on anything else. Anyhow, we seem to have a relatively few POVs here. Thanks Jose and Roland for taking the time to look things over and provide feedback. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev