Re: [libav-devel] [PATCH 1/9] lavc: Add codec metadata to indicate hardware support

[Anton Khirnov]

ec, int 
> index);
> +
> +/**
>   * @defgroup lavc_hwaccel AVHWAccel
>   * @{
>   */
> diff --git a/libavcodec/hwaccel.h b/libavcodec/hwaccel.h
> index 60dbe81c8..a066bc40f 100644
> --- a/libavcodec/hwaccel.h
> +++ b/libavcodec/hwaccel.h
> @@ -19,6 +19,24 @@
>  #ifndef AVCODEC_HWACCEL_H
>  #define AVCODEC_HWACCEL_H
>  
> +#include "avcodec.h"
> +
> +
>  #define HWACCEL_CAP_ASYNC_SAFE  (1 << 0)
>  
> +
> +typedef struct FFCodecHWConfig {
> +/**
> + * This is the structure which will be returned to the user by
> + * avcodec_get_hw_config().
> + */
> +AVCodecHWConfig public;

const?

> +/**
> + * If this configuration uses a hwaccel, a pointer to it.
> + * If not, NULL.
> + */
> +AVHWAccel *hwaccel;

const?

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

Re: [libav-devel] API breakage with old avcodec_decode_* functions

[Anton Khirnov]

Hi,
Quoting James Cowgill (2017-10-30 16:50:58)
> Hi,
> 
> This is an issue which was reported in a number of places. I'm asking
> here since libav was the origin of the commit which broke things and
> hopefully someone here has an answer.
> 
> https://trac.ffmpeg.org/ticket/6775
> https://bugzilla.gnome.org/show_bug.cgi?id=789193
> https://bugs.debian.org/879673
> 
> In commit 061a0c14bb57 ("decode: restructure the core decoding code"),
> the avcodec_decode_* APIs were rewritten in terms of the new
> avcodec_send_packet / avcodec_receive_frame APIs. Unfortunately this
> changed the behavior of the old APIs on receipt of a NULL packet. In the
> old API this meant "drain the codec of the remaining frames". Usually
> this was done at the end of the stream, but it worked (to an extent)
> when done in the middle of a stream. In the new API, a NULL packet means
> the end of file and no more packets can be sent to the codec. Since NULL
> packets are passed through to avcodec_send_packet in the compatability
> code added in the above commit, applications which send NULL packets in
> the middle of a stream are broken with new libav / ffmpeg.
> 
> gst-libav exploited this by sending NULL packets to
> avcodec_decode_video4 whenever some input packets were lost
> (unfortunately I don't know the details of this very well). It also
> sends a NULL packet before trying to decode anything.
> 
> My questions are: is what gst-libav is doing legal in the old API? If it
> is, what can be done to fix it? If it isn't, what is gst-libav supposed
> to do to avoid loosing functionality?
>

I suppose it's not been explicitly documented (not many things were in
ye olde days), but my view is that it's illegal and not allowed. That it
might have worked without too many problems is mere coincidence, I don't
recall any developer ever considering this as valid API usage.

Hard to tell you how to fix it if you don't know exactly what it's there
for. If you have packet loss, you're not required to do anything special
with the decoder, you just do not give the packets you don't have.
Similarly, I don't see any reason why sending a NULL packet at the
beginning should ever accomplish anything useful.

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

Re: [libav-devel] [PATCH 6/6] hwcontext_vaapi: Set message callbacks on internally-created devices

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:06)
> The message callbacks are library-safe in libva2, so we can now use
> them.
> ---
>  libavutil/hwcontext_vaapi.c | 21 +
>  1 file changed, 21 insertions(+)
> 

Looks very good to me

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

Re: [libav-devel] [PATCH 5/6] vaapi: Always free parameter buffers after vaEndPicture() with libva2

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:05)
> This is an ABI change in libva2: previously the Intel driver had this
> behaviour and it was implemented as a driver quirk, but now it is part
> of the specification so all drivers must do it.
> ---
>  libavcodec/vaapi_decode.c  | 4 ++--
>  libavcodec/vaapi_encode.c  | 4 ++--
>  libavfilter/vf_deinterlace_vaapi.c | 2 +-
>  libavfilter/vf_scale_vaapi.c   | 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)

Ok

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

Re: [libav-devel] [PATCH 4/6] vaapi_h264: Do not use deprecated header type

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:04)
> SEI headers should be inserted as generic raw data (the old specific
> type has been deprecated in libva2).
> ---
>  libavcodec/vaapi_encode_h264.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 833d442d0..388d950cc 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -252,7 +252,7 @@ static int 
> vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>  
>  ff_cbs_fragment_uninit(>cbc, au);
>  
> -*type = VAEncPackedHeaderH264_SEI;
> +*type = VAEncPackedHeaderRawData;

This makes no difference for old libva?

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

Re: [libav-devel] [PATCH 3/6] vaapi: Remove H.264 baseline profile

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:03)
> This has been deprecated in libva2 because hardware does not and will not
> support it.  Therefore never consider it for decode, and for encode assume
> the user meant constrained baseline profile instead.
> ---
>  libavcodec/vaapi_decode.c  | 1 -
>  libavcodec/vaapi_encode_h264.c | 7 ---
>  2 files changed, 4 insertions(+), 4 deletions(-)

Ok

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

Re: [libav-devel] [PATCH 2/6] configure: Add config option for libva2 (VAAPI 1)

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:02)
> ---
>  configure | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/configure b/configure
> index a3cfe3768..54795aee0 100755
> --- a/configure
> +++ b/configure
> @@ -1804,6 +1804,7 @@ CONFIG_EXTRA="
>  texturedsp
>  texturedspenc
>  tpeldsp
> +vaapi_1

Isn't SYSTEM_FEATURES the more correct places for this?

Otherwise looks ok

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

Re: [libav-devel] [PATCH 1/6] vaapi: Disable deprecation warnings around use of struct vaapi_context

[Anton Khirnov]

Quoting Mark Thompson (2017-10-02 00:01:01)
> ---
>  libavcodec/vaapi_decode.h | 3 +++
>  1 file changed, 3 insertions(+)
> 

LGTM

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

Re: [libav-devel] [PATCH v4] lavc: external hardware frame pool initialization

[Anton Khirnov]

Quoting wm4 (2017-10-19 16:38:20)
> This adds a new API, which allows the API user to query the required
> AVHWFramesContext parameters. This also reduces code duplication across
> the hwaccels by introducing ff_decode_get_hw_frames_ctx(), which uses
> the new API function. It takes care of initializing the hw_frames_ctx
> if needed, and does additional error handling and API usage checking.
> 
> Support for VDA and Cuvid missing.
> ---
> With updated doxygen.
> ---

Thanks, pushed.

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

Re: [libav-devel] [PATCH v2] lavc: external hardware frame pool initialization

[Anton Khirnov]

ing to use for decoding. Calling it outside of get_format is not 
> allowed,
> + *   and can trigger undefined behavior.
> + * - If the decoder does not support this functionality, AVERROR(ENOENT) will
> + *   be returned. This happens only if the format is a software format, or if
> + *   the decoder does not support custom allocated AVHWFramesContext 
> properly.
> + *   Be aware that even if this returns successfully, hwaccel initialization
> + *   could fail later.
> + * - The hw_pix_fmt must be one of the choices suggested by get_format. If 
> the
> + *   user decides to use a hw_frames_ctx prepared with this API function, the
> + *   user must return the same hw_pix_fmt from get_format.
> + * - The hw_frames_ctx must be allocated from a device type that supports the
> + *   given hw_pix_fmt.
> + * - The passed hw_frames_ctx must not have been initialized yet.
> + * - The API function may overwrite any fields in the hw_frames_ctx. It will 
> not
> + *   actually initialize the context. A user calls this API function to get
> + *   basic parameters, and can afterwards modify the parameters as needed.
> + * - After calling this API function, it is the user's responsibility to
> + *   initialize the hw_frames_ctx, and to set AVCodecContext.hw_frames_ctx
> + *   to it. If done, this must be done before returning from get_format (this
> + *   is implied by the normal AVCodecContext.hw_frames_ctx API rules).
> + * - The hw_frames_ctx parameters may change every time time get_format is
> + *   called. Also, AVCodecContext.hw_frames_ctx is reset before get_format. 
> So
> + *   you are inherently required to go through this process again on every
> + *   get_format call.
> + * - It is perfectly possible to call this function without actually using
> + *   the resulting hw_frames_ctx. One use-case might be trying to reuse a
> + *   previously initialized hw_frames_ctx, and calling this API function only
> + *   to test whether the required frame parameters have changed.
> + *
> + * The function will set at least the following fields on hw_frames_ctx
> + * (potentially more, depending on hwaccel API):
> + *
> + * - Set the format field to hw_pix_fmt.
> + * - Set the sw_format field to the most suited and most versatile format. 
> (An
> + *   implication is that this will prefer generic formats over opaque formats
> + *   with arbitrary restrictions, if possible.)
> + * - Set the width/height fields to the coded frame size, rounded up to the
> + *   API-specific minimum alignment.
> + * - Only _if_ the hwaccel requires a pre-allocated pool: set the 
> initial_pool_size
> + *   field to the number of maximum reference surfaces possible with the 
> codec,
> + *   plus 1 surface for the user to work (meaning the user can safely 
> reference
> + *   at most 1 decoded surface at a time), plus additional buffering 
> introduced
> + *   by frame threading. If the hwaccel does not require pre-allocation, the
> + *   field is left to 0, and the decoder will allocate new surfaces on demand
> + *   during decoding.
> + *
> + * @param avctx The context which is currently calling get_format, and which
> + *  implicitly contains all state needed for filling 
> hw_frames_ctx
> + *  properly.
> + * @param hw_pix_fmt The hwaccel format you are going to return from 
> get_format.
> + * @param hw_frames_ctx A reference to an _uninitialized_ AVHWFramesContext.
> + *  Fields will be set to values required for decoding.

The frames context can potentially contain user-allocated content which
is then freed in the user-set free() callback. The semantics of that
memory management is then unclear.

Is there any reason the av_hwframe_ctx_alloc() call must be done by the
user? Doing it inside this function (and so changing the last parameter
to AVBufferRef**) seems to avoid plenty of weird corner cases with no
disadvantages.

Then we have to say whether this function is allowed to set the
opaque/free fields and any possibly-allocated fields in the
type-specific contexts (e.g. attributes for vaapi). Forbidding it makes
it all simpler but may bite us later when it turns out they are
necessary because some hwaccel is particularly insane.

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

Re: [libav-devel] [PATCH] lavf: make avio_read_partial() public

[Anton Khirnov]

Quoting wm4 (2017-08-18 15:35:49)
> On Fri, 18 Aug 2017 15:22:28 +0200
> Anton Khirnov <an...@khirnov.net> wrote:
> 
> > Quoting wm4 (2017-08-18 14:44:45)
> > > On Fri, 18 Aug 2017 15:40:32 +0300 (EEST)
> > > Martin Storsjö <mar...@martin.st> wrote:
> > >   
> > > > On Fri, 18 Aug 2017, Anton Khirnov wrote:
> > > >   
> > > > > Quoting wm4 (2017-08-17 15:01:44)
> > > > >> Main use-case is proxying avio through a foreign I/O layer and a 
> > > > >> custom
> > > > >> AVIO context, without losing latency and performance characteristics.
> > > > >> ---
> > > > >>  doc/APIchanges  | 3 +++
> > > > >>  libavformat/avio.h  | 9 +
> > > > >>  libavformat/avio_internal.h | 8 
> > > > >>  libavformat/aviobuf.c   | 2 +-
> > > > >>  libavformat/rawdec.c| 2 +-
> > > > >>  libavformat/rtsp.c  | 2 +-
> > > > >>  libavformat/version.h   | 2 +-
> > > > >>  7 files changed, 16 insertions(+), 12 deletions(-)
> > > > >> 
> > > > >> diff --git a/doc/APIchanges b/doc/APIchanges
> > > > >> index 463247f48e..ed90be890d 100644
> > > > >> --- a/doc/APIchanges
> > > > >> +++ b/doc/APIchanges
> > > > >> @@ -13,6 +13,9 @@ libavutil: 2017-03-23
> > > > >>
> > > > >>  API changes, most recent first:
> > > > >> 
> > > > >> +2016-xx-xx - xxx - lavf 58.1.0 - avio.h
> > > > >> +  Add avio_read_partial().
> > > > >> +
> > > > >>  2017-xx-xx - xxx - lavu 56.4.0 - imgutils.h
> > > > >>Add av_image_fill_black().
> > > > >> 
> > > > >> diff --git a/libavformat/avio.h b/libavformat/avio.h
> > > > >> index e65135ed99..f604c4ad41 100644
> > > > >> --- a/libavformat/avio.h
> > > > >> +++ b/libavformat/avio.h
> > > > >> @@ -331,6 +331,15 @@ void avio_flush(AVIOContext *s);
> > > > >>   */
> > > > >>  int avio_read(AVIOContext *s, unsigned char *buf, int size);
> > > > >> 
> > > > >> +/**
> > > > >> + * Read size bytes from AVIOContext into buf. Unlike avio_read(), 
> > > > >> this is allowed
> > > > >> + * to read fewer bytes than requested. The missing bytes can be 
> > > > >> read in the next
> > > > >> + * call. This always tries to read at least 1 byte.
> > > > >> + * Useful to reduce latency in certain cases.
> > > > >> + * @return number of bytes read or AVERROR
> > > > >> + */
> > > > >> +int avio_read_partial(AVIOContext *s, unsigned char *buf, int 
> > > > >> size);
> > > > >
> > > > > Maybe this would be a good opportunity to make this size_t
> > > > 
> > > > Hmm, not sure how much point there is in that, since all of the 
> > > > intermediate api (the avio callbacks) still are in int. So I'm 
> > > > wondering 
> > > > if it's better to change things gradually with a large inconsistency 
> > > > internally in the avio layer, or just switch it all at once (if 
> > > > needed).  
> > > 
> > > I'd vote the latter.  
> > 
> > Well, it's easier to change internals than public API.
> > 
> > In this case I'd prefer a gradual transition.
> > 
> 
> I'm telling you that makes no sense. int is everywhere in the avio
> code, including the central/most used parts of the API, and it won't
> help a single bit to change an obscure function to size_t, and for
> which the only effect of the change is probably to trigger weird bugs.

Ok, seems I'm in the minority, so I won't press this further.
I'd just suggest considering int64, since 4G is not _that_ insanely huge
and is actually the type we use internally for file sizes/offests.
But if you disagree, I'm fine with either.

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

Re: [libav-devel] [PATCH] lavf: make avio_read_partial() public

[Anton Khirnov]

Quoting wm4 (2017-08-18 14:44:45)
> On Fri, 18 Aug 2017 15:40:32 +0300 (EEST)
> Martin Storsjö <mar...@martin.st> wrote:
> 
> > On Fri, 18 Aug 2017, Anton Khirnov wrote:
> > 
> > > Quoting wm4 (2017-08-17 15:01:44)  
> > >> Main use-case is proxying avio through a foreign I/O layer and a custom
> > >> AVIO context, without losing latency and performance characteristics.
> > >> ---
> > >>  doc/APIchanges  | 3 +++
> > >>  libavformat/avio.h  | 9 +
> > >>  libavformat/avio_internal.h | 8 
> > >>  libavformat/aviobuf.c   | 2 +-
> > >>  libavformat/rawdec.c| 2 +-
> > >>  libavformat/rtsp.c  | 2 +-
> > >>  libavformat/version.h   | 2 +-
> > >>  7 files changed, 16 insertions(+), 12 deletions(-)
> > >> 
> > >> diff --git a/doc/APIchanges b/doc/APIchanges
> > >> index 463247f48e..ed90be890d 100644
> > >> --- a/doc/APIchanges
> > >> +++ b/doc/APIchanges
> > >> @@ -13,6 +13,9 @@ libavutil: 2017-03-23
> > >>
> > >>  API changes, most recent first:
> > >> 
> > >> +2016-xx-xx - xxx - lavf 58.1.0 - avio.h
> > >> +  Add avio_read_partial().
> > >> +
> > >>  2017-xx-xx - xxx - lavu 56.4.0 - imgutils.h
> > >>Add av_image_fill_black().
> > >> 
> > >> diff --git a/libavformat/avio.h b/libavformat/avio.h
> > >> index e65135ed99..f604c4ad41 100644
> > >> --- a/libavformat/avio.h
> > >> +++ b/libavformat/avio.h
> > >> @@ -331,6 +331,15 @@ void avio_flush(AVIOContext *s);
> > >>   */
> > >>  int avio_read(AVIOContext *s, unsigned char *buf, int size);
> > >> 
> > >> +/**
> > >> + * Read size bytes from AVIOContext into buf. Unlike avio_read(), this 
> > >> is allowed
> > >> + * to read fewer bytes than requested. The missing bytes can be read in 
> > >> the next
> > >> + * call. This always tries to read at least 1 byte.
> > >> + * Useful to reduce latency in certain cases.
> > >> + * @return number of bytes read or AVERROR
> > >> + */
> > >> +int avio_read_partial(AVIOContext *s, unsigned char *buf, int size);  
> > >
> > > Maybe this would be a good opportunity to make this size_t  
> > 
> > Hmm, not sure how much point there is in that, since all of the 
> > intermediate api (the avio callbacks) still are in int. So I'm wondering 
> > if it's better to change things gradually with a large inconsistency 
> > internally in the avio layer, or just switch it all at once (if needed).
> 
> I'd vote the latter.

Well, it's easier to change internals than public API.

In this case I'd prefer a gradual transition.

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

Re: [libav-devel] [PATCH 01/10] lavc: Add support for external hardware frame pool initialisation

[Anton Khirnov]

> +
>  static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
>  {
>  FramePool *pool = avctx->internal->pool;
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 403fb4a09..023d94285 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -276,6 +276,23 @@ int ff_side_data_update_matrix_encoding(AVFrame *frame,
>  int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat *fmt);
>  
>  /**
> + * Perform any additional setup required for hardware frames.
> + *
> + * This calls avctx->init_hw_frames() if set, and in that case all setup
> + * will be performed by the user.
> + * If unset, it will set default values which may be influenced by other
> + * user settings (such as avctx->extra_hw_frames).
> + *
> + * avctx->hw_frames_ctx must be set before calling this function.
> + * Inside avctx->hw_frames_ctx, the fields format, sw_format, width and
> + * height must be set.  If dynamically allocated pools are not supported,
> + * then initial_pool_size must also be set, to the minimum hardware frame
> + * pool size necessary for decode (taking into account reference frames
> + * and delay as appropriate).
> + */
> +int ff_init_hw_frames(AVCodecContext *avctx);

Please don't use internal.h as a dump for everything, this belongs in
hwaccel.h

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

Re: [libav-devel] [PATCH 5/5] mpeg2enc: Don't mark all streams as component video

[Anton Khirnov]

Quoting Mark Thompson (2017-08-15 01:02:40)
> Since there is no information about the source format, "unspecified"
> is the correct value to write here.
> 
> All tests using the MPEG-2 encoder are updated, as this changes the
> header on all outputs.
> ---
> The actual diffs are all trivial, e.g. for fate-lavf-mxf:

Ok.

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

Re: [libav-devel] [PATCH 3/5] lavc: Add mpeg2_metadata bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-08-15 01:02:38)
> ---
>  configure   |   1 +
>  doc/bitstream_filters.texi  |  36 
>  libavcodec/Makefile |   1 +
>  libavcodec/bitstream_filters.c  |   1 +
>  libavcodec/mpeg2_metadata_bsf.c | 356 
> 
>  5 files changed, 395 insertions(+)
>  create mode 100644 libavcodec/mpeg2_metadata_bsf.c
> 
> +static int mpeg2_metadata_update_fragment(AVBSFContext *bsf,
> +  CodedBitstreamFragment *frag)
> +{
> +MPEG2MetadataContext *ctx = bsf->priv_data;
> +MPEG2RawSequenceHeader*sh = NULL;
> +MPEG2RawSequenceExtension *se = NULL;
> +MPEG2RawSequenceDisplayExtension *sde = NULL;
> +int i, se_pos, add_sde = 0;
> +
> +for (i = 0; i < frag->nb_units; i++) {
> +if (frag->units[i].type == MPEG2_START_SEQUENCE_HEADER) {
> +sh = frag->units[i].content;
> +} else if (frag->units[i].type == MPEG2_START_EXTENSION) {
> +MPEG2RawExtensionData *ext = frag->units[i].content;
> +if (ext->extension_start_code_identifier ==
> +MPEG2_EXTENSION_SEQUENCE) {
> +se = >data.sequence;
> +se_pos = i;
> +} else if (ext->extension_start_code_identifier ==
> +MPEG2_EXTENSION_SEQUENCE_DISPLAY) {
> +sde = >data.sequence_display;
> +}
> +}
> +}
> +
> +if (!sh || !se) {
> +// No sequence header and sequence extension: not an MPEG-2 video
> +// sequence.
> +if (sh) {
> +av_log(bsf, AV_LOG_WARNING, "Stream contains a sequence "
> +   "header but not a sequence extension: maybe it's "
> +   "actually MPEG-1?\n");

If it actually is MPEG-1 then this will get spammy, won't it? Maybe only
print it once.

> +static int mpeg2_metadata_filter(AVBSFContext *bsf, AVPacket *out)
> +{
> +MPEG2MetadataContext *ctx = bsf->priv_data;
> +AVPacket *in = NULL;
> +CodedBitstreamFragment *frag = >fragment;
> +int err;
> +
> +err = ff_bsf_get_packet(bsf, );
> +if (err < 0)
> +goto fail;
> +
> +err = ff_cbs_read_packet(>cbc, frag, in);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
> +goto fail;
> +}
> +
> +err = mpeg2_metadata_update_fragment(bsf, frag);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to update frame fragment.\n");
> +goto fail;
> +}
> +
> +err = ff_cbs_write_packet(>cbc, out, frag);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
> +goto fail;
> +}
> +
> +err = av_packet_copy_props(out, in);
> +if (err < 0)
> +goto fail;

This leaks the contents of out I think.
Not sure if we should make the generic code unref it on failure.

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

Re: [libav-devel] [PATCH 2/5] lavc: Add coded bitstream read/write support for MPEG-2

[Anton Khirnov]

Quoting Mark Thompson (2017-08-15 01:02:37)
> Also enable MPEG-2 support in the trace_headers filter.
> ---
> * Updated to add build system.
> * Writing support is redone to not have a huge buffer.
> * Cleaned the tricky part of the split setup a bit.
> * Unit free actually implemented so it doesn't leak all the memory.
> 
> 
>  configure  |   4 +-
>  doc/bitstream_filters.texi |   2 +-
>  libavcodec/Makefile|   1 +
>  libavcodec/cbs.c   |   3 +
>  libavcodec/cbs_internal.h  |   1 +
>  libavcodec/cbs_mpeg2.c | 409 
> +
>  libavcodec/cbs_mpeg2.h | 216 +
>  libavcodec/cbs_mpeg2_syntax_template.c | 340 +++
>  libavcodec/trace_headers_bsf.c |   1 +
>  9 files changed, 975 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/cbs_mpeg2.c
>  create mode 100644 libavcodec/cbs_mpeg2.h
>  create mode 100644 libavcodec/cbs_mpeg2_syntax_template.c
> 

LGTM

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

Re: [libav-devel] [PATCH] lavf: make avio_read_partial() public

[Anton Khirnov]

Quoting wm4 (2017-08-17 15:01:44)
> Main use-case is proxying avio through a foreign I/O layer and a custom
> AVIO context, without losing latency and performance characteristics.
> ---
>  doc/APIchanges  | 3 +++
>  libavformat/avio.h  | 9 +
>  libavformat/avio_internal.h | 8 
>  libavformat/aviobuf.c   | 2 +-
>  libavformat/rawdec.c| 2 +-
>  libavformat/rtsp.c  | 2 +-
>  libavformat/version.h   | 2 +-
>  7 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 463247f48e..ed90be890d 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2017-03-23
>  
>  API changes, most recent first:
>  
> +2016-xx-xx - xxx - lavf 58.1.0 - avio.h
> +  Add avio_read_partial().
> +
>  2017-xx-xx - xxx - lavu 56.4.0 - imgutils.h
>Add av_image_fill_black().
>  
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index e65135ed99..f604c4ad41 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -331,6 +331,15 @@ void avio_flush(AVIOContext *s);
>   */
>  int avio_read(AVIOContext *s, unsigned char *buf, int size);
>  
> +/**
> + * Read size bytes from AVIOContext into buf. Unlike avio_read(), this is 
> allowed
> + * to read fewer bytes than requested. The missing bytes can be read in the 
> next
> + * call. This always tries to read at least 1 byte.
> + * Useful to reduce latency in certain cases.
> + * @return number of bytes read or AVERROR
> + */
> +int avio_read_partial(AVIOContext *s, unsigned char *buf, int size);

Maybe this would be a good opportunity to make this size_t

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

[libav-devel] [PATCH 1/2] h264dec: use a large enough field for reference list modification values

[Anton Khirnov]

pic_num can be at most 17-bit, so uint8_t is not sufficient.

Found-By: Bradley Sepos 
CC: libav-sta...@libav.org
---
 libavcodec/h264dec.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index ddfe224..cce5e19 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -268,7 +268,7 @@ typedef struct H264SliceContext {
  *   according to picture reordering 
in slice header */
 struct {
 uint8_t op;
-uint8_t val;
+uint32_t val;
 } ref_modifications[2][32];
 int nb_ref_modifications[2];
 
-- 
2.0.0

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

[libav-devel] [PATCH 2/2] FATE: add a test for the H.264 sample fixed by 7c4f6f6

[Anton Khirnov]

---
 tests/fate/h264.mak  |  2 ++
 tests/ref/fate/h264-ref-pic-mod-overflow | 21 +
 2 files changed, 23 insertions(+)
 create mode 100644 tests/ref/fate/h264-ref-pic-mod-overflow

diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
index bdc390d..7676fb8 100644
--- a/tests/fate/h264.mak
+++ b/tests/fate/h264.mak
@@ -183,6 +183,7 @@ FATE_H264  := $(FATE_H264:%=fate-h264-conformance-%)
\
   fate-h264-intra-refresh-recovery  \
   fate-h264-lossless\
   fate-h264-missing-frame   \
+  fate-h264-ref-pic-mod-overflow\
 
 FATE_H264-$(call DEMDEC, H264, H264) += $(FATE_H264)
 FATE_H264-$(call DEMDEC,  MOV, H264) += fate-h264-crop-to-container
@@ -396,6 +397,7 @@ fate-h264-intra-refresh-recovery: CMD = 
framecrc -i $(TARGET_SAM
 fate-h264-invalid-ref-mod:CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/h264refframeregression.mp4 -an -frames 10 -pix_fmt 
yuv420p10le
 fate-h264-lossless:   CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/lossless.h264
 fate-h264-mixed-nal-coding:   CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/mixed-nal-coding.mp4
+fate-h264-ref-pic-mod-overflow:   CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/ref-pic-mod-overflow.h264
 fate-h264-twofields-packet:   CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/twofields_packet.mp4 -an -frames 30
 fate-h264-unescaped-extradata:CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/unescaped_extradata.mp4 -an -frames 10
 fate-h264-missing-frame:  CMD = framecrc -i 
$(TARGET_SAMPLES)/h264/nondeterministic_cut.h264
diff --git a/tests/ref/fate/h264-ref-pic-mod-overflow 
b/tests/ref/fate/h264-ref-pic-mod-overflow
new file mode 100644
index 000..b84ef5a
--- /dev/null
+++ b/tests/ref/fate/h264-ref-pic-mod-overflow
@@ -0,0 +1,21 @@
+#tb 0: 1/25
+0,  0,  0,1,  3110400, 0x5cd0bee9
+0,  1,  1,1,  3110400, 0x19d0155c
+0,  2,  2,1,  3110400, 0x3519be4b
+0,  3,  3,1,  3110400, 0x87dc6708
+0,  4,  4,1,  3110400, 0xef3c1056
+0,  5,  5,1,  3110400, 0x5064aad2
+0,  6,  6,1,  3110400, 0xbfb35057
+0,  7,  7,1,  3110400, 0x6089eb95
+0,  8,  8,1,  3110400, 0x85de8dea
+0,  9,  9,1,  3110400, 0x216b1e5a
+0, 10, 10,1,  3110400, 0xba73bdc4
+0, 11, 11,1,  3110400, 0x536e437a
+0, 12, 12,1,  3110400, 0x4be0cdbd
+0, 13, 13,1,  3110400, 0x7316462d
+0, 14, 14,1,  3110400, 0x80eb8622
+0, 15, 15,1,  3110400, 0xa612a70b
+0, 16, 16,1,  3110400, 0x9ff65345
+0, 17, 17,1,  3110400, 0x57003dfa
+0, 18, 18,1,  3110400, 0xf29e036e
+0, 19, 19,1,  3110400, 0x70285363
-- 
2.0.0

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

Re: [libav-devel] [PATCH] mpeg2enc: Don't mark all streams as component video

[Anton Khirnov]

Quoting Mark Thompson (2017-08-12 23:16:13)
> Since there is no information about the source format, "unspecified"
> is the correct value to write here.
> ---
>  libavcodec/mpeg12enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index 103f3aaa7..406950901 100644
> --- a/libavcodec/mpeg12enc.c
> +++ b/libavcodec/mpeg12enc.c
> @@ -297,7 +297,7 @@ static void mpeg1_encode_sequence_header(MpegEncContext 
> *s)
>  
>  put_header(s, EXT_START_CODE);
>  put_bits(>pb, 4, 2); // sequence 
> display extension
> -put_bits(>pb, 3, 0); // video_format: 
> 0 is components
> +put_bits(>pb, 3, 5); // video_format: 
> 5 is unspecified
>  put_bits(>pb, 1, 1); // 
> colour_description
>  put_bits(>pb, 8, s->avctx->color_primaries); // 
> colour_primaries
>  put_bits(>pb, 8, s->avctx->color_trc);   // 
> transfer_characteristics
> -- 
> 2.11.0

Sounds reasonable. I hope it doesn't break any crappy decoders.

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

Re: [libav-devel] [PATCH 14/14] hevc: Remove unused hevc_ps_enc.c

[Anton Khirnov]

Quoting Mark Thompson (2017-08-11 01:37:09)
> Replaced with more complete implementation via coded bitstream infrastructure.
> ---
> Unchanged.

Ok

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

Re: [libav-devel] [PATCH 13/14] qsvenc_hevc: Replace ad-hoc VPS writing with CBS implementation

[Anton Khirnov]

Quoting Mark Thompson (2017-08-11 01:37:08)
> This copies more information which should be present from the SPS.
> It also fixes the value of vps_temporal_id_nesting_flag, which was
> previously incorrect for a single-layer stream (the standard states
> that it must be 1, and the reference decoder barfs if it isn't).
> ---
> Build system change only.

Looks ok. I'll try to test it on Windows later. Poke me if I forget.

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

Re: [libav-devel] [PATCH 12/14] vaapi_h265: Add support for AUD NAL units

[Anton Khirnov]

Quoting Mark Thompson (2017-08-11 01:37:07)
> Matching the H.264 encoder.
> ---

Looks ok

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

Re: [libav-devel] [PATCH 11/14] vaapi_h265: Convert to use coded bitstream infrastructure

[Anton Khirnov]

Quoting Mark Thompson (2017-08-11 01:37:06)
>  
> -priv->ctu_width = FFALIGN(ctx->surface_width,  32) / 32;
> -priv->ctu_height= FFALIGN(ctx->surface_height, 32) / 32;
> +// This seems to be an Intel driver constraint.  Despite MinCbSizeY
> +// being 8, we are still required to encode at 16-pixel alignment and
> +// then crop back (so 1080 lines is still encode as 1088 + cropping).
> +priv->ctu_width = FFALIGN(ctx->surface_width,  16) / 16;
> +priv->ctu_height= FFALIGN(ctx->surface_height, 16) / 16;

Is this related to the rest of the patch? Does not seem so.

Otherwise looks ok as far as I can tell.

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

Re: [libav-devel] [PATCH 10/14] vaapi_h264: Add support for SEI recovery points

[Anton Khirnov]

Quoting Mark Thompson (2017-08-11 01:37:05)
> Included by default with non-IDR intra frames.

LGTM

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

Re: [libav-devel] [PATCH 2/2 V2] libavfilter/vf_vpp: Add common filters of the qsv vpp

[Anton Khirnov]

;+{ "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, 
>{.str="w*ch/cw"}, 0, 255, .flags = FLAGS },
>+{ NULL }
>+};
>+
>+static const char *const var_names[] = {
>+"iw", "in_w",
>+"ih", "in_h",
>+"ow", "out_w", "w",
>+"oh", "out_h", "h",
>+"cw",
>+"ch",
>+"cx",
>+"cy",
>+NULL
>+};
>+
>+enum var_name {
>+VAR_iW, VAR_IN_W,
>+VAR_iH, VAR_IN_H,
>+VAR_oW, VAR_OUT_W, VAR_W,
>+VAR_oH, VAR_OUT_H, VAR_H,
>+CW,
>+CH,
>+CX,
>+CY,
>+VAR_VARS_NB
>+};
>+
>+static int eval_expr(AVFilterContext *ctx)
>+{
>+#define PASS_EXPR(e, s) {\
>+ret = av_expr_parse(, s, var_names, NULL, NULL, NULL, NULL, 0, ctx); \
>+if (ret < 0) {\
>+av_log(ctx, AV_LOG_ERROR, "Error when passing '%s'.\n", s);\
>+return ret;\

Same leak as in the overlay filter.

>+}\
>+}
>+#define CALC_EXPR(e, v, i) {\
>+i = v = av_expr_eval(e, var_values, NULL); \
>+}
>+VPPContext *vpp = ctx->priv;
>+double  var_values[VAR_VARS_NB] = { NAN };
>+AVExpr *w_expr = NULL, *h_expr = NULL;
>+AVExpr *cw_expr = NULL, *ch_expr = NULL;
>+AVExpr *cx_expr = NULL, *cy_expr = NULL;
>+int ret = 0;
>+
>+PASS_EXPR(cw_expr, vpp->cw);
>+PASS_EXPR(ch_expr, vpp->ch);
>+
>+PASS_EXPR(w_expr, vpp->ow);
>+PASS_EXPR(h_expr, vpp->oh);
>+
>+PASS_EXPR(cx_expr, vpp->cx);
>+PASS_EXPR(cy_expr, vpp->cy);
>+
>+var_values[VAR_iW] =
>+var_values[VAR_IN_W] = ctx->inputs[0]->w;
>+
>+var_values[VAR_iH] =
>+var_values[VAR_IN_H] = ctx->inputs[0]->h;
>+
>+/* crop params */
>+CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
>+CALC_EXPR(ch_expr, var_values[CH], vpp->crop_h);
>+
>+/* calc again in case cw is relative to ch */
>+CALC_EXPR(cw_expr, var_values[CW], vpp->crop_w);
>+
>+CALC_EXPR(w_expr,
>+var_values[VAR_OUT_W] = var_values[VAR_oW] = var_values[VAR_W],
>+vpp->out_width);
>+CALC_EXPR(h_expr,
>+var_values[VAR_OUT_H] = var_values[VAR_oH] = var_values[VAR_H],
>+vpp->out_height);
>+
>+/* calc again in case ow is relative to oh */
>+CALC_EXPR(w_expr,
>+var_values[VAR_OUT_W] = var_values[VAR_oW] = var_values[VAR_W],
>+vpp->out_width);
>+
>+
>+CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
>+CALC_EXPR(cy_expr, var_values[CY], vpp->crop_y);
>+
>+/* calc again in case cx is relative to cy */
>+CALC_EXPR(cx_expr, var_values[CX], vpp->crop_x);
>+
>+if((vpp->crop_w != var_values[VAR_iW]) || (vpp->crop_h != 
>var_values[VAR_iH])) {

Why only width/height and not the x/y?
Also, missing space.

>+vpp->use_crop = 1;
>+av_log(NULL, AV_LOG_INFO, "Using the crop feature \n");

This seems unnecessary. And in any case it should use a proper logging
context and have a lower verbosity level.

>+ }
>+
>+av_expr_free(w_expr);
>+    av_expr_free(h_expr);
>+av_expr_free(cw_expr);
>+av_expr_free(ch_expr);
>+av_expr_free(cx_expr);
>+av_expr_free(cy_expr);
>+#undef PASS_EXPR
>+#undef CALC_EXPR
>+
>+return ret;
>+}
>+
>+static int config_input(AVFilterLink *inlink)
>+{
>+AVFilterContext *ctx = inlink->dst;
>+VPPContext  *vpp = ctx->priv;
>+int  ret;
>+
>+if (vpp->framerate.den == 0 || vpp->framerate.num == 0)
>+vpp->framerate = inlink->frame_rate;

The input framerate is not always set, the input could be VFR or its
framerate could be unknown. Can MSDK do VFR to CFR conversion?

>+
>+if (av_q2d(vpp->framerate) != av_q2d(inlink->frame_rate))

Use av_cmp_q()

>+vpp->use_frc = 1;
>+
>+ret = eval_expr(ctx);
>+if (ret != 0) {
>+av_log(ctx, AV_LOG_ERROR, "Fail to eval expr.\n");
>+return ret;
>+}
>+
>+if (vpp->out_height == 0 || vpp->out_width == 0) {
>+vpp->out_width  = inlink->w;
>+vpp->out_height = inlink->h;
>+}
>+
>+if (vpp->use_crop) {
>+if (vpp->crop_x < 0)  vpp->crop_x = 0;

Here and below use FFMIN/FFMAX for clipping, it's shorter and more
readable.

>+if (vpp->crop_y < 0)  vpp->crop_y = 0;
>+
>+if(vpp->crop_w % 2)   vpp->crop_w++;
>+if(vpp->crop_h % 2)   vpp->crop_h++;
>+if(vpp->out_width % 2)vpp->out_width++;
>+if(vpp->out_height % 2)   vpp->out_height++;

This should not silently do something different from what the caller
requested - it should at the very least warn.

>+
>+if(vpp->crop_w + vpp->crop_x > inlink->w)
>+   vpp->crop_x = inlink->w - vpp->crop_w;
>+if(vpp->crop_h + vpp->crop_y > inlink->h)
>+   vpp->crop_y = inlink->h - vpp->crop_h;
>+}
>+
>+return 0;
>+}

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

Re: [libav-devel] [PATCH 1/2 V6] libavfilter/overlay_qsv: Add QSV overlay vpp filter

[Anton Khirnov]

Quoting Diego Biurrun (2017-08-12 10:10:30)
> On Fri, Aug 11, 2017 at 09:46:17AM +0800, Huang, Zhengxu wrote:
> > +s->surface_ptrs_out = 
> > av_mallocz_array(out_frames_hwctx->nb_surfaces,
> > +   
> > sizeof(*s->surface_ptrs_out));
> > +if (!s->surface_ptrs_out) {
> > +av_buffer_unref(_frames_ref);
> > +return AVERROR(ENOMEM);
> > +}
> > +
> > +for (i = 0; i < out_frames_hwctx->nb_surfaces; i++)
> > +s->surface_ptrs_out[i] = out_frames_hwctx->surfaces + i;
> > +s->nb_surface_ptrs_out = out_frames_hwctx->nb_surfaces;
> > +
> > +av_buffer_unref(>hw_frames_ctx);
> > +outlink->hw_frames_ctx = out_frames_ref;
> > +} else
> > +s->out_mem_mode = MFX_MEMTYPE_SYSTEM_MEMORY;
> > +
> > +/* extract the properties of the "master" session given to us */
> > +ret = MFXQueryIMPL(device_hwctx->session, );
> > +if (ret == MFX_ERR_NONE)
> > +ret = MFXQueryVersion(device_hwctx->session, );
> > +if (ret != MFX_ERR_NONE) {
> > +av_log(avctx, AV_LOG_ERROR, "Error querying the session 
> > attributes\n");
> > +return AVERROR_UNKNOWN;
> > +}
> 
> Using plain "else" instead of another if would be clearer.

That's not the same thing as what the code does now. Read it more
carefully.

> 
> > +/* create a "slave" session with those same properties, to be used for 
> > vpp*/
> 
> space before *
> 
> > +int ff_qsvvpp_create(AVFilterContext *avctx, QSVVPPContext **vpp, 
> > QSVVPPParam *param)
> > +{
> > +int ret = 0, i;
> 
> The init is pointless.
> 
> > +QSVVPPContext *s;
> > +
> > +s = av_mallocz(sizeof(*s));
> > +if (!s) {
> > +ret = AVERROR(ENOMEM);
> > +goto failed;
> > +}
> 
> No need for the goto here, you can just return directly.
> 
> > +/* Init each input's information*/
> 
> space before *
> 
> > +failed:
> > +if (s)
> > +ff_qsvvpp_free();
> > +
> > +return ret;
> > +}
> 
> The if is unnecessary if you dropped the goto above.

It's actually unnecessary in all cases, since ff_qsvvpp_free() is a
no-op on NULL.

> 
> > +int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame 
> > *picref)
> > +{
> > +in_frame = submit_frame(s, inlink, picref);
> > +if (!in_frame) {
> > +av_log(ctx, AV_LOG_ERROR, "Fail to submit frame on input[%d]\n",
> > +FF_INLINK_IDX(inlink));
> 
> "Failed", indentation is off.
> 
> > +return AVERROR(EINVAL);
> 
> Is this an error due to user-supplied data or user-supplied configuration?
> Because that's what EINVAL is for.

Should be ENOMEM

> 
> Don't you leak vpp->comp_conf.InputStream here?
> 

No, uninit is always called even if init fails.

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

Re: [libav-devel] [PATCH 1/2 V5] libavfilter/overlay_qsv: Add QSV overlay vpp filter

[Anton Khirnov]

Quoting Huang, Zhengxu (2017-08-10 03:35:09)

>From 873222a2fa0c0686e0611bc769707947fe243d27 Mon Sep 17 00:00:00 2001
>+static int eval_expr(AVFilterContext *ctx)
>+{
>+QSVOverlayContext *vpp = ctx->priv;
>+double *var_values = vpp->var_values;
>+intret = 0;
>+AVExpr *ox_expr, *oy_expr, *ow_expr, *oh_expr;

Those need to be initialized to NULL, otherwise you have invalid frees
in the failure path.

Otherwise looks ok.

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

Re: [libav-devel] [PATCH 13/22] vaapi_h264: Add support for AUD NAL units

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:36)
> Adds a new private option to enable them (off by default).
> ---
>  libavcodec/vaapi_encode_h264.c | 33 +
>  1 file changed, 33 insertions(+)

Looks ok

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

Re: [libav-devel] [PATCH 12/22] vaapi_h264: Convert to use coded bitstream infrastructure

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:35)
> ---
>  libavcodec/Makefile|2 +-
>  libavcodec/vaapi_encode_h264.c | 1421 
> 
>  2 files changed, 548 insertions(+), 875 deletions(-)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 0d8d5eb44..d1fd6fae2 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -272,7 +272,7 @@ OBJS-$(CONFIG_H264_NVENC_ENCODER)  += nvenc_h264.o
>  OBJS-$(CONFIG_H264_OMX_ENCODER)+= omx.o
>  OBJS-$(CONFIG_H264_QSV_DECODER)+= qsvdec_h2645.o
>  OBJS-$(CONFIG_H264_QSV_ENCODER)+= qsvenc_h264.o
> -OBJS-$(CONFIG_H264_VAAPI_ENCODER)  += vaapi_encode_h264.o 
> vaapi_encode_h26x.o
> +OBJS-$(CONFIG_H264_VAAPI_ENCODER)  += vaapi_encode_h264.o

I'd expect a dependency on cbs here (or in configure as discussed on
IRC).

The rest looks sane

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

Re: [libav-devel] [PATCH 11/22] lavc: Add hevc_metadata bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:34)
> This is able to modify some header metadata found in the VPS/SPS/VUI,
> and can also add/remove AUDs.
> ---
>  doc/bitstream_filters.texi |  54 ++
>  libavcodec/Makefile|   2 +
>  libavcodec/bitstream_filters.c |   1 +
>  libavcodec/h265_metadata_bsf.c | 429 
> +
>  4 files changed, 486 insertions(+)
>  create mode 100644 libavcodec/h265_metadata_bsf.c
> 
> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index f121d4757..834cf07f6 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -107,6 +107,60 @@ confuse other transformations which require correct 
> extradata.
>  A new single global PPS is created, and all of the redundant PPSs
>  within the stream are removed.
>  
> +@section hevc_metadata
> +
> +Modify metadata embedded in an HEVC stream.
> +
> +@table @option
> +@item aud
> +Insert or remove AUD NAL units in all access units of the stream.
> +
> +@table @samp
> +@item insert
> +@item remove
> +@end table
> +
> +@item sample_aspect_ratio
> +Set the sample aspect ratio in the stream in the VUI parameters.
> +
> +@item video_format
> +@item video_full_range_flag
> +Set the video format in the stream (see H.265 section E.3.1 and
> +table E.2).
> +
> +@item colour_primaries
> +@item transfer_characteristics
> +@item matrix_coefficients
> +Set the colour description in the stream (see H.265 section E.3.1
> +and tables E.3, E.4 and E.5).
> +
> +@item chroma_sample_loc_type
> +Set the chroma sample location in the stream (see H.265 section
> +E.3.1 and figure E.1).
> +
> +@item tick_rate
> +Set the tick rate in the VPS and VUI parameters (num_units_in_tick /
> +time_scale).  Combined with @option{num_ticks_poc_diff_one}, this can
> +set a constant framerate in the stream.  Note that it is likely to be
> +overridden by container parameters when the stream is in a container.
> +
> +@item num_ticks_poc_diff_one
> +Set poc_proportional_to_timing_flag in VPS and VUI and use this value
> +to set num_ticks_poc_diff_one_minus1 (see H.265 sections 7.4.3.1 and
> +E.3.1).  Ignored if @option{tick_rate} is not also set.
> +
> +@item crop_left
> +@item crop_right
> +@item crop_top
> +@item crop_bottom
> +Set the conformance window cropping offsets in the SPS.  These values
> +will replace the current ones if the stream is already cropped.
> +
> +The unit of these values is chroma pixels after subsampling (see
> +H.265 section 7.4.3.2.1).

I suppose this should be updated in line with the h264 one.

Otherwise looks ok.

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

Re: [libav-devel] [PATCH 07/22] lavc: Add coded bitstream read/write support for MPEG-2

[Anton Khirnov]

int8_t intra_quantiser_matrix[64];
> +uint8_t load_non_intra_quantiser_matrix;
> +uint8_t non_intra_quantiser_matrix[64];
> +} MPEG2RawSequenceHeader;
> +
> +typedef struct MPEG2RawUserData {
> +uint8_t user_data_start_code;
> +
> +uint8_t *user_data;
> +size_t user_data_length;
> +} MPEG2RawUserData;
> +
> +typedef struct MPEG2RawSequenceExtension {
> +uint8_t profile_and_level_indication;
> +uint8_t progressive_sequence;
> +uint8_t chroma_format;
> +uint8_t horizontal_size_extension;
> +uint8_t vertical_size_extension;
> +uint16_t bit_rate_extension;
> +uint8_t vbv_buffer_size_extension;
> +uint8_t low_delay;
> +uint8_t frame_rate_extension_n;
> +uint8_t frame_rate_extension_d;
> +} MPEG2RawSequenceExtension;
> +
> +typedef struct MPEG2RawSequenceDisplayExtension {
> +uint8_t video_format;
> +
> +uint8_t colour_description;
> +uint8_t colour_primaries;
> +uint8_t transfer_characteristics;
> +uint8_t matrix_coefficients;
> +
> +uint16_t display_horizontal_size;
> +uint16_t display_vertical_size;
> +} MPEG2RawSequenceDisplayExtension;
> +
> +typedef struct MPEG2RawGroupOfPicturesHeader {
> +uint8_t group_start_code;
> +
> +uint32_t time_code;
> +uint8_t closed_gop;
> +uint8_t broken_link;
> +} MPEG2RawGroupOfPicturesHeader;
> +
> +typedef struct MPEG2RawPictureHeader {
> +uint8_t picture_start_code;
> +
> +uint16_t temporal_reference;
> +uint8_t picture_coding_type;
> +uint16_t vbv_delay;
> +
> +uint8_t full_pel_forward_vector;
> +uint8_t forward_f_code;
> +uint8_t full_pel_backward_vector;
> +uint8_t backward_f_code;
> +
> +uint8_t extra_bit_picture;
> +} MPEG2RawPictureHeader;
> +
> +typedef struct MPEG2RawPictureCodingExtension {
> +uint8_t f_code[2][2];
> +
> +uint8_t intra_dc_precision;
> +uint8_t picture_structure;
> +uint8_t top_field_first;
> +uint8_t frame_pred_frame_dct;
> +uint8_t concealment_motion_vectors;
> +uint8_t q_scale_type;
> +uint8_t intra_vlc_format;
> +uint8_t alternate_scan;
> +uint8_t repeat_first_field;
> +uint8_t chroma_420_type;
> +uint8_t progressive_frame;
> +
> +uint8_t composite_display_flag;
> +uint8_t v_axis;
> +uint8_t field_sequence;
> +uint8_t sub_carrier;
> +uint8_t burst_amplitude;
> +uint8_t sub_carrier_phase;
> +} MPEG2RawPictureCodingExtension;
> +
> +typedef struct MPEG2RawQuantMatrixExtension {
> +uint8_t load_intra_quantiser_matrix;
> +uint8_t intra_quantiser_matrix[64];
> +uint8_t load_non_intra_quantiser_matrix;
> +uint8_t non_intra_quantiser_matrix[64];
> +uint8_t load_chroma_intra_quantiser_matrix;
> +uint8_t chroma_intra_quantiser_matrix[64];
> +uint8_t load_chroma_non_intra_quantiser_matrix;
> +uint8_t chroma_non_intra_quantiser_matrix[64];
> +} MPEG2RawQuantMatrixExtension;
> +
> +typedef struct MPEG2RawExtensionData {
> +uint8_t extension_start_code;
> +uint8_t extension_start_code_identifier;
> +
> +union {
> +MPEG2RawSequenceExtension sequence;
> +MPEG2RawSequenceDisplayExtension sequence_display;
> +MPEG2RawQuantMatrixExtension quant_matrix;
> +MPEG2RawPictureCodingExtension picture_coding;
> +} data;
> +} MPEG2RawExtensionData;
> +
> +typedef struct MPEG2RawSliceHeader {
> +uint8_t slice_vertical_position;
> +
> +uint8_t slice_vertical_position_extension;
> +uint8_t priority_breakpoint;
> +
> +uint8_t quantiser_scale_code;
> +
> +uint8_t slice_extension_flag;
> +uint8_t intra_slice;
> +uint8_t slice_picture_id_enable;
> +uint8_t slice_picture_id;
> +
> +uint8_t extra_bit_slice;
> +
> +size_t extra_information_length;
> +uint8_t *extra_information;

Does this get freed? Same for user_data.

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

Re: [libav-devel] [PATCH 10/22] lavc: Add h264_redundant_pps bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:33)
> This applies a specific fixup to some Bluray streams which contain
> redundant PPSs modifying irrelevant parameters of the stream which
> confuse other transformations which require correct extradata.
> 
> A new single global PPS is created, and all of the redundant PPSs
> within the stream are removed.
> ---

Seems no changes here, so still ok

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

Re: [libav-devel] [PATCH 09/22] lavc: Add h264_metadata bitstream filter

[Anton Khirnov]

primary_pic_type_mask &= ~(1 << j);
> +}
> +}
> +}
> +for (j = 0; j < FF_ARRAY_ELEMS(primary_pic_type_table); j++)
> +if (primary_pic_type_mask & (1 << j))
> +break;
> +if (j >= FF_ARRAY_ELEMS(primary_pic_type_table)) {
> +av_log(bsf, AV_LOG_ERROR, "No usable primary_pic_type: "
> +   "invalid slice types?\n");
> +err = AVERROR_INVALIDDATA;
> +goto fail;
> +}
> +
> +aud->nal_unit_header.nal_unit_type = H264_NAL_AUD;
> +aud->primary_pic_type = j;
> +
> +err = ff_cbs_insert_unit_content(>cbc, au,
> + 0, H264_NAL_AUD, aud);
> +if (err) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to insert AUD.\n");
> +goto fail;
> +}
> +}
> +}
> +
> +has_sps = 0;
> +for (i = 0; i < au->nb_units; i++) {
> +if (au->units[i].type == H264_NAL_SPS) {
> +h264_metadata_update_sps(bsf, au->units[i].content);
> +has_sps = 1;
> +}
> +}
> +
> +// Only insert the SEI in access units containing SPSs.
> +if (has_sps && ctx->sei_user_data) {
> +H264RawSEI *sei;
> +H264RawSEIPayload *payload;
> +H264RawSEIUserDataUnregistered *udu;
> +int sei_pos;
> +
> +for (i = 0; i < au->nb_units; i++) {
> +if (au->units[i].type == H264_NAL_SEI ||
> +au->units[i].type == H264_NAL_SLICE ||
> +au->units[i].type == H264_NAL_IDR_SLICE)
> +break;
> +}
> +sei_pos = i;
> +
> +if (sei_pos < au->nb_units &&
> +au->units[sei_pos].type == H264_NAL_SEI) {
> +sei = au->units[sei_pos].content;
> +} else {
> +sei = >sei_nal;
> +memset(sei, 0, sizeof(*sei));
> +
> +sei->nal_unit_header.nal_unit_type = H264_NAL_SEI;
> +
> +err = ff_cbs_insert_unit_content(>cbc, au,
> + sei_pos, H264_NAL_SEI, sei);
> +if (err < 0) {
> +av_log(bsf, AV_LOG_ERROR, "Failed to insert SEI.\n");
> +goto fail;
> +}
> +}
> +
> +payload = >payload[sei->payload_count];
> +
> +payload->payload_type = H264_SEI_TYPE_USER_DATA_UNREGISTERED;
> +udu = >payload.user_data_unregistered;
> +
> +for (i = j = 0; j < 32 && ctx->sei_user_data[i]; i++) {
> +int c, v;
> +c = ctx->sei_user_data[i];
> +if (c == '-') {
> +continue;
> +} else if (av_isxdigit(c)) {
> +  c = av_tolower(c);
> +  v = (c <= '9' ? c - '0' : c - 'a' + 10);
> +} else {
> +  goto invalid_user_data;

weird indentation

Otherwise ok

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

Re: [libav-devel] [PATCH 08/22] lavc: Add trace_headers bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:31)
> +static int trace_headers(AVBSFContext *bsf, AVPacket *out)
> +{
> +TraceHeadersContext *ctx = bsf->priv_data;
> +CodedBitstreamFragment au;
> +AVPacket *in;
> +char tmp[256] = { 0 };
> +int err;
> +
> +err = ff_bsf_get_packet(bsf, );
> +if (err < 0)
> +return err;
> +
> +if (in->flags & AV_PKT_FLAG_KEY)
> +av_strlcat(tmp, ", key frame", sizeof(tmp));
> +if (in->flags & AV_PKT_FLAG_CORRUPT)
> +av_strlcat(tmp, ", corrupt", sizeof(tmp));
> +
> +if (in->pts != AV_NOPTS_VALUE)
> +av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, in->pts);

since you're going all out here, might as well print 'nopts' for
AV_NOPTS_VALUE

Otherwise looks ok.

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

Re: [libav-devel] [PATCH 06/22] lavc: Add coded bitstream read/write support for H.265

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:29)
> ---
> * NAL unit header extracted to a separate structure.
> * VPS no longer required (for patch 18).
> * Inferred prediction weights fixed.
> 
> 
>  libavcodec/cbs.c |1 +
>  libavcodec/cbs_h2645.c   |  410 +++-
>  libavcodec/cbs_h265.h|  537 +++
>  libavcodec/cbs_h265_syntax.c | 1502 
> ++
>  libavcodec/cbs_internal.h|1 +
>  5 files changed, 2448 insertions(+), 3 deletions(-)
>  create mode 100644 libavcodec/cbs_h265.h
>  create mode 100644 libavcodec/cbs_h265_syntax.c

Ok

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

Re: [libav-devel] [PATCH 05/22] lavc: Add coded bitstream read/write support for H.264

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:28)
> ---
>  libavcodec/cbs.c |1 +
>  libavcodec/cbs_h264.h|  427 +++
>  libavcodec/cbs_h2645.c   |  800 +++
>  libavcodec/cbs_h2645.h   |   43 ++
>  libavcodec/cbs_h264_syntax.c | 1230 
> ++
>  libavcodec/cbs_internal.h|3 +
>  6 files changed, 2504 insertions(+)
>  create mode 100644 libavcodec/cbs_h264.h
>  create mode 100644 libavcodec/cbs_h2645.c
>  create mode 100644 libavcodec/cbs_h2645.h
>  create mode 100644 libavcodec/cbs_h264_syntax.c

ok

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

Re: [libav-devel] [PATCH 04/22] lavc: Add coded bitstream read/write API

[Anton Khirnov]

Quoting Mark Thompson (2017-07-29 23:16:27)
> ---
>  libavcodec/cbs.c  | 656 
> ++
>  libavcodec/cbs.h  | 274 +++
>  libavcodec/cbs_internal.h |  99 +++
>  3 files changed, 1029 insertions(+)
>  create mode 100644 libavcodec/cbs.c
>  create mode 100644 libavcodec/cbs.h
>  create mode 100644 libavcodec/cbs_internal.h
> 

This one looks unchanged, so still ok

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

Re: [libav-devel] [PATCH 1/2 V4] libavfilter/overlay_qsv: Add QSV overlay vpp filter

[Anton Khirnov]

55);
>+st->PixelAlphaEnable  = have_alpha_planar(inlink);
>+
>+return 0;
>+}
>+
>+static int config_output(AVFilterLink *outlink)
>+{
>+AVFilterContext   *ctx = outlink->src;
>+QSVOverlayContext *vpp = ctx->priv;
>+AVFilterLink  *inlink = ctx->inputs[0];
>+
>+av_log(ctx, AV_LOG_DEBUG, "Output is of %s.\n", 
>av_get_pix_fmt_name(outlink->format));
>+
>+outlink->w  = vpp->var_values[VAR_MW];
>+outlink->h  = vpp->var_values[VAR_MH];
>+outlink->frame_rate = inlink->frame_rate;
>+outlink->time_base  = av_inv_q(outlink->frame_rate);
>+
>+return ff_qsvvpp_create(ctx, >qsv, >qsv_param);
>+}
>+
>+static int blend_frame(AVFilterContext *ctx,
>+AVFrame *mpic, AVFrame *opic)

broken indentation

>+{
>+intret = 0;
>+QSVOverlayContext *vpp = ctx->priv;
>+AVFrame *opic_copy = NULL;
>+
>+ret = ff_qsvvpp_filter_frame(vpp->qsv, ctx->inputs[0], mpic);
>+if (ret == 0 || ret == AVERROR(EAGAIN)) {
>+/**
>+ * Make a copy of the overlay frame. Because:

It's not a copy, it's a new reference IIUC. The data remains the same, right?

>+static int request_frame(AVFilterLink *outlink)
>+{
>+AVFilterContext *ctx = outlink->src;
>+QSVOverlayContext *s = ctx->priv;
>+AVRational   tb_main = ctx->inputs[MAIN]->time_base;
>+AVRational   tb_over = ctx->inputs[OVERLAY]->time_base;
>+int  ret = 0;
>+
>+/* get a frame on the main input */
>+if (!s->main) {
>+ret = ff_request_frame(ctx->inputs[MAIN]);
>+if (ret < 0)
>+return ret;
>+}
>+
>+/* get a new frame on the overlay input, on EOF check setting 
>'eof_action' */
>+if (!s->over_next) {
>+ret = ff_request_frame(ctx->inputs[OVERLAY]);
>+if (ret == AVERROR_EOF)
>+   return handle_overlay_eof(ctx);
>+else if (ret < 0)
>+return ret;
>+}
>+
>+while (s->main->pts != AV_NOPTS_VALUE &&
>+   s->over_next->pts != AV_NOPTS_VALUE &&
>+   av_compare_ts(s->over_next->pts, tb_over, s->main->pts, tb_main) < 
>0) {
>+av_frame_free(>over_prev);
>+FFSWAP(AVFrame*, s->over_prev, s->over_next);
>+
>+ret = ff_request_frame(ctx->inputs[OVERLAY]);
>+if (ret == AVERROR_EOF)
>+return handle_overlay_eof(ctx);
>+else if (ret < 0)
>+return ret;
>+}
>+
>+if (s->main->pts == AV_NOPTS_VALUE ||
>+s->over_next->pts == AV_NOPTS_VALUE ||
>+!av_compare_ts(s->over_next->pts, tb_over, s->main->pts, tb_main)) {
>+ret = blend_frame(ctx, s->main, s->over_next);
>+av_frame_free(>over_prev);
>+FFSWAP(AVFrame*, s->over_prev, s->over_next);
>+} else if (s->over_prev) {
>+ret = blend_frame(ctx, s->main, s->over_prev);
>+} else {
>+av_frame_free(>main);
>+ret = AVERROR(EAGAIN);
>+}
>+
>+s->main = NULL;
>+
>+return ret ;
>+}

This duplication of nontrivial code with vf_overlay is suboptimal. Would it be
hard to share?

>+
>+static int filter_frame_main(AVFilterLink *inlink, AVFrame *frame)
>+{
>+QSVOverlayContext *s = inlink->dst->priv;
>+
>+av_assert0(!s->main);
>+s->main = frame;
>+
>+return 0;
>+}
>+
>+static int filter_frame_overlay(AVFilterLink *inlink, AVFrame *frame)
>+{
>+QSVOverlayContext *s = inlink->dst->priv;
>+
>+av_assert0(!s->over_next);
>+s->over_next = frame;
>+
>+return 0;
>+}
>+
>+static int overlay_qsv_init(AVFilterContext *ctx)
>+{
>+QSVOverlayContext *vpp = ctx->priv;
>+
>+/* fill composite config */
>+vpp->comp_conf.Header.BufferId = MFX_EXTBUFF_VPP_COMPOSITE;
>+vpp->comp_conf.Header.BufferSz = sizeof(vpp->comp_conf);
>+vpp->comp_conf.NumInputStream  = ctx->nb_inputs;
>+vpp->comp_conf.InputStream =
>+av_mallocz(sizeof(*vpp->comp_conf.InputStream) * ctx->nb_inputs);

nit: av_mallocz_array()

>+if (!vpp->comp_conf.InputStream)
>+return AVERROR(ENOMEM);
>+
>+/* initialize QSVVPP params */
>+vpp->qsv_param.filter_frame = NULL;
>+vpp->qsv_param.ext_buf  = av_mallocz(sizeof(*vpp->qsv_param.ext_buf));
>+if (!vpp->qsv_param.ext_buf)
>+return AVERROR(ENOMEM);
>+
>+vpp->qsv_param.ext_buf[0]= (mfxExtBuffer *)>comp_conf;
>+vpp->qsv_param.num_ext_buf   = 1;
>+vpp->qsv_param.out_sw_format = AV_PIX_FMT_NV12;
>+vpp->qsv_param.num_crop  = 0;
>+
>+return 0;
>+}
>+
>+static void overlay_qsv_uninit(AVFilterContext *ctx)
>+{
>+QSVOverlayContext *vpp = ctx->priv;
>+
>+av_frame_free(>main);
>+av_frame_free(>over_prev);
>+av_frame_free(>over_next);
>+ff_qsvvpp_free(>qsv);
>+av_freep(>comp_conf.InputStream);
>+av_freep(>qsv_param.ext_buf);
>+}
>+
>+static int overlay_qsv_query_formats(AVFilterContext *ctx)
>+{
>+int i;
>+
>+static const enum AVPixelFormat main_in_fmts[] = {
>+AV_PIX_FMT_YUV420P,
>+AV_PIX_FMT_NV12,
>+AV_PIX_FMT_YUYV422,
>+AV_PIX_FMT_RGB32,
>+AV_PIX_FMT_QSV,
>+AV_PIX_FMT_NONE
>+};
>+static const enum AVPixelFormat out_pix_fmts[] = {
>+AV_PIX_FMT_NV12,
>+AV_PIX_FMT_QSV,
>+AV_PIX_FMT_NONE
>+};
>+
>+for(i = 0; i < ctx->nb_inputs; i++)

missing space

>+AVFilter ff_vf_overlay_qsv = {
>+.name  = "overlay_qsv",
>+.description   = NULL_IF_CONFIG_SMALL("Quick Sync Video overlay."),
>+.priv_size = sizeof(QSVOverlayContext),
>+.query_formats = overlay_qsv_query_formats,
>+.init  = overlay_qsv_init,
>+.uninit= overlay_qsv_uninit,
>+.inputs= overlay_qsv_inputs,
>+.outputs   = overlay_qsv_outputs,
>+.priv_class= _qsv_class,
>+   .flags_internal = FF_FILTER_FLAG_HWFRAME_AWARE,

broken indentation


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

Re: [libav-devel] [PATCH] avconv: Do not issue a warning if no specific devices are selected

[Anton Khirnov]

Quoting Luca Barbato (2017-07-21 00:53:18)
> The hardware encoders currently do pick the first compatible device
> available by themselves.

I don't think this is true for all encoders. Perhaps what we should do
instead is auto-create the device like we do for decoding.

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

[libav-devel] [PATCH] cuvid: add cuvid.h to SKIPHEADERS

[Anton Khirnov]

---
 libavcodec/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index ac13166..eba651f 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -797,6 +797,7 @@ SKIPHEADERS+= %_tablegen.h  
\
   tableprint.h  \
   $(ARCH)/vp56_arith.h  \
 
+SKIPHEADERS-$(CONFIG_CUVID)+= cuvid.h
 SKIPHEADERS-$(CONFIG_D3D11VA)  += d3d11va.h dxva2_internal.h
 SKIPHEADERS-$(CONFIG_DXVA2)+= dxva2.h dxva2_internal.h
 SKIPHEADERS-$(CONFIG_LIBSCHROEDINGER)  += libschroedinger.h
-- 
2.0.0

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

Re: [libav-devel] [PATCH v3 1/2] lavc, lavu: move frame cropping to a convenience function

[Anton Khirnov]

Both pushed, thanks.

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

Re: [libav-devel] [PATCH] h264dec: add a CUVID hwaccel

[Anton Khirnov]

Quoting Sean McGovern (2017-07-27 07:42:29)
> Hi Anton,
> 
> On Mon, Jul 24, 2017 at 12:50 PM, Anton Khirnov <an...@khirnov.net> wrote:
> > Quoting Luca Barbato (2017-07-24 18:21:20)
> >> On 24/07/2017 15:15, Anton Khirnov wrote:
> >> > Some parts of the code are based on a patch by
> >> > Timo Rothenpieler <t...@rothenpieler.org>
> >> > ---
> >> > Now with high bit depth support
> >> > ---
> >>
> 
> Looking at FATE, this may need a proper header guard -- or should the
> entire module be disabled on systems that don't have CUVID (like my
> Solaris instances)?

What part of FATE? I don't see anything related to this set.

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

[libav-devel] [PATCH 4/4] h264dec: Fix mix of lossless and lossy MBs decoding

[Anton Khirnov]

From: Anton Mitrofanov <bugmas...@narod.ru>

CC: libav-sta...@libav.org

Signed-off-by: Anton Khirnov <an...@khirnov.net>
---
 libavcodec/h264_cabac.c | 16 
 libavcodec/h264_cavlc.c | 16 
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c
index 5dd285c..c0b9e30 100644
--- a/libavcodec/h264_cabac.c
+++ b/libavcodec/h264_cabac.c
@@ -2371,14 +2371,6 @@ decode_intra_mb:
 const uint8_t *scan, *scan8x8;
 const uint32_t *qmul;
 
-if(IS_INTERLACED(mb_type)){
-scan8x8 = sl->qscale ? h->field_scan8x8 : h->field_scan8x8_q0;
-scan= sl->qscale ? h->field_scan : h->field_scan_q0;
-}else{
-scan8x8 = sl->qscale ? h->zigzag_scan8x8 : h->zigzag_scan8x8_q0;
-scan= sl->qscale ? h->zigzag_scan : h->zigzag_scan_q0;
-}
-
 // decode_cabac_mb_dqp
 if(get_cabac_noinline( >cabac, >cabac_state[60 + 
(sl->last_qscale_diff != 0)])){
 int val = 1;
@@ -2409,6 +2401,14 @@ decode_intra_mb:
 }else
 sl->last_qscale_diff=0;
 
+if(IS_INTERLACED(mb_type)){
+scan8x8 = sl->qscale ? h->field_scan8x8 : h->field_scan8x8_q0;
+scan= sl->qscale ? h->field_scan : h->field_scan_q0;
+}else{
+scan8x8 = sl->qscale ? h->zigzag_scan8x8 : h->zigzag_scan8x8_q0;
+scan= sl->qscale ? h->zigzag_scan : h->zigzag_scan_q0;
+}
+
 decode_cabac_luma_residual(h, sl, scan, scan8x8, pixel_shift, mb_type, 
cbp, 0);
 if (CHROMA444(h)) {
 decode_cabac_luma_residual(h, sl, scan, scan8x8, pixel_shift, 
mb_type, cbp, 1);
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index c11e211..d57062b 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -1093,14 +1093,6 @@ decode_intra_mb:
 const uint8_t *scan, *scan8x8;
 const int max_qp = 51 + 6 * (h->ps.sps->bit_depth_luma - 8);
 
-if(IS_INTERLACED(mb_type)){
-scan8x8 = sl->qscale ? h->field_scan8x8_cavlc : 
h->field_scan8x8_cavlc_q0;
-scan= sl->qscale ? h->field_scan : h->field_scan_q0;
-}else{
-scan8x8 = sl->qscale ? h->zigzag_scan8x8_cavlc : 
h->zigzag_scan8x8_cavlc_q0;
-scan= sl->qscale ? h->zigzag_scan : h->zigzag_scan_q0;
-}
-
 dquant= get_se_golomb(>gb);
 
 sl->qscale += dquant;
@@ -1117,6 +1109,14 @@ decode_intra_mb:
 sl->chroma_qp[0] = get_chroma_qp(h->ps.pps, 0, sl->qscale);
 sl->chroma_qp[1] = get_chroma_qp(h->ps.pps, 1, sl->qscale);
 
+if(IS_INTERLACED(mb_type)){
+scan8x8 = sl->qscale ? h->field_scan8x8_cavlc : 
h->field_scan8x8_cavlc_q0;
+scan= sl->qscale ? h->field_scan : h->field_scan_q0;
+}else{
+scan8x8 = sl->qscale ? h->zigzag_scan8x8_cavlc : 
h->zigzag_scan8x8_cavlc_q0;
+scan= sl->qscale ? h->zigzag_scan : h->zigzag_scan_q0;
+}
+
 if ((ret = decode_luma_residual(h, sl, gb, scan, scan8x8, pixel_shift, 
mb_type, cbp, 0)) < 0 ) {
 return -1;
 }
-- 
2.0.0

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

[libav-devel] [PATCH 3/4] h264_cabac: Fix CABAC+8x8dct in 4:4:4

[Anton Khirnov]

From: Anton Mitrofanov <bugmas...@narod.ru>

Use the correct ctxIdxInc calculation for coded_block_flag.
Keep old behavior for old versions of x264 for backward compatibility.

CC: libav-sta...@libav.org

Signed-off-by: Anton Khirnov <an...@khirnov.net>
---
 libavcodec/h264_cabac.c | 47 +--
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/libavcodec/h264_cabac.c b/libavcodec/h264_cabac.c
index b28e486..5dd285c 100644
--- a/libavcodec/h264_cabac.c
+++ b/libavcodec/h264_cabac.c
@@ -2329,21 +2329,40 @@ decode_intra_mb:
 if (CHROMA444(h) && IS_8x8DCT(mb_type)){
 int i;
 uint8_t *nnz_cache = sl->non_zero_count_cache;
-for (i = 0; i < 2; i++){
-if (sl->left_type[LEFT(i)] && !IS_8x8DCT(sl->left_type[LEFT(i)])) {
-nnz_cache[3+8* 1 + 2*8*i]=
-nnz_cache[3+8* 2 + 2*8*i]=
-nnz_cache[3+8* 6 + 2*8*i]=
-nnz_cache[3+8* 7 + 2*8*i]=
-nnz_cache[3+8*11 + 2*8*i]=
-nnz_cache[3+8*12 + 2*8*i]= IS_INTRA(mb_type) ? 64 : 0;
+if (h->x264_build < 151U) {
+for (i = 0; i < 2; i++){
+if (sl->left_type[LEFT(i)] && 
!IS_8x8DCT(sl->left_type[LEFT(i)])) {
+nnz_cache[3+8* 1 + 2*8*i]=
+nnz_cache[3+8* 2 + 2*8*i]=
+nnz_cache[3+8* 6 + 2*8*i]=
+nnz_cache[3+8* 7 + 2*8*i]=
+nnz_cache[3+8*11 + 2*8*i]=
+nnz_cache[3+8*12 + 2*8*i]= IS_INTRA(mb_type) ? 64 : 0;
+}
+}
+if (sl->top_type && !IS_8x8DCT(sl->top_type)){
+uint32_t top_empty = !IS_INTRA(mb_type) ? 0 : 0x40404040;
+AV_WN32A(_cache[4+8* 0], top_empty);
+AV_WN32A(_cache[4+8* 5], top_empty);
+AV_WN32A(_cache[4+8*10], top_empty);
+}
+} else {
+for (i = 0; i < 2; i++){
+if (sl->left_type[LEFT(i)] && 
!IS_8x8DCT(sl->left_type[LEFT(i)])) {
+nnz_cache[3+8* 1 + 2*8*i]=
+nnz_cache[3+8* 2 + 2*8*i]=
+nnz_cache[3+8* 6 + 2*8*i]=
+nnz_cache[3+8* 7 + 2*8*i]=
+nnz_cache[3+8*11 + 2*8*i]=
+nnz_cache[3+8*12 + 2*8*i]= 
!IS_INTRA_PCM(sl->left_type[LEFT(i)]) ? 0 : 64;
+}
+}
+if (sl->top_type && !IS_8x8DCT(sl->top_type)){
+uint32_t top_empty = !IS_INTRA_PCM(sl->top_type) ? 0 : 
0x40404040;
+AV_WN32A(_cache[4+8* 0], top_empty);
+AV_WN32A(_cache[4+8* 5], top_empty);
+AV_WN32A(_cache[4+8*10], top_empty);
 }
-}
-if (sl->top_type && !IS_8x8DCT(sl->top_type)){
-uint32_t top_empty = !IS_INTRA(mb_type) ? 0 : 0x40404040;
-AV_WN32A(_cache[4+8* 0], top_empty);
-AV_WN32A(_cache[4+8* 5], top_empty);
-AV_WN32A(_cache[4+8*10], top_empty);
 }
 }
 h->cur_pic.mb_type[mb_xy] = mb_type;
-- 
2.0.0

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

[libav-devel] [PATCH 1/4] h264dec: track the last seen value of x264_build

[Anton Khirnov]

Do not use the one in the SEI directly as that is reset at certain
points.

Inspired by patches from Michael Niedermayer  and
Anton Mitrofanov .

CC: libav-sta...@libav.org
---
 libavcodec/h264_direct.c | 4 ++--
 libavcodec/h264_slice.c  | 6 +-
 libavcodec/h264dec.c | 1 +
 libavcodec/h264dec.h | 1 +
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavcodec/h264_direct.c b/libavcodec/h264_direct.c
index 7ec49b6..abac259 100644
--- a/libavcodec/h264_direct.c
+++ b/libavcodec/h264_direct.c
@@ -391,7 +391,7 @@ single_col:
  (l1ref0[0] < 0 && !l1ref1[0] &&
   FFABS(l1mv1[0][0]) <= 1 &&
   FFABS(l1mv1[0][1]) <= 1 &&
-  h->sei.unregistered.x264_build > 33U))) {
+  h->x264_build > 33U))) {
 a = b = 0;
 if (ref[0] > 0)
 a = mv[0];
@@ -426,7 +426,7 @@ single_col:
 (l1ref0[i8] == 0 ||
  (l1ref0[i8] < 0 &&
   l1ref1[i8] == 0 &&
-  h->sei.unregistered.x264_build > 33U))) {
+  h->x264_build > 33U))) {
 const int16_t (*l1mv)[2] = l1ref0[i8] == 0 ? l1mv0 : l1mv1;
 if (IS_SUB_8X8(sub_mb_type)) {
 const int16_t *mv_col = l1mv[x8 * 3 + y8 * 3 * b4_stride];
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c9f1dbb..e7408b2 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -403,6 +403,7 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
 
 h->enable_er   = h1->enable_er;
 h->workaround_bugs = h1->workaround_bugs;
+h->x264_build  = h1->x264_build;
 h->droppable   = h1->droppable;
 
 // extradata/NAL handling
@@ -509,6 +510,9 @@ static int h264_frame_start(H264Context *h)
 
 h->mb_aff_frame = h->ps.sps->mb_aff && (h->picture_structure == 
PICT_FRAME);
 
+if (h->sei.unregistered.x264_build >= 0)
+h->x264_build = h->sei.unregistered.x264_build;
+
 assert(h->cur_pic_ptr->long_ref == 0);
 
 return 0;
@@ -847,7 +851,7 @@ static int h264_slice_header_init(H264Context *h)
 
 if (sps->timing_info_present_flag) {
 int64_t den = sps->time_scale;
-if (h->sei.unregistered.x264_build < 44U)
+if (h->x264_build < 44U)
 den *= 2;
 av_reduce(>avctx->framerate.den, >avctx->framerate.num,
   sps->num_units_in_tick, den, 1 << 30);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 2a532a7..7a8293e 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -293,6 +293,7 @@ static int h264_init_context(AVCodecContext *avctx, 
H264Context *h)
 h->flags = avctx->flags;
 h->poc.prev_poc_msb  = 1 << 16;
 h->recovery_frame= -1;
+h->x264_build= -1;
 h->frame_recovered   = 0;
 
 h->next_outputed_poc = INT_MIN;
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index fc7beeb..ddfe224 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -361,6 +361,7 @@ typedef struct H264Context {
 int context_initialized;
 int flags;
 int workaround_bugs;
+int x264_build;
 /* Set when slice threading is used and at least one slice uses deblocking
  * mode 1 (i.e. across slice boundaries). Then we disable the loop filter
  * during normal MB decoding and execute it serially at the end.
-- 
2.0.0

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

[libav-devel] [PATCH 2/4] h264dec: fix Lossless Decoding (Profile 244) for 8x8 Intra Prediction

[Anton Khirnov]

From: Yogender Kumar Gupta <yogender.gu...@gmail.com>

CC: libav-sta...@libav.org

Signed-off-by: Anton Khirnov <an...@khirnov.net>
---
 libavcodec/h264_mb.c   |  7 +++-
 libavcodec/h264pred.c  |  2 ++
 libavcodec/h264pred.h  |  3 ++
 libavcodec/h264pred_template.c | 73 ++
 4 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/libavcodec/h264_mb.c b/libavcodec/h264_mb.c
index f037bd5..51d73ce 100644
--- a/libavcodec/h264_mb.c
+++ b/libavcodec/h264_mb.c
@@ -636,7 +636,12 @@ static av_always_inline void 
hl_decode_mb_predict_luma(const H264Context *h,
 uint8_t *const ptr = dest_y + block_offset[i];
 const int dir  = sl->intra4x4_pred_mode_cache[scan8[i]];
 if (transform_bypass && h->ps.sps->profile_idc == 244 && dir 
<= 1) {
-h->hpc.pred8x8l_add[dir](ptr, sl->mb + (i * 16 + p * 256 
<< pixel_shift), linesize);
+if (h->x264_build < 151U) {
+h->hpc.pred8x8l_add[dir](ptr, sl->mb + (i * 16 + p * 
256 << pixel_shift), linesize);
+} else
+h->hpc.pred8x8l_filter_add[dir](ptr, sl->mb + (i * 16 
+ p * 256 << pixel_shift),
+(sl-> 
topleft_samples_available << i) & 0x8000,
+
(sl->topright_samples_available << i) & 0x4000, linesize);
 } else {
 const int nnz = sl->non_zero_count_cache[scan8[i + p * 
16]];
 h->hpc.pred8x8l[dir](ptr, (sl->topleft_samples_available 
<< i) & 0x8000,
diff --git a/libavcodec/h264pred.c b/libavcodec/h264pred.c
index 7627eb0..135babc 100644
--- a/libavcodec/h264pred.c
+++ b/libavcodec/h264pred.c
@@ -552,6 +552,8 @@ av_cold void ff_h264_pred_init(H264PredContext *h, int 
codec_id,
 h->pred4x4_add  [ HOR_PRED   ]= FUNCC(pred4x4_horizontal_add  , 
depth);\
 h->pred8x8l_add [VERT_PRED   ]= FUNCC(pred8x8l_vertical_add   , 
depth);\
 h->pred8x8l_add [ HOR_PRED   ]= FUNCC(pred8x8l_horizontal_add , 
depth);\
+h->pred8x8l_filter_add [VERT_PRED   ]= FUNCC(pred8x8l_vertical_filter_add  
 , depth);\
+h->pred8x8l_filter_add [ HOR_PRED   ]= 
FUNCC(pred8x8l_horizontal_filter_add , depth);\
 if (chroma_format_idc <= 1) {\
 h->pred8x8_add  [VERT_PRED8x8]= FUNCC(pred8x8_vertical_add, 
depth);\
 h->pred8x8_add  [ HOR_PRED8x8]= FUNCC(pred8x8_horizontal_add  , 
depth);\
diff --git a/libavcodec/h264pred.h b/libavcodec/h264pred.h
index 60e74349..795d8f3 100644
--- a/libavcodec/h264pred.h
+++ b/libavcodec/h264pred.h
@@ -101,6 +101,9 @@ typedef struct H264PredContext {
   int16_t *block /*align 16*/, ptrdiff_t stride);
 void(*pred8x8l_add[2])(uint8_t *pix /*align  8*/,
int16_t *block /*align 16*/, ptrdiff_t stride);
+void(*pred8x8l_filter_add[2])(uint8_t *pix /*align  8*/,
+  int16_t *block /*align 16*/,
+  int topleft, int topright, ptrdiff_t stride);
 void(*pred8x8_add[3])(uint8_t *pix /*align  8*/,
   const int *block_offset,
   int16_t *block /*align 16*/, ptrdiff_t stride);
diff --git a/libavcodec/h264pred_template.c b/libavcodec/h264pred_template.c
index 8492b2b..02494aa 100644
--- a/libavcodec/h264pred_template.c
+++ b/libavcodec/h264pred_template.c
@@ -1123,6 +1123,79 @@ static void FUNCC(pred8x8l_horizontal_up)(uint8_t *_src, 
int has_topleft,
 SRC(5,6)=SRC(5,7)=SRC(6,4)=SRC(6,5)=SRC(6,6)=
 SRC(6,7)=SRC(7,4)=SRC(7,5)=SRC(7,6)=SRC(7,7)= l7;
 }
+
+static void FUNCC(pred8x8l_vertical_filter_add)(uint8_t *_src, int16_t 
*_block, int has_topleft,
+int has_topright, ptrdiff_t 
_stride)
+{
+int i;
+pixel *src = (pixel*)_src;
+const dctcoef *block = (const dctcoef*)_block;
+pixel pix[8];
+int stride = _stride/sizeof(pixel);
+PREDICT_8x8_LOAD_TOP;
+
+pix[0] = t0;
+pix[1] = t1;
+pix[2] = t2;
+pix[3] = t3;
+pix[4] = t4;
+pix[5] = t5;
+pix[6] = t6;
+pix[7] = t7;
+
+for (i = 0; i < 8; i++) {
+pixel v = pix[i];
+src[0 * stride] = v += block[0];
+src[1 * stride] = v += block[8];
+src[2 * stride] = v += block[16];
+src[3 * stride] = v += block[24];
+src[4 * stride] = v += block[32];
+src[5 * stride] = v += block[40];
+src[6 * stride] = v += block[48];
+src[7 * stride] = v +  block[56];
+src++;
+block++;
+}
+
+memset(_block, 0, sizeof(dctcoef) * 64);
+}
+
+static void FUNCC(pred8x8l_horizontal_filter_add

Re: [libav-devel] [PATCH 06/15] lavc: Add coded bitstream read/write support for H.265

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:12)
> ---
>  libavcodec/cbs.c |1 +
>  libavcodec/cbs_h2645.c   |  410 +++-
>  libavcodec/cbs_h265.h|  544 
>  libavcodec/cbs_h265_syntax.c | 1482 
> ++
>  libavcodec/cbs_internal.h|1 +
>  5 files changed, 2435 insertions(+), 3 deletions(-)
>  create mode 100644 libavcodec/cbs_h265.h
>  create mode 100644 libavcodec/cbs_h265_syntax.c
> 

Looks okish from a quick look

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

Re: [libav-devel] [PATCH] h264dec: add a CUVID hwaccel

[Anton Khirnov]

Quoting Luca Barbato (2017-07-24 18:21:20)
> On 24/07/2017 15:15, Anton Khirnov wrote:
> > Some parts of the code are based on a patch by
> > Timo Rothenpieler <t...@rothenpieler.org>
> > ---
> > Now with high bit depth support
> > ---
> 
> I recently updated the nvidia-video-codec distribution to the version 8,
> I guess that's needed for that, isn't it?

To get 10bit decoding yes. 8bit should still work with older versions of
the header/derivers.

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

[libav-devel] [PATCH] hevcdec: add a CUVID hwaccel

[Anton Khirnov]

---
Now with 10bit decoding
---
 Changelog   |   2 +-
 configure   |   3 +
 libavcodec/Makefile |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/cuvid.c  |   1 +
 libavcodec/cuvid_hevc.c | 280 
 libavcodec/hevcdec.c|   9 +-
 7 files changed, 295 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/cuvid_hevc.c

diff --git a/Changelog b/Changelog
index f3c8f7a..59a2eba 100644
--- a/Changelog
+++ b/Changelog
@@ -17,7 +17,7 @@ version :
 - ClearVideo decoder (I-frames only)
 - support for decoding through D3D11VA in avconv
 - Cinepak encoder
-- NVIDIA CUVID-accelerated H.264 decoding
+- NVIDIA CUVID-accelerated H.264 and HEVC decoding
 
 
 version 12:
diff --git a/configure b/configure
index 0eeb46b..23e0ab6 100755
--- a/configure
+++ b/configure
@@ -2210,6 +2210,8 @@ h264_vda_old_hwaccel_deps="vda"
 h264_vda_old_hwaccel_select="h264_decoder"
 h264_vdpau_hwaccel_deps="vdpau"
 h264_vdpau_hwaccel_select="h264_decoder"
+hevc_cuvid_hwaccel_deps="cuvid CUVIDHEVCPICPARAMS"
+hevc_cuvid_hwaccel_select="hevc_decoder"
 hevc_d3d11va_hwaccel_deps="d3d11va DXVA_PicParams_HEVC"
 hevc_d3d11va_hwaccel_select="hevc_decoder"
 hevc_d3d11va2_hwaccel_deps="d3d11va DXVA_PicParams_HEVC"
@@ -4698,6 +4700,7 @@ check_lib psapi"windows.h psapi.h"
GetProcessMemoryInfo -lpsapi
 check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
 
 check_type "cuviddec.h" "CUVIDH264PICPARAMS"
+check_type "cuviddec.h" "CUVIDHEVCPICPARAMS"
 check_struct "cuviddec.h" "CUVIDDECODECREATEINFO" bitDepthMinus8
 
 check_type "windows.h dxva.h" "DXVA_PicParams_HEVC" 
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -D_CRT_BUILD_DESKTOP_APP=0
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 66f6f9e..12c8678 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -640,6 +640,7 @@ OBJS-$(CONFIG_H264_QSV_HWACCEL)   += qsvdec_h2645.o
 OBJS-$(CONFIG_H264_VAAPI_HWACCEL) += vaapi_h264.o
 OBJS-$(CONFIG_H264_VDA_HWACCEL)   += vda_h264.o
 OBJS-$(CONFIG_H264_VDPAU_HWACCEL) += vdpau_h264.o
+OBJS-$(CONFIG_HEVC_CUVID_HWACCEL) += cuvid_hevc.o
 OBJS-$(CONFIG_HEVC_D3D11VA_HWACCEL)   += dxva2_hevc.o
 OBJS-$(CONFIG_HEVC_DXVA2_HWACCEL) += dxva2_hevc.o
 OBJS-$(CONFIG_HEVC_QSV_HWACCEL)   += qsvdec_h2645.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 97b8810..717e18f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -78,6 +78,7 @@ void avcodec_register_all(void)
 REGISTER_HWACCEL(H264_VDA,  h264_vda);
 REGISTER_HWACCEL(H264_VDA_OLD,  h264_vda_old);
 REGISTER_HWACCEL(H264_VDPAU,h264_vdpau);
+REGISTER_HWACCEL(HEVC_CUVID,hevc_cuvid);
 REGISTER_HWACCEL(HEVC_D3D11VA,  hevc_d3d11va);
 REGISTER_HWACCEL(HEVC_D3D11VA2, hevc_d3d11va2);
 REGISTER_HWACCEL(HEVC_DXVA2,hevc_dxva2);
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 69f624c..2d35e92 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -53,6 +53,7 @@ static int map_avcodec_id(enum AVCodecID id)
 {
 switch (id) {
 case AV_CODEC_ID_H264: return cudaVideoCodec_H264;
+case AV_CODEC_ID_HEVC: return cudaVideoCodec_HEVC;
 }
 return -1;
 }
diff --git a/libavcodec/cuvid_hevc.c b/libavcodec/cuvid_hevc.c
new file mode 100644
index 000..5de9bca
--- /dev/null
+++ b/libavcodec/cuvid_hevc.c
@@ -0,0 +1,280 @@
+/*
+ * HEVC HW decode acceleration through CUVID
+ *
+ * Copyright (c) 2017 Anton Khirnov
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+#include 
+
+#include "avcodec.h"
+#include "cuvid.h"
+#include "decode.h"
+#include "internal.h"
+#include "hevcdec.h"
+#include "hevc_data.h"
+
+static void dpb_add(CUVIDHEVCPICPARAMS *pp, int idx, const HEVCFrame *src)
+{
+FrameDecodeData *fdd = (FrameDecodeData*)src->frame->opaque_ref->data;
+const CUVIDFrame *cf = fdd->hwaccel_priv;
+
+pp->RefPic

[libav-devel] [PATCH] h264dec: add a CUVID hwaccel

[Anton Khirnov]

e_header frei0r.h
 enabled gnutls&& require_pkg_config gnutls gnutls gnutls/gnutls.h 
gnutls_global_init
 enabled libbs2b   && require_pkg_config libbs2b libbs2b bs2b.h 
bs2b_open
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 2b91588..66f6f9e 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -625,6 +625,7 @@ OBJS-$(CONFIG_ADPCM_YAMAHA_DECODER)   += adpcm.o 
adpcm_data.o
 OBJS-$(CONFIG_ADPCM_YAMAHA_ENCODER)   += adpcmenc.o adpcm_data.o
 
 # hardware accelerators
+OBJS-$(CONFIG_CUVID)  += cuvid.o
 OBJS-$(CONFIG_D3D11VA)+= dxva2.o
 OBJS-$(CONFIG_DXVA2)  += dxva2.o
 OBJS-$(CONFIG_VAAPI)  += vaapi_decode.o
@@ -632,6 +633,7 @@ OBJS-$(CONFIG_VDA)+= vda.o
 OBJS-$(CONFIG_VDPAU)  += vdpau.o
 
 OBJS-$(CONFIG_H263_VAAPI_HWACCEL) += vaapi_mpeg4.o
+OBJS-$(CONFIG_H264_CUVID_HWACCEL) += cuvid_h264.o
 OBJS-$(CONFIG_H264_D3D11VA_HWACCEL)   += dxva2_h264.o
 OBJS-$(CONFIG_H264_DXVA2_HWACCEL) += dxva2_h264.o
 OBJS-$(CONFIG_H264_QSV_HWACCEL)   += qsvdec_h2645.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 5635ae1..97b8810 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -68,6 +68,7 @@ void avcodec_register_all(void)
 
 /* hardware accelerators */
 REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
+REGISTER_HWACCEL(H264_CUVID,h264_cuvid);
 REGISTER_HWACCEL(H264_D3D11VA,  h264_d3d11va);
 REGISTER_HWACCEL(H264_D3D11VA2, h264_d3d11va2);
 REGISTER_HWACCEL(H264_DXVA2,h264_dxva2);
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
new file mode 100644
index 000..69f624c
--- /dev/null
+++ b/libavcodec/cuvid.c
@@ -0,0 +1,425 @@
+/*
+ * HW decode acceleration through CUVID
+ *
+ * Copyright (c) 2016 Anton Khirnov
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+
+#include "config.h"
+
+#include "libavutil/common.h"
+#include "libavutil/error.h"
+#include "libavutil/hwcontext.h"
+#include "libavutil/hwcontext_cuda.h"
+#include "libavutil/pixdesc.h"
+#include "libavutil/pixfmt.h"
+
+#include "avcodec.h"
+#include "decode.h"
+#include "cuvid.h"
+#include "internal.h"
+
+typedef struct CUVIDDecoder {
+CUvideodecoder decoder;
+
+AVBufferRef *hw_device_ref;
+CUcontextcuda_ctx;
+} CUVIDDecoder;
+
+typedef struct CUVIDFramePool {
+unsigned int dpb_size;
+unsigned int nb_allocated;
+} CUVIDFramePool;
+
+static int map_avcodec_id(enum AVCodecID id)
+{
+switch (id) {
+case AV_CODEC_ID_H264: return cudaVideoCodec_H264;
+}
+return -1;
+}
+
+static int map_chroma_format(enum AVPixelFormat pix_fmt)
+{
+int shift_h = 0, shift_v = 0;
+
+av_pix_fmt_get_chroma_sub_sample(pix_fmt, _h, _v);
+
+if (shift_h == 1 && shift_v == 1)
+return cudaVideoChromaFormat_420;
+else if (shift_h == 1 && shift_v == 0)
+return cudaVideoChromaFormat_422;
+else if (shift_h == 0 && shift_v == 0)
+return cudaVideoChromaFormat_444;
+
+return -1;
+}
+
+static void cuvid_decoder_free(void *opaque, uint8_t *data)
+{
+CUVIDDecoder *decoder = (CUVIDDecoder*)data;
+
+if (decoder->decoder)
+cuvidDestroyDecoder(decoder->decoder);
+
+av_buffer_unref(>hw_device_ref);
+
+av_freep();
+}
+
+static int cuvid_decoder_create(AVBufferRef **out, AVBufferRef *hw_device_ref,
+CUVIDDECODECREATEINFO *params, void *logctx)
+{
+AVHWDeviceContext  *hw_device_ctx = 
(AVHWDeviceContext*)hw_device_ref->data;
+AVCUDADeviceContext *device_hwctx = hw_device_ctx->hwctx;
+
+AVBufferRef *decoder_ref;
+CUVIDDecoder *decoder;
+
+CUcontext dummy;
+CUresult err;
+int ret;
+
+decoder = av_mallocz(sizeof(*decoder));
+if (!decoder)
+return AVERROR(ENOMEM);
+
+decoder_ref = av_buffer_create((uint8_t*)decoder, sizeof(*decoder),
+   cuvid_decoder_free, NULL, 
AV_BUFFER_FLAG_READONLY);
+if (!deco

Re: [libav-devel] [PATCH 2/3] fate/hevc: specify output pixel format explicitly

[Anton Khirnov]

Quoting Hendrik Leppkes (2017-07-24 12:09:52)
> On Mon, Jul 24, 2017 at 11:46 AM, Anton Khirnov <an...@khirnov.net> wrote:
> > This allows running those tests with hwaccel.
> > ---
> >  tests/fate/hevc.mak | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> > index 5446969..fe3ef26 100644
> > --- a/tests/fate/hevc.mak
> > +++ b/tests/fate/hevc.mak
> > @@ -144,7 +144,7 @@ HEVC_SAMPLES_10BIT =\
> >
> >  define FATE_HEVC_TEST
> >  FATE_HEVC += fate-hevc-conformance-$(1)
> > -fate-hevc-conformance-$(1): CMD = framecrc -vsync 0 -i 
> > $(TARGET_SAMPLES)/hevc-conformance/$(1).bit
> > +fate-hevc-conformance-$(1): CMD = framecrc -vsync 0 -i 
> > $(TARGET_SAMPLES)/hevc-conformance/$(1).bit -pix_fmt yuv420p
> >  endef
> >
> >  define FATE_HEVC_TEST_10BIT
> 
> While you're in here, how about the 10-bit tests as well?

That's already done for 10bit, to get the same endianness everywhere.

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

[libav-devel] [PATCH 1/3] hevcdec: set the active SPS before calling get_format()

[Anton Khirnov]

This way the SPS is available to the hwaccel init code.
---
 libavcodec/hevcdec.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index f6bbb70..664e4ac 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -490,13 +490,14 @@ static int hls_slice_header(HEVCContext *s)
 
 ff_hevc_clear_refs(s);
 
+ret = set_sps(s, sps, sps->pix_fmt);
+if (ret < 0)
+return ret;
+
 pix_fmt = get_format(s, sps);
 if (pix_fmt < 0)
 return pix_fmt;
-
-ret = set_sps(s, sps, pix_fmt);
-if (ret < 0)
-return ret;
+s->avctx->pix_fmt = pix_fmt;
 
 s->seq_decode = (s->seq_decode + 1) & 0xff;
 s->max_ra = INT_MAX;
-- 
2.0.0

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

[libav-devel] [PATCH 2/3] fate/hevc: specify output pixel format explicitly

[Anton Khirnov]

This allows running those tests with hwaccel.
---
 tests/fate/hevc.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 5446969..fe3ef26 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -144,7 +144,7 @@ HEVC_SAMPLES_10BIT =\
 
 define FATE_HEVC_TEST
 FATE_HEVC += fate-hevc-conformance-$(1)
-fate-hevc-conformance-$(1): CMD = framecrc -vsync 0 -i 
$(TARGET_SAMPLES)/hevc-conformance/$(1).bit
+fate-hevc-conformance-$(1): CMD = framecrc -vsync 0 -i 
$(TARGET_SAMPLES)/hevc-conformance/$(1).bit -pix_fmt yuv420p
 endef
 
 define FATE_HEVC_TEST_10BIT
-- 
2.0.0

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

[libav-devel] [PATCH 3/3] hevcdec: add a CUVID hwaccel

[Anton Khirnov]

---
 Changelog   |   2 +-
 configure   |   3 +
 libavcodec/Makefile |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/cuvid.c  |   1 +
 libavcodec/cuvid_hevc.c | 280 
 libavcodec/hevcdec.c|   6 +-
 7 files changed, 292 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/cuvid_hevc.c

diff --git a/Changelog b/Changelog
index f3c8f7a..59a2eba 100644
--- a/Changelog
+++ b/Changelog
@@ -17,7 +17,7 @@ version :
 - ClearVideo decoder (I-frames only)
 - support for decoding through D3D11VA in avconv
 - Cinepak encoder
-- NVIDIA CUVID-accelerated H.264 decoding
+- NVIDIA CUVID-accelerated H.264 and HEVC decoding
 
 
 version 12:
diff --git a/configure b/configure
index d31403c..cf6b862 100755
--- a/configure
+++ b/configure
@@ -2209,6 +2209,8 @@ h264_vda_old_hwaccel_deps="vda"
 h264_vda_old_hwaccel_select="h264_decoder"
 h264_vdpau_hwaccel_deps="vdpau"
 h264_vdpau_hwaccel_select="h264_decoder"
+hevc_cuvid_hwaccel_deps="cuvid CUVIDHEVCPICPARAMS"
+hevc_cuvid_hwaccel_select="hevc_decoder"
 hevc_d3d11va_hwaccel_deps="d3d11va DXVA_PicParams_HEVC"
 hevc_d3d11va_hwaccel_select="hevc_decoder"
 hevc_d3d11va2_hwaccel_deps="d3d11va DXVA_PicParams_HEVC"
@@ -4697,6 +4699,7 @@ check_lib psapi"windows.h psapi.h"
GetProcessMemoryInfo -lpsapi
 check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
 
 check_type "cuviddec.h" "CUVIDH264PICPARAMS"
+check_type "cuviddec.h" "CUVIDHEVCPICPARAMS"
 
 check_type "windows.h dxva.h" "DXVA_PicParams_HEVC" 
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -D_CRT_BUILD_DESKTOP_APP=0
 check_type "windows.h d3d11.h" "ID3D11VideoDecoder"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 66f6f9e..12c8678 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -640,6 +640,7 @@ OBJS-$(CONFIG_H264_QSV_HWACCEL)   += qsvdec_h2645.o
 OBJS-$(CONFIG_H264_VAAPI_HWACCEL) += vaapi_h264.o
 OBJS-$(CONFIG_H264_VDA_HWACCEL)   += vda_h264.o
 OBJS-$(CONFIG_H264_VDPAU_HWACCEL) += vdpau_h264.o
+OBJS-$(CONFIG_HEVC_CUVID_HWACCEL) += cuvid_hevc.o
 OBJS-$(CONFIG_HEVC_D3D11VA_HWACCEL)   += dxva2_hevc.o
 OBJS-$(CONFIG_HEVC_DXVA2_HWACCEL) += dxva2_hevc.o
 OBJS-$(CONFIG_HEVC_QSV_HWACCEL)   += qsvdec_h2645.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 97b8810..717e18f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -78,6 +78,7 @@ void avcodec_register_all(void)
 REGISTER_HWACCEL(H264_VDA,  h264_vda);
 REGISTER_HWACCEL(H264_VDA_OLD,  h264_vda_old);
 REGISTER_HWACCEL(H264_VDPAU,h264_vdpau);
+REGISTER_HWACCEL(HEVC_CUVID,hevc_cuvid);
 REGISTER_HWACCEL(HEVC_D3D11VA,  hevc_d3d11va);
 REGISTER_HWACCEL(HEVC_D3D11VA2, hevc_d3d11va2);
 REGISTER_HWACCEL(HEVC_DXVA2,hevc_dxva2);
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 22b966c..9abccb6 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -51,6 +51,7 @@ static int map_avcodec_id(enum AVCodecID id)
 {
 switch (id) {
 case AV_CODEC_ID_H264: return cudaVideoCodec_H264;
+case AV_CODEC_ID_HEVC: return cudaVideoCodec_HEVC;
 }
 return -1;
 }
diff --git a/libavcodec/cuvid_hevc.c b/libavcodec/cuvid_hevc.c
new file mode 100644
index 000..5de9bca
--- /dev/null
+++ b/libavcodec/cuvid_hevc.c
@@ -0,0 +1,280 @@
+/*
+ * HEVC HW decode acceleration through CUVID
+ *
+ * Copyright (c) 2017 Anton Khirnov
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+#include 
+
+#include "avcodec.h"
+#include "cuvid.h"
+#include "decode.h"
+#include "internal.h"
+#include "hevcdec.h"
+#include "hevc_data.h"
+
+static void dpb_add(CUVIDHEVCPICPARAMS *pp, int idx, const HEVCFrame *src)
+{
+FrameDecodeData *fdd = (FrameDecodeData*)src->frame->opaque_ref->data;
+const CUVIDFrame *cf = fdd->hwaccel_priv;
+
+pp->RefPicIdx[idx]  = cf ? cf->idx 

Re: [libav-devel] [PATCH 04/15] lavc: Add coded bitstream read/write API

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:10)
> ---
>  libavcodec/cbs.c  | 656 
> ++
>  libavcodec/cbs.h  | 274 +++
>  libavcodec/cbs_internal.h |  99 +++
>  3 files changed, 1029 insertions(+)
>  create mode 100644 libavcodec/cbs.c
>  create mode 100644 libavcodec/cbs.h
>  create mode 100644 libavcodec/cbs_internal.h
> 

Ok

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

Re: [libav-devel] [PATCH 05/15] lavc: Add coded bitstream read/write support for H.264

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:11)
> ---
>  libavcodec/cbs.c |1 +
>  libavcodec/cbs_h264.h|  426 +++
>  libavcodec/cbs_h2645.c   |  800 +++
>  libavcodec/cbs_h2645.h   |   42 ++
>  libavcodec/cbs_h264_syntax.c | 1224 
> ++
>  libavcodec/cbs_internal.h|3 +
>  6 files changed, 2496 insertions(+)
>  create mode 100644 libavcodec/cbs_h264.h
>  create mode 100644 libavcodec/cbs_h2645.c
>  create mode 100644 libavcodec/cbs_h2645.h
>  create mode 100644 libavcodec/cbs_h264_syntax.c

Ok

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

Re: [libav-devel] [PATCH 10/15] lavc: Add h264_redundant_pps bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:16)
> This applies a specific fixup to some Bluray streams which contain
> redundant PPSs modifying irrelevant parameters of the stream which
> confuse other transformations which require correct extradata.
> 
> A new single global PPS is created, and all of the redundant PPSs
> within the stream are removed.
> ---
>  doc/bitstream_filters.texi  |   9 ++
>  libavcodec/Makefile |   2 +
>  libavcodec/bitstream_filters.c  |   1 +
>  libavcodec/h264_redundant_pps_bsf.c | 176 
> 
>  4 files changed, 188 insertions(+)
>  create mode 100644 libavcodec/h264_redundant_pps_bsf.c

LGTM

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

Re: [libav-devel] [PATCH 09/15] lavc: Add h264_metadata bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:15)
> This is able to modify some header metadata found in the SPS/VUI,
> and can also add/remove AUDs and insert user data in SEI NAL units.
> ---
>  doc/bitstream_filters.texi |  47 +
>  libavcodec/Makefile|   2 +
>  libavcodec/bitstream_filters.c |   1 +
>  libavcodec/h264_metadata_bsf.c | 462 
> +
>  4 files changed, 512 insertions(+)
>  create mode 100644 libavcodec/h264_metadata_bsf.c
> 

Looks ok

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

Re: [libav-devel] [PATCH 08/15] lavc: Add trace_headers bitstream filter

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:14)
> +static int trace_headers(AVBSFContext *bsf, AVPacket *out)
> +{
> +TraceHeadersContext *ctx = bsf->priv_data;
> +CodedBitstreamFragment au;
> +AVPacket *in;
> +int err;
> +
> +err = ff_bsf_get_packet(bsf, );
> +if (err < 0)
> +return err;
> +
> +if (in->pts == AV_NOPTS_VALUE)
> +av_log(bsf, AV_LOG_INFO, "Packet (%sno pts)\n",
> +   in->flags & AV_PKT_FLAG_KEY ? "key frame, " : "");
> +else
> +av_log(bsf, AV_LOG_INFO, "Packet (%spts %"PRId64")\n",
> +   in->flags & AV_PKT_FLAG_KEY ? "key frame, " : "", in->pts);

Why not log the dts as well?

Otherwise looks ok.

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

Re: [libav-devel] [PATCH 03/15] hevc: Validate the number of long term reference pictures

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:09)
> This would overflow if the stream contained a value greater than the
> maximum allowed by the standard (32).
> ---
>  libavcodec/hevc_ps.c | 6 ++
>  1 file changed, 6 insertions(+)
> 

Looks ok and should go into stable.

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

Re: [libav-devel] [PATCH 02/15] hevc: Improve stream constraint values in common header

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:08)
> Add comments to describe the sources of the constraint values expressed here,
> and add some more related values which will be used in following patches.
> 
> Fix the incorrect values for SPS and PPS count (they are not the same as those
> used for H.264), and remove HEVC_MAX_CU_SIZE because it is not used anywhere.
> ---
>  libavcodec/hevc.h| 57 
> +---
>  libavcodec/hevc_ps.c |  2 +-
>  libavcodec/hevc_ps.h |  6 +++---
>  libavformat/hevc.c   |  6 +++---
>  4 files changed, 52 insertions(+), 19 deletions(-)
> 

LGTM

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

Re: [libav-devel] [PATCH 01/15] h264: Add stream constraint values to the common header

[Anton Khirnov]

Quoting Mark Thompson (2017-06-24 01:39:07)
> With comments describing the derivation of each value.
> ---
>  libavcodec/h264.h | 45 +
>  1 file changed, 45 insertions(+)
> 

No objections from me.
An enum is a bit unusual, but I don't see it as a problem.

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

Re: [libav-devel] [PATCH 08/25] libavfilter changes for the new channel layout API

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:52)
> This patch contains the following commits:
> 
> avtools: Use the new channel layout API in libavfilter

Again, this does not belong here.
The libraries should be converted separately from their users, and FATE
should pass both before and after. That helps ensuring that the compat
layers are correct.

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

Re: [libav-devel] [PATCH 07/25] lavfi, buffersrc: switch to the new channel layout API

[Anton Khirnov]

nnel layout %s.\n",
> -   s->channel_layout_str);
> -return AVERROR(EINVAL);
> -}
> -
>  if (!(s->fifo = av_fifo_alloc(sizeof(AVFrame*
>  return AVERROR(ENOMEM);
>  
>  if (!s->time_base.num)
>  s->time_base = (AVRational){1, s->sample_rate};
>  
> +chlstr = av_channel_layout_describe(>ch_layout);
>  av_log(ctx, AV_LOG_VERBOSE, "tb:%d/%d samplefmt:%s samplerate: %d "
> "ch layout:%s\n", s->time_base.num, s->time_base.den, 
> s->sample_fmt_str,
> -   s->sample_rate, s->channel_layout_str);
> +   s->sample_rate, chlstr);
> +av_free(chlstr);
>  
>  return ret;
>  }
> @@ -344,7 +347,7 @@ static int query_formats(AVFilterContext *ctx)
>  ff_add_format(,   c->sample_rate);
>  ff_set_common_samplerates(ctx, samplerates);
>  
> -ff_add_channel_layout(_layouts, c->channel_layout);
> +ff_add_channel_layout(_layouts, c->ch_layout.u.mask);
>  ff_set_common_channel_layouts(ctx, channel_layouts);
>  break;
>  default:
> @@ -357,6 +360,7 @@ static int query_formats(AVFilterContext *ctx)
>  static int config_props(AVFilterLink *link)
>  {
>  BufferSourceContext *c = link->src->priv;
> +int ret;
>  
>  switch (link->type) {
>  case AVMEDIA_TYPE_VIDEO:
> @@ -371,7 +375,16 @@ static int config_props(AVFilterLink *link)
>  }
>  break;
>  case AVMEDIA_TYPE_AUDIO:
> -link->channel_layout = c->channel_layout;
> +ret = av_channel_layout_copy(>ch_layout, >ch_layout);
> +if (ret < 0)
> +return ret;
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +if (c->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ||
> +c->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC)
> +link->channel_layout = c->ch_layout.u.mask;

Same as above.


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

Re: [libav-devel] [PATCH 06/25] lavr: switch to the new channel layout API

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:50)
> diff --git a/libavresample/utils.c b/libavresample/utils.c
> index bab2153b4b..15c827efbe 100644
> --- a/libavresample/utils.c
> +++ b/libavresample/utils.c
> @@ -18,6 +18,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include "libavutil/channel_layout.h"
>  #include "libavutil/common.h"
>  #include "libavutil/dict.h"
>  #include "libavutil/error.h"
> @@ -35,6 +36,7 @@
>  
>  int avresample_open(AVAudioResampleContext *avr)
>  {
> +int in_ch, out_ch;
>  int ret;
>  
>  if (avresample_is_open(avr)) {
> @@ -42,24 +44,67 @@ int avresample_open(AVAudioResampleContext *avr)
>  return AVERROR(EINVAL);
>  }
>  
> +if ( avr->in_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM ||
> +avr->out_ch_layout.order == AV_CHANNEL_ORDER_CUSTOM) {
> +av_log(avr, AV_LOG_ERROR,
> +  "Resampling a custom channel layout order is not 
> supported.\n");
> +   return AVERROR(ENOSYS);

Why? I don't remember if I tested it properly back then, but IIRC the
matrix building code should work custom orders just fine.

> +}
> +
> +if (avr->in_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +if (avr->in_ch_layout.nb_channels > 63) {
> +   av_log(avr, AV_LOG_ERROR,
> +  "Unspecified channel layout order is supported only for up 
> "
> +  "to 63 channels (got %d).\n", 
> avr->in_ch_layout.nb_channels);
> +   return AVERROR(ENOSYS);
> +}
> +av_channel_layout_default(>in_ch_layout, 
> avr->in_ch_layout.nb_channels);
> +}
> +if (avr->out_ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) {
> +if (avr->out_ch_layout.nb_channels > 63) {
> +   av_log(avr, AV_LOG_ERROR,
> +  "Unspecified channel layout order is supported only for up 
> "
> +  "to 63 channels (got %d).\n", 
> avr->out_ch_layout.nb_channels);
> +   return AVERROR(ENOSYS);
> +}
> +av_channel_layout_default(>out_ch_layout, 
> avr->out_ch_layout.nb_channels);
> +}

Why are those needed? Seems they are redundant given the other checks
right below.


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

Re: [libav-devel] [PATCH 05/25] avtools: Use the new channel layout API in AVFrame

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:49)
> ---
>  avtools/avconv.c| 2 +-
>  avtools/avconv_filter.c | 2 +-
>  avtools/avplay.c| 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 

Again, seems premature. The switch should be done at the end. If it
needs to be done here, then your compatibility layer is broken.

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

Re: [libav-devel] [PATCH 04/25] avframe: switch to the new channel layout API

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:48)
> From: Anton Khirnov <an...@khirnov.net>
> 
> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
> ---
>  libavcodec/decode.c | 67 
>  libavcodec/encode.c |  9 ++
>  libavutil/frame.c   | 88 
> +++--
>  libavutil/frame.h   | 12 +++-
>  4 files changed, 146 insertions(+), 30 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index a49cd77e51..b0d6b9fb33 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -132,7 +132,7 @@ static int unrefcount_frame(AVCodecInternal *avci, 
> AVFrame *frame)
>  memcpy(frame->data, avci->to_free->data, sizeof(frame->data));
>  memcpy(frame->linesize, avci->to_free->linesize, 
> sizeof(frame->linesize));
>  if (avci->to_free->extended_data != avci->to_free->data) {
> -int planes = 
> av_get_channel_layout_nb_channels(avci->to_free->channel_layout);
> +int planes = avci->to_free->ch_layout.nb_channels;
>  int size   = planes * sizeof(*frame->extended_data);
>  
>  if (!size) {
> @@ -153,9 +153,19 @@ static int unrefcount_frame(AVCodecInternal *avci, 
> AVFrame *frame)
>  frame->format = avci->to_free->format;
>  frame->width  = avci->to_free->width;
>  frame->height = avci->to_free->height;
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +FF_DISABLE_DEPRECATION_WARNINGS
>  frame->channel_layout = avci->to_free->channel_layout;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>  frame->nb_samples = avci->to_free->nb_samples;
>  
> +ret = av_channel_layout_copy(>ch_layout, 
> >to_free->ch_layout);
> +if (ret < 0) {
> +av_frame_unref(frame);
> +return ret;
> +}
> +
>  return 0;
>  }
>  
> @@ -887,10 +897,20 @@ static int update_frame_pool(AVCodecContext *avctx, 
> AVFrame *frame)
>  break;
>  }
>  case AVMEDIA_TYPE_AUDIO: {
> -int ch = 
> av_get_channel_layout_nb_channels(frame->channel_layout);
> +int ch = frame->ch_layout.nb_channels;
>  int planar = av_sample_fmt_is_planar(frame->format);
>  int planes = planar ? ch : 1;
>  
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +if (!ch && frame->channel_layout) {
> +av_channel_layout_from_mask(>ch_layout, 
> frame->channel_layout);
> +ch = frame->ch_layout.nb_channels;
> +planes = planar ? ch : 1;
> +}
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif

What is this for? This code is the internal get_buffer2()
implementation, so the new channel layout API should always be used.

> +
>  if (pool->format == frame->format && pool->planes == planes &&
>  pool->channels == ch && frame->nb_samples == pool->samples)
>  return 0;
> @@ -,28 +1131,35 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame 
> *frame, int flags)
>  frame->sample_rate= avctx->sample_rate;
>  if (frame->format < 0)
>  frame->format = avctx->sample_fmt;
> +if (!frame->ch_layout.nb_channels) {
> +if (avctx->channel_layout)
> +av_channel_layout_from_mask(>ch_layout, 
> avctx->channel_layout);
> +else
> +av_channel_layout_default(>ch_layout, 
> avctx->channels);
> +}

Same, why the deprecated API use.

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

Re: [libav-devel] [PATCH 04/25] avframe: switch to the new channel layout API

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:48)
> From: Anton Khirnov <an...@khirnov.net>
> 
> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
> ---
>  libavcodec/decode.c | 67 
>  libavcodec/encode.c |  9 ++

Why are there lavc changes in this patch. Should they not come after
lavc conversion? I'd expect this patch to break FATE.

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

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

[Anton Khirnov]

Quoting wm4 (2017-07-05 20:53:54)
> On Fri, 30 Jun 2017 12:51:16 -0400
> Vittorio Giovara <vittorio.giov...@gmail.com> wrote:
> 
> > On Fri, Jun 30, 2017 at 4:38 AM, wm4 <nfx...@googlemail.com> wrote:
> > > On Thu, 29 Jun 2017 17:28:51 -0400
> > > Vittorio Giovara <vittorio.giov...@gmail.com> wrote:
> > >  
> > >> From: Anton Khirnov <an...@khirnov.net>
> > >>
> > >> The new API is more extensible and allows for custom layouts.
> > >> More accurate information is exported, eg for decoders that do not
> > >> set a channel layout, lavc will not make one up for them.
> > >>
> > >> Deprecate the old API working with just uint64_t bitmasks.
> > >>
> > >> Expanded and completed by Vittorio Giovara <vittorio.giov...@gmail.com>.
> > >> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
> > >> ---  
> > >
> > >  
> > >> +
> > >> +/**
> > >> + * Initialize a channel layout from a given string description.
> > >> + * The input string can be represented by:
> > >> + *  - the formal channel layout name (returned by 
> > >> av_channel_layout_describe())
> > >> + *  - single or multiple channel names (returned by av_channel_name()
> > >> + *or concatenated with "|")
> > >> + *  - a hexadecimal value of a channel layout (eg. "0x4")
> > >> + *  - the number of channels with default layout (eg. "5")
> > >> + *  - the number of unordered channels (eg. "4 channels")
> > >> + *
> > >> + * @note channel_layout should be properly allocated as described 
> > >> above.  
> > >
> > > Above, where?  
> > 
> > in the structure description, I'll add this locally
> > 
> > >> + *
> > >> + * @param channel_layout input channel layout
> > >> + * @param str string describing the channel layout
> > >> + * @return 0 channel layout was detected, AVERROR_INVALIDATATA otherwise
> > >> + */
> > >> +int av_channel_layout_from_string(AVChannelLayout *channel_layout,
> > >> +  const char *str);  
> > >
> > > I still think you need to describe in what state channel_layout should
> > > be.
> > >
> > > Why is this so hard? It's always the same with Libav API docs. AVFrame
> > > sort of has the same problem.  
> > 
> > ok if you insist ^^
> > 
> > >> +/**
> > >> + * Get the default channel layout for a given number of channels.
> > >> + *
> > >> + * @note channel_layout should be properly allocated as described above.
> > >> + *
> > >> + * @param channel_layout the layout structure to be initialized
> > >> + * @param nb_channels number of channels
> > >> + */
> > >> +void av_channel_layout_default(AVChannelLayout *ch_layout, int 
> > >> nb_channels);
> > >> +
> > >> +/**
> > >> + * Free any allocated data in the channel layout and reset the channel
> > >> + * count to 0.
> > >> + *
> > >> + * @note this only used for structure initialization and for freeing the
> > >> + *   allocated memory for AV_CHANNEL_ORDER_CUSTOM order.
> > >> + *
> > >> + * @param channel_layout the layout structure to be uninitialized
> > >> + */
> > >> +void av_channel_layout_uninit(AVChannelLayout *channel_layout);
> > >> +
> > >> +/**
> > >> + * Make a copy of a channel layout. This differs from just assigning 
> > >> src to dst
> > >> + * in that it allocates and copies the map for AV_CHANNEL_ORDER_CUSTOM.
> > >> + *  
> > >  
> > >> + * @note the destination channel_layout will be always uninitialized 
> > >> before copy.  
> > >
> > > What does this statement even mean? Does it call
> > >
> > >   av_channel_layout_uninit(dst);
> > >
> > > as first line?  
> > 
> > Correct, is that unclear? How would you phrase it?
> 
> Well, the situation is that memory allocations are involved, and the
> state of the dst argument is unclear. You have to define exactly in
> what state it has to be, or the user is inevitably going to make
> mistakes, unless he reads the source code and makes assumptions about
> the API based on it.
> 
> In such a case, there are about the fo

Re: [libav-devel] [PATCH 01/25] Add a new channel layout API

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-29 00:10:45)
> +
> +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, or with channels that could be 
> skipped.
> + */
> +AV_CHANNEL_ORDER_CUSTOM,
> +/**
> + * Only the channel count is specified, without any further information
> + * about the channel order.

This used to be just 'about the channels', and I think that was more
correct -- here we lack any informations about what the channels are,
not just their ordering. Alternatively 'about the channel order or
semantics'.

> +
> +/**
> + * Initialize a channel layout from a given string description.
> + * The input string can be represented by:
> + *  - the formal channel layout name (returned by 
> av_channel_layout_describe())
> + *  - single or multiple channel names (returned by av_channel_name()
> + *or concatenated with "|")

Shouldn't this be
 - single or multiple channel names (returned by av_channel_name())
   concatenated with "|"

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

Re: [libav-devel] [PATCH 5/5] h264dec: add a CUVID hwaccel

[Anton Khirnov]

Quoting Luca Barbato (2017-07-21 19:28:29)
> On 21/07/2017 15:19, Anton Khirnov wrote:
> > Some parts of the code are based on a patch by
> > Timo Rothenpieler <t...@rothenpieler.org>
> > ---
> 
> Seems fine, once I manage to boot the desktop I can test it.
> 
> lu
> 
> PS: Do you have plans for hevc as well?

Sure, adding all the other codecs should be simple.

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

Re: [libav-devel] [PATCH v2] imgutils: add function to clear an image to black

[Anton Khirnov]

Quoting wm4 (2017-07-16 12:41:59)
> Black isn't always just memset(ptr, 0, size). Limited YUV in particular
> requires relatively non-obvious values, and filling a frame with
> repeating 0 bytes is disallowed in some contexts. With component sizes
> larger than 8 or packed YUV, this can become relatively complicated. So
> having a generic function for this seems helpful.
> 
> In order to handle the complex cases in a generic way without destroying
> performance, this code attempts to compute a black pixel, and then uses
> that value to clear the image data quickly by using a function like
> memset.
> 
> Common cases like yuv410p10 or rgba can't be handled with a simple
> memset, so there is some code to fill memory with 2 and 4 byte patterns.
> For the remaining cases, a generic slow fallback is used. This code
> should probably have a 8 byte case too, to deal with rgba64.
> 
> ---
> Fixed:
> - simply reduction to memset code
> - fix monow/monob handling
> - change alpha range
> - add 8 bytes path (untested)
> - allow dst_data==NULL with special semantics
> TODO: APIchanges, version bumps, FATE test
> ---
>  libavutil/imgutils.c | 167 
> +++
>  libavutil/imgutils.h |  27 +
>  2 files changed, 194 insertions(+)
> 
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 84abb11656..c43185ccff 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -435,3 +435,170 @@ int av_image_copy_to_buffer(uint8_t *dst, int dst_size,
>  
>  return size;
>  }
> +
> +// Fill dst[0..dst_size] with the bytes in clear[0..clear_size]. The clear
> +// bytes are repeated until dst_size is reached. If dst_size is unaligned 
> (i.e.
> +// dst_size%clear_size!=0), the remaining data will be filled with the 
> beginning
> +// of the clear data only.
> +static void memset_bytes(uint8_t *dst, size_t dst_size, uint8_t *clear,
> + size_t clear_size)
> +{
> +size_t pos = 0;
> +int same = 1;
> +int i;
> +
> +if (!clear_size)
> +return;
> +
> +// Reduce to memset() if possible.
> +for (i = 0; i < clear_size; i++) {
> +if (clear[i] != clear[0]) {
> +same = 0;
> +break;
> +}
> +}
> +if (same)
> +clear_size = 1;
> +
> +if (clear_size == 1) {
> +memset(dst, clear[0], dst_size);
> +dst_size = 0;
> +} else if (clear_size == 2) {
> +uint16_t val = AV_RN16(clear);
> +for (; dst_size >= 2; dst_size -= 2) {
> +AV_WN16(dst, val);
> +dst += 2;
> +}
> +} else if (clear_size == 4) {
> +uint32_t val = AV_RN32(clear);
> +for (; dst_size >= 4; dst_size -= 4) {
> +AV_WN32(dst, val);
> +dst += 4;
> +}
> +} else if (clear_size == 8) {
> +uint32_t val = AV_RN64(clear);
> +for (; dst_size >= 8; dst_size -= 8) {
> +AV_WN64(dst, val);
> +dst += 8;
> +}
> +}
> +
> +for (; dst_size; dst_size--)
> +*dst++ = clear[pos++ % clear_size];
> +}
> +
> +// Maximum size in bytes of a plane element (usually a pixel, or multiple 
> pixels
> +// if it's a subsampled packed format).
> +#define MAX_PIXEL_SIZE 32
> +
> +int av_image_fill_black(uint8_t *dst_data[4], const ptrdiff_t 
> dst_linesize[4],
> +enum AVPixelFormat pix_fmt, enum AVColorRange range,
> +int width, int height)
> +{
> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> +int nb_planes = av_pix_fmt_count_planes(pix_fmt);
> +// A pixel on each plane, with a value that represents black.
> +// Consider e.g. AV_PIX_FMT_UYVY422 for non-trivial cases.
> +uint8_t pixel[4][MAX_PIXEL_SIZE] = {0}; // clear padding with 0
> +int pixel_size[4] = {0};

The naming is a bit confusing, since those are not individual pixels,
but blocks of "maximum step size". How about block_val / block_size for
those two.

Otherwise looks ok-ish. Not exactly pretty, but that's probably
unavoidable for such code.

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

Re: [libav-devel] [PATCH v2] lavc, lavu: move frame cropping to a convenience function

[Anton Khirnov]

Quoting wm4 (2017-07-21 22:56:43)
> On Fri, 21 Jul 2017 22:32:43 +0200
> Anton Khirnov <an...@khirnov.net> wrote:
> 
> > Quoting wm4 (2017-07-12 17:02:49)
> > > +int av_frame_apply_cropping(AVFrame *frame, int flags)
> > > +{
> > > +const AVPixFmtDescriptor *desc;
> > > +size_t offsets[4];
> > > +int i;
> > > +
> > > +if (!(frame->width > 0 && frame->height > 0))
> > > +return AVERROR(EINVAL);
> > > +
> > > +/* make sure we are noisy about decoders returning invalid cropping 
> > > data */  
> > 
> > The comment does not apply anymore.
> > 
> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > > index f9ffb5bbbf..eb1fa0cba5 100644
> > > --- a/libavutil/frame.h
> > > +++ b/libavutil/frame.h
> > > @@ -580,6 +580,38 @@ AVFrameSideData *av_frame_get_side_data(const 
> > > AVFrame *frame,
> > >   */
> > >  void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType 
> > > type);
> > >  
> > > +
> > > +/**
> > > + * Flags for frame cropping.
> > > + */
> > > +enum {
> > > +/**
> > > + * Apply the maximum possible cropping, even if it requires setting 
> > > the
> > > + * AVFrame.data[] entries to unaligned pointers. Using this is 
> > > strongly
> > > + * discouraged, especially if the data is to be fed back to Libav 
> > > API. It
> > > + * can cause performance issues, and in some cases crashes.  
> > 
> > 'discouraged' is too weak IMO, to me it means that it's something which
> > is allowed but suboptimal. Whereas passing invalid data to libav* is
> > outright forbidden as UB, and is very likely to crash. Suggested
> > wording:
> > 'Note that all Libav APIs (except those that explicitly specify
> > otherwise) expect the frame data to be aligned, so when this flag is
> > used, the resulting frame must not be fed back to Libav.'
> > 
> > Otherwise looks ok, modulo apichanges/bump
> > 
> 
> Isn't that wording to strict? I'm fairly sure you'd still be allowed to
> e.g. copy the frame data, or pass it to swscale. (Yes, libswscale is
> officially fine with unaligned data, just that it might switch to C
> code paths.)

Then we should document those functions as allowing unaligned data and
then what I wrote will still be true. And for sws i would almost expect
a copy+simd to be faster than the C paths.

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

Re: [libav-devel] [PATCH v2] lavc, lavu: move frame cropping to a convenience function

[Anton Khirnov]

Quoting wm4 (2017-07-12 17:02:49)
> +int av_frame_apply_cropping(AVFrame *frame, int flags)
> +{
> +const AVPixFmtDescriptor *desc;
> +size_t offsets[4];
> +int i;
> +
> +if (!(frame->width > 0 && frame->height > 0))
> +return AVERROR(EINVAL);
> +
> +/* make sure we are noisy about decoders returning invalid cropping data 
> */

The comment does not apply anymore.

> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index f9ffb5bbbf..eb1fa0cba5 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -580,6 +580,38 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame 
> *frame,
>   */
>  void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType 
> type);
>  
> +
> +/**
> + * Flags for frame cropping.
> + */
> +enum {
> +/**
> + * Apply the maximum possible cropping, even if it requires setting the
> + * AVFrame.data[] entries to unaligned pointers. Using this is strongly
> + * discouraged, especially if the data is to be fed back to Libav API. It
> + * can cause performance issues, and in some cases crashes.

'discouraged' is too weak IMO, to me it means that it's something which
is allowed but suboptimal. Whereas passing invalid data to libav* is
outright forbidden as UB, and is very likely to crash. Suggested
wording:
'Note that all Libav APIs (except those that explicitly specify
otherwise) expect the frame data to be aligned, so when this flag is
used, the resulting frame must not be fed back to Libav.'

Otherwise looks ok, modulo apichanges/bump

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

Re: [libav-devel] [PATCH] lavf: allow avformat_close_input() with NULL

[Anton Khirnov]

Quoting wm4 (2017-07-16 12:43:09)
> Behaves more like FFmpeg, makes some API users not crash on exit.
> ---
>  libavformat/utils.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index eaba473914..24a4335016 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2733,7 +2733,12 @@ void avformat_free_context(AVFormatContext *s)
>  void avformat_close_input(AVFormatContext **ps)
>  {
>  AVFormatContext *s = *ps;
> -AVIOContext *pb= s->pb;
> +AVIOContext *pb;
> +
> +if (!ps || !*ps)

This is overly lax IMO. *ps being NULL should be allowed, in analogy
with free(NULL) beeing a no-op. But ps being NULL makes no sense at all
ever and just means that the caller is broken, so that should crash.

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

[libav-devel] [PATCH] caf: add an Opus tag

[Anton Khirnov]

CC: libav-sta...@libav.org
---
 libavformat/caf.c| 1 +
 libavformat/cafdec.c | 8 
 2 files changed, 9 insertions(+)

diff --git a/libavformat/caf.c b/libavformat/caf.c
index cf128d5..c299cad 100644
--- a/libavformat/caf.c
+++ b/libavformat/caf.c
@@ -49,6 +49,7 @@ const AVCodecTag ff_codec_caf_tags[] = {
 { AV_CODEC_ID_QCELP,   MKBETAG('Q','c','l','p') },
 { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','2') },
 { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','C') },
+{ AV_CODEC_ID_OPUS,MKBETAG('o','p','u','s') },
   /* currently unsupported codecs */
   /*{ AC-3 over S/PDIF  MKBETAG('c','a','c','3') },*/
   /*{ MPEG4CELP MKBETAG('c','e','l','p') },*/
diff --git a/libavformat/cafdec.c b/libavformat/cafdec.c
index efc8c49..d6b6007 100644
--- a/libavformat/cafdec.c
+++ b/libavformat/cafdec.c
@@ -156,6 +156,14 @@ static int read_kuki_chunk(AVFormatContext *s, int64_t 
size)
 avio_skip(pb, size - ALAC_NEW_KUKI);
 }
 st->codecpar->extradata_size = ALAC_HEADER;
+} else if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) {
+// The data layout for Opus is currently unknown, so we do not export
+// extradata at all. Multichannel streams are not supported.
+if (st->codecpar->channels > 2) {
+avpriv_request_sample(s, "multichannel Opus in CAF");
+return AVERROR_PATCHWELCOME;
+}
+avio_skip(pb, size);
 } else {
 st->codecpar->extradata = av_mallocz(size + 
AV_INPUT_BUFFER_PADDING_SIZE);
 if (!st->codecpar->extradata)
-- 
2.0.0

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

Re: [libav-devel] [PATCH] caf: add an Opus tag

[Anton Khirnov]

Quoting James Almer (2017-07-21 17:49:33)
> On 7/21/2017 9:19 AM, Anton Khirnov wrote:
> > Quoting Anton Khirnov (2017-07-21 13:59:41)
> >> CC: libav-sta...@libav.org
> >> ---
> >>  libavformat/caf.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/libavformat/caf.c b/libavformat/caf.c
> >> index cf128d5..c299cad 100644
> >> --- a/libavformat/caf.c
> >> +++ b/libavformat/caf.c
> >> @@ -49,6 +49,7 @@ const AVCodecTag ff_codec_caf_tags[] = {
> >>  { AV_CODEC_ID_QCELP,   MKBETAG('Q','c','l','p') },
> >>  { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','2') },
> >>  { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','C') },
> >> +{ AV_CODEC_ID_OPUS,MKBETAG('o','p','u','s') },
> >>/* currently unsupported codecs */
> >>/*{ AC-3 over S/PDIF  MKBETAG('c','a','c','3') },*/
> >>/*{ MPEG4CELP MKBETAG('c','e','l','p') },*/
> >> -- 
> >> 2.0.0
> >>
> > 
> > Please disregard, seems I didn't test this properly back when I wrote
> > it.
> > Sorry for the noise.
> 
> Were you able to figure out the full contents of the kuki chunk for Opus
> files?
> I looked at it briefly on two files, one stereo and one mono. First
> value was 2048 on both (no idea its meaning), second was sample rate
> (Apparently fixed to 48000 and not original sample rate as in OpusHead),
> third was the same value as frames_per_packet from the desc chunk,
> fourth was -1000 on both files as well (also no idea), and fifth was
> channel count.
> 
> Other formats would just dump the contents from the relevant ISOM box in
> the kuki chunk, but for some reason Opus doesn't. None of the values
> looked like pre_skip to me, so i don't think we could manually create a
> meaningful OpusHead to fill the stream's extradata.
> 
> The user that created the two files i checked also said he couldn't
> create a multichannel one, and demuxing/decoding seemed to work fine
> when i left extradata empty (the decoder fills it with defaults if
> channels <= 2).

Sounds about right. I think the reasonable thing to do would be to just
leave the extradata empty until we see some multichannel files.

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

[libav-devel] [PATCH 2/5] decode: add a method for attaching lavc-internal data to frames

[Anton Khirnov]

Use the AVFrame.opaque_ref field. The original user's opaque_ref is
wrapped in the lavc struct and then unwrapped before the frame is
returned to the caller.

This new struct will be useful in the following commits.
---
 libavcodec/decode.c | 57 +
 libavcodec/decode.h | 13 
 2 files changed, 70 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 89a6d34..9cd3081 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -406,6 +406,26 @@ static int decode_receive_frame_internal(AVCodecContext 
*avctx, AVFrame *frame)
 if (ret == AVERROR_EOF)
 avci->draining_done = 1;
 
+/* unwrap the per-frame decode data and restore the original opaque_ref*/
+if (!ret) {
+/* the only case where decode data is not set should be decoders
+ * that do not call ff_get_buffer() */
+av_assert0((frame->opaque_ref && frame->opaque_ref->size == 
sizeof(FrameDecodeData)) ||
+   !(avctx->codec->capabilities & AV_CODEC_CAP_DR1));
+
+if (frame->opaque_ref) {
+FrameDecodeData *fdd;
+AVBufferRef *user_opaque_ref;
+
+fdd = (FrameDecodeData*)frame->opaque_ref->data;
+
+user_opaque_ref = fdd->user_opaque_ref;
+fdd->user_opaque_ref = NULL;
+av_buffer_unref(>opaque_ref);
+frame->opaque_ref = user_opaque_ref;
+}
+}
+
 return ret;
 }
 
@@ -1073,6 +1093,37 @@ FF_ENABLE_DEPRECATION_WARNINGS
 return 0;
 }
 
+static void decode_data_free(void *opaque, uint8_t *data)
+{
+FrameDecodeData *fdd = (FrameDecodeData*)data;
+
+av_buffer_unref(>user_opaque_ref);
+
+av_freep();
+}
+
+static int attach_decode_data(AVFrame *frame)
+{
+AVBufferRef *fdd_buf;
+FrameDecodeData *fdd;
+
+fdd = av_mallocz(sizeof(*fdd));
+if (!fdd)
+return AVERROR(ENOMEM);
+
+fdd_buf = av_buffer_create((uint8_t*)fdd, sizeof(*fdd), decode_data_free,
+   NULL, AV_BUFFER_FLAG_READONLY);
+if (!fdd_buf) {
+av_freep();
+return AVERROR(ENOMEM);
+}
+
+fdd->user_opaque_ref = frame->opaque_ref;
+frame->opaque_ref= fdd_buf;
+
+return 0;
+}
+
 int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
 {
 const AVHWAccel *hwaccel = avctx->hwaccel;
@@ -1146,6 +1197,12 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, 
int flags)
 avctx->sw_pix_fmt = avctx->pix_fmt;
 
 ret = avctx->get_buffer2(avctx, frame, flags);
+if (ret < 0)
+goto end;
+
+ret = attach_decode_data(frame);
+if (ret < 0)
+goto end;
 
 end:
 if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions &&
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 2f29cf6..61b53b2 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -21,9 +21,22 @@
 #ifndef AVCODEC_DECODE_H
 #define AVCODEC_DECODE_H
 
+#include "libavutil/buffer.h"
+
 #include "avcodec.h"
 
 /**
+ * This struct stores per-frame lavc-internal data and is attached to it via
+ * opaque_ref.
+ */
+typedef struct FrameDecodeData {
+/**
+ * The original user-set opaque_ref.
+ */
+AVBufferRef *user_opaque_ref;
+} FrameDecodeData;
+
+/**
  * Called by decoders to get the next packet for decoding.
  *
  * @param pkt An empty packet to be filled with data.
-- 
2.0.0

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

[libav-devel] [PATCH 4/5] decode: add a per-frame private data for hwaccel use

[Anton Khirnov]

This will be useful in the CUVID hwaccel. It should also eventually
replace current decoder-specific mechanisms used by various other
hwaccels.
---
 libavcodec/decode.c | 3 +++
 libavcodec/decode.h | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 01e4dd2..a255773 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1110,6 +1110,9 @@ static void decode_data_free(void *opaque, uint8_t *data)
 if (fdd->post_process_opaque_free)
 fdd->post_process_opaque_free(fdd->post_process_opaque);
 
+if (fdd->hwaccel_priv_free)
+fdd->hwaccel_priv_free(fdd->hwaccel_priv);
+
 av_freep();
 }
 
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 72052f1..235f355 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -49,6 +49,12 @@ typedef struct FrameDecodeData {
 int (*post_process)(void *logctx, AVFrame *frame);
 void *post_process_opaque;
 void (*post_process_opaque_free)(void *opaque);
+
+/**
+ * Per-frame private data for hwaccels.
+ */
+void *hwaccel_priv;
+void (*hwaccel_priv_free)(void *priv);
 } FrameDecodeData;
 
 /**
-- 
2.0.0

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

[libav-devel] [PATCH 3/5] decode: add a mechanism for performing delayed processing on the decoded frames

[Anton Khirnov]

This will be useful in the CUVID hwaccel.
---
 libavcodec/decode.c | 11 +++
 libavcodec/decode.h | 15 +++
 2 files changed, 26 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 9cd3081..01e4dd2 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -419,6 +419,14 @@ static int decode_receive_frame_internal(AVCodecContext 
*avctx, AVFrame *frame)
 
 fdd = (FrameDecodeData*)frame->opaque_ref->data;
 
+if (fdd->post_process) {
+ret = fdd->post_process(avctx, frame);
+if (ret < 0) {
+av_frame_unref(frame);
+return ret;
+}
+}
+
 user_opaque_ref = fdd->user_opaque_ref;
 fdd->user_opaque_ref = NULL;
 av_buffer_unref(>opaque_ref);
@@ -1099,6 +1107,9 @@ static void decode_data_free(void *opaque, uint8_t *data)
 
 av_buffer_unref(>user_opaque_ref);
 
+if (fdd->post_process_opaque_free)
+fdd->post_process_opaque_free(fdd->post_process_opaque);
+
 av_freep();
 }
 
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 61b53b2..72052f1 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -22,6 +22,7 @@
 #define AVCODEC_DECODE_H
 
 #include "libavutil/buffer.h"
+#include "libavutil/frame.h"
 
 #include "avcodec.h"
 
@@ -34,6 +35,20 @@ typedef struct FrameDecodeData {
  * The original user-set opaque_ref.
  */
 AVBufferRef *user_opaque_ref;
+
+/**
+ * The callback to perform some delayed processing on the frame right
+ * before it is returned to the caller.
+ *
+ * @note This code is called at some unspecified point after the frame is
+ * returned from the decoder's decode/receive_frame call. Therefore it 
cannot rely
+ * on AVCodecContext being in any specific state, so it does not get to
+ * access AVCodecContext directly at all. All the state it needs must be
+ * stored in the post_process_opaque object.
+ */
+int (*post_process)(void *logctx, AVFrame *frame);
+void *post_process_opaque;
+void (*post_process_opaque_free)(void *opaque);
 } FrameDecodeData;
 
 /**
-- 
2.0.0

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

[libav-devel] [PATCH 1/5] decode: avoid leaks on failure in ff_get_buffer()

[Anton Khirnov]

If the get_buffer() call fails, the frame might have some side data
already set. Make sure it gets freed.
---
 libavcodec/decode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 175a6fa..89a6d34 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1154,6 +1154,9 @@ end:
 frame->height = avctx->height;
 }
 
+if (ret < 0)
+av_frame_unref(frame);
+
 return ret;
 }
 
-- 
2.0.0

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

[libav-devel] [PATCH 5/5] h264dec: add a CUVID hwaccel

[Anton Khirnov]

Makefile
+++ b/libavcodec/Makefile
@@ -625,6 +625,7 @@ OBJS-$(CONFIG_ADPCM_YAMAHA_DECODER)   += adpcm.o 
adpcm_data.o
 OBJS-$(CONFIG_ADPCM_YAMAHA_ENCODER)   += adpcmenc.o adpcm_data.o
 
 # hardware accelerators
+OBJS-$(CONFIG_CUVID)  += cuvid.o
 OBJS-$(CONFIG_D3D11VA)+= dxva2.o
 OBJS-$(CONFIG_DXVA2)  += dxva2.o
 OBJS-$(CONFIG_VAAPI)  += vaapi_decode.o
@@ -632,6 +633,7 @@ OBJS-$(CONFIG_VDA)+= vda.o
 OBJS-$(CONFIG_VDPAU)  += vdpau.o
 
 OBJS-$(CONFIG_H263_VAAPI_HWACCEL) += vaapi_mpeg4.o
+OBJS-$(CONFIG_H264_CUVID_HWACCEL) += cuvid_h264.o
 OBJS-$(CONFIG_H264_D3D11VA_HWACCEL)   += dxva2_h264.o
 OBJS-$(CONFIG_H264_DXVA2_HWACCEL) += dxva2_h264.o
 OBJS-$(CONFIG_H264_QSV_HWACCEL)   += qsvdec_h2645.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 5635ae1..97b8810 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -68,6 +68,7 @@ void avcodec_register_all(void)
 
 /* hardware accelerators */
 REGISTER_HWACCEL(H263_VAAPI,h263_vaapi);
+REGISTER_HWACCEL(H264_CUVID,h264_cuvid);
 REGISTER_HWACCEL(H264_D3D11VA,  h264_d3d11va);
     REGISTER_HWACCEL(H264_D3D11VA2, h264_d3d11va2);
 REGISTER_HWACCEL(H264_DXVA2,h264_dxva2);
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
new file mode 100644
index 000..22b966c
--- /dev/null
+++ b/libavcodec/cuvid.c
@@ -0,0 +1,410 @@
+/*
+ * HW decode acceleration through CUVID
+ *
+ * Copyright (c) 2016 Anton Khirnov
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+
+#include "libavutil/common.h"
+#include "libavutil/error.h"
+#include "libavutil/hwcontext.h"
+#include "libavutil/hwcontext_cuda.h"
+#include "libavutil/pixdesc.h"
+#include "libavutil/pixfmt.h"
+
+#include "avcodec.h"
+#include "decode.h"
+#include "cuvid.h"
+#include "internal.h"
+
+typedef struct CUVIDDecoder {
+CUvideodecoder decoder;
+
+AVBufferRef *hw_device_ref;
+CUcontextcuda_ctx;
+} CUVIDDecoder;
+
+typedef struct CUVIDFramePool {
+unsigned int dpb_size;
+unsigned int nb_allocated;
+} CUVIDFramePool;
+
+static int map_avcodec_id(enum AVCodecID id)
+{
+switch (id) {
+case AV_CODEC_ID_H264: return cudaVideoCodec_H264;
+}
+return -1;
+}
+
+static int map_chroma_format(enum AVPixelFormat pix_fmt)
+{
+int shift_h = 0, shift_v = 0;
+
+av_pix_fmt_get_chroma_sub_sample(pix_fmt, _h, _v);
+
+if (shift_h == 1 && shift_v == 1)
+return cudaVideoChromaFormat_420;
+else if (shift_h == 1 && shift_v == 0)
+return cudaVideoChromaFormat_422;
+else if (shift_h == 0 && shift_v == 0)
+return cudaVideoChromaFormat_444;
+
+return -1;
+}
+
+static void cuvid_decoder_free(void *opaque, uint8_t *data)
+{
+CUVIDDecoder *decoder = (CUVIDDecoder*)data;
+
+if (decoder->decoder)
+cuvidDestroyDecoder(decoder->decoder);
+
+av_buffer_unref(>hw_device_ref);
+
+av_freep();
+}
+
+static int cuvid_decoder_create(AVBufferRef **out, AVBufferRef *hw_device_ref,
+CUVIDDECODECREATEINFO *params, void *logctx)
+{
+AVHWDeviceContext  *hw_device_ctx = 
(AVHWDeviceContext*)hw_device_ref->data;
+AVCUDADeviceContext *device_hwctx = hw_device_ctx->hwctx;
+
+AVBufferRef *decoder_ref;
+CUVIDDecoder *decoder;
+
+CUcontext dummy;
+CUresult err;
+int ret;
+
+decoder = av_mallocz(sizeof(*decoder));
+if (!decoder)
+return AVERROR(ENOMEM);
+
+decoder_ref = av_buffer_create((uint8_t*)decoder, sizeof(*decoder),
+   cuvid_decoder_free, NULL, 
AV_BUFFER_FLAG_READONLY);
+if (!decoder_ref) {
+av_freep();
+return AVERROR(ENOMEM);
+}
+
+decoder->hw_device_ref = av_buffer_ref(hw_device_ref);
+if (!decoder->hw_device_ref) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+decoder->cuda_ctx = device_hwctx->cuda_ctx;
+
+err = cuCtxPushCurrent(decoder->cuda_ctx);
+if (err !=

Re: [libav-devel] [PATCH] caf: add an Opus tag

[Anton Khirnov]

Quoting Anton Khirnov (2017-07-21 13:59:41)
> CC: libav-sta...@libav.org
> ---
>  libavformat/caf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/caf.c b/libavformat/caf.c
> index cf128d5..c299cad 100644
> --- a/libavformat/caf.c
> +++ b/libavformat/caf.c
> @@ -49,6 +49,7 @@ const AVCodecTag ff_codec_caf_tags[] = {
>  { AV_CODEC_ID_QCELP,   MKBETAG('Q','c','l','p') },
>  { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','2') },
>  { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','C') },
> +{ AV_CODEC_ID_OPUS,MKBETAG('o','p','u','s') },
>/* currently unsupported codecs */
>/*{ AC-3 over S/PDIF  MKBETAG('c','a','c','3') },*/
>/*{ MPEG4CELP MKBETAG('c','e','l','p') },*/
> -- 
> 2.0.0
> 

Please disregard, seems I didn't test this properly back when I wrote
it.
Sorry for the noise.


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

[libav-devel] [PATCH] caf: add an Opus tag

[Anton Khirnov]

CC: libav-sta...@libav.org
---
 libavformat/caf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/caf.c b/libavformat/caf.c
index cf128d5..c299cad 100644
--- a/libavformat/caf.c
+++ b/libavformat/caf.c
@@ -49,6 +49,7 @@ const AVCodecTag ff_codec_caf_tags[] = {
 { AV_CODEC_ID_QCELP,   MKBETAG('Q','c','l','p') },
 { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','2') },
 { AV_CODEC_ID_QDM2,MKBETAG('Q','D','M','C') },
+{ AV_CODEC_ID_OPUS,MKBETAG('o','p','u','s') },
   /* currently unsupported codecs */
   /*{ AC-3 over S/PDIF  MKBETAG('c','a','c','3') },*/
   /*{ MPEG4CELP MKBETAG('c','e','l','p') },*/
-- 
2.0.0

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

Re: [libav-devel] [PATCH 1/6] avcodec/h264_slice: Also copy x264_build in ff_h264_update_thread_context()

[Anton Khirnov]

Quoting Vittorio Giovara (2017-06-16 00:56:11)
> From: Michael Niedermayer <michae...@gmx.at>
> 
> Fixes fate-h264-lossless
> 
> Signed-off-by: Michael Niedermayer <michae...@gmx.at>
> ---
>  libavcodec/h264_slice.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index c9f1dbb86f..0ce4127a1d 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -408,6 +408,7 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
>  // extradata/NAL handling
>  h->is_avc = h1->is_avc;
>  h->nal_length_size = h1->nal_length_size;
> +h->sei.unregistered.x264_build = h1->sei.unregistered.x264_build;
>  
>  memcpy(>poc,>poc,sizeof(h->poc));
>  
> -- 
> 2.13.1

This (and the following patch) is the wrong way to do it. H264SEI should
store the content of the last parsed SEI, other code should not mess
with it.

The right thing to do is maintain a copy of x264_build in H264Context.

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

Re: [libav-devel] [PATCH 1/2] smacker: Check that the data size is a multiple of a sample vector

[Anton Khirnov]

Quoting Diego Biurrun (2017-05-30 18:14:08)
> From: Michael Niedermayer <mich...@niedermayer.cc>
> 
> Fixes out of array access
> Fixes: 
> ce19e41f0ef1e52a23edc488faecdb58/asan_heap-oob_2504e97_4202_ffa0df1baed14022b9bfd4f8ac23d0cb.smk
> 
> Bug-Id: CVE-2015-8365
> CC: libav-sta...@libav.org
> 
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> (cherry picked from commit 4a9af07a49295e014b059c1ab624c40345af5892)
> Signed-off-by: Diego Biurrun <di...@biurrun.de>
> ---
>  libavcodec/smacker.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
> index c4b4bc7..4111c3b 100644
> --- a/libavcodec/smacker.c
> +++ b/libavcodec/smacker.c
> @@ -639,6 +639,10 @@ static int smka_decode_frame(AVCodecContext *avctx, void 
> *data,
>  
>  /* get output buffer */
>  frame->nb_samples = unp_size / (avctx->channels * (bits + 1));
> +if (unp_size % (avctx->channels * (bits + 1))) {
> +av_log(avctx, AV_LOG_ERROR, "unp_size %d is odd\n", unp_size);
> +return AVERROR(EINVAL);
> +}

The placement of the check is weird, it would make more sense among the
other checks immediately above.

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

Re: [libav-devel] [PATCH] avconv: Always initialize the opkt struct on streamcopy

[Anton Khirnov]

Quoting Luca Barbato (2017-05-29 15:31:34)
> On 5/29/17 2:43 PM, wm4 wrote:
> > On Mon, 29 May 2017 13:59:40 +0200
> > Luca Barbato <lu_z...@gentoo.org> wrote:
> > 
> >> ---
> >>  avtools/avconv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/avtools/avconv.c b/avtools/avconv.c
> >> index 719d289ff9..c30e1953ed 100644
> >> --- a/avtools/avconv.c
> >> +++ b/avtools/avconv.c
> >> @@ -1127,14 +1127,14 @@ static void do_streamcopy(InputStream *ist, 
> >> OutputStream *ost, const AVPacket *p
> >>  int64_t ost_tb_start_time = av_rescale_q(start_time, AV_TIME_BASE_Q, 
> >> ost->mux_timebase);
> >>  AVPacket opkt;
> >>  
> >> +av_init_packet();
> >> +
> >>  // EOF: flush output bitstream filters.
> >>  if (!pkt) {
> >>  output_packet(of, , ost, 1);
> >>  return;
> >>  }
> >>  
> >> -av_init_packet();
> >> -
> >>  if ((!ost->frame_number && !(pkt->flags & AV_PKT_FLAG_KEY)) &&
> >>  !ost->copy_initial_nonkeyframes)
> >>  return;
> > 
> > This doesn't initialize all AVPacket fields.
> 
> I can throw in a { 0 } if you like it better :)

Stab. It's not a question of "liking", there are correct things to do
and incorrect things to do. So do the correct thing.

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

Re: [libav-devel] [PATCH 002/200] lavu: support AVChannelLayout AVOptions

[Anton Khirnov]

Quoting wm4 (2017-05-26 20:03:07)
> On Fri, 26 May 2017 19:30:40 +0200
> Anton Khirnov <an...@khirnov.net> wrote:
> 
> > Quoting Luca Barbato (2017-05-23 22:08:49)
> > > On 5/17/17 7:46 PM, Vittorio Giovara wrote:  
> > > > +av_channel_layout_uninit(dst);
> > > > +return av_channel_layout_copy(dst, channel_layout);  
> > > 
> > > Maybe put the uninit directly in the layout_copy so there isn't risk to
> > > leak memory if you forget.  
> > 
> > I am against this. Freeing memory should be explicit.
> > 
> 
> Makes for hard to use API.
> 
> Like I never know whether AVFrame dest parameters require the frame to
> be "initialized" or whatever.

Hard? I'd agree with "more verbose", but not hard.
AVFrame should always be in one of two states:
- empty, after it's allocated or unreffed or move_ref()ed from or such
- filled, when somebody put something in it
When you pass it to any APIs that write into it, it should be empty. I
don't see how that is hard, it seems pretty intuitive to me.

OTOH implicit frees make it easy to leak stuff, because then it becomes
unclear what you do or don't have to free explicitily.

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

Re: [libav-devel] [PATCH 2/3] log: move all the "advanced" logging from lavu into cmdutils

[Anton Khirnov]

Quoting Vittorio Giovara (2017-05-22 22:05:55)
> On Sat, May 20, 2017 at 7:57 AM, Anton Khirnov <an...@khirnov.net> wrote:
> > The default logging callback in lavu currently contains several
> > "advanced" features, such as
> > - suppressing repeated messages
> > - automatically hiding the log prefix
> > - color support
> > They add significant complexity to the logging callback and - more
> > importantly - global state, making logging not thread-safe (and strictly
> > speaking introducing UB).
> >
> > Many (perhaps most) of our callers either do not care for such fancy
> > features, or already use a custom logging callback. Therefore, it is
> > better to move them to cmdutils (for use by avtools) and leave the
> > default logging callback simple, straightforward and safe.
> > ---
> 
> Would it be possible to move the new log code to a separate file than 
> cmdutils?
> It would prevent polluting the file with highly specific log functions.
> 
> > One thing for discussion is the logging prefix (the [mp3 @ 0xdeadface] 
> > thingy).
> > The current code checks whether the message contains a newline, and when it
> > doesn't it toggles a flag to skip the prefix on the next invocation. Since 
> > we
> > don't want such global flags, I see two possibilities:
> > * just abolish the prefices (what I did)
> > * print them always
> > Comments/other suggestions welcome.
> > ---
> 
> I'd print it always, but introduce a set of functions that allow you
> print to a buffer and then use av_log to print it with a sutffix as
> normal.

That smells like a rather annoying complication to me. There are really
quite many calls using this pattern.

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

Re: [libav-devel] [PATCH 05/14] lavc: Add coded bitstream read/write support for H.264

[Anton Khirnov]

Quoting Mark Thompson (2017-05-26 01:37:21)
> On 21/05/17 09:46, Anton Khirnov wrote:
> > Quoting Mark Thompson (2017-05-14 23:24:11)
> >> +
> >> +start = end = 6;
> >> +for (i = 0; i < count; i++) {
> >> +size = AV_RB16(data + end) + 2;
> >> +if (end + size > frag->data_size)
> >> +return AVERROR_INVALIDDATA;
> >> +end += size;
> >> +}
> >> +err = ff_h2645_packet_split(, frag->data + start, end - 
> >> start,
> >> +ctx, 1, 2, AV_CODEC_ID_H264);
> >> +if (err < 0) {
> >> +av_log(ctx, AV_LOG_ERROR, "Failed to split AVCC SPSs.\n");
> >> +return err;
> >> +}
> >> +
> >> +err = cbs_h2645_fragment_add_nals(ctx, frag, );
> >> +ff_h2645_packet_uninit();
> > 
> > You could store the packet in the context and call uninit only once at
> > the end. It doesn't need to (and shouldn't) be called after every split.
> 
> Ah, right - you can keep splitting into the same packet structure and then 
> add all of the NAL units and uninit once at the end.  I didn't notice that.
> 
> Yeah, that would be nicer.  I don't see any reason why it shouldn't stay on 
> the stack, though.

I mean once per whole filter lifetime. That way you don't keep
allocating and freeing the RBSP buffer. And it's slightly safer against
leaks.

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

Re: [libav-devel] [PATCH 04/14] lavc: Add coded bitstream read/write API

[Anton Khirnov]

Quoting Mark Thompson (2017-05-26 00:46:09)
> On 20/05/17 07:35, Anton Khirnov wrote:
> > Quoting Mark Thompson (2017-05-14 23:24:10)
> >> +int ff_cbs_insert_unit(CodedBitstreamContext *ctx,
> >> +   CodedBitstreamFragment *frag,
> >> +   int position, int type, void *content)
> >> +{
> >> +int err;
> >> +
> >> +if (position == -1)
> >> +position = frag->nb_units;
> >> +if (position < 0 || position > frag->nb_units)
> >> +return AVERROR(EINVAL);
> >> +
> >> +err = av_reallocp_array(>units,
> >> +frag->nb_units + 1,
> >> +sizeof(*frag->units));
> > 
> > If I'm reading right, units themselves need to be uninited, so reallocp
> > is not what you want here.
> 
> Not sure what you're getting at here?  There aren't any pointers to the unit 
> structure, so if it gets moved when adding one this is fine.

reallocp frees the array on failure, which leaks the contents of units.

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

Re: [libav-devel] [PATCH 1/3] build: Skip generating .version files when reconfiguring

[Anton Khirnov]

The commit message could use more detail, it's not entirely obvious why
this is the correct thing to do.

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

Re: [libav-devel] [PATCH 002/200] lavu: support AVChannelLayout AVOptions

[Anton Khirnov]

Quoting Luca Barbato (2017-05-23 22:08:49)
> On 5/17/17 7:46 PM, Vittorio Giovara wrote:
> > +av_channel_layout_uninit(dst);
> > +return av_channel_layout_copy(dst, channel_layout);
> 
> Maybe put the uninit directly in the layout_copy so there isn't risk to
> leak memory if you forget.

I am against this. Freeing memory should be explicit.

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

Re: [libav-devel] [PATCH 05/14] lavc: Add coded bitstream read/write support for H.264

[Anton Khirnov]

 "first byte %u.", *data);
> +return AVERROR_INVALIDDATA;
> +}
> +
> +h264->nal_length_size = (data[4] & 3) + 1;
> +count = data[5] & 0x1f;

Unchecked reads. I'd use the bytestream2 API.

> +
> +start = end = 6;
> +for (i = 0; i < count; i++) {
> +size = AV_RB16(data + end) + 2;
> +if (end + size > frag->data_size)
> +return AVERROR_INVALIDDATA;
> +end += size;
> +}
> +err = ff_h2645_packet_split(, frag->data + start, end - start,
> +ctx, 1, 2, AV_CODEC_ID_H264);
> +if (err < 0) {
> +av_log(ctx, AV_LOG_ERROR, "Failed to split AVCC SPSs.\n");
> +return err;
> +}
> +
> +err = cbs_h2645_fragment_add_nals(ctx, frag, );
> +ff_h2645_packet_uninit();

You could store the packet in the context and call uninit only once at
the end. It doesn't need to (and shouldn't) be called after every split.

> +static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
> +  CodedBitstreamUnit *unit)
> +{
> +BitstreamContext bc;
> +    int err;
> +
> +err = bitstream_init(, unit->data, 8 * unit->data_size);
> +if (err < 0)
> +return err;
> +
> +switch (unit->type) {
> +case H264_NAL_SPS:
> +{
> +H264RawSPS *sps;
> +
> +sps = av_mallocz(sizeof(*sps));
> +if (!sps)
> +return AVERROR(ENOMEM);
> +err = cbs_h264_read_sps(ctx, , sps);
> +if (err < 0) {
> +av_free(sps);
> +return err;
> +}
> +
> +cbs_h264_replace_sps(ctx, sps);

Should probably check the return value. Same below.


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

Re: [libav-devel] [PATCH 2/3] log: move all the "advanced" logging from lavu into cmdutils

[Anton Khirnov]

Quoting Hendrik Leppkes (2017-05-20 21:20:58)
> On Sat, May 20, 2017 at 9:15 PM, Anton Khirnov <an...@khirnov.net> wrote:
> > Quoting Martin Storsjö (2017-05-20 20:28:35)
> >> On Sat, 20 May 2017, Anton Khirnov wrote:
> >>
> >> > The default logging callback in lavu currently contains several
> >> > "advanced" features, such as
> >> > - suppressing repeated messages
> >> > - automatically hiding the log prefix
> >> > - color support
> >> > They add significant complexity to the logging callback and - more
> >> > importantly - global state, making logging not thread-safe (and strictly
> >> > speaking introducing UB).
> >> >
> >> > Many (perhaps most) of our callers either do not care for such fancy
> >> > features, or already use a custom logging callback. Therefore, it is
> >> > better to move them to cmdutils (for use by avtools) and leave the
> >> > default logging callback simple, straightforward and safe.
> >> > ---
> >> > I hope everyone agrees that having global state in the default logging 
> >> > callback
> >> > is a BadThing(tm).
> >> >
> >> > One thing for discussion is the logging prefix (the [mp3 @ 0xdeadface] 
> >> > thingy).
> >> > The current code checks whether the message contains a newline, and when 
> >> > it
> >> > doesn't it toggles a flag to skip the prefix on the next invocation. 
> >> > Since we
> >> > don't want such global flags, I see two possibilities:
> >> > * just abolish the prefices (what I did)
> >> > * print them always
> >> > Comments/other suggestions welcome.
> >>
> >> I kinda would like to keep the prefixes. Instead I'd prefer to always
> >> print them - in how many places do we even print a single log line by
> >> doing multiple av_log calls? I guess those can be fixed somehow (like by
> >> concatenating things into a local buffer in the caller).
> >
> > From what I recall it's not that rare, plus I'd rather not force this
> > extra constraint on all calls to av_log().
> >
> > One thing we could consider is changing the prefix into a suffix, which
> > is appended to the messages that end in a newline.
> >
> 
> That would look rather ugly. The good thing about the prefix is that
> its somewhat consistent in format and always at the beginning, making
> scanning for certain entries with your eyes relatively easy.

I certainly agree that it's less than perfect, but Martin's proposal has more
significant disadvantages IMO. And note that this will only be seen in programs
that do not bother to implement their own log callback (i.e. not in avconv and
other tools).

Or do you propose something else?

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

Re: [libav-devel] [PATCH 2/3] log: move all the "advanced" logging from lavu into cmdutils

[Anton Khirnov]

Quoting Martin Storsjö (2017-05-20 20:28:35)
> On Sat, 20 May 2017, Anton Khirnov wrote:
> 
> > The default logging callback in lavu currently contains several
> > "advanced" features, such as
> > - suppressing repeated messages
> > - automatically hiding the log prefix
> > - color support
> > They add significant complexity to the logging callback and - more
> > importantly - global state, making logging not thread-safe (and strictly
> > speaking introducing UB).
> >
> > Many (perhaps most) of our callers either do not care for such fancy
> > features, or already use a custom logging callback. Therefore, it is
> > better to move them to cmdutils (for use by avtools) and leave the
> > default logging callback simple, straightforward and safe.
> > ---
> > I hope everyone agrees that having global state in the default logging 
> > callback
> > is a BadThing(tm).
> >
> > One thing for discussion is the logging prefix (the [mp3 @ 0xdeadface] 
> > thingy).
> > The current code checks whether the message contains a newline, and when it
> > doesn't it toggles a flag to skip the prefix on the next invocation. Since 
> > we
> > don't want such global flags, I see two possibilities:
> > * just abolish the prefices (what I did)
> > * print them always
> > Comments/other suggestions welcome.
> 
> I kinda would like to keep the prefixes. Instead I'd prefer to always 
> print them - in how many places do we even print a single log line by 
> doing multiple av_log calls? I guess those can be fixed somehow (like by 
> concatenating things into a local buffer in the caller).

From what I recall it's not that rare, plus I'd rather not force this
extra constraint on all calls to av_log().

One thing we could consider is changing the prefix into a suffix, which
is appended to the messages that end in a newline.

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

[libav-devel] [PATCH 1/3] cmdutils: drop libavformat/network.h include

[Anton Khirnov]

It is not a public header and has not been used since
10173c0e58e557582dbd659f42c6aa164a8682db
---
 avtools/cmdutils.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/avtools/cmdutils.c b/avtools/cmdutils.c
index b0445eb..a19df30 100644
--- a/avtools/cmdutils.c
+++ b/avtools/cmdutils.c
@@ -47,9 +47,6 @@
 #include "libavutil/cpu.h"
 #include "avversion.h"
 #include "cmdutils.h"
-#if CONFIG_NETWORK
-#include "libavformat/network.h"
-#endif
 #if HAVE_SYS_RESOURCE_H
 #include 
 #include 
-- 
2.0.0

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

[libav-devel] [PATCH 2/3] log: move all the "advanced" logging from lavu into cmdutils

[Anton Khirnov]

The default logging callback in lavu currently contains several
"advanced" features, such as
- suppressing repeated messages
- automatically hiding the log prefix
- color support
They add significant complexity to the logging callback and - more
importantly - global state, making logging not thread-safe (and strictly
speaking introducing UB).

Many (perhaps most) of our callers either do not care for such fancy
features, or already use a custom logging callback. Therefore, it is
better to move them to cmdutils (for use by avtools) and leave the
default logging callback simple, straightforward and safe.
---
I hope everyone agrees that having global state in the default logging callback
is a BadThing(tm).

One thing for discussion is the logging prefix (the [mp3 @ 0xdeadface] thingy).
The current code checks whether the message contains a newline, and when it
doesn't it toggles a flag to skip the prefix on the next invocation. Since we
don't want such global flags, I see two possibilities:
* just abolish the prefices (what I did)
* print them always
Comments/other suggestions welcome.
---
 avtools/avconv.c|   2 +-
 avtools/avplay.c|   2 +-
 avtools/avprobe.c   |   1 +
 avtools/cmdutils.c  | 153 ++--
 avtools/cmdutils.h  |   2 +
 libavutil/log.c | 134 +
 libavutil/version.h |   2 +-
 7 files changed, 156 insertions(+), 140 deletions(-)

diff --git a/avtools/avconv.c b/avtools/avconv.c
index 719d289..6a1186d 100644
--- a/avtools/avconv.c
+++ b/avtools/avconv.c
@@ -2899,7 +2899,7 @@ int main(int argc, char **argv)
 
 register_exit(avconv_cleanup);
 
-av_log_set_flags(AV_LOG_SKIP_REPEATED);
+logging_init();
 parse_loglevel(argc, argv, options);
 
 avcodec_register_all();
diff --git a/avtools/avplay.c b/avtools/avplay.c
index b6dbc52..0c7cdf0 100644
--- a/avtools/avplay.c
+++ b/avtools/avplay.c
@@ -3009,7 +3009,7 @@ int main(int argc, char **argv)
 {
 int flags;
 
-av_log_set_flags(AV_LOG_SKIP_REPEATED);
+logging_init();
 parse_loglevel(argc, argv, options);
 
 /* register all codecs, demux and protocols */
diff --git a/avtools/avprobe.c b/avtools/avprobe.c
index a9ca193..3a98519 100644
--- a/avtools/avprobe.c
+++ b/avtools/avprobe.c
@@ -1170,6 +1170,7 @@ int main(int argc, char **argv)
 exit(1);
 
 register_exit(avprobe_cleanup);
+logging_init();
 
 options = real_options;
 parse_loglevel(argc, argv, options);
diff --git a/avtools/cmdutils.c b/avtools/cmdutils.c
index a19df30..070b5ff 100644
--- a/avtools/cmdutils.c
+++ b/avtools/cmdutils.c
@@ -19,17 +19,34 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "config.h"
+
 #include 
 #include 
 #include 
 #include 
 #include 
 
+#if HAVE_VALGRIND_VALGRIND_H
+#include 
+/* this is the log level at which valgrind will output a full backtrace */
+#define BACKTRACE_LOGLEVEL AV_LOG_ERROR
+#endif
+
+#if HAVE_UNISTD_H
+#include 
+#endif
+#if HAVE_IO_H
+#include 
+#endif
+
+#if HAVE_SETCONSOLETEXTATTRIBUTE
+#include 
+#endif
+
 /* Include only the enabled headers since some compilers (namely, Sun
Studio) will not omit unused inline functions and create undefined
references to libraries that are not being built. */
-
-#include "config.h"
 #include "libavformat/avformat.h"
 #include "libavfilter/avfilter.h"
 #include "libavdevice/avdevice.h"
@@ -57,6 +74,8 @@ AVDictionary *format_opts, *codec_opts, *resample_opts;
 
 static const int this_year = 2017;
 
+static int log_level = AV_LOG_INFO;
+
 void init_opts(void)
 {
 #if CONFIG_SWSCALE
@@ -729,7 +748,7 @@ int opt_loglevel(void *optctx, const char *opt, const char 
*arg)
 
 for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
 if (!strcmp(log_levels[i].name, arg)) {
-av_log_set_level(log_levels[i].level);
+log_level = log_levels[i].level;
 return 0;
 }
 }
@@ -742,7 +761,7 @@ int opt_loglevel(void *optctx, const char *opt, const char 
*arg)
 av_log(NULL, AV_LOG_FATAL, "\"%s\"\n", log_levels[i].name);
 exit_program(1);
 }
-av_log_set_level(level);
+log_level = level;
 return 0;
 }
 
@@ -1697,3 +1716,129 @@ const char *media_type_string(enum AVMediaType 
media_type)
 default:  return "unknown";
 }
 }
+
+#define NB_LEVELS 8
+#if HAVE_SETCONSOLETEXTATTRIBUTE
+static const uint8_t color[NB_LEVELS] = { 12, 12, 12, 14, 7, 10, 11, 8};
+static int16_t background, attr_orig;
+static HANDLE con;
+#define set_color(x)  SetConsoleTextAttribute(con, background | color[x])
+#define reset_color() SetConsoleTextAttribute(con, attr_orig)
+#define print_256color(x)
+#else
+static const uint8_t color[NB_LEVELS] = {
+0x41, 0x41, 0x11, 0x03, 9, 0x02, 0x06, 0x07
+};
+#define set_color(x)  fprintf(stderr, "\033[%d;3%dm", color[x] >> 4, 
color[x]&15)
+#define print_256color(x) fprintf(stderr, 

[libav-devel] [PATCH 3/3] log: deprecate now unused AV_LOG_SKIP_REPEATED

[Anton Khirnov]

---
 libavutil/log.h | 9 +++--
 libavutil/version.h | 3 +++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavutil/log.h b/libavutil/log.h
index ce00bcc..0fc8b9c 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -249,15 +249,12 @@ void av_log_default_callback(void *avcl, int level, const 
char *fmt,
  */
 const char* av_default_item_name(void* ctx);
 
+#if FF_API_SKIP_REPEATED
 /**
- * Skip repeated messages, this requires the user app to use av_log() instead 
of
- * (f)printf as the 2 would otherwise interfere and lead to
- * "Last message repeated x times" messages below (f)printf messages with some
- * bad luck.
- * Also to receive the last, "last repeated" line if any, the user app must
- * call av_log(NULL, AV_LOG_QUIET, ""); at the end
+ * @deprecated this flag has no effect
  */
 #define AV_LOG_SKIP_REPEATED 1
+#endif
 void av_log_set_flags(int arg);
 
 /**
diff --git a/libavutil/version.h b/libavutil/version.h
index 38e1906..a539ea1 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -96,6 +96,9 @@
 #ifndef FF_API_CRYPTO_SIZE_T
 #define FF_API_CRYPTO_SIZE_T(LIBAVUTIL_VERSION_MAJOR < 57)
 #endif
+#ifndef FF_API_SKIP_REPEATED
+#define FF_API_SKIP_REPEATED(LIBAVUTIL_VERSION_MAJOR < 57)
+#endif
 
 
 /**
-- 
2.0.0

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