| Message ID | 1540393553-10656-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 24, 2018 at 5:06 PM Jun Zhao <mypopydev@gmail.com> wrote: > > From: Jun Zhao <jun.zhao@intel.com> > > Add checking to avoid calling ff_thread_get_format repeatedly > whenever new slice header decoded. > > Signed-off-by: Lin Xie <lin.xie@intel.com> > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > --- > libavcodec/hevcdec.c | 17 +++++++++++------ > 1 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c > index a3b5c8c..16032a5 100644 > --- a/libavcodec/hevcdec.c > +++ b/libavcodec/hevcdec.c > @@ -317,7 +317,6 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > const HEVCWindow *ow = &sps->output_window; > unsigned int num = 0, den = 0; > > - avctx->pix_fmt = sps->pix_fmt; > avctx->coded_width = sps->width; > avctx->coded_height = sps->height; > avctx->width = sps->width - ow->left_offset - ow->right_offset; > @@ -357,7 +356,7 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > num, den, 1 << 30); > } > > -static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > +static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps, int force_callback) > { > #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL + \ > CONFIG_HEVC_D3D11VA_HWACCEL * 2 + \ > @@ -366,6 +365,8 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > CONFIG_HEVC_VIDEOTOOLBOX_HWACCEL + \ > CONFIG_HEVC_VDPAU_HWACCEL) > enum AVPixelFormat pix_fmts[HWACCEL_MAX + 2], *fmt = pix_fmts; > + const enum AVPixelFormat *choices = pix_fmts; > + int i; > > switch (sps->pix_fmt) { > case AV_PIX_FMT_YUV420P: > @@ -418,6 +419,11 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > *fmt++ = sps->pix_fmt; > *fmt = AV_PIX_FMT_NONE; > > + if (!force_callback) > + for (i = 0; choices[i] != AV_PIX_FMT_NONE; i++) > + if (choices[i] == s->avctx->pix_fmt) > + return choices[i]; > + > return ff_thread_get_format(s->avctx, pix_fmts); > } > > @@ -439,8 +445,6 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps, > > export_stream_params(s->avctx, &s->ps, sps); > > - s->avctx->pix_fmt = pix_fmt; > - > ff_hevc_pred_init(&s->hpc, sps->bit_depth); > ff_hevc_dsp_init (&s->hevcdsp, sps->bit_depth); > ff_videodsp_init (&s->vdsp, sps->bit_depth); > @@ -526,10 +530,11 @@ static int hls_slice_header(HEVCContext *s) > if (ret < 0) > return ret; > > - pix_fmt = get_format(s, sps); > + pix_fmt = get_format(s, sps, s->avctx->hwaccel ? 0 : 1); This seems rather wonky to me. Either there is a format change or there isn't, which should be the deciding factor if get_format is being called, not if a hwaccel is already on or not. - Hendrik
On Wed, Oct 24, 2018 at 5:09 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > On Wed, Oct 24, 2018 at 5:06 PM Jun Zhao <mypopydev@gmail.com> wrote: > > > > From: Jun Zhao <jun.zhao@intel.com> > > > > Add checking to avoid calling ff_thread_get_format repeatedly > > whenever new slice header decoded. > > > > Signed-off-by: Lin Xie <lin.xie@intel.com> > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > --- > > libavcodec/hevcdec.c | 17 +++++++++++------ > > 1 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c > > index a3b5c8c..16032a5 100644 > > --- a/libavcodec/hevcdec.c > > +++ b/libavcodec/hevcdec.c > > @@ -317,7 +317,6 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > > const HEVCWindow *ow = &sps->output_window; > > unsigned int num = 0, den = 0; > > > > - avctx->pix_fmt = sps->pix_fmt; > > avctx->coded_width = sps->width; > > avctx->coded_height = sps->height; > > avctx->width = sps->width - ow->left_offset - ow->right_offset; > > @@ -357,7 +356,7 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > > num, den, 1 << 30); > > } > > > > -static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > +static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps, int force_callback) > > { > > #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL + \ > > CONFIG_HEVC_D3D11VA_HWACCEL * 2 + \ > > @@ -366,6 +365,8 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > CONFIG_HEVC_VIDEOTOOLBOX_HWACCEL + \ > > CONFIG_HEVC_VDPAU_HWACCEL) > > enum AVPixelFormat pix_fmts[HWACCEL_MAX + 2], *fmt = pix_fmts; > > + const enum AVPixelFormat *choices = pix_fmts; > > + int i; > > > > switch (sps->pix_fmt) { > > case AV_PIX_FMT_YUV420P: > > @@ -418,6 +419,11 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > *fmt++ = sps->pix_fmt; > > *fmt = AV_PIX_FMT_NONE; > > > > + if (!force_callback) > > + for (i = 0; choices[i] != AV_PIX_FMT_NONE; i++) > > + if (choices[i] == s->avctx->pix_fmt) > > + return choices[i]; > > + Additionally, there can be a lot more properties that might result in a different hwaccel selection, or even software fallback. Resolution, frame rate, who knows what else. The point of get_format is not only to determine the frame format, but also notify the usercode / hwaccel about any changes in stream properties. If too many calls to get_format cause serious issues, then typically the usercode that implements get_format is written badly. It should verify that the current hwaccel is still capable of decoding the new format, and only then act, instead of always re-creating the hardware decoder. I realize that our avcodec code is also guilty of that, but I've complained about that before, and thats the prime reason I also don't use it. :) - Hendrik
On Wed, Oct 24, 2018 at 11:10 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > On Wed, Oct 24, 2018 at 5:06 PM Jun Zhao <mypopydev@gmail.com> wrote: > > > > From: Jun Zhao <jun.zhao@intel.com> > > > > Add checking to avoid calling ff_thread_get_format repeatedly > > whenever new slice header decoded. > > > > Signed-off-by: Lin Xie <lin.xie@intel.com> > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > --- > > libavcodec/hevcdec.c | 17 +++++++++++------ > > 1 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c > > index a3b5c8c..16032a5 100644 > > --- a/libavcodec/hevcdec.c > > +++ b/libavcodec/hevcdec.c > > @@ -317,7 +317,6 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > > const HEVCWindow *ow = &sps->output_window; > > unsigned int num = 0, den = 0; > > > > - avctx->pix_fmt = sps->pix_fmt; > > avctx->coded_width = sps->width; > > avctx->coded_height = sps->height; > > avctx->width = sps->width - ow->left_offset - ow->right_offset; > > @@ -357,7 +356,7 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, > > num, den, 1 << 30); > > } > > > > -static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > +static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps, int force_callback) > > { > > #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL + \ > > CONFIG_HEVC_D3D11VA_HWACCEL * 2 + \ > > @@ -366,6 +365,8 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > CONFIG_HEVC_VIDEOTOOLBOX_HWACCEL + \ > > CONFIG_HEVC_VDPAU_HWACCEL) > > enum AVPixelFormat pix_fmts[HWACCEL_MAX + 2], *fmt = pix_fmts; > > + const enum AVPixelFormat *choices = pix_fmts; > > + int i; > > > > switch (sps->pix_fmt) { > > case AV_PIX_FMT_YUV420P: > > @@ -418,6 +419,11 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) > > *fmt++ = sps->pix_fmt; > > *fmt = AV_PIX_FMT_NONE; > > > > + if (!force_callback) > > + for (i = 0; choices[i] != AV_PIX_FMT_NONE; i++) > > + if (choices[i] == s->avctx->pix_fmt) > > + return choices[i]; > > + > > return ff_thread_get_format(s->avctx, pix_fmts); > > } > > > > @@ -439,8 +445,6 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps, > > > > export_stream_params(s->avctx, &s->ps, sps); > > > > - s->avctx->pix_fmt = pix_fmt; > > - > > ff_hevc_pred_init(&s->hpc, sps->bit_depth); > > ff_hevc_dsp_init (&s->hevcdsp, sps->bit_depth); > > ff_videodsp_init (&s->vdsp, sps->bit_depth); > > @@ -526,10 +530,11 @@ static int hls_slice_header(HEVCContext *s) > > if (ret < 0) > > return ret; > > > > - pix_fmt = get_format(s, sps); > > + pix_fmt = get_format(s, sps, s->avctx->hwaccel ? 0 : 1); > > This seems rather wonky to me. Either there is a format change or > there isn't, which should be the deciding factor if get_format is > being called, not if a hwaccel is already on or not. > I know the concern now, in fact, this patch wants to reduce the call number for hwaccel_uninit/hwaccel_init (in ff_get_format) again and again , so use the hwaccel as flag in this case. Maybe we need to found a more suitable in this case.
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c index a3b5c8c..16032a5 100644 --- a/libavcodec/hevcdec.c +++ b/libavcodec/hevcdec.c @@ -317,7 +317,6 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, const HEVCWindow *ow = &sps->output_window; unsigned int num = 0, den = 0; - avctx->pix_fmt = sps->pix_fmt; avctx->coded_width = sps->width; avctx->coded_height = sps->height; avctx->width = sps->width - ow->left_offset - ow->right_offset; @@ -357,7 +356,7 @@ static void export_stream_params(AVCodecContext *avctx, const HEVCParamSets *ps, num, den, 1 << 30); } -static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) +static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps, int force_callback) { #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL + \ CONFIG_HEVC_D3D11VA_HWACCEL * 2 + \ @@ -366,6 +365,8 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) CONFIG_HEVC_VIDEOTOOLBOX_HWACCEL + \ CONFIG_HEVC_VDPAU_HWACCEL) enum AVPixelFormat pix_fmts[HWACCEL_MAX + 2], *fmt = pix_fmts; + const enum AVPixelFormat *choices = pix_fmts; + int i; switch (sps->pix_fmt) { case AV_PIX_FMT_YUV420P: @@ -418,6 +419,11 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) *fmt++ = sps->pix_fmt; *fmt = AV_PIX_FMT_NONE; + if (!force_callback) + for (i = 0; choices[i] != AV_PIX_FMT_NONE; i++) + if (choices[i] == s->avctx->pix_fmt) + return choices[i]; + return ff_thread_get_format(s->avctx, pix_fmts); } @@ -439,8 +445,6 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps, export_stream_params(s->avctx, &s->ps, sps); - s->avctx->pix_fmt = pix_fmt; - ff_hevc_pred_init(&s->hpc, sps->bit_depth); ff_hevc_dsp_init (&s->hevcdsp, sps->bit_depth); ff_videodsp_init (&s->vdsp, sps->bit_depth); @@ -526,10 +530,11 @@ static int hls_slice_header(HEVCContext *s) if (ret < 0) return ret; - pix_fmt = get_format(s, sps); + pix_fmt = get_format(s, sps, s->avctx->hwaccel ? 0 : 1); if (pix_fmt < 0) return pix_fmt; - s->avctx->pix_fmt = pix_fmt; + if (s->avctx->pix_fmt != pix_fmt) + s->avctx->pix_fmt = pix_fmt; s->seq_decode = (s->seq_decode + 1) & 0xff; s->max_ra = INT_MAX;