| Message ID | 20171211123458.GG470@tsuri.lan |
|---|---|
| State | Superseded |
| Headers | show |
On 11/12/17 12:34, Matthieu Bouron wrote: >> >> New patch attached fixing errors in get_format() by keeping the original >> AVCodecHWConfigInternal (ad-hoc) and adding a new one (hw-device) with the >> device_type field set to the MediaCodec. >> >> The updated patchset works without errors with both the old hwaccel method and >> the new device_ctx method. > > From 2bbdae2141ba8ca8db54175f4440ac8190f5953d Mon Sep 17 00:00:00 2001 > From: Aman Gupta <aman@tmm1.net> > Date: Sun, 3 Dec 2017 17:32:22 -0800 > Subject: [PATCH 2/2] libavcodec/mediacodec: use AVMediaCodecDeviceContext > hw_device_ctx if set > > Signed-off-by: Matthieu Bouron <matthieu.bouron@gmail.com> > --- > libavcodec/mediacodecdec.c | 8 ++++++++ > libavcodec/mediacodecdec_common.c | 14 +++++++++++++- > 2 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c > index 86cc629430..c8ad0b80e7 100644 > --- a/libavcodec/mediacodecdec.c > +++ b/libavcodec/mediacodecdec.c > @@ -543,6 +543,14 @@ static const AVCodecHWConfigInternal *mediacodec_hw_configs[] = { > }, > .hwaccel = NULL, > }, > + &(const AVCodecHWConfigInternal) { > + .public = { > + .pix_fmt = AV_PIX_FMT_MEDIACODEC, > + .methods = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX, This wants to be combined with the previous entry (or together the methods): ff_get_format() looks for the first entry matching the pix_fmt, so it will never find this one. I think this might work anyway because AD_HOC is technically "do whatever you like, I can't check it", but it also wouldn't notice e.g. a non-matching device in that case. > + .device_type = AV_HWDEVICE_TYPE_MEDIACODEC, > + }, > + .hwaccel = NULL, > + }, > NULL > }; > > diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c > index cb2f6ae5e5..a9147f3a08 100644 > --- a/libavcodec/mediacodecdec_common.c > +++ b/libavcodec/mediacodecdec_common.c > @@ -24,6 +24,7 @@ > #include <sys/types.h> > > #include "libavutil/common.h" > +#include "libavutil/hwcontext_mediacodec.h" > #include "libavutil/mem.h" > #include "libavutil/log.h" > #include "libavutil/pixfmt.h" > @@ -476,7 +477,18 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s, > if (pix_fmt == AV_PIX_FMT_MEDIACODEC) { > AVMediaCodecContext *user_ctx = avctx->hwaccel_context; > > - if (user_ctx && user_ctx->surface) { > + if (avctx->hw_device_ctx) { > + AVHWDeviceContext *device_ctx = (AVHWDeviceContext*)(avctx->hw_device_ctx->data); > + if (device_ctx->type == AV_HWDEVICE_TYPE_MEDIACODEC) { > + if (device_ctx->hwctx) { > + AVMediaCodecDeviceContext *mediacodec_ctx = (AVMediaCodecDeviceContext *)device_ctx->hwctx; > + s->surface = ff_mediacodec_surface_ref(mediacodec_ctx->surface, avctx); > + av_log(avctx, AV_LOG_INFO, "Using surface %p\n", s->surface); > + } > + } > + } > + > + if (!s->surface && user_ctx && user_ctx->surface) { > s->surface = ff_mediacodec_surface_ref(user_ctx->surface, avctx); > av_log(avctx, AV_LOG_INFO, "Using surface %p\n", s->surface); > } > -- > 2.15.1 > Rest of the set looks fine to me (but I can't test it). Thanks, - Mark
On Mon, 11 Dec 2017 22:10:24 +0000 Mark Thompson <sw@jkqxz.net> wrote: > On 11/12/17 12:34, Matthieu Bouron wrote: > >> > >> New patch attached fixing errors in get_format() by keeping the original > >> AVCodecHWConfigInternal (ad-hoc) and adding a new one (hw-device) with the > >> device_type field set to the MediaCodec. > >> > >> The updated patchset works without errors with both the old hwaccel method and > >> the new device_ctx method. > > > > From 2bbdae2141ba8ca8db54175f4440ac8190f5953d Mon Sep 17 00:00:00 2001 > > From: Aman Gupta <aman@tmm1.net> > > Date: Sun, 3 Dec 2017 17:32:22 -0800 > > Subject: [PATCH 2/2] libavcodec/mediacodec: use AVMediaCodecDeviceContext > > hw_device_ctx if set > > > > Signed-off-by: Matthieu Bouron <matthieu.bouron@gmail.com> > > --- > > libavcodec/mediacodecdec.c | 8 ++++++++ > > libavcodec/mediacodecdec_common.c | 14 +++++++++++++- > > 2 files changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c > > index 86cc629430..c8ad0b80e7 100644 > > --- a/libavcodec/mediacodecdec.c > > +++ b/libavcodec/mediacodecdec.c > > @@ -543,6 +543,14 @@ static const AVCodecHWConfigInternal *mediacodec_hw_configs[] = { > > }, > > .hwaccel = NULL, > > }, > > + &(const AVCodecHWConfigInternal) { > > + .public = { > > + .pix_fmt = AV_PIX_FMT_MEDIACODEC, > > + .methods = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX, > > This wants to be combined with the previous entry (or together the methods): ff_get_format() looks for the first entry matching the pix_fmt, so it will never find this one. I think this might work anyway because AD_HOC is technically "do whatever you like, I can't check it", but it also wouldn't notice e.g. a non-matching device in that case. Hm, I guess I agree. From an API user perspective, each AVCodecHWConfig entry should cover the underlying mechanism fully. So if like in this case, the underlying code/method is the same (mediacodec stuff), then all methods for using it share the same AVCodecHWConfig entry. The only cases where AVCodecs have multiple config entries are hwaccels, where each hwaccel is backed by a completely different method. Don't know how this works if a full stream decoder supports multiple device types, though.
diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c index 86cc629430..c8ad0b80e7 100644 --- a/libavcodec/mediacodecdec.c +++ b/libavcodec/mediacodecdec.c @@ -543,6 +543,14 @@ static const AVCodecHWConfigInternal *mediacodec_hw_configs[] = { }, .hwaccel = NULL, }, + &(const AVCodecHWConfigInternal) { + .public = { + .pix_fmt = AV_PIX_FMT_MEDIACODEC, + .methods = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX, + .device_type = AV_HWDEVICE_TYPE_MEDIACODEC, + }, + .hwaccel = NULL, + }, NULL }; diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c index cb2f6ae5e5..a9147f3a08 100644 --- a/libavcodec/mediacodecdec_common.c +++ b/libavcodec/mediacodecdec_common.c @@ -24,6 +24,7 @@ #include <sys/types.h> #include "libavutil/common.h" +#include "libavutil/hwcontext_mediacodec.h" #include "libavutil/mem.h" #include "libavutil/log.h" #include "libavutil/pixfmt.h" @@ -476,7 +477,18 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s, if (pix_fmt == AV_PIX_FMT_MEDIACODEC) { AVMediaCodecContext *user_ctx = avctx->hwaccel_context; - if (user_ctx && user_ctx->surface) { + if (avctx->hw_device_ctx) { + AVHWDeviceContext *device_ctx = (AVHWDeviceContext*)(avctx->hw_device_ctx->data); + if (device_ctx->type == AV_HWDEVICE_TYPE_MEDIACODEC) { + if (device_ctx->hwctx) { + AVMediaCodecDeviceContext *mediacodec_ctx = (AVMediaCodecDeviceContext *)device_ctx->hwctx; + s->surface = ff_mediacodec_surface_ref(mediacodec_ctx->surface, avctx); + av_log(avctx, AV_LOG_INFO, "Using surface %p\n", s->surface); + } + } + } + + if (!s->surface && user_ctx && user_ctx->surface) { s->surface = ff_mediacodec_surface_ref(user_ctx->surface, avctx); av_log(avctx, AV_LOG_INFO, "Using surface %p\n", s->surface); }