Re: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 5:44 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support
> 
> On 13/03/2019 02:46, Guo, Yejun wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> >> Of Mark Thompson
> >> Sent: Wednesday, March 13, 2019 8:18 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support
> >>
> >> ---
> >>  libavcodec/vaapi_encode.c   | 128
> >> 
> >>  libavcodec/vaapi_encode.h   |  11 +++
> >>  libavcodec/vaapi_encode_h264.c  |   2 +
> >>  libavcodec/vaapi_encode_h265.c  |   2 +
> >>  libavcodec/vaapi_encode_mpeg2.c |   2 +
> >>  libavcodec/vaapi_encode_vp8.c   |   2 +
> >>  libavcodec/vaapi_encode_vp9.c   |   2 +
> >>  7 files changed, 149 insertions(+)
> >>
> >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> >> index 2dda451882..03a7f5ce3e 100644
> >> --- a/libavcodec/vaapi_encode.c
> >> +++ b/libavcodec/vaapi_encode.c
> >> @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext
> >> *avctx,
> >>  int err, i;
> >>  char data[MAX_PARAM_BUFFER_SIZE];
> >>  size_t bit_len;
> >> +AVFrameSideData *sd;
> >>
> >>  av_log(avctx, AV_LOG_DEBUG, "Issuing encode for
> >> pic %"PRId64"/%"PRId64" "
> >> "as type %s.\n", pic->display_order, pic->encode_order,
> >> @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext
> >> *avctx,
> >>  }
> >>  }
> >>
> >> +sd = av_frame_get_side_data(pic->input_image,
> >> +AV_FRAME_DATA_REGIONS_OF_INTEREST);
> >> +
> >> +#if VA_CHECK_VERSION(1, 0, 0)
> >> +if (sd && ctx->roi_allowed) {
> >> +const AVRegionOfInterest *roi;
> >> +int nb_roi;
> >> +VAEncMiscParameterBuffer param_misc = {
> >> +.type = VAEncMiscParameterTypeROI,
> >> +};
> >> +VAEncMiscParameterBufferROI param_roi;
> >> +// VAAPI requires the second structure to immediately follow the
> >> +// first in the parameter buffer, but they have different natural
> >> +// alignment on 64-bit architectures (4 vs. 8, since the ROI
> >> +// structure contains a pointer).  This means we can't make a
> >> +// single working structure, nor can we make the buffer
> >> +// separately and map it because dereferencing the second pointer
> >> +// would be undefined.  Therefore, construct the two parts
> >> +// separately and combine them into a single character buffer
> >> +// before uploading.
> >> +char param_buffer[sizeof(param_misc) + sizeof(param_roi)];
> >> +int i, v;
> >> +
> >> +roi = (const AVRegionOfInterest*)sd->data;
> >> +av_assert0(roi->self_size && sd->size % roi->self_size == 0);
> >
> > it is possible if the user forget to set the value of roi->self_size.
> > assert is not feasible.
> 
> Not setting self_size violates the API contract ("Must be set to the size of 
> this
> data structure") and therefore invokes undefined behaviour.  Aborting when
> undefined behaviour is first detected is the correct response.
> 
> >> +nb_roi = sd->size / roi->self_size;
> >> +if (nb_roi > ctx->roi_regions) {
> >> +if (!ctx->roi_warned) {
> >> +av_log(avctx, AV_LOG_WARNING, "More ROIs set than "
> >> +   "supported by driver (%d > %d).\n",
> >> +   nb_roi, ctx->roi_regions);
> >> +ctx->roi_warned = 1;
> >> +}
> >> +nb_roi = ctx->roi_regions;
> >> +}
> >> +
> >> +pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi));
> >> +if (!pic->roi) {
> >> +err = AVERROR(ENOMEM);
> >> +goto fail;
> >> +}
> >
> > shall we add comments here to explain the list visit order?
> 
> Sure, added.
> 
> >> +for (i = 0; i < nb_roi; i++) {
> >> +roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * 
> >> i);
> >
> > same comment as libx264
> 
> I guess, though I'm not sure filling the code with guards detecting undefined
> behaviour cases really has any value.
> 
> I've added an additional note on the API that the value must be the same on
> all entries.

I personally prefer code check and report error, anyway, I do not insist it.

> 
> >> +
> >> +v = roi->qoffset.num * ctx->roi_quant_range / 
> >> roi->qoffset.den;
> >
> > shall we check here if roi->qoffset-den is zero?
> 
> Sure, I'll add an assert for the invalid fraction since the API requires 
> [-1,+1].
> 
> >> +av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) -
> >>> %+d.\n",
> >> +   roi->x, roi->y, roi->width, roi->height, v);
> >> +

Re: [FFmpeg-devel] [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

[Fāng-ruì Sòng]

On Thu, Mar 14, 2019 at 10:36 AM Fāng-ruì Sòng  wrote:
>
> On Thu, Mar 14, 2019 at 9:59 AM Henrik Gramner  wrote:
> >
> > On Wed, Feb 20, 2019 at 8:03 PM Fāng-ruì Sòng
> >  wrote:
> > > --- a/libavutil/mem.h
> > > +++ b/libavutil/mem.h
> > >
> > > +#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__))
> > > +#define DECLARE_HIDDEN __attribute__ ((visibility ("hidden")))
> > > +#else
> > > +#define DECLARE_HIDDEN
> > > +#endif
> >
> > libavutil/mem.h is a public header so any defines added should have
> > appropriate prefixes (yes, the existing defines violate this which is
> > something that should be addressed, but that's a different issue).
>
> Do you have suggestions on the naming? av_declare_hidden?
>
> > Alternatively maybe those macros should be moved to some internal
> > header because I don't really see any value of having inline asm
> > support macros public.
>
> That would change too many files. I'd rather not do that in this patch.

I still don't have access to my neomutt so I send the patch as an
attached file via the webmail


-- 
宋方睿
From 64fc0ce5fa80d6d2b6bc93f539cca48a844bc672 Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Wed, 20 Feb 2019 16:25:46 +0800
Subject: [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden")
 for __GNUC__

Inline asm code assumes these DECLARE_ASM_ALIGNED declared global constants are non-preemptive, e.g.

libavcodec/x86/cabac.h
"lea"MANGLE(ff_h264_cabac_tables)", %0  \n\t"

These constants are currently defined in C files with default visibility.
Such object files cannot link to a shared object without
-Wl,-Bsymbolic or -Wl,--version-script,libavcodec/libavcodec.ver:

ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol ff_h264_cabac_tables; recompile with -fPIC

It is better to express the intention explicitly and mark such global
constants hidden. -Bsymbolic is dangerous as it breaks C++ semantics
about address uniqueness of inline functions, type_info, ...
--version-script can have the same problem unless used very carefully.

DECLARE_ASM_CONST uses the "static" specifier to indicate internal
linkage. The visibility annotation is unnecessary.

Signed-off-by: Fangrui Song 
---
 libavutil/mem.h | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavutil/mem.h b/libavutil/mem.h
index 5fb1a02dd9..98a14b826b 100644
--- a/libavutil/mem.h
+++ b/libavutil/mem.h
@@ -100,6 +100,12 @@
  * @param v Name of the variable
  */
 
+#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__))
+#define av_visibility_hidden __attribute__ ((visibility ("hidden")))
+#else
+#define av_visibility_hidden
+#endif
+
 #if defined(__INTEL_COMPILER) && __INTEL_COMPILER < 1110 || defined(__SUNPRO_C)
 #define DECLARE_ALIGNED(n,t,v)  t __attribute__ ((aligned (n))) v
 #define DECLARE_ASM_ALIGNED(n,t,v)  t __attribute__ ((aligned (n))) v
@@ -110,7 +116,7 @@
 #define DECLARE_ASM_CONST(n,t,v)static const t av_used __attribute__ ((aligned (FFMIN(n, 16 v
 #elif defined(__GNUC__) || defined(__clang__)
 #define DECLARE_ALIGNED(n,t,v)  t __attribute__ ((aligned (n))) v
-#define DECLARE_ASM_ALIGNED(n,t,v)  t av_used __attribute__ ((aligned (n))) v
+#define DECLARE_ASM_ALIGNED(n,t,v)  t av_used __attribute__ ((aligned (n))) av_visibility_hidden v
 #define DECLARE_ASM_CONST(n,t,v)static const t av_used __attribute__ ((aligned (n))) v
 #elif defined(_MSC_VER)
 #define DECLARE_ALIGNED(n,t,v)  __declspec(align(n)) t v
-- 
2.20.1

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

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Sun, Jing A]

On Wed, Mar 13, 2019 at 8:29 PM Sun, Jing A  wrote:

>
>
> > On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A 
> wrote:
>
> Hi Giovara,
>
>
>
> SVT HEVC has the interface to enable/disable sending a vui structure 
> in the HEVC bitstream, but supports no interface for setting the color 
> properties before encoding yet. I will be opening an issue in SVT HEVC 
> github asking if they have plans to add such feature, and will keep 
> you posted. In the meantime, I think it is not blocking the first 
> version of this plugin’s merging , is it?
>
>
>
>
>
> > It kind-of is, what is the point of setting the HDR information (or
> encoding in hevc at all) if you can't set the related color properties?
>
> > At the very least there should be a big fat warning that notifies 
> > users
> of this missing feature, when such information is lost.
>
> > --
>
> > Vittorio
>
>
>
> SVT HEVC’s HDR is also an on-off switch, please refer to 
> https://github.com/intel/SVT-HEVC/blob/master/Docs/svt-hevc_encoder_us
> er_guide.md
> .
>
>
>
> -Jing
>

>Sorry, I fail to see the logical connection between my comment and your reply.
>--
>Vittorio

The feature request is being considered: 
https://github.com/intel/SVT-HEVC/issues/148
-Jing
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Vittorio Giovara]

On Wed, Mar 13, 2019 at 8:29 PM Sun, Jing A  wrote:

>
>
> > On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A 
> wrote:
>
> Hi Giovara,
>
>
>
> SVT HEVC has the interface to enable/disable sending a vui structure in
> the HEVC bitstream, but supports no interface for setting the color
> properties before encoding yet. I will be opening an issue in SVT HEVC
> github asking if they have plans to add such feature, and will keep you
> posted. In the meantime, I think it is not blocking the first version of
> this plugin’s merging , is it?
>
>
>
>
>
> > It kind-of is, what is the point of setting the HDR information (or
> encoding in hevc at all) if you can't set the related color properties?
>
> > At the very least there should be a big fat warning that notifies users
> of this missing feature, when such information is lost.
>
> > --
>
> > Vittorio
>
>
>
> SVT HEVC’s HDR is also an on-off switch, please refer to
> https://github.com/intel/SVT-HEVC/blob/master/Docs/svt-hevc_encoder_user_guide.md
> .
>
>
>
> -Jing
>

Sorry, I fail to see the logical connection between my comment and your
reply.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

[Fāng-ruì Sòng]

On Thu, Mar 14, 2019 at 9:59 AM Henrik Gramner  wrote:
>
> On Wed, Feb 20, 2019 at 8:03 PM Fāng-ruì Sòng
>  wrote:
> > --- a/libavutil/mem.h
> > +++ b/libavutil/mem.h
> >
> > +#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__))
> > +#define DECLARE_HIDDEN __attribute__ ((visibility ("hidden")))
> > +#else
> > +#define DECLARE_HIDDEN
> > +#endif
>
> libavutil/mem.h is a public header so any defines added should have
> appropriate prefixes (yes, the existing defines violate this which is
> something that should be addressed, but that's a different issue).

Do you have suggestions on the naming? av_declare_hidden?

> Alternatively maybe those macros should be moved to some internal
> header because I don't really see any value of having inline asm
> support macros public.

That would change too many files. I'd rather not do that in this patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

[Henrik Gramner]

On Wed, Feb 20, 2019 at 8:03 PM Fāng-ruì Sòng
 wrote:
> --- a/libavutil/mem.h
> +++ b/libavutil/mem.h
>
> +#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__))
> +#define DECLARE_HIDDEN __attribute__ ((visibility ("hidden")))
> +#else
> +#define DECLARE_HIDDEN
> +#endif

libavutil/mem.h is a public header so any defines added should have
appropriate prefixes (yes, the existing defines violate this which is
something that should be addressed, but that's a different issue).

Alternatively maybe those macros should be moved to some internal
header because I don't really see any value of having inline asm
support macros public.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

[Fāng-ruì Sòng]

Ping :)

In short, when people build .so from ffmpeg .o/.a files, they no longer
need -Bsymbolic
-Bsymbolic is bad because it breaks C++ semantics, e.g. address uniqueness
of inline functions and their static local variables, uniqueness of
template specializations, address uniqueness of type_info objects (used for
C++ exceptions), etc

On Thu, Mar 7, 2019 at 11:53 AM Fāng-ruì Sòng  wrote:

> Sorry, wrong signed-off-by line..
>
> (isn't using my usual mail client mutt and i get very sick of the
> webmail...)
>
> On Thu, Mar 7, 2019 at 11:36 AM Fāng-ruì Sòng  wrote:
>
>> On Wed, Mar 6, 2019 at 4:14 PM Carl Eugen Hoyos 
>> wrote:
>>
>>> 2019-02-21 2:37 GMT+01:00, Fāng-ruì Sòng <
>>> maskray-at-google@ffmpeg.org>:
>>> > Sorry if this doesn't attach to the correct thread as I didn't
>>> > subscribe to this list and don't know the Message-ID of the thread.
>>>
>>> The thread works fine here with gmail but your patch was corrupted,
>>> you have to attach it.
>>>
>>> Carl Eugen
>>>
>>>
>> Sorry. Here is the attached patch (git format-patch -1 --stdout).
>>
>
>
> --
> 宋方睿
>


-- 
宋方睿
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avformat/smoothstreamingenc:add bitrate calculate

[Jun Li]

On Tue, Mar 12, 2019 at 6:24 PM Jun Li  wrote:

> Hi Carl,
> Thanks for review, is there any more issue I need to take care ?
> or any process I need to follow before applying the patch? I am new to the
> community, don't know too much.
>
> Thanks
> -Jun
>
> On Mon, Mar 11, 2019 at 11:44 AM Carl Eugen Hoyos 
> wrote:
>
>> 2019-03-11 18:58 GMT+01:00, Jun Li :
>>
>> > Smooth is not the only one need bitrate for manifest, Dash also need
>> this
>> > value for "Bandwidth" field:
>> > https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/dashenc.c#L730
>> > Although there is some difference, both the calculation are based on
>> > fragment size, which is the result after muxing.
>>
>> Thank you for the explanation!
>>
>> Carl Eugen
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>  pingx2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Sun, Jing A]

> On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A 
> mailto:jing.a@intel.com>> wrote:
Hi Giovara,

SVT HEVC has the interface to enable/disable sending a vui structure in the 
HEVC bitstream, but supports no interface for setting the color properties 
before encoding yet. I will be opening an issue in SVT HEVC github asking if 
they have plans to add such feature, and will keep you posted. In the meantime, 
I think it is not blocking the first version of this plugin’s merging , is it?


> It kind-of is, what is the point of setting the HDR information (or encoding 
> in hevc at all) if you can't set the related color properties?
> At the very least there should be a big fat warning that notifies users of 
> this missing feature, when such information is lost.
> --
> Vittorio

SVT HEVC’s HDR is also an on-off switch, please refer to 
https://github.com/intel/SVT-HEVC/blob/master/Docs/svt-hevc_encoder_user_guide.md.

-Jing
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/arbc: Check nb_segments before allocating and copying frame

[Michael Niedermayer]

Fixes: Timeout (30sec -> 2sec)
Fixes: 
13578/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ARBC_fuzzer-5685625527730176

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/arbc.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
index 841a9f10ac..52c9a197f9 100644
--- a/libavcodec/arbc.c
+++ b/libavcodec/arbc.c
@@ -121,6 +121,15 @@ static int decode_frame(AVCodecContext *avctx, void *data,
 if (avpkt->size < 10)
 return AVERROR_INVALIDDATA;
 
+bytestream2_init(>gb, avpkt->data, avpkt->size);
+bytestream2_skip(>gb, 8);
+nb_segments = bytestream2_get_le16(>gb);
+if (nb_segments == 0)
+keyframe = 0;
+
+if (7 * nb_segments > bytestream2_get_bytes_left(>gb))
+return AVERROR_INVALIDDATA;
+
 if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
 return ret;
 
@@ -130,12 +139,6 @@ static int decode_frame(AVCodecContext *avctx, void *data,
 return ret;
 }
 
-bytestream2_init(>gb, avpkt->data, avpkt->size);
-bytestream2_skip(>gb, 8);
-nb_segments = bytestream2_get_le16(>gb);
-if (nb_segments == 0)
-keyframe = 0;
-
 for (int i = 0; i < nb_segments; i++) {
 int resolution_flag;
 uint8_t fill[3];
-- 
2.21.0

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

Re: [FFmpeg-devel] [PATCH v1] avformat/rtpdec.h remove unused variable

[Michael Niedermayer]

On Wed, Mar 13, 2019 at 07:22:41AM +0800, Steven Liu wrote:
> 
> 
> > On Mar 13, 2019, at 05:26, Jun Li  wrote:
> > 
> > Looks like the variable 'cur_timestamp' is not used anywhere.
> > So remove this variable.
> > 
> > Signed-off-by: Jun Li 
> > ---
> > libavformat/rtpdec.h | 1 -
> > 1 file changed, 1 deletion(-)
> > 
> > diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
> > index 5a47d6f79d..9144edbe8b 100644
> > --- a/libavformat/rtpdec.h
> > +++ b/libavformat/rtpdec.h
> > @@ -154,7 +154,6 @@ struct RTPDemuxContext {
> > uint16_t seq;
> > uint32_t timestamp;
> > uint32_t base_timestamp;
> > -uint32_t cur_timestamp;
> > int64_t  unwrapped_timestamp;
> > int64_t  range_start_offset;
> > int max_payload_size;
> > -- 
> > 2.17.1
> > 
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> LGTM

will apply

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] Added more commandline options for libaom encoder.

[Sam John]

The following are the newly added options:
arnr_max_frames, arnr_strength, aq_mode, denoise_noise_level, 
denoise_block_size,
rc_undershoot_pct, rc_overshoot_pct, minsection_pct, maxsection_pct, 
frame_parallel,
enable_cdef, and enable_global_motion.

Also added macros for compiling for aom 1.0.0 and fixed the default values.
---
 libavcodec/libaomenc.c | 88 --
 1 file changed, 84 insertions(+), 4 deletions(-)

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index faec61cacd..7d4a5ac447 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -66,36 +66,65 @@ typedef struct AOMEncoderContext {
 struct FrameListData *coded_frame_list;
 int cpu_used;
 int auto_alt_ref;
+int arnr_max_frames;
+int arnr_strength;
+int aq_mode;
 int lag_in_frames;
 int error_resilient;
 int crf;
 int static_thresh;
 int drop_threshold;
+int denoise_noise_level;
+int denoise_block_size;
 uint64_t sse[4];
 int have_sse; /**< true if we have pending sse[] */
 uint64_t frame_number;
+int rc_undershoot_pct;
+int rc_overshoot_pct;
+int minsection_pct;
+int maxsection_pct;
+int frame_parallel;
 int tile_cols, tile_rows;
 int tile_cols_log2, tile_rows_log2;
 aom_superblock_size_t superblock_size;
 int uniform_tiles;
 int row_mt;
+int enable_cdef;
+int enable_global_motion;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
 [AOME_SET_CPUUSED]  = "AOME_SET_CPUUSED",
 [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL",
 [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF",
+[AOME_SET_ARNR_MAXFRAMES]   = "AOME_SET_ARNR_MAXFRAMES",
+[AOME_SET_ARNR_STRENGTH]= "AOME_SET_ARNR_STRENGTH",
 [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD",
 [AV1E_SET_COLOR_RANGE]  = "AV1E_SET_COLOR_RANGE",
 [AV1E_SET_COLOR_PRIMARIES]  = "AV1E_SET_COLOR_PRIMARIES",
 [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS",
 [AV1E_SET_TRANSFER_CHARACTERISTICS] = "AV1E_SET_TRANSFER_CHARACTERISTICS",
+[AV1E_SET_AQ_MODE]  = "AV1E_SET_AQ_MODE",
+[AV1E_SET_FRAME_PARALLEL_DECODING] = "AV1E_SET_FRAME_PARALLEL_DECODING",
 [AV1E_SET_SUPERBLOCK_SIZE]  = "AV1E_SET_SUPERBLOCK_SIZE",
 [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS",
 [AV1E_SET_TILE_ROWS]= "AV1E_SET_TILE_ROWS",
 #ifdef AOM_CTRL_AV1E_SET_ROW_MT
 [AV1E_SET_ROW_MT]   = "AV1E_SET_ROW_MT",
 #endif
+#ifdef AV1E_SET_DENOISE_NOISE_LEVEL
+[AV1E_SET_DENOISE_NOISE_LEVEL] =  "AV1E_SET_DENOISE_NOISE_LEVEL",
+#endif
+#ifdef AV1E_SET_DENOISE_BLOCK_SIZE
+[AV1E_SET_DENOISE_BLOCK_SIZE] =   "AV1E_SET_DENOISE_BLOCK_SIZE",
+#endif
+#ifdef AV1E_SET_MAX_REFERENCE_FRAMES
+[AV1E_SET_MAX_REFERENCE_FRAMES] = "AV1E_SET_MAX_REFERENCE_FRAMES",
+#endif
+#ifdef AV1E_SET_ENABLE_GLOBAL_MOTION
+[AV1E_SET_ENABLE_GLOBAL_MOTION] = "AV1E_SET_ENABLE_GLOBAL_MOTION",
+#endif
+[AV1E_SET_ENABLE_CDEF]  = "AV1E_SET_ENABLE_CDEF",
 };
 
 static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
@@ -567,10 +596,14 @@ static av_cold int aom_init(AVCodecContext *avctx,
 
 // 0-100 (0 => CBR, 100 => VBR)
 enccfg.rc_2pass_vbr_bias_pct   = round(avctx->qcompress * 100);
-if (avctx->bit_rate)
+if (ctx->minsection_pct >= 0)
+enccfg.rc_2pass_vbr_minsection_pct = ctx->minsection_pct;
+else if (avctx->bit_rate)
 enccfg.rc_2pass_vbr_minsection_pct =
 avctx->rc_min_rate * 100LL / avctx->bit_rate;
-if (avctx->rc_max_rate)
+if (ctx->maxsection_pct >= 0)
+enccfg.rc_2pass_vbr_maxsection_pct = ctx->maxsection_pct;
+else if (avctx->rc_max_rate)
 enccfg.rc_2pass_vbr_maxsection_pct =
 avctx->rc_max_rate * 100LL / avctx->bit_rate;
 
@@ -582,6 +615,11 @@ static av_cold int aom_init(AVCodecContext *avctx,
 avctx->rc_initial_buffer_occupancy * 1000LL / avctx->bit_rate;
 enccfg.rc_buf_optimal_sz = enccfg.rc_buf_sz * 5 / 6;
 
+if (ctx->rc_undershoot_pct >= 0)
+enccfg.rc_undershoot_pct = ctx->rc_undershoot_pct;
+if (ctx->rc_overshoot_pct >= 0)
+enccfg.rc_overshoot_pct = ctx->rc_overshoot_pct;
+
 // _enc_init() will balk if kf_min_dist differs from max w/AOM_KF_AUTO
 if (avctx->keyint_min >= 0 && avctx->keyint_min == avctx->gop_size)
 enccfg.kf_min_dist = avctx->keyint_min;
@@ -643,7 +681,12 @@ static av_cold int aom_init(AVCodecContext *avctx,
 codecctl_int(avctx, AOME_SET_CPUUSED, ctx->cpu_used);
 if (ctx->auto_alt_ref >= 0)
 codecctl_int(avctx, AOME_SET_ENABLEAUTOALTREF, ctx->auto_alt_ref);
-
+if (ctx->arnr_max_frames >= 0)
+codecctl_int(avctx, AOME_SET_ARNR_MAXFRAMES,   ctx->arnr_max_frames);
+if (ctx->arnr_strength >= 0)
+codecctl_int(avctx, AOME_SET_ARNR_STRENGTH,ctx->arnr_strength);
+if (ctx->enable_cdef >= 0)
+

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Vittorio Giovara]

On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A  wrote:

> Hi Giovara,
>
>
>
> SVT HEVC has the interface to enable/disable sending a vui structure in
> the HEVC bitstream, but supports no interface for setting the color
> properties before encoding yet. I will be opening an issue in SVT HEVC
> github asking if they have plans to add such feature, and will keep you
> posted. In the meantime, I think it is not blocking the first version of
> this plugin’s merging , is it?
>
>
>
It kind-of is, what is the point of setting the HDR information (or
encoding in hevc at all) if you can't set the related color properties?
At the very least there should be a big fat warning that notifies users of
this missing feature, when such information is lost.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavcodec/zmbvenc: Add support for RGB formats

[Matthew Fearnley]

> On 12 Mar 2019, at 11:46, Tomas Härdin  wrote:
> 
> tis 2019-03-12 klockan 10:27 + skrev Matthew Fearnley:
>>> On 11 Mar 2019, at 10:37, Tomas Härdin  wrote:
>>> 
>>> 
>>> There's some justification for adding sub-8bpp, like BMP. bmp.c
>>> converts all of them except GRAY8 to PAL8. Bitdepths besides 1, 4
>>> and 8
>>> don't work at all.
>>> 
>>> One way to at least allow both the bmp and zmbv encoders to do sub-
>>> 8bpp 
>>> from PAL8 would be to keep track of the maximum number of colors in
>>> some appropriate struct.
>>> 
>>> Adding proper sub-8bpp support would involve a lot of libsws
>>> headache I
>>> suspect.
>> 
>> It occurs to me that adding sub-8bpp has some implications:
>> 
>> My current understanding (I could be wrong) is that FFmpeg tends to
>> detect the pix_fmt based on the first frame. If FFmpeg detects the
>> first frame as e.g. PAL4, and chooses that as its output, that means
>> the rest of the video will have to be encodable as PAL4, otherwise it
>> (obviously) won’t be encoded properly.
>> 
>> So adding a PAL4 format puts a new constraint on encoders (inside and
>> outside FFmpeg) to not encode frames in a way that looks like PAL4,
>> unless the whole video will be encodable that way.
> 
> Yes, FFmpeg will probe the initial format of the video and audio.
> Nothing says these are constant. There are FATE samples specifically
> for files that change resolution. Since ZMBV is a DOS capture codec,
> and DOS programs frequently change resolution and colordepth, this is
> indeed something we have to think about. Example: DOS boots into mode
> 3h, 80x25 16-color text. A recording may start in this mode, then
> switch to mode 13h (320x200 256 colors graphics), if I understand the
> format correctly.
DOSBox actually avoids this issue by outputting to a new file whenever it 
detects a change in colour depth, dimensions or FPS. So in practice, these 
remain constant over a single video.

I’ve tried stitching two AVI files together (palette + RGB, same dimensions), 
and found that if a palette-capable format like ‘png’ is chosen, all of the 
resulting AVI file is encoded with a palette, and RGB sections are dithered.  
That possibly suggests FFmpeg doesn’t like changing format halfway through 
videos, for AVI at least?
> 
>> If FFmpeg supports PAL8 only, then it can be tempting to optimise
>> videos to encode as sub-8bpp whenever possible, knowing they will
>> always (in FFmpeg at least) decode to PAL8. But this could break
>> format detection for tools outside FFmpeg, if they choose to add sub-
>> 8bpp support.
>> 
>> The safest thing FFmpeg can do is to always decode sub-8bpp to PAL8,
>> and to emit PAL8 frames as exactly 8bpp (where applicable). It could
>> still offer encoding formats for PAL1/2/4, but these formats could
>> only be detected by scanning the whole video.
>> 
>> The suggestion of bits_per_raw_sample sounds interesting. What would
>> that look like in practice?
> 
> The decoder would set bits_per_raw_sample on every keyframe. It and
> resolution might change during the course of a ZMBV file, as explained
> above.
> 
> I think frivolously changing the bitdepth is a bad idea, at least
> inside the encoder. If the encoder is fed with changing bitdepths then
> it's a different story.
> 
>>> Just a small thing to be clear: ZMBV_ENABLE_24BPP is not defined
>>> anywhere, so we're free to do however we want with it. It's not
>>> going
>>> to break anyone's workflow unless they were foolish enough to
>>> encode
>>> 24-bit ZMBVs outside of the non-existing spec.
>> 
>> True. But they might be understandably puzzled if they encode as 24-
>> bit, and then find the channels swapped when they decode.
> 
> Yeah, that's why we'd need to get this nailed down
> 
>> Thanks for writing the email to the DOSBox crew.
>> If they choose a channel order, then we have good grounds for fixing
>> the encoder (if need be), and implementing the decoder in the same
>> way.
>> It occurs to me that they might (in theory) also want to specify 2/4
>> byte alignment on RGB, like with the MVs.  My gut says there’d be
>> very little benefit though, and it would only be seen with strange
>> video / block widths.
>> 
>> It also occurs to me this will may warrant a version bump in the
>> format, to give an easy error case for decoders that don’t expect it.
>> Particularly if our decoder has to redefine its channel order.
> 
> Are there any decoders besides ours and dosbox's?
I don’t know of any public implementations.

(That said, I have written a stand-alone tool that encodes/decodes ZMBV, but 
I’ve not published it anywhere. It’s based heavily on the DOSBox 
implementation.)
> It also turns out the creator of this codec is Harekiet, who hangs out
> in #revision on IRCnet
Ah ok, do you know him?
It sounds like he’s not concerned too much about what direction we take the 
format in. But I guess anything we implement may not get made official unless 
DOSBox adds decoding support.

By the way, I’m happy for this 

Re: [FFmpeg-devel] [PATCH v6] Improved the performance of 1 decode + N filter graphs and adaptive bitrate.

[Michael Niedermayer]

On Wed, Mar 13, 2019 at 05:54:53PM -0400, Shaofei Wang wrote:
> It enabled multiple simple filter graph concurrency, which bring above about
> 4%~20% improvement in some 1:N scenarios by CPU or GPU acceleration
> 
> Below are some test cases and comparison as reference.
> (Hardware platform: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz)
> (Software: Intel iHD driver - 16.9.00100, CentOS 7)

This causes aborts.
I suggest you test it with a fuzzer
example:
Program received signal SIGABRT, Aborted.
0x7fffefeaec37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
Python Exception  No module named gdb.frames: 
#0  0x7fffefeaec37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x7fffefeb2028 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x0042bf71 in strict_pthread_join (thread=0, value_ptr=0x0) at 
./libavutil/thread.h:55
#3  0x0042d412 in ffmpeg_cleanup (ret=1) at fftools/ffmpeg.c:526
#4  0x004251a8 in exit_program (ret=1) at fftools/cmdutils.c:139
#5  0x0043f152 in main (argc=6, argv=0x7fffe3a8) at 
fftools/ffmpeg.c:5032

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpegaudiodec_template: add ability to check CRC

[Lynne]

8 Mar 2019, 17:48 by d...@lynne.ee:

>
>
>
> 7 Mar 2019, 21:18 by > mich...@niedermayer.cc 
> > :
>
>> On Wed, Mar 06, 2019 at 07:09:52PM +0100, Lynne wrote:
>>
>>> A lot of files have CRC included.
>>> The CRC only covers 34 bytes at most from the frame but it should still be
>>> enough for some amount of error detection.
>>>
>>> mpegaudiodec_template.c |   20 +---
>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>> e8276f62fa92aa3f78e53b182b4ca7a2a460754c  
>>> 0001-mpegaudiodec_template-add-ability-to-check-CRC.patch
>>> From e1f4410f35d3d7f774a0de59ab72764033d14900 Mon Sep 17 00:00:00 2001
>>> From: Lynne <>> >>> d...@lynne.ee > 
>>> d...@lynne.ee >> >
>>> Date: Wed, 6 Mar 2019 17:04:04 +
>>> Subject: [PATCH] mpegaudiodec_template: add ability to check CRC
>>>
>>> A lot of files have CRC included.
>>> The CRC only covers 34 bytes at most from the frame but it should still be
>>> enough for some amount of error detection.
>>> ---
>>>  libavcodec/mpegaudiodec_template.c | 20 +---
>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/mpegaudiodec_template.c 
>>> b/libavcodec/mpegaudiodec_template.c
>>> index 9cce88e263..0881b60bf5 100644
>>> --- a/libavcodec/mpegaudiodec_template.c
>>> +++ b/libavcodec/mpegaudiodec_template.c
>>> @@ -27,6 +27,7 @@
>>>  #include "libavutil/attributes.h"
>>>  #include "libavutil/avassert.h"
>>>  #include "libavutil/channel_layout.h"
>>> +#include "libavutil/crc.h"
>>>  #include "libavutil/float_dsp.h"
>>>  #include "libavutil/libm.h"
>>>  #include "avcodec.h"
>>> @@ -1565,9 +1566,22 @@ static int mp_decode_frame(MPADecodeContext *s, 
>>> OUT_INT **samples,
>>>  
>>>  init_get_bits(>gb, buf + HEADER_SIZE, (buf_size - HEADER_SIZE) * 8);
>>>  
>>> -/* skip error protection field */
>>> -if (s->error_protection)
>>> -skip_bits(>gb, 16);
>>> +if (s->error_protection) {
>>> +uint16_t crc = get_bits(>gb, 16);
>>> +if (s->err_recognition & AV_EF_CRCCHECK) {
>>> +const int sec_len = s->lsf ? ((s->nb_channels == 1) ? 9  : 17) 
>>> :
>>> + ((s->nb_channels == 1) ? 17 : 32);
>>> +const AVCRC *crc_tab = av_crc_get_table(AV_CRC_16_ANSI);
>>> +uint32_t crc_cal = av_crc(crc_tab, UINT16_MAX, [2], 2);
>>> +crc_cal = av_crc(crc_tab, crc_cal, [6], sec_len);
>>> +
>>> +if (av_bswap16(crc) ^ crc_cal) {
>>> +av_log(s->avctx, AV_LOG_ERROR, "CRC mismatch!\n");
>>> +if (s->err_recognition & AV_EF_EXPLODE)
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>> +}
>>> +}
>>>
>>
>> For files with crcs and with damage, do they sound better with the
>> check and error out or without ?
>>
>> The behavior which provides the best user experience should be the
>> default
>>
>> It also may make sense to add one of AV_EF_CAREFUL / AV_EF_COMPLIANT / 
>> AV_EF_AGGRESSIVE
>> depending on how the check affects actual real world files
>>
>
> This is just a quick check to verify the files are uncorrupted, it shouldn't
> make broken files sound better unless the decoder somehow outputs white noise
> when it tries to decode them.
> I corrupted some files with -bsf:a noise=0.5 and couldn't notice an 
> improvement
> with or without -err_detect crccheck+explode even though the outputs looked 
> different. crccheck+explode makes much less errors about incorrect timestamps.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org 
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> 
>

Ping? Could someone take a look at this patch and my other 2 (apedec and pngdec 
CRC)?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 4/6] libx265: Update ROI behaviour to match documentation

[Michael Niedermayer]

On Wed, Mar 13, 2019 at 04:44:29PM +0100, Michael Niedermayer wrote:
> On Wed, Mar 13, 2019 at 12:17:46AM +, Mark Thompson wrote:
> > Equivalent to the previous patch for libx264.
> > ---
> >  libavcodec/libx265.c | 42 +-
> >  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> breaks build
> 
> CClibavcodec/vaapi_encode.o
> libavcodec/vaapi_encode.c: In function ‘vaapi_encode_issue’:
> libavcodec/vaapi_encode.c:566:18: error: ‘VAAPIEncodePicture’ has no member 
> named ‘roi’
>  av_freep(>roi);
>   ^
> libavcodec/vaapi_encode.c: In function ‘vaapi_encode_free’:
> libavcodec/vaapi_encode.c:701:18: error: ‘VAAPIEncodePicture’ has no member 
> named ‘roi’
>  av_freep(>roi);
>   ^
> make: *** [libavcodec/vaapi_encode.o] Error 1

wrong commit this is caused by "vaapi_encode: Add ROI support"

sorry

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 4/6] libx265: Update ROI behaviour to match documentation

[Michael Niedermayer]

On Wed, Mar 13, 2019 at 12:17:46AM +, Mark Thompson wrote:
> Equivalent to the previous patch for libx264.
> ---
>  libavcodec/libx265.c | 42 +-
>  1 file changed, 21 insertions(+), 21 deletions(-)

breaks build

CC  libavcodec/vaapi_encode.o
libavcodec/vaapi_encode.c: In function ‘vaapi_encode_issue’:
libavcodec/vaapi_encode.c:566:18: error: ‘VAAPIEncodePicture’ has no member 
named ‘roi’
 av_freep(>roi);
  ^
libavcodec/vaapi_encode.c: In function ‘vaapi_encode_free’:
libavcodec/vaapi_encode.c:701:18: error: ‘VAAPIEncodePicture’ has no member 
named ‘roi’
 av_freep(>roi);
  ^
make: *** [libavcodec/vaapi_encode.o] Error 1


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat/movenc: mov: added subtitle codec tags to codec tag list

[Paweł Wegner]

On Mon, Mar 4, 2019 at 10:52 AM Paweł Wegner 
wrote:

> On Mon, Mar 4, 2019 at 10:50 AM Paweł Wegner 
> wrote:
>
>> ping
>>
>> On Mon, Feb 25, 2019 at 11:50 AM Paweł Wegner 
>> wrote:
>>
>>> This fixes avformat_query_codec incorrectly returning 0 for
>>> mov container and mov_text subtitles.
>>>
>>> Signed-off-by: Paweł Wegner 
>>> ---
>>>  libavformat/movenc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index 77943304b5..8969d5b170 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -6858,7 +6858,7 @@ AVOutputFormat ff_mov_muxer = {
>>>  .deinit= mov_free,
>>>  .flags = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH |
>>> AVFMT_TS_NEGATIVE,
>>>  .codec_tag = (const AVCodecTag* const []){
>>> -ff_codec_movvideo_tags, ff_codec_movaudio_tags, 0
>>> +ff_codec_movvideo_tags, ff_codec_movaudio_tags,
>>> ff_codec_movsubtitle_tags, 0
>>>  },
>>>  .check_bitstream   = mov_check_bitstream,
>>>  .priv_class= _muxer_class,
>>> --
>>> 2.17.1
>>>
>>> Sorry for top posted ping.
>
ping x2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv2] avcodec/nvenc: Reconfigure resolution on-the-fly

[Oliver Collyer]

>>> Can you explain the actual intended use-case for this?
>>> 
>>> The current way to handle resolution changes (or any other stream change of 
>>> similar magnitude, like a pixel format change) is to flush the existing 
>>> encoder and then make a new one with the new parameters.  Even ignoring the 
>>> trivial benefit that all encoders handle this with no additional code, it 
>>> has the significant advantage that all of the stream metadata, including 
>>> parameter sets, can be handled correctly.  The handling here does rather 
>>> badly at this - stream metadata will be misleading, and if you take one of 
>>> these streams and try to put it into some container with global headers you 
>>> may well end up with a quite broken file.
>>> 
>> 
>> I’m not sure I follow; your logic seems contradictory here - clearly if you 
>> are trying to do this in a stream with global headers you’re going to run 
>> into trouble, but during writing to such a stream whether you 1) flush, 
>> delete and re-create, or 2) reconfigure the encoder is going to have the 
>> same effect iand won’t change anything since it’s not the encoder writing 
>> any such global stream headers it’s the muxer? Or did you mean something 
>> else?
> 
> Yeah, that wasn't very clear, sorry.  The main point I'm trying to make on 
> the stream metadata is that if you flush and re-create then all of that /is/ 
> available to the user.  They can of course then throw it all away to get back 
> to the state you have if they wish, but there was at least the option to make 
> use of it (e.g. if new global headers can be handled, maybe by starting a new 
> fragment).
> 

Ok, this is probably where I'm a bit sketchy as to how things work, so please 
bear with me (or don't worry about this if you don't have time!!!) - so if you 
flush, and re-create the encoder it would write the new width/height to the 
output stream?

In my code I roughly do this:

- allocate output format context
- for each input stream I am interested in, I create an output stream
- find an encoder for each stream and allocate the context
- set up the encoder with the parameters I need (so width, height etc) and any 
other options
- open the encoder
- copy the codec parameters to the stream (avcodec_parameters_from_context)
- write header for the output format
- start encoding my data

I believe I originally took and adapted these steps from the ffmpeg sample code.

So now, if I want to change width/height, I just change it directly in the 
encoder context width/height, and with this patch it picks it up and since I am 
only interested in mpegts it works fine. Presumably, the reconfigure does not 
trigger any kind of change to the output stream meta data. 

Whereas, you are saying that when I want to change width/height if I instead:

- flush the encoder
- free encoder context
- create a new encoder context
- set up the encoder with the new parameters
- open the encoder

This would give me a different result? So does opening the encoder (as opposed 
to reconfiguring it) fill in a bunch of output stream meta data then? Wouldn't 
I still need to do the step above "copy the codec parameters to the stream" and 
then write a new header for the output format myself?

> I guess it's not clear to me that adding this new external interface for the 
> special case is really a good trade-off.
> 
>> I’m using it in a server app where I want to quickly and efficiently change 
>> the video size/bitrate of a transport stream sent over long distance either 
>> when the remote user requests or in response to changing network conditions, 
>> with as little disruption to the viewing experience as possible.
> 
> Does the stop-start actually take longer?  Hardware encoders generally try to 
> minimise the amount of state they need to initialise and carry (because TDM), 
> so recreating it should not be a costly operation.  (Of course, I have no 
> idea what proprietary blobs will do.)
> 

That I do not know, but when I was looking at adding support for this into my 
project I noticed that it already handles bitrate and DAR changes so it was 
trivial to add resolution changes too (with the exception of needing to specify 
a maximum width/height).

>> For the record when this patch is used in conjunction with encoding into an 
>> mpegts stream it plays fine in VLC/libVLC, which defects the video changes 
>> in the stream and recreates the vout/resizes the video accordingly.
> 
> Yes, I can see that it should be fine in MPEG-TS.
> 
>>> Given that, I think there should be some justification for why you might 
>>> want to do this rather than following the existing approach.  Some mention 
>>> in the API (avcodec.h) to explain what's going on might be a good idea, 
>>> since that is currently assuming that parameters like width/height are 
>>> fixed and must be set before opening the encoder.  Relatedly, if there 
>>> isn't any support for retrieving new metadata then it should probably 
>>> display a big 

Re: [FFmpeg-devel] [PATCHv2] avcodec/nvenc: Reconfigure resolution on-the-fly

[Mark Thompson]

On 13/03/2019 01:01, Oliver Collyer wrote:
>> On 12 Mar 2019, at 23:40, Mark Thompson  wrote:
>>
>>> On 08/03/2019 07:44, Oliver Collyer wrote:
>>> From c704e3535f866d9f89535b9df59db9ca9811bcf9 Mon Sep 17 00:00:00 2001
>>> From: Oliver Collyer 
>>> Date: Fri, 8 Mar 2019 07:42:41 +
>>> Subject: [PATCH 1/1] avcodec/nvenc: Reconfigure resolution on-the-fly
>>>
>>> ---
>>> libavcodec/nvenc.c  | 31 ---
>>> libavcodec/nvenc.h  |  3 +++
>>> libavcodec/nvenc_h264.c |  4 
>>> libavcodec/nvenc_hevc.c |  4 
>>> 4 files changed, 39 insertions(+), 3 deletions(-)
>>
>> Can you explain the actual intended use-case for this?
>>
>> The current way to handle resolution changes (or any other stream change of 
>> similar magnitude, like a pixel format change) is to flush the existing 
>> encoder and then make a new one with the new parameters.  Even ignoring the 
>> trivial benefit that all encoders handle this with no additional code, it 
>> has the significant advantage that all of the stream metadata, including 
>> parameter sets, can be handled correctly.  The handling here does rather 
>> badly at this - stream metadata will be misleading, and if you take one of 
>> these streams and try to put it into some container with global headers you 
>> may well end up with a quite broken file.
>>
> 
> I’m not sure I follow; your logic seems contradictory here - clearly if you 
> are trying to do this in a stream with global headers you’re going to run 
> into trouble, but during writing to such a stream whether you 1) flush, 
> delete and re-create, or 2) reconfigure the encoder is going to have the same 
> effect iand won’t change anything since it’s not the encoder writing any such 
> global stream headers it’s the muxer? Or did you mean something else?

Yeah, that wasn't very clear, sorry.  The main point I'm trying to make on the 
stream metadata is that if you flush and re-create then all of that /is/ 
available to the user.  They can of course then throw it all away to get back 
to the state you have if they wish, but there was at least the option to make 
use of it (e.g. if new global headers can be handled, maybe by starting a new 
fragment).

I guess it's not clear to me that adding this new external interface for the 
special case is really a good trade-off.

> I’m using it in a server app where I want to quickly and efficiently change 
> the video size/bitrate of a transport stream sent over long distance either 
> when the remote user requests or in response to changing network conditions, 
> with as little disruption to the viewing experience as possible.

Does the stop-start actually take longer?  Hardware encoders generally try to 
minimise the amount of state they need to initialise and carry (because TDM), 
so recreating it should not be a costly operation.  (Of course, I have no idea 
what proprietary blobs will do.)

> For the record when this patch is used in conjunction with encoding into an 
> mpegts stream it plays fine in VLC/libVLC, which defects the video changes in 
> the stream and recreates the vout/resizes the video accordingly.

Yes, I can see that it should be fine in MPEG-TS.

>> Given that, I think there should be some justification for why you might 
>> want to do this rather than following the existing approach.  Some mention 
>> in the API (avcodec.h) to explain what's going on might be a good idea, 
>> since that is currently assuming that parameters like width/height are fixed 
>> and must be set before opening the encoder.  Relatedly, if there isn't any 
>> support for retrieving new metadata then it should probably display a big 
>> warning if the user tries to make a stream like this with global headers, 
>> since the global headers provided to the user on startup won't actually work 
>> for the whole stream.
>>
> 
> I think the fact this functionality is only accessible programmatically means 
> that the bar would be quite high, ie the user likely knows what they are 
> doing, but I can certainly put a comment next to the width/height avcodecctx 
> members along the lines of “In some limited scenarios such as when using 
> nvenc the width/height can be changed during encoding and will be detected by 
> the encoder causing it to reconfigure itself to the new picture sIze. Extreme 
> care should be taken when doing this with a format that uses global headers 
> as the global headers will no longer correspond to the adjusted picture size!”

Yeah, that's probably more sensible than what I suggested.

> Alternatively, maybe this is all a bit too obscure and it’s better left in my 
> own customised ffmpeg branch? That would be quite ok, although the code does 
> already handle dynamic bitrate and DAR changes so I figured to offer you the 
> patch...

Maybe.  If your use-case ends up genuinely better somehow then I think it 
probably would be sensible to include (and maybe investigate doing it in other 
cases so that nvenc isn't a surprising 

Re: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support

[Mark Thompson]

On 13/03/2019 02:46, Guo, Yejun wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Mark Thompson
>> Sent: Wednesday, March 13, 2019 8:18 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support
>>
>> ---
>>  libavcodec/vaapi_encode.c   | 128
>> 
>>  libavcodec/vaapi_encode.h   |  11 +++
>>  libavcodec/vaapi_encode_h264.c  |   2 +
>>  libavcodec/vaapi_encode_h265.c  |   2 +
>>  libavcodec/vaapi_encode_mpeg2.c |   2 +
>>  libavcodec/vaapi_encode_vp8.c   |   2 +
>>  libavcodec/vaapi_encode_vp9.c   |   2 +
>>  7 files changed, 149 insertions(+)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 2dda451882..03a7f5ce3e 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext
>> *avctx,
>>  int err, i;
>>  char data[MAX_PARAM_BUFFER_SIZE];
>>  size_t bit_len;
>> +AVFrameSideData *sd;
>>
>>  av_log(avctx, AV_LOG_DEBUG, "Issuing encode for
>> pic %"PRId64"/%"PRId64" "
>> "as type %s.\n", pic->display_order, pic->encode_order,
>> @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext
>> *avctx,
>>  }
>>  }
>>
>> +sd = av_frame_get_side_data(pic->input_image,
>> +AV_FRAME_DATA_REGIONS_OF_INTEREST);
>> +
>> +#if VA_CHECK_VERSION(1, 0, 0)
>> +if (sd && ctx->roi_allowed) {
>> +const AVRegionOfInterest *roi;
>> +int nb_roi;
>> +VAEncMiscParameterBuffer param_misc = {
>> +.type = VAEncMiscParameterTypeROI,
>> +};
>> +VAEncMiscParameterBufferROI param_roi;
>> +// VAAPI requires the second structure to immediately follow the
>> +// first in the parameter buffer, but they have different natural
>> +// alignment on 64-bit architectures (4 vs. 8, since the ROI
>> +// structure contains a pointer).  This means we can't make a
>> +// single working structure, nor can we make the buffer
>> +// separately and map it because dereferencing the second pointer
>> +// would be undefined.  Therefore, construct the two parts
>> +// separately and combine them into a single character buffer
>> +// before uploading.
>> +char param_buffer[sizeof(param_misc) + sizeof(param_roi)];
>> +int i, v;
>> +
>> +roi = (const AVRegionOfInterest*)sd->data;
>> +av_assert0(roi->self_size && sd->size % roi->self_size == 0);
> 
> it is possible if the user forget to set the value of roi->self_size. 
> assert is not feasible. 

Not setting self_size violates the API contract ("Must be set to the size of 
this data structure") and therefore invokes undefined behaviour.  Aborting when 
undefined behaviour is first detected is the correct response.

>> +nb_roi = sd->size / roi->self_size;
>> +if (nb_roi > ctx->roi_regions) {
>> +if (!ctx->roi_warned) {
>> +av_log(avctx, AV_LOG_WARNING, "More ROIs set than "
>> +   "supported by driver (%d > %d).\n",
>> +   nb_roi, ctx->roi_regions);
>> +ctx->roi_warned = 1;
>> +}
>> +nb_roi = ctx->roi_regions;
>> +}
>> +
>> +pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi));
>> +if (!pic->roi) {
>> +err = AVERROR(ENOMEM);
>> +goto fail;
>> +}
> 
> shall we add comments here to explain the list visit order?

Sure, added.

>> +for (i = 0; i < nb_roi; i++) {
>> +roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * 
>> i);
> 
> same comment as libx264

I guess, though I'm not sure filling the code with guards detecting undefined 
behaviour cases really has any value.

I've added an additional note on the API that the value must be the same on all 
entries.

>> +
>> +v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den;
> 
> shall we check here if roi->qoffset-den is zero?

Sure, I'll add an assert for the invalid fraction since the API requires 
[-1,+1].

>> +av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) -
>>> %+d.\n",
>> +   roi->x, roi->y, roi->width, roi->height, v);
>> +
>> +pic->roi[i] = (VAEncROI) {
>> +.roi_rectangle = {
>> +.x  = roi->x,
>> +.y  = roi->y,
>> +.width  = roi->width,
>> +.height = roi->height,
>> +},
>> +.roi_value = av_clip_c(v, INT8_MIN, INT8_MAX),
>> +};
>> +}
>> +
>> +param_roi = (VAEncMiscParameterBufferROI) {
>> +.num_roi  = nb_roi,
>> +.max_delta_qp = INT8_MAX,
>> +.min_delta_qp = 0,
>> +.roi

[FFmpeg-devel] [PATCH] avformat/aiffdec: parse replaygain metadata

[Moritz Barsnick]

Signed-off-by: Moritz Barsnick 
---
Tested against sample provided here:
https://ffmpeg.org/pipermail/ffmpeg-user/2019-March/043717.html

 libavformat/aiffdec.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
index 7c701e0c70..ea2955816a 100644
--- a/libavformat/aiffdec.c
+++ b/libavformat/aiffdec.c
@@ -29,6 +29,7 @@
 #include "isom.h"
 #include "id3v2.h"
 #include "mov_chan.h"
+#include "replaygain.h"
 
 #define AIFF0
 #define AIFF_C_VERSION1 0xA2805140
@@ -348,6 +349,10 @@ static int aiff_read_header(AVFormatContext *s)
 }
 }
 
+ret = ff_replaygain_export(st, s->metadata);
+if (ret < 0)
+return ret;
+
 got_sound:
 if (!st->codecpar->block_align && st->codecpar->codec_id == 
AV_CODEC_ID_QCELP) {
 av_log(s, AV_LOG_WARNING, "qcelp without wave chunk, assuming full 
rate\n");
-- 
2.20.1

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

[FFmpeg-devel] [PATCH v6] Improved the performance of 1 decode + N filter graphs and adaptive bitrate.

[Shaofei Wang]

It enabled multiple simple filter graph concurrency, which bring above about
4%~20% improvement in some 1:N scenarios by CPU or GPU acceleration

Below are some test cases and comparison as reference.
(Hardware platform: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz)
(Software: Intel iHD driver - 16.9.00100, CentOS 7)

For 1:N transcode by GPU acceleration with vaapi:
./ffmpeg -vaapi_device /dev/dri/renderD128 -hwaccel vaapi \
-hwaccel_output_format vaapi \
-i ~/Videos/1920x1080p_30.00_x264_qp28.h264 \
-vf "scale_vaapi=1280:720" -c:v h264_vaapi -f null /dev/null \
-vf "scale_vaapi=720:480" -c:v h264_vaapi -f null /dev/null

test results:
2 encoders 5 encoders 10 encoders
Improved   6.1%6.9%   5.5%

For 1:N transcode by GPU acceleration with QSV:
./ffmpeg -hwaccel qsv -c:v h264_qsv \
-i ~/Videos/1920x1080p_30.00_x264_qp28.h264 \
-vf "scale_qsv=1280:720:format=nv12" -c:v h264_qsv -f null /dev/null \
-vf "scale_qsv=720:480:format=nv12" -c:v h264_qsv -f null /dev/null

test results:
2 encoders  5 encoders 10 encoders
Improved   6%   4% 15%

For Intel GPU acceleration case, 1 decode to N scaling, by QSV:
./ffmpeg -hwaccel qsv -c:v h264_qsv \
-i ~/Videos/1920x1080p_30.00_x264_qp28.h264 \
-vf "scale_qsv=1280:720:format=nv12,hwdownload" -pix_fmt nv12 -f null 
/dev/null \
-vf "scale_qsv=720:480:format=nv12,hwdownload" -pix_fmt nv12 -f null 
/dev/null

test results:
2 scale  5 scale   10 scale
Improved   12% 21%21%

For CPU only 1 decode to N scaling:
./ffmpeg -i ~/Videos/1920x1080p_30.00_x264_qp28.h264 \
-vf "scale=1280:720" -pix_fmt nv12 -f null /dev/null \
-vf "scale=720:480" -pix_fmt nv12 -f null /dev/null

test results:
2 scale  5 scale   10 scale
Improved   25%107%   148%

Signed-off-by: Wang, Shaofei 
---
Passed fate and refine the possible data race.
The patch will only effect on multiple SIMPLE filter graphs pipeline

 fftools/ffmpeg.c | 172 +--
 fftools/ffmpeg.h |  13 +
 2 files changed, 169 insertions(+), 16 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 544f1a1..c0c9ca8 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -164,7 +164,13 @@ static struct termios oldtty;
 static int restore_tty;
 #endif
 
+/* enable abr threads when there were multiple simple filter graphs*/
+static int abr_threads_enabled = 0;
+
 #if HAVE_THREADS
+pthread_mutex_t fg_config_mutex;
+pthread_mutex_t ost_init_mutex;
+
 static void free_input_threads(void);
 #endif
 
@@ -509,6 +515,17 @@ static void ffmpeg_cleanup(int ret)
 }
 av_fifo_freep(>inputs[j]->ist->sub2video.sub_queue);
 }
+#if HAVE_THREADS
+if (abr_threads_enabled) {
+av_frame_free(>inputs[j]->input_frm);
+pthread_mutex_lock(>inputs[j]->process_mutex);
+fg->inputs[j]->waited_frm = NULL;
+fg->inputs[j]->t_end = 1;
+pthread_cond_signal(>inputs[j]->process_cond);
+pthread_mutex_unlock(>inputs[j]->process_mutex);
+pthread_join(fg->inputs[j]->abr_thread, NULL);
+}
+#endif
 av_buffer_unref(>inputs[j]->hw_frames_ctx);
 av_freep(>inputs[j]->name);
 av_freep(>inputs[j]);
@@ -1419,12 +1436,13 @@ static void finish_output_stream(OutputStream *ost)
  *
  * @return  0 for success, <0 for severe errors
  */
-static int reap_filters(int flush)
+static int reap_filters(int flush, InputFilter * ifilter)
 {
 AVFrame *filtered_frame = NULL;
 int i;
 
-/* Reap all buffers present in the buffer sinks */
+/* Reap all buffers present in the buffer sinks or just reap specified
+ * buffer which related with the filter graph who got ifilter as input*/
 for (i = 0; i < nb_output_streams; i++) {
 OutputStream *ost = output_streams[i];
 OutputFile*of = output_files[ost->file_index];
@@ -1432,13 +1450,25 @@ static int reap_filters(int flush)
 AVCodecContext *enc = ost->enc_ctx;
 int ret = 0;
 
+if (ifilter && abr_threads_enabled)
+if (ost != ifilter->graph->outputs[0])
+continue;
+
 if (!ost->filter || !ost->filter->graph->graph)
 continue;
 filter = ost->filter->filter;
 
 if (!ost->initialized) {
 char error[1024] = "";
+#if HAVE_THREADS
+if (abr_threads_enabled)
+pthread_mutex_lock(_init_mutex);
+#endif
 ret = init_output_stream(ost, error, sizeof(error));
+#if HAVE_THREADS
+if (abr_threads_enabled)
+pthread_mutex_unlock(_init_mutex);
+#endif
 if (ret < 0) {
 av_log(NULL, AV_LOG_ERROR, "Error initializing output stream 
%d:%d -- %s\n",
ost->file_index, 

Re: [FFmpeg-devel] [PATCHv3] avcodec/nvenc: Reconfigure resolution on-the-fly

[Oliver Collyer]

>> 
>> 
>> Can you explain the actual intended use-case for this?
>> 
>> The current way to handle resolution changes (or any other stream change of 
>> similar magnitude, like a pixel format change) is to flush the existing 
>> encoder and then make a new one with the new parameters.  Even ignoring the 
>> trivial benefit that all encoders handle this with no additional code, it 
>> has the significant advantage that all of the stream metadata, including 
>> parameter sets, can be handled correctly.  The handling here does rather 
>> badly at this - stream metadata will be misleading, and if you take one of 
>> these streams and try to put it into some container with global headers you 
>> may well end up with a quite broken file.
>> 
> 
> I’m not sure I follow; your logic seems contradictory here - clearly if you 
> are trying to do this in a stream with global headers you’re going to run 
> into trouble, but during writing to such a stream whether you 1) flush, 
> delete and re-create, or 2) reconfigure the encoder is going to have the same 
> effect iand won’t change anything since it’s not the encoder writing any such 
> global stream headers it’s the muxer? Or did you mean something else?
> 
> I’m using it in a server app where I want to quickly and efficiently change 
> the video size/bitrate of a transport stream sent over long distance either 
> when the remote user requests or in response to changing network conditions, 
> with as little disruption to the viewing experience as possible.
> 
> For the record when this patch is used in conjunction with encoding into an 
> mpegts stream it plays fine in VLC/libVLC, which defects the video changes in 
> the stream and recreates the vout/resizes the video accordingly.
> 
>> Given that, I think there should be some justification for why you might 
>> want to do this rather than following the existing approach.  Some mention 
>> in the API (avcodec.h) to explain what's going on might be a good idea, 
>> since that is currently assuming that parameters like width/height are fixed 
>> and must be set before opening the encoder.  Relatedly, if there isn't any 
>> support for retrieving new metadata then it should probably display a big 
>> warning if the user tries to make a stream like this with global headers, 
>> since the global headers provided to the user on startup won't actually work 
>> for the whole stream.
>> 
> 
> I think the fact this functionality is only accessible programmatically means 
> that the bar would be quite high, ie the user likely knows what they are 
> doing, but I can certainly put a comment next to the width/height avcodecctx 
> members along the lines of “In some limited scenarios such as when using 
> nvenc the width/height can be changed during encoding and will be detected by 
> the encoder causing it to reconfigure itself to the new picture sIze. Extreme 
> care should be taken when doing this with a format that uses global headers 
> as the global headers will no longer correspond to the adjusted picture size!”
> 
> Alternatively, maybe this is all a bit too obscure and it’s better left in my 
> own customised ffmpeg branch? That would be quite ok, although the code does 
> already handle dynamic bitrate and DAR changes so I figured to offer you the 
> patch...
> 

Updated patch with added comment following Mark's feedback.

Regards

Oliver



0001-avcodec-nvenc-Reconfigure-resolution-on-the-fly.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 2/2] fate/mxf: add mxf user comments tests

[Michael Niedermayer]

On Tue, Mar 12, 2019 at 10:04:32PM +0100, Michael Niedermayer wrote:
> On Mon, Mar 11, 2019 at 01:22:38PM -0700, mindm...@gmail.com wrote:
> > From: Mark Reid 
> > 
> > ---
> >  tests/fate/mxf.mak  | 15 ++-
> >  tests/ref/fate/mxf-d10-user-comments|  1 +
> >  tests/ref/fate/mxf-opatom-user-comments |  1 +
> >  tests/ref/fate/mxf-user-comments|  1 +
> >  4 files changed, 17 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/ref/fate/mxf-d10-user-comments
> >  create mode 100644 tests/ref/fate/mxf-opatom-user-comments
> >  create mode 100644 tests/ref/fate/mxf-user-comments
> 
> tested on x86-64 and mips qemu

will apply patchset

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH V4] avcodec/libvpxenc: add VP8 support for ROI-based encoding

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of James Zern
> Sent: Wednesday, March 13, 2019 7:45 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V4] avcodec/libvpxenc: add VP8
> support for ROI-based encoding
> 
> On Fri, Mar 8, 2019 at 5:07 AM Guo, Yejun  wrote:
> >
> > Signed-off-by: Guo, Yejun 
> > ---
> >  libavcodec/libvpxenc.c | 150
> +
> >  1 file changed, 150 insertions(+)
> >
> > [...]
> > +active_map.active_map = av_malloc(active_map.rows *
> active_map.cols);
> > +if (!active_map.active_map) {
> > +av_log(avctx, AV_LOG_ERROR, "active_map alloc failed.\n");
> > +ret = AVERROR(ENOMEM);
> > +goto fail;
> > +}
> > +/* set 1 to enable the corresponding element of
> vpx_roi_map_t.roi_map. */
> > +memset(active_map.active_map, 1, active_map.rows *
> active_map.cols);
> >
> 
> ROI is independent of active map, you don't need it for ROI to work.
> In this case you're setting the entire frame as active which may not
> be what you want. For this patch maybe we should focus on only setting
> the ROI.

yes, I mis-understood the libvpx API, will send out a new version after Mark 
T's patches pushed.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] lavu/opencl: replace va_ext.h with standard name

[Song, Ruiling]

> -Original Message-
> From: Song, Ruiling
> Sent: Tuesday, January 22, 2019 3:16 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Song, Ruiling 
> Subject: [PATCH 1/5] lavu/opencl: replace va_ext.h with standard name
> 
> Khronos OpenCL header (https://github.com/KhronosGroup/OpenCL-Headers)
> uses cl_va_api_media_sharing_intel.h. And Intel's official OpenCL driver
> for Intel GPU (https://github.com/intel/compute-runtime) was compiled
> against Khronos OpenCL header. So it's better to align with Khronos.
> 
> Signed-off-by: Ruiling Song 
> ---
>  configure| 2 +-
>  libavutil/hwcontext_opencl.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
ping?
If nobody against, I will push the patchset next week.

Ruiling
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Wednesday, March 13, 2019 8:18 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 6/6] lavfi: addroi filter
> 
> This can be used to add region of interest side data to video frames.
> ---
> Now using the x,y,w,h style to match other filters.
> 
> 
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_addroi.c  | 265

filter documentation is missed.

> +++
>  3 files changed, 267 insertions(+)
>  create mode 100644 libavfilter/vf_addroi.c
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index fef6ec5c55..31ae738a50 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -149,6 +149,7 @@ OBJS-$(CONFIG_SINE_FILTER)   += 
> asrc_sine.o
>  OBJS-$(CONFIG_ANULLSINK_FILTER)  += asink_anullsink.o
> 
>  # video filters
> +OBJS-$(CONFIG_ADDROI_FILTER) += vf_addroi.o
>  OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
>  OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
>  OBJS-$(CONFIG_AMPLIFY_FILTER)+= vf_amplify.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index c51ae0f3c7..52413c8f0d 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -140,6 +140,7 @@ extern AVFilter ff_asrc_sine;
> 
>  extern AVFilter ff_asink_anullsink;
> 
> +extern AVFilter ff_vf_addroi;
>  extern AVFilter ff_vf_alphaextract;
>  extern AVFilter ff_vf_alphamerge;
>  extern AVFilter ff_vf_amplify;
> diff --git a/libavfilter/vf_addroi.c b/libavfilter/vf_addroi.c
> new file mode 100644
> index 00..8bca5e7371
> --- /dev/null
> +++ b/libavfilter/vf_addroi.c
> @@ -0,0 +1,265 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include "libavutil/eval.h"
> +#include "libavutil/opt.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +enum {
> +X, Y, W, H,
> +};
> +static const char *addroi_param_names[] = {
> +"x", "y", "w", "h",
> +};
> +
> +enum {
> +VAR_IW,
> +VAR_IH,
> +NB_VARS,
> +};
> +static const char *const addroi_var_names[] = {
> +"iw",
> +"ih",
> +};
> +
> +typedef struct AddROIContext {
> +const AVClass *class;
> +
> +char *region_str[4];
> +
> +AVExpr *region_expr[4];
> +double vars[NB_VARS];
> +
> +int region[4];
> +AVRational qoffset;
> +
> +int clear;
> +} AddROIContext;

some people don't like the abbreviation of "ROI" in struct type name, see 
comment for my patch.
shall we rename it to AddRegionOfInterestContext?

> +
> +static int addroi_config_input(AVFilterLink *inlink)
> +{
> +AVFilterContext *avctx = inlink->dst;
> +AddROIContext *ctx = avctx->priv;
> +int i;
> +double val;
> +
> +ctx->vars[VAR_IW] = inlink->w;
> +ctx->vars[VAR_IH] = inlink->h;
> +
> +for (i = 0; i < 4; i++) {
> +int max_value;
> +switch (i) {
> +case X: max_value = inlink->w;  break;
> +case Y: max_value = inlink->h;  break;
> +case W: max_value = inlink->w - ctx->region[X]; break;
> +case H: max_value = inlink->h - ctx->region[Y]; break;
> +}
> +
> +val = av_expr_eval(ctx->region_expr[i], ctx->vars, NULL);

I understand that av_expr_* function is used to calculate expressions such as 
"8*9", but
not quite understand why ctx->vars here, also the third parameter of 
av_expr_parse below.

If needed, we can use a local variable vars within this function, instead of 
within ctx?

> +if (val < 0.0) {
> +av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
> +   "less than zero - using zero instead.\n", val,
> +   addroi_param_names[i]);
> +val = 0.0;
> +} else if (val > max_value) {
> +av_log(avctx, AV_LOG_WARNING, "Calculated value %g for %s is "
> +   "greater than maximum allowed value %d - "
> +   "using %d instead.\n", val, addroi_param_names[i],
> +   max_value, max_value);
> + 

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

[Sun, Jing A]

On  Monday, March 11, 2019 10:50 PM Vittorio Giovara vittorio.giov...@gmail.com 
wrote:

>On Mon, Mar 11, 2019 at 12:50 AM Sun, Jing A  wrote:
>I just searched my inbox again but failed to find that email of question you 
>mentioned.
>
>Yeah I often see my mail bounced with this message:
>
>Address not found
>Your message wasn't delivered to jun.z...@intel.com because the address 
>couldn't be found, or is unable to receive mail. 
 
>For reference this was the message on the mailing list 
>https://ffmpeg.org/pipermail/ffmpeg-devel/2019-March/240663.html
>
>Could you please elaborate your request? What is the preservation for and how 
>is it expected to work? 
>
>Yes of course, when you encode an HEVC stream you should be able to 
>signal how the color properties of the video buffers should be 
>rendered. This is usually conveyed with three parameters, the matrix 
>coefficients, the color primaries and the transfer characteristics. Without 
>such information, the data stored in the video may be interpreted differently 
>and often incorrectly by modern video players, causing image degradation, 
>wrong rendering and off colors.

>For HEVC they are usually expressed in the stream itself, under the VUI, and 
>it is kinda expected that modern encoder allow to set them to any of the 
>applicable values.
>In ffmpeg-land, they are represented by the colorspace, color_primaries and 
>color_transfer options in AVCodecContext and carried over through the whole 
>video processing.
>--
>Vittorio

Hi Giovara,

SVT HEVC has the interface to enable/disable sending a vui structure in the 
HEVC bitstream, but supports no interface for setting the color properties 
before encoding yet. I will be opening an issue in SVT HEVC github asking if 
they have plans to add such feature, and will keep you posted. In the meantime, 
I think it is not blocking the first version of this plugin’s merging , is it?

In SVT HEVC user guide: "VideoUsabilityInfo - Enables or disables sending a vui 
structure in the HEVC Elementary bitstream. 0 = OFF, 1 = ON"

Regards,
SUN, Jing

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