On Tue, 31 Jan 2017 00:08:18 +0000 Mark Thompson <[email protected]> wrote:
> In this case, the user only supplies a device and the frame context > is allocated internally by lavc. > --- > On 30/01/17 12:48, wm4 wrote: > > On Mon, 30 Jan 2017 10:58:07 +0000 > > Mark Thompson <[email protected]> wrote: > > > >> On 30/01/17 09:32, wm4 wrote: > >>> Last but not least, I'm wondering if it would be possible to not unref > >>> and recreate the hw frames pool if the surface format/size doesn't > >>> change. This would be useful for e.g. seeks. Seeks imply a > >>> avcodec_flush_buffers() call, which destroys the hwdec state and causes > >>> get_format() to be called again. > >> > >> The API requires us to call get_format(), and that implies that we have to > >> clear AVCodecContext.hw_frames_ctx (for current users, that API feature is > >> already fixed). > >> > >> So, this code could cache the frames context that it allocated last time > >> and then reuse it if the user offers the same device and the parameters > >> are identical? I think that would work, but it would also transiently > >> allocate more memory than it otherwise would when things change - is that > >> ok? > >> > > > > Isn't the user disallowed to touch hw_frames_ctx if hw_device_ctx is > > set in the AVCodecContext? Which would mean the vaapi-specific init > > function could unref it before allocating a new frames context. > > > > On the other hand, it might be possible that most potential users of > > this feature will be fine with inefficient seeks. They can always go > > further by implementing it manually by setting get_format if more > > optimization is required. (With the main value of this new API being > > ease of entry, instead of having to implement semi-obscure callbacks.) > > How about this? We save a reference to the frames context and then reuse it > if the parameters match. (Not well tested, and not in the seek case: there > might be some obvious error here. I will test it more if people like the > idea.) > > + // Find the first format in the list which matches the expected > + // bit depth and subsampling. If none are found (this can happen > + // when 10-bit streams are decoded to 8-bit surfaces, for example) > + // then just take the first format on the list. > + ctx->surface_format = constraints->valid_sw_formats[0]; > + sw_desc = av_pix_fmt_desc_get(avctx->sw_pix_fmt); > + for (i = 0; constraints->valid_sw_formats[i] != AV_PIX_FMT_NONE; i++) { > + desc = av_pix_fmt_desc_get(constraints->valid_sw_formats[i]); > + if (desc->nb_components != sw_desc->nb_components || > + desc->log2_chroma_w != sw_desc->log2_chroma_w || > + desc->log2_chroma_h != sw_desc->log2_chroma_h) > + continue; > + for (j = 0; j < desc->nb_components; j++) { > + if (desc->comp[j].depth != sw_desc->comp[j].depth) > + break; > } > + if (j < desc->nb_components) > + continue; > + ctx->surface_format = constraints->valid_sw_formats[i]; > + break; > + } This just came to my mind: could avcodec_find_best_pix_fmt2() be used to select the best format? Though it's a very tricky function and could possibly yield bad results in corner cases. > + if (!avctx->hw_frames_ctx) { > + int reuse_saved_frames = 0; > + > + if (ctx->saved_frames_ref) { > + ctx->frames = (AVHWFramesContext*)ctx->saved_frames_ref->data; > + > + if (ctx->frames->width == avctx->coded_width && > + ctx->frames->height == avctx->coded_height && > + ctx->frames->sw_format == ctx->surface_format && > + ctx->frames->initial_pool_size == ctx->surface_count) { > + reuse_saved_frames = 1; > + } > + } > + > + if (reuse_saved_frames) { > + avctx->hw_frames_ctx = av_buffer_ref(ctx->saved_frames_ref); > + if (!avctx->hw_frames_ctx) { > + err = AVERROR(ENOMEM); > + goto fail; > + } > + > + ctx->hwfc = ctx->frames->hwctx; > + } else { > + av_buffer_unref(&ctx->saved_frames_ref); > + > + avctx->hw_frames_ctx = > av_hwframe_ctx_alloc(avctx->hw_device_ctx); > + ctx->frames = (AVHWFramesContext*)avctx->hw_frames_ctx->data; > + > + ctx->frames->format = AV_PIX_FMT_VAAPI; > + ctx->frames->width = avctx->coded_width; > + ctx->frames->height = avctx->coded_height; > + > + ctx->frames->sw_format = ctx->surface_format; > + ctx->frames->initial_pool_size = ctx->surface_count; > + > + err = av_hwframe_ctx_init(avctx->hw_frames_ctx); > + if (err < 0) { > + av_log(avctx, AV_LOG_ERROR, "Failed to initialise internal " > + "frames context: %d.\n", err); > + goto fail; > + } > + > + ctx->hwfc = ctx->frames->hwctx; > + > + ctx->saved_frames_ref = av_buffer_ref(avctx->hw_frames_ctx); > + if (!ctx->saved_frames_ref) { > + err = AVERROR(ENOMEM); > + goto fail; > + } > + } > + } else { > + av_buffer_unref(&ctx->saved_frames_ref); > + } > + > vas = vaCreateContext(ctx->hwctx->display, ctx->va_config, > avctx->coded_width, avctx->coded_height, > VA_PROGRESSIVE, > @@ -540,6 +671,8 @@ int ff_vaapi_decode_uninit(AVCodecContext *avctx) > } > } > > + av_buffer_unref(&ctx->saved_frames_ref); > + > #if FF_API_VAAPI_CONTEXT > } > #endif > diff --git a/libavcodec/vaapi_decode.h b/libavcodec/vaapi_decode.h > index 08b212d03..1211c3ff3 100644 > --- a/libavcodec/vaapi_decode.h > +++ b/libavcodec/vaapi_decode.h > @@ -69,6 +69,11 @@ typedef struct VAAPIDecodeContext { > > AVHWFramesContext *frames; > AVVAAPIFramesContext *hwfc; > + > + enum AVPixelFormat surface_format; > + int surface_count; > + > + AVBufferRef *saved_frames_ref; > } VAAPIDecodeContext; > > The frame saving stuff is approximately what I thought it should be. But yeah, it won't work, because ff_get_format() calls ff_vaapi_decode_uninit() first thing. saved_frames_ref could be a field in AVCodecInternal instead. It would also need to be released if ff_get_format() determines software fallback, and you'd need to check the hwcontext type and whether it's the same device before reuse. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
