Re: [FFmpeg-devel] [PATCH] avformat/mux: Check pkt->stream_index before using it

[Anton Khirnov]

Quoting Andreas Rheinhardt (2020-05-11 23:58:47)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2020-05-10 21:35:54)
> >> Anton Khirnov:
> >>> Quoting Marton Balint (2020-05-10 19:45:04)
> 
> 
>  On Sun, 10 May 2020, Anton Khirnov wrote:
> 
> > Quoting Andreas Rheinhardt (2020-05-08 00:55:00)
> >> This commit fixes two recent regressions both of which are about using
> >> pkt->stream_index as index in an AVFormatContext's streams array before
> >> actually comparing the value with the count of streams in said array.
> >> 96e5e6abb9851d7a26ba21703955d5826ac857c0 did this in
> >> prepare_input_packet() and 64063512227c4c87a7d16a1076481dc6baf19841 did
> >> likewise in write_packets_common().
> >>
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >> The same error in the same file applied on the same day by two 
> >> different
> >> people. How unlikely.
> >
> > How is it a regression? Isn't it rather invalid API use?
> 
>  Fun fact: 7b03b65bf0d02519c86750d2da33f413e11cf0c6
> 
>  Yes, it is kind of invalid API use, but since the check is already 
>  there, 
>  we should make it actually worthwile.
> >>>
> >>> lol
> >>>
> >>> I agree that checking for it is a good idea, obviously, but I wouldn't
> >>> call it a regression.
> >>>
> >> How about rephrasing the first sentence to: "This commit stops using
> >> pkt->stream_index as index in an AVFormatContext's streams array before
> >> actually comparing the value with the count of streams in said array."
> > 
> > Sure, sounds good.
> > 
> 
> >
> > Not that I object to having a check. But then why is check_packet()
> > called so deep and not immediately on entry to the muxer?
> 
>  I guess it is not that deep, but recent factorization efforts hidden it 
>  a 
>  bit.
> >>>
> >>> You can see in my original commit it is the very first thing done after
> >>> entering the muxer. Right now it's several function calls deep.
> >>>
> >> I could make it the very first thing called in write_packets_common().
> > 
> > Why not move the check_packet() call out of prepare_packet() into
> > av_[interleaved_]write_frame() instead?
> > 
> This would add code duplication and IMO it is nicer to only check a
> packet for validity if there is actually a packet to check.

Okay. I don't entirely agree, but this is not important enough to waste
time arguing about. Feel free to push.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mpegtsenc: Don't use heap allocated array to store pids

[Marton Balint]

On Mon, 11 May 2020, Andriy Gelman wrote:


On Fri, 01. May 17:54, Marton Balint wrote:



On Thu, 30 Apr 2020, Andriy Gelman wrote:

> From: Andriy Gelman 
> 
> A temporary heap array currently stores pids from all streams.  It is

> used to make sure there are no duplicated pids. However, this array is
> not needed because the pids from past streams are stored in the
> MpegTSWriteStream structs.
> 


LGTM, thanks.

Marton



Hi Marton, 


Thanks for the review.
Is it ok to push both patches?


Sure, go ahead.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] pixblockdsp, avdct: Add get_pixels_unaligned

[Martin Storsjö]

Use this in vf_spp.c, where the get_pixels operation is done on
unaligned source addresses.

This fixes fate-filter-spp on armv7.
---
People more familiar with the other assembly implementations of
get_pixels (in particular, x86) can hook them up to
get_pixels_unaligned if unaligned use explicitly is ok; as far as I
can read at least ff_get_pixels_mmx, it looks like it expects the source
to be aligned, but in practice it does seem to run fine even if
ff_get_pixels_sse2 is disabled.
---
 libavcodec/avdct.c   | 1 +
 libavcodec/avdct.h   | 4 
 libavcodec/pixblockdsp.c | 2 ++
 libavcodec/pixblockdsp.h | 3 +++
 libavfilter/vf_spp.c | 2 +-
 5 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavcodec/avdct.c b/libavcodec/avdct.c
index 7c761cf39a..e8fa41f73b 100644
--- a/libavcodec/avdct.c
+++ b/libavcodec/avdct.c
@@ -120,6 +120,7 @@ int avcodec_dct_init(AVDCT *dsp)
 PixblockDSPContext pdsp;
 ff_pixblockdsp_init(&pdsp, avctx);
 COPY(pdsp, get_pixels);
+COPY(pdsp, get_pixels_unaligned);
 }
 #endif
 
diff --git a/libavcodec/avdct.h b/libavcodec/avdct.h
index 272422e44c..6411fab6f6 100644
--- a/libavcodec/avdct.h
+++ b/libavcodec/avdct.h
@@ -67,6 +67,10 @@ typedef struct AVDCT {
ptrdiff_t line_size);
 
 int bits_per_sample;
+
+void (*get_pixels_unaligned)(int16_t *block /* align 16 */,
+   const uint8_t *pixels,
+   ptrdiff_t line_size);
 } AVDCT;
 
 /**
diff --git a/libavcodec/pixblockdsp.c b/libavcodec/pixblockdsp.c
index 50e1d1d735..a79e547776 100644
--- a/libavcodec/pixblockdsp.c
+++ b/libavcodec/pixblockdsp.c
@@ -90,10 +90,12 @@ av_cold void ff_pixblockdsp_init(PixblockDSPContext *c, 
AVCodecContext *avctx)
 case 10:
 case 12:
 case 14:
+c->get_pixels_unaligned =
 c->get_pixels = get_pixels_16_c;
 break;
 default:
 if (avctx->bits_per_raw_sample<=8 || avctx->codec_type != 
AVMEDIA_TYPE_VIDEO) {
+c->get_pixels_unaligned =
 c->get_pixels = get_pixels_8_c;
 }
 break;
diff --git a/libavcodec/pixblockdsp.h b/libavcodec/pixblockdsp.h
index e036700ff0..fddb467212 100644
--- a/libavcodec/pixblockdsp.h
+++ b/libavcodec/pixblockdsp.h
@@ -29,6 +29,9 @@ typedef struct PixblockDSPContext {
 void (*get_pixels)(int16_t *av_restrict block /* align 16 */,
const uint8_t *pixels /* align 8 */,
ptrdiff_t stride);
+void (*get_pixels_unaligned)(int16_t *av_restrict block /* align 16 */,
+   const uint8_t *pixels,
+   ptrdiff_t stride);
 void (*diff_pixels)(int16_t *av_restrict block /* align 16 */,
 const uint8_t *s1 /* align 8 */,
 const uint8_t *s2 /* align 8 */,
diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
index 6bee91b309..a83b1195c0 100644
--- a/libavfilter/vf_spp.c
+++ b/libavfilter/vf_spp.c
@@ -283,7 +283,7 @@ static void filter(SPPContext *p, uint8_t *dst, uint8_t 
*src,
 const int x1 = x + offset[i + count - 1][0];
 const int y1 = y + offset[i + count - 1][1];
 const int index = x1 + y1*linesize;
-p->dct->get_pixels(block, p->src + sample_bytes*index, 
sample_bytes*linesize);
+p->dct->get_pixels_unaligned(block, p->src + 
sample_bytes*index, sample_bytes*linesize);
 p->dct->fdct(block);
 p->requantize(block2, block, qp, p->dct->idct_permutation);
 p->dct->idct(block2);
-- 
2.17.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5 1/3] lavc/libopenh264enc: Rewrite profile handling

[Martin Storsjö]

On Wed, 6 May 2020, Linjie Fu wrote:


Support the profiles "constrained_baseline" and "high" for libopenh264
version >= 1.8, support "constrained_baseline" and "main" for earlier
version.

If option not supported with current version, convert to constrained
baseline with a warning for users.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 44 +
1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 3bb3cceb64..12c71ca0e9 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -44,7 +44,7 @@ typedef struct SVCContext {
ISVCEncoder *encoder;
int slice_mode;
int loopfilter;
-char *profile;
+int profile;
int max_nal_size;
int skip_frames;
int skipped;
@@ -75,7 +75,12 @@ static const AVOption options[] = {
#endif
#endif
{ "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT, 
{ .i64 = 1 }, 0, 1, VE },
-{ "profile", "set profile restrictions", OFFSET(profile), 
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
+{ "profile", "set profile restrictions", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = 
FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0x, VE, "profile" },
+#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = value }, 0, 0, 
VE, "profile"
+{ PROFILE("constrained_baseline", 
FF_PROFILE_H264_CONSTRAINED_BASELINE) },
+{ PROFILE("main", FF_PROFILE_H264_MAIN) },
+{ PROFILE("high", FF_PROFILE_H264_HIGH) },
+#undef PROFILE
{ "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), 
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
{ "allow_skip_frames", "allow skipping frames to hit the target bitrate", 
OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
{ "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 
1, VE },
@@ -176,10 +181,41 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.iLoopFilterDisableIdc  = !s->loopfilter;
param.iEntropyCodingModeFlag = 0;
param.iMultipleThreadIdc = avctx->thread_count;
-if (s->profile && !strcmp(s->profile, "main"))
+
+if (s->profile == FF_PROFILE_UNKNOWN)
+s->profile = !s->cabac ? FF_PROFILE_H264_CONSTRAINED_BASELINE :
+#if OPENH264_VER_AT_LEAST(1, 8)
+ FF_PROFILE_H264_HIGH;
+#else
+ FF_PROFILE_H264_MAIN;
+#endif
+
+switch (s->profile) {
+#if OPENH264_VER_AT_LEAST(1, 8)
+case FF_PROFILE_H264_HIGH:
param.iEntropyCodingModeFlag = 1;
-else if (!s->profile && s->cabac)
+av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
+"select EProfileIdc PRO_HIGH in libopenh264.\n");
+break;
+#else
+case FF_PROFILE_H264_MAIN:
param.iEntropyCodingModeFlag = 1;
+av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
+"select EProfileIdc PRO_MAIN in libopenh264.\n");
+break;
+#endif
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:
+case FF_PROFILE_UNKNOWN:
+param.iEntropyCodingModeFlag = 0;
+av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
+   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+break;
+default:
+param.iEntropyCodingModeFlag = 0;
+av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
+   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+break;
+}

param.sSpatialLayers[0].iVideoWidth = param.iPicWidth;
param.sSpatialLayers[0].iVideoHeight= param.iPicHeight;
--
2.17.1


Thanks, this looks ok to me.

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5 2/3] lavc/libopenh264enc: Allow specifying the profile through AVCodecContext

[Martin Storsjö]

On Wed, 6 May 2020, Linjie Fu wrote:


And determine the profile with following priority:
1. s->profile; then
2. avctx->profile; then
3. s->cabac; then
4. a default profile.

This seems more natural in case user somehow sets both avctx->profile and
s->profile.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 15 +++
1 file changed, 15 insertions(+)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 12c71ca0e9..4c57fa1c89 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -182,6 +182,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.iEntropyCodingModeFlag = 0;
param.iMultipleThreadIdc = avctx->thread_count;

+/* Allow specifying the libopenh264 profile through AVCodecContext. */
+if (FF_PROFILE_UNKNOWN == s->profile &&
+FF_PROFILE_UNKNOWN != avctx->profile)
+switch (avctx->profile) {
+case FF_PROFILE_H264_HIGH:
+case FF_PROFILE_H264_MAIN:
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:
+s->profile = avctx->profile;
+break;
+default:
+av_log(avctx, AV_LOG_WARNING,
+   "Unsupported avctx->profile: %d.\n", avctx->profile);
+break;
+}
+
if (s->profile == FF_PROFILE_UNKNOWN)
s->profile = !s->cabac ? FF_PROFILE_H264_CONSTRAINED_BASELINE :
#if OPENH264_VER_AT_LEAST(1, 8)
--
2.17.1


LGTM

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5 3/3] lavc/libopenh264enc: Add coder option to replace cabac

[Martin Storsjö]

On Wed, 6 May 2020, Linjie Fu wrote:


Set DEPRECATED flag to option cabac, replace with coder. The
priority logic is:
1. s->coder; then
2. avctx->coder_type; then
3. s->cabac.

Change the default option to -1 and allow the default cabac to be
determined by profile.

Add FF_API_OPENH264_CABAC macro for cabac to remove this option after
LIBAVCODEC_VERSION_MAJOR = 59.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 40 +
libavcodec/version.h|  3 +++
2 files changed, 30 insertions(+), 13 deletions(-)


I guess this looks ok - thanks!

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Nicolas George]

lance.lmw...@gmail.com (12020-05-11):
> From: Limin Wang 
> 
> These are similar to the existing FF_ALLOC_ARRAY_OR_GOTO & 
> FF_ALLOCZ_ARRAY_OR_GOTO,
> but the elsize is calcuated by sizeof(*p)
> 
> Signed-off-by: Limin Wang 
> ---
>  libavutil/internal.h | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/libavutil/internal.h b/libavutil/internal.h
> index 4acbcf5..1be9001 100644
> --- a/libavutil/internal.h
> +++ b/libavutil/internal.h
> @@ -173,6 +173,24 @@
>  }\
>  }
>  
> +#define FF_ALLOC_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> +{\
> +p = av_malloc_array(nelem, sizeof(*p));\
> +if (!p) {\
> +av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> +goto label;\
> +}\
> +}
> +
> +#define FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> +{\
> +p = av_mallocz_array(nelem, sizeof(*p));\
> +if (!p) {\
> +av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> +goto label;\
> +}\
> +}
> +
>  #include "libm.h"
>  
>  /**

Please NO!

These functions have a terrible design, let us fix them before extending
them.

First design mistake: no error code. A helper function for testing
memory allocation failure where AVERROR(ENOMEM) does not appear is
absurd.

Second design mistake: printing a message. Return the error code, let
the caller print the error message.

Third design mistake: hard-coded use of goto.

Best design:

#define FF_ALLOC_ARRAY_OR_GOTO(p, nelem, elsize, ret, error)\
{\
p = av_malloc_array(nelem, elsize);\
if (!p) {\
ret = AVERROR(ENOMEM); \
error;
}\
}

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: Add MediaFoundation encoder wrapper

[Martin Storsjö]

On Sun, 10 May 2020, Marton Balint wrote:




On Sun, 10 May 2020, Martin Storsjö wrote:


From: wm4 

This contains encoder wrappers for H264, HEVC, AAC, AC3 and MP3.

This is based on top of an original patch by wm4
. The original patch supported both encoding
and decoding, but this patch only includes encoding.

The patch contains further changes by Paweł Wegner
 (primarily for splitting out the encoding
parts of the original patch) and further cleanup, build compatibility
fixes and tweaks for use with Qualcomm encoders by Martin Storsjö.
---
This allows access to the HW video encoder on Windows on ARM64
on Qualcomm platforms. However, to actually use that, one has to
manually choose nv12 as input pixel format, otherwise the encoder
format negotiation fails.

I've tried to read up on the feedback this patch got the earlier
times it was posted and address those issues. In particular,
this is enabled automatically if suitable headers are available.
The built binary still runs on Vista (even if the required MF
functionality isn't available there).

Building succeeds with MSVC, old and new mingw-w64 toolchains,
and isn't detected nor enabled on mingw.org toolchains. The configure
check looks for one of the API details used; mingw-w64 versions
from before that feature was added won't try to build the code,
while newer ones should have enough features to build it successfully.
---
configure  |   11 +
libavcodec/Makefile|1 +
libavcodec/allcodecs.c |5 +
libavcodec/mf_utils.c  |  677 +++
libavcodec/mf_utils.h  |  138 +
libavcodec/mfenc.c | 1181 
libavcodec/version.h   |2 +-
7 files changed, 2014 insertions(+), 1 deletion(-)
create mode 100644 libavcodec/mf_utils.c
create mode 100644 libavcodec/mf_utils.h
create mode 100644 libavcodec/mfenc.c


Missing docs update.


Which one is mandatory here? I added a changelog entry locally now as wel. 
I guess it could have short general description in doc/encoders.texi, or 
does one have to duplicate the info about all the options as well?


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: Add MediaFoundation encoder wrapper

[Martin Storsjö]

On Sun, 10 May 2020, James Almer wrote:


On 5/9/2020 6:41 PM, Martin Storsjö wrote:

+#define MF_ENCODER(MEDIATYPE, NAME, ID, OPTS, EXTRA) \
+static const AVClass ff_ ## NAME ## _mf_encoder_class = {  
\
+.class_name = #NAME "_mf", 
\
+.item_name  = av_default_item_name,
\
+.option = OPTS,
\
+.version= LIBAVUTIL_VERSION_INT,   
\
+}; 
\
+AVCodec ff_ ## NAME ## _mf_encoder = { 
\
+.priv_class = &ff_ ## NAME ## _mf_encoder_class,   
\
+.name   = #NAME "_mf", 
\
+.long_name  = NULL_IF_CONFIG_SMALL(#ID " via MediaFoundation"),
\
+.type   = AVMEDIA_TYPE_ ## MEDIATYPE,  
\
+.id = AV_CODEC_ID_ ## ID,  
\
+.priv_data_size = sizeof(MFContext),   
\
+.init   = mf_init, 
\
+.close  = mf_close,
\
+.send_frame = mf_send_frame,   
\
+.receive_packet = mf_receive_packet,   
\
+EXTRA  
\
+.capabilities   = AV_CODEC_CAP_DELAY,  
\


Should probably also be AV_CODEC_CAP_HYBRID.


Sure, added locally.


+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |   
\
+  FF_CODEC_CAP_INIT_CLEANUP,   
\
+};
+
+#define AFMTS \
+.sample_fmts= (const enum AVSampleFormat[]){ AV_SAMPLE_FMT_S16,
\
+ AV_SAMPLE_FMT_NONE },
+
+MF_ENCODER(AUDIO, aac, AAC, NULL, AFMTS);
+MF_ENCODER(AUDIO, ac3, AC3, NULL, AFMTS);
+MF_ENCODER(AUDIO, mp3, MP3, NULL, AFMTS);
+
+#define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption venc_opts[] = {
+{"rate_control",  "Select rate control mode", OFFSET(opt_enc_rc), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, VE, "rate_control"},
+{ "default",  "Default mode", 0, AV_OPT_TYPE_CONST, {.i64 = -1}, 0, 0, VE, 
"rate_control"},
+{ "cbr",  "CBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_CBR}, 0, 0, VE, "rate_control"},
+{ "pc_vbr",   "Peak constrained VBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_PeakConstrainedVBR}, 0, 0, VE, "rate_control"},
+{ "u_vbr","Unconstrained VBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_UnconstrainedVBR}, 0, 0, VE, "rate_control"},
+{ "quality",  "Quality mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_Quality}, 0, 0, VE, "rate_control" },
+// The following rate_control modes require Windows 8.
+{ "ld_vbr",   "Low delay VBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_LowDelayVBR}, 0, 0, VE, "rate_control"},
+{ "g_vbr","Global VBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_GlobalVBR}, 0, 0, VE, "rate_control" },
+{ "gld_vbr",  "Global low delay VBR mode", 0, AV_OPT_TYPE_CONST, {.i64 = 
ff_eAVEncCommonRateControlMode_GlobalLowDelayVBR}, 0, 0, VE, "rate_control"},
+{"quality",   "Quality", OFFSET(opt_enc_quality), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, 100, VE},
+{"hw_encoding",   "Force hardware encoding", OFFSET(opt_enc_hw), AV_OPT_TYPE_BOOL, 
{.i64 = 0}, 0, 1, VE, "hw_encoding"},


Can't you attempt to init using hw by default and if that fails
gracefully fallback to the sw implementation?
This option could instead just attempt to force disable or enable hw
(deafault -1/auto), and if hw can't be used when forced it would just
abort instead of falling back to sw.


It's not very straightforward to fall back, there's usually a rather long 
sequence of option setting until it's clear whether the chosen encoder 
works in the desired mode or not.


One common situation is e.g. that the system default SW encoder supports 
both yuv420p and nv12, while the HW encoder only supports nv12. Now if I'd 
prefer to use the hardware encoder, but it fails to use it (since the 
encoder turned out to only support nv12), falling back to SW would be a 
bit unexpected. (Although I guess one could retain that behaviour if the 
flag has been set to an explicit value.)


In any case, I'd rather leave this aspect as a patch welcome feature for 
others if they want to implement it - I don't see it as essent

Re: [FFmpeg-devel] [PATCH] avcodec: Add MediaFoundation encoder wrapper

[Martin Storsjö]

On Sun, 10 May 2020, Lou Logan wrote:


On Sat, May 9, 2020, at 1:41 PM, Martin Storsjö wrote:


+  --enable-mf  enable decoding via MediaFoundation [auto]


encoding/decoding typo I presume.


Good catch, thanks. (The original patch supported both, but for now it's 
thinned down to only encoding.)



I would prefer --enable-mediafoundation as it is more descriptive.


Sure, I can change that.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] avcodec: Add MediaFoundation encoder wrapper

[Martin Storsjö]

From: wm4 

This contains encoder wrappers for H264, HEVC, AAC, AC3 and MP3.

This is based on top of an original patch by wm4
. The original patch supported both encoding
and decoding, but this patch only includes encoding.

The patch contains further changes by Paweł Wegner
 (primarily for splitting out the encoding
parts of the original patch) and further cleanup, build compatibility
fixes and tweaks for use with Qualcomm encoders by Martin Storsjö.
---
v2: Added AV_CODEC_CAP_HYBRID, removed a leftover unused AVBSFContext,
added changelog and encoders.text entries, renamed the configure option
and config item to "mediafoundation" instead of "mf", changed the
makefile to keep individual additions of object files for each
individual encoder (so that it isn't compiled if all encoders are
disabled, even if CONFIG_MEDIAFOUNDATION is enabled).
---
 Changelog  |1 +
 configure  |   11 +
 doc/encoders.texi  |8 +
 libavcodec/Makefile|6 +
 libavcodec/allcodecs.c |5 +
 libavcodec/mf_utils.c  |  677 +++
 libavcodec/mf_utils.h  |  138 +
 libavcodec/mfenc.c | 1178 
 libavcodec/version.h   |2 +-
 9 files changed, 2025 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/mf_utils.c
 create mode 100644 libavcodec/mf_utils.h
 create mode 100644 libavcodec/mfenc.c

diff --git a/Changelog b/Changelog
index b75d2b6b96..57d722a31d 100644
--- a/Changelog
+++ b/Changelog
@@ -66,6 +66,7 @@ version :
 - asubboost filter
 - Pro Pinball Series Soundbank demuxer
 - pcm_rechunk bitstream filter
+- MediaFoundation encoder wrapper
 
 
 version 4.2:
diff --git a/configure b/configure
index a45c0fbde6..e8ec0a282b 100755
--- a/configure
+++ b/configure
@@ -304,6 +304,7 @@ External library support:
   --enable-mbedtls enable mbedTLS, needed for https support
if openssl, gnutls or libtls is not used [no]
   --enable-mediacodec  enable Android MediaCodec support [no]
+  --enable-mediafoundation enable encoding via MediaFoundation [auto]
   --enable-libmysofa   enable libmysofa, needed for sofalizer filter [no]
   --enable-openal  enable OpenAL 1.1 capture support [no]
   --enable-opencl  enable OpenCL processing [no]
@@ -1704,6 +1705,7 @@ EXTERNAL_AUTODETECT_LIBRARY_LIST="
 libxcb_shape
 libxcb_xfixes
 lzma
+mediafoundation
 schannel
 sdl2
 securetransport
@@ -3011,6 +3013,8 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
 wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
 
 # hardware-accelerated codecs
+mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
+mediafoundation_extralibs="-lmfplat -lmfuuid -lole32 -lstrmiids"
 omx_deps="libdl pthreads"
 omx_rpi_select="omx"
 qsv_deps="libmfx"
@@ -3035,6 +3039,8 @@ nvenc_deps="ffnvcodec"
 nvenc_deps_any="libdl LoadLibrary"
 nvenc_encoder_deps="nvenc"
 
+aac_mf_encoder_deps="mediafoundation"
+ac3_mf_encoder_deps="mediafoundation"
 h263_v4l2m2m_decoder_deps="v4l2_m2m h263_v4l2_m2m"
 h263_v4l2m2m_encoder_deps="v4l2_m2m h263_v4l2_m2m"
 h264_amf_encoder_deps="amf"
@@ -3043,6 +3049,7 @@ h264_cuvid_decoder_deps="cuvid"
 h264_cuvid_decoder_select="h264_mp4toannexb_bsf"
 h264_mediacodec_decoder_deps="mediacodec"
 h264_mediacodec_decoder_select="h264_mp4toannexb_bsf h264_parser"
+h264_mf_encoder_deps="mediafoundation"
 h264_mmal_decoder_deps="mmal"
 h264_nvenc_encoder_deps="nvenc"
 h264_omx_encoder_deps="omx"
@@ -3059,6 +3066,7 @@ hevc_cuvid_decoder_deps="cuvid"
 hevc_cuvid_decoder_select="hevc_mp4toannexb_bsf"
 hevc_mediacodec_decoder_deps="mediacodec"
 hevc_mediacodec_decoder_select="hevc_mp4toannexb_bsf hevc_parser"
+hevc_mf_encoder_deps="mediafoundation"
 hevc_nvenc_encoder_deps="nvenc"
 hevc_qsv_decoder_select="hevc_mp4toannexb_bsf qsvdec"
 hevc_qsv_encoder_select="hevcparse qsvenc"
@@ -3075,6 +3083,7 @@ mjpeg_qsv_encoder_deps="libmfx"
 mjpeg_qsv_encoder_select="qsvenc"
 mjpeg_vaapi_encoder_deps="VAEncPictureParameterBufferJPEG"
 mjpeg_vaapi_encoder_select="cbs_jpeg jpegtables vaapi_encode"
+mp3_mf_encoder_deps="mediafoundation"
 mpeg1_cuvid_decoder_deps="cuvid"
 mpeg1_v4l2m2m_decoder_deps="v4l2_m2m mpeg1_v4l2_m2m"
 mpeg2_crystalhd_decoder_select="crystalhd"
@@ -6100,6 +6109,7 @@ check_headers io.h
 check_headers linux/perf_event.h
 check_headers libcrystalhd/libcrystalhd_if.h
 check_headers malloc.h
+check_headers mftransform.h
 check_headers net/udplite.h
 check_headers poll.h
 check_headers sys/param.h
@@ -6162,6 +6172,7 @@ check_type "windows.h dxva.h" "DXVA_PicParams_VP9" 
-DWINAPI_FAMILY=WINAPI_FAMILY
 check_type "windows.h d3d11.h" "ID3D11VideoDecoder"
 check_type "windows.h d3d11.h" "ID3D11VideoContext"
 check_type "d3d9.h dxva2api.h" DXVA2_ConfigPictureDecode -D_WIN32_WINNT=0x0602
+check_func_headers mfapi.h MFCreateAlignedMemoryBuffer -lmfplat
 
 check_type "vdpau/vdpau.h" "VdpPictureInfoHEVC"
 check_type "vdpau/vdpau.h" "VdpPictureInfoVP9"
diff --git a/doc/encoders.texi 

Re: [FFmpeg-devel] [PATCH v3] libavformat/flacdec: Workaround for truncated metadata picture size

[Mattias Wadman]

Hi, any chance to get this merged?

On Sun, May 3, 2020 at 12:32 PM Mattias Wadman  wrote:
>
> Sorry for nagging, something left to fix?
>
> On Mon, Apr 27, 2020 at 10:10 AM Mattias Wadman
>  wrote:
> >
> > Hi again, looks ok?
> >
> > On Fri, Apr 24, 2020 at 12:54 PM Mattias Wadman
> >  wrote:
> > >
> > > lavf flacenc could previously write truncated metadata picture size if
> > > the picture data did not fit in 24 bits. Detect this by truncting the
> > > size found inside the picture block and if it matches the block size
> > > use it and read rest of picture data.
> > >
> > > Also only enable this workaround flac files and not ogg files with flac
> > > METADATA_BLOCK_PICTURE comment.
> > >
> > > flacenc was fixed in e447a4d112bcfee10126c54eb4481fa8712957c8
> > > before the fix a broken flac for reproduction could be generated with:
> > > ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 
> > > -map 1:0 -c:v:0 bmp -disposition:1 attached_pic -t 1 test.flac
> > >
> > > Fixes ticket 6333
> > > ---
> > >  libavformat/flac_picture.c   | 35 +++
> > >  libavformat/flac_picture.h   |  2 +-
> > >  libavformat/flacdec.c|  2 +-
> > >  libavformat/oggparsevorbis.c |  2 +-
> > >  4 files changed, 30 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> > > index 81ddf80465..61277e9dee 100644
> > > --- a/libavformat/flac_picture.c
> > > +++ b/libavformat/flac_picture.c
> > > @@ -27,7 +27,7 @@
> > >  #include "id3v2.h"
> > >  #include "internal.h"
> > >
> > > -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
> > > +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int 
> > > buf_size, int truncate_workaround)
> > >  {
> > >  const CodecMime *mime = ff_id3v2_mime_tags;
> > >  enum AVCodecID id = AV_CODEC_ID_NONE;
> > > @@ -36,7 +36,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t 
> > > *buf, int buf_size)
> > >  GetByteContext g;
> > >  AVStream *st;
> > >  int width, height, ret = 0;
> > > -unsigned int len, type;
> > > +unsigned int type;
> > > +uint32_t len, left, trunclen = 0;
> > >
> > >  if (buf_size < 34) {
> > >  av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> > > short\n");
> > > @@ -114,16 +115,34 @@ int ff_flac_parse_picture(AVFormatContext *s, 
> > > uint8_t *buf, int buf_size)
> > >
> > >  /* picture data */
> > >  len = bytestream2_get_be32u(&g);
> > > -if (len <= 0 || len > bytestream2_get_bytes_left(&g)) {
> > > -av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> > > short\n");
> > > -if (s->error_recognition & AV_EF_EXPLODE)
> > > -ret = AVERROR_INVALIDDATA;
> > > -goto fail;
> > > +
> > > +left = bytestream2_get_bytes_left(&g);
> > > +if (len <= 0 || len > left) {
> > > +// Workaround lavf flacenc bug that allowed writing truncated 
> > > metadata picture block size if
> > > +// picture size did not fit in 24 bits
> > > +if (truncate_workaround && len > left && (len & 0xff) == 
> > > left) {
> > > +av_log(s, AV_LOG_INFO, "Correcting truncated metadata 
> > > picture size from %d to %d\n", left, len);
> > > +trunclen = len - left;
> > > +} else {
> > > +av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> > > short\n");
> > > +if (s->error_recognition & AV_EF_EXPLODE)
> > > +ret = AVERROR_INVALIDDATA;
> > > +goto fail;
> > > +}
> > >  }
> > >  if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
> > >  RETURN_ERROR(AVERROR(ENOMEM));
> > >  }
> > > -bytestream2_get_bufferu(&g, data->data, len);
> > > +
> > > +if (trunclen == 0) {
> > > +bytestream2_get_bufferu(&g, data->data, len);
> > > +} else {
> > > +// If truncation was detect copy all data from block and read 
> > > missing bytes
> > > +// not included in the block size
> > > +bytestream2_get_bufferu(&g, data->data, left);
> > > +if (avio_read(s->pb, data->data + len - trunclen, trunclen) < 
> > > trunclen)
> > > +RETURN_ERROR(AVERROR_INVALIDDATA);
> > > +}
> > >  memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> > >
> > >  if (AV_RB64(data->data) == PNGSIG)
> > > diff --git a/libavformat/flac_picture.h b/libavformat/flac_picture.h
> > > index 4374b6f4f6..61fd0c8806 100644
> > > --- a/libavformat/flac_picture.h
> > > +++ b/libavformat/flac_picture.h
> > > @@ -26,6 +26,6 @@
> > >
> > >  #define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0)
> > >
> > > -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int 
> > > buf_size);
> > > +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int 
> > > buf_size, int truncate_workaround);
> > >
> > >  #endif /* AVFORMAT_FLAC_PICTURE_H */
> 

[FFmpeg-devel] [PATCH 1/2] lavc/hevc: Add poc_msb_present filed in LongTermRPS

[Linjie Fu]

From: Xu Guangxin 

delta_poc_msb_present_flag is needed in find_ref_idx() to
indicate whether MSB of POC should be taken into account.

Details in 8.3.2.

Signed-off-by: Xu Guangxin 
Signed-off-by: Linjie Fu 
---
 libavcodec/hevc_ps.h | 1 +
 libavcodec/hevcdec.c | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
index 8e1bccd..238edd3 100644
--- a/libavcodec/hevc_ps.h
+++ b/libavcodec/hevc_ps.h
@@ -41,6 +41,7 @@ typedef struct ShortTermRPS {
 
 typedef struct LongTermRPS {
 int poc[32];
+uint8_t poc_msb_present[32];
 uint8_t used[32];
 uint8_t nb_refs;
 } LongTermRPS;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 78299f4..0772608 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -280,7 +280,6 @@ static int decode_lt_rps(HEVCContext *s, LongTermRPS *rps, 
GetBitContext *gb)
 rps->nb_refs = nb_sh + nb_sps;
 
 for (i = 0; i < rps->nb_refs; i++) {
-uint8_t delta_poc_msb_present;
 
 if (i < nb_sps) {
 uint8_t lt_idx_sps = 0;
@@ -295,8 +294,8 @@ static int decode_lt_rps(HEVCContext *s, LongTermRPS *rps, 
GetBitContext *gb)
 rps->used[i] = get_bits1(gb);
 }
 
-delta_poc_msb_present = get_bits1(gb);
-if (delta_poc_msb_present) {
+rps->poc_msb_present[i] = get_bits1(gb);
+if (rps->poc_msb_present[i]) {
 int64_t delta = get_ue_golomb_long(gb);
 int64_t poc;
 
-- 
2.7.4

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] lavc/hevc_refs: Fix the logic of find_ref_idx()

[Linjie Fu]

From: Xu Guangxin 

Currently find_ref_idx() would trigger 2 scans in DPB to find the
requested POC:
1. Firstly, ignore MSB of ref->poc and search for the requested POC;
2. Secondly, compare the entire ref->poc with requested POC;

For long term reference, we are able to only check LSB if MSB is not
presented(e.g. delta_poc_msb_present_flag == 0). However, for short
term reference, we should never ignore poc's MSB and it should be
kind of bit-exact. (Details in 8.3.2)

Otherwise this leads to decoding failures like:
[hevc @ 0x5638f4328600] Error constructing the frame RPS.
[hevc @ 0x5638f4328600] Error parsing NAL unit #2.
[hevc @ 0x5638f4338a80] Could not find ref with POC 21
Error while decoding stream #0:0: Invalid data found when processing input

Search the requested POC based on whether MSB is used, and avoid
the 2-times scan for DPB buffer. This benefits both native HEVC
decoder and integrated HW decoders.

Signed-off-by: Linjie Fu 
---

Since it's kind of difficult to generate an identical bitstream for
fate or test, I'd like to elaborate more about one of the failures:

requested POC = 5;
LtMask = (1 << 4) - 1 = 15;
ref[0]->poc = 21; // unexpected ref for poc = 5 (short term)
ref[1]->poc = 5;  // expected ref for poc = 5 (short term)

Hence find_ref_idx() would wrongly return a ref with poc = 21, which
leads to the decoding error.

 libavcodec/hevc_refs.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
index 7870a72..73aa6d8 100644
--- a/libavcodec/hevc_refs.c
+++ b/libavcodec/hevc_refs.c
@@ -358,24 +358,26 @@ int ff_hevc_slice_rpl(HEVCContext *s)
 return 0;
 }
 
-static HEVCFrame *find_ref_idx(HEVCContext *s, int poc)
+static HEVCFrame *find_ref_idx(HEVCContext *s, int poc, uint8_t use_msb)
 {
 int i;
-int LtMask = (1 << s->ps.sps->log2_max_poc_lsb) - 1;
 
-for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
-HEVCFrame *ref = &s->DPB[i];
-if (ref->frame->buf[0] && (ref->sequence == s->seq_decode)) {
-if ((ref->poc & LtMask) == poc)
-return ref;
+if (use_msb) {
+for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
+HEVCFrame *ref = &s->DPB[i];
+if (ref->frame->buf[0] && (ref->sequence == s->seq_decode)) {
+if (ref->poc == poc)
+return ref;
+}
 }
-}
-
-for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
-HEVCFrame *ref = &s->DPB[i];
-if (ref->frame->buf[0] && ref->sequence == s->seq_decode) {
-if (ref->poc == poc || (ref->poc & LtMask) == poc)
-return ref;
+} else {
+int LtMask = (1 << s->ps.sps->log2_max_poc_lsb) - 1;
+for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
+HEVCFrame *ref = &s->DPB[i];
+if (ref->frame->buf[0] && ref->sequence == s->seq_decode) {
+if ((ref->poc & LtMask) == poc)
+return ref;
+}
 }
 }
 
@@ -427,9 +429,9 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int 
poc)
 
 /* add a reference with the given poc to the list and mark it as used in DPB */
 static int add_candidate_ref(HEVCContext *s, RefPicList *list,
- int poc, int ref_flag)
+ int poc, int ref_flag, uint8_t use_msb)
 {
-HEVCFrame *ref = find_ref_idx(s, poc);
+HEVCFrame *ref = find_ref_idx(s, poc, use_msb);
 
 if (ref == s->ref || list->nb_refs >= HEVC_MAX_REFS)
 return AVERROR_INVALIDDATA;
@@ -485,7 +487,7 @@ int ff_hevc_frame_rps(HEVCContext *s)
 else
 list = ST_CURR_AFT;
 
-ret = add_candidate_ref(s, &rps[list], poc, HEVC_FRAME_FLAG_SHORT_REF);
+ret = add_candidate_ref(s, &rps[list], poc, HEVC_FRAME_FLAG_SHORT_REF, 
1);
 if (ret < 0)
 goto fail;
 }
@@ -495,7 +497,7 @@ int ff_hevc_frame_rps(HEVCContext *s)
 int poc  = long_rps->poc[i];
 int list = long_rps->used[i] ? LT_CURR : LT_FOLL;
 
-ret = add_candidate_ref(s, &rps[list], poc, HEVC_FRAME_FLAG_LONG_REF);
+ret = add_candidate_ref(s, &rps[list], poc, HEVC_FRAME_FLAG_LONG_REF, 
long_rps->poc_msb_present[i]);
 if (ret < 0)
 goto fail;
 }
-- 
2.7.4

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 1/4] lavc/vaapi_encode: add EQUAL_MULTI_ROWS support for slice structure

[Linjie Fu]

VA_ENC_SLICE_STRUCTURE_EQUAL_MULTI_ROWS is added to in the latest
libva (1.8.0) which matches the hardware behaviour:

/** \brief Driver supports any number of rows per slice but they must be the 
same
*   for all slices except for the last one, which must be equal or smaller
*   to the previous slices. */

And VA_ENC_SLICE_STRUCTURE_EQUAL_ROWS is kind of deprecated for iHD
since it's somehow introduced in [1] however misleading from what we
actually handle, and one row per slice would not get a good quality.

Caps query support in driver is WIP, and this would fix the multi slice
encoding for VAEntrypointEncSliceLP.

[1]

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_encode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index cb05ebd..234618a 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1888,6 +1888,9 @@ static av_cold int 
vaapi_encode_init_slice_structure(AVCodecContext *avctx)
 req_slices = avctx->slices;
 }
 if (slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_ROWS ||
+#if VA_CHECK_VERSION(1, 8, 0)
+slice_structure & VA_ENC_SLICE_STRUCTURE_EQUAL_MULTI_ROWS ||
+#endif
 slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_MACROBLOCKS) {
 ctx->nb_slices  = req_slices;
 ctx->slice_size = ctx->slice_block_rows / ctx->nb_slices;
-- 
2.7.4

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 2/4] lavc/vaapi_encode: wrap slice codes into row slice functions

[Linjie Fu]

Wrap current whole-row slice codes into following functions:
 - vaapi_encode_make_row_slice()
 - vaapi_encode_init_row_slice_structure()

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_encode.c | 190 ++
 1 file changed, 109 insertions(+), 81 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 234618a..8deec02 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -162,6 +162,61 @@ static int vaapi_encode_wait(AVCodecContext *avctx,
 return 0;
 }
 
+static int vaapi_encode_make_row_slice(AVCodecContext *avctx,
+   VAAPIEncodePicture *pic)
+{
+VAAPIEncodeContext *ctx = avctx->priv_data;
+VAAPIEncodeSlice *slice;
+int i, rounding;
+
+for (i = 0; i < pic->nb_slices; i++)
+pic->slices[i].row_size = ctx->slice_size;
+
+rounding = ctx->slice_block_rows - ctx->nb_slices * ctx->slice_size;
+if (rounding > 0) {
+// Place rounding error at top and bottom of frame.
+av_assert0(rounding < pic->nb_slices);
+// Some Intel drivers contain a bug where the encoder will fail
+// if the last slice is smaller than the one before it.  Since
+// that's straightforward to avoid here, just do so.
+if (rounding <= 2) {
+for (i = 0; i < rounding; i++)
+++pic->slices[i].row_size;
+} else {
+for (i = 0; i < (rounding + 1) / 2; i++)
+++pic->slices[pic->nb_slices - i - 1].row_size;
+for (i = 0; i < rounding / 2; i++)
+++pic->slices[i].row_size;
+}
+} else if (rounding < 0) {
+// Remove rounding error from last slice only.
+av_assert0(rounding < ctx->slice_size);
+pic->slices[pic->nb_slices - 1].row_size += rounding;
+}
+
+for (i = 0; i < pic->nb_slices; i++) {
+slice = &pic->slices[i];
+slice->index = i;
+if (i == 0) {
+slice->row_start   = 0;
+slice->block_start = 0;
+} else {
+const VAAPIEncodeSlice *prev = &pic->slices[i - 1];
+slice->row_start   = prev->row_start   + prev->row_size;
+slice->block_start = prev->block_start + prev->block_size;
+}
+slice->block_size  = slice->row_size * ctx->slice_block_cols;
+
+av_log(avctx, AV_LOG_DEBUG, "Slice %d: %d-%d (%d rows), "
+   "%d-%d (%d blocks).\n", i, slice->row_start,
+   slice->row_start + slice->row_size - 1, slice->row_size,
+   slice->block_start, slice->block_start + slice->block_size - 1,
+   slice->block_size);
+}
+
+return 0;
+}
+
 static int vaapi_encode_issue(AVCodecContext *avctx,
   VAAPIEncodePicture *pic)
 {
@@ -345,57 +400,17 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 if (pic->nb_slices == 0)
 pic->nb_slices = ctx->nb_slices;
 if (pic->nb_slices > 0) {
-int rounding;
-
 pic->slices = av_mallocz_array(pic->nb_slices, sizeof(*pic->slices));
 if (!pic->slices) {
 err = AVERROR(ENOMEM);
 goto fail;
 }
 
-for (i = 0; i < pic->nb_slices; i++)
-pic->slices[i].row_size = ctx->slice_size;
-
-rounding = ctx->slice_block_rows - ctx->nb_slices * ctx->slice_size;
-if (rounding > 0) {
-// Place rounding error at top and bottom of frame.
-av_assert0(rounding < pic->nb_slices);
-// Some Intel drivers contain a bug where the encoder will fail
-// if the last slice is smaller than the one before it.  Since
-// that's straightforward to avoid here, just do so.
-if (rounding <= 2) {
-for (i = 0; i < rounding; i++)
-++pic->slices[i].row_size;
-} else {
-for (i = 0; i < (rounding + 1) / 2; i++)
-++pic->slices[pic->nb_slices - i - 1].row_size;
-for (i = 0; i < rounding / 2; i++)
-++pic->slices[i].row_size;
-}
-} else if (rounding < 0) {
-// Remove rounding error from last slice only.
-av_assert0(rounding < ctx->slice_size);
-pic->slices[pic->nb_slices - 1].row_size += rounding;
-}
+vaapi_encode_make_row_slice(avctx, pic);
 }
+
 for (i = 0; i < pic->nb_slices; i++) {
 slice = &pic->slices[i];
-slice->index = i;
-if (i == 0) {
-slice->row_start   = 0;
-slice->block_start = 0;
-} else {
-const VAAPIEncodeSlice *prev = &pic->slices[i - 1];
-slice->row_start   = prev->row_start   + prev->row_size;
-slice->block_start = prev->block_start + prev->block_size;
-}
-slice->block_size  = slice->row_size * ctx->slice_block_cols;
-
-av_log(avctx, AV_LOG_DEBUG, "Sl

[FFmpeg-devel] [PATCH] lavc/vaapi_hevc: add missing max_8bit_constraint_flag

[Linjie Fu]

This is accidentally missed while rebasing.

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_hevc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/vaapi_hevc.c b/libavcodec/vaapi_hevc.c
index c83d481..9083331 100644
--- a/libavcodec/vaapi_hevc.c
+++ b/libavcodec/vaapi_hevc.c
@@ -505,6 +505,7 @@ static int ptl_convert(const PTLCommon *general_ptl, 
H265RawProfileTierLevel *h2
 copy_field(frame_only_constraint_flag);
 copy_field(max_12bit_constraint_flag);
 copy_field(max_10bit_constraint_flag);
+copy_field(max_8bit_constraint_flag);
 copy_field(max_422chroma_constraint_flag);
 copy_field(max_420chroma_constraint_flag);
 copy_field(max_monochrome_constraint_flag);
-- 
2.7.4

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 3/4] lavc/vaapi_encode: add tile slice encoding support

[Linjie Fu]

Add functions to initialize tile slice structure and make tile slice:
 - vaapi_encode_init_tile_slice_structure
 - vaapi_encode_make_tile_slice

Tile slice is not allowed to cross the boundary of a tile due to
the constraints of media-driver. Currently adding support for one
slice per tile.

N x N tile encoding is supposed to be supported with the the
capability of ARBITRARY_MACROBLOCKS slice structures.

N X 1 tile encoding should also work in ARBITRARY_ROWS slice
structure.

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_encode.c | 128 +++---
 libavcodec/vaapi_encode.h |  16 ++
 2 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 8deec02..2eb8b48 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -217,6 +217,33 @@ static int vaapi_encode_make_row_slice(AVCodecContext 
*avctx,
 return 0;
 }
 
+static int vaapi_encode_make_tile_slice(AVCodecContext *avctx,
+VAAPIEncodePicture *pic)
+{
+VAAPIEncodeContext *ctx = avctx->priv_data;
+VAAPIEncodeSlice *slice;
+int i, j, index;
+
+for (i = 0; i < ctx->tile_cols; i++) {
+for (j = 0; j < ctx->tile_rows; j++) {
+index= j * ctx->tile_cols + i;
+slice= &pic->slices[index];
+slice->index = index;
+
+pic->slices[index].block_start = ctx->col_bd[i] +
+ ctx->row_bd[j] * 
ctx->slice_block_cols;
+pic->slices[index].block_size  = ctx->row_height[j] * 
ctx->col_width[i];
+
+av_log(avctx, AV_LOG_DEBUG, "Slice %2d: (%2d, %2d) start at: %4d "
+   "width:%2d height:%2d (%d blocks).\n", index, ctx->col_bd[i],
+   ctx->row_bd[j], slice->block_start, ctx->col_width[i],
+   ctx->row_height[j], slice->block_size);
+}
+}
+
+return 0;
+}
+
 static int vaapi_encode_issue(AVCodecContext *avctx,
   VAAPIEncodePicture *pic)
 {
@@ -406,7 +433,10 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 goto fail;
 }
 
-vaapi_encode_make_row_slice(avctx, pic);
+if (ctx->tile_rows && ctx->tile_cols)
+vaapi_encode_make_tile_slice(avctx, pic);
+else
+vaapi_encode_make_row_slice(avctx, pic);
 }
 
 for (i = 0; i < pic->nb_slices; i++) {
@@ -1891,11 +1921,76 @@ static av_cold int 
vaapi_encode_init_row_slice_structure(AVCodecContext *avctx,
 return 0;
 }
 
+static av_cold int vaapi_encode_init_tile_slice_structure(AVCodecContext 
*avctx,
+  uint32_t 
slice_structure)
+{
+VAAPIEncodeContext *ctx = avctx->priv_data;
+int i, req_tiles;
+
+if (!(slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_MACROBLOCKS ||
+ (slice_structure & VA_ENC_SLICE_STRUCTURE_ARBITRARY_ROWS &&
+  ctx->tile_cols == 1))) {
+av_log(avctx, AV_LOG_ERROR, "Supported slice structure (%#x) doesn't 
work for "
+   "current tile requirement.\n", slice_structure);
+return AVERROR(EINVAL);
+}
+
+if (ctx->tile_rows > ctx->slice_block_rows ||
+ctx->tile_cols > ctx->slice_block_cols) {
+av_log(avctx, AV_LOG_WARNING, "Not enough block rows/cols (%d x %d) "
+   "for configured number of tile (%d x %d); ",
+   ctx->slice_block_rows, ctx->slice_block_cols,
+   ctx->tile_rows, ctx->tile_cols);
+ctx->tile_rows = ctx->tile_rows > ctx->slice_block_rows ?
+  ctx->slice_block_rows : 
ctx->tile_rows;
+ctx->tile_cols = ctx->tile_cols > ctx->slice_block_cols ?
+  ctx->slice_block_cols : 
ctx->tile_cols;
+av_log(avctx, AV_LOG_WARNING, "using allowed maximum (%d x %d).\n",
+   ctx->tile_rows, ctx->tile_cols);
+}
+
+req_tiles = ctx->tile_rows * ctx->tile_cols;
+
+// Tile slice is not allowed to cross the boundary of a tile due to
+// the constraints of media-driver. Currently we support one slice
+// per tile. This could be extended to multiple slices per tile.
+if (avctx->slices != req_tiles)
+av_log(avctx, AV_LOG_WARNING, "The number of requested slices "
+   "mismatches with configured number of tile (%d != %d); "
+   "using requested tile number for slice.\n",
+   avctx->slices, req_tiles);
+
+ctx->nb_slices = req_tiles;
+
+// Default in uniform spacing
+// 6-3, 6-5
+for (i = 0; i < ctx->tile_cols; i++) {
+ctx->col_width[i] = ( i + 1 ) * ctx->slice_block_cols / ctx->tile_cols 
-
+i * ctx->slice_block_cols / ctx->tile_cols;
+ctx->col_bd[i + 1]  = ctx->col_bd[i] + ctx->col_width[i];
+}
+// 6-4, 6-6
+for (i = 0; i < ctx->tile_rows; i++

[FFmpeg-devel] [PATCH v2 4/4] lavc/vaapi_encode_h265: add h265 tile encoding support

[Linjie Fu]

Default to enable uniform_spacing_flag. Guess level by the tile
rows/cols. Supported for ICL+ platforms.

Also add documentations.

To encode with 4 rows 2 columns:
ffmpeg ... -c:v hevc_vaapi -tiles 4x2 ...
ffmpeg ... -c:v hevc_vaapi -tile_rows 4 -tile_cols 2 ...

Suggested-by: Jun Zhao 
Signed-off-by: Linjie Fu 
---
 doc/encoders.texi  | 13 +
 libavcodec/vaapi_encode_h265.c | 38 +-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index aa3a6ee..ea4ae28 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -3093,6 +3093,19 @@ Include HDR metadata if the input frames have it
 messages).
 @end table
 
+@item tiles
+Set the number of tiles to encode the input video with, as rows x columns.
+Larger numbers allow greater parallelism in both encoding and decoding, but
+may decrease coding efficiency.
+
+@item tile_rows
+Selects how many rows of tiles to encode with. For example, 4 tile rows would
+be requested by setting the tile_rows option to 4.
+
+@item tile_cols
+Selects how many columns of tiles to encode with. For example, 5 tile columns
+would be requested by setting the tile_cols option to 5.
+
 @end table
 
 @item mjpeg_vaapi
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 92e0510..51e3847 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -63,6 +63,9 @@ typedef struct VAAPIEncodeH265Context {
 int level;
 int sei;
 
+int trows;
+int tcols;
+
 // Derived settings.
 int fixed_qp_idr;
 int fixed_qp_p;
@@ -345,7 +348,7 @@ static int 
vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 
 level = ff_h265_guess_level(ptl, avctx->bit_rate,
 ctx->surface_width, ctx->surface_height,
-ctx->nb_slices, 1, 1,
+ctx->nb_slices, ctx->tile_rows, 
ctx->tile_cols,
 (ctx->b_per_p > 0) + 1);
 if (level) {
 av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);
@@ -558,6 +561,20 @@ static int 
vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 
 pps->pps_loop_filter_across_slices_enabled_flag = 1;
 
+if (ctx->tile_rows && ctx->tile_cols) {
+pps->tiles_enabled_flag = 1;
+pps->uniform_spacing_flag   = 1;
+
+pps->num_tile_rows_minus1   = ctx->tile_rows - 1;
+pps->num_tile_columns_minus1= ctx->tile_cols - 1;
+
+pps->loop_filter_across_tiles_enabled_flag = 1;
+
+for (i = 0; i <= pps->num_tile_rows_minus1; i++)
+pps->row_height_minus1[i]   = ctx->row_height[i] - 1;
+for (i = 0; i <= pps->num_tile_columns_minus1; i++)
+pps->column_width_minus1[i] = ctx->col_width[i] - 1;
+}
 
 // Fill VAAPI parameter buffers.
 
@@ -666,6 +683,13 @@ static int 
vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 },
 };
 
+if (pps->tiles_enabled_flag) {
+for (i = 0; i <= vpic->num_tile_rows_minus1; i++)
+vpic->row_height_minus1[i]   = pps->row_height_minus1[i];
+for (i = 0; i <= vpic->num_tile_columns_minus1; i++)
+vpic->column_width_minus1[i] = pps->column_width_minus1[i];
+}
+
 return 0;
 }
 
@@ -1181,6 +1205,11 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext 
*avctx)
 if (priv->qp > 0)
 ctx->explicit_qp = priv->qp;
 
+if (priv->trows && priv->tcols) {
+ctx->tile_rows = priv->trows;
+ctx->tile_cols = priv->tcols;
+}
+
 return ff_vaapi_encode_init(avctx);
 }
 
@@ -1257,6 +1286,13 @@ static const AVOption vaapi_encode_h265_options[] = {
   { .i64 = SEI_MASTERING_DISPLAY | SEI_CONTENT_LIGHT_LEVEL },
   INT_MIN, INT_MAX, FLAGS, "sei" },
 
+{ "tiles", "Tile rows x cols",
+  OFFSET(trows), AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, FLAGS },
+{ "tile_rows", "Number of rows for tile encoding",
+  OFFSET(trows), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS },
+{ "tile_cols", "Number of cols for tile encoding",
+  OFFSET(tcols), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS },
+
 { NULL },
 };
 
-- 
2.7.4

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[lance . lmwang]

On Tue, May 12, 2020 at 11:49:01AM +0200, Nicolas George wrote:
> lance.lmw...@gmail.com (12020-05-11):
> > From: Limin Wang 
> > 
> > These are similar to the existing FF_ALLOC_ARRAY_OR_GOTO & 
> > FF_ALLOCZ_ARRAY_OR_GOTO,
> > but the elsize is calcuated by sizeof(*p)
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavutil/internal.h | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > index 4acbcf5..1be9001 100644
> > --- a/libavutil/internal.h
> > +++ b/libavutil/internal.h
> > @@ -173,6 +173,24 @@
> >  }\
> >  }
> >  
> > +#define FF_ALLOC_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> > +{\
> > +p = av_malloc_array(nelem, sizeof(*p));\
> > +if (!p) {\
> > +av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> > +goto label;\
> > +}\
> > +}
> > +
> > +#define FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(ctx, p, nelem, label)\
> > +{\
> > +p = av_mallocz_array(nelem, sizeof(*p));\
> > +if (!p) {\
> > +av_log(ctx, AV_LOG_ERROR, "Cannot allocate memory.\n");\
> > +goto label;\
> > +}\
> > +}
> > +
> >  #include "libm.h"
> >  
> >  /**
> 
> Please NO!
> 
> These functions have a terrible design, let us fix them before extending
> them.
> 
> First design mistake: no error code. A helper function for testing
> memory allocation failure where AVERROR(ENOMEM) does not appear is
> absurd.
> 
> Second design mistake: printing a message. Return the error code, let
> the caller print the error message.
> 
> Third design mistake: hard-coded use of goto.
No, it's not hard-coded, you can name your own goto label.

So below is my change by your proposal:
 #define FF_ALLOC_ARRAY_OR_GOTO(p, nelem, elsize, ret, label)\
 {\
 p = av_malloc_array(nelem, elsize);\
 if (!p) {\
 ret = AVERROR(ENOMEM); \
 goto label;
 }\
 }


> 
> Best design:
> 
> #define FF_ALLOC_ARRAY_OR_GOTO(p, nelem, elsize, ret, error)\
> {\
> p = av_malloc_array(nelem, elsize);\
> if (!p) {\
> ret = AVERROR(ENOMEM); \
> error;
> }\
> }
> 
> Regards,
> 
> -- 
>   Nicolas George



-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v5 1/3] lavc/libopenh264enc: Rewrite profile handling

[Fu, Linjie]

> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Tuesday, May 12, 2020 17:36
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: Re: [FFmpeg-devel] [PATCH v5 1/3] lavc/libopenh264enc: Rewrite
> profile handling
> 
> On Wed, 6 May 2020, Linjie Fu wrote:
> 
> > Support the profiles "constrained_baseline" and "high" for libopenh264
> > version >= 1.8, support "constrained_baseline" and "main" for earlier
> > version.
> >
> > If option not supported with current version, convert to constrained
> > baseline with a warning for users.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> > libavcodec/libopenh264enc.c | 44
> +
> > 1 file changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 3bb3cceb64..12c71ca0e9 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -44,7 +44,7 @@ typedef struct SVCContext {
> > ISVCEncoder *encoder;
> > int slice_mode;
> > int loopfilter;
> > -char *profile;
> > +int profile;
> > int max_nal_size;
> > int skip_frames;
> > int skipped;
> > @@ -75,7 +75,12 @@ static const AVOption options[] = {
> > #endif
> > #endif
> > { "loopfilter", "enable loop filter", OFFSET(loopfilter), 
> > AV_OPT_TYPE_INT,
> { .i64 = 1 }, 0, 1, VE },
> > -{ "profile", "set profile restrictions", OFFSET(profile),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
> > +{ "profile", "set profile restrictions", OFFSET(profile), 
> > AV_OPT_TYPE_INT,
> { .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0x, VE,
> "profile" },
> > +#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST,
> { .i64 = value }, 0, 0, VE, "profile"
> > +{ PROFILE("constrained_baseline",
> FF_PROFILE_H264_CONSTRAINED_BASELINE) },
> > +{ PROFILE("main", FF_PROFILE_H264_MAIN) },
> > +{ PROFILE("high", FF_PROFILE_H264_HIGH) },
> > +#undef PROFILE
> > { "max_nal_size", "set maximum NAL size in bytes",
> OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
> > { "allow_skip_frames", "allow skipping frames to hit the target 
> > bitrate",
> OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> > { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 1, VE },
> > @@ -176,10 +181,41 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > param.iLoopFilterDisableIdc  = !s->loopfilter;
> > param.iEntropyCodingModeFlag = 0;
> > param.iMultipleThreadIdc = avctx->thread_count;
> > -if (s->profile && !strcmp(s->profile, "main"))
> > +
> > +if (s->profile == FF_PROFILE_UNKNOWN)
> > +s->profile = !s->cabac ? FF_PROFILE_H264_CONSTRAINED_BASELINE :
> > +#if OPENH264_VER_AT_LEAST(1, 8)
> > + FF_PROFILE_H264_HIGH;
> > +#else
> > + FF_PROFILE_H264_MAIN;
> > +#endif
> > +
> > +switch (s->profile) {
> > +#if OPENH264_VER_AT_LEAST(1, 8)
> > +case FF_PROFILE_H264_HIGH:
> > param.iEntropyCodingModeFlag = 1;
> > -else if (!s->profile && s->cabac)
> > +av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > +"select EProfileIdc PRO_HIGH in libopenh264.\n");
> > +break;
> > +#else
> > +case FF_PROFILE_H264_MAIN:
> > param.iEntropyCodingModeFlag = 1;
> > +av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > +"select EProfileIdc PRO_MAIN in libopenh264.\n");
> > +break;
> > +#endif
> > +case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> > +case FF_PROFILE_UNKNOWN:
> > +param.iEntropyCodingModeFlag = 0;
> > +av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
> > +   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +break;
> > +default:
> > +param.iEntropyCodingModeFlag = 0;
> > +av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
> > +   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +break;
> > +}
> >
> > param.sSpatialLayers[0].iVideoWidth = param.iPicWidth;
> > param.sSpatialLayers[0].iVideoHeight= param.iPicHeight;
> > --
> > 2.17.1
> 
> Thanks, this looks ok to me.
> 
> // Martin
> 
Thanks for the review.

- Linjie

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Nicolas George]

lance.lmw...@gmail.com (12020-05-12):
> No, it's not hard-coded, you can name your own goto label.

The label is not hard-coded. The goto is hard-coded. It should not be.
Frequently, a return would be enough.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Marton Balint]

On Tue, 12 May 2020, Nicolas George wrote:


lance.lmw...@gmail.com (12020-05-12):

No, it's not hard-coded, you can name your own goto label.


The label is not hard-coded. The goto is hard-coded. It should not be.
Frequently, a return would be enough.



You mean this?

 #define FF_ALLOC_ARRAY_OR_ERROR(p, nelem, elsize, errstatement)\
 {\
 p = av_malloc_array(nelem, elsize);\
 if (!p) {\
 errstatement; \
 }\
 }

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/option_table: mark venc_params as a video decoder flag opt type

[James Almer]

It's not meant for audio or subtitles, or for encoders of any kind.

Signed-off-by: James Almer 
---
 libavcodec/options_table.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 695fa5c211..573675351b 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -81,7 +81,7 @@ static const AVOption avcodec_options[] = {
 {"export_side_data", "Export metadata as side data", OFFSET(export_side_data), 
AV_OPT_TYPE_FLAGS, {.i64 = DEFAULT}, 0, UINT_MAX, A|V|S|D|E, 
"export_side_data"},
 {"mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST, 
{.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
 {"prft", "export Producer Reference Time through packet side data", 0, 
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX, 
A|V|S|E, "export_side_data"},
-{"venc_params", "export video encoding parameters through frame side data", 0, 
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN, 
INT_MAX, A|V|S|E, "export_side_data"},
+{"venc_params", "export video encoding parameters through frame side data", 0, 
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN, 
INT_MAX, V|D, "export_side_data"},
 {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0, 
INT_MAX},
 {"g", "set the group of picture (GOP) size", OFFSET(gop_size), 
AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
 {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), 
AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
-- 
2.26.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> You mean this?
> 
>  #define FF_ALLOC_ARRAY_OR_ERROR(p, nelem, elsize, errstatement)\
>  {\
>  p = av_malloc_array(nelem, elsize);\
>  if (!p) {\
>  errstatement; \
>  }\
>  }

Exactly.

But I am rather in favor of making "something = AVERROR(ENOMEM)"
mandatory too:

#define FF_ALLOC_ARRAY_OR_ERROR(p, nelem, elsize, ret, errstatement)\
{\
p = av_malloc_array(nelem, elsize);\
if (!p) {\
ret = AVERROR(ENOMEM); \
errstatement; \
}\
}

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[lance . lmwang]

On Tue, May 12, 2020 at 03:58:24PM +0200, Nicolas George wrote:
> lance.lmw...@gmail.com (12020-05-12):
> > No, it's not hard-coded, you can name your own goto label.
> 
> The label is not hard-coded. The goto is hard-coded. It should not be.
> Frequently, a return would be enough.

return the error code directly? I recall some goto have extra cleanup function,
I'm not sure whether it's OK to return error code directly without goto.

> 
> Regards,
> 
> -- 
>   Nicolas George



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[lance . lmwang]

On Tue, May 12, 2020 at 04:11:34PM +0200, Nicolas George wrote:
> Marton Balint (12020-05-12):
> > You mean this?
> > 
> >  #define FF_ALLOC_ARRAY_OR_ERROR(p, nelem, elsize, errstatement)\
> >  {\
> >  p = av_malloc_array(nelem, elsize);\
> >  if (!p) {\
> >  errstatement; \
> >  }\
> >  }
> 
> Exactly.
> 
> But I am rather in favor of making "something = AVERROR(ENOMEM)"
> mandatory too:
> 
> #define FF_ALLOC_ARRAY_OR_ERROR(p, nelem, elsize, ret, errstatement)\
> {\
> p = av_malloc_array(nelem, elsize);\
> if (!p) {\
> ret = AVERROR(ENOMEM); \
> errstatement; \
> }\
> }

OK, it's more clear, I got it.

> 
> Regards,
> 
> -- 
>   Nicolas George



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/mv30: fix warning: suggest braces around initialization of subobject [-Wmissing-braces]

[lance . lmwang]

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavcodec/mv30.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mv30.c b/libavcodec/mv30.c
index fed9bcd..7e67133 100644
--- a/libavcodec/mv30.c
+++ b/libavcodec/mv30.c
@@ -421,7 +421,7 @@ static int decode_intra(AVCodecContext *avctx, 
GetBitContext *gb, AVFrame *frame
 
 for (int y = 0; y < avctx->height; y += 16) {
 GetByteContext gbyte;
-int pfill[3][1] = { 0 };
+int pfill[3][1] = { {0} };
 int nb_codes = get_bits(gb, 16);
 
 av_fast_padded_malloc(&s->coeffs, &s->coeffs_size, nb_codes * 
sizeof(*s->coeffs));
@@ -504,7 +504,7 @@ static int decode_inter(AVCodecContext *avctx, 
GetBitContext *gb,
 
 for (int y = 0; y < avctx->height; y += 16) {
 GetByteContext gbyte;
-int pfill[3][1] = { 0 };
+int pfill[3][1] = { {0} };
 int nb_codes = get_bits(gb, 16);
 
 skip_bits(gb, 8);
-- 
1.8.3.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Nicolas George]

lance.lmw...@gmail.com (12020-05-12):
> return the error code directly? I recall some goto have extra cleanup 
> function,
> I'm not sure whether it's OK to return error code directly without goto.

There are many places that do. Forcing them to use a goto just to be
able to benefit from that macro would be backwards.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO

[Limin Wang]

On Tue, May 12, 2020 at 05:29:13PM +0200, Nicolas George wrote:
> lance.lmw...@gmail.com (12020-05-12):
> > return the error code directly? I recall some goto have extra cleanup 
> > function,
> > I'm not sure whether it's OK to return error code directly without goto.
> 
> There are many places that do. Forcing them to use a goto just to be
> able to benefit from that macro would be backwards.

OK, I'll submit a patch with the proposal for review.

> 
> Regards,
> 
> -- 
>   Nicolas George



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec: improve the macros function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[lance . lmwang]

From: Limin Wang 

Please refer to the details here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/1589212343-8334-1-git-send-email-lance.lmw...@gmail.com/

Suggested-by: Nicolas George 
Signed-off-by: Limin Wang 
---
 libavcodec/aacenc.c  |   8 ++--
 libavcodec/ac3enc.c  | 102 +--
 libavcodec/ac3enc_template.c |  15 +++
 libavcodec/adpcmenc.c|  35 +--
 libavcodec/alac.c|  17 +++-
 libavcodec/apedec.c  |   9 ++--
 libavcodec/dnxhdenc.c|  73 +++
 libavcodec/h264dec.c |  69 ++---
 libavcodec/iirfilter.c   |  12 ++---
 libavcodec/mpegpicture.c |  11 ++---
 libavcodec/mpegvideo.c   |  78 +++--
 libavcodec/mpegvideo_enc.c   |  31 +
 libavcodec/snow.c|  25 +--
 libavcodec/twinvq.c  |  23 +-
 libavutil/internal.h |  24 +-
 libswscale/utils.c   |  40 -
 16 files changed, 260 insertions(+), 312 deletions(-)

diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index 4d0abb1..9855e40 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -939,16 +939,14 @@ static av_cold int dsp_init(AVCodecContext *avctx, 
AACEncContext *s)
 
 static av_cold int alloc_buffers(AVCodecContext *avctx, AACEncContext *s)
 {
-int ch;
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
sizeof(ChannelElement), alloc_fail);
+int ret, ch;
+FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
ret, return ret);
 
 for(ch = 0; ch < s->channels; ch++)
 s->planar_samples[ch] = s->buffer.samples + 3 * 1024 * ch;
 
 return 0;
-alloc_fail:
-return AVERROR(ENOMEM);
 }
 
 static av_cold void aac_encode_init_tables(void)
diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index ddbc7b2..19434d4 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -2322,60 +2322,60 @@ static av_cold void set_bandwidth(AC3EncodeContext *s)
 
 static av_cold int allocate_buffers(AC3EncodeContext *s)
 {
-AVCodecContext *avctx = s->avctx;
 int blk, ch;
 int channels = s->channels + 1; /* includes coupling channel */
 int channel_blocks = channels * s->num_blocks;
 int total_coefs= AC3_MAX_COEFS * channel_blocks;
+int ret;
+
+if (ret = s->allocate_sample_buffers(s))
+return ret;
 
-if (s->allocate_sample_buffers(s))
-goto alloc_fail;
-
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap_buffer, total_coefs,
- sizeof(*s->bap_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap1_buffer, total_coefs,
- sizeof(*s->bap1_buffer), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->mdct_coef_buffer, total_coefs,
-  sizeof(*s->mdct_coef_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->exp_buffer, total_coefs,
- sizeof(*s->exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->grouped_exp_buffer, channel_blocks, 128 *
- sizeof(*s->grouped_exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->psd_buffer, total_coefs,
- sizeof(*s->psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->band_psd_buffer, channel_blocks, 64 *
- sizeof(*s->band_psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->mask_buffer, channel_blocks, 64 *
- sizeof(*s->mask_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->qmant_buffer, total_coefs,
- sizeof(*s->qmant_buffer), alloc_fail);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap_buffer, total_coefs,
+ sizeof(*s->bap_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap1_buffer, total_coefs,
+ sizeof(*s->bap1_buffer), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->mdct_coef_buffer, total_coefs,
+  sizeof(*s->mdct_coef_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->exp_buffer, total_coefs,
+ sizeof(*s->exp_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->grouped_exp_buffer, channel_blocks, 128 *
+ sizeof(*s->grouped_exp_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->psd_buffer, total_coefs,
+ sizeof(*s->psd_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->band_psd_buffer, channel_blocks, 64 *
+ sizeof(*s->band_psd_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->mask_buffer, channel_blocks, 64 *
+ sizeof(*s->mask_buffer), ret, return ret);
+FF_ALLOC

Re: [FFmpeg-devel] [PATCH] avcodec: improve the macros function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[lance . lmwang]

On Wed, May 13, 2020 at 12:01:22AM +0800, lance.lmw...@gmail.com wrote:

Sorry, ignore this for wrong patch.

> From: Limin Wang 
> 
> Please refer to the details here:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1589212343-8334-1-git-send-email-lance.lmw...@gmail.com/
> 
> Suggested-by: Nicolas George 
> Signed-off-by: Limin Wang 
> ---
>  libavcodec/aacenc.c  |   8 ++--
>  libavcodec/ac3enc.c  | 102 
> +--
>  libavcodec/ac3enc_template.c |  15 +++
>  libavcodec/adpcmenc.c|  35 +--
>  libavcodec/alac.c|  17 +++-
>  libavcodec/apedec.c  |   9 ++--
>  libavcodec/dnxhdenc.c|  73 +++
>  libavcodec/h264dec.c |  69 ++---
>  libavcodec/iirfilter.c   |  12 ++---
>  libavcodec/mpegpicture.c |  11 ++---
>  libavcodec/mpegvideo.c   |  78 +++--
>  libavcodec/mpegvideo_enc.c   |  31 +
>  libavcodec/snow.c|  25 +--
>  libavcodec/twinvq.c  |  23 +-
>  libavutil/internal.h |  24 +-
>  libswscale/utils.c   |  40 -
>  16 files changed, 260 insertions(+), 312 deletions(-)
> 
> diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> index 4d0abb1..9855e40 100644
> --- a/libavcodec/aacenc.c
> +++ b/libavcodec/aacenc.c
> @@ -939,16 +939,14 @@ static av_cold int dsp_init(AVCodecContext *avctx, 
> AACEncContext *s)
>  
>  static av_cold int alloc_buffers(AVCodecContext *avctx, AACEncContext *s)
>  {
> -int ch;
> -FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 
> * sizeof(s->buffer.samples[0]), alloc_fail);
> -FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
> sizeof(ChannelElement), alloc_fail);
> +int ret, ch;
> +FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
> sizeof(s->buffer.samples[0]), ret, return ret);
> +FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
> ret, return ret);
>  
>  for(ch = 0; ch < s->channels; ch++)
>  s->planar_samples[ch] = s->buffer.samples + 3 * 1024 * ch;
>  
>  return 0;
> -alloc_fail:
> -return AVERROR(ENOMEM);
>  }
>  
>  static av_cold void aac_encode_init_tables(void)
> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
> index ddbc7b2..19434d4 100644
> --- a/libavcodec/ac3enc.c
> +++ b/libavcodec/ac3enc.c
> @@ -2322,60 +2322,60 @@ static av_cold void set_bandwidth(AC3EncodeContext *s)
>  
>  static av_cold int allocate_buffers(AC3EncodeContext *s)
>  {
> -AVCodecContext *avctx = s->avctx;
>  int blk, ch;
>  int channels = s->channels + 1; /* includes coupling channel */
>  int channel_blocks = channels * s->num_blocks;
>  int total_coefs= AC3_MAX_COEFS * channel_blocks;
> +int ret;
> +
> +if (ret = s->allocate_sample_buffers(s))
> +return ret;
>  
> -if (s->allocate_sample_buffers(s))
> -goto alloc_fail;
> -
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap_buffer, total_coefs,
> - sizeof(*s->bap_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap1_buffer, total_coefs,
> - sizeof(*s->bap1_buffer), alloc_fail);
> -FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->mdct_coef_buffer, total_coefs,
> -  sizeof(*s->mdct_coef_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->exp_buffer, total_coefs,
> - sizeof(*s->exp_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->grouped_exp_buffer, channel_blocks, 128 
> *
> - sizeof(*s->grouped_exp_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->psd_buffer, total_coefs,
> - sizeof(*s->psd_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->band_psd_buffer, channel_blocks, 64 *
> - sizeof(*s->band_psd_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->mask_buffer, channel_blocks, 64 *
> - sizeof(*s->mask_buffer), alloc_fail);
> -FF_ALLOC_ARRAY_OR_GOTO(avctx, s->qmant_buffer, total_coefs,
> - sizeof(*s->qmant_buffer), alloc_fail);
> +FF_ALLOC_ARRAY_OR_GOTO(s->bap_buffer, total_coefs,
> + sizeof(*s->bap_buffer), ret, return ret);
> +FF_ALLOC_ARRAY_OR_GOTO(s->bap1_buffer, total_coefs,
> + sizeof(*s->bap1_buffer), ret, return ret);
> +FF_ALLOCZ_ARRAY_OR_GOTO(s->mdct_coef_buffer, total_coefs,
> +  sizeof(*s->mdct_coef_buffer), ret, return ret);
> +FF_ALLOC_ARRAY_OR_GOTO(s->exp_buffer, total_coefs,
> + sizeof(*s->exp_buffer), ret, return ret);
> +FF_ALLOC_ARRAY_OR_GOTO(s->grouped_exp_buffer, channel_blocks, 128 *
> + sizeof(*s->grouped_exp_buffer), ret, return ret);
> +FF_ALLOC_ARRAY_OR_GOTO(s->psd_buffer, total_coefs,
> +

[FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[lance . lmwang]

From: Limin Wang 

Please refer to the details here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/1589212343-8334-1-git-send-email-lance.lmw...@gmail.com/

Suggested-by: Nicolas George 
Signed-off-by: Limin Wang 
---
 libavcodec/aacenc.c  |   8 ++--
 libavcodec/ac3enc.c  | 102 +--
 libavcodec/ac3enc_template.c |  15 +++
 libavcodec/adpcmenc.c|  35 +--
 libavcodec/alac.c|  17 +++-
 libavcodec/apedec.c  |   9 ++--
 libavcodec/dnxhdenc.c|  73 +++
 libavcodec/h264dec.c |  69 ++---
 libavcodec/iirfilter.c   |  12 ++---
 libavcodec/mpegpicture.c |  11 ++---
 libavcodec/mpegvideo.c   |  78 +++--
 libavcodec/mpegvideo_enc.c   |  31 +
 libavcodec/snow.c|  25 +--
 libavcodec/twinvq.c  |  23 +-
 libavutil/internal.h |  24 +-
 libswscale/utils.c   |  40 -
 16 files changed, 260 insertions(+), 312 deletions(-)

diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index 4d0abb1..9855e40 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -939,16 +939,14 @@ static av_cold int dsp_init(AVCodecContext *avctx, 
AACEncContext *s)
 
 static av_cold int alloc_buffers(AVCodecContext *avctx, AACEncContext *s)
 {
-int ch;
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
sizeof(ChannelElement), alloc_fail);
+int ret, ch;
+FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
ret, return ret);
 
 for(ch = 0; ch < s->channels; ch++)
 s->planar_samples[ch] = s->buffer.samples + 3 * 1024 * ch;
 
 return 0;
-alloc_fail:
-return AVERROR(ENOMEM);
 }
 
 static av_cold void aac_encode_init_tables(void)
diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index ddbc7b2..19434d4 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -2322,60 +2322,60 @@ static av_cold void set_bandwidth(AC3EncodeContext *s)
 
 static av_cold int allocate_buffers(AC3EncodeContext *s)
 {
-AVCodecContext *avctx = s->avctx;
 int blk, ch;
 int channels = s->channels + 1; /* includes coupling channel */
 int channel_blocks = channels * s->num_blocks;
 int total_coefs= AC3_MAX_COEFS * channel_blocks;
+int ret;
+
+if (ret = s->allocate_sample_buffers(s))
+return ret;
 
-if (s->allocate_sample_buffers(s))
-goto alloc_fail;
-
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap_buffer, total_coefs,
- sizeof(*s->bap_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap1_buffer, total_coefs,
- sizeof(*s->bap1_buffer), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->mdct_coef_buffer, total_coefs,
-  sizeof(*s->mdct_coef_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->exp_buffer, total_coefs,
- sizeof(*s->exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->grouped_exp_buffer, channel_blocks, 128 *
- sizeof(*s->grouped_exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->psd_buffer, total_coefs,
- sizeof(*s->psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->band_psd_buffer, channel_blocks, 64 *
- sizeof(*s->band_psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->mask_buffer, channel_blocks, 64 *
- sizeof(*s->mask_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->qmant_buffer, total_coefs,
- sizeof(*s->qmant_buffer), alloc_fail);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap_buffer, total_coefs,
+ sizeof(*s->bap_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap1_buffer, total_coefs,
+ sizeof(*s->bap1_buffer), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->mdct_coef_buffer, total_coefs,
+  sizeof(*s->mdct_coef_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->exp_buffer, total_coefs,
+ sizeof(*s->exp_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->grouped_exp_buffer, channel_blocks, 128 *
+ sizeof(*s->grouped_exp_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->psd_buffer, total_coefs,
+ sizeof(*s->psd_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->band_psd_buffer, channel_blocks, 64 *
+ sizeof(*s->band_psd_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->mask_buffer, channel_blocks, 64 *
+ sizeof(*s->mask_buffer), ret, return ret);
+FF_ALLOC

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Marton Balint]

On Wed, 13 May 2020, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Please refer to the details here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/1589212343-8334-1-git-send-email-lance.lmw...@gmail.com/

Suggested-by: Nicolas George 
Signed-off-by: Limin Wang 
---
libavcodec/aacenc.c  |   8 ++--
libavcodec/ac3enc.c  | 102 +--
libavcodec/ac3enc_template.c |  15 +++
libavcodec/adpcmenc.c|  35 +--
libavcodec/alac.c|  17 +++-
libavcodec/apedec.c  |   9 ++--
libavcodec/dnxhdenc.c|  73 +++
libavcodec/h264dec.c |  69 ++---
libavcodec/iirfilter.c   |  12 ++---
libavcodec/mpegpicture.c |  11 ++---
libavcodec/mpegvideo.c   |  78 +++--
libavcodec/mpegvideo_enc.c   |  31 +
libavcodec/snow.c|  25 +--
libavcodec/twinvq.c  |  23 +-
libavutil/internal.h |  24 +-
libswscale/utils.c   |  40 -
16 files changed, 260 insertions(+), 312 deletions(-)

diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index 4d0abb1..9855e40 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -939,16 +939,14 @@ static av_cold int dsp_init(AVCodecContext *avctx, 
AACEncContext *s)

static av_cold int alloc_buffers(AVCodecContext *avctx, AACEncContext *s)
{
-int ch;
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
sizeof(ChannelElement), alloc_fail);
+int ret, ch;
+FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
ret, return ret);


If you want to change the existing macro, then you have to rename it, 
because it no longer does goto...


Also I kind of disagree with Nicholas that we should always assign a ret 
variable, I think a single error statement is better. For example in the 
case above a goto fail is a lot nicer (and returning ENOMEM) there.


Please wait a bit before a conclusion is reached about the macro, 
and only implement it later.


Also this patch is non-trivial now, and some changes are definitely not OK 
(e.g removing  ff_h264_free_tables(h) and similar cleanup functions from 
the failure path).


Regards,
Marton




for(ch = 0; ch < s->channels; ch++)
s->planar_samples[ch] = s->buffer.samples + 3 * 1024 * ch;

return 0;
-alloc_fail:
-return AVERROR(ENOMEM);
}

static av_cold void aac_encode_init_tables(void)
diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index ddbc7b2..19434d4 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -2322,60 +2322,60 @@ static av_cold void set_bandwidth(AC3EncodeContext *s)

static av_cold int allocate_buffers(AC3EncodeContext *s)
{
-AVCodecContext *avctx = s->avctx;
int blk, ch;
int channels = s->channels + 1; /* includes coupling channel */
int channel_blocks = channels * s->num_blocks;
int total_coefs= AC3_MAX_COEFS * channel_blocks;
+int ret;
+
+if (ret = s->allocate_sample_buffers(s))
+return ret;

-if (s->allocate_sample_buffers(s))
-goto alloc_fail;
-
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap_buffer, total_coefs,
- sizeof(*s->bap_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap1_buffer, total_coefs,
- sizeof(*s->bap1_buffer), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->mdct_coef_buffer, total_coefs,
-  sizeof(*s->mdct_coef_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->exp_buffer, total_coefs,
- sizeof(*s->exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->grouped_exp_buffer, channel_blocks, 128 *
- sizeof(*s->grouped_exp_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->psd_buffer, total_coefs,
- sizeof(*s->psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->band_psd_buffer, channel_blocks, 64 *
- sizeof(*s->band_psd_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->mask_buffer, channel_blocks, 64 *
- sizeof(*s->mask_buffer), alloc_fail);
-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->qmant_buffer, total_coefs,
- sizeof(*s->qmant_buffer), alloc_fail);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap_buffer, total_coefs,
+ sizeof(*s->bap_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->bap1_buffer, total_coefs,
+ sizeof(*s->bap1_buffer), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->mdct_coef_buffer, total_coefs,
+  sizeof(*s->mdct_coef_buffer), ret, return ret);
+FF_ALLOC_ARRAY_OR_GOTO(s->

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

On Fri, May 8, 2020 at 5:21 PM Dale Curtis  wrote:

> On Wed, May 6, 2020 at 7:03 AM Michael Niedermayer 
> wrote:
>
>> On Mon, May 04, 2020 at 04:06:56PM -0700, Dale Curtis wrote:
>> > On Mon, May 4, 2020 at 3:39 PM Michael Niedermayer
>> 
>> > wrote:
>> >
>> > > On Mon, May 04, 2020 at 02:19:47PM -0700, Dale Curtis wrote:
>> > > > On Mon, May 4, 2020 at 1:48 PM Michael Niedermayer
>> > > 
>> > > [...]
>> > >
>> >
>> >  You snipped out the example I provided, but did you have an opinion on
>> which approach looked best there?
>>
>> yes because it was messed up from linebreaks which made both variants
>> unreadable
>>
>
> Sorry, here's it in pastebin: https://pastebin.com/raw/p1VyuktF
>

Bump, for this question. Thanks. As mentioned earlier, I have 5 integer
overflow fuzzing issues I'd like to resolve using whatever we decide here.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avutil/opt: add AV_OPT_FLAG_CHILD_CONSTS

[Marton Balint]

On Tue, 12 May 2020, Michael Niedermayer wrote:


On Mon, May 11, 2020 at 09:35:17PM +0200, Marton Balint wrote:

This will be used for AVCodecContext->profile. By specifying constants in the
encoders we won't have to use the common AVCodecContext options table and
different encoders can use the same profile name even with different values.

Signed-off-by: Marton Balint 
---
 doc/APIchanges  | 3 +++
 libavutil/opt.c | 3 ++-
 libavutil/opt.h | 1 +
 libavutil/version.h | 2 +-
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 75cfdb08b0..235888c174 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21

 API changes, most recent first:

+2020-05-xx - xx - lavu 56.44.101 - opt.h
+  Add AV_OPT_FLAG_CHILD_CONSTS.
+
 2020-05-10 - xx - lavu 56.44.100 - hwcontext_vulkan.h
   Add enabled_inst_extensions, num_enabled_inst_extensions, 
enabled_dev_extensions
   and num_enabled_dev_extensions fields to AVVulkanDeviceContext
diff --git a/libavutil/opt.c b/libavutil/opt.c
index b792dec01c..423313bce2 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -256,11 +256,12 @@ static int set_string_number(void *obj, void *target_obj, 
const AVOption *o, con
 }

 {
-const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, 
o->unit, 0, 0);
 int res;
 int ci = 0;
 double const_values[64];
 const char * const_names[64];
+int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ? 
AV_OPT_SEARCH_CHILDREN : 0;
+const AVOption *o_named = av_opt_find(target_obj, i ? buf : val, 
o->unit, 0, search_flags);
 if (o_named && o_named->type == AV_OPT_TYPE_CONST)
 d = DEFAULT_NUMVAL(o_named);
 else {
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 1969c984dd..e46119572a 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -291,6 +291,7 @@ typedef struct AVOption {
 #define AV_OPT_FLAG_RUNTIME_PARAM   (1<<15) ///< a generic parameter which can 
be set by the user at runtime
 #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can 
be set by the user for filtering
 #define AV_OPT_FLAG_DEPRECATED  (1<<17) ///< set if option is deprecated, 
users should refer to AVOption.help text for more information



+#define AV_OPT_FLAG_CHILD_CONSTS(1<<18) ///< set if option constants can 
also reside in child objects


why is this needed and not default ?


Because a child object can possibly use the same option name with 
different semantics or for a different purpose.


In order for this to not cause problems, the unit name used have to be 
somewhat unique, and AV_OPT_FLAG_CHILD_CONSTS should only be set for 
cases where it already is. That is why I change the unit name of 
AVCodecContext->profile to "avctx.profile" in a later patch.


Regards,
Marton



thx

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> > -FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 
> > 1024 * sizeof(s->buffer.samples[0]), alloc_fail);
> > -FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
> > sizeof(ChannelElement), alloc_fail);
> > +int ret, ch;
> > +FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
> > sizeof(s->buffer.samples[0]), ret, return ret);
> > +FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], 
> > sizeof(ChannelElement), ret, return ret);
> 
> If you want to change the existing macro, then you have to rename it,
> because it no longer does goto...

Indeed. But also, it must do the same. Replacing a goto with a direct
return is not correct.

> Also I kind of disagree with Nicholas that we should always assign a ret
> variable, I think a single error statement is better. For example in the
> case above a goto fail is a lot nicer (and returning ENOMEM) there.

This is a mistake because you invent the error code on the call site.
The function/macro knows what the error code, the caller uses the error
code. Otherwise, changes in the function/macro can leave the error code
invalid.

We should never let the caller assume the error code.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Marton Balint]

On Tue, 12 May 2020, Nicolas George wrote:


Marton Balint (12020-05-12):

-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), alloc_fail);
-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
sizeof(ChannelElement), alloc_fail);
+int ret, ch;
+FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
sizeof(s->buffer.samples[0]), ret, return ret);
+FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
ret, return ret);


If you want to change the existing macro, then you have to rename it,
because it no longer does goto...


Indeed. But also, it must do the same. Replacing a goto with a direct
return is not correct.


Also I kind of disagree with Nicholas that we should always assign a ret
variable, I think a single error statement is better. For example in the
case above a goto fail is a lot nicer (and returning ENOMEM) there.


This is a mistake because you invent the error code on the call site.
The function/macro knows what the error code, the caller uses the error
code. Otherwise, changes in the function/macro can leave the error code
invalid.

We should never let the caller assume the error code.


And you assume that I want to assign the error code to ret. Wrong. What if 
I want to return it as is? Or what if I want to return NULL beacuse the 
function returns a pointer? Using variables is complicated. Constants 
make the code more simple and readable.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> And you assume that I want to assign the error code to ret. Wrong. What if I
> want to return it as is?

Assign it to ret and forward ret.

>  Or what if I want to return NULL beacuse the
> function returns a pointer?

Bad design, fix it.

>   Using variables is complicated. Constants make
> the code more simple and readable.

Simpler now, much more complex later to fix the incorrect error codes.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Marton Balint]

On Tue, 12 May 2020, Nicolas George wrote:


Marton Balint (12020-05-12):

And you assume that I want to assign the error code to ret. Wrong. What if I
want to return it as is?


Assign it to ret and forward ret.


   Or what if I want to return NULL beacuse the
function returns a pointer?


Bad design, fix it.


Using variables is complicated. Constants make
the code more simple and readable.


Simpler now, much more complex later to fix the incorrect error codes.


I think you want to force your preferred pattern to cases which clearly do 
not benefit from it. I still have to disagree. I suggest we make multilpe 
macros, and the user can decide which one to use.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> I think you want to force your preferred pattern to cases which clearly do
> not benefit from it.

I want to prevent any pattern where the error code is lost or invented.
We already had enough work old cases where we were not careful enough
with it, I oppose to anything that could give us more similar work.

If you do not like my proposal, feel free to make counter-proposals, but
if they pose the risk of losing the error code or let the caller invent
it, I will oppose them.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Marton Balint]

On Tue, 12 May 2020, Nicolas George wrote:


Marton Balint (12020-05-12):

I think you want to force your preferred pattern to cases which clearly do
not benefit from it.


I want to prevent any pattern where the error code is lost or invented.
We already had enough work old cases where we were not careful enough
with it, I oppose to anything that could give us more similar work.

If you do not like my proposal, feel free to make counter-proposals, but
if they pose the risk of losing the error code or let the caller invent
it, I will oppose them.


Then I guess this patch got bikeshedded to pieces with no clear 
resolution. It was a cleanup attempt anyway, so I suggest the author to 
simply abandon it.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Nicolas George]

Marton Balint (12020-05-12):
> Then I guess this patch got bikeshedded to pieces with no clear resolution.
> It was a cleanup attempt anyway, so I suggest the author to simply abandon
> it.

Do you mean you prefer to kill the patch than reach a consensus?

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] pixblockdsp, avdct: Add get_pixels_unaligned

[Michael Niedermayer]

On Tue, May 12, 2020 at 11:25:34AM +0300, Martin Storsjö wrote:
> Use this in vf_spp.c, where the get_pixels operation is done on
> unaligned source addresses.
> 
> This fixes fate-filter-spp on armv7.

LGTM


> ---
> People more familiar with the other assembly implementations of
> get_pixels (in particular, x86) can hook them up to
> get_pixels_unaligned if unaligned use explicitly is ok; as far as I
> can read at least ff_get_pixels_mmx, it looks like it expects the source
> to be aligned, but in practice it does seem to run fine even if
> ff_get_pixels_sse2 is disabled.
> ---

id suggest that code that has been tested with unaligned data and works
is enabled

thx


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

There will always be a question for which you do not know the correct answer.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/cdtoons: Check sprite_offset is within the packet

[Michael Niedermayer]

On Mon, May 11, 2020 at 11:12:45PM +0200, Paul B Mahol wrote:
> probably ok

will apply

thx

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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/iff: Test video_size being non zero

[Michael Niedermayer]

On Mon, May 11, 2020 at 10:30:45PM +0200, Michael Niedermayer wrote:
> Fixes: Out of array access
> Fixes: 
> 20659/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5658548592967680
> Fixes: 
> 20659/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5723561177382912
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/iff.c | 2 ++
>  1 file changed, 2 insertions(+)

will apply

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/ralf: Check num_blocks before use

[Michael Niedermayer]

On Mon, May 11, 2020 at 10:30:46PM +0200, Michael Niedermayer wrote:
> Fixes: out of array access
> Fixes: 
> 20659/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RALF_fuzzer-5739471895265280
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/ralf.c | 2 ++
>  1 file changed, 2 insertions(+)

will apply

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

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavcodec/decode: Mark decode_simple_internal() as inline

[Michael Niedermayer]

On Tue, May 12, 2020 at 12:46:35AM +0200, Michael Niedermayer wrote:
> This was suggested in
> https://github.com/google/oss-fuzz/issues/3787
> to reduce the grouping errors by oss-fuzz
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/decode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] pixblockdsp, avdct: Add get_pixels_unaligned

[Martin Storsjö]

On Tue, 12 May 2020, Michael Niedermayer wrote:


On Tue, May 12, 2020 at 11:25:34AM +0300, Martin Storsjö wrote:

Use this in vf_spp.c, where the get_pixels operation is done on
unaligned source addresses.

This fixes fate-filter-spp on armv7.


LGTM



---
People more familiar with the other assembly implementations of
get_pixels (in particular, x86) can hook them up to
get_pixels_unaligned if unaligned use explicitly is ok; as far as I
can read at least ff_get_pixels_mmx, it looks like it expects the source
to be aligned, but in practice it does seem to run fine even if
ff_get_pixels_sse2 is disabled.
---


id suggest that code that has been tested with unaligned data and works
is enabled


Well ff_get_pixels_mmx uses "mova", which I'd presume is "aligned", but it 
still doesn't seem to crash when used with vf_spp today, even if that one 
uses it on unaligned addresses. So I'm not sure if I'm just being lucky, 
or is it guaranteed that this "mova" always will work on unaligned 
addresses?


As someone who doesn't do much x86 asm, I wouldn't base such an assignment 
just on "it seems to work for me".


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] pixblockdsp, avdct: Add get_pixels_unaligned

[Michael Niedermayer]

On Tue, May 12, 2020 at 10:04:13PM +0300, Martin Storsjö wrote:
> On Tue, 12 May 2020, Michael Niedermayer wrote:
> 
> >On Tue, May 12, 2020 at 11:25:34AM +0300, Martin Storsjö wrote:
> >>Use this in vf_spp.c, where the get_pixels operation is done on
> >>unaligned source addresses.
> >>
> >>This fixes fate-filter-spp on armv7.
> >
> >LGTM
> >
> >
> >>---
> >>People more familiar with the other assembly implementations of
> >>get_pixels (in particular, x86) can hook them up to
> >>get_pixels_unaligned if unaligned use explicitly is ok; as far as I
> >>can read at least ff_get_pixels_mmx, it looks like it expects the source
> >>to be aligned, but in practice it does seem to run fine even if
> >>ff_get_pixels_sse2 is disabled.
> >>---
> >
> >id suggest that code that has been tested with unaligned data and works
> >is enabled
> 
> Well ff_get_pixels_mmx uses "mova", which I'd presume is "aligned", but it
> still doesn't seem to crash when used with vf_spp today, even if that one
> uses it on unaligned addresses. So I'm not sure if I'm just being lucky, or
> is it guaranteed that this "mova" always will work on unaligned addresses?
> 
> As someone who doesn't do much x86 asm, I wouldn't base such an assignment
> just on "it seems to work for me".

you can look at libavutil/x86/x86inc.asm
for mmx both mova and movu are mapped to the same instruction

%define mova movq
%define movu movq

Also there is intels instruction set reference that should document alignment
requirements but that doesnt seem to
document alignment requirements in a way that i can easily find from
starting with a random instruction name

but likely someone reading has a link to nice short / table / summary of 
alignment requirements 

thx

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 0/2] avformat/tls_channel: fixes to make keep-alive work

[Jan Ekström]

Without these two changes, keep-alive utilizing functionality as well as
other connectivity that does not close the connection would not get all
the available data, and instead would be in a loop of recv waiting to be
stopped by the retry wrapper (which would then promptly call read
again).

Quite possible that this also fixes Trac ticket #7894, but unfortunately
I do not have any RTMPS streams available for testing.

The improvements were tested with an HTTPS HLS stream, and the quality
of life difference is significant.

Jan Ekström (2):
  avformat/tls_schannel: always decrypt all received data
  avformat/tls_schannel: immediately return decrypted data if available

 libavformat/tls_schannel.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.26.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avformat/tls_schannel: immediately return decrypted data if available

[Jan Ekström]

Until now, we would have only attempted to utilize already decrypted
data if it was enough to fill the size of buffer requested, that could
very well be up to 32 kilobytes.

With keep-alive connections this would just lead to recv blocking
until rw_timeout had been reached, as the connection would not be
officially closed after each transfer. This would also lead to a
loop, as such timed out I/O request would just be attempted again.

By just returning teh available decrypted data, keep-alive based
connectivity such as HLS playback is fixed with schannel.
---
 libavformat/tls_schannel.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c
index 7a8842e7fe..4a1c5901d0 100644
--- a/libavformat/tls_schannel.c
+++ b/libavformat/tls_schannel.c
@@ -392,7 +392,12 @@ static int tls_read(URLContext *h, uint8_t *buf, int len)
 int size, ret;
 int min_enc_buf_size = len + SCHANNEL_FREE_BUFFER_SIZE;
 
-if (len <= c->dec_buf_offset)
+if (c->dec_buf_offset > 0)
+/* If we have some left-over data from previous network activity,
+ * return it first in case it is enough. It may contain
+ * data that is required to know whether this connection
+ * is still required or not, esp. in case of HTTP keep-alive
+ * connections. */
 goto cleanup;
 
 if (c->sspi_close_notify)
-- 
2.26.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext: add av_hwdevice_ctx_create_derived2

[Mark Thompson]

On 12/05/2020 20:16, Lynne wrote:
> From 45ec8f730a183cd98b1d2d705e7a9582ef2f3f28 Mon Sep 17 00:00:00 2001
> From: Lynne 
> Date: Mon, 11 May 2020 11:02:19 +0100
> Subject: [PATCH] hwcontext: add av_hwdevice_ctx_create_derived_opts
> 
> This allows for users who derive devices to set options for the
> new device context they derive.
> The main use case of this is to allow users to enable extensions
> (such as surface drawing extensions) in Vulkan while deriving from
> the device their frames are on. That way, users don't need to write
> any initialization code themselves, since currently Vulkan prevents
> mixing instances and devices.
> Also, with this, users can also set custom OpenCL extensions such
> as cl_khr_gl_sharing and cl_khr_gl_depth_images.
> Apart from OpenCL and Vulkan, other hwcontexts ignore the opts
> argument since they don't support options at all (or in VAAPI's case,
> options are only used for device selection, which device_derive overrides).
> ---
>  doc/APIchanges |  3 +++
>  libavutil/hwcontext.c  | 16 +---
>  libavutil/hwcontext.h  | 20 
>  libavutil/hwcontext_cuda.c |  1 +
>  libavutil/hwcontext_internal.h |  2 +-
>  libavutil/hwcontext_opencl.c   | 13 +++--
>  libavutil/hwcontext_qsv.c  |  2 +-
>  libavutil/hwcontext_vaapi.c|  2 +-
>  libavutil/hwcontext_vulkan.c   |  8 
>  libavutil/version.h|  2 +-
>  10 files changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 75cfdb08b0..9504da5fa4 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-05-11 - xx - lavu 56.45.100 - hwcontext.h
> +  Add av_hwdevice_ctx_create_derived_opts.
> +
>  2020-05-10 - xx - lavu 56.44.100 - hwcontext_vulkan.h
>Add enabled_inst_extensions, num_enabled_inst_extensions, 
> enabled_dev_extensions
>and num_enabled_dev_extensions fields to AVVulkanDeviceContext
> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
> index b01612de05..9d7b928a6d 100644
> --- a/libavutil/hwcontext.c
> +++ b/libavutil/hwcontext.c
> @@ -643,9 +643,10 @@ fail:
>  return ret;
>  }
>  
> -int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
> -   enum AVHWDeviceType type,
> -   AVBufferRef *src_ref, int flags)
> +int av_hwdevice_ctx_create_derived_opts(AVBufferRef **dst_ref_ptr,
> +enum AVHWDeviceType type,
> +AVDictionary *options,
> +AVBufferRef *src_ref, int flags)
>  {
>  AVBufferRef *dst_ref = NULL, *tmp_ref;
>  AVHWDeviceContext *dst_ctx, *tmp_ctx;
> @@ -677,6 +678,7 @@ int av_hwdevice_ctx_create_derived(AVBufferRef 
> **dst_ref_ptr,
>  tmp_ctx = (AVHWDeviceContext*)tmp_ref->data;
>  if (dst_ctx->internal->hw_type->device_derive) {
>  ret = dst_ctx->internal->hw_type->device_derive(dst_ctx,
> +options,
>  tmp_ctx,
>  flags);
>  if (ret == 0) {
> @@ -709,6 +711,14 @@ fail:
>  return ret;
>  }
>  
> +int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
> +   enum AVHWDeviceType type,
> +   AVBufferRef *src_ref, int flags)
> +{
> +return av_hwdevice_ctx_create_derived_opts(dst_ref_ptr, type, NULL,
> +   src_ref, flags);
> +}
> +
>  static void ff_hwframe_unmap(void *opaque, uint8_t *data)
>  {
>  HWMapDescriptor *hwmap = (HWMapDescriptor*)data;
> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> index f874af9f8f..23e2135255 100644
> --- a/libavutil/hwcontext.h
> +++ b/libavutil/hwcontext.h
> @@ -328,6 +328,26 @@ int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ctx,
> enum AVHWDeviceType type,
> AVBufferRef *src_ctx, int flags);
>  
> +/**
> + * Create a new device of the specified type from an existing device.
> + *
> + * This function performs the same action as av_hwdevice_ctx_create_derived,
> + * however, it is able to set options for the new device to be derived.
> + *
> + * @param dst_ctx On success, a reference to the newly-created
> + *AVHWDeviceContext.
> + * @param typeThe type of the new device to create.
> + * @param options Options for the new device to create, same format as in
> + *av_hwdevice_ctx_create.
> + * @param src_ctx A reference to an existing AVHWDeviceContext which will be
> + *used to create the new device.
> + * @param flags   Currently unused; shoul

[FFmpeg-devel] [PATCH 1/2] avformat/tls_schannel: always decrypt all received data

[Jan Ekström]

The dec_buf seems to be properly managed between read calls,
and we have no logic to decrypt before attempting socket I/O.
Thus - until now - such data would not be decrypted in case of
connections such as HTTP keep-alive, as the recv call would
always get executed first, block until rw_timeout, and then get
retried by retry_transfer_wrapper.

Thus - if data is received - decrypt all of it right away. This way
it is available for the following requests in case they can be
satisfied with it.
---
 libavformat/tls_schannel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c
index 4f0badcb8d..7a8842e7fe 100644
--- a/libavformat/tls_schannel.c
+++ b/libavformat/tls_schannel.c
@@ -424,7 +424,7 @@ static int tls_read(URLContext *h, uint8_t *buf, int len)
 c->enc_buf_offset += ret;
 }
 
-while (c->enc_buf_offset > 0 && sspi_ret == SEC_E_OK && c->dec_buf_offset 
< len) {
+while (c->enc_buf_offset > 0 && sspi_ret == SEC_E_OK) {
 /*  input buffer */
 init_sec_buffer(&inbuf[0], SECBUFFER_DATA, c->enc_buf, 
c->enc_buf_offset);
 
-- 
2.26.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/tls_schannel: always decrypt all received data

[Hendrik Leppkes]

On Wed, May 13, 2020 at 12:35 AM Jan Ekström  wrote:
>
> The dec_buf seems to be properly managed between read calls,
> and we have no logic to decrypt before attempting socket I/O.
> Thus - until now - such data would not be decrypted in case of
> connections such as HTTP keep-alive, as the recv call would
> always get executed first, block until rw_timeout, and then get
> retried by retry_transfer_wrapper.
>
> Thus - if data is received - decrypt all of it right away. This way
> it is available for the following requests in case they can be
> satisfied with it.
> ---
>  libavformat/tls_schannel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c
> index 4f0badcb8d..7a8842e7fe 100644
> --- a/libavformat/tls_schannel.c
> +++ b/libavformat/tls_schannel.c
> @@ -424,7 +424,7 @@ static int tls_read(URLContext *h, uint8_t *buf, int len)
>  c->enc_buf_offset += ret;
>  }
>
> -while (c->enc_buf_offset > 0 && sspi_ret == SEC_E_OK && 
> c->dec_buf_offset < len) {
> +while (c->enc_buf_offset > 0 && sspi_ret == SEC_E_OK) {
>  /*  input buffer */
>  init_sec_buffer(&inbuf[0], SECBUFFER_DATA, c->enc_buf, 
> c->enc_buf_offset);
>
> --
> 2.26.2

 LGTM.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10

[Mark Thompson]

On 12/05/2020 22:42, Lynne wrote:
> Apr 22, 2020, 06:28 by fei.w.w...@intel.com:
>>> -Original Message-
>>> From: Wang, Fei W 
>>> Sent: Wednesday, April 22, 2020 1:23 PM
>>> To: ffmpeg-devel@ffmpeg.org
>>> Cc: Wang, Fei W 
>>> Subject: [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10
>>>
>>> The format is packed RGB with each channel 10 bits available and include 2
>>> bits unused.
>>>
>>> Signed-off-by: Fei Wang 
>>>
> 
> Ping on this patch? I kind of need it to support 10 bit Vulkan sw_formats.
> I'm happy with the name as-is.

Yes, but we probably need to reach a consensus on whether we are accepting the 
addition of these V210-like packed formats first - there were voices against it 
last time this was raised.

I am in favour of allowing it because it is a common buffer format used in both 
graphics and display hardware, and appears in pretty much all related APIs 
(OpenGL, OpenCL, Vulkan, KMS, D3D).

Thanks,

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Limin Wang]

On Tue, May 12, 2020 at 07:58:29PM +0200, Nicolas George wrote:
> Marton Balint (12020-05-12):
> > And you assume that I want to assign the error code to ret. Wrong. What if I
> > want to return it as is?
> 
> Assign it to ret and forward ret.
> 
> >Or what if I want to return NULL beacuse the
> > function returns a pointer?
> 
> Bad design, fix it.

I have notice one such condition:
ff_iir_filter_init_coeffs() in iirfilter.c will return NULL without
error code.

> 
> > Using variables is complicated. Constants make
> > the code more simple and readable.
> 
> Simpler now, much more complex later to fix the incorrect error codes.
> 
> Regards,
> 
> -- 
>   Nicolas George



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/adpcm: Fix integer overflow in ADPCM THP

[Michael Niedermayer]

On Wed, Feb 12, 2020 at 11:03:19PM +0100, Michael Niedermayer wrote:
> The reference (thp.txt) uses floats so wrap around would seem incorrect.
> 
> Fixes: signed integer overflow: 1073741824 + 1073741824 cannot be represented 
> in type 'int'
> Fixes: 
> 20658/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ADPCM_THP_fuzzer-5646302555930624
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/adpcm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

will apply


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

Democracy is the form of government in which you can choose your dictator


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/ivi: Clear got_p_frame before decoding a new frame using it

[Michael Niedermayer]

Fixes: assertion failure
Fixes: 
21666/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_INDEO4_fuzzer-5706468994318336

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

diff --git a/libavcodec/ivi.c b/libavcodec/ivi.c
index 7d3749b818b..c5c50fb5c12 100644
--- a/libavcodec/ivi.c
+++ b/libavcodec/ivi.c
@@ -1192,6 +1192,8 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
*data, int *got_frame,
 AVPacket pkt;
 pkt.data = avpkt->data + (get_bits_count(&ctx->gb) >> 3);
 pkt.size = get_bits_left(&ctx->gb) >> 3;
+ctx->got_p_frame = 0;
+av_frame_unref(ctx->p_frame);
 ff_ivi_decode_frame(avctx, ctx->p_frame, &ctx->got_p_frame, &pkt);
 }
 }
-- 
2.17.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext: add av_hwdevice_ctx_create_derived2

[Lynne]

May 11, 2020, 14:09 by d...@lynne.ee:

> May 11, 2020, 13:55 by jamr...@gmail.com:
>
>> On 5/11/2020 7:25 AM, Lynne wrote:
>>
>>> This allows for users who derive devices to set options for the 
>>> new device context they derive.
>>> The main use case of this is to allow users to enable extensions
>>> (such as surface drawing extensions) in Vulkan while deriving from
>>> the device their frames are on. That way, users don't need to write
>>> any initialization code themselves, since currently Vulkan prevents
>>> mixing instances and devices.
>>> Also, with this, users can also set custom OpenCL extensions such
>>> as cl_khr_gl_sharing and cl_khr_gl_depth_images.
>>> Apart from OpenCL and Vulkan, other hwcontexts ignore the opts
>>> argument since they don't support options at all (or in VAAPI's case,
>>> options are only used for device selection, which device_derive overrides).
>>>
>>> Patch attached.
>>>
>>
>> Could this be av_hwdevice_ctx_create_derived_dict() or similar, please?
>> The 2 suffix is pretty ugly and it would be nice if we can avoid adding
>> more of them.
>>
>
> Sure, changed locally to av_hwdevice_ctx_create_derived_opts.
>

I've been testing and running this code for the past 2 days, planning to push 
this tomorrow if no one LGTMs.
The patch with said name change is attached.

Thanks.

>From 45ec8f730a183cd98b1d2d705e7a9582ef2f3f28 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Mon, 11 May 2020 11:02:19 +0100
Subject: [PATCH] hwcontext: add av_hwdevice_ctx_create_derived_opts

This allows for users who derive devices to set options for the
new device context they derive.
The main use case of this is to allow users to enable extensions
(such as surface drawing extensions) in Vulkan while deriving from
the device their frames are on. That way, users don't need to write
any initialization code themselves, since currently Vulkan prevents
mixing instances and devices.
Also, with this, users can also set custom OpenCL extensions such
as cl_khr_gl_sharing and cl_khr_gl_depth_images.
Apart from OpenCL and Vulkan, other hwcontexts ignore the opts
argument since they don't support options at all (or in VAAPI's case,
options are only used for device selection, which device_derive overrides).
---
 doc/APIchanges |  3 +++
 libavutil/hwcontext.c  | 16 +---
 libavutil/hwcontext.h  | 20 
 libavutil/hwcontext_cuda.c |  1 +
 libavutil/hwcontext_internal.h |  2 +-
 libavutil/hwcontext_opencl.c   | 13 +++--
 libavutil/hwcontext_qsv.c  |  2 +-
 libavutil/hwcontext_vaapi.c|  2 +-
 libavutil/hwcontext_vulkan.c   |  8 
 libavutil/version.h|  2 +-
 10 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 75cfdb08b0..9504da5fa4 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2020-05-11 - xx - lavu 56.45.100 - hwcontext.h
+  Add av_hwdevice_ctx_create_derived_opts.
+
 2020-05-10 - xx - lavu 56.44.100 - hwcontext_vulkan.h
   Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions
   and num_enabled_dev_extensions fields to AVVulkanDeviceContext
diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
index b01612de05..9d7b928a6d 100644
--- a/libavutil/hwcontext.c
+++ b/libavutil/hwcontext.c
@@ -643,9 +643,10 @@ fail:
 return ret;
 }
 
-int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
-   enum AVHWDeviceType type,
-   AVBufferRef *src_ref, int flags)
+int av_hwdevice_ctx_create_derived_opts(AVBufferRef **dst_ref_ptr,
+enum AVHWDeviceType type,
+AVDictionary *options,
+AVBufferRef *src_ref, int flags)
 {
 AVBufferRef *dst_ref = NULL, *tmp_ref;
 AVHWDeviceContext *dst_ctx, *tmp_ctx;
@@ -677,6 +678,7 @@ int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
 tmp_ctx = (AVHWDeviceContext*)tmp_ref->data;
 if (dst_ctx->internal->hw_type->device_derive) {
 ret = dst_ctx->internal->hw_type->device_derive(dst_ctx,
+options,
 tmp_ctx,
 flags);
 if (ret == 0) {
@@ -709,6 +711,14 @@ fail:
 return ret;
 }
 
+int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
+   enum AVHWDeviceType type,
+   AVBufferRef *src_ref, int flags)
+{
+return av_hwdevice_ctx_create_derived_opts(dst_ref_ptr, type, NULL,
+   src_ref, flags);
+}
+
 static void ff_hwframe_unmap(void *opaque, uint8_t *data)
 {
 HWMapDescriptor *hwm

Re: [FFmpeg-devel] [PATCH] avcodec: improve the function of FF_ALLOC{Z}{_ARRAY}_OR_GOTO

[Limin Wang]

On Tue, May 12, 2020 at 06:58:34PM +0200, Marton Balint wrote:
> 
> 
> On Wed, 13 May 2020, lance.lmw...@gmail.com wrote:
> 
> >From: Limin Wang 
> >
> >Please refer to the details here:
> >https://patchwork.ffmpeg.org/project/ffmpeg/patch/1589212343-8334-1-git-send-email-lance.lmw...@gmail.com/
> >
> >Suggested-by: Nicolas George 
> >Signed-off-by: Limin Wang 
> >---
> >libavcodec/aacenc.c  |   8 ++--
> >libavcodec/ac3enc.c  | 102 
> >+--
> >libavcodec/ac3enc_template.c |  15 +++
> >libavcodec/adpcmenc.c|  35 +--
> >libavcodec/alac.c|  17 +++-
> >libavcodec/apedec.c  |   9 ++--
> >libavcodec/dnxhdenc.c|  73 +++
> >libavcodec/h264dec.c |  69 ++---
> >libavcodec/iirfilter.c   |  12 ++---
> >libavcodec/mpegpicture.c |  11 ++---
> >libavcodec/mpegvideo.c   |  78 +++--
> >libavcodec/mpegvideo_enc.c   |  31 +
> >libavcodec/snow.c|  25 +--
> >libavcodec/twinvq.c  |  23 +-
> >libavutil/internal.h |  24 +-
> >libswscale/utils.c   |  40 -
> >16 files changed, 260 insertions(+), 312 deletions(-)
> >
> >diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
> >index 4d0abb1..9855e40 100644
> >--- a/libavcodec/aacenc.c
> >+++ b/libavcodec/aacenc.c
> >@@ -939,16 +939,14 @@ static av_cold int dsp_init(AVCodecContext *avctx, 
> >AACEncContext *s)
> >
> >static av_cold int alloc_buffers(AVCodecContext *avctx, AACEncContext *s)
> >{
> >-int ch;
> >-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->buffer.samples, s->channels, 3 * 1024 
> >* sizeof(s->buffer.samples[0]), alloc_fail);
> >-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->cpe, s->chan_map[0], 
> >sizeof(ChannelElement), alloc_fail);
> >+int ret, ch;
> >+FF_ALLOCZ_ARRAY_OR_GOTO(s->buffer.samples, s->channels, 3 * 1024 * 
> >sizeof(s->buffer.samples[0]), ret, return ret);
> >+FF_ALLOCZ_ARRAY_OR_GOTO(s->cpe, s->chan_map[0], sizeof(ChannelElement), 
> >ret, return ret);
> 
> If you want to change the existing macro, then you have to rename
> it, because it no longer does goto...
> 
sorry, I forgot to change it

> Also I kind of disagree with Nicholas that we should always assign a
> ret variable, I think a single error statement is better. For
> example in the case above a goto fail is a lot nicer (and returning
> ENOMEM) there.
> 
> Please wait a bit before a conclusion is reached about the macro,
> and only implement it later.
> 
> Also this patch is non-trivial now, and some changes are definitely
> not OK (e.g removing  ff_h264_free_tables(h) and similar cleanup
> functions from the failure path).

Yes, I notice it and it's better to keep goto without return first.
If you're failed to disagree with Nicholas, I'll not update anymore.

> 
> Regards,
> Marton
> 
> 
> >
> >for(ch = 0; ch < s->channels; ch++)
> >s->planar_samples[ch] = s->buffer.samples + 3 * 1024 * ch;
> >
> >return 0;
> >-alloc_fail:
> >-return AVERROR(ENOMEM);
> >}
> >
> >static av_cold void aac_encode_init_tables(void)
> >diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
> >index ddbc7b2..19434d4 100644
> >--- a/libavcodec/ac3enc.c
> >+++ b/libavcodec/ac3enc.c
> >@@ -2322,60 +2322,60 @@ static av_cold void set_bandwidth(AC3EncodeContext 
> >*s)
> >
> >static av_cold int allocate_buffers(AC3EncodeContext *s)
> >{
> >-AVCodecContext *avctx = s->avctx;
> >int blk, ch;
> >int channels = s->channels + 1; /* includes coupling channel */
> >int channel_blocks = channels * s->num_blocks;
> >int total_coefs= AC3_MAX_COEFS * channel_blocks;
> >+int ret;
> >+
> >+if (ret = s->allocate_sample_buffers(s))
> >+return ret;
> >
> >-if (s->allocate_sample_buffers(s))
> >-goto alloc_fail;
> >-
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap_buffer, total_coefs,
> >- sizeof(*s->bap_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->bap1_buffer, total_coefs,
> >- sizeof(*s->bap1_buffer), alloc_fail);
> >-FF_ALLOCZ_ARRAY_OR_GOTO(avctx, s->mdct_coef_buffer, total_coefs,
> >-  sizeof(*s->mdct_coef_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->exp_buffer, total_coefs,
> >- sizeof(*s->exp_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->grouped_exp_buffer, channel_blocks, 
> >128 *
> >- sizeof(*s->grouped_exp_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->psd_buffer, total_coefs,
> >- sizeof(*s->psd_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->band_psd_buffer, channel_blocks, 64 *
> >- sizeof(*s->band_psd_buffer), alloc_fail);
> >-FF_ALLOC_ARRAY_OR_GOTO(avctx, s->mask_buffer, channel_blocks, 64 *
> >- sizeof(*s->mask_b

Re: [FFmpeg-devel] [PATCH 2/2] avformat/tls_schannel: immediately return decrypted data if available

[Hendrik Leppkes]

On Wed, May 13, 2020 at 12:12 AM Jan Ekström  wrote:
>
> Until now, we would have only attempted to utilize already decrypted
> data if it was enough to fill the size of buffer requested, that could
> very well be up to 32 kilobytes.
>
> With keep-alive connections this would just lead to recv blocking
> until rw_timeout had been reached, as the connection would not be
> officially closed after each transfer. This would also lead to a
> loop, as such timed out I/O request would just be attempted again.
>
> By just returning teh available decrypted data, keep-alive based
> connectivity such as HLS playback is fixed with schannel.
> ---
>  libavformat/tls_schannel.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c
> index 7a8842e7fe..4a1c5901d0 100644
> --- a/libavformat/tls_schannel.c
> +++ b/libavformat/tls_schannel.c
> @@ -392,7 +392,12 @@ static int tls_read(URLContext *h, uint8_t *buf, int len)
>  int size, ret;
>  int min_enc_buf_size = len + SCHANNEL_FREE_BUFFER_SIZE;
>
> -if (len <= c->dec_buf_offset)
> +if (c->dec_buf_offset > 0)
> +/* If we have some left-over data from previous network activity,
> + * return it first in case it is enough. It may contain
> + * data that is required to know whether this connection
> + * is still required or not, esp. in case of HTTP keep-alive
> + * connections. */
>  goto cleanup;
>

Can you move the comment above the if(..)?
I feel like if blocks with no braces should have the single statement
follow immediately, with no breaks from comments or otherwise, just to
make sure its very clear that it belongs together.

Change otherwise LGTM.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10

[Lynne]

Apr 22, 2020, 06:28 by fei.w.w...@intel.com:

>> -Original Message-
>> From: Wang, Fei W 
>> Sent: Wednesday, April 22, 2020 1:23 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Wang, Fei W 
>> Subject: [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10
>>
>> The format is packed RGB with each channel 10 bits available and include 2
>> bits unused.
>>
>> Signed-off-by: Fei Wang 
>>

Ping on this patch? I kind of need it to support 10 bit Vulkan sw_formats.
I'm happy with the name as-is.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/option_table: mark venc_params as a video decoder flag opt type

[myp...@gmail.com]

On Tue, May 12, 2020 at 10:09 PM James Almer  wrote:
>
> It's not meant for audio or subtitles, or for encoders of any kind.
>
> Signed-off-by: James Almer 
> ---
>  libavcodec/options_table.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 695fa5c211..573675351b 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -81,7 +81,7 @@ static const AVOption avcodec_options[] = {
>  {"export_side_data", "Export metadata as side data", 
> OFFSET(export_side_data), AV_OPT_TYPE_FLAGS, {.i64 = DEFAULT}, 0, UINT_MAX, 
> A|V|S|D|E, "export_side_data"},
>  {"mvs", "export motion vectors through frame side data", 0, 
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, 
> "export_side_data"},
>  {"prft", "export Producer Reference Time through packet side data", 0, 
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX, 
> A|V|S|E, "export_side_data"},
> -{"venc_params", "export video encoding parameters through frame side data", 
> 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, 
> INT_MIN, INT_MAX, A|V|S|E, "export_side_data"},
> +{"venc_params", "export video encoding parameters through frame side data", 
> 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, 
> INT_MIN, INT_MAX, V|D, "export_side_data"},
>  {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0, 
> INT_MAX},
>  {"g", "set the group of picture (GOP) size", OFFSET(gop_size), 
> AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
>  {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), 
> AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
> --
> 2.26.2
>
LGTM
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] hwcontext: add av_hwdevice_ctx_create_derived2

[Lynne]

May 12, 2020, 23:16 by s...@jkqxz.net:

> On 12/05/2020 20:16, Lynne wrote:
>
>> From 45ec8f730a183cd98b1d2d705e7a9582ef2f3f28 Mon Sep 17 00:00:00 2001
>> From: Lynne 
>> Date: Mon, 11 May 2020 11:02:19 +0100
>> Subject: [PATCH] hwcontext: add av_hwdevice_ctx_create_derived_opts
>>
>
> Can you make the argument names and order consistent with the other functions 
> here?
>
> int av_hwdevice_ctx_create(AVBufferRef **device_ctx, enum AVHWDeviceType type,
>  const char *device, AVDictionary *opts, int flags);
>
> int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ctx,
>  enum AVHWDeviceType type,
>  AVBufferRef *src_ctx, int flags);
>
> int av_hwdevice_ctx_create_derived_opts(AVBufferRef **dst_ctx,
>  enum AVHWDeviceType type,
>  AVBufferRef *src_ctx,
>  AVDictionary *options, int flags);
>
> (Also make sure that the names match the documentation, which they do after 
> my suggested change.)
>

Changed. I copied the arguments from av_hwdevice_ctx_create_derived in 
hwcontext.c and didn't
notice they were different to the ones in the header.



>>
>> /**
>>  * Allocate an AVHWFramesContext tied to a given device context.
>> ...
>> diff --git a/libavutil/hwcontext_internal.h b/libavutil/hwcontext_internal.h
>> index dba0f39944..9d66245a23 100644
>> --- a/libavutil/hwcontext_internal.h
>> +++ b/libavutil/hwcontext_internal.h
>> @@ -66,7 +66,7 @@ typedef struct HWContextType {
>>  
>>  int  (*device_create)(AVHWDeviceContext *ctx, const char 
>> *device,
>>  AVDictionary *opts, int flags);
>> -int  (*device_derive)(AVHWDeviceContext *dst_ctx,
>> +int  (*device_derive)(AVHWDeviceContext *dst_ctx, 
>> AVDictionary *opts,
>>  AVHWDeviceContext *src_ctx, int flags);
>>
>
> Arguments in the same order as device_create, please.
>

Changed.



>>
>> int  (*device_init)(AVHWDeviceContext *ctx);
>> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
>> index 41fdfe96f1..b5a282b55b 100644
>> --- a/libavutil/hwcontext_opencl.c
>> +++ b/libavutil/hwcontext_opencl.c
>> @@ -1193,7 +1193,7 @@ static int 
>> opencl_filter_drm_arm_device(AVHWDeviceContext *hwdev,
>>  }
>>  #endif
>>  
>> -static int opencl_device_derive(AVHWDeviceContext *hwdev,
>> +static int opencl_device_derive(AVHWDeviceContext *hwdev, AVDictionary 
>> *opts,
>>  AVHWDeviceContext *src_ctx,
>>  int flags)
>>  {
>> @@ -1207,16 +1207,17 @@ static int opencl_device_derive(AVHWDeviceContext 
>> *hwdev,
>>  // Surface mapping works via DRM PRIME fds with no special
>>  // initialisation required in advance.  This just finds the
>>  // Beignet ICD by name.
>> -AVDictionary *opts = NULL;
>> +AVDictionary *selector_opts = NULL;
>> +av_dict_copy(&selector_opts, opts, 0);
>>  
>> -err = av_dict_set(&opts, "platform_vendor", "Intel", 0);
>> +err = av_dict_set(&selector_opts, "platform_vendor", "Intel", 
>> 0);
>>  if (err >= 0)
>> -err = av_dict_set(&opts, "platform_version", "beignet", 0);
>> +err = av_dict_set(&selector_opts, "platform_version", 
>> "beignet", 0);
>>  if (err >= 0) {
>>  OpenCLDeviceSelector selector = {
>>  .platform_index  = -1,
>>  .device_index= 0,
>> -.context = opts,
>> +.context = selector_opts,
>>  .enumerate_platforms = &opencl_enumerate_platforms,
>>  .filter_platform = &opencl_filter_platform,
>>  .enumerate_devices   = &opencl_enumerate_devices,
>> @@ -1224,7 +1225,7 @@ static int opencl_device_derive(AVHWDeviceContext 
>> *hwdev,
>>  };
>>  err = opencl_device_create_internal(hwdev, &selector, NULL);
>>  }
>> -av_dict_free(&opts);
>> +av_dict_free(&selector_opts);
>>  }
>>  break;
>>  #endif
>>
>
> Can you explain what this change to the code is trying to do?  I might be 
> missing something, but the only additional effect that I can see of passing 
> through the outer options is that it might unexpectedly prevent the search 
> from finding the Beignet ICD (e.g. by having some conflicting device option).
>

I thought the selector options were passed onto device_init through some 
mechanism I couldn't see,
where I thought they're set, and I mistakenly thought "device_extensions" in 
the opencl_device_params
was a list of all accepted keys for options, but I was wrong - they're only 
used as filters for devices.
Still, the selector code is... somewhat sphagetti, so its not that easy to 
figure out how it works.
I've removed this change, only kept the change of name from opts to 
selector_opts so it won't
conflict with the new argument.



>> ...
>>
>> I've been testing and running this code for the past 2 days, planning to 
>> push this tomorrow if no one LGTMs.
>>
>
> For internal things, maybe.  When adding external API which is intended to 
> last forever I don't think it is a good idea to be unilaterally pushing 
> things after three days.
>

Its not a large patch, a

Re: [FFmpeg-devel] [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10

[Lynne]

May 12, 2020, 23:42 by s...@jkqxz.net:

> On 12/05/2020 22:42, Lynne wrote:
>
>> Apr 22, 2020, 06:28 by fei.w.w...@intel.com:
>>
 -Original Message-
 From: Wang, Fei W 
 Sent: Wednesday, April 22, 2020 1:23 PM
 To: ffmpeg-devel@ffmpeg.org
 Cc: Wang, Fei W 
 Subject: [PATCH v2 1/3] lavu/pix_fmt: add new pixel format x2rgb10

 The format is packed RGB with each channel 10 bits available and include 2
 bits unused.

 Signed-off-by: Fei Wang 

>>
>> Ping on this patch? I kind of need it to support 10 bit Vulkan sw_formats.
>> I'm happy with the name as-is.
>>
>
> Yes, but we probably need to reach a consensus on whether we are accepting 
> the addition of these V210-like packed formats first - there were voices 
> against it last time this was raised.
>
> I am in favour of allowing it because it is a common buffer format used in 
> both graphics and display hardware, and appears in pretty much all related 
> APIs (OpenGL, OpenCL, Vulkan, KMS, D3D).
>

Same. With it, we would be able to support kmsgrab from 10bit displays, which 
are becoming more
and more popular.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".