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 719953 - encoder: h264: fix coded buffer size
encoder: h264: fix coded buffer size
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on: 719529
Blocks: 719412
 
 
Reported: 2013-12-06 09:12 UTC by Gwenole Beauchesne
Modified: 2014-01-13 16:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Header size limits (9.50 KB, text/plain)
2013-12-06 12:36 UTC, Gwenole Beauchesne
Details

Description Gwenole Beauchesne 2013-12-06 09:12:15 UTC
VA coded buffers are GPU allocated memory and we should avoid possible buffer
overflows that would hang the system. The coded buffer limits set in base
GstVaapiEncoder is only valid for the macroblock layer. Extra storage is required to fit additional packed headers we insert into the resulting coded buffer.
Comment 1 Gwenole Beauchesne 2013-12-06 09:16:30 UTC
Fixed in git master branch actually. Bug opened to keep track of the resolution. It's incomplete but enough for the current usages and implementation. The extra size needed to hold the headers could be 300+ KB.

Bug kept open until bug #719529 is fixed in a way to also provide the maximum number of supported slices per frame to the upper layers. An additional patch is needed here so that to use at most slice_num slices in the calculation.
Comment 2 Gwenole Beauchesne 2013-12-06 12:36:25 UTC
Created attachment 263659 [details]
Header size limits

For reference, here is the list of syntax elements I used to compute the maximum number of bits for each header, covering the valid range of values as of the 2012 edition of the standard.

Ideally, we could probably fine tune the limits, based on the coded resolution for instance, but an approximate 300 KB size overhead looks acceptable enough(?) for each coded buffer.
Comment 3 Gwenole Beauchesne 2014-01-13 06:32:44 UTC
This is now fixed, with the exact number of slices used to compute the total size of slice headers. We could probably include an extra 4-bytes at the end for 00 00 00 00, but this rather looks like an implementation detail that should be handled internally at the driver level, should it require it.
Comment 4 Gwenole Beauchesne 2014-01-13 16:37:01 UTC
commit 00e0af9a7c471fc59c6cb33589b5462c98cd79f8
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Sun Jan 12 22:14:11 2014 +0100

    encoder: h264: refine size of coded buffer.
    
    Refine the heuristic to determine the maximum size of a coded buffer
    to account for the exact number of slices. set_context_info() is the
    last step during codec reconfiguration, no additional change is done
    afterwards, so re-using the num_slices field here is fine.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=719953

commit 2940a74ea45956edb8ec3e94bb983e255fcf0dc1
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Thu Dec 5 18:13:54 2013 +0100

    encoder: fix computation of max coded buffer size (again).
    
    The previous fix was only valid to express the maximum size of the
    macroblock layer, i.e. without any headers. Now, also account for
    the slice headers and top picture header, but also any other header
    we might stuff into the VA coded buffer, e.g. sequence headers.

commit b864d1f71acc47bd41b8e7171c4ad4d6628849f4
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Wed Dec 4 19:10:13 2013 +0100

    encoder: fix computation of max coded buffer size.
    
    Fix coded buffer size for each codec. A generic issue was that the
    number of macroblocks was incorrectly computed. The second issue was
    specific to MPEG-2 were the max number of bits per macroblock, and
    as defined by the standard, was incorrectly mapped to the (lower)
    H.264 requirement. i.e. 4608 bits vs. 3200 bits limit.