[FFmpeg-devel] [PATCH 1/3] lavc/qsv: apply AVCodecContext AVOption -threads to QSV
By default the SDK creates a thread for each CPU when creating a mfx
session for decoding / encoding, which results in CPU overhead on a
multi CPU system. Actually creating 2 threads is a better choice for
most cases in practice.
This patch allows user to specify the number of threads created for a
mfx session via option -threads. If the number is not specified, 2
threads will be created by default.
Note the SDK requires at least 2 threads to avoid dead locks[1]
[1]https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/mfx_lib/scheduler/linux/src/mfx_scheduler_core_ischeduler.cpp#L90-L93
---
libavcodec/qsv.c | 32
libavcodec/qsvdec.c | 1 +
libavcodec/qsvenc_h264.c | 2 +-
libavcodec/qsvenc_hevc.c | 2 +-
libavcodec/qsvenc_jpeg.c | 1 +
libavcodec/qsvenc_mpeg2.c | 2 +-
libavcodec/qsvenc_vp9.c | 2 +-
7 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index 6e3154e1a3..c725883c5c 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -390,11 +390,27 @@ int ff_qsv_init_internal_session(AVCodecContext *avctx,
QSVSession *qs,
const char *desc;
int ret;
+#if QSV_VERSION_ATLEAST(1, 15)
+mfxExtBuffer *ext_params[1];
+mfxExtThreadsParam thread_param;
+#endif
+
#if QSV_VERSION_ATLEAST(1, 16)
init_par.GPUCopy= gpu_copy;
#endif
init_par.Implementation = impl;
init_par.Version= ver;
+
+#if QSV_VERSION_ATLEAST(1, 15)
+memset(&thread_param, 0, sizeof(thread_param));
+thread_param.Header.BufferId = MFX_EXTBUFF_THREADS_PARAM;
+thread_param.Header.BufferSz = sizeof(thread_param);
+thread_param.NumThread = FFMAX(2, avctx->thread_count);
+ext_params[0]= (mfxExtBuffer *)&thread_param;
+init_par.ExtParam= (mfxExtBuffer **)&ext_params;
+init_par.NumExtParam = 1;
+#endif
+
ret = MFXInitEx(init_par, &qs->session);
if (ret < 0)
return ff_qsv_print_error(avctx, ret,
@@ -709,6 +725,11 @@ int ff_qsv_init_session_device(AVCodecContext *avctx,
mfxSession *psession,
int i, ret;
+#if QSV_VERSION_ATLEAST(1, 15)
+mfxExtBuffer *ext_params[1];
+mfxExtThreadsParam thread_param;
+#endif
+
err = MFXQueryIMPL(parent_session, &impl);
if (err == MFX_ERR_NONE)
err = MFXQueryVersion(parent_session, &ver);
@@ -734,6 +755,17 @@ int ff_qsv_init_session_device(AVCodecContext *avctx,
mfxSession *psession,
#endif
init_par.Implementation = impl;
init_par.Version= ver;
+
+#if QSV_VERSION_ATLEAST(1, 15)
+memset(&thread_param, 0, sizeof(thread_param));
+thread_param.Header.BufferId = MFX_EXTBUFF_THREADS_PARAM;
+thread_param.Header.BufferSz = sizeof(thread_param);
+thread_param.NumThread = FFMAX(2, avctx->thread_count);
+ext_params[0]= (mfxExtBuffer *)&thread_param;
+init_par.ExtParam= (mfxExtBuffer **)&ext_params;
+init_par.NumExtParam = 1;
+#endif
+
err = MFXInitEx(init_par, &session);
if (err != MFX_ERR_NONE)
return ff_qsv_print_error(avctx, err,
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index 5f2e641373..d3365b6f3b 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -846,6 +846,7 @@ AVCodec ff_##x##_qsv_decoder = { \
.close = qsv_decode_close, \
.bsfs = bsf_name, \
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HYBRID, \
+.caps_internal = FF_CODEC_CAP_AUTO_THREADS, \
.priv_class = &x##_qsv_class, \
.pix_fmts = (const enum AVPixelFormat[]){ AV_PIX_FMT_NV12, \
AV_PIX_FMT_P010, \
diff --git a/libavcodec/qsvenc_h264.c b/libavcodec/qsvenc_h264.c
index ddafc45ec3..fb587ff87c 100644
--- a/libavcodec/qsvenc_h264.c
+++ b/libavcodec/qsvenc_h264.c
@@ -196,7 +196,7 @@ AVCodec ff_h264_qsv_encoder = {
AV_PIX_FMT_NONE },
.priv_class = &class,
.defaults = qsv_enc_defaults,
-.caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
+.caps_internal = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_AUTO_THREADS,
.wrapper_name = "qsv",
.hw_configs = ff_qsv_enc_hw_configs,
};
diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
index 347f30655e..a9e5906309 100644
--- a/libavcodec/qsvenc_hevc.c
+++ b/libavcodec/qsvenc_hevc.c
@@ -289,7 +289,7 @@ AVCodec ff_hevc_qsv_encoder = {
AV_PIX_FMT_NONE },
.priv_class = &class,
.defaults = qsv_enc_defaults,
-.caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
+.caps_internal = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_AUTO_THREADS,
.wrapper_name = "qsv",
.hw_configs = ff_qsv_enc_hw_configs,
};
diff --git a/libavcodec/qsvenc_jpeg.c b/libavcodec/qsvenc_jpeg.c
index f76af948
[FFmpeg-devel] [PATCH 2/3] lavu/hwcontext_qsv: limit the number of threads for QSV HW device to 2
The session created for QSV HW device will be used as parent session
only, so we needn't create more threads for this session
---
libavutil/hwcontext_qsv.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 30b0d81f84..84cf5015ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -1142,6 +1142,12 @@ static int
qsv_device_derive_from_child(AVHWDeviceContext *ctx,
mfxHandleType handle_type;
mfxStatus err;
int ret;
+mfxInitParam init_par = { MFX_IMPL_AUTO_ANY };
+
+#if QSV_VERSION_ATLEAST(1, 15)
+mfxExtBuffer *ext_params[1];
+mfxExtThreadsParam thread_param;
+#endif
switch (child_device_ctx->type) {
#if CONFIG_VAAPI
@@ -1167,7 +1173,20 @@ static int
qsv_device_derive_from_child(AVHWDeviceContext *ctx,
goto fail;
}
-err = MFXInit(implementation, &ver, &hwctx->session);
+init_par.Implementation = implementation;
+init_par.Version= ver;
+
+#if QSV_VERSION_ATLEAST(1, 15)
+memset(&thread_param, 0, sizeof(thread_param));
+thread_param.Header.BufferId = MFX_EXTBUFF_THREADS_PARAM;
+thread_param.Header.BufferSz = sizeof(thread_param);
+thread_param.NumThread = 2;
+ext_params[0]= (mfxExtBuffer *)&thread_param;
+init_par.ExtParam= (mfxExtBuffer **)&ext_params;
+init_par.NumExtParam = 1;
+#endif
+
+err = MFXInitEx(init_par, &hwctx->session);
if (err != MFX_ERR_NONE) {
av_log(ctx, AV_LOG_ERROR, "Error initializing an MFX session: "
"%d.\n", err);
@@ -1188,7 +1207,8 @@ static int qsv_device_derive_from_child(AVHWDeviceContext
*ctx,
MFXClose(hwctx->session);
-err = MFXInit(implementation, &ver, &hwctx->session);
+init_par.Version = ver;
+err = MFXInitEx(init_par, &hwctx->session);
if (err != MFX_ERR_NONE) {
av_log(ctx, AV_LOG_ERROR,
"Error initializing an MFX session: %d.\n", err);
--
2.25.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".
[FFmpeg-devel] [PATCH 3/3] lavu/hwcontext_qsv: limit the number of threads used for hwupload & hwdownload to 2
The session for hwupload & hwdownload is used to copy data between
system and video memory, 2 threads are sufficient for the copy in the
SDK.
---
libavutil/hwcontext_qsv.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 84cf5015ff..5dbe2b1701 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -456,8 +456,25 @@ static int qsv_init_internal_session(AVHWFramesContext
*ctx,
mfxVideoParam par;
mfxStatus err;
+mfxInitParam init_par = { MFX_IMPL_AUTO_ANY };
-err = MFXInit(device_priv->impl, &device_priv->ver, session);
+#if QSV_VERSION_ATLEAST(1, 15)
+mfxExtBuffer *ext_params[1];
+mfxExtThreadsParam thread_param;
+
+memset(&thread_param, 0, sizeof(thread_param));
+thread_param.Header.BufferId = MFX_EXTBUFF_THREADS_PARAM;
+thread_param.Header.BufferSz = sizeof(thread_param);
+thread_param.NumThread = 2;
+ext_params[0]= (mfxExtBuffer *)&thread_param;
+init_par.ExtParam= (mfxExtBuffer **)&ext_params;
+init_par.NumExtParam = 1;
+#endif
+
+init_par.Implementation = device_priv->impl;
+init_par.Version = device_priv->ver;
+
+err = MFXInitEx(init_par, session);
if (err != MFX_ERR_NONE) {
av_log(ctx, AV_LOG_ERROR, "Error initializing an internal session\n");
return AVERROR_UNKNOWN;
--
2.25.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 2/3] avcodec/adpcm_swf: remove memory allocation during trellis encoding
On 3/4/21 11:19 am, Zane van Iperen wrote: On 1/4/21 9:34 pm, Zane van Iperen wrote: The block size is hardcoded, so the buffer size is always known. Statically allocate the buffer on the stack. Signed-off-by: Zane van Iperen --- libavcodec/adpcmenc.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Ping on parts 2 and 3. My concern with part 3 is the re-initialisation of fields from extradata. I'm not sure if it should be done on only init, or on both init and flush. ___ Ping again. Will apply in a few days if no objections. ___ 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/7] tests/fate: add a test for APNG_DISPOSE_OP_BACKGROUND
Could someone please upload https://philip.html5.org/tests/apng/015.png -> /apng/015.png? Thanks, -- 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".
[FFmpeg-devel] [PATCH] lavc/pngdec: always create a copy for APNG_DISPOSE_OP_BACKGROUND
Calling av_frame_make_writable() from decoders is tricky, especially
when frame threading is used. It is much simpler and safer to just make
a private copy of the frame.
This is not expected to have a major performance impact, since
APNG_DISPOSE_OP_BACKGROUND is not used often and
av_frame_make_writable() would typically make a copy anyway.
Found-by: James Almer
---
libavcodec/pngdec.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 562c5ffea4..3e509e8ae0 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -89,6 +89,8 @@ typedef struct PNGDecContext {
int has_trns;
uint8_t transparent_color_be[6];
+uint8_t *background_buf;
+unsigned background_buf_allocated;
uint32_t palette[256];
uint8_t *crow_buf;
uint8_t *last_row;
@@ -1079,19 +1081,20 @@ static int handle_p_frame_apng(AVCodecContext *avctx,
PNGDecContext *s,
ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
// need to reset a rectangle to background:
-// create a new writable copy
if (s->last_dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
-int ret = av_frame_make_writable(s->last_picture.f);
-if (ret < 0)
-return ret;
+av_fast_malloc(&s->background_buf, &s->background_buf_allocated,
+ src_stride * p->height);
+if (!s->background_buf)
+return AVERROR(ENOMEM);
-src= s->last_picture.f->data[0];
-src_stride = s->last_picture.f->linesize[0];
+memcpy(s->background_buf, src, src_stride * p->height);
for (y = s->last_y_offset; y < s->last_y_offset + s->last_h; y++) {
-memset(s->last_picture.f->data[0] + src_stride * y +
+memset(s->background_buf + src_stride * y +
s->bpp * s->last_x_offset, 0, s->bpp * s->last_w);
}
+
+src = s->background_buf;
}
// copy unchanged rectangles from the last frame
@@ -1716,6 +1719,7 @@ static av_cold int png_dec_end(AVCodecContext *avctx)
s->last_row_size = 0;
av_freep(&s->tmp_row);
s->tmp_row_size = 0;
+av_freep(&s->background_buf);
av_freep(&s->iccp_data);
av_dict_free(&s->frame_metadata);
--
2.30.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] lavc/pngdec: always create a copy for APNG_DISPOSE_OP_BACKGROUND
Anton Khirnov:
> Calling av_frame_make_writable() from decoders is tricky, especially
> when frame threading is used. It is much simpler and safer to just make
> a private copy of the frame.
> This is not expected to have a major performance impact, since
> APNG_DISPOSE_OP_BACKGROUND is not used often and
> av_frame_make_writable() would typically make a copy anyway.
>
> Found-by: James Almer
> ---
> libavcodec/pngdec.c | 18 +++---
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index 562c5ffea4..3e509e8ae0 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -89,6 +89,8 @@ typedef struct PNGDecContext {
> int has_trns;
> uint8_t transparent_color_be[6];
>
> +uint8_t *background_buf;
> +unsigned background_buf_allocated;
> uint32_t palette[256];
> uint8_t *crow_buf;
> uint8_t *last_row;
> @@ -1079,19 +1081,20 @@ static int handle_p_frame_apng(AVCodecContext *avctx,
> PNGDecContext *s,
> ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
>
> // need to reset a rectangle to background:
> -// create a new writable copy
> if (s->last_dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
> -int ret = av_frame_make_writable(s->last_picture.f);
> -if (ret < 0)
> -return ret;
> +av_fast_malloc(&s->background_buf, &s->background_buf_allocated,
> + src_stride * p->height);
No check for whether the frame is already writable?
> +if (!s->background_buf)
> +return AVERROR(ENOMEM);
>
> -src= s->last_picture.f->data[0];
> -src_stride = s->last_picture.f->linesize[0];
> +memcpy(s->background_buf, src, src_stride * p->height);
>
> for (y = s->last_y_offset; y < s->last_y_offset + s->last_h; y++) {
> -memset(s->last_picture.f->data[0] + src_stride * y +
> +memset(s->background_buf + src_stride * y +
> s->bpp * s->last_x_offset, 0, s->bpp * s->last_w);
> }
> +
> +src = s->background_buf;
> }
>
> // copy unchanged rectangles from the last frame
> @@ -1716,6 +1719,7 @@ static av_cold int png_dec_end(AVCodecContext *avctx)
> s->last_row_size = 0;
> av_freep(&s->tmp_row);
> s->tmp_row_size = 0;
> +av_freep(&s->background_buf);
>
> av_freep(&s->iccp_data);
> av_dict_free(&s->frame_metadata);
>
___
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] Re: [PATCH 01/17] lavu: postpone child_class_next API removal
Quoting Marton Balint (2021-04-05 14:32:54) > > > On Mon, 5 Apr 2021, Anton Khirnov wrote: > > > It has only been deprecated a year ago. > > Considering that this API has limited public usage, I don't think we have > to be strict about the 2 years here, so IMHO it is fine to remove it > at the bump. > Okay, will drop patch if nobody objects. -- 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".
[FFmpeg-devel] [PATCH] lavu/video_enc_params: make sure blocks are properly aligned
---
libavutil/video_enc_params.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
index 635176ab91..1779a8799a 100644
--- a/libavutil/video_enc_params.c
+++ b/libavutil/video_enc_params.c
@@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
AVVideoEncParamsType type,
unsigned int nb_blocks, size_t
*out_size)
{
AVVideoEncParams *par;
-size_t size;
+size_t blocks_offset = FFALIGN(sizeof(*par), 8);
+size_t size = blocks_offset;
-size = sizeof(*par);
if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
return NULL;
size += sizeof(AVVideoBlockParams) * nb_blocks;
@@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
AVVideoEncParamsType type,
par->type = type;
par->nb_blocks = nb_blocks;
par->block_size= sizeof(AVVideoBlockParams);
-par->blocks_offset = sizeof(*par);
+par->blocks_offset = blocks_offset;
if (out_size)
*out_size = size;
--
2.30.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 2/2] avfilter/af_astats: Remove fraction part of integer metadata entries
Quoting Tobias Rapp (2021-03-26 13:58:07) > Using integer format makes the metadata display similar to the log output. > > Fix missing "Overall" metadata key name prefix where necessary. Also > replace whitespace in some metadata key names with underscore character > for consistency. > > Signed-off-by: Tobias Rapp > --- > libavfilter/af_astats.c | 56 > +++-- > 1 file changed, 31 insertions(+), 25 deletions(-) Does this mean that there are no stability guarantees for metadata exported by filters? -- 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] lavc/pngdec: always create a copy for APNG_DISPOSE_OP_BACKGROUND
Quoting Andreas Rheinhardt (2021-04-08 10:56:42)
> Anton Khirnov:
> > Calling av_frame_make_writable() from decoders is tricky, especially
> > when frame threading is used. It is much simpler and safer to just make
> > a private copy of the frame.
> > This is not expected to have a major performance impact, since
> > APNG_DISPOSE_OP_BACKGROUND is not used often and
> > av_frame_make_writable() would typically make a copy anyway.
> >
> > Found-by: James Almer
> > ---
> > libavcodec/pngdec.c | 18 +++---
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index 562c5ffea4..3e509e8ae0 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -89,6 +89,8 @@ typedef struct PNGDecContext {
> > int has_trns;
> > uint8_t transparent_color_be[6];
> >
> > +uint8_t *background_buf;
> > +unsigned background_buf_allocated;
> > uint32_t palette[256];
> > uint8_t *crow_buf;
> > uint8_t *last_row;
> > @@ -1079,19 +1081,20 @@ static int handle_p_frame_apng(AVCodecContext
> > *avctx, PNGDecContext *s,
> > ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
> >
> > // need to reset a rectangle to background:
> > -// create a new writable copy
> > if (s->last_dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
> > -int ret = av_frame_make_writable(s->last_picture.f);
> > -if (ret < 0)
> > -return ret;
> > +av_fast_malloc(&s->background_buf, &s->background_buf_allocated,
> > + src_stride * p->height);
>
> No check for whether the frame is already writable?
Yes, it doesn't seem worth the extra code.
Frankly I've had enough of pngdec bugs and would just prefer to be done
with it.
--
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 2/2] avfilter/af_astats: Remove fraction part of integer metadata entries
Anton Khirnov (12021-04-08): > Does this mean that there are no stability guarantees for metadata > exported by filters? We can have stability for the components that are good enough to be stable, and no stability yet for components that need enhancing. 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] lavu/video_enc_params: make sure blocks are properly aligned
Anton Khirnov:
> ---
> libavutil/video_enc_params.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
> index 635176ab91..1779a8799a 100644
> --- a/libavutil/video_enc_params.c
> +++ b/libavutil/video_enc_params.c
> @@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> AVVideoEncParamsType type,
> unsigned int nb_blocks, size_t
> *out_size)
> {
> AVVideoEncParams *par;
> -size_t size;
> +size_t blocks_offset = FFALIGN(sizeof(*par), 8);
> +size_t size = blocks_offset;
>
> -size = sizeof(*par);
> if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
> return NULL;
> size += sizeof(AVVideoBlockParams) * nb_blocks;
> @@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> AVVideoEncParamsType type,
> par->type = type;
> par->nb_blocks = nb_blocks;
> par->block_size= sizeof(AVVideoBlockParams);
> -par->blocks_offset = sizeof(*par);
> +par->blocks_offset = blocks_offset;
>
> if (out_size)
> *out_size = size;
>
Wouldn't it be safer to just define a struct { AVVideoEncParams header;
AVVideoBlockParams blocks[1]; } and use the offset of blocks in that?
- Andreas
___
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] lavu/video_enc_params: make sure blocks are properly aligned
Quoting Andreas Rheinhardt (2021-04-08 11:41:41)
> Anton Khirnov:
> > ---
> > libavutil/video_enc_params.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
> > index 635176ab91..1779a8799a 100644
> > --- a/libavutil/video_enc_params.c
> > +++ b/libavutil/video_enc_params.c
> > @@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> > AVVideoEncParamsType type,
> > unsigned int nb_blocks, size_t
> > *out_size)
> > {
> > AVVideoEncParams *par;
> > -size_t size;
> > +size_t blocks_offset = FFALIGN(sizeof(*par), 8);
> > +size_t size = blocks_offset;
> >
> > -size = sizeof(*par);
> > if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
> > return NULL;
> > size += sizeof(AVVideoBlockParams) * nb_blocks;
> > @@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> > AVVideoEncParamsType type,
> > par->type = type;
> > par->nb_blocks = nb_blocks;
> > par->block_size= sizeof(AVVideoBlockParams);
> > -par->blocks_offset = sizeof(*par);
> > +par->blocks_offset = blocks_offset;
> >
> > if (out_size)
> > *out_size = size;
> >
> Wouldn't it be safer to just define a struct { AVVideoEncParams header;
> AVVideoBlockParams blocks[1]; } and use the offset of blocks in that?
I don't see how it would be safer and it prevents you from adding new
fields to AVVideoEncParams
--
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] lavu/video_enc_params: make sure blocks are properly aligned
Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-04-08 11:41:41)
>> Anton Khirnov:
>>> ---
>>> libavutil/video_enc_params.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
>>> index 635176ab91..1779a8799a 100644
>>> --- a/libavutil/video_enc_params.c
>>> +++ b/libavutil/video_enc_params.c
>>> @@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
>>> AVVideoEncParamsType type,
>>> unsigned int nb_blocks, size_t
>>> *out_size)
>>> {
>>> AVVideoEncParams *par;
>>> -size_t size;
>>> +size_t blocks_offset = FFALIGN(sizeof(*par), 8);
>>> +size_t size = blocks_offset;
>>>
>>> -size = sizeof(*par);
>>> if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
>>> return NULL;
>>> size += sizeof(AVVideoBlockParams) * nb_blocks;
>>> @@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
>>> AVVideoEncParamsType type,
>>> par->type = type;
>>> par->nb_blocks = nb_blocks;
>>> par->block_size= sizeof(AVVideoBlockParams);
>>> -par->blocks_offset = sizeof(*par);
>>> +par->blocks_offset = blocks_offset;
>>>
>>> if (out_size)
>>> *out_size = size;
>>>
>> Wouldn't it be safer to just define a struct { AVVideoEncParams header;
>> AVVideoBlockParams blocks[1]; } and use the offset of blocks in that?
>
> I don't see how it would be safer and it prevents you from adding new
> fields to AVVideoEncParams
>
Safer: You are making assumptions about the alignment of
AVVideoBlockParams; these assumptions may or may not be fulfilled in
practice.
And I really don't get your point about this preventing the addition of
new fields to AVVideoEncParams. If something is added to
AVVideoEncParams, sizeof(header) and offsetof(blocks) will grow
automatically as required.
- Andreas
___
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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 8, 2021, 07:35 by yejun@intel.com:
>
>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of Lynne
>> Sent: 2021年4月8日 12:14
>> To: FFmpeg development discussions and patches
>> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>> Apr 8, 2021, 04:48 by yejun@intel.com:
>>
>> >> > +
>> >> > +#ifndef AVUTIL_BOUNDINGBOX_H
>> >> > +#define AVUTIL_BOUNDINGBOX_H
>> >> > +
>> >> > +#include "rational.h"
>> >> > +#include "avassert.h"
>> >> > +#include "frame.h"
>> >> > +
>> >> > +typedef struct AVBoundingBox {
>> >> > +/**
>> >> > + * Distance in pixels from the top edge of the frame to top
>> >> > + * and bottom, and from the left edge of the frame to left and
>> >> > + * right, defining the bounding box.
>> >> > + */
>> >> > +int top;
>> >> > +int left;
>> >> > +int bottom;
>> >> > +int right;
>> >> >
>> >>
>> >> Why not x, y, w, h?
>> >> It makes more sense, all of coordinates go like this.
>> >>
>> >
>> > We want to keep it consistent with 'struct AVRegionOfInterest',
>> > we also have a plan to add a filter which converts from bounding box
>> > to RegionOfInterest which impacts the encoder.
>> >
>>
>> Not a good idea. Region of interest is only useful to indicate a
>> single region, while this API describes multiple regions
>> within the frame. The video_enc_params API follows the
>> x, y, w, h because it's the easiest to work with, so I'm sure
>> it's well worth going to such coordinates.
>>
>
> RegionOfInterest is similar with boundingbox, it is used for multiple
> regions in an array, see AV_FRAME_DATA_REGIONS_OF_INTEREST.
>
> anyway, I'm ok to change to x,y,w,h.
>
> btw, do we need to change to x,y,w,h for RegionOfInterest? Is such
> change allowed after current major version change?
>
>> >> > +
>> >> > +typedef struct AVBoundingBoxHeader {
>> >> > +/**
>> >> > + * Information about how the bounding box is generated.
>> >> > + * for example, the DNN model name.
>> >> > + */
>> >> > +char source[128];
>> >> > +
>> >> > +/**
>> >> > + * The size of frame when it is detected.
>> >> > + */
>> >> > +int frame_width;
>> >> > +int frame_height;
>> >> >
>> >>
>> >> Why? This side data is attached to AVFrames only, where we
>> >> already have width and height.
>> >>
>> >
>> > The detection result will be used by other filters, for example,
>> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
>> >
>> > The filter dnn_detect detects all the objects (cat, dog, person ...) in a
>> > frame, while dnn_classify classifies one detected object (for example,
>> > person)
>> > for its attribute (for example, emotion, etc.)
>> >
>> > The filter dnn_classify have to check if the frame size is changed after
>> > it is detected, to handle the below filter chain:
>> > dnn_detect -> scale -> dnn_classify
>> >
>>
>> This doesn't look good. Why is dnn_classify needing to know
>> the original frame size at all?
>>
>
> For example, the original size of the frame is 100*100, and dnn_detect
> detects a face at place (10, 10) -> (30, 40), such data will be saved in
> AVBoundingBox.top/left/right/bottom.
>
> Then, the frame is scaled into 50*50.
>
> Then, dnn_classify is used to analyze the emotion of the face, it needs to
> know the frame size (100*100) when it is detected, otherwise, it does not
> work with just (10,10), (30,40) and 50*50.
>
Why can't the scale filter also rescale this side data as well?
>> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
>> >> > index a5ed91b20a..41e22de02a 100644
>> >> > --- a/libavutil/frame.h
>> >> > +++ b/libavutil/frame.h
>> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
>> >> > * Must be present for every frame which should have film grain applied.
>> >> > */
>> >> > AV_FRAME_DATA_FILM_GRAIN_PARAMS,
>> >> > +
>> >> > +/**
>> >> > + * Bounding boxes for object detection and classification, the
>> >> > data is
>> a
>> >> AVBoundingBoxHeader
>> >> > + * followed with an array of AVBoudingBox, and
>> >> AVBoundingBoxHeader.nb_bboxes is the number
>> >> > + * of array element.
>> >> > + */
>> >> > +AV_FRAME_DATA_BOUNDING_BOXES,
>> >> > };
>> >> >
>> >>
>> >> Finally, why call it a Bounding Box? It's not descriptive at all.
>> >> How about "Object Classification"? It makes much more sense, it's
>> >> exactly what this is. So AVObjectClassification, AVObjectClassification,
>> >> AV_FRAME_DATA_OBJECT_CLASSIFICATION and so on.
>> >>
>> >
>> > In object detection papers, bounding box is usually used.
>> > We'd better use the same term, imho, thanks.
>> >
>>
>> Not in this case, API users won't have any idea what this is or what
>> it's for. This is user-facing code after all.
>> Papers in fields can get away with overloading language, but we're
>> trying to make a concise API. Object classification makes sense
>> because this is exactly what this is.
>>
>
> The term bounding box is dominating
[FFmpeg-devel] [GSoC 2021] about proposal
To whom it may concern: May I ask if my GSoC proposal draft is visible, the name should be "FFmpeg Windows Screen Record via Desktop Duplication" Thanks! :) ___ 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] avformat/webvttenc: Fix use of uninitialized variable
Happened in 9168a1c0e67b5c31727b12329b6f52d2bb5e0a14. Signed-off-by: Andreas Rheinhardt --- libavformat/webvttenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/webvttenc.c b/libavformat/webvttenc.c index 809fead69f..2bc86041a7 100644 --- a/libavformat/webvttenc.c +++ b/libavformat/webvttenc.c @@ -87,7 +87,7 @@ static int webvtt_write_packet(AVFormatContext *ctx, AVPacket *pkt) settings = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_SETTINGS, &settings_size); -if (settings_size_int > INT_MAX) +if (settings_size > INT_MAX) return AVERROR(EINVAL); settings_size_int = settings_size; -- 2.27.0 ___ 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] lavu/video_enc_params: make sure blocks are properly aligned
Quoting Andreas Rheinhardt (2021-04-08 12:27:03)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2021-04-08 11:41:41)
> >> Anton Khirnov:
> >>> ---
> >>> libavutil/video_enc_params.c | 6 +++---
> >>> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
> >>> index 635176ab91..1779a8799a 100644
> >>> --- a/libavutil/video_enc_params.c
> >>> +++ b/libavutil/video_enc_params.c
> >>> @@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> >>> AVVideoEncParamsType type,
> >>> unsigned int nb_blocks,
> >>> size_t *out_size)
> >>> {
> >>> AVVideoEncParams *par;
> >>> -size_t size;
> >>> +size_t blocks_offset = FFALIGN(sizeof(*par), 8);
> >>> +size_t size = blocks_offset;
> >>>
> >>> -size = sizeof(*par);
> >>> if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
> >>> return NULL;
> >>> size += sizeof(AVVideoBlockParams) * nb_blocks;
> >>> @@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum
> >>> AVVideoEncParamsType type,
> >>> par->type = type;
> >>> par->nb_blocks = nb_blocks;
> >>> par->block_size= sizeof(AVVideoBlockParams);
> >>> -par->blocks_offset = sizeof(*par);
> >>> +par->blocks_offset = blocks_offset;
> >>>
> >>> if (out_size)
> >>> *out_size = size;
> >>>
> >> Wouldn't it be safer to just define a struct { AVVideoEncParams header;
> >> AVVideoBlockParams blocks[1]; } and use the offset of blocks in that?
> >
> > I don't see how it would be safer and it prevents you from adding new
> > fields to AVVideoEncParams
> >
> Safer: You are making assumptions about the alignment of
> AVVideoBlockParams; these assumptions may or may not be fulfilled in
> practice.
> And I really don't get your point about this preventing the addition of
> new fields to AVVideoEncParams. If something is added to
> AVVideoEncParams, sizeof(header) and offsetof(blocks) will grow
> automatically as required.
Nevermind, I misunderstood your suggestion. I suppose it works better
indeed.
--
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 v4] avformat/utils: add helper functions to retrieve index entries from an AVStream
Quoting James Almer (2021-04-04 01:12:00) > Signed-off-by: James Almer > --- > Now using the avformat_ prefix as Anton requested. I forgot about it when i > made v3. > > libavformat/avformat.h | 39 +++ > libavformat/utils.c| 27 +++ > 2 files changed, 66 insertions(+) > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 6a9b09160c..a1e87ef891 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -2767,6 +2767,45 @@ int av_find_default_stream_index(AVFormatContext *s); > */ > int av_index_search_timestamp(AVStream *st, int64_t timestamp, int flags); > > +/** > + * Get the index entry count for the given AVStream. > + * > + * @param st stream > + * @return the number of index entries in the stream > + */ > +int avformat_index_get_entries_count(AVStream *st); > + > +/** > + * Get the AVIndexEntry corresponding to the given index. > + * > + * @param st Stream containing the requested AVIndexEntry. > + * @param idx The desired index. > + * @return A pointer to the requested AVIndexEntry if it exists, NULL > otherwise. > + * > + * @note The pointer returned by this function is only guaranteed to be valid > + * until any function that could alter the stream or the > AVFormatContext > + * that cointains it is called. > + */ > +const AVIndexEntry *avformat_index_get_entry(AVStream *st, int idx); > + > +/** > + * Get the AVIndexEntry corresponding to the given timestamp. > + * > + * @param st Stream containing the requested AVIndexEntry. > + * @param timestamp Timestamp to retrieve the index entry for. > + * @param flags If AVSEEK_FLAG_BACKWARD then the returned entry will > correspond > + *to the timestamp which is <= the requested one, if > backward > + *is 0, then it will be >= > + *if AVSEEK_FLAG_ANY seek to any frame, only keyframes > otherwise. > + * @return A pointer to the requested AVIndexEntry if it exists, NULL > otherwise. > + * > + * @note The pointer returned by this function is only guaranteed to be valid > + * until any function that could alter the stream or the > AVFormatContext > + * that cointains it is called. How can the user know which functions can "alter the stream or the AVFormatContext"? I would prefer invalidating the pointer on ANY lavf call on that AVFormatContext or its children, including calls to avformat_index_get_entry*. That would allow us more freedom to change the implementation later. -- 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/7] tests/fate: add a test for APNG_DISPOSE_OP_BACKGROUND
On 4/8/2021 5:44 AM, Anton Khirnov wrote: Could someone please upload https://philip.html5.org/tests/apng/015.png -> /apng/015.png? Thanks, Uploaded. ___ 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 v4] avformat/utils: add helper functions to retrieve index entries from an AVStream
On 4/8/2021 10:09 AM, Anton Khirnov wrote: Quoting James Almer (2021-04-04 01:12:00) Signed-off-by: James Almer --- Now using the avformat_ prefix as Anton requested. I forgot about it when i made v3. libavformat/avformat.h | 39 +++ libavformat/utils.c| 27 +++ 2 files changed, 66 insertions(+) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 6a9b09160c..a1e87ef891 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -2767,6 +2767,45 @@ int av_find_default_stream_index(AVFormatContext *s); */ int av_index_search_timestamp(AVStream *st, int64_t timestamp, int flags); +/** + * Get the index entry count for the given AVStream. + * + * @param st stream + * @return the number of index entries in the stream + */ +int avformat_index_get_entries_count(AVStream *st); + +/** + * Get the AVIndexEntry corresponding to the given index. + * + * @param st Stream containing the requested AVIndexEntry. + * @param idx The desired index. + * @return A pointer to the requested AVIndexEntry if it exists, NULL otherwise. + * + * @note The pointer returned by this function is only guaranteed to be valid + * until any function that could alter the stream or the AVFormatContext + * that cointains it is called. + */ +const AVIndexEntry *avformat_index_get_entry(AVStream *st, int idx); + +/** + * Get the AVIndexEntry corresponding to the given timestamp. + * + * @param st Stream containing the requested AVIndexEntry. + * @param timestamp Timestamp to retrieve the index entry for. + * @param flags If AVSEEK_FLAG_BACKWARD then the returned entry will correspond + *to the timestamp which is <= the requested one, if backward + *is 0, then it will be >= + *if AVSEEK_FLAG_ANY seek to any frame, only keyframes otherwise. + * @return A pointer to the requested AVIndexEntry if it exists, NULL otherwise. + * + * @note The pointer returned by this function is only guaranteed to be valid + * until any function that could alter the stream or the AVFormatContext + * that cointains it is called. How can the user know which functions can "alter the stream or the AVFormatContext"? Any function that takes a non const AVStream or AVFormatContext should be considered one that can potentially alter the stream. I would prefer invalidating the pointer on ANY lavf call on that AVFormatContext or its children, including calls to avformat_index_get_entry*. That would allow us more freedom to change the implementation later. avformat_index_get_entry() takes a const AVStream after i included Andreas suggestion when i pushed this patch the other day, so in theory it should not change the index array stored within the AVStream. I'd have to remove the const qualifier. If that's preferred, I can send a patch to make this change to the doxy and 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 v4] avformat/utils: add helper functions to retrieve index entries from an AVStream
Quoting James Almer (2021-04-08 15:19:02) > On 4/8/2021 10:09 AM, Anton Khirnov wrote: > > Quoting James Almer (2021-04-04 01:12:00) > >> Signed-off-by: James Almer > >> --- > >> Now using the avformat_ prefix as Anton requested. I forgot about it when i > >> made v3. > >> > >> libavformat/avformat.h | 39 +++ > >> libavformat/utils.c| 27 +++ > >> 2 files changed, 66 insertions(+) > >> > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h > >> index 6a9b09160c..a1e87ef891 100644 > >> --- a/libavformat/avformat.h > >> +++ b/libavformat/avformat.h > >> @@ -2767,6 +2767,45 @@ int av_find_default_stream_index(AVFormatContext > >> *s); > >>*/ > >> int av_index_search_timestamp(AVStream *st, int64_t timestamp, int > >> flags); > >> > >> +/** > >> + * Get the index entry count for the given AVStream. > >> + * > >> + * @param st stream > >> + * @return the number of index entries in the stream > >> + */ > >> +int avformat_index_get_entries_count(AVStream *st); > >> + > >> +/** > >> + * Get the AVIndexEntry corresponding to the given index. > >> + * > >> + * @param st Stream containing the requested AVIndexEntry. > >> + * @param idx The desired index. > >> + * @return A pointer to the requested AVIndexEntry if it exists, NULL > >> otherwise. > >> + * > >> + * @note The pointer returned by this function is only guaranteed to be > >> valid > >> + * until any function that could alter the stream or the > >> AVFormatContext > >> + * that cointains it is called. > >> + */ > >> +const AVIndexEntry *avformat_index_get_entry(AVStream *st, int idx); > >> + > >> +/** > >> + * Get the AVIndexEntry corresponding to the given timestamp. > >> + * > >> + * @param st Stream containing the requested AVIndexEntry. > >> + * @param timestamp Timestamp to retrieve the index entry for. > >> + * @param flags If AVSEEK_FLAG_BACKWARD then the returned entry > >> will correspond > >> + *to the timestamp which is <= the requested one, if > >> backward > >> + *is 0, then it will be >= > >> + *if AVSEEK_FLAG_ANY seek to any frame, only > >> keyframes otherwise. > >> + * @return A pointer to the requested AVIndexEntry if it exists, NULL > >> otherwise. > >> + * > >> + * @note The pointer returned by this function is only guaranteed to be > >> valid > >> + * until any function that could alter the stream or the > >> AVFormatContext > >> + * that cointains it is called. > > > > How can the user know which functions can "alter the stream or the > > AVFormatContext"? > > Any function that takes a non const AVStream or AVFormatContext should > be considered one that can potentially alter the stream. > > > > > I would prefer invalidating the pointer on ANY lavf call on that > > AVFormatContext or its children, including calls to > > avformat_index_get_entry*. That would allow us more freedom to change > > the implementation later. > > avformat_index_get_entry() takes a const AVStream after i included > Andreas suggestion when i pushed this patch the other day, so in theory > it should not change the index array stored within the AVStream. I'd > have to remove the const qualifier. > If that's preferred, I can send a patch to make this change to the doxy > and signature. It seems preferable to me, but YMMV. -- 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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
> -Original Message-
> From: ffmpeg-devel On Behalf Of Lynne
> Sent: 2021年4月8日 19:35
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
>
> Apr 8, 2021, 07:35 by yejun@intel.com:
>
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of
> Lynne
> >> Sent: 2021年4月8日 12:14
> >> To: FFmpeg development discussions and patches
>
> >> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> >> AV_FRAME_DATA_BOUNDING_BOXES
> >>
> >> Apr 8, 2021, 04:48 by yejun@intel.com:
> >>
> >> >> > +
> >> >> > +#ifndef AVUTIL_BOUNDINGBOX_H
> >> >> > +#define AVUTIL_BOUNDINGBOX_H
> >> >> > +
> >> >> > +#include "rational.h"
> >> >> > +#include "avassert.h"
> >> >> > +#include "frame.h"
> >> >> > +
> >> >> > +typedef struct AVBoundingBox {
> >> >> > +/**
> >> >> > + * Distance in pixels from the top edge of the frame to top
> >> >> > + * and bottom, and from the left edge of the frame to left and
> >> >> > + * right, defining the bounding box.
> >> >> > + */
> >> >> > +int top;
> >> >> > +int left;
> >> >> > +int bottom;
> >> >> > +int right;
> >> >> >
> >> >>
> >> >> Why not x, y, w, h?
> >> >> It makes more sense, all of coordinates go like this.
> >> >>
> >> >
> >> > We want to keep it consistent with 'struct AVRegionOfInterest',
> >> > we also have a plan to add a filter which converts from bounding box
> >> > to RegionOfInterest which impacts the encoder.
> >> >
> >>
> >> Not a good idea. Region of interest is only useful to indicate a
> >> single region, while this API describes multiple regions
> >> within the frame. The video_enc_params API follows the
> >> x, y, w, h because it's the easiest to work with, so I'm sure
> >> it's well worth going to such coordinates.
> >>
> >
> > RegionOfInterest is similar with boundingbox, it is used for multiple
> > regions in an array, see AV_FRAME_DATA_REGIONS_OF_INTEREST.
> >
> > anyway, I'm ok to change to x,y,w,h.
> >
> > btw, do we need to change to x,y,w,h for RegionOfInterest? Is such
> > change allowed after current major version change?
any comment on this about RegionOfInterest? thanks.
> >
> >> >> > +
> >> >> > +typedef struct AVBoundingBoxHeader {
> >> >> > +/**
> >> >> > + * Information about how the bounding box is generated.
> >> >> > + * for example, the DNN model name.
> >> >> > + */
> >> >> > +char source[128];
> >> >> > +
> >> >> > +/**
> >> >> > + * The size of frame when it is detected.
> >> >> > + */
> >> >> > +int frame_width;
> >> >> > +int frame_height;
> >> >> >
> >> >>
> >> >> Why? This side data is attached to AVFrames only, where we
> >> >> already have width and height.
> >> >>
> >> >
> >> > The detection result will be used by other filters, for example,
> >> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
> >> >
> >> > The filter dnn_detect detects all the objects (cat, dog, person ...) in a
> >> > frame, while dnn_classify classifies one detected object (for example,
> person)
> >> > for its attribute (for example, emotion, etc.)
> >> >
> >> > The filter dnn_classify have to check if the frame size is changed after
> >> > it is detected, to handle the below filter chain:
> >> > dnn_detect -> scale -> dnn_classify
> >> >
> >>
> >> This doesn't look good. Why is dnn_classify needing to know
> >> the original frame size at all?
> >>
> >
> > For example, the original size of the frame is 100*100, and dnn_detect
> > detects a face at place (10, 10) -> (30, 40), such data will be saved in
> > AVBoundingBox.top/left/right/bottom.
> >
> > Then, the frame is scaled into 50*50.
> >
> > Then, dnn_classify is used to analyze the emotion of the face, it needs to
> > know the frame size (100*100) when it is detected, otherwise, it does not
> > work with just (10,10), (30,40) and 50*50.
> >
>
> Why can't the scale filter also rescale this side data as well?
I'm afraid that we could not make sure all such filters (including filters in
the
future) to do the rescale. And in the previous discussion, I got to know that
'many other existing side-data types are invalidated by scaling'.
So, we need frame_width and frame_height here.
>
>
> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> >> > index a5ed91b20a..41e22de02a 100644
> >> >> > --- a/libavutil/frame.h
> >> >> > +++ b/libavutil/frame.h
> >> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
> >> >> > * Must be present for every frame which should have film grain
> applied.
> >> >> > */
> >> >> > AV_FRAME_DATA_FILM_GRAIN_PARAMS,
> >> >> > +
> >> >> > +/**
> >> >> > + * Bounding boxes for object detection and classification, the
> data is
> >> a
> >> >> AVBoundingBoxHeader
> >> >> > + * followed with an array of AVBoudingBox, and
> >> >> AVBoundingBoxHeader.nb_bboxes is the number
> >> >> > + * of array element.
> >> >> > + */
>
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Guo, Yejun (12021-04-08): > I'm afraid that we could not make sure all such filters (including filters in > the > future) to do the rescale. And in the previous discussion, I got to know that > 'many other existing side-data types are invalidated by scaling'. > > So, we need frame_width and frame_height here. But the crop or pad filters will also invalidate the information, and in a different way. How should frame_width and frame_height be used to solve all? 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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
> -Original Message- > From: ffmpeg-devel On Behalf Of Nicolas > George > Sent: 2021年4月8日 22:54 > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data > AV_FRAME_DATA_BOUNDING_BOXES > > Guo, Yejun (12021-04-08): > > I'm afraid that we could not make sure all such filters (including filters > > in the > > future) to do the rescale. And in the previous discussion, I got to know > > that > > 'many other existing side-data types are invalidated by scaling'. > > > > So, we need frame_width and frame_height here. > > But the crop or pad filters will also invalidate the information, and in > a different way. How should frame_width and frame_height be used to > solve all? thanks for the new info, my idea was to rescale the bounding box for the frame size change. Looks that we can just output a warning message and don't do the classification if the size changes, otherwise, the filter will give wrong result. > > 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".
Re: [FFmpeg-devel] 4.4 Release Name
On Thu, Apr 08, 2021 at 01:58:23AM +0200, Tomas Härdin wrote: > fre 2021-04-02 klockan 16:35 +0530 skrev Gyan Doshi: > > > > On 2021-04-02 15:29, Michael Niedermayer wrote: > > > Hi all > > > > > > We still need to choose the name for 4.4 > > > previous unused suggestions where: > > > Von Neumann, Lorentz, Poincaré, Desitter, De Broglie, Gauss, Galois, > > > Viterbi, Darwin > > > > > > Feel free to reply with name suggestions > > > If theres a tie then i will randomly pick amongth the tied, i assume all > > > previous > > > suggestions where suggested exactly once already. > > > > Rao > > > > after one of the inventors of the DCT. > > I like this, but the timing isn't great considering he just died its the leading choice ATM from the votes and ill probably make the release today assuming nothing unexpected happens. If anyone wants to change their vote they of course have till the release or more precissely shortly before when ill do a last check of the votes thx [] -- 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] lavc/pngdec: always create a copy for APNG_DISPOSE_OP_BACKGROUND
On Thu, Apr 08, 2021 at 10:46:54AM +0200, Anton Khirnov wrote: > Calling av_frame_make_writable() from decoders is tricky, especially > when frame threading is used. It is much simpler and safer to just make > a private copy of the frame. > This is not expected to have a major performance impact, since > APNG_DISPOSE_OP_BACKGROUND is not used often and > av_frame_make_writable() would typically make a copy anyway. > > Found-by: James Almer > --- > libavcodec/pngdec.c | 18 +++--- > 1 file changed, 11 insertions(+), 7 deletions(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway 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/msp2dec: Check available space in RLE decoder
On Wed, Apr 07, 2021 at 04:59:09PM +0200, Michael Niedermayer wrote:
> On Wed, Apr 07, 2021 at 12:42:50AM +0200, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > Fixes: out of array read
> > > Fixes:
> > > 32968/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSP2_fuzzer-5315296027082752
> > >
> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > libavcodec/msp2dec.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/msp2dec.c b/libavcodec/msp2dec.c
> > > index cc548d218a..87057fb5e2 100644
> > > --- a/libavcodec/msp2dec.c
> > > +++ b/libavcodec/msp2dec.c
> > > @@ -68,9 +68,10 @@ static int msp2_decode_frame(AVCodecContext *avctx,
> > >
> > > bytestream2_init(&gb, buf, pkt_size);
> > > x = 0;
> > > -while (bytestream2_get_bytes_left(&gb) && x < width) {
> > > +while (bytestream2_get_bytes_left(&gb) > 0 && x < width) {
> >
> > This decoder uses the checked bytestream2 API, so != 0 and > 0 should be
> > equivalent for bytestream2_get_bytes_left(&gb).
>
> I changed it to "> 0" because it felt clearer&more robust
> i will drop that as its not needed for the bugfix
>
>
> >
> > > int size = bytestream2_get_byte(&gb);
> > > if (size) {
> > > +size = FFMIN(size, bytestream2_get_bytes_left(&gb));
> > > memcpy(p->data[0] + y * p->linesize[0] + x, gb.buffer,
> > > FFMIN(size, width - x));
> >
> > width can include seven bytes of the packet's padding, but it stays
> > within the padding, so I wonder where the out of array read comes from.
> > The only fishy thing in this decoder I see is that 2 * avctx->height
> > might overflow.
>
> size is a value read from the bytestream, theres no guarntee that
> the amount that byte calls for is not beyond the end of the bytestream
> and its not checked by memcpy
will apply and backport
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 8, 2021, 16:51 by yejun@intel.com:
>
>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of Lynne
>> Sent: 2021年4月8日 19:35
>> To: FFmpeg development discussions and patches
>> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>> Apr 8, 2021, 07:35 by yejun@intel.com:
>>
>> >
>> >
>> >> -Original Message-
>> >> From: ffmpeg-devel On Behalf Of
>> Lynne
>> >> Sent: 2021年4月8日 12:14
>> >> To: FFmpeg development discussions and patches
>>
>> >> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> >> AV_FRAME_DATA_BOUNDING_BOXES
>> >>
>> >> Apr 8, 2021, 04:48 by yejun@intel.com:
>> >>
>> >> >> > +
>> >> >> > +#ifndef AVUTIL_BOUNDINGBOX_H
>> >> >> > +#define AVUTIL_BOUNDINGBOX_H
>> >> >> > +
>> >> >> > +#include "rational.h"
>> >> >> > +#include "avassert.h"
>> >> >> > +#include "frame.h"
>> >> >> > +
>> >> >> > +typedef struct AVBoundingBox {
>> >> >> > +/**
>> >> >> > + * Distance in pixels from the top edge of the frame to top
>> >> >> > + * and bottom, and from the left edge of the frame to left and
>> >> >> > + * right, defining the bounding box.
>> >> >> > + */
>> >> >> > +int top;
>> >> >> > +int left;
>> >> >> > +int bottom;
>> >> >> > +int right;
>> >> >> >
>> >> >>
>> >> >> Why not x, y, w, h?
>> >> >> It makes more sense, all of coordinates go like this.
>> >> >>
>> >> >
>> >> > We want to keep it consistent with 'struct AVRegionOfInterest',
>> >> > we also have a plan to add a filter which converts from bounding box
>> >> > to RegionOfInterest which impacts the encoder.
>> >> >
>> >>
>> >> Not a good idea. Region of interest is only useful to indicate a
>> >> single region, while this API describes multiple regions
>> >> within the frame. The video_enc_params API follows the
>> >> x, y, w, h because it's the easiest to work with, so I'm sure
>> >> it's well worth going to such coordinates.
>> >>
>> >
>> > RegionOfInterest is similar with boundingbox, it is used for multiple
>> > regions in an array, see AV_FRAME_DATA_REGIONS_OF_INTEREST.
>> >
>> > anyway, I'm ok to change to x,y,w,h.
>> >
>> > btw, do we need to change to x,y,w,h for RegionOfInterest? Is such
>> > change allowed after current major version change?
>>
>
> any comment on this about RegionOfInterest? thanks.
>
>> >
>> >> >> > +
>> >> >> > +typedef struct AVBoundingBoxHeader {
>> >> >> > +/**
>> >> >> > + * Information about how the bounding box is generated.
>> >> >> > + * for example, the DNN model name.
>> >> >> > + */
>> >> >> > +char source[128];
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * The size of frame when it is detected.
>> >> >> > + */
>> >> >> > +int frame_width;
>> >> >> > +int frame_height;
>> >> >> >
>> >> >>
>> >> >> Why? This side data is attached to AVFrames only, where we
>> >> >> already have width and height.
>> >> >>
>> >> >
>> >> > The detection result will be used by other filters, for example,
>> >> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
>> >> >
>> >> > The filter dnn_detect detects all the objects (cat, dog, person ...) in
>> >> > a
>> >> > frame, while dnn_classify classifies one detected object (for example,
>> person)
>> >> > for its attribute (for example, emotion, etc.)
>> >> >
>> >> > The filter dnn_classify have to check if the frame size is changed after
>> >> > it is detected, to handle the below filter chain:
>> >> > dnn_detect -> scale -> dnn_classify
>> >> >
>> >>
>> >> This doesn't look good. Why is dnn_classify needing to know
>> >> the original frame size at all?
>> >>
>> >
>> > For example, the original size of the frame is 100*100, and dnn_detect
>> > detects a face at place (10, 10) -> (30, 40), such data will be saved in
>> > AVBoundingBox.top/left/right/bottom.
>> >
>> > Then, the frame is scaled into 50*50.
>> >
>> > Then, dnn_classify is used to analyze the emotion of the face, it needs to
>> > know the frame size (100*100) when it is detected, otherwise, it does not
>> > work with just (10,10), (30,40) and 50*50.
>> >
>>
>> Why can't the scale filter also rescale this side data as well?
>>
>
> I'm afraid that we could not make sure all such filters (including filters in
> the
> future) to do the rescale. And in the previous discussion, I got to know that
> 'many other existing side-data types are invalidated by scaling'.
>
> So, we need frame_width and frame_height here.
>
No, you don't. You just need to make sure filters which change resolution
or do cropping also change the side data parameters.
It's called maintainership. As-is, this won't even work with cropping,
only with basic aspect ratio preserving scaling.
For the lack of a better term, this is a hack.
I would accept just specifying that if the frame dimensions are
altered in any way, the side-data is no longer valid and it's up
to users to figure that out by out of bound coordinates.
This is what we currently do with vi
[FFmpeg-devel] [PATCH] avformat/utils: constrain the guaranteed lifetime of the pointer returned by avformat_index_get_entry()
This will give us more room to improve the implementation later.
Suggested-by: Anton Khirnov
Signed-off-by: James Almer
---
libavformat/avformat.h | 12 ++--
libavformat/utils.c| 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 8600ee1bf7..8b49871cdc 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -2770,10 +2770,10 @@ int avformat_index_get_entries_count(const AVStream
*st);
* @return A pointer to the requested AVIndexEntry if it exists, NULL
otherwise.
*
* @note The pointer returned by this function is only guaranteed to be valid
- * until any function that could alter the stream or the AVFormatContext
- * that contains it is called.
+ * until any function that takes the stream or the parent AVFormatContext
+ * as input argument is called.
*/
-const AVIndexEntry *avformat_index_get_entry(const AVStream *st, int idx);
+const AVIndexEntry *avformat_index_get_entry(AVStream *st, int idx);
/**
* Get the AVIndexEntry corresponding to the given timestamp.
@@ -2787,10 +2787,10 @@ const AVIndexEntry *avformat_index_get_entry(const
AVStream *st, int idx);
* @return A pointer to the requested AVIndexEntry if it exists, NULL
otherwise.
*
* @note The pointer returned by this function is only guaranteed to be valid
- * until any function that could alter the stream or the AVFormatContext
- * that contains it is called.
+ * until any function that takes the stream or the parent AVFormatContext
+ * as input argument is called.
*/
-const AVIndexEntry *avformat_index_get_entry_from_timestamp(const AVStream *st,
+const AVIndexEntry *avformat_index_get_entry_from_timestamp(AVStream *st,
int64_t
wanted_timestamp,
int flags);
/**
diff --git a/libavformat/utils.c b/libavformat/utils.c
index d9971d7fd3..3ea34fa042 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2169,7 +2169,7 @@ int avformat_index_get_entries_count(const AVStream *st)
return st->internal->nb_index_entries;
}
-const AVIndexEntry *avformat_index_get_entry(const AVStream *st, int idx)
+const AVIndexEntry *avformat_index_get_entry(AVStream *st, int idx)
{
if (idx < 0 || idx >= st->internal->nb_index_entries)
return NULL;
@@ -2177,7 +2177,7 @@ const AVIndexEntry *avformat_index_get_entry(const
AVStream *st, int idx)
return &st->internal->index_entries[idx];
}
-const AVIndexEntry *avformat_index_get_entry_from_timestamp(const AVStream *st,
+const AVIndexEntry *avformat_index_get_entry_from_timestamp(AVStream *st,
int64_t
wanted_timestamp,
int flags)
{
--
2.31.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] [GSoC 2021] about proposal
You might want to check your email client as it has added a bunch of ` ` which mangles your message when viewed ___ 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] lavc/aarch64: add pred16x16 10-bit functions
here are the benchmarks https://0x1.st/kX.txt
---
libavcodec/aarch64/h264pred_init.c | 75 +++---
libavcodec/aarch64/h264pred_neon.S | 123 +
2 files changed, 168 insertions(+), 30 deletions(-)
diff --git a/libavcodec/aarch64/h264pred_init.c
b/libavcodec/aarch64/h264pred_init.c
index b144376f90..d74205c2de 100644
--- a/libavcodec/aarch64/h264pred_init.c
+++ b/libavcodec/aarch64/h264pred_init.c
@@ -45,42 +45,57 @@ void ff_pred8x8_0lt_dc_neon(uint8_t *src, ptrdiff_t stride);
void ff_pred8x8_l00_dc_neon(uint8_t *src, ptrdiff_t stride);
void ff_pred8x8_0l0_dc_neon(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_128_dc_neon_10(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_top_dc_neon_10(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_left_dc_neon_10(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_dc_neon_10(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_hor_neon_10(uint8_t *src, ptrdiff_t stride);
+void ff_pred16x16_vert_neon_10(uint8_t *src, ptrdiff_t stride);
+
static av_cold void h264_pred_init_neon(H264PredContext *h, int codec_id,
const int bit_depth,
const int chroma_format_idc)
{
-const int high_depth = bit_depth > 8;
-
-if (high_depth)
-return;
-
-if (chroma_format_idc <= 1) {
-h->pred8x8[VERT_PRED8x8 ] = ff_pred8x8_vert_neon;
-h->pred8x8[HOR_PRED8x8 ] = ff_pred8x8_hor_neon;
-if (codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
-h->pred8x8[PLANE_PRED8x8] = ff_pred8x8_plane_neon;
-h->pred8x8[DC_128_PRED8x8 ] = ff_pred8x8_128_dc_neon;
-if (codec_id != AV_CODEC_ID_RV40 && codec_id != AV_CODEC_ID_VP7 &&
-codec_id != AV_CODEC_ID_VP8) {
-h->pred8x8[DC_PRED8x8 ] = ff_pred8x8_dc_neon;
-h->pred8x8[LEFT_DC_PRED8x8] = ff_pred8x8_left_dc_neon;
-h->pred8x8[TOP_DC_PRED8x8 ] = ff_pred8x8_top_dc_neon;
-h->pred8x8[ALZHEIMER_DC_L0T_PRED8x8] = ff_pred8x8_l0t_dc_neon;
-h->pred8x8[ALZHEIMER_DC_0LT_PRED8x8] = ff_pred8x8_0lt_dc_neon;
-h->pred8x8[ALZHEIMER_DC_L00_PRED8x8] = ff_pred8x8_l00_dc_neon;
-h->pred8x8[ALZHEIMER_DC_0L0_PRED8x8] = ff_pred8x8_0l0_dc_neon;
+if (bit_depth == 8) {
+if (chroma_format_idc <= 1) {
+h->pred8x8[VERT_PRED8x8 ] = ff_pred8x8_vert_neon;
+h->pred8x8[HOR_PRED8x8 ] = ff_pred8x8_hor_neon;
+if (codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
+h->pred8x8[PLANE_PRED8x8] = ff_pred8x8_plane_neon;
+h->pred8x8[DC_128_PRED8x8 ] = ff_pred8x8_128_dc_neon;
+if (codec_id != AV_CODEC_ID_RV40 && codec_id != AV_CODEC_ID_VP7 &&
+codec_id != AV_CODEC_ID_VP8) {
+h->pred8x8[DC_PRED8x8 ] = ff_pred8x8_dc_neon;
+h->pred8x8[LEFT_DC_PRED8x8] = ff_pred8x8_left_dc_neon;
+h->pred8x8[TOP_DC_PRED8x8 ] = ff_pred8x8_top_dc_neon;
+h->pred8x8[ALZHEIMER_DC_L0T_PRED8x8] = ff_pred8x8_l0t_dc_neon;
+h->pred8x8[ALZHEIMER_DC_0LT_PRED8x8] = ff_pred8x8_0lt_dc_neon;
+h->pred8x8[ALZHEIMER_DC_L00_PRED8x8] = ff_pred8x8_l00_dc_neon;
+h->pred8x8[ALZHEIMER_DC_0L0_PRED8x8] = ff_pred8x8_0l0_dc_neon;
+}
}
-}
-h->pred16x16[DC_PRED8x8 ] = ff_pred16x16_dc_neon;
-h->pred16x16[VERT_PRED8x8 ] = ff_pred16x16_vert_neon;
-h->pred16x16[HOR_PRED8x8] = ff_pred16x16_hor_neon;
-h->pred16x16[LEFT_DC_PRED8x8] = ff_pred16x16_left_dc_neon;
-h->pred16x16[TOP_DC_PRED8x8 ] = ff_pred16x16_top_dc_neon;
-h->pred16x16[DC_128_PRED8x8 ] = ff_pred16x16_128_dc_neon;
-if (codec_id != AV_CODEC_ID_SVQ3 && codec_id != AV_CODEC_ID_RV40 &&
-codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
-h->pred16x16[PLANE_PRED8x8 ] = ff_pred16x16_plane_neon;
+h->pred16x16[DC_PRED8x8 ] = ff_pred16x16_dc_neon;
+h->pred16x16[VERT_PRED8x8 ] = ff_pred16x16_vert_neon;
+h->pred16x16[HOR_PRED8x8] = ff_pred16x16_hor_neon;
+h->pred16x16[LEFT_DC_PRED8x8] = ff_pred16x16_left_dc_neon;
+h->pred16x16[TOP_DC_PRED8x8 ] = ff_pred16x16_top_dc_neon;
+h->pred16x16[DC_128_PRED8x8 ] = ff_pred16x16_128_dc_neon;
+if (codec_id != AV_CODEC_ID_SVQ3 && codec_id != AV_CODEC_ID_RV40 &&
+codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
+h->pred16x16[PLANE_PRED8x8 ] = ff_pred16x16_plane_neon;
+}
+if (bit_depth == 10) {
+h->pred16x16[DC_PRED8x8 ] = ff_pred16x16_dc_neon_10;
+h->pred16x16[VERT_PRED8x8 ] = ff_pred16x16_vert_neon_10;
+h->pred16x16[HOR_PRED8x8] = ff_pred16x16_hor_neon_10;
+h->pred16x16[TOP_DC_PRED8x8 ] = ff_pred16x16_top_dc_neon_10;
+h->pred16x16[LEFT_DC_PRED8x8] = ff_pred16x16_left_dc_neon_10;
+
Re: [FFmpeg-devel] [PATCH] lavc/aarch64: add pred16x16 10-bit functions
Am Do., 8. Apr. 2021 um 21:10 Uhr schrieb Mikhail Nitenko :
>
> here are the benchmarks https://0x1.st/kX.txt
Instead please add the relevant lines to the commit message.
> ---
> libavcodec/aarch64/h264pred_init.c | 75 +++---
> libavcodec/aarch64/h264pred_neon.S | 123 +
> 2 files changed, 168 insertions(+), 30 deletions(-)
>
> diff --git a/libavcodec/aarch64/h264pred_init.c
> b/libavcodec/aarch64/h264pred_init.c
> index b144376f90..d74205c2de 100644
> --- a/libavcodec/aarch64/h264pred_init.c
> +++ b/libavcodec/aarch64/h264pred_init.c
> @@ -45,42 +45,57 @@ void ff_pred8x8_0lt_dc_neon(uint8_t *src, ptrdiff_t
> stride);
> void ff_pred8x8_l00_dc_neon(uint8_t *src, ptrdiff_t stride);
> void ff_pred8x8_0l0_dc_neon(uint8_t *src, ptrdiff_t stride);
>
> +void ff_pred16x16_128_dc_neon_10(uint8_t *src, ptrdiff_t stride);
> +void ff_pred16x16_top_dc_neon_10(uint8_t *src, ptrdiff_t stride);
> +void ff_pred16x16_left_dc_neon_10(uint8_t *src, ptrdiff_t stride);
> +void ff_pred16x16_dc_neon_10(uint8_t *src, ptrdiff_t stride);
> +void ff_pred16x16_hor_neon_10(uint8_t *src, ptrdiff_t stride);
> +void ff_pred16x16_vert_neon_10(uint8_t *src, ptrdiff_t stride);
> +
> static av_cold void h264_pred_init_neon(H264PredContext *h, int codec_id,
> const int bit_depth,
> const int chroma_format_idc)
> {
> -const int high_depth = bit_depth > 8;
> -
> -if (high_depth)
> -return;
> -
> -if (chroma_format_idc <= 1) {
> -h->pred8x8[VERT_PRED8x8 ] = ff_pred8x8_vert_neon;
> -h->pred8x8[HOR_PRED8x8 ] = ff_pred8x8_hor_neon;
> -if (codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
> -h->pred8x8[PLANE_PRED8x8] = ff_pred8x8_plane_neon;
> -h->pred8x8[DC_128_PRED8x8 ] = ff_pred8x8_128_dc_neon;
> -if (codec_id != AV_CODEC_ID_RV40 && codec_id != AV_CODEC_ID_VP7 &&
> -codec_id != AV_CODEC_ID_VP8) {
> -h->pred8x8[DC_PRED8x8 ] = ff_pred8x8_dc_neon;
> -h->pred8x8[LEFT_DC_PRED8x8] = ff_pred8x8_left_dc_neon;
> -h->pred8x8[TOP_DC_PRED8x8 ] = ff_pred8x8_top_dc_neon;
> -h->pred8x8[ALZHEIMER_DC_L0T_PRED8x8] = ff_pred8x8_l0t_dc_neon;
> -h->pred8x8[ALZHEIMER_DC_0LT_PRED8x8] = ff_pred8x8_0lt_dc_neon;
> -h->pred8x8[ALZHEIMER_DC_L00_PRED8x8] = ff_pred8x8_l00_dc_neon;
> -h->pred8x8[ALZHEIMER_DC_0L0_PRED8x8] = ff_pred8x8_0l0_dc_neon;
> +if (bit_depth == 8) {
> +if (chroma_format_idc <= 1) {
> +h->pred8x8[VERT_PRED8x8 ] = ff_pred8x8_vert_neon;
> +h->pred8x8[HOR_PRED8x8 ] = ff_pred8x8_hor_neon;
> +if (codec_id != AV_CODEC_ID_VP7 && codec_id != AV_CODEC_ID_VP8)
> +h->pred8x8[PLANE_PRED8x8] = ff_pred8x8_plane_neon;
> +h->pred8x8[DC_128_PRED8x8 ] = ff_pred8x8_128_dc_neon;
> +if (codec_id != AV_CODEC_ID_RV40 && codec_id != AV_CODEC_ID_VP7
> &&
> +codec_id != AV_CODEC_ID_VP8) {
> +h->pred8x8[DC_PRED8x8 ] = ff_pred8x8_dc_neon;
> +h->pred8x8[LEFT_DC_PRED8x8] = ff_pred8x8_left_dc_neon;
> +h->pred8x8[TOP_DC_PRED8x8 ] = ff_pred8x8_top_dc_neon;
> +h->pred8x8[ALZHEIMER_DC_L0T_PRED8x8] =
> ff_pred8x8_l0t_dc_neon;
> +h->pred8x8[ALZHEIMER_DC_0LT_PRED8x8] =
> ff_pred8x8_0lt_dc_neon;
> +h->pred8x8[ALZHEIMER_DC_L00_PRED8x8] =
> ff_pred8x8_l00_dc_neon;
> +h->pred8x8[ALZHEIMER_DC_0L0_PRED8x8] =
> ff_pred8x8_0l0_dc_neon;
Please do not re-indent these lines in the patch that adds the new functions,
feel free to send a separate patch for the re-indentation.
Carl Eugen
___
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 1/7] avcodec/rv34: Move dsp init code to rv30/rv40
It avoids both runtime and compile-time checks.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/rv30.c | 1 +
libavcodec/rv34.c | 9 -
libavcodec/rv40.c | 1 +
3 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/libavcodec/rv30.c b/libavcodec/rv30.c
index 36cd5345fd..e2b75fbec0 100644
--- a/libavcodec/rv30.c
+++ b/libavcodec/rv30.c
@@ -285,6 +285,7 @@ static av_cold int rv30_decode_init(AVCodecContext *avctx)
r->loop_filter= rv30_loop_filter;
r->luma_dc_quant_i = rv30_luma_dc_quant;
r->luma_dc_quant_p = rv30_luma_dc_quant;
+ff_rv30dsp_init(&r->rdsp);
return 0;
}
diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index 99e580a09a..e68072de06 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -1503,15 +1503,6 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
ff_h264_pred_init(&r->h, AV_CODEC_ID_RV40, 8, 1);
-#if CONFIG_RV30_DECODER
-if (avctx->codec_id == AV_CODEC_ID_RV30)
-ff_rv30dsp_init(&r->rdsp);
-#endif
-#if CONFIG_RV40_DECODER
-if (avctx->codec_id == AV_CODEC_ID_RV40)
-ff_rv40dsp_init(&r->rdsp);
-#endif
-
if ((ret = rv34_decoder_alloc(r)) < 0) {
ff_mpv_common_end(&r->s);
return ret;
diff --git a/libavcodec/rv40.c b/libavcodec/rv40.c
index e0903226b9..8532a872d9 100644
--- a/libavcodec/rv40.c
+++ b/libavcodec/rv40.c
@@ -567,6 +567,7 @@ static av_cold int rv40_decode_init(AVCodecContext *avctx)
r->loop_filter= rv40_loop_filter;
r->luma_dc_quant_i = rv40_luma_dc_quant[0];
r->luma_dc_quant_p = rv40_luma_dc_quant[1];
+ff_rv40dsp_init(&r->rdsp);
return 0;
}
--
2.27.0
___
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/7] avcodec/h263dec, mpeg12dec: Remove redundant writes
ff_mpv_decode_init() already sets MpegEncContext.codec_id.
Signed-off-by: Andreas Rheinhardt
---
Supersedes http://ffmpeg.org/pipermail/ffmpeg-devel/2021-April/278741.html
libavcodec/h263dec.c | 1 -
libavcodec/mpeg12dec.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index e8b4d83e6e..192cc487da 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -130,7 +130,6 @@ av_cold int ff_h263_decode_init(AVCodecContext *avctx)
avctx->codec->id);
return AVERROR(ENOSYS);
}
-s->codec_id= avctx->codec->id;
if (avctx->codec_tag == AV_RL32("L263") || avctx->codec_tag ==
AV_RL32("S263"))
if (avctx->extradata_size == 56 && avctx->extradata[0] == 1)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 94221da2c1..2d2b7517ad 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1067,7 +1067,6 @@ static av_cold int mpeg_decode_init(AVCodecContext *avctx)
s->mpeg_enc_ctx_allocated = 0;
s->mpeg_enc_ctx.picture_number = 0;
s->repeat_field= 0;
-s->mpeg_enc_ctx.codec_id = avctx->codec->id;
avctx->color_range = AVCOL_RANGE_MPEG;
return 0;
}
--
2.27.0
___
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 4/7] avcodec/vc1: Don't pretend ff_vc1_init_common() can fail
From: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt
---
libavcodec/mss2.c | 3 +--
libavcodec/vc1.c| 4 +---
libavcodec/vc1.h| 2 +-
libavcodec/vc1_parser.c | 3 ++-
libavcodec/vc1dec.c | 3 +--
5 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c
index 9434a740a7..3e3205ae92 100644
--- a/libavcodec/mss2.c
+++ b/libavcodec/mss2.c
@@ -751,8 +751,7 @@ static av_cold int wmv9_init(AVCodecContext *avctx)
v->s.avctx= avctx;
-if ((ret = ff_vc1_init_common(v)) < 0)
-return ret;
+ff_vc1_init_common(v);
ff_vc1dsp_init(&v->vc1dsp);
v->profile = PROFILE_MAIN;
diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
index cd9975d8cf..5d854b35d2 100644
--- a/libavcodec/vc1.c
+++ b/libavcodec/vc1.c
@@ -1695,7 +1695,7 @@ static av_cold void vc1_init_static(void)
* @param v The VC1Context to initialize
* @return Status
*/
-av_cold int ff_vc1_init_common(VC1Context *v)
+av_cold void ff_vc1_init_common(VC1Context *v)
{
static AVOnce init_static_once = AV_ONCE_INIT;
@@ -1709,6 +1709,4 @@ av_cold int ff_vc1_init_common(VC1Context *v)
/* VLC tables */
ff_thread_once(&init_static_once, vc1_init_static);
-
-return 0;
}
diff --git a/libavcodec/vc1.h b/libavcodec/vc1.h
index 4559a06cb6..3e5368b891 100644
--- a/libavcodec/vc1.h
+++ b/libavcodec/vc1.h
@@ -413,7 +413,7 @@ int ff_vc1_decode_entry_point(AVCodecContext *avctx,
VC1Context *v, GetBitContex
int ff_vc1_parse_frame_header(VC1Context *v, GetBitContext *gb);
int ff_vc1_parse_frame_header_adv(VC1Context *v, GetBitContext *gb);
-int ff_vc1_init_common(VC1Context *v);
+void ff_vc1_init_common(VC1Context *v);
int ff_vc1_decode_init_alloc_tables(VC1Context *v);
void ff_vc1_init_transposed_scantables(VC1Context *v);
diff --git a/libavcodec/vc1_parser.c b/libavcodec/vc1_parser.c
index 493ffde611..1a9d3c0140 100644
--- a/libavcodec/vc1_parser.c
+++ b/libavcodec/vc1_parser.c
@@ -283,7 +283,8 @@ static av_cold int vc1_parse_init(AVCodecParserContext *s)
vpc->bytes_to_skip = 0;
vpc->unesc_index = 0;
vpc->search_state = NO_MATCH;
-return ff_vc1_init_common(&vpc->v);
+ff_vc1_init_common(&vpc->v);
+return 0;
}
AVCodecParser ff_vc1_parser = {
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index ea93e11588..a08f7fe847 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -434,8 +434,7 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
return AVERROR_INVALIDDATA;
v->s.avctx = avctx;
-if ((ret = ff_vc1_init_common(v)) < 0)
-return ret;
+ff_vc1_init_common(v);
if (avctx->codec_id == AV_CODEC_ID_WMV3 || avctx->codec_id ==
AV_CODEC_ID_WMV3IMAGE) {
int count = 0;
--
2.27.0
___
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 5/7] avcodec/mss2: Remove redundant initialization of vc1 dsp
ff_vc1_init_common() already does it. Signed-off-by: Andreas Rheinhardt --- libavcodec/mss2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c index 3e3205ae92..a27fae7559 100644 --- a/libavcodec/mss2.c +++ b/libavcodec/mss2.c @@ -752,7 +752,6 @@ static av_cold int wmv9_init(AVCodecContext *avctx) v->s.avctx= avctx; ff_vc1_init_common(v); -ff_vc1dsp_init(&v->vc1dsp); v->profile = PROFILE_MAIN; -- 2.27.0 ___ 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 6/7] avcodec/vc1: Remove unused hrd fields
Unused since be3492ec7eb2dbb0748c123af911a06c125c90db.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/vc1.c| 2 --
libavcodec/vc1.h| 2 --
libavcodec/vc1dec.c | 2 --
3 files changed, 6 deletions(-)
diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
index 5d854b35d2..b7140c089c 100644
--- a/libavcodec/vc1.c
+++ b/libavcodec/vc1.c
@@ -1699,8 +1699,6 @@ av_cold void ff_vc1_init_common(VC1Context *v)
{
static AVOnce init_static_once = AV_ONCE_INIT;
-v->hrd_rate = v->hrd_buffer = NULL;
-
/* defaults */
v->pq = -1;
v->mvrange = 0; /* 7.1.1.18, p80 */
diff --git a/libavcodec/vc1.h b/libavcodec/vc1.h
index 3e5368b891..9b25f0872f 100644
--- a/libavcodec/vc1.h
+++ b/libavcodec/vc1.h
@@ -322,8 +322,6 @@ typedef struct VC1Context{
uint8_t* over_flags_plane; ///< Overflags bitplane
int overflg_is_raw;
uint8_t condover;
-uint16_t *hrd_rate, *hrd_buffer;
-uint8_t *hrd_fullness;
uint8_t range_mapy_flag;
uint8_t range_mapuv_flag;
uint8_t range_mapy;
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index a08f7fe847..8f112bf9b2 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -593,8 +593,6 @@ av_cold int ff_vc1_decode_end(AVCodecContext *avctx)
for (i = 0; i < 4; i++)
av_freep(&v->sr_rows[i >> 1][i & 1]);
-av_freep(&v->hrd_rate);
-av_freep(&v->hrd_buffer);
ff_mpv_common_end(&v->s);
av_freep(&v->mv_type_mb_plane);
av_freep(&v->direct_mb_plane);
--
2.27.0
___
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 7/7] avcodec/msmpeg4dec: Avoid duplication of VLC init code
Signed-off-by: Andreas Rheinhardt
---
All the users of these VLCs and of many more only use VLC.table and
nothing else (the number of bits is hardcoded). Does someone object
to keeping just these pointers in such circumstances?
libavcodec/msmpeg4dec.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c
index 63f9faa1bb..8fcd5b94cd 100644
--- a/libavcodec/msmpeg4dec.c
+++ b/libavcodec/msmpeg4dec.c
@@ -356,18 +356,16 @@ av_cold int ff_msmpeg4_decode_init(AVCodecContext *avctx)
&ff_v2_mb_type[0][1], 2, 1,
&ff_v2_mb_type[0][0], 2, 1, 128);
-INIT_VLC_STATIC(&ff_mb_non_intra_vlc[0], MB_NON_INTRA_VLC_BITS, 128,
- &ff_wmv2_inter_table[0][0][1], 8, 4,
- &ff_wmv2_inter_table[0][0][0], 8, 4, 1636);
-INIT_VLC_STATIC(&ff_mb_non_intra_vlc[1], MB_NON_INTRA_VLC_BITS, 128,
- &ff_wmv2_inter_table[1][0][1], 8, 4,
- &ff_wmv2_inter_table[1][0][0], 8, 4, 2648);
-INIT_VLC_STATIC(&ff_mb_non_intra_vlc[2], MB_NON_INTRA_VLC_BITS, 128,
- &ff_wmv2_inter_table[2][0][1], 8, 4,
- &ff_wmv2_inter_table[2][0][0], 8, 4, 1532);
-INIT_VLC_STATIC(&ff_mb_non_intra_vlc[3], MB_NON_INTRA_VLC_BITS, 128,
- &ff_wmv2_inter_table[3][0][1], 8, 4,
- &ff_wmv2_inter_table[3][0][0], 8, 4, 2488);
+for (unsigned i = 0, offset = 0; i < 4; i++) {
+static VLC_TYPE vlc_buf[1636 + 2648 + 1532 + 2488][2];
+ff_mb_non_intra_vlc[i].table = &vlc_buf[offset];
+ff_mb_non_intra_vlc[i].table_allocated = FF_ARRAY_ELEMS(vlc_buf) -
offset;
+init_vlc(&ff_mb_non_intra_vlc[i], MB_NON_INTRA_VLC_BITS, 128,
+ &ff_wmv2_inter_table[i][0][1], 8, 4,
+ &ff_wmv2_inter_table[i][0][0], 8, 4,
+ INIT_VLC_STATIC_OVERLONG);
+offset += ff_mb_non_intra_vlc[i].table_size;
+}
INIT_VLC_STATIC(&ff_msmp4_mb_i_vlc, MB_INTRA_VLC_BITS, 64,
&ff_msmp4_mb_i_table[0][1], 4, 2,
--
2.27.0
___
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 3/7] avcodec/msmpeg4enc: Remove dead code for inexistent VC-1 encoder
Signed-off-by: Andreas Rheinhardt
---
libavcodec/msmpeg4enc.c | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/libavcodec/msmpeg4enc.c b/libavcodec/msmpeg4enc.c
index 59a18532c2..7147015042 100644
--- a/libavcodec/msmpeg4enc.c
+++ b/libavcodec/msmpeg4enc.c
@@ -492,8 +492,7 @@ void ff_msmpeg4_encode_mb(MpegEncContext * s,
static void msmpeg4_encode_dc(MpegEncContext * s, int level, int n, int
*dir_ptr)
{
int sign, code;
-int pred, av_uninit(extquant);
-int extrabits = 0;
+int pred;
int16_t *dc_val;
pred = ff_msmpeg4_pred_dc(s, n, &dc_val, dir_ptr);
@@ -527,15 +526,6 @@ static void msmpeg4_encode_dc(MpegEncContext * s, int
level, int n, int *dir_ptr
code = level;
if (code > DC_MAX)
code = DC_MAX;
-else if( s->msmpeg4_version>=6 ) {
-if( s->qscale == 1 ) {
-extquant = (level + 3) & 0x3;
-code = ((level+3)>>2);
-} else if( s->qscale == 2 ) {
-extquant = (level + 1) & 0x1;
-code = ((level+1)>>1);
-}
-}
if (s->dc_table_index == 0) {
if (n < 4) {
@@ -551,13 +541,8 @@ static void msmpeg4_encode_dc(MpegEncContext * s, int
level, int n, int *dir_ptr
}
}
-if(s->msmpeg4_version>=6 && s->qscale<=2)
-extrabits = 3 - s->qscale;
-
if (code == DC_MAX)
-put_bits(&s->pb, 8 + extrabits, level);
-else if(extrabits > 0)//== VC1 && s->qscale<=2
-put_bits(&s->pb, extrabits, extquant);
+put_bits(&s->pb, 8, level);
if (level != 0) {
put_bits(&s->pb, 1, sign);
@@ -596,7 +581,7 @@ void ff_msmpeg4_encode_block(MpegEncContext * s, int16_t *
block, int n)
}
/* recalculate block_last_index for M$ wmv1 */
-if(s->msmpeg4_version>=4 && s->msmpeg4_version<6 &&
s->block_last_index[n]>0){
+if (s->msmpeg4_version >= 4 && s->block_last_index[n] > 0) {
for(last_index=63; last_index>=0; last_index--){
if(block[scantable[last_index]]) break;
}
@@ -656,7 +641,7 @@ void ff_msmpeg4_encode_block(MpegEncContext * s, int16_t *
block, int n)
s->esc3_run_length= 6;
//ESCLVLSZ + ESCRUNSZ
if(s->qscale<8)
-put_bits(&s->pb, 6 +
(s->msmpeg4_version>=6), 3);
+put_bits(&s->pb, 6, 3);
else
put_bits(&s->pb, 8, 3);
}
--
2.27.0
___
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] lavc/pngdec: always create a copy for APNG_DISPOSE_OP_BACKGROUND
On Thu, Apr 08, 2021 at 05:34:30PM +0200, Michael Niedermayer wrote: > On Thu, Apr 08, 2021 at 10:46:54AM +0200, Anton Khirnov wrote: > > Calling av_frame_make_writable() from decoders is tricky, especially > > when frame threading is used. It is much simpler and safer to just make > > a private copy of the frame. > > This is not expected to have a major performance impact, since > > APNG_DISPOSE_OP_BACKGROUND is not used often and > > av_frame_make_writable() would typically make a copy anyway. > > > > Found-by: James Almer > > --- > > libavcodec/pngdec.c | 18 +++--- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > LGTM i will apply and backport this if it has not been applied when i make the 4.4 release thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. 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/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
On Wed, Jan 27, 2021 at 3:40 PM wrote:
> From: Levi Dooley
>
> There was an assumption in the existing code that indentation would not
> occur more than once on the same row.
> This was a bad assumption. There are examples of 608 streams which call
> handle_pac multiple times on the same row with different indentation.
> As the code was before this change, the new indentation would overwrite
> existing text with spaces.
> These changes make indentation skip over columns instead. Text gets
> cleared with spaces on handle_edm.
> Instead of relying on the null character, trailing spaces are trimmed off
> the end of a row.
> This is necessary so that a null character doesn't end up between two
> words.
>
> Signed-off-by: Levi Dooley
>
Tried this out and it looks good to me. I can confirm it fixes subtle
issues where content was previously being overwritten.
Aman
> ---
> libavcodec/ccaption_dec.c | 56 ---
> tests/ref/fate/sub-scc| 2 +-
> 2 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index a208e19b95..525e010897 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -352,6 +352,17 @@ static void write_char(CCaptionSubContext *ctx,
> struct Screen *screen, char ch)
> }
> }
>
> +/**
> + * Increment the cursor_column by 1, and ensure that there are no null
> characters left behind in the row.
> + */
> +static void skip_char(CCaptionSubContext *ctx, struct Screen *screen)
> +{
> +if (!screen->characters[ctx->cursor_row][ctx->cursor_column])
> +write_char(ctx, screen, ' ');
> +else
> +ctx->cursor_column++;
> +}
> +
> /**
> * This function after validating parity bit, also remove it from data
> pair.
> * The first byte doesn't pass parity, we replace it with a solid blank
> @@ -459,6 +470,7 @@ static int capture_screen(CCaptionSubContext *ctx)
> if (CHECK_FLAG(screen->row_used, i)) {
> const char *row = screen->characters[i];
> const char *charset = screen->charsets[i];
> +
> j = 0;
> while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN)
> j++;
> @@ -476,13 +488,19 @@ static int capture_screen(CCaptionSubContext *ctx)
> const char *color = screen->colors[i];
> const char *charset = screen->charsets[i];
> const char *override;
> -int x, y, seen_char = 0;
> +int x, y, row_end, seen_char = 0;
> j = 0;
>
> /* skip leading space */
> while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN &&
> j < tab)
> j++;
>
> +/* skip trailing space */
> +row_end = SCREEN_COLUMNS-1;
> +while (row_end >= 0 && row[row_end] == ' ' &&
> charset[row_end] == CCSET_BASIC_AMERICAN) {
> +row_end--;
> +}
> +
> x = ASS_DEFAULT_PLAYRESX * (0.1 + 0.0250 * j);
> y = ASS_DEFAULT_PLAYRESY * (0.1 + 0.0533 * i);
> av_bprintf(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y);
> @@ -490,7 +508,7 @@ static int capture_screen(CCaptionSubContext *ctx)
> for (; j < SCREEN_COLUMNS; j++) {
> const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag
> = "";
>
> -if (row[j] == 0)
> +if (j > row_end || row[j] == 0)
> break;
>
> if (prev_font != font[j]) {
> @@ -624,7 +642,8 @@ static void handle_textattr(CCaptionSubContext *ctx,
> uint8_t hi, uint8_t lo)
> ctx->cursor_font = pac2_attribs[i][1];
>
> SET_FLAG(screen->row_used, ctx->cursor_row);
> -write_char(ctx, screen, ' ');
> +
> +skip_char(ctx, screen);
> }
>
> static void handle_pac(CCaptionSubContext *ctx, uint8_t hi, uint8_t lo)
> @@ -644,13 +663,13 @@ static void handle_pac(CCaptionSubContext *ctx,
> uint8_t hi, uint8_t lo)
> lo &= 0x1f;
>
> ctx->cursor_row = row_map[index] - 1;
> -ctx->cursor_color = pac2_attribs[lo][0];
> +ctx->cursor_color = pac2_attribs[lo][0];
> ctx->cursor_font = pac2_attribs[lo][1];
> ctx->cursor_charset = CCSET_BASIC_AMERICAN;
> ctx->cursor_column = 0;
> indent = pac2_attribs[lo][2];
> for (i = 0; i < indent; i++) {
> -write_char(ctx, screen, ' ');
> +skip_char(ctx, screen);
> }
> }
>
> @@ -667,6 +686,14 @@ static int handle_edm(CCaptionSubContext *ctx)
> screen->row_used = 0;
> ctx->bg_color = CCCOL_BLACK;
>
> +for (int i = 0; i < SCREEN_ROWS+1; ++i) {
> +memset(screen->characters[i], ' ',
> SCREEN_COLUMNS);
> +memset(screen->colors[i], CCCOL_WHITE,
> SCREEN_COLUMNS);
> +memset(screen->bgs[i],CCCOL_BLACK,
> SCREEN_COLUMNS);
> +memset(screen->charsets[i], CCSET_BASIC_AMERICAN,
> SCREEN_COLUMNS);
> +memset(screen->fonts[i], CCFONT_REGULAR,
>
Re: [FFmpeg-devel] [PATCH] lavc/aarch64: add pred16x16 10-bit functions
On Thu, 8 Apr 2021, Mikhail Nitenko wrote:
here are the benchmarks https://0x1.st/kX.txt
---
libavcodec/aarch64/h264pred_init.c | 75 +++---
libavcodec/aarch64/h264pred_neon.S | 123 +
2 files changed, 168 insertions(+), 30 deletions(-)
av_cold void ff_h264_pred_init_aarch64(H264PredContext *h, int codec_id,
diff --git a/libavcodec/aarch64/h264pred_neon.S
b/libavcodec/aarch64/h264pred_neon.S
index 213b40b3e7..633b401d59 100644
--- a/libavcodec/aarch64/h264pred_neon.S
+++ b/libavcodec/aarch64/h264pred_neon.S
@@ -359,3 +359,126 @@ function ff_pred8x8_0l0_dc_neon, export=1
dup v1.8b, v1.b[0]
b .L_pred8x8_dc_end
endfunc
+
+.macro ldcol.16 rd, rs, rt, n=4, hi=0
+.if \n >= 4 || \hi == 0
+ld1 {\rd\().h}[0], [\rs], \rt
+ld1 {\rd\().h}[1], [\rs], \rt
+.endif
+.if \n >= 4 || \hi == 1
+ld1 {\rd\().h}[2], [\rs], \rt
+ld1 {\rd\().h}[3], [\rs], \rt
+.endif
+.if \n == 8
+ld1 {\rd\().h}[4], [\rs], \rt
+ld1 {\rd\().h}[5], [\rs], \rt
+ld1 {\rd\().h}[6], [\rs], \rt
+ld1 {\rd\().h}[7], [\rs], \rt
+.endif
+.endm
I believe this could be a bit faster by using two alternating registers
that are incremented - but as the existing code doesn't do that, it's not
necessary.
+
+function ff_pred16x16_128_dc_neon_10, export=1
+moviv0.8h, #2, lsl #8 // 512, 1 << (bit_depth - 1)
+
+b .L_pred16x16_dc_10_end
+endfunc
+
+function ff_pred16x16_top_dc_neon_10, export=1
+sub x2, x0, x1
+
+ld1 {v0.8h}, [x2], #16
+ld1 {v1.8h}, [x2]
This can be one single instruction, ld1 {v0.8h, v1.8h}, [x2]
+uaddlv s0, v0.8h
+uaddlv s1, v1.8h
When adding up 8 elements that are 10 bit, they still fit in 16 bit (it
only requires 13 bit), so you don't need uaddlv here, addv would be
better. And when adding the two results, it still fits in 16 bit (then
it'd use 14 bits).
+
+add v0.2s, v0.2s, v1.2s
+
+rshrn v0.4h, v0.4s, #4
+dup v0.8h, v0.h[0]
+b .L_pred16x16_dc_10_end
+endfunc
+
+function ff_pred16x16_left_dc_neon_10, export=1
+sub x2, x0, #2 // access to the "left" column
+ldcol.16v0, x2, x1, 8
+ldcol.16v1, x2, x1, 8 // load "left" column
+
+uaddlv s0, v0.8h
+uaddlv s1, v1.8h
Same thing here, addv+addv should be enough
+
+add v0.2s, v0.2s, v1.2s
+
+rshrn v0.4h, v0.4s, #4
+dup v0.8h, v0.h[0]
+b .L_pred16x16_dc_10_end
+endfunc
+
+function ff_pred16x16_dc_neon_10, export=1
+sub x2, x0, x1 // access to the "top" row
+sub x3, x0, #2 // access to the "left" column
+
+ld1 {v0.8h}, [x2], #16
+ld1 {v1.8h}, [x2]
One single ld1 {v0.8h, v1.8h}
+ldcol.16v2, x3, x1, 8
+ldcol.16v3, x3, x1, 8 // load pixels in "top" col and
"left" row
+
+uaddlv s0, v0.8h
+uaddlv s1, v1.8h
+uaddlv s2, v2.8h // sum all pixels in the "top" row and
"left" col
+uaddlv s3, v3.8h // (sum stays in v0-v3 registers)
addv
+
+add v0.2s, v0.2s, v1.2s
+add v0.2s, v0.2s, v2.2s
+add v0.2s, v0.2s, v3.2s // sum registers v0-v3
+
+rshrn v0.4h, v0.4s, #5 // right shift vector
+dup v0.8h, v0.h[0] // fill vector with 0th value (dcsplat)
These comments are kinda pointless here
+.L_pred16x16_dc_10_end:
+sub x1, x1, #16
+mov w3, #8
+6: st1 {v0.8h}, [x0], #16
+st1 {v0.8h}, [x0], x1
This can be one single "st1 {v0.8h, v1.8h}, [x0], x1" if you make sure
that v1 contains the same
+st1 {v0.8h}, [x0], #16
+st1 {v0.8h}, [x0], x1
+
+subsw3, w3, #1
+b.ne6b
+ret
+endfunc
+
+function ff_pred16x16_hor_neon_10, export=1
+sub x2, x0, #2
+sub x3, x1, #16
+
+mov w4, #16
+1: ld1r{v0.8h}, [x2], x1
+st1 {v0.8h}, [x0], #16
+st1 {v0.8h}, [x0], x3
This might be ok here, but also do check if copying the value to v1 and
doing one single "st1 {v0.8h, v1.8h}" is faster.
+
+subsw4, w4, #1
+b.ne1b
+ret
+endfunc
+
+function ff_pred16x16_vert_neon_10, export=1
+sub x2, x0, x1
+add
[FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: Remove remnant of inline-timing subtitle packets
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskaenc.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 609a588f78..a4ddf1fa6a 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2187,7 +2187,7 @@ static int mkv_write_vtt_blocks(AVFormatContext *s,
AVIOContext *pb, const AVPac
put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, pkt->duration);
end_ebml_master(pb, blockgroup);
-return pkt->duration;
+return 0;
}
static int mkv_end_cluster(AVFormatContext *s)
@@ -2358,9 +2358,9 @@ static int mkv_write_packet_internal(AVFormatContext *s,
const AVPacket *pkt)
}
} else {
if (par->codec_id == AV_CODEC_ID_WEBVTT) {
-duration = mkv_write_vtt_blocks(s, pb, pkt);
-if (duration < 0)
-return duration;
+ret = mkv_write_vtt_blocks(s, pb, pkt);
+if (ret < 0)
+return ret;
} else {
ebml_master blockgroup = start_ebml_master(pb,
MATROSKA_ID_BLOCKGROUP,
mkv_blockgroup_size(pkt->size,
--
2.27.0
___
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/matroskaenc: Put subtitles without duration into SimpleBlocks
The value zero for AVPacket.duration means that the duration is unknown,
which in practice means "play this subtitle until overridden by the next
subtitle". Yet for Matroska a BlockGroup with duration zero means
that the subtitle really has a duration zero. "Display until overridden"
is achieved by not setting a duration on the container level at all and
this is achieved by using a SimpleBlock or a BlockGroup without
duration. This commit implements this.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskaenc.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index a4ddf1fa6a..725e44c962 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -585,7 +585,7 @@ static int mkv_assemble_cues(AVStream **streams,
AVIOContext *dyn_cp,
put_ebml_uint(cuepoint, MATROSKA_ID_CUETRACK ,
tracks[idx].track_num);
put_ebml_uint(cuepoint, MATROSKA_ID_CUECLUSTERPOSITION ,
entry->cluster_pos);
put_ebml_uint(cuepoint, MATROSKA_ID_CUERELATIVEPOSITION,
entry->relative_pos);
-if (entry->duration != -1)
+if (entry->duration > 0)
put_ebml_uint(cuepoint, MATROSKA_ID_CUEDURATION,
entry->duration);
end_ebml_master(cuepoint, track_positions);
} while (++entry < end && entry->pts == pts);
@@ -2307,7 +2307,7 @@ static int mkv_write_packet_internal(AVFormatContext *s,
const AVPacket *pkt)
AVCodecParameters *par = s->streams[pkt->stream_index]->codecpar;
mkv_track *track= &mkv->tracks[pkt->stream_index];
int keyframe= !!(pkt->flags & AV_PKT_FLAG_KEY);
-int duration= pkt->duration;
+int64_t duration= pkt->duration;
int ret;
int64_t ts = track->write_dts ? pkt->dts : pkt->pts;
int64_t relative_packet_pos;
@@ -2344,14 +2344,17 @@ static int mkv_write_packet_internal(AVFormatContext
*s, const AVPacket *pkt)
relative_packet_pos = avio_tell(pb);
-if (par->codec_type != AVMEDIA_TYPE_SUBTITLE) {
+if (par->codec_type != AVMEDIA_TYPE_SUBTITLE ||
+(par->codec_id != AV_CODEC_ID_WEBVTT && duration <= 0)) {
ret = mkv_write_block(s, pb, MATROSKA_ID_SIMPLEBLOCK, pkt, keyframe);
if (ret < 0)
return ret;
if (keyframe && IS_SEEKABLE(s->pb, mkv) &&
-(par->codec_type == AVMEDIA_TYPE_VIDEO || !mkv->have_video &&
!track->has_cue)) {
+(par->codec_type == AVMEDIA_TYPE_VIDEO||
+ par->codec_type == AVMEDIA_TYPE_SUBTITLE ||
+ !mkv->have_video && !track->has_cue)) {
ret = mkv_add_cuepoint(mkv, pkt->stream_index, ts,
- mkv->cluster_pos, relative_packet_pos, -1);
+ mkv->cluster_pos, relative_packet_pos, 0);
if (ret < 0)
return ret;
track->has_cue = 1;
--
2.27.0
___
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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
> -Original Message-
> From: ffmpeg-devel On Behalf Of Lynne
> Sent: 2021年4月9日 0:57
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
>
First of all, thanks for the quick replies, I see, all the discussions/comments
are to
make this patch better, thank you.
> >> >
> >> >> >> > +
> >> >> >> > +typedef struct AVBoundingBoxHeader {
> >> >> >> > +/**
> >> >> >> > + * Information about how the bounding box is generated.
> >> >> >> > + * for example, the DNN model name.
> >> >> >> > + */
> >> >> >> > +char source[128];
> >> >> >> > +
> >> >> >> > +/**
> >> >> >> > + * The size of frame when it is detected.
> >> >> >> > + */
> >> >> >> > +int frame_width;
> >> >> >> > +int frame_height;
> >> >> >> >
> >> >> >>
> >> >> >> Why? This side data is attached to AVFrames only, where we
> >> >> >> already have width and height.
> >> >> >>
> >> >> >
> >> >> > The detection result will be used by other filters, for example,
> >> >> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
> >> >> >
> >> >> > The filter dnn_detect detects all the objects (cat, dog, person ...)
> >> >> > in a
> >> >> > frame, while dnn_classify classifies one detected object (for example,
> >> person)
> >> >> > for its attribute (for example, emotion, etc.)
> >> >> >
> >> >> > The filter dnn_classify have to check if the frame size is changed
> >> >> > after
> >> >> > it is detected, to handle the below filter chain:
> >> >> > dnn_detect -> scale -> dnn_classify
> >> >> >
> >> >>
> >> >> This doesn't look good. Why is dnn_classify needing to know
> >> >> the original frame size at all?
> >> >>
> >> >
> >> > For example, the original size of the frame is 100*100, and dnn_detect
> >> > detects a face at place (10, 10) -> (30, 40), such data will be saved in
> >> > AVBoundingBox.top/left/right/bottom.
> >> >
> >> > Then, the frame is scaled into 50*50.
> >> >
> >> > Then, dnn_classify is used to analyze the emotion of the face, it needs
> >> > to
> >> > know the frame size (100*100) when it is detected, otherwise, it does not
> >> > work with just (10,10), (30,40) and 50*50.
> >> >
> >>
> >> Why can't the scale filter also rescale this side data as well?
> >>
> >
> > I'm afraid that we could not make sure all such filters (including filters
> > in the
> > future) to do the rescale. And in the previous discussion, I got to know
> > that
> > 'many other existing side-data types are invalidated by scaling'.
> >
> > So, we need frame_width and frame_height here.
> >
>
> No, you don't. You just need to make sure filters which change resolution
> or do cropping also change the side data parameters.
> It's called maintainership. As-is, this won't even work with cropping,
> only with basic aspect ratio preserving scaling.
> For the lack of a better term, this is a hack.
As discussed in previous email, for the frame size change case, dnn_classify
(and other filters which use the detection result, for example drawbox) can
just output a warning message to tell user what happens, and don't do the
classification, otherwise, it will give a wrong/weird result which makes the
user confused.
>
> I would accept just specifying that if the frame dimensions are
> altered in any way, the side-data is no longer valid and it's up
> to users to figure that out by out of bound coordinates.
> This is what we currently do with video_enc_params.
frame_width/frame_height is not perfect (for the cases such as: scale down
+ crop + scale up to the same size), but it provides more info than the checking
of 'out of bound coordinates'. There are many other possible issues when the
coordinates are within the frame.
If we think we'd better not let user get more info from the warning message,
I'm ok to remove them.
I'll remove them if there's another comment supporting the removal, and
there's no objection.
>
>
> >> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> >> >> > index a5ed91b20a..41e22de02a 100644
> >> >> >> > --- a/libavutil/frame.h
> >> >> >> > +++ b/libavutil/frame.h
> >> >> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
> >> >> >> > * Must be present for every frame which should have film grain
> >> applied.
> >> >> >> > */
> >> >> >> > AV_FRAME_DATA_FILM_GRAIN_PARAMS,
> >> >> >> > +
> >> >> >> > +/**
> >> >> >> > + * Bounding boxes for object detection and classification, the
> >> data is
> >> >> a
> >> >> >> AVBoundingBoxHeader
> >> >> >> > + * followed with an array of AVBoudingBox, and
> >> >> >> AVBoundingBoxHeader.nb_bboxes is the number
> >> >> >> > + * of array element.
> >> >> >> > + */
> >> >> >> > +AV_FRAME_DATA_BOUNDING_BOXES,
> >> >> >> > };
> >> >> >> >
> >> >> >>
> >> >> >> Finally, why call it a Bounding Box? It's not descriptive at all.
> >> >> >> How about "Object Classification"? It makes much more sense, it's
> >> >> >> exac
