On 30/01/17 09:09, wm4 wrote:
> On Sun, 29 Jan 2017 19:56:39 +0000
> Mark Thompson <[email protected]> wrote:
> 
>> For use by codec implementations which can allocate frames internally.
>> ---
>>  doc/APIchanges       |  3 +++
>>  libavcodec/avcodec.h | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>>  libavcodec/utils.c   |  1 +
>>  libavcodec/version.h |  2 +-
>>  4 files changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index c8c2a219f..59d7549d1 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:     2015-08-28
>>  
>>  API changes, most recent first:
>>  
>> +2017-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..e7f9ec711 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -3092,8 +3092,14 @@ 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 (for 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 supplied by the
>> +     * user, the reference is thereafter owned (and freed) by libavcodec.
>> +     *
>> +     * 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 +3116,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 can be read only inside the get_buffer2() callback
>> +     *             (which must return a buffer from the given context).
> 
> Any reason for this specific restriction?
> 
> Shouldn't it be ok to access it any time - just that it might not be
> set to something useful? (Could be NULL or wrong format.)

If the decoder is single-threaded, yes.  If it isn't, then I think that inside 
get_buffer2() is the only place where you can guarantee there isn't a data race 
(which is undefined behaviour in C11).

>> +     *
>> +     * - encoding: Unused.  In this case the user must not supply hardware
>> +     *             frames to the encoder.
>>       */
>>      AVBufferRef *hw_frames_ctx;
>>  
>> @@ -3139,6 +3159,29 @@ 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.
> 
> I'm not sure about the word "may". For some decoders, the user can
> either set hw_device_ctx, or he can set a hw_frames_ctx (but they
> can't both be set). Some decoders support hw_frames_ctx only. Shouldn't
> it say "have to" instead of "may"? Maybe the wording is fine - I'm not a
> native speaker.

It's trying to say "if you ever want to supply the frames, you need to use 
hw_frames_ctx" - that is, even if you only want to do it some of the time you 
still need to make the frames context all the time.

  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 user ever intends to supply any of the frames used
  for decoding then hw_frames_ctx must be used instead.

>> +     *
>> +     * 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.

Should mention encoders here too:

  For encoders, this field must be set before avcodec_open2() is called.
  (Encoders can only use a single device.)

>> +     */
>> +    AVBufferRef *hw_device_ctx;
>>  } 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);
>>  
>>      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, \
> 
> Otherwise LGTM.

Thanks,

- Mark

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to