On 18/01/17 07:51, wm4 wrote:
> On Tue, 17 Jan 2017 22:49:18 +0000
> Mark Thompson <[email protected]> wrote:
>
>> For use by codec implementations which require a device to operate
>> but will allocate frames internally.
>> ---
>> See
>> <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-January/205883.html>
>> for a possible use of it (which I can bring here if people want - that was
>> written mostly to supersede a submitted patch trying to achieve part of the
>> same result by horrible means, but it pushed me into actually doing this
>> bit...).
>>
>>
>> doc/APIchanges | 3 +++
>> libavcodec/avcodec.h | 38 ++++++++++++++++++++++++++++++++++++--
>> libavcodec/utils.c | 1 +
>> libavcodec/version.h | 2 +-
>> 4 files changed, 41 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index c8c2a219f..23885ca65 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil: 2015-08-28
>>
>> API changes, most recent first:
>>
>> +2016-xx-xx - xxxxxxx - lavc 57.32.0 - avcodec.h
>> + Add AVCodecContext.hw_device_ctx.
>> +
>> 2016-xx-xx - xxxxxxx - lavc 57.31.0 - avcodec.h
>> Add AVCodecContext.apply_cropping to control whether cropping
>> is handled by libavcodec or the caller.
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 18721561d..c52239c21 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -3092,8 +3092,13 @@ typedef struct AVCodecContext {
>>
>> /**
>> * A reference to the AVHWFramesContext describing the input (for
>> encoding)
>> - * or output (decoding) frames. The reference is set by the caller and
>> - * afterwards owned (and freed) by libavcodec.
>> + * or output (decoding) frames which will be used by a hardware codec.
>> + *
>> + * It can be set by the user if they wish to supply the frames used, or
>> it
>> + * can be set automatically by libavcodec for decoding when get_format()
>> + * indicates that it should output hardware frames.
>> + *
>> + * If set by the user:
>> *
>> * - decoding: This field should be set by the caller from the
>> get_format()
>> * callback. The previous reference (if any) will always be
>> @@ -3110,6 +3115,20 @@ typedef struct AVCodecContext {
>> * AVCodecContext.pix_fmt.
>> *
>> * This field should be set before avcodec_open2() is
>> called.
>> + *
>> + * If set by libavcodec:
>> + *
>> + * - decoding: The hw_device_ctx field should be set by the user before
>> + * calling avcodec_open2(). If at some later point a
>> hardware
>> + * pixel format is selected by get_format(), then a new
>> + * AVHWFramesContext will be created by the decoder and put
>> + * in this field.
>> + *
>> + * It will be valid when get_buffer2() is called (which must
>> + * then return a buffer from the given context).
>
> Is it a requirement that a decoder sets hw_frames_ctx if hw_device_ctx
> was set? (Regardless of whether output frames actually use a frames
> ctx.)
I think the decoder must always write it in that case, yes - with the hwframe
context it is using with that buffer, or to NULL if using non-hwframe output
(overwriting any previous value).
Note that the statement that it is valid inside get_buffer2() is strict (I
should probably have written that more clearly) - after that point there is no
more synchronisation, so it can have been overwritten in preparation for a new
get_format() call.
>> + *
>> + * - encoding: Unused. In this case the user must not supply hardware
>> + * frames to the encoder.
>> */
>> AVBufferRef *hw_frames_ctx;
>>
>> @@ -3139,6 +3158,21 @@ typedef struct AVCodecContext {
>> * (with the display dimensions being determined by the crop_* fields).
>> */
>> int apply_cropping;
>> +
>> + /**
>> + * A reference to the AVHWDeviceContext describing the device which will
>> + * be used by a hardware encoder/decoder. The reference is set by the
>> + * caller and afterwards owned (and freed) by libavcodec.
>> + *
>> + * This should only be used if either the codec device does not require
>> + * hardware frames or any that are used are allocated internally by
>> + * libavcodec. If the frames may be supplied by the user, then
>> + * hw_frames_ctx must be used instead.
>> + *
>> + * If a device is to be supplied, this field should be set before
>> + * avcodec_open2() is called.
>> + */
>> + AVBufferRef *hw_device_ctx;
>
> I assume the implication is that some hw decoder wrappers (i.e. not a
> true hwaccels) will use this field directly on opening.
Yes, I'm deliberately trying to cater for that case. (Mainly because it's how
the cuvid decoder (currently in the other tine, proposed here) works, which I
think would be better off using this rather than its current abuse of
hw_frames_ctx.)
> It's a bit odd that true hwaccels are so flexible that they can even
> choose/change the hw decoder implementation on get_format calls, while
> the same is apparently forbidden by API for hw_device_ctx.
>
> We could allow it, but then we'd need to document the difference
> between true hwaccels and hardware decoder wrappers - would be too
> complex?
>
> Maybe add this (not sure if it's good):
>
> If you want to select or change the hardware device on demand in
> the get_format callback, set hw_device_ctx to NULL and set the
> hw_frames_ctx field manually. Not all decoders support this.
>
> (Or am I making everything too complicated again.)
We can go further than that - changing the device only is fine, you don't need
to set hw_frames_ctx.
Maybe something like:
If the same device is to be used throughout the lifetime of a decoder,
this field should be set before avcodec_open2() is called. Some decoders
can only use one device and therefore require this behaviour.
For other decoders, the hw_device_ctx field can be set inside the
get_format() callback to indicate the device to use for decoding a
particular section of the stream. Any previous value in the field must
be unreferenced before being overwritten. This also supports changing
between use of hw_device_ctx and hw_frames_ctx - when doing so, the
other field must always be set to NULL.
(Hopefully that has an implicit "if you aren't a crazy person, use the first
method".)
>> } AVCodecContext;
>>
>> /**
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 2978109a2..1d316bd03 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -801,6 +801,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
>> avctx->nb_coded_side_data = 0;
>>
>> av_buffer_unref(&avctx->hw_frames_ctx);
>> + av_buffer_unref(&avctx->hw_device_ctx);
>
> I find that one odd, but ok.
Well, where else would it be unreffed?
>>
>> if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
>> av_opt_free(avctx->priv_data);
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index df0c01f4c..271bc9d6b 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -28,7 +28,7 @@
>> #include "libavutil/version.h"
>>
>> #define LIBAVCODEC_VERSION_MAJOR 57
>> -#define LIBAVCODEC_VERSION_MINOR 31
>> +#define LIBAVCODEC_VERSION_MINOR 32
>> #define LIBAVCODEC_VERSION_MICRO 0
>>
>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
