On Sun, May 10, 2015 at 9:46 AM, Hendrik Leppkes <[email protected]> wrote:
> On Sun, May 10, 2015 at 2:46 AM, Luca Barbato <[email protected]> wrote:
>> From: Stefano Pigozzi <[email protected]>
>>
>> Signed-off-by: Luca Barbato <[email protected]>
>> ---
>>
>> Will be pushed tomorrow if nobody is against it.
>>
>>  doc/APIchanges        |  3 +++
>>  libavcodec/vda.c      | 27 ++++++++++++++++++++++++++-
>>  libavcodec/vda.h      | 18 ++++++++++++++++++
>>  libavcodec/vda_h264.c |  5 ++++-
>>  libavcodec/version.h  |  4 ++--
>>  5 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 5934f45..5d39ec6 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:     2014-08-09
>>
>>  API changes, most recent first:
>>
>> +2015-xx-xx - xxxxxxx - lavc 56.23.0
>> +  Add av_vda_default_init2.
>> +
>>  2015-xx-xx - xxxxxxx - lavu 54.12.0
>>    Add AV_LOG_TRACE for extremely verbose debugging.
>>
>> diff --git a/libavcodec/vda.c b/libavcodec/vda.c
>> index f71fb16..a1a14de 100644
>> --- a/libavcodec/vda.c
>> +++ b/libavcodec/vda.c
>> @@ -19,14 +19,34 @@
>>  #include "config.h"
>>
>>  #include "libavutil/mem.h"
>> +#include "libavutil/opt.h"
>>
>>  #include "vda.h"
>>  #include "vda_internal.h"
>>
>> +#define OFFSET(x) offsetof(AVVDAContext, x)
>> +#define DEC AV_OPT_FLAG_DECODING_PARAM
>> +static const AVOption options[] = {
>> +    { "cv_pix_fmt_type", "CVPixelBuffer Format Type of the decoded frames",
>> +          OFFSET(cv_pix_fmt_type), AV_OPT_TYPE_INT64,
>> +          {.i64 = (uint64_t) kCVPixelFormatType_422YpCbCr8},
>> +          INT64_MIN, INT64_MAX, DEC },
>> +    { NULL },
>> +};
>> +
>> +static const AVClass vda_context_class = {
>> +    .class_name = "VDA context",
>> +    .item_name  = av_default_item_name,
>> +    .option     = options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>>  #if CONFIG_H264_VDA_HWACCEL
>>  AVVDAContext *av_vda_alloc_context(void)
>>  {
>>      AVVDAContext *ret = av_mallocz(sizeof(*ret));
>> +    ret->class = &vda_context_class;
>> +    av_opt_set_defaults(ret);
>>
>>      if (ret)
>>          ret->output_callback = ff_vda_output_callback;
>> @@ -36,7 +56,12 @@ AVVDAContext *av_vda_alloc_context(void)
>>
>>  int av_vda_default_init(AVCodecContext *avctx)
>>  {
>> -    avctx->hwaccel_context = av_vda_alloc_context();
>> +    return av_vda_default_init2(avctx, NULL);
>> +}
>> +
>> +int av_vda_default_init2(AVCodecContext *avctx, AVVDAContext *vdactx)
>> +{
>> +    avctx->hwaccel_context = vdactx ?: av_vda_alloc_context();
>>      if (!avctx->hwaccel_context)
>>          return AVERROR(ENOMEM);
>>      return ff_vda_default_init(avctx);
>> diff --git a/libavcodec/vda.h b/libavcodec/vda.h
>> index 9aa5d29..82f45dd 100644
>> --- a/libavcodec/vda.h
>> +++ b/libavcodec/vda.h
>> @@ -143,6 +143,13 @@ int ff_vda_destroy_decoder(struct vda_context *vda_ctx);
>>   * av_vda_alloc_context() and freed with av_free().
>>   */
>>  typedef struct AVVDAContext {
>> +    // Class for private options
>> +    const AVClass *class;
>> +
>> +    // CVPixelBuffer Format Type that VDA will use for decoded frames; set 
>> by
>> +    // a private option
>> +    OSType cv_pix_fmt_type;
>> +
>>      /**
>>       * VDA decoder object. Created and freed by the caller.
>>       */
>> @@ -181,6 +188,17 @@ AVVDAContext *av_vda_alloc_context(void);
>>  int av_vda_default_init(AVCodecContext *avctx);
>>
>>  /**
>> + * This is a convenience function that sets up the VDA context using an
>> + * internal implementation.
>> + *
>> + * @param avctx the corresponding codec context
>> + * @param vdactx the VDA context to use
>> + *
>> + * @return >= 0 on success, a negative AVERROR code on failure
>> + */
>> +int av_vda_default_init2(AVCodecContext *avctx, AVVDAContext *vdactx);
>> +
>> +/**
>>   * This function must be called to free the VDA context initialized with
>>   * av_vda_default_init().
>>   *
>> diff --git a/libavcodec/vda_h264.c b/libavcodec/vda_h264.c
>> index 3c0775b..fb7a9255 100644
>> --- a/libavcodec/vda_h264.c
>> +++ b/libavcodec/vda_h264.c
>> @@ -24,6 +24,7 @@
>>  #include <CoreFoundation/CFData.h>
>>  #include <CoreFoundation/CFString.h>
>>
>> +#include "libavutil/opt.h"
>>  #include "libavutil/avutil.h"
>>  #include "h264.h"
>>  #include "internal.h"
>> @@ -380,7 +381,9 @@ int ff_vda_default_init(AVCodecContext *avctx)
>>      CFMutableDictionaryRef buffer_attributes;
>>      CFMutableDictionaryRef io_surface_properties;
>>      CFNumberRef cv_pix_fmt;
>> -    int32_t fmt = 'avc1', pix_fmt = kCVPixelFormatType_422YpCbCr8;
>> +    int32_t fmt = 'avc1';
>> +    int64_t pix_fmt;
>> +    av_opt_get_int(vda_ctx, "cv_pix_fmt_type", 0, &pix_fmt);
>>
>>      // kCVPixelFormatType_420YpCbCr8Planar;
>>
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index 50c2b48..c478ca3 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -29,8 +29,8 @@
>>  #include "libavutil/version.h"
>>
>>  #define LIBAVCODEC_VERSION_MAJOR 56
>> -#define LIBAVCODEC_VERSION_MINOR 23
>> -#define LIBAVCODEC_VERSION_MICRO  2
>> +#define LIBAVCODEC_VERSION_MINOR 24
>> +#define LIBAVCODEC_VERSION_MICRO  0
>>
>>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>>                                                 LIBAVCODEC_VERSION_MINOR, \
>> --
>> 1.9.0
>>
>
>
> You are adding stuff at the beginning of a public struct, thats going
> to break ABI. A minor bump isn't going to cut it.
>


To be slightly constructive about this:
Skip the avclass, i'm not sure why there is an avoption for this one
thing anyway?
Instead add the new member to the end of the struct, and init it to
its default value in the allocation function (av_vda_alloc_context)

That way the ABI remains stable, and you get the same end-result.

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

Reply via email to