Re: [libav-devel] [PATCH] vaapi_encode: Move quality option in common code

2017-05-12 Thread Anton Khirnov
Quoting Mark Thompson (2017-04-30 20:43:34)
> Use AVCodecContext.compression_level rather than a private option,
> replacing the H.264-specific quality option (which stays only for
> compatibility).
> 
> This now works with the H.265 encoder in the i965 driver, as well as
> the existing cases with the H.264 encoder.
> ---
>  doc/encoders.texi  |  9 ++---
>  libavcodec/vaapi_encode.c  | 33 +
>  libavcodec/vaapi_encode.h  |  6 ++
>  libavcodec/vaapi_encode_h264.c | 25 ++---
>  4 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 7cebe39c1..c369e03bf 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -948,7 +948,13 @@ The following standard libavcodec options are used:
>  @item
>  @option{rc_init_occupancy} / @option{rc_initial_buffer_occupancy}
>  @item
> +@option{compression_level}
> +
> +Speed / quality tradeoff: higher values are faster / worse quality.
> +@item
>  @option{q} / @option{global_quality}
> +
> +Size / quality tradeoff: higher values are smaller / worse quality.
>  @item
>  @option{qmin}
>  (only: @option{qmax} is not supported)
> @@ -969,9 +975,6 @@ The following standard libavcodec options are used:
>  @option{level} sets the value of @emph{level_idc}.
>  
>  @table @option
> -@item quality
> -Set the local encoding quality/speed tradeoff (range 1-8, higher values are 
> faster; not all
> -systems implement all levels).
>  @item low_power
>  Use low-power encoding mode.
>  @end table
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 620518419..5e5177a5d 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1423,6 +1423,39 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>  goto fail;
>  }
>  
> +if (avctx->compression_level >= 0) {
> +#if VA_CHECK_VERSION(0, 36, 0)
> +VAConfigAttrib attr = { VAConfigAttribEncQualityRange };
> +
> +vas = vaGetConfigAttributes(ctx->hwctx->display,
> +ctx->va_profile,
> +ctx->va_entrypoint,
> +, 1);
> +if (vas != VA_STATUS_SUCCESS) {
> +av_log(avctx, AV_LOG_WARNING, "Failed to query quality "
> +   "attribute: will use default compression level.\n");
> +} else if (avctx->compression_level > attr.value) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid compression level: "
> +   "valid range is 0-%d.\n", attr.value);
> +err = AVERROR(EINVAL);

It looks a bit strange to me that this branch fails, while the case
where vaapi doesn't support this option continues.

Otherwise looks fine.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] doc: Document hwupload, hwdownload and hwmap filters

2017-05-12 Thread Anton Khirnov
Quoting Mark Thompson (2017-04-30 23:13:26)
> ---
>  doc/filters.texi | 98 
> 
>  1 file changed, 98 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 947fd7915..f57fbf6c4 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1657,6 +1657,104 @@ A floating point number which specifies chroma 
> temporal strength. It defaults to
>  @var{luma_tmp}*@var{chroma_spatial}/@var{luma_spatial}.
>  @end table
>  
> +@section hwdownload
> +
> +Download hardware frames to system memory.
> +
> +The input must be in hardware frames, and the output a non-hardware format.
> +Not all formats will be supported on the output - it may be necessary to 
> insert
> +an additional @option{format} filter immediately following in the graph to 
> get
> +the output in a supported format.
> +
> +@section hwmap
> +
> +Map hardware frames to system memory or to another device.
> +
> +This filter has several different modes of operation; which is used depends 
> on

which one?

> +the input and output formats:
> +@itemize
> +@item
> +Hardware frame input, normal frame output
> +
> +Map the input frames to system memory and give them to the output.  If the

'pass them to the output' sounds better to me

> +original hardware frame is later required (for example, after overlaying
> +something else on part of it), the @option{hwmap} filter can be used again
> +in the next mode to retrieve them.

retrieve _it_, or alternatively 'original hardware frames_s_' above

Otherwise LGTM

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] configure: rename hevc_ps to hevcparse

2017-05-12 Thread Anton Khirnov
Quoting Diego Biurrun (2017-05-03 09:58:32)
> On Tue, May 02, 2017 at 06:06:52PM -0300, James Almer wrote:
> > Build h2645_parse.o with it, as every hevc_ps dependency also needs it.
> > This is more in line with h264's h264parse module.
> > 
> > Signed-off-by: James Almer 
> > ---
> >  configure   | 10 +-
> >  libavcodec/Makefile |  8 
> >  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> Not against. Anton?

Sure, fine with me

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/200] Add a new channel layout API

2017-05-12 Thread Anton Khirnov
Quoting wm4 (2017-05-06 05:13:37)
> On Fri,  5 May 2017 22:20:18 -0400
> Vittorio Giovara  wrote:
> 
> 
> > +enum AVChannelOrder {
> > +/**
> > + * The native channel order, i.e. the channels are in the same order in
> > + * which they are defined in the AVChannel enum. This supports up to 63
> > + * different channels.
> > + */
> > +AV_CHANNEL_ORDER_NATIVE,
> > +/**
> > + * The channel order does not correspond to any other predefined order 
> > and
> > + * is stored as an explicit map. For example, this could be used to 
> > support
> > + * layouts with 64 or more channels.
> > + */
> > +AV_CHANNEL_ORDER_CUSTOM,
> > +/**
> > + * Only the channel count is specified, without any further information
> > + * about the channels.
> > + */
> > +AV_CHANNEL_ORDER_UNSPEC,
> > +};
> 
> 
> > +typedef struct AVChannelLayout {
> > +/**
> > + * Channel order used in this layout.
> > + */
> > +enum AVChannelOrder order;
> > +
> > +/**
> > + * Number of channels in this layout. Mandatory field.
> > + */
> > +int nb_channels;
> > +
> > +/**
> > + * Details about which channels are present in this layout.
> > + * For AV_CHANNEL_ORDER_UNSPEC, this field is undefined and must not be
> > + * used.
> > + */
> > +union {
> > +/**
> > + * This member must be used for AV_CHANNEL_ORDER_NATIVE.
> > + * It is a bitmask, where the position of each set bit means that 
> > the
> > + * AVChannel with the corresponding value is present.
> > + *
> > + * I.e. when (mask & (1 << AV_CHAN_FOO)) is non-zero, then 
> > AV_CHAN_FOO
> > + * is present in the layout. Otherwise it is not present.
> > + *
> > + * @note when a channel layout using a bitmask is constructed or
> > + * modified manually (i.e.  not using any of the 
> > av_channel_layout_*
> > + * functions), the code doing it must ensure that the number of 
> > set bits
> > + * is equal to nb_channels.
> > + */
> > +uint64_t mask;
> > +/**
> > + * This member must be used when the channel order is not native, 
> > and
> > + * represents a nb_channels-sized array. Its semantics may 
> > depending on
> > + * the channel order.
> > + *
> > + * - For AV_CHANNEL_ORDER_CUSTOM: each element signals the 
> > presence of
> > + *   the AVChannel with the corresponding value: eg. when map[i] is
> > + *   equal to AV_CHAN_FOO, then AV_CHAN_FOO is the i-th channel in
> > + *   the audio data.
> 
> Does it allow a channel to be mapped more than once? Does it allow
> padding channels (that would be all-0)?

FWIW it was my intention from the beginning that this API should allow
us to "properly" support retarted things like dual-mono. So yes, any
channel can be mapped any number of times with the custom order.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] hevcdec: move the MD5 context out of HEVCSEIPictureHash back into HEVCContext

2017-05-12 Thread Luca Barbato
On 5/12/17 4:40 PM, Anton Khirnov wrote:
> HEVCSEIPictureHash should store only the information extracted from the
> bitstream and exported to the higher layer (the decoder or the parser).
> The MD5 context is allocated, used and freed by this higher layer, so it
> makes more sense for it to also be stored there.
> ---
>  libavcodec/hevc_sei.h |  3 ---
>  libavcodec/hevcdec.c  | 12 ++--
>  libavcodec/hevcdec.h  |  2 ++
>  3 files changed, 8 insertions(+), 9 deletions(-)
> 

Ok.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] hevcdec: move the MD5 context out of HEVCSEIPictureHash back into HEVCContext

2017-05-12 Thread Anton Khirnov
HEVCSEIPictureHash should store only the information extracted from the
bitstream and exported to the higher layer (the decoder or the parser).
The MD5 context is allocated, used and freed by this higher layer, so it
makes more sense for it to also be stored there.
---
 libavcodec/hevc_sei.h |  3 ---
 libavcodec/hevcdec.c  | 12 ++--
 libavcodec/hevcdec.h  |  2 ++
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
index bdc283a..b699fef 100644
--- a/libavcodec/hevc_sei.h
+++ b/libavcodec/hevc_sei.h
@@ -23,8 +23,6 @@
 
 #include 
 
-#include "libavutil/md5.h"
-
 #include "get_bits.h"
 
 /**
@@ -59,7 +57,6 @@ typedef enum {
 } HEVC_SEI_Type;
 
 typedef struct HEVCSEIPictureHash {
-struct AVMD5 *md5_ctx;
 uint8_t   md5[3][16];
 uint8_t is_md5;
 } HEVCSEIPictureHash;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 96e47cd..69d5908 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2683,7 +2683,7 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
 int h = (i == 1 || i == 2) ? (height >> desc->log2_chroma_h) : height;
 uint8_t md5[16];
 
-av_md5_init(s->sei.picture_hash.md5_ctx);
+av_md5_init(s->md5_ctx);
 for (j = 0; j < h; j++) {
 const uint8_t *src = frame->data[i] + j * frame->linesize[i];
 #if HAVE_BIGENDIAN
@@ -2693,9 +2693,9 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
 src = s->checksum_buf;
 }
 #endif
-av_md5_update(s->sei.picture_hash.md5_ctx, src, w << pixel_shift);
+av_md5_update(s->md5_ctx, src, w << pixel_shift);
 }
-av_md5_final(s->sei.picture_hash.md5_ctx, md5);
+av_md5_final(s->md5_ctx, md5);
 
 if (!memcmp(md5, s->sei.picture_hash.md5[i], 16)) {
 av_log   (s->avctx, AV_LOG_DEBUG, "plane %d - correct ", i);
@@ -2893,7 +2893,7 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx)
 
 pic_arrays_free(s);
 
-av_freep(>sei.picture_hash.md5_ctx);
+av_freep(>md5_ctx);
 
 av_frame_free(>tmp_frame);
 av_frame_free(>output_frame);
@@ -2939,8 +2939,8 @@ static av_cold int hevc_init_context(AVCodecContext 
*avctx)
 
 s->max_ra = INT_MAX;
 
-s->sei.picture_hash.md5_ctx = av_md5_alloc();
-if (!s->sei.picture_hash.md5_ctx)
+s->md5_ctx = av_md5_alloc();
+if (!s->md5_ctx)
 goto fail;
 
 ff_bswapdsp_init(>bdsp);
diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
index f2c5892..7adb826 100644
--- a/libavcodec/hevcdec.h
+++ b/libavcodec/hevcdec.h
@@ -27,6 +27,7 @@
 #include 
 
 #include "libavutil/buffer.h"
+#include "libavutil/md5.h"
 
 #include "avcodec.h"
 #include "bswapdsp.h"
@@ -462,6 +463,7 @@ typedef struct HEVCContext {
 
 HEVCParamSets ps;
 HEVCSEI sei;
+struct AVMD5 *md5_ctx;
 
 AVBufferPool *tab_mvf_pool;
 AVBufferPool *rpl_tab_pool;
-- 
2.0.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel