[vlc-devel] [PATCH] vdpau: fix video scaling

[Rinat]

В Mon, 13 Jun 2016 21:27:12 +0300
Rémi Denis-Courmont <remi at remlab.net> пишет:

> [ Adding VDPAU list]
> 
> On Monday 13 June 2016 19:53:02 Rinat Ibragimov wrote:
> > There is a possible issue in VdpVideoMixerRender [1] call. While
> > destination_rect is set to dst_rect, which is fine, destination_video_rect
> > parameter is set to NULL, which makes video keep the size, while it's
> > expected to fill the entire output rectangle.  
> 
> The specification states that:
> "If [destination_video_rect is]_NULL, the destination rectangle will be sized 
> to match the source rectangle, and will be located at the origin."
> 
> This is confusing, because the destination rectangle parameter is always set 
> explicitly, and so it obviously cannot be (re)sized and (re)positioned 
> relative to its own self, but to the output surface. Only the destination 
> video rectangle is optional.

Yep, I was always implicitly inserting word "video" while read that part. Now
when you pointed that out, it's confusing indeed.

> 
> You interpret it as:
> "If [destination_video_rect is]_NULL, the destination video rectangle will 
> have the same size as the source rectangle, and will be located at the 
> origin."

Yep, my thoughts exactly.

> 
> But I think that the intended meaning was rather:
> "If [destination_video_rect is]_NULL, the destination video rectangle will 
> fit/match the source rectangle to the destination rectangle, and will be 
> located at the origin.

But there was nothing about fitting source rectangle to destination rectangle.
Only destination to source.

Could you also clarify how other VDPAU driver should handle NULL for
destination_video_rect, if your interpretation is the right one? Should it
fill whole destination rectangle then? What should happen, if destination
rectangle was specified, but its top-left wasn't (0,0)? Should be right-bottom
point of destination rectangle kept, or should be width and height taken from
destination rectangle while moving destination video rectangle to the (0,0)?
Is there anything about keeping aspect ratio?

> 
> In any case, that appears to be on the behaviour of the de factor reference 
> NVIDIA drivers, and it is a much more reasonable and useful default, than what 
> you propose.

I was told by two different people that they were experiencing scaling issues
with NVIDIA adapters also. But during details extraction it turned out that they
both have hardware with NVIDIA Optimus, so it wasn't NVIDIA's VDPAU driver they
used. Since then I also had a chance to check it on NVIDIA adapter, and scaling
was fine there.

I'm not very happy with that interpretation, but must admit that behavior of 
NVIDIA's drivers is the reference.

> 
> Aaron, can you bring some clarity here? Either way, I think the Doxygen should 
> be clarified.
> 
> > Proposed patch sets destination_video_rect to dst_rect, which makes
> > video scale when player window is resized.
> > 
> > 
> > [1]
> > http://http.download.nvidia.com/XFree86/vdpau/doxygen/html/group___vdp_vide
> > o_mixer.html#ga62bf3bf8c5f01322a03b07065c5ea3db
> > 
> > ---
> >  modules/hw/vdpau/chroma.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/modules/hw/vdpau/chroma.c b/modules/hw/vdpau/chroma.c
> > index a42aed2..d13515f 100644
> > --- a/modules/hw/vdpau/chroma.c
> > +++ b/modules/hw/vdpau/chroma.c
> > @@ -652,7 +652,8 @@ static picture_t *Render(filter_t *filter, picture_t
> > *src, bool import) err = vdp_video_mixer_render(sys->vdp, sys->mixer,
> > VDP_INVALID_HANDLE, NULL, structure,
> >                                   MAX_PAST, past, surface, MAX_FUTURE,
> > future, -                                 &src_rect, output, &dst_rect,
> > NULL, 0, NULL); +                                 &src_rect, output,
> > &dst_rect, &dst_rect, 0, +                                 NULL);
> >      if (err != VDP_STATUS_OK)
> >      {
> >          msg_Err(filter, "video %s %s failure: %s", "mixer", "rendering",  
>