[Mesa-dev] [PATCH 2/7] st/vdpau: exit gracefully if we fail to create video buffer

[Christian König]

Am 18.08.2013 14:20, schrieb Emil Velikov:
> On 18/08/13 12:31, Christian König wrote:
>> Am 17.08.2013 23:51, schrieb Emil Velikov:
>>> Otherwise we risk causing memory corruption.
>>>
>>> v2: forgot the mutex_unlock()
>>>
>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> NAK. Failing is actually not correct here, cause we can still make it
>> work by allocating the video buffer later on decoding or uploading of
>> image data.
>>
> Let me see if I get this right
>
> The API dictates that one can create a surface and there is no guarantee
> that a video buffer will be associated with it, is that correct ?

Actually the API doesn't dictates anything about this (late vs. early 
allocation of buffers), it's more like I want to give the driver control 
over this.

> Afaics after creating such a surface anyone can call
> vlVdpVideoSurfaceGetBitsYCbCr() thus getting a NO_IMPLEMENTATION, but
> then again who in their right mind would want to do that :)

Returning NO_IMPLEMENTATION in case a surface doesn't have a backing 
store yet probably isn't such a good idea. Just clearing the destination 
buffers to black in vlVdpVideoSurfaceGetBitsYCbCr sound like the valid 
response to this.

Regards,
Christian.

> Cheers
> Emil
>> Instead we shouldn't try to create the video buffer in the first place
>> if get_video_param(...,PIPE_VIDEO_PROFILE_UNKNOWN,
>> PIPE_VIDEO_CAP_PREFERED_FORMAT) returns PIPE_FORMAT_NONE.
>>
>> Additional to that if the surface can't be created we shouldn't call
>> vlVdpVideoSurfaceClear.
>>
>> Christian.
>>
>>> ---
>>>    src/gallium/state_trackers/vdpau/surface.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/src/gallium/state_trackers/vdpau/surface.c
>>> b/src/gallium/state_trackers/vdpau/surface.c
>>> index 1bebe6c..16ad5c4 100644
>>> --- a/src/gallium/state_trackers/vdpau/surface.c
>>> +++ b/src/gallium/state_trackers/vdpau/surface.c
>>> @@ -87,6 +87,12 @@ vlVdpVideoSurfaceCreate(VdpDevice device,
>>> VdpChromaType chroma_type,
>>>          PIPE_VIDEO_CAP_PREFERS_INTERLACED
>>>       );
>>>       p_surf->video_buffer = pipe->create_video_buffer(pipe,
>>> &p_surf->templat);
>>> +   if (!p_surf->video_buffer) {
>>> +      pipe_mutex_unlock(dev->mutex);
>>> +      ret = VDP_STATUS_RESOURCES;
>>> +      goto inv_device;
>>> +   }
>>> +
>>>       vlVdpVideoSurfaceClear(p_surf);
>>>       pipe_mutex_unlock(dev->mutex);
>>>    
>