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 719820 - encoder: h264: crash in CQP mode with min-qp=48
encoder: h264: crash in CQP mode with min-qp=48
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-04 05:51 UTC by Wind Yuan
Modified: 2014-01-13 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix wrong qp init (2.23 KB, patch)
2013-12-04 05:56 UTC, Wind Yuan
none Details | Review

Description Wind Yuan 2013-12-04 05:51:56 UTC
gst-launch-1.0 -v filesrc location=720p5994_parkrun_ter.yuv '!' videoparse format=i420 width=1280 height=720 '!' vaapiencodeh264 rate-control=cqp init-qp=48 key-period=1 max-bframes=0 '!' qtmux faststart=true '!' filesink location=enc.mp4

gst-launch-1.0: gen6_mfc.c:1190: gen6_mfc_avc_batchbuffer_slice: Assertion `qp >= 0 && qp < 52' failed.
Aborted (core dumped)
Comment 1 Wind Yuan 2013-12-04 05:56:17 UTC
Created attachment 263484 [details] [review]
fix wrong qp init

The crashed caused by 2 issues.
1. init_qp not initialized in plugin _init
2. slice_qp_delta set wrong. the fix currently set to 0 before any better idea on qp_delta.
Comment 2 Gwenole Beauchesne 2013-12-04 07:18:09 UTC
(In reply to comment #1)
> The crashed caused by 2 issues.
> 1. init_qp not initialized in plugin _init

It should, through the default values of the properties.
Comment 3 Wind Yuan 2013-12-04 09:50:58 UTC
a typo in the patch,
+  encoder->min_qp = -1;
should be
encode->min_qp = -1;
Comment 4 Gwenole Beauchesne 2014-01-13 06:24:00 UTC
The initial bug (crash) can be fixed by adding G_PARAM_CONSTRUCT to the property flags. That way, the properties are initialized to their correct default values, and all of them needed that actually.

Alternative is the fix derived from bug #719529. That is enough to avoid the crash.

Now, for slice_qp_delta, Wind, could you please open a new issue and more detailed discussion on it? There is indeed something else to do about it, but we could postpone it, as the delta can be negative, we can still get the expected QP constant if init_qp < min_qp.
Comment 5 Wind Yuan 2014-01-13 08:34:57 UTC
(In reply to comment #4)
> The initial bug (crash) can be fixed by adding G_PARAM_CONSTRUCT to the
> property flags. That way, the properties are initialized to their correct
> default values, and all of them needed that actually.
> 
> Alternative is the fix derived from bug #719529. That is enough to avoid the
> crash.
Yes, if this can be fixed by bug #719529 should be better.
> 
> Now, for slice_qp_delta, Wind, could you please open a new issue and more
> detailed discussion on it? There is indeed something else to do about it, but
> we could postpone it, as the delta can be negative, we can still get the
> expected QP constant if init_qp < min_qp.
I opened new bug to for slice_qp_delta in bug #722082. suggestions welcome.
min_qp should constrain the slice/macro-block final qp larger than or equal to mini_qp if I understand correctly. but since intel-driver doesn't have this constraint and old libva min_qp removed, it may only affect the slice_qp_delta
Comment 6 Gwenole Beauchesne 2014-01-13 16:43:11 UTC
The original commit where the crash was fixed is:

commit a43d06dcf5b109165ba39c8214124433c4d4a5ee
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Wed Jan 8 18:36:46 2014 +0100

    vaapiencode: update for new properties API.
    
    Update MPEG-2 and H.264 encode elements to cope with the new core
    libgstvaapi properties API. i.e. all configurable properties are now
    directly handled at the GstVaapiEncoder level.
    
    Besides, this also makes sure to not use or modify the GstVaapiEncoder
    private definitions directly. Private data need to remain private.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=719529