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

[Rinat Ibragimov]

ping?

>Среда, 15 июня 2016, 0:27 +03:00 от Rinat <ibragimovrinat at mail.ru>:
>
>В 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", 
>> 
>


---
Rinat