| Message ID | 20190326194651.7121-2-zhong.li@intel.com |
|---|---|
| State | New |
| Headers | show |
On 26/03/2019 19:46, Zhong Li wrote: > cbs trace qsv vps header failed due to some reasons: > 1. vps_temporal_id_nesting_flag is not set but spec required it must to > be 1 when vps_max_sub_layers_minus1 is equal to 0. > 2. vps_num_hrd_parameters is not set and written. > 3. other issues in ff_hevc_encode_nal_vps() (have fixed in pervious commit). > > Reproduce: ffmpeg -hwaccel qsv -v verbose -c:v h264_qsv -i bbb_sunflower_1080p_30fps_normal.mp4 -vframes 1 > -c:v hevc_qsv -bsf:v trace_headers -f null - > > Signed-off-by: Zhong Li <zhong.li@intel.com> > --- > libavcodec/qsvenc_hevc.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) I had a patch sitting around for a long time for this - see <https://lists.libav.org/pipermail/libav-devel/2017-December/085498.html>. It just deletes all of the ad-hoc writing code and uses the tested CBS paths instead, which have the additional advantage of correctly preserving various things which the existing code doesn't cover at all (e.g. newer PTL flags which didn't exist when this was written). The main problem with it was that I didn't have enough libmfx platforms at the time to be confident in testing of it, so it got stalled. Would it be useful to resurrect that? - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Thursday, March 28, 2019 6:06 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavc/qsvenc: fix hevc vps extradata > issues > > On 26/03/2019 19:46, Zhong Li wrote: > > cbs trace qsv vps header failed due to some reasons: > > 1. vps_temporal_id_nesting_flag is not set but spec required it must to > > be 1 when vps_max_sub_layers_minus1 is equal to 0. > > 2. vps_num_hrd_parameters is not set and written. > > 3. other issues in ff_hevc_encode_nal_vps() (have fixed in pervious > commit). > > > > Reproduce: ffmpeg -hwaccel qsv -v verbose -c:v h264_qsv -i > > bbb_sunflower_1080p_30fps_normal.mp4 -vframes 1 -c:v hevc_qsv > -bsf:v > > trace_headers -f null - > > > > Signed-off-by: Zhong Li <zhong.li@intel.com> > > --- > > libavcodec/qsvenc_hevc.c | 16 +++++++++------- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > I had a patch sitting around for a long time for this - see > <https://lists.libav.org/pipermail/libav-devel/2017-December/085498.html>. > It just deletes all of the ad-hoc writing code and uses the tested CBS paths > instead, which have the additional advantage of correctly preserving various > things which the existing code doesn't cover at all (e.g. newer PTL flags > which didn't exist when this was written). > > The main problem with it was that I didn't have enough libmfx platforms at > the time to be confident in testing of it, so it got stalled. > > Would it be useful to resurrect that? > > - Mark MSDK can support vps extra_data since V1.17. Actually now it have been moved to V1.29. So maybe we can consider the lightweight way to fix the fake vps error issue, instead of introduce cbs. Using cbs can be a good idea if we decided to delete ff_hevc_encode_nal_vps() totally (IHMO, qsv hevc encoder is the only user of ff_hevc_encode_nal_vps()).
diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c index 3d7e9cfba4..edc9a8ab54 100644 --- a/libavcodec/qsvenc_hevc.c +++ b/libavcodec/qsvenc_hevc.c @@ -107,6 +107,7 @@ static int generate_fake_vps(QSVEncContext *q, AVCodecContext *avctx) /* generate the VPS */ vps.vps_max_layers = 1; vps.vps_max_sub_layers = sps.max_sub_layers; + vps.vps_temporal_id_nesting_flag = sps.temporal_id_nesting_flag; memcpy(&vps.ptl, &sps.ptl, sizeof(vps.ptl)); vps.vps_sub_layer_ordering_info_present_flag = 1; for (i = 0; i < HEVC_MAX_SUB_LAYERS; i++) { @@ -120,7 +121,8 @@ static int generate_fake_vps(QSVEncContext *q, AVCodecContext *avctx) vps.vps_num_units_in_tick = sps.vui.vui_num_units_in_tick; vps.vps_time_scale = sps.vui.vui_time_scale; vps.vps_poc_proportional_to_timing_flag = sps.vui.vui_poc_proportional_to_timing_flag; - vps.vps_num_ticks_poc_diff_one = sps.vui.vui_num_ticks_poc_diff_one_minus1 + 1; + vps.vps_num_ticks_poc_diff_one = sps.vui.vui_num_ticks_poc_diff_one_minus1 + 1; + vps.vps_num_hrd_parameters = 0; /* generate the encoded RBSP form of the VPS */ ret = ff_hevc_encode_nal_vps(&vps, sps.vps_id, vps_rbsp_buf, sizeof(vps_rbsp_buf)); @@ -138,12 +140,12 @@ static int generate_fake_vps(QSVEncContext *q, AVCodecContext *avctx) bytestream2_put_byte(&pbc, 1); // header while (bytestream2_get_bytes_left(&gbc)) { - uint32_t b = bytestream2_peek_be24(&gbc); - if (b <= 3) { - bytestream2_put_be24(&pbc, 3); - bytestream2_skip(&gbc, 2); - } else - bytestream2_put_byte(&pbc, bytestream2_get_byte(&gbc)); + if (bytestream2_get_bytes_left(&gbc) >= 3 && + bytestream2_peek_be24(&gbc) <= 3) { + bytestream2_put_be24(&pbc, 3); + bytestream2_skip(&gbc, 2); + } else + bytestream2_put_byte(&pbc, bytestream2_get_byte(&gbc)); } vps_size = bytestream2_tell_p(&pbc);
cbs trace qsv vps header failed due to some reasons: 1. vps_temporal_id_nesting_flag is not set but spec required it must to be 1 when vps_max_sub_layers_minus1 is equal to 0. 2. vps_num_hrd_parameters is not set and written. 3. other issues in ff_hevc_encode_nal_vps() (have fixed in pervious commit). Reproduce: ffmpeg -hwaccel qsv -v verbose -c:v h264_qsv -i bbb_sunflower_1080p_30fps_normal.mp4 -vframes 1 -c:v hevc_qsv -bsf:v trace_headers -f null - Signed-off-by: Zhong Li <zhong.li@intel.com> --- libavcodec/qsvenc_hevc.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)