After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 697060 - videoencoder: extract offset and strides from the buffer’s GstVideoMeta if present
videoencoder: extract offset and strides from the buffer’s GstVideoMeta if pr...
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.0.5
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-04-01 18:06 UTC by Jose P. Carballo
Modified: 2013-04-05 15:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch proposed to fix the problem described in this issue. (2.47 KB, patch)
2013-04-01 18:06 UTC, Jose P. Carballo
rejected Details | Review

Description Jose P. Carballo 2013-04-01 18:06:12 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).
Comment 1 Wim Taymans 2013-04-04 08:57:20 UTC
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?
Comment 2 Jose P. Carballo 2013-04-04 15:47:51 UTC
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.
Comment 3 Sebastian Dröge (slomo) 2013-04-05 11:38:16 UTC
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
Comment 4 Jose P. Carballo 2013-04-05 15:36:15 UTC
Thanks Wim and Sebastian for your help. The issue is now closed, the patch is not needed.