| Message ID | 20211130084406.758250-1-wenbin.chen@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,V2,1/3] libavcodec/vaapi_decode: fix the problem that init_pool_size < nb_surface | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
Quoting Wenbin Chen (2021-11-30 09:44:04) > For vaapi if the init_pool_size is not zero, the pool size is fixed. > This means max surfaces is init_pool_size, but when mapping vaapi > frame to qsv frame, the init_pool_size < nb_surface. The cause is that > vaapi_decode_make_config() config the init_pool_size and it is called > twice. The first time is to init frame_context and the second time is to > init codec. On the second time the init_pool_size is changed to original > value so the init_pool_size is lower than the reall size because > pool_size used to initialize frame_context need to plus thread_count and > 3 (guarantee 4 base work surfaces). Now add code to make sure > init_pool_size is only set once. Now the following commandline works: > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 \ > -hwaccel_output_format vaapi -i input.264 \ > -vf "hwmap=derive_device=qsv,format=qsv" \ > -c:v h264_qsv output.264 > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> > --- > libavcodec/vaapi_decode.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c > index 665af370ed..aab8162989 100644 > --- a/libavcodec/vaapi_decode.c > +++ b/libavcodec/vaapi_decode.c > @@ -572,22 +572,24 @@ static int vaapi_decode_make_config(AVCodecContext *avctx, > if (err < 0) > goto fail; > > - frames->initial_pool_size = 1; > - // Add per-codec number of surfaces used for storing reference frames. > - switch (avctx->codec_id) { > - case AV_CODEC_ID_H264: > - case AV_CODEC_ID_HEVC: > - case AV_CODEC_ID_AV1: > - frames->initial_pool_size += 16; > - break; > - case AV_CODEC_ID_VP9: > - frames->initial_pool_size += 8; > - break; > - case AV_CODEC_ID_VP8: > - frames->initial_pool_size += 3; > - break; > - default: > - frames->initial_pool_size += 2; > + if (!frames->initial_pool_size) { > + frames->initial_pool_size = 1; > + // Add per-codec number of surfaces used for storing reference frames. > + switch (avctx->codec_id) { > + case AV_CODEC_ID_H264: > + case AV_CODEC_ID_HEVC: > + case AV_CODEC_ID_AV1: > + frames->initial_pool_size += 16; > + break; > + case AV_CODEC_ID_VP9: > + frames->initial_pool_size += 8; > + break; > + case AV_CODEC_ID_VP8: > + frames->initial_pool_size += 3; > + break; > + default: > + frames->initial_pool_size += 2; > + } Seems to me that ff_vaapi_decode_init() should not pass hw_frames_ctx to vaapi_decode_make_config() at all, because it is already initialized and must not be modified.
> Quoting Wenbin Chen (2021-11-30 09:44:04) > > For vaapi if the init_pool_size is not zero, the pool size is fixed. > > This means max surfaces is init_pool_size, but when mapping vaapi > > frame to qsv frame, the init_pool_size < nb_surface. The cause is that > > vaapi_decode_make_config() config the init_pool_size and it is called > > twice. The first time is to init frame_context and the second time is to > > init codec. On the second time the init_pool_size is changed to original > > value so the init_pool_size is lower than the reall size because > > pool_size used to initialize frame_context need to plus thread_count and > > 3 (guarantee 4 base work surfaces). Now add code to make sure > > init_pool_size is only set once. Now the following commandline works: > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 \ > > -hwaccel_output_format vaapi -i input.264 \ > > -vf "hwmap=derive_device=qsv,format=qsv" \ > > -c:v h264_qsv output.264 > > > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> > > --- > > libavcodec/vaapi_decode.c | 34 ++++++++++++++++++---------------- > > 1 file changed, 18 insertions(+), 16 deletions(-) > > > > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c > > index 665af370ed..aab8162989 100644 > > --- a/libavcodec/vaapi_decode.c > > +++ b/libavcodec/vaapi_decode.c > > @@ -572,22 +572,24 @@ static int > vaapi_decode_make_config(AVCodecContext *avctx, > > if (err < 0) > > goto fail; > > > > - frames->initial_pool_size = 1; > > - // Add per-codec number of surfaces used for storing reference > frames. > > - switch (avctx->codec_id) { > > - case AV_CODEC_ID_H264: > > - case AV_CODEC_ID_HEVC: > > - case AV_CODEC_ID_AV1: > > - frames->initial_pool_size += 16; > > - break; > > - case AV_CODEC_ID_VP9: > > - frames->initial_pool_size += 8; > > - break; > > - case AV_CODEC_ID_VP8: > > - frames->initial_pool_size += 3; > > - break; > > - default: > > - frames->initial_pool_size += 2; > > + if (!frames->initial_pool_size) { > > + frames->initial_pool_size = 1; > > + // Add per-codec number of surfaces used for storing reference > frames. > > + switch (avctx->codec_id) { > > + case AV_CODEC_ID_H264: > > + case AV_CODEC_ID_HEVC: > > + case AV_CODEC_ID_AV1: > > + frames->initial_pool_size += 16; > > + break; > > + case AV_CODEC_ID_VP9: > > + frames->initial_pool_size += 8; > > + break; > > + case AV_CODEC_ID_VP8: > > + frames->initial_pool_size += 3; > > + break; > > + default: > > + frames->initial_pool_size += 2; > > + } > > Seems to me that ff_vaapi_decode_init() should not pass hw_frames_ctx to > vaapi_decode_make_config() at all, because it is already initialized and > must not be modified. > > -- > Anton Khirnov You are right. I don't need to add an new check. ff_vaapi_decode_init() should pass NULL to vaapi_decode_make_config. Thanks for your review. I will submit a new patch Regards Wenbin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c index 665af370ed..aab8162989 100644 --- a/libavcodec/vaapi_decode.c +++ b/libavcodec/vaapi_decode.c @@ -572,22 +572,24 @@ static int vaapi_decode_make_config(AVCodecContext *avctx, if (err < 0) goto fail; - frames->initial_pool_size = 1; - // Add per-codec number of surfaces used for storing reference frames. - switch (avctx->codec_id) { - case AV_CODEC_ID_H264: - case AV_CODEC_ID_HEVC: - case AV_CODEC_ID_AV1: - frames->initial_pool_size += 16; - break; - case AV_CODEC_ID_VP9: - frames->initial_pool_size += 8; - break; - case AV_CODEC_ID_VP8: - frames->initial_pool_size += 3; - break; - default: - frames->initial_pool_size += 2; + if (!frames->initial_pool_size) { + frames->initial_pool_size = 1; + // Add per-codec number of surfaces used for storing reference frames. + switch (avctx->codec_id) { + case AV_CODEC_ID_H264: + case AV_CODEC_ID_HEVC: + case AV_CODEC_ID_AV1: + frames->initial_pool_size += 16; + break; + case AV_CODEC_ID_VP9: + frames->initial_pool_size += 8; + break; + case AV_CODEC_ID_VP8: + frames->initial_pool_size += 3; + break; + default: + frames->initial_pool_size += 2; + } } }
For vaapi if the init_pool_size is not zero, the pool size is fixed. This means max surfaces is init_pool_size, but when mapping vaapi frame to qsv frame, the init_pool_size < nb_surface. The cause is that vaapi_decode_make_config() config the init_pool_size and it is called twice. The first time is to init frame_context and the second time is to init codec. On the second time the init_pool_size is changed to original value so the init_pool_size is lower than the reall size because pool_size used to initialize frame_context need to plus thread_count and 3 (guarantee 4 base work surfaces). Now add code to make sure init_pool_size is only set once. Now the following commandline works: ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 \ -hwaccel_output_format vaapi -i input.264 \ -vf "hwmap=derive_device=qsv,format=qsv" \ -c:v h264_qsv output.264 Signed-off-by: Wenbin Chen <wenbin.chen@intel.com> --- libavcodec/vaapi_decode.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)