GNOME Bugzilla – Bug 697060
videoencoder: extract offset and strides from the buffer’s GstVideoMeta if present
Last modified: 2013-04-05 15:36:40 UTC
Created attachment 240320 [details] [review] Patch proposed to fix the problem described in this issue. [1] gst-plugins-good-1.0.5: The function gst_v4l2_buffer_pool_alloc_buffer() (file gstv4l2bufferpool.c) has the ability to set the stride and offset metadata of a buffer, according to the bytes per line information obtained by the capture driver through gst_v4l2_object_set_format() (file gstv4l2object.c). [2] gst-plugins-base-1.0.5: The GstVideoEncoder class (file gstvideoencoder.c), in function gst_video_encoder_setcaps(), do not use the metadata associated with the buffer (see [1]) to fill the state (GstVideoCodecState) structure. We discovered this problem when working with a capture driver that provides lines aligned to 32 bits. For example, for NV12, on a resolution of width 720 (not multiple of 32), the driver would return 736 (multiple of 32) bytes per line, so the stride/offset need to be set according to these particular values, and not just according to the video format, width, and height, which is what is currently being done (see fill_planes() in file video-info.c).
This is not quite right. The idea is that the encoder, when it needs access to the frame data, uses the GstVideoFrame API, which should combine the GstVideoInfo and GstVideoMeta to return a structure with correct strides etc. What encoder is causing problems?
What we think is that is just a difference where you get the correct stride. With this patch, we are able to capture the correct stride in set_format(), which is conveniently called only when configured. If we don't use the patch, we would use handle_frame() like you suggest, but then we would get the stride on every frame, and for efficiency, possibly need to add some logic to get the stride only when we want to (the first frame and caps reconfiguration). In the GstVideoEncoder documentation we read: "Base class collects input data and metadata into a frame and hands this to subclass' handle_frame.", pretty much what you suggested. Now we think this is the most appropriate, please let us know if you have more comments on this matter. The encoder is a hardware accelerated h264 custom encoder, under development by RidgeRun (https://github.com/RidgeRun/gst-ce-plugin/). Thanks.
The strides *can* change for every buffer, the GstMeta with the strides is per-buffer information. You can't reliable get it in set_format
Thanks Wim and Sebastian for your help. The issue is now closed, the patch is not needed.