[vlc-devel] [PATCH 1/3] nvdec: fix copy heights and increments

[Quentin Chateau]

Oh right, I see what you mean.

What caught my eye is the inconsistency of the "WidthInBytes" and 
"Height" params as well as the increment on the srcY between the 
different copies in the different files.

I suppose that it works because the different values used for the 
different copies all have the same value, even though as you said they 
have a different meaning and cannot be assumed to always be the same. I 
still think there are some things to change with these parameters to 
make them strictly correct, even though I agree I was completely off 
with my patchset.

Thanks to remind me of all these details

On 25/03/2020 14:54, Steve Lhomme wrote:
> Hi,
> On 2020-03-24 17:38, quentin.chateau at deepskycorp.com wrote:
>> From: Quentin Chateau <quentin.chateau at deepskycorp.com>
>>
>> The srcY increment matches the copy height.
>
> Are you sure about that ?
> The bufferHeight is the decoderHeight value which is the 
> CUVIDEOFORMAT.coded_height.
>
> It's very likely that for odd values or codecs like VP8/VP9 there's 
> going to be some padding. The "coded" value is usually the value with 
> the extra padding. That padding is not necessarily the same padding we 
> have on planes in VLC.
>
> Same thing with the horizontal pitch.
>
> It may also be the same in your GPU but not some others that have a 
> different decoder and/or driver.
>
>> The fields WidthInBytes and Height both refer
>> to the same data source to be consistent.
>> ---
>>   modules/hw/nvdec/chroma.c   | 7 ++++---
>>   modules/hw/nvdec/nvdec.c    | 4 ++--
>>   modules/hw/nvdec/nvdec_gl.c | 2 +-
>>   3 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/modules/hw/nvdec/chroma.c b/modules/hw/nvdec/chroma.c
>> index 88353f5cbe..638d8605ec 100644
>> --- a/modules/hw/nvdec/chroma.c
>> +++ b/modules/hw/nvdec/chroma.c
>> @@ -77,6 +77,7 @@ static picture_t * FilterCUDAToCPU( filter_t 
>> *p_filter, picture_t *src )
>>               memset(plane.p_pixels, 0xFF, plane.i_pitch * 
>> plane.i_visible_lines);
>>               continue;
>>           }
>> +
>>           CUDA_MEMCPY2D cu_cpy = {
>>               .srcMemoryType  = CU_MEMORYTYPE_DEVICE,
>>               .srcDevice      = srcpic->devicePtr,
>> @@ -85,13 +86,13 @@ static picture_t * FilterCUDAToCPU( filter_t 
>> *p_filter, picture_t *src )
>>               .dstMemoryType  = CU_MEMORYTYPE_HOST,
>>               .dstHost        = plane.p_pixels,
>>               .dstPitch       = plane.i_pitch,
>> -            .WidthInBytes   = __MIN(srcpic->bufferPitch, 
>> (unsigned)dst->p[0].i_pitch),
>> -            .Height         = __MIN(srcpic->bufferHeight, 
>> (unsigned)plane.i_visible_lines),
>> +            .WidthInBytes   = plane.i_pitch,
>> +            .Height         = plane.i_visible_lines,
>>           };
>>           result = CALL_CUDA(cuMemcpy2DAsync, &cu_cpy, 0);
>>           if (result != VLC_SUCCESS)
>>               goto done;
>> -        srcY += srcpic->bufferHeight;
>> +        srcY += plane.i_visible_lines;
>>       }
>>       picture_CopyProperties(dst, src);
>>   diff --git a/modules/hw/nvdec/nvdec.c b/modules/hw/nvdec/nvdec.c
>> index 76aec6a246..cff5a263d0 100644
>> --- a/modules/hw/nvdec/nvdec.c
>> +++ b/modules/hw/nvdec/nvdec.c
>> @@ -486,13 +486,13 @@ static int CUDAAPI HandlePictureDisplay(void 
>> *p_opaque, CUVIDPARSERDISPINFO *p_d
>>                   .dstMemoryType  = CU_MEMORYTYPE_HOST,
>>                   .dstHost        = plane.p_pixels,
>>                   .dstPitch       = plane.i_pitch,
>> -                .WidthInBytes   = i_pitch,
>> +                .WidthInBytes   = plane.i_pitch,
>>                   .Height         = plane.i_visible_lines,
>>               };
>>               result = CALL_CUDA_DEC(cuMemcpy2DAsync, &cu_cpy, 0);
>>               if (result != VLC_SUCCESS)
>>                   goto error;
>> -            srcY += p_sys->decoderHeight;
>> +            srcY += plane.i_visible_lines;
>>           }
>>       }
>>   diff --git a/modules/hw/nvdec/nvdec_gl.c b/modules/hw/nvdec/nvdec_gl.c
>> index 137e730028..dc69179aea 100644
>> --- a/modules/hw/nvdec/nvdec_gl.c
>> +++ b/modules/hw/nvdec/nvdec_gl.c
>> @@ -138,7 +138,7 @@ tc_nvdec_gl_update(const struct vlc_gl_interop 
>> *interop, GLuint textures[],
>>           result = CALL_CUDA(cuMemcpy2DAsync, &cu_cpy, 0);
>>           if (result != VLC_SUCCESS)
>>               goto error;
>> -        srcY += srcpic->bufferHeight;
>> +        srcY += tex_heights[i];
>>       }
>>     error:
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel