[Mesa-dev] Status of VDPAU and XvMC state-trackers (was Re: Build error on current xvmc-r600 pipe-video)

[Jose Fonseca]

----- Original Message -----
> On Mon, Jun 13, 2011 at 3:23 PM, Jose Fonseca <jfonseca at vmware.com>
> wrote:
> >
> > ----- Original Message -----
> >> Am 05.06.2011 06:31, schrieb Younes Manton:
> >> > 2011/6/4 Jose Fonseca <jfonseca at vmware.com>:
> >> >> At very least there are ovious things that need to be fixed:
> >> >>
> >> >> - get_param / is_format_supported should not be duplicated from
> >> >> screen.
> >> >
> >> > This is also deliberate.
> >
> >> > Params and surface formats may depend on
> >> > the
> >> > codec/profile/dimensions of the video stream the context was
> >> > created
> >> > to decode. There is not always enough info available in
> >> > pipe_screen
> >> > alone to determine if a particular cap or surface is supported.
> >> > The
> >> > current implementation largely wraps pipe_screen because again
> >> > it's
> >> > using the 3D pipeline and we don't have to deal with funny
> >> > decoding
> >> > hardware constraints.
> >>
> >> I'm not sure if that's the right answer though, couldn't you just
> >> as
> >> well require a driver to handle all dimensions etc. for a given
> >> codec?
> >> If necessary should just use the shader stages (or cpu...)
> >> instead?
> >>
> >> Also, just glancing over the interface changes:
> >> +enum pipe_video_codec
> >> +{
> >> +   PIPE_VIDEO_CODEC_UNKNOWN = 0,
> >> +   PIPE_VIDEO_CODEC_MPEG12,   /**< MPEG1, MPEG2 */
> >> +   PIPE_VIDEO_CODEC_MPEG4,    /**< DIVX, XVID */
> >> +   PIPE_VIDEO_CODEC_VC1,      /**< WMV */
> >> +   PIPE_VIDEO_CODEC_MPEG4_AVC /**< H.264 */
> >> +};
> >> +
> >> +enum pipe_video_profile
> >> +{
> >> +   PIPE_VIDEO_PROFILE_UNKNOWN,
> >> +   PIPE_VIDEO_PROFILE_MPEG1,
> >> +   PIPE_VIDEO_PROFILE_MPEG2_SIMPLE,
> >> +   PIPE_VIDEO_PROFILE_MPEG2_MAIN,
> >> +   PIPE_VIDEO_PROFILE_MPEG4_SIMPLE,
> >> +   PIPE_VIDEO_PROFILE_MPEG4_ADVANCED_SIMPLE,
> >> +   PIPE_VIDEO_PROFILE_VC1_SIMPLE,
> >> +   PIPE_VIDEO_PROFILE_VC1_MAIN,
> >> +   PIPE_VIDEO_PROFILE_VC1_ADVANCED,
> >> +   PIPE_VIDEO_PROFILE_MPEG4_AVC_BASELINE,
> >> +   PIPE_VIDEO_PROFILE_MPEG4_AVC_MAIN,
> >> +   PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH
> >> +};
> >> Do you really need both here?
> >>
> >> @@ -229,9 +229,27 @@ enum pipe_format {
> >>     PIPE_FORMAT_L32A32_FLOAT            = 161,
> >>     PIPE_FORMAT_I32_FLOAT               = 162,
> >>
> >> +   PIPE_FORMAT_YV12                    = 163,
> >> +   PIPE_FORMAT_YV16                    = 164,
> >> +   PIPE_FORMAT_IYUV                    = 165,  /**< aka I420 */
> >> +   PIPE_FORMAT_NV12                    = 166,
> >> +   PIPE_FORMAT_NV21                    = 167,
> >> +   PIPE_FORMAT_AYUV                    =
> >> PIPE_FORMAT_A8R8G8B8_UNORM,
> >> +   PIPE_FORMAT_VUYA                    =
> >> PIPE_FORMAT_B8G8R8A8_UNORM,
> >> +   PIPE_FORMAT_XYUV                    =
> >> PIPE_FORMAT_X8R8G8B8_UNORM,
> >> +   PIPE_FORMAT_VUYX                    =
> >> PIPE_FORMAT_B8G8R8X8_UNORM,
> >> +   PIPE_FORMAT_IA44                    = 168,
> >> +   PIPE_FORMAT_AI44                    = 169,
> >> +
> >>     PIPE_FORMAT_COUNT
> >>  };
> >> Defining these formats as another format feels wrong. There might
> >> be
> >> reasons why you'd want to use them in normal 3d contexts (ok maybe
> >> not
> >> really) but if you can't distinguish the format that's a no go.
> >
> > Yes this is also incorrect. Blitting from PIPE_FORMAT_AYUV to
> > PIPE_FORMAT_A8R8G8B8_UNORM is not a no-op.
> > I actually have llvmpipe AYUV support implemented in a private
> > branch.
> 
> This wasn't intended to be permanent, just a quick hack that worked
> for softpipe and 3D decoding, since for the 3D decoding case you'll
> end up substituting an RGBA surface for AYUV anyhow.
> 
> >> Frankly I'm not sure if all these formats really should be simple
> >> PIPE_FORMATs even, since chances you can use them in normal 3d
> >> contexts
> >> are next to zero anyway (especially the planar stuff hurts).
> >
> > That's fine. Pixel formats just need to uniquely describe out to
> > interpret the pixels. A 3d context doesn't need to support all of
> > them.
> >
> > I'll see
> >> though where that's coming from (as pipe_surface,
> >> pipe_sampler_state
> >> and
> >> friends are reused, even though the entry points are not). Though
> >> I'm
> >> not sure the all-new-entry points with reused gallium structs is
> >> really
> >> the right approach. Maybe if you need separate contexts etc.
> >> anyway
> >> (to
> >> be able to exploit video hardware) it would be better if you'd
> >> just
> >> use
> >> all your own structs better suited for video tasks. The vl code
> >> could
> >> then translate that stuff to "normal" gallium.
> >> If others are happy with the interface, I won't object though.
> >> I've
> >> no
> >> clue really how a better interface would look like...
> >
> > My gut feel looking at [1] is that pipe_video_context interface
> > should be either an extension of pipe_context, or optional
> > entry-points in pipe_context. Because there's a lot of
> > functionality needed for a pipe_context (low level resource
> > management, relocations, fences), that will be definitely useful
> > for video processing too.  And instead of duplicating/mirroring
> > entry-points the existing entry-points should be enlarged to
> > convey the necessary semantics.
> >
> > But honestly, why don't we simply move pipe_video_context out of
> > src/gallium/include into the state tracker for the time being, and
> > leave the admittedly complex problem of designing the gallium
> > interface until there is an actual use case / proof-of-concept in
> > hardware? This means we can use the video state tracker without
> > having to make boiler plate changes for every single pipe driver,
> > nor advertising an interface for which there's no confidence it is
> > adequate.
> 
> It started out as a state tracker. I don't see what there is to gain
> by putting more work into moving back just to move back yet again at
> some point in the future. Rather than going in circles I'll just
> defer
> to what was discussed previously when I first brought it up on the
> list. [1]
> 
> If drivers want to share code between their 3D and decoding contexts
> they can already do that via pipe_screen/winsys/libdrm, at least
> that's the case with Nouveau, however I don't think that subclassing
> pipe_context or having optional hooks there is significantly
> different
> than having it in pipe_video_context when it comes down to it.
> 
> Also, there was a working hardware implementation, it just wasn't
> merged into the Nouveau kernel module nor libdrm and so not to
> pipe-video either, however that implementation worked without many
> changes to the current pipe_video_context. Now that Nouveau has
> kernel
> support I'm going to try and resurrect those patches and adapt them
> to
> Christian's changes so we can iron out any HW issues.
> 
> [1]
> http://www.mail-archive.com/mesa3d-dev@lists.sourceforge.net/msg06301.html


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