GNOME Bugzilla – Bug 778750
For vaapih264enc without h264parse, using the adaptive 8x8 DCT encoding tool breaks 720p 30 fps clips
Last modified: 2017-03-20 18:05:20 UTC
The following command creates an invalid video clip: gst-launch-1.0 -e videotestsrc num-buffers=60 ! video/x-raw,format=NV12,width=1280,height=720,framerate=30/1 ! vaapih264enc ! mp4mux ! filesink location=test.mp4 When playing with this command: gst-play-1.0 -v --gst-debug=*:4,vaapi*:4 test.mp4 It throws this error: 0:00:00.073091622 1373 0x7ff9b0004850 WARN codecparsers_h264 gsth264parser.c:508:gst_h264_parse_vui_parameters: failed to read uint8, nbits: 1 0:00:00.073109398 1373 0x7ff9b0004850 WARN codecparsers_h264 gsth264parser.c:522:gst_h264_parse_vui_parameters: error parsing "VUI Parameters" 0:00:00.073120611 1373 0x7ff9b0004850 WARN codecparsers_h264 gsth264parser.c:1776:gst_h264_parse_sps: error parsing "Sequence parameter set" 0:00:00.073133640 1373 0x7ff9b0004850 ERROR vaapidecode gstvaapidecode.c:1094:gst_vaapidecode_parse_frame: parse error 8 Also running the clip through the Intel Video Pro Analyzer shows that there are decoding issues with the Sequence Parameter Set and Picture Parameter Set. This problem relates to the usage of the adaptive 8x8 DCT encoding tool and was introduced when the flag for that feature was turned on: 4aec5bd encoder: h264: Use high profile by default It is very specific to the 720p resolution and the 30 fps framerarate. When testing VGA, 1080p and 4k, they all work. Also changing the framerate to e.g. 20 or 35 fps makes it work. A workaround is to add an h264parse element to the capture command: gst-launch-1.0 -e videotestsrc num-buffers=60 ! video/x-raw,format=NV12,width=1280,height=720,framerate=30/1 ! vaapih264enc ! h264parse ! mp4mux ! filesink location=test.mp4 libva info: VA-API version 0.39.4 libva info: va_getDriverName() returns 0 libva info: Trying to open /usr/lib/dri/i965_drv_video.so libva info: Found init function __vaDriverInit_0_39 libva info: va_openDriver() returns 0 vainfo: VA-API version: 0.39 (libva 1.7.3) vainfo: Driver version: Intel i965 driver for Intel(R) Broxton - 1.8.0.pre1 (1.7.3-287-g05d2d25)
As I mentioned in the mail, "adding h264parse" is not a workaround. I always prefer to have the h264parse after the encoder. But it is good to check why the pipeline with out parser is failing.
Created attachment 347810 [details] [review] libs: encoder: h264: apply emulation prevention bytes Applys EPB when providing codec data. Otherwise produced h264 might be broken when decoder/parser, considering EPB, starts processing. ----- I refered to libav and x264enc. Tested on gstreamer and vlc. I think it needs to double-check. @Soren Friis, Could you confirm this patch? again?:)
Thanks. I can confirm that this patch fixes the problem that I have been seeing. I unfortunately cannot comment too much on the internal mechanics of this fix. I.e. whether this is the correct way to fix it or not. You should probably get someone else to review as well.
Comment on attachment 347810 [details] [review] libs: encoder: h264: apply emulation prevention bytes A couple comments about this patch: 1. I would document in the commit log what is the emulation_prevention_three_bytes with the information in the H264 spec (7.3.1 NAL unit syntax and 7.4.1 NAL unit semantics) 2. This patch looks like a copy of what ffmpeg does here [1]. The license is LGPL so we can reuse that code, but we have to honour the original author: Mark Thompson <sw@jkqxz.net> [2] 3. AFAIK H265 also uses this emulation prevention technique, so we should refactor this code as a shared function. 4. My guts tell me that it could be a better way to add this emulation prevention code, aligned with the gstvaapi code style. 1. http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/vaapi_encode_h26x.c;h=d806f9b52b354fdf55070f3d0acc44c79948df94;hb=HEAD#l23 2. http://git.videolan.org/?p=ffmpeg.git;a=commit;h=2c62fcdf5d617791a653d7957d449f75569eede0
Created attachment 348150 [details] [review] libs: encoder: h264/5: fix wrong return value
Created attachment 348151 [details] [review] libs: utils: h26x: creates vaapiutils_h26x Since there are duplicated code in h264/265 encoder, we could refactor these code to avoid duplicated code.
Created attachment 348152 [details] [review] libs: utils: h26x: implements gst_vaapi_utils_h26x_write_nal_unit Implements gst_vaapi_utils_h26x_write_nal_unit, which does write nal unit's length and data to bitwriter. Note that this applies EPB(Emulation Prevention Bytes). Otherwise produced codec_data might be broken when decoder/parser considering EPB, starts parsing. See also Annex B of H264, which describes EPB.
Review of attachment 348152 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiutils_h26x.c @@ +78,3 @@ + } else { + if ((src[sp] & ~3) == 0) { + // emulation_prevention_byte: 0x03 can you change this use the old C comment style? just for homogeneity.
Review of attachment 348150 [details] [review]: lgtm
Review of attachment 348151 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiutils_h26x.c @@ +2,3 @@ + * gstvaapiutils_h26x.c - H.26x related utilities + * + * Copyright (C) 2017 Intel Corporation Add the Author tag, keeping the original authors of the code that you moved here.
Created attachment 348157 [details] [review] libs: utils: h26x: implements gst_vaapi_utils_h26x_write_nal_unit Implements gst_vaapi_utils_h26x_write_nal_unit, which does write nal unit's length and data to bitwriter. Note that this applies EPB(Emulation Prevention Bytes). Otherwise produced codec_data might be broken when decoder/parser considering EPB, starts parsing. See also Annex B of H264, which describes EPB.
I modified according to your sugestion in the third patch together.
Comment on attachment 348150 [details] [review] libs: encoder: h264/5: fix wrong return value Attachment 348150 [details] pushed as 257cfb6 - libs: encoder: h264/5: fix wrong return value
commit 9ed6ac1f763589dd3f77fdccfe6b3487c3a836ba Author: Hyunjun Ko <zzoon@igalia.com> Date: Fri Mar 17 17:14:01 2017 +0900 libs: h26x: adds gst_vaapi_utils_h26x_write_nal_unit() Implements gst_vaapi_utils_h26x_write_nal_unit(), which writes NAL unit length and data to a bitwriter. Note that this helper function applies EPB (Emulation Prevention Bytes), since otherwise produced codec_data might be broken when decoder/parser considering EPB, starts parsing. See sections 7.3 and 7.4 of the H264 and H264 specifications, which describes the emulation_prevention_three_byte. https://bugzilla.gnome.org/show_bug.cgi?id=778750 Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com> commit 49b370ed600c713f0c309bf6091994393af2a6bd Author: Hyunjun Ko <zzoon@igalia.com> Date: Fri Mar 17 16:49:41 2017 +0900 libs: utils: h26x: create vaapiutils_h26x Since there is duplicated code in h264/265 encoder, we could refactor it to avoid duplicated code. https://bugzilla.gnome.org/show_bug.cgi?id=778750 Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>