GNOME Bugzilla – Bug 785029
kmssink: Add dumb buffer support for driver pitch with multi planar formats
Last modified: 2017-07-25 19:46:26 UTC
Fix rendering of multi planar video when pitch is different from the video width.
Created attachment 355759 [details] [review] kmsallocator: inline gst_kms_allocator_alloc_empty() No semantic change, just renamed the 'tmp' variable to a more meaningful name and to use the same structure as in gst_kms_allocator_bo_alloc(). Needed as I'm going to move the gst_memory_init() call after the allocation of the DUMB buffer.
Created attachment 355760 [details] [review] kmsallocator: use the actual maxsize of the allocated buffer Use the actual size of the allocated DUMB buffer to GstMemory rather than assuming it has exactly the requested size. It may be bigger because of alignment constraints. Also add a check to prevent overflow with buggy drivers.
Created attachment 355761 [details] [review] kmsallocator: add pitch support for planar formats We used to to handle the driver pitch only for single plan video format. Add support for multi planes format by re-using the extrapolate function from the v4l2 element. Also use this pitch to calculate the proper offsets.
(In reply to Guillaume Desmottes from comment #0) > Fix rendering of multi planar video when pitch is different from the video > width. So you can report to the driver dev, the selected pitches and offsets are passed back to the driver. So you should never see bad rendering as the driver is supposed to validate and then it may pick a slow path or fail. Bad rendering is not a GStreamer bug here. It's nice feature though to try and match the driver pitch.
Created attachment 356041 [details] [review] kmsallocator: add driver pitch support for planar formats We used to to handle the driver pitch only for single plan video format. Add support for multi planes format by re-using the extrapolate function from the v4l2 element. Also use this pitch to calculate the proper offsets. Prevent DRM drivers to pick a slow path if the pitches/offsets don't match the ones it reported.
Review of attachment 355760 [details] [review]: ::: sys/kms/gstkmsallocator.c @@ +445,1 @@ if (!gst_kms_allocator_add_fb (alloc, kmsmem, vinfo->offset, vinfo)) add_fb still update the vinfo->stride[0] if 1 plane, which render the vinfo->size non-coherant. We also need to remove this hack from add_fb and probably update the vinfo from the bo->pitch, for single plane.
Review of attachment 356041 [details] [review]: ... single plan*e* ... ::: sys/kms/gstkmsallocator.c @@ +427,3 @@ + } else { + offsets[i] = mem_offsets[i]; + } All this code should be done outside of add_fb. When you create the bo, update the VideoInfo properly, setup then memory and then add the framebuffer.
Created attachment 356320 [details] [review] kmsallocator: add driver pitch support for planar formats We used to to handle the driver pitch only for single plan video format. Add support for multi planes format by re-using the extrapolate function from the v4l2 element. Also use this pitch to calculate the proper offsets. Prevent DRM drivers to pick a slow path if the pitches/offsets don't match the ones it reported.
Review of attachment 356320 [details] [review]: ::: sys/kms/gstkmsallocator.c @@ +198,3 @@ + + offs += + continue; This is not working on my platform. Shouldn't we pass the frame height as 3rd argument? If I pass GST_VIDEO_INFO_HEIGHT then it works. @@ +209,2 @@ kmsmem->bo->handle = arg.handle; + /* will be used a memory maxsize */ "as memory maxsize"
Review of attachment 356320 [details] [review]: ::: sys/kms/gstkmsallocator.c @@ +198,3 @@ + + offs += + pitch * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (vinfo->finfo, i, arg.width); Yes, just a typo, does it work for you after this fix ?
Created attachment 356355 [details] [review] kmsallocator: add driver pitch support for planar formats We used to to handle the driver pitch only for single plan video format. Add support for multi planes format by re-using the extrapolate function from the v4l2 element. Also use this pitch to calculate the proper offsets. Prevent DRM drivers to pick a slow path if the pitches/offsets don't match the ones it reported.
Created attachment 356356 [details] [review] kmsallocator: add driver pitch support for planar formats We used to to handle the driver pitch only for single plan video format. Add support for multi planes format by re-using the extrapolate function from the v4l2 element. Also use this pitch to calculate the proper offsets. Prevent DRM drivers to pick a slow path if the pitches/offsets don't match the ones it reported.
Review of attachment 356356 [details] [review]: Looks good to me and working fine on my zynqultrascaleplus board.
Attachment 355759 [details] pushed as f9379b5 - kmsallocator: inline gst_kms_allocator_alloc_empty() Attachment 356356 [details] pushed as 2d67189 - kmsallocator: add driver pitch support for planar formats