Re: [FFmpeg-devel] [PATCH] avcodec: add a mention about get_encode_buffer in the old encode API doxy

2021-03-12 Thread Anton Khirnov
Quoting James Almer (2021-03-13 00:15:37)
> Direct users to the callback that should be used to keep supporting user
> provided buffers with the new encode API.
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/avcodec.h | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fbd4804160..76911baea2 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3680,7 +3680,9 @@ void av_parser_close(AVCodecParserContext *s);
>   *not be used.
>   * @return  0 on success, negative error code on failure
>   *
> - * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead
> + * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead, and
> + * if allowed and required, set AVCodecContext.get_encode_buffer 
> to
> + * a custom function to pass user supplied output buffers.

This sentence would be easier to read if it was split in two IMO.

-- 
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] libavutil/timer: Fix clang reserved-user-defined-literal

2021-03-12 Thread Anton Khirnov
Quoting Christopher Degawa (2021-03-13 05:20:37)
> clang errors when compiling with C++11 about needing spaces between
> literal and identifier

Why would you compile it with C++11? It's a private header, is it not?

-- 
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 5/5] lavfi/dnn_backend_openvino.c: fix mem leak for TaskItem upon error

2021-03-12 Thread Guo, Yejun
---
 libavfilter/dnn/dnn_backend_openvino.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavfilter/dnn/dnn_backend_openvino.c 
b/libavfilter/dnn/dnn_backend_openvino.c
index 55cb0c757e..9a47d74c15 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -678,12 +678,6 @@ DNNReturnType ff_dnn_execute_model_async_ov(const DNNModel 
*model, const char *i
 return DNN_ERROR;
 }
 
-task = av_malloc(sizeof(*task));
-if (!task) {
-av_log(ctx, AV_LOG_ERROR, "unable to alloc memory for task item.\n");
-return DNN_ERROR;
-}
-
 if (!ov_model->exe_network) {
 if (init_model_ov(ov_model, input_name, output_names[0]) != 
DNN_SUCCESS) {
 av_log(ctx, AV_LOG_ERROR, "Failed init OpenVINO exectuable network 
or inference request\n");
@@ -691,6 +685,12 @@ DNNReturnType ff_dnn_execute_model_async_ov(const DNNModel 
*model, const char *i
 }
 }
 
+task = av_malloc(sizeof(*task));
+if (!task) {
+av_log(ctx, AV_LOG_ERROR, "unable to alloc memory for task item.\n");
+return DNN_ERROR;
+}
+
 task->done = 0;
 task->do_ioproc = 1;
 task->async = 1;
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 4/5] lavfi/dnn_backend_openvino.c: fix mem leak for RequestItem upon error

2021-03-12 Thread Guo, Yejun
---
 libavfilter/dnn/dnn_backend_openvino.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/libavfilter/dnn/dnn_backend_openvino.c 
b/libavfilter/dnn/dnn_backend_openvino.c
index 50de6a996e..55cb0c757e 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -353,25 +353,23 @@ static DNNReturnType init_model_ov(OVModel *ov_model, 
const char *input_name, co
 goto err;
 }
 
+item->callback.completeCallBackFunc = infer_completion_callback;
+item->callback.args = item;
+if (ff_safe_queue_push_back(ov_model->request_queue, item) < 0) {
+av_freep();
+goto err;
+}
+
 status = ie_exec_network_create_infer_request(ov_model->exe_network, 
>infer_request);
 if (status != OK) {
-av_freep();
 goto err;
 }
 
 item->tasks = av_malloc_array(ctx->options.batch_size, 
sizeof(*item->tasks));
 if (!item->tasks) {
-av_freep();
 goto err;
 }
 item->task_count = 0;
-
-item->callback.completeCallBackFunc = infer_completion_callback;
-item->callback.args = item;
-if (ff_safe_queue_push_back(ov_model->request_queue, item) < 0) {
-av_freep();
-goto err;
-}
 }
 
 ov_model->task_queue = ff_queue_create();
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 3/5] lavfi/dnn_backend_openvino.c: fix typo upon error

2021-03-12 Thread Guo, Yejun
---
 libavfilter/dnn/dnn_backend_openvino.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/dnn/dnn_backend_openvino.c 
b/libavfilter/dnn/dnn_backend_openvino.c
index df1631e86e..50de6a996e 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -311,7 +311,7 @@ static DNNReturnType init_model_ov(OVModel *ov_model, const 
char *input_name, co
 status = ie_network_set_input_precision(ov_model->network, input_name, 
U8);
 if (status != OK) {
 av_log(ctx, AV_LOG_ERROR, "Failed to set input precision as U8 for 
%s\n", input_name);
-return DNN_ERROR;
+goto err;
 }
 }
 
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 1/5] lavfi/dnn_backend_openvino.c: fix mem leak for AVFrame upon error

2021-03-12 Thread Guo, Yejun
---
 libavfilter/dnn/dnn_backend_openvino.c | 30 ++
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/libavfilter/dnn/dnn_backend_openvino.c 
b/libavfilter/dnn/dnn_backend_openvino.c
index 5be053b7f8..d86fb124d5 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -485,25 +485,12 @@ static DNNReturnType get_output_ov(void *model, const 
char *input_name, int inpu
 OVContext *ctx = _model->ctx;
 TaskItem task;
 RequestItem request;
-AVFrame *in_frame = av_frame_alloc();
+AVFrame *in_frame = NULL;
 AVFrame *out_frame = NULL;
 TaskItem *ptask = 
 IEStatusCode status;
 input_shapes_t input_shapes;
 
-if (!in_frame) {
-av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for input 
frame\n");
-return DNN_ERROR;
-}
-out_frame = av_frame_alloc();
-if (!out_frame) {
-av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for output 
frame\n");
-av_frame_free(_frame);
-return DNN_ERROR;
-}
-in_frame->width = input_width;
-in_frame->height = input_height;
-
 if (ctx->options.input_resizable) {
 status = ie_network_get_input_shapes(ov_model->network, _shapes);
 input_shapes.shapes->shape.dims[2] = input_height;
@@ -523,6 +510,21 @@ static DNNReturnType get_output_ov(void *model, const char 
*input_name, int inpu
 }
 }
 
+in_frame = av_frame_alloc();
+if (!in_frame) {
+av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for input 
frame\n");
+return DNN_ERROR;
+}
+in_frame->width = input_width;
+in_frame->height = input_height;
+
+out_frame = av_frame_alloc();
+if (!out_frame) {
+av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for output 
frame\n");
+av_frame_free(_frame);
+return DNN_ERROR;
+}
+
 task.done = 0;
 task.do_ioproc = 0;
 task.async = 0;
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 2/5] lavfi/dnn_backend_openvino.c: fix mem leak for input_blob and output_blob upon error

2021-03-12 Thread Guo, Yejun
---
 libavfilter/dnn/dnn_backend_openvino.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavfilter/dnn/dnn_backend_openvino.c 
b/libavfilter/dnn/dnn_backend_openvino.c
index d86fb124d5..df1631e86e 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -141,12 +141,14 @@ static DNNReturnType fill_model_input_ov(OVModel 
*ov_model, RequestItem *request
 status |= ie_blob_get_dims(input_blob, );
 status |= ie_blob_get_precision(input_blob, );
 if (status != OK) {
+ie_blob_free(_blob);
 av_log(ctx, AV_LOG_ERROR, "Failed to get input blob dims/precision\n");
 return DNN_ERROR;
 }
 
 status = ie_blob_get_buffer(input_blob, _buffer);
 if (status != OK) {
+ie_blob_free(_blob);
 av_log(ctx, AV_LOG_ERROR, "Failed to get input blob buffer\n");
 return DNN_ERROR;
 }
@@ -211,6 +213,7 @@ static void infer_completion_callback(void *args)
 
 status = ie_blob_get_buffer(output_blob, _buffer);
 if (status != OK) {
+ie_blob_free(_blob);
 av_log(ctx, AV_LOG_ERROR, "Failed to access output memory\n");
 return;
 }
@@ -218,6 +221,7 @@ static void infer_completion_callback(void *args)
 status |= ie_blob_get_dims(output_blob, );
 status |= ie_blob_get_precision(output_blob, );
 if (status != OK) {
+ie_blob_free(_blob);
 av_log(ctx, AV_LOG_ERROR, "Failed to get dims or precision of 
output\n");
 return;
 }
-- 
2.17.1

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

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

Re: [FFmpeg-devel] [PATCH] x11grab: capture a window instead of the whole screen

2021-03-12 Thread Andriy Gelman
On Sun, 28. Feb 10:39, Andriy Gelman wrote:
> On Sun, 28. Feb 13:57, sgerwk-at-aol@ffmpeg.org wrote:
> > Hi,
> > 
> > On Sat, 27 Feb 2021, Andriy Gelman wrote:
> > 
> > > On Mon, 22. Feb 17:53, sgerwk-at-aol@ffmpeg.org wrote:
> > > > Hi,
> > > > 
> > > > On Sun, 21 Feb 2021, Andriy Gelman wrote:
> > > > > Hi,
> > > > > > Thanks for updating the patch. Sorry for the delay in getting
> > > > you some feedback..
> > > > > > When I tested with -show_mouse 1 -show_region 1 -window_id xx,
> > > > the mouse and
> > > > > border get drawn in the wrong place. There is around (0.6cm error). 
> > > > > Can you
> > > > > reproduce?
> > > > >
> > > > 
> > > > I didn't notice the problem because the wm I use places the top-level
> > > > windows in a window of the same size - at position 0,0. Still, I could
> > > > reproduce it by capturing a subwindow.
> > > > 
> > > > The problem was indeed what you suspected: geo->x,geo->y is the position
> > > > inside the parent, but translate includes it already as it is the 
> > > > position
> > > > within the root window.
> > > > 
> > > > > > From e13c1be7abd6989b3ad80fd8086fe6a0819fd810 Mon Sep 17 00:00:00 
> > > > > > 2001
> > > > > > From: sgerwk 
> > > > > > Date: Wed, 10 Feb 2021 17:36:00 +0100
> > > > > > Subject: [PATCH] libavdevice/xcbgrab: option for grabbing a window 
> > > > > > instead of
> > > > > >  desktop
> > > > > > > > ---
> > > > > >  doc/indevs.texi   | 14 +++-
> > > > > >  libavdevice/xcbgrab.c | 79 
> > > > > > ---
> > > > > >  2 files changed, 72 insertions(+), 21 deletions(-)
> > > > > > > > diff --git a/doc/indevs.texi b/doc/indevs.texi
> > > > > > index 3924d03..48fd2b1 100644
> > > > > > --- a/doc/indevs.texi
> > > > > > +++ b/doc/indevs.texi
> > > > > > @@ -1564,8 +1564,20 @@ With @var{follow_mouse}:
> > > > > >  ffmpeg -f x11grab -follow_mouse centered -show_region 1 -framerate 
> > > > > > 25 -video_size cif -i :0.0 out.mpg
> > > > > >  @end example
> > > > > > > > +@item window_id
> > > > > > +Grab this window, instead of the whole screen.
> > > > > > +
> > > > > > +The id of a window can be found by xwininfo(1), possibly with 
> > > > > > options -tree and
> > > > > > +-root.
> > > > > > +
> > > > > > +If the window is later enlarged, the new area is not recorded. 
> > > > > > Video ends when
> > > > > > +the window is closed, unmapped (i.e., iconified) or shrunk beyond 
> > > > > > the video
> > > > > > +size (which defaults to the initial window size).
> > > > > > +
> > > > > > +This option disables options @option{follow_mouse} and 
> > > > > > @option{select_region}.
> > > > > > +
> > > > > >  @item video_size
> > > > > > -Set the video frame size. Default is the full desktop.
> > > > > > +Set the video frame size. Default is the full desktop or window.
> > > > > > > >  @item grab_x
> > > > > >  @item grab_y
> > > > > > diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
> > > > > > index be5d5ea..7697090 100644
> > > > > > --- a/libavdevice/xcbgrab.c
> > > > > > +++ b/libavdevice/xcbgrab.c
> > > > > > @@ -60,6 +60,8 @@ typedef struct XCBGrabContext {
> > > > > >  AVRational time_base;
> > > > > >  int64_t frame_duration;
> > > > > > > > +xcb_window_t window_id;
> > > > > > > +int win_x, win_y;
> > > > > > The position of the window is always recalculated so I don't
> > > > think win_x and
> > > > > win_y should be part of the of XCBGrabContext.
> > > > >
> > > > 
> > > > Done. Some functions now require win_x and win_y because they no longer
> > > > find them in XCBGrabContext.
> > > > 
> > > > > >  int x, y;
> > > > > >  int width, height;
> > > > > >  int frame_size;
> > > > > > @@ -82,6 +84,7 @@ typedef struct XCBGrabContext {
> > > > > >  #define OFFSET(x) offsetof(XCBGrabContext, x)
> > > > > >  #define D AV_OPT_FLAG_DECODING_PARAM
> > > > > >  static const AVOption options[] = {
> > > > > > +{ "window_id", "Window to capture", OFFSET(window_id), 
> > > > > > AV_OPT_TYPE_INT, { .i64 = XCB_NONE }, 0, UINT32_MAX, D },
> > > > > >  { "x", "Initial x coordinate.", OFFSET(x), AV_OPT_TYPE_INT, { 
> > > > > > .i64 = 0 }, 0, INT_MAX, D },
> > > > > >  { "y", "Initial y coordinate.", OFFSET(y), AV_OPT_TYPE_INT, { 
> > > > > > .i64 = 0 }, 0, INT_MAX, D },
> > > > > >  { "grab_x", "Initial x coordinate.", OFFSET(x), 
> > > > > > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, D },
> > > > > > @@ -157,7 +160,7 @@ static int xcbgrab_frame(AVFormatContext *s, 
> > > > > > AVPacket *pkt)
> > > > > >  XCBGrabContext *c = s->priv_data;
> > > > > >  xcb_get_image_cookie_t iq;
> > > > > >  xcb_get_image_reply_t *img;
> > > > > > -xcb_drawable_t drawable = c->screen->root;
> > > > > > +xcb_drawable_t drawable = c->window_id;
> > > > > >  xcb_generic_error_t *e = NULL;
> > > > > >  uint8_t *data;
> > > > > >  int length;
> > > > > > @@ -267,7 +270,7 @@ static int xcbgrab_frame_shm(AVFormatContext 
> > > > > > *s, AVPacket 

Re: [FFmpeg-devel] [PATCH] avformat/httpauth: don't overwrite auth digest with unimplemented algorithm

2021-03-12 Thread Andriy Gelman
On Sun, 07. Mar 18:14, Andriy Gelman wrote:
> From: Andriy Gelman 
> 
> In rtsp/http authentication the server may provide several options for
> hash algorithms. This includes MD5, SHA2-256 and SHA2-512/256 (RFC 7616
> Section 3.7). Currently only support for MD5 is implemented in the auth code.
> 
> If the SHA2 option follows the MD5 option in the server reply, the
> latter option will overwrite the MD5 auth info and the authorization
> will fail.  This patch only overwrites the auth info if it's MD5.
> 
> Fixes ticket #9127.
> 
> Signed-off-by: Andriy Gelman 
> ---
> 
> An alternative may be to add the SHA2 code to http auth. I can work on this if
> people think it's a better option.
> 
> Also, I could only test that the MD5 option doesn't get overwritten by 
> modifying
> server responses in gdb. I could not find an rtsp server that has the SHA2
> option as in #9127. 
> 
> 
>  libavformat/httpauth.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
> index 4f79c78edc..0e57c5c3e5 100644
> --- a/libavformat/httpauth.c
> +++ b/libavformat/httpauth.c
> @@ -101,12 +101,21 @@ void ff_http_auth_handle_header(HTTPAuthState *state, 
> const char *key,
> state);
>  } else if (av_stristart(value, "Digest ", ) &&
> state->auth_type <= HTTP_AUTH_DIGEST) {
> +HTTPAuthState state_copy;
> +const char* algorithm;
> +memcpy(_copy, state, sizeof(state_copy));
> +
>  state->auth_type = HTTP_AUTH_DIGEST;
>  memset(>digest_params, 0, sizeof(DigestParams));
>  state->realm[0] = 0;
>  state->stale = 0;
>  ff_parse_key_value(p, (ff_parse_key_val_cb) handle_digest_params,
> state);
> +algorithm = state->digest_params.algorithm;
> +if (strcmp(algorithm, "") && strcmp(algorithm, "MD5") && 
> strcmp(algorithm, "MD5-sess")) {
> +memcpy(state, _copy, sizeof(state_copy));
> +return;
> +}
>  choose_qop(state->digest_params.qop,
> sizeof(state->digest_params.qop));
>  if (!av_strcasecmp(state->digest_params.stale, "true"))
> -- 
> 2.30.1
> 

ping

-- 
Andriy
___
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] libavutil/timer: Fix clang reserved-user-defined-literal

2021-03-12 Thread Christopher Degawa
clang errors when compiling with C++11 about needing spaces between
literal and identifier

Signed-off-by: Christopher Degawa 
---
 libavutil/timer.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/timer.h b/libavutil/timer.h
index 0bb353cfce..737bfc160e 100644
--- a/libavutil/timer.h
+++ b/libavutil/timer.h
@@ -87,7 +87,7 @@
 if (((tcount + tskip_count) & (tcount + tskip_count - 1)) == 0) { \
 int i;\
 av_log(NULL, AV_LOG_ERROR,\
-   "%7"PRIu64" " FF_TIMER_UNITS " in %s,%8d runs,%7d skips",   
   \
+   "%7" PRIu64 " " FF_TIMER_UNITS " in %s,%8d runs,%7d skips", 
 \
tsum * 10 / tcount, id, tcount, tskip_count);  \
 for (i = 0; i < 32; i++)  \
 av_log(NULL, AV_LOG_VERBOSE, " %2d", 
av_log2(2*thistogram[i]));\
-- 
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 v2 02/18] cbs_h264: Add support for frame packing arrangement SEI messages

2021-03-12 Thread Nuo Mi
On Mon, Feb 22, 2021 at 3:53 AM Mark Thompson  wrote:

> ---
>  libavcodec/cbs_h264.h | 23 
>  libavcodec/cbs_h2645.c|  6 +
>  libavcodec/cbs_h264_syntax_template.c | 39 +++
>  3 files changed, 68 insertions(+)
>
> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
> index 9eb97eae24..1466ed12fa 100644
> --- a/libavcodec/cbs_h264.h
> +++ b/libavcodec/cbs_h264.h
> @@ -282,6 +282,29 @@ typedef struct H264RawSEIRecoveryPoint {
>  uint8_t changing_slice_group_idc;
>  } H264RawSEIRecoveryPoint;
>
> +typedef struct H264RawSEIFramePackingArrangement {
> +uint32_t frame_packing_arrangement_id;
> +uint8_t  frame_packing_arrangement_cancel_flag;
> +
> +uint8_t  frame_packing_arrangement_type;
> +uint8_t  quincunx_sampling_flag;
> +uint8_t  content_interpretation_type;
> +uint8_t  spatial_flipping_flag;
> +uint8_t  frame0_flipped_flag;
> +uint8_t  field_views_flag;
> +uint8_t  current_frame_is_frame0_flag;
> +uint8_t  frame0_self_contained_flag;
> +uint8_t  frame1_self_contained_flag;
> +uint8_t  frame0_grid_position_x;
> +uint8_t  frame0_grid_position_y;
> +uint8_t  frame1_grid_position_x;
> +uint8_t  frame1_grid_position_y;
> +uint8_t  frame_packing_arrangement_reserved_byte;
> +uint16_t frame_packing_arrangement_repetition_period;
> +
> +uint8_t  frame_packing_arrangement_extension_flag;
> +} H264RawSEIFramePackingArrangement;
> +
>  typedef struct H264RawSEIDisplayOrientation {
>  uint8_t display_orientation_cancel_flag;
>  uint8_t hor_flip;
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 6005d46e0d..0c591871d4 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -1570,6 +1570,12 @@ static const SEIMessageTypeDescriptor
> cbs_sei_h264_types[] = {
>  sizeof(H264RawSEIRecoveryPoint),
>  SEI_MESSAGE_RW(h264, sei_recovery_point),
>  },
> +{
> +SEI_TYPE_FRAME_PACKING_ARRANGEMENT,
> +1, 0,
> +sizeof(H264RawSEIFramePackingArrangement),
> +SEI_MESSAGE_RW(h264, sei_frame_packing_arrangement),
> +},
>  {
>  SEI_TYPE_DISPLAY_ORIENTATION,
>  1, 0,
> diff --git a/libavcodec/cbs_h264_syntax_template.c
> b/libavcodec/cbs_h264_syntax_template.c
> index 9587f33985..e03d41e47a 100644
> --- a/libavcodec/cbs_h264_syntax_template.c
> +++ b/libavcodec/cbs_h264_syntax_template.c
> @@ -719,6 +719,45 @@ static int
> FUNC(sei_recovery_point)(CodedBitstreamContext *ctx, RWContext *rw,
>  return 0;
>  }
>
> +
> +
> +static int FUNC(sei_frame_packing_arrangement)
> +(CodedBitstreamContext *ctx, RWContext *rw,
> + H264RawSEIFramePackingArrangement *current, SEIMessageState *sei)
> +{
> +int err;
> +
> +HEADER("Frame Packing Arrangement");
> +
> +ue(frame_packing_arrangement_id, 0, UINT32_MAX - 1);
> +flag(frame_packing_arrangement_cancel_flag);
> +
> +if (!current->frame_packing_arrangement_cancel_flag) {
> +u(7, frame_packing_arrangement_type, 0, 7);
> +flag(quincunx_sampling_flag);
> +u(6, content_interpretation_type, 0, 2);

>2 is reserved, should we use ub(6,  content_interpretation_type)?

>



+flag(spatial_flipping_flag);
> +flag(frame0_flipped_flag);
> +flag(field_views_flag);
> +flag(current_frame_is_frame0_flag);
> +flag(frame0_self_contained_flag);
> +flag(frame1_self_contained_flag);
> +if (!current->quincunx_sampling_flag &&
> +current->frame_packing_arrangement_type != 5) {
> +ub(4, frame0_grid_position_x);
> +ub(4, frame0_grid_position_y);
> +ub(4, frame1_grid_position_x);
> +ub(4, frame1_grid_position_y);
> +}
> +u(8, frame_packing_arrangement_reserved_byte, 0, 0);
>
ub for reserved bytes?

> +ue(frame_packing_arrangement_repetition_period, 0, 16384);
> +}
> +
> +flag(frame_packing_arrangement_extension_flag);
> +
> +return 0;
> +}
> +
>  static int FUNC(sei_display_orientation)(CodedBitstreamContext *ctx,
> RWContext *rw,
>   H264RawSEIDisplayOrientation
> *current,
>   SEIMessageState *sei)
> --
> 2.30.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 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/decode: Remove outdated comment

2021-03-12 Thread James Almer

On 3/12/2021 9:26 PM, Andreas Rheinhardt wrote:

Removing it was forgotten in 417d473bde220a1f267bc694835c129a5adc4309.

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/decode.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 495e9e8b23..5f7e9bda3e 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -299,7 +299,6 @@ static inline int decode_simple_internal(AVCodecContext 
*avctx, AVFrame *frame,
  AVCodecInternal   *avci = avctx->internal;
  DecodeSimpleContext *ds = >ds;
  AVPacket   *pkt = ds->in_pkt;
-// copy to ensure we do not change pkt
  int got_frame, actual_got_frame;
  int ret;


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

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

[FFmpeg-devel] [PATCH] avcodec/decode: Remove outdated comment

2021-03-12 Thread Andreas Rheinhardt
Removing it was forgotten in 417d473bde220a1f267bc694835c129a5adc4309.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/decode.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 495e9e8b23..5f7e9bda3e 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -299,7 +299,6 @@ static inline int decode_simple_internal(AVCodecContext 
*avctx, AVFrame *frame,
 AVCodecInternal   *avci = avctx->internal;
 DecodeSimpleContext *ds = >ds;
 AVPacket   *pkt = ds->in_pkt;
-// copy to ensure we do not change pkt
 int got_frame, actual_got_frame;
 int ret;
 
-- 
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/5] avcodec/vaapi_encode: use AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/vaapi_encode.c   | 2 +-
 libavcodec/vaapi_encode_h264.c  | 3 ++-
 libavcodec/vaapi_encode_h265.c  | 3 ++-
 libavcodec/vaapi_encode_mjpeg.c | 2 +-
 libavcodec/vaapi_encode_mpeg2.c | 3 ++-
 libavcodec/vaapi_encode_vp8.c   | 3 ++-
 libavcodec/vaapi_encode_vp9.c   | 3 ++-
 7 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 3c57c07f67..607858435f 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -650,7 +650,7 @@ static int vaapi_encode_output(AVCodecContext *avctx,
 for (buf = buf_list; buf; buf = buf->next)
 total_size += buf->size;
 
-err = av_new_packet(pkt, total_size);
+err = ff_get_encode_buffer(avctx, pkt, total_size, 0);
 ptr = pkt->data;
 
 if (err < 0)
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index ce73e269f9..6e20e2d015 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -1334,7 +1334,8 @@ AVCodec ff_h264_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _encode_h264_close,
 .priv_class = _encode_h264_class,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_h264_defaults,
 .pix_fmts = (const enum AVPixelFormat[]) {
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 2c356fbd38..287ac58690 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -1312,7 +1312,8 @@ AVCodec ff_hevc_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _encode_h265_close,
 .priv_class = _encode_h265_class,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_h265_defaults,
 .pix_fmts = (const enum AVPixelFormat[]) {
diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
index 557f59f00c..3878f3eeb9 100644
--- a/libavcodec/vaapi_encode_mjpeg.c
+++ b/libavcodec/vaapi_encode_mjpeg.c
@@ -562,7 +562,7 @@ AVCodec ff_mjpeg_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _encode_mjpeg_close,
 .priv_class = _encode_mjpeg_class,
-.capabilities   = AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_HARDWARE | AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_mjpeg_defaults,
 .pix_fmts = (const enum AVPixelFormat[]) {
diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
index df2c62b8e7..4c81d785d6 100644
--- a/libavcodec/vaapi_encode_mpeg2.c
+++ b/libavcodec/vaapi_encode_mpeg2.c
@@ -698,7 +698,8 @@ AVCodec ff_mpeg2_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _encode_mpeg2_close,
 .priv_class = _encode_mpeg2_class,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_mpeg2_defaults,
 .pix_fmts = (const enum AVPixelFormat[]) {
diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c
index 51039fa19a..66cf2321be 100644
--- a/libavcodec/vaapi_encode_vp8.c
+++ b/libavcodec/vaapi_encode_vp8.c
@@ -255,7 +255,8 @@ AVCodec ff_vp8_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _vaapi_encode_close,
 .priv_class = _encode_vp8_class,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_vp8_defaults,
 .pix_fmts = (const enum AVPixelFormat[]) {
diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index 4f3b55ed2d..0e1c52c92a 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -289,7 +289,8 @@ AVCodec ff_vp9_vaapi_encoder = {
 .receive_packet = _vaapi_encode_receive_packet,
 .close  = _vaapi_encode_close,
 .priv_class = _encode_vp9_class,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .defaults   = vaapi_encode_vp9_defaults,
 .pix_fmts = (const enum 

[FFmpeg-devel] [PATCH 4/5] avcodec/nvenc: use AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/nvenc.c  | 2 +-
 libavcodec/nvenc_h264.c | 2 +-
 libavcodec/nvenc_hevc.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index fbf55ebc9d..dddee8cac1 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -1971,7 +1971,7 @@ static int process_output_surface(AVCodecContext *avctx, 
AVPacket *pkt, NvencSur
 goto error;
 }
 
-res = av_new_packet(pkt, lock_params.bitstreamSizeInBytes);
+res = ff_get_encode_buffer(avctx, pkt, lock_params.bitstreamSizeInBytes, 
0);
 
 if (res < 0) {
 p_nvenc->nvEncUnlockBitstream(ctx->nvencoder, 
tmpoutsurf->output_surface);
diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
index 4c118d3138..4c2585876e 100644
--- a/libavcodec/nvenc_h264.c
+++ b/libavcodec/nvenc_h264.c
@@ -287,7 +287,7 @@ AVCodec ff_h264_nvenc_encoder = {
 .priv_class = _nvenc_class,
 .defaults   = defaults,
 .capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
-  AV_CODEC_CAP_ENCODER_FLUSH,
+  AV_CODEC_CAP_ENCODER_FLUSH | AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .pix_fmts   = ff_nvenc_pix_fmts,
 .wrapper_name   = "nvenc",
diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
index 031d2ef44e..441e7871d2 100644
--- a/libavcodec/nvenc_hevc.c
+++ b/libavcodec/nvenc_hevc.c
@@ -237,7 +237,7 @@ AVCodec ff_hevc_nvenc_encoder = {
 .defaults   = defaults,
 .pix_fmts   = ff_nvenc_pix_fmts,
 .capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
-  AV_CODEC_CAP_ENCODER_FLUSH,
+  AV_CODEC_CAP_ENCODER_FLUSH | AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .wrapper_name   = "nvenc",
 .hw_configs = ff_nvenc_hw_configs,
-- 
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".

[FFmpeg-devel] [PATCH 3/5] avcodec/mfenc: use AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/mfenc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c
index 7fdc7af330..d70e49351a 100644
--- a/libavcodec/mfenc.c
+++ b/libavcodec/mfenc.c
@@ -243,7 +243,7 @@ static int mf_sample_to_avpacket(AVCodecContext *avctx, 
IMFSample *sample, AVPac
 if (FAILED(hr))
 return AVERROR_EXTERNAL;
 
-if ((ret = av_new_packet(avpkt, len)) < 0)
+if ((ret = ff_get_encode_buffer(avctx, avpkt, len, 0)) < 0)
 return ret;
 
 IMFSample_ConvertToContiguousBuffer(sample, );
@@ -1163,7 +1163,8 @@ static int mf_close(AVCodecContext *avctx)
 .close  = mf_close,
\
 .receive_packet = mf_receive_packet,   
\
 EXTRA  
\
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HYBRID,
\
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HYBRID |   
\
+  AV_CODEC_CAP_DR1,
\
 .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |   
\
   FF_CODEC_CAP_INIT_CLEANUP,   
\
 };
-- 
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".

[FFmpeg-devel] [PATCH 2/5] avcodec/librav1e: use AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/librav1e.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/librav1e.c b/libavcodec/librav1e.c
index 2d5acc7d8e..276fb1b039 100644
--- a/libavcodec/librav1e.c
+++ b/libavcodec/librav1e.c
@@ -532,7 +532,7 @@ retry:
 return AVERROR_UNKNOWN;
 }
 
-ret = av_new_packet(pkt, rpkt->len);
+ret = ff_get_encode_buffer(avctx, pkt, rpkt->len, 0);
 if (ret < 0) {
 av_log(avctx, AV_LOG_ERROR, "Could not allocate packet.\n");
 rav1e_packet_unref(rpkt);
@@ -624,7 +624,8 @@ AVCodec ff_librav1e_encoder = {
 .priv_class = ,
 .defaults   = librav1e_defaults,
 .pix_fmts   = librav1e_pix_fmts,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .wrapper_name   = "librav1e",
 };
-- 
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".

[FFmpeg-devel] [PATCH 1/5] avcodec/amfenc: use AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/amfenc.c  | 2 +-
 libavcodec/amfenc_h264.c | 3 ++-
 libavcodec/amfenc_hevc.c | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index e234cfd354..fb23ed738c 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -452,7 +452,7 @@ static int amf_copy_buffer(AVCodecContext *avctx, AVPacket 
*pkt, AMFBuffer *buff
 int64_t  timestamp = AV_NOPTS_VALUE;
 int64_t  size = buffer->pVtbl->GetSize(buffer);
 
-if ((ret = av_new_packet(pkt, size)) < 0) {
+if ((ret = ff_get_encode_buffer(avctx, pkt, size, 0)) < 0) {
 return ret;
 }
 memcpy(pkt->data, buffer->pVtbl->GetNative(buffer), size);
diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
index 622ee85946..fe30d7f11d 100644
--- a/libavcodec/amfenc_h264.c
+++ b/libavcodec/amfenc_h264.c
@@ -388,7 +388,8 @@ AVCodec ff_h264_amf_encoder = {
 .priv_data_size = sizeof(AmfContext),
 .priv_class = _amf_class,
 .defaults   = defaults,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .pix_fmts   = ff_amf_pix_fmts,
 .wrapper_name   = "amf",
diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
index a1225c09e3..b5c840e5f3 100644
--- a/libavcodec/amfenc_hevc.c
+++ b/libavcodec/amfenc_hevc.c
@@ -320,7 +320,8 @@ AVCodec ff_hevc_amf_encoder = {
 .priv_data_size = sizeof(AmfContext),
 .priv_class = _amf_class,
 .defaults   = defaults,
-.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE,
+.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE |
+  AV_CODEC_CAP_DR1,
 .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
 .pix_fmts   = ff_amf_pix_fmts,
 .wrapper_name   = "amf",
-- 
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".

[FFmpeg-devel] [PATCH 0/5] Initial usage of AVCodecContext.get_encode_buffer()

2021-03-12 Thread James Almer
For ffmpeg 4.4, as the last version featuring avcodec_encode_video2/audio2 and
since marking an encoder as AV_CODEC_CAP_DR1 capable will make it unusable with
that API, we should only port encoders using the AVCodec.receive_packet() API.

The rest can be ported after either ffmpeg 4.4 is branched out from master, or
the major version bumped and the old encode API removed.

James Almer (5):
  avcodec/amfenc: use AVCodecContext.get_encode_buffer()
  avcodec/librav1e: use AVCodecContext.get_encode_buffer()
  avcodec/mfenc: use AVCodecContext.get_encode_buffer()
  avcodec/nvenc: use AVCodecContext.get_encode_buffer()
  avcodec/vaapi_encode: use AVCodecContext.get_encode_buffer()

 libavcodec/amfenc.c | 2 +-
 libavcodec/amfenc_h264.c| 3 ++-
 libavcodec/amfenc_hevc.c| 3 ++-
 libavcodec/librav1e.c   | 5 +++--
 libavcodec/mfenc.c  | 5 +++--
 libavcodec/nvenc.c  | 2 +-
 libavcodec/nvenc_h264.c | 2 +-
 libavcodec/nvenc_hevc.c | 2 +-
 libavcodec/vaapi_encode.c   | 2 +-
 libavcodec/vaapi_encode_h264.c  | 3 ++-
 libavcodec/vaapi_encode_h265.c  | 3 ++-
 libavcodec/vaapi_encode_mjpeg.c | 2 +-
 libavcodec/vaapi_encode_mpeg2.c | 3 ++-
 libavcodec/vaapi_encode_vp8.c   | 3 ++-
 libavcodec/vaapi_encode_vp9.c   | 3 ++-
 15 files changed, 26 insertions(+), 17 deletions(-)

-- 
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 v2 2/5] avdevice/alsa_dec: make sure we have enough data in non-blocking mode

2021-03-12 Thread Marton Balint



On Wed, 10 Mar 2021, Marton Balint wrote:




On Sat, 6 Mar 2021, Marton Balint wrote:




On Mon, 1 Mar 2021, Marton Balint wrote:

Otherwise we might return 1-2 samples per packet if av_read_frame() call 

rate is

only sligthly less than the stream sample rate.


Ping for this and the rest of the series. Note that the approach in 
this patch makes 1/5 unneeded because constant frame size is now indeed 
guaranteed.


Will apply the series soon.


Applied.

Regards,
Marton



Regards,
Marton



Signed-off-by: Marton Balint 
---
libavdevice/alsa.c |  5 +
libavdevice/alsa.h |  1 +
libavdevice/alsa_dec.c | 22 --
3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
index 117b2ea144..ee282fac16 100644
--- a/libavdevice/alsa.c
+++ b/libavdevice/alsa.c
@@ -286,6 +286,10 @@ av_cold int ff_alsa_open(AVFormatContext *ctx, 

snd_pcm_stream_t mode,

}
}

+s->pkt = av_packet_alloc();
+if (!s->pkt)
+goto fail1;
+
s->h = h;
return 0;

@@ -308,6 +312,7 @@ av_cold int ff_alsa_close(AVFormatContext *s1)
if (CONFIG_ALSA_INDEV)
ff_timefilter_destroy(s->timefilter);
snd_pcm_close(s->h);
+av_packet_free(>pkt);
return 0;
}

diff --git a/libavdevice/alsa.h b/libavdevice/alsa.h
index 1ed8c82199..07783c983a 100644
--- a/libavdevice/alsa.h
+++ b/libavdevice/alsa.h
@@ -58,6 +58,7 @@ typedef struct AlsaData {
void *reorder_buf;
int reorder_buf_size; ///< in frames
int64_t timestamp; ///< current timestamp, without latency applied.
+AVPacket *pkt;
} AlsaData;

/**
diff --git a/libavdevice/alsa_dec.c b/libavdevice/alsa_dec.c
index 6d568737b3..88bf32d25f 100644
--- a/libavdevice/alsa_dec.c
+++ b/libavdevice/alsa_dec.c
@@ -104,34 +104,36 @@ static int audio_read_packet(AVFormatContext *s1, 

AVPacket *pkt)

int64_t dts;
snd_pcm_sframes_t delay = 0;

-if (av_new_packet(pkt, s->period_size * s->frame_size) < 0) {
-return AVERROR(EIO);
+if (!s->pkt->data) {
+int ret = av_new_packet(s->pkt, s->period_size * s->frame_size);
+if (ret < 0)
+return ret;
+s->pkt->size = 0;
}

-while ((res = snd_pcm_readi(s->h, pkt->data, s->period_size)) < 0) {
+do {
+while ((res = snd_pcm_readi(s->h, s->pkt->data + s->pkt->size, 

s->period_size - s->pkt->size / s->frame_size)) < 0) {

if (res == -EAGAIN) {
-av_packet_unref(pkt);
-
return AVERROR(EAGAIN);
}
+s->pkt->size = 0;
if (ff_alsa_xrun_recover(s1, res) < 0) {
av_log(s1, AV_LOG_ERROR, "ALSA read error: %s\n",
   snd_strerror(res));
-av_packet_unref(pkt);
-
return AVERROR(EIO);
}
ff_timefilter_reset(s->timefilter);
-}
+}
+s->pkt->size += res * s->frame_size;
+} while (s->pkt->size < s->period_size * s->frame_size);

+av_packet_move_ref(pkt, s->pkt);
dts = av_gettime();
snd_pcm_delay(s->h, );
dts -= av_rescale(delay + res, 100, s->sample_rate);
pkt->pts = ff_timefilter_update(s->timefilter, dts, s->last_period);
s->last_period = res;

-pkt->size = res * s->frame_size;
-
return 0;
}

--
2.26.2

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

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

___
ffmpeg-devel 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 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 mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] avcodec: add a mention about get_encode_buffer in the old encode API doxy

2021-03-12 Thread James Almer
Direct users to the callback that should be used to keep supporting user
provided buffers with the new encode API.

Signed-off-by: James Almer 
---
 libavcodec/avcodec.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index fbd4804160..76911baea2 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3680,7 +3680,9 @@ void av_parser_close(AVCodecParserContext *s);
  *not be used.
  * @return  0 on success, negative error code on failure
  *
- * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead
+ * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead, and
+ * if allowed and required, set AVCodecContext.get_encode_buffer to
+ * a custom function to pass user supplied output buffers.
  */
 attribute_deprecated
 int avcodec_encode_audio2(AVCodecContext *avctx, AVPacket *avpkt,
@@ -3719,7 +3721,9 @@ int avcodec_encode_audio2(AVCodecContext *avctx, AVPacket 
*avpkt,
  *not be used.
  * @return  0 on success, negative error code on failure
  *
- * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead
+ * @deprecated use avcodec_send_frame()/avcodec_receive_packet() instead, and
+ * if allowed and required, set AVCodecContext.get_encode_buffer to
+ * a custom function to pass user supplied output buffers.
  */
 attribute_deprecated
 int avcodec_encode_video2(AVCodecContext *avctx, AVPacket *avpkt,
-- 
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/8] avcodec/h264_cavlc: Remove redundant check

2021-03-12 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> The only caller to ff_h264_decode_init_vlc() already uses
> ff_thread_once() for the call; ergo the check via a simple int with
> static storage duration in ff_h264_decode_init_vlc() is redundant.
> And if it were not redundant, it would be a potential for data races.
> So remove it.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/h264_cavlc.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
> index 6481992e58..d0548d84f6 100644
> --- a/libavcodec/h264_cavlc.c
> +++ b/libavcodec/h264_cavlc.c
> @@ -325,12 +325,8 @@ static av_cold void init_cavlc_level_tab(void){
>  }
>  
>  av_cold void ff_h264_decode_init_vlc(void){
> -static int done = 0;
> -
> -if (!done) {
>  int i;
>  int offset;
> -done = 1;
>  
>  chroma_dc_coeff_token_vlc.table = chroma_dc_coeff_token_vlc_table;
>  chroma_dc_coeff_token_vlc.table_allocated = 
> chroma_dc_coeff_token_vlc_table_size;
> @@ -410,7 +406,6 @@ av_cold void ff_h264_decode_init_vlc(void){
>   INIT_VLC_USE_NEW_STATIC);
>  
>  init_cavlc_level_tab();
> -}
>  }
>  
>  static inline int get_level_prefix(GetBitContext *gb){
> 
Will apply patches 2-5 of this patchset tomorrow unless there are
objections.

- 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: add VKAPI hwcontext implementation

2021-03-12 Thread Suji Velupillai
On Fri, Mar 12, 2021 at 1:12 AM Jean-Baptiste Kempf  wrote:

> On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:
> > Initial commit to add VKAPI hardware accelerator implementation.
> > The depedency component vkil source code can be obtained from github
> > https://github.com/Broadcom/vkil
>
> This has no license, no readme, no description.
>
I'll have this addressed in the github.


>
> What is the hardware supported?
>
It's an M.2 module connected via PCIe that supports video transcoding. Some
high level info can be found here
https://engineering.fb.com/2019/03/14/data-center-engineering/accelerating-infrastructure/

Does it work for rPI?
>
No

>
> --
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> ___
> 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".


smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 02/18] cbs_h264: Add support for frame packing arrangement SEI messages

2021-03-12 Thread Mark Thompson

On 23/02/2021 16:39, James Almer wrote:

On 2/21/2021 4:51 PM, Mark Thompson wrote:

---
  libavcodec/cbs_h264.h | 23 
  libavcodec/cbs_h2645.c    |  6 +
  libavcodec/cbs_h264_syntax_template.c | 39 +++
  3 files changed, 68 insertions(+)

...
diff --git a/libavcodec/cbs_h264_syntax_template.c 
b/libavcodec/cbs_h264_syntax_template.c
index 9587f33985..e03d41e47a 100644
--- a/libavcodec/cbs_h264_syntax_template.c
+++ b/libavcodec/cbs_h264_syntax_template.c
@@ -719,6 +719,45 @@ static int FUNC(sei_recovery_point)(CodedBitstreamContext 
*ctx, RWContext *rw,
  return 0;
  }
+
+
+static int FUNC(sei_frame_packing_arrangement)
+    (CodedBitstreamContext *ctx, RWContext *rw,
+ H264RawSEIFramePackingArrangement *current, SEIMessageState *sei)
+{
+    int err;
+
+    HEADER("Frame Packing Arrangement");
+
+    ue(frame_packing_arrangement_id, 0, UINT32_MAX - 1);
+    flag(frame_packing_arrangement_cancel_flag);
+
+    if (!current->frame_packing_arrangement_cancel_flag) {
+    u(7, frame_packing_arrangement_type, 0, 7);
+    flag(quincunx_sampling_flag);
+    u(6, content_interpretation_type, 0, 2);
+    flag(spatial_flipping_flag);
+    flag(frame0_flipped_flag);
+    flag(field_views_flag);
+    flag(current_frame_is_frame0_flag);
+    flag(frame0_self_contained_flag);
+    flag(frame1_self_contained_flag);
+    if (!current->quincunx_sampling_flag &&
+    current->frame_packing_arrangement_type != 5) {


nit: maybe H264_SEI_FPA_TYPE_INTERLEAVE_TEMPORAL instead of 5.


The standard does say exactly "!= 5", so I prefer to keep to that.  (It would 
be nice if the H.2(6[456]|74) specs made better use of enum constants, but we are stuck 
with what they actually do.)

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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 7:10 PM, Lynne wrote:




Mar 12, 2021, 21:00 by jamr...@gmail.com:


On 3/12/2021 4:42 PM, Michael Niedermayer wrote:



Related to this, what do the flags passed into get_encoder_buffer() mean
exactly ?
are they "hints" so that a new/unknown flag can be ignored by an implementation
or what else should an application do with a unknown flag, this should
be clarified in the documentation



They are hints, same as in get_buffer2(), like with the _REF flag. They can't 
really be requirements since the callback should be able to ignore new flags it 
doesn't understand.



If we have something, I'd rather not have a hint for alignment, but a comment
recommending "aligned data might be faster depending on the architecture and
encoder" or something along those lines.
That way in the future we can make AVPacket->data aligned without needing
to deprecate an alignment hint here.


Added a comment like that, and pushed.

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".

Re: [FFmpeg-devel] [PATCH] cbs_sei: Detect payload overflows when reading SEI messages

2021-03-12 Thread Mark Thompson

On 21/02/2021 20:58, Michael Niedermayer wrote:

On Tue, Feb 02, 2021 at 08:58:11PM +, Mark Thompson wrote:

The top-level GetBitContext is sized for the whole NAL unit, so it fails
to detect overflows where a payload continues into the following message.
To fix that, we make a new context on the stack for reading each payload.
---
On 01/02/2021 22:31, Michael Niedermayer wrote:

Fixes: Timeout
Fixes: 
29892/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-6310830956216320

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

diff --git a/libavcodec/cbs_sei_syntax_template.c 
b/libavcodec/cbs_sei_syntax_template.c
index 9114e61ff6..3b9bc942f5 100644
--- a/libavcodec/cbs_sei_syntax_template.c
+++ b/libavcodec/cbs_sei_syntax_template.c
@@ -178,6 +178,8 @@ static int FUNC(message)(CodedBitstreamContext *ctx, 
RWContext *rw,
   GetBitContext tmp = *rw;
   int trailing_bits, trailing_zero_bits;
+if (8 * current->payload_size < bits_written)
+return AVERROR_INVALIDDATA;
   bits_left = 8 * current->payload_size - bits_written;
   if (bits_left > 8)
   skip_bits_long(, bits_left - 8);


So it looks like the actual problem is that we don't detect payload overflow, 
so the calculation here underflows if the payload is invalid such that we read 
more bits than there actually are.

How about this answer, which tries to fix the general problem by detecting 
overflow properly -



does it fix your fuzzed case?


yes


Added appropriate annotation and applied.

Thanks,

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

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

Re: [FFmpeg-devel] [PATCH] cbs_h265: Detect more reference combinations which would overflow the DPB

2021-03-12 Thread Mark Thompson

On 21/02/2021 20:54, Michael Niedermayer wrote:

On Wed, Feb 03, 2021 at 09:34:07PM +, Mark Thompson wrote:

In total, the number of short term references (from the selected short
term ref pic set), the number of long term references (combining both the
used candidates from the SPS and those defined in the slice header) and
the number of instances of the current picture (usually one, but can be
two if current picture reference is enabled) must never exceed the size
of the DPB.  This is a generalisation of the condition associated with
num_long_term_pics in 7.4.7.1.

We use this to apply tighter bounds to the number of long term pictures
referred to in the slice header, and also to detect the invalid case where
the second reference to the current picture would not fit in the DPB (this
case can't be detected earlier because an STRPS with 15 pictures can still
be valid in the same stream when used with a different PPS which does not
require two DPB slots for the current picture).
---



Michael: does this fix your fuzz case for num_long_term_sps?


yes


Added appropriate annotation and applied.

Thanks,

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

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

Re: [FFmpeg-devel] [PATCH 9/9] avcodec/cbs_h26[45]: Remove redundant enum constants

2021-03-12 Thread Mark Thompson

On 10/03/2021 10:29, Andreas Rheinhardt wrote:

Unused since 8843607f495c95c1e67a3ce3d6f15dca6e252439.

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/cbs_h264.h | 10 --
  libavcodec/cbs_h265.h |  9 -
  2 files changed, 19 deletions(-)

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index 9eb97eae24..5a8641a333 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -28,16 +28,6 @@
  #include "h264.h"
  
  
-enum {

-// This limit is arbitrary - it is sufficient for one message of each
-// type plus some repeats, and will therefore easily cover all sane
-// streams.  However, it is possible to make technically-valid streams
-// for which it will fail (for example, by including a large number of
-// user-data-unregistered messages).
-H264_MAX_SEI_PAYLOADS = 64,
-};
-
-
  typedef struct H264RawNALUnitHeader {
  uint8_t nal_ref_idc;
  uint8_t nal_unit_type;
diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
index 738cbeec2c..f7753f1206 100644
--- a/libavcodec/cbs_h265.h
+++ b/libavcodec/cbs_h265.h
@@ -26,15 +26,6 @@
  #include "cbs_sei.h"
  #include "hevc.h"
  
-enum {

-// This limit is arbitrary - it is sufficient for one message of each
-// type plus some repeats, and will therefore easily cover all sane
-// streams.  However, it is possible to make technically-valid streams
-// for which it will fail (for example, by including a large number of
-// user-data-unregistered messages).
-H265_MAX_SEI_PAYLOADS = 64,
-};
-
  typedef struct H265RawNALUnitHeader {
  uint8_t nal_unit_type;
  uint8_t nuh_layer_id;



Yes, LGTM.

Thanks,

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

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

Re: [FFmpeg-devel] [PATCH 7/8] avcodec/cbs_sei: Fix leak of AVBufferRef on error

2021-03-12 Thread Mark Thompson

On 10/03/2021 01:06, Andreas Rheinhardt wrote:

An AVBufferRef (and the corresponding AVBuffer and the underlying actual
buffer) would leak in ff_cbs_sei_add_message() on error in case an error
happened after its creation and before it has been attached to more
permanent storage. Fix this by only creating the AVBufferRef immediately
before attaching it to its intended target position.

(Given that no SEI message currently created is refcounted, the above
can't happen at the moment. But Coverity already nevertheless noticed:
This commit fixes Coverity issue #1473521.)

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/cbs_sei.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

...


Patches 1, 6, 7 LGTM.

Thanks,

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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Lynne



Mar 12, 2021, 21:00 by jamr...@gmail.com:

> On 3/12/2021 4:42 PM, Michael Niedermayer wrote:
>
>>
>> Related to this, what do the flags passed into get_encoder_buffer() mean
>> exactly ?
>> are they "hints" so that a new/unknown flag can be ignored by an 
>> implementation
>> or what else should an application do with a unknown flag, this should
>> be clarified in the documentation
>>
>
> They are hints, same as in get_buffer2(), like with the _REF flag. They can't 
> really be requirements since the callback should be able to ignore new flags 
> it doesn't understand.
>

If we have something, I'd rather not have a hint for alignment, but a comment
recommending "aligned data might be faster depending on the architecture and
encoder" or something along those lines.
That way in the future we can make AVPacket->data aligned without needing
to deprecate an alignment hint here.
___
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: add VKAPI hwcontext implementation

2021-03-12 Thread Mark Thompson

On 12/03/2021 18:29, Suji Velupillai wrote:

On Fri, Mar 12, 2021 at 1:12 AM Jean-Baptiste Kempf  wrote:


On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:

Initial commit to add VKAPI hardware accelerator implementation.
The depedency component vkil source code can be obtained from github
https://github.com/Broadcom/vkil


This has no license, no readme, no description.


I'll have this addressed in the github.




What is the hardware supported?


It's an M.2 module connected via PCIe that supports video transcoding. Some
high level info can be found here
https://engineering.fb.com/2019/03/14/data-center-engineering/accelerating-infrastructure/

Does it work for rPI?



No


So it's some sort of custom hardware for Facebook servers, and presumably 
therefore of no use to anyone outside Facebook?  Why is this being submitted to 
FFmpeg?

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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Mark Thompson

On 22/02/2021 22:27, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2() is for decoders, and
implements for the new encode API the functionality of the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line about how the callback
must be thread safe.

  libavcodec/avcodec.h | 45 
  libavcodec/codec.h   |  8 ---
  libavcodec/encode.c  | 54 +++-
  libavcodec/encode.h  |  8 +++
  libavcodec/options.c |  1 +
  5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
   */
  #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
  struct AVCodecInternal;
  /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
   * - encoding: set by user
   */
  int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to calculate the required buffer 
size,
+ * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with something else.  E.g. 
you pass a pointer to the block of memory following where you are going to put 
your header data (for something you are going to send over the network, say).

* You want to avoid a copy when passing the output directly to something 
external.  E.g. you pass a pointer to a memory-mapped device buffer (such as a 
V4L2 buffer, say).

In the second case, write-only memory on an external device seems possible, as 
does memory which is, say, readable but uncached, so reading it is a really bad 
idea.


Allowing the second case would depend on how encoders behave. Some may attempt 
to read data already written to the output packet. It's not like all of them 
allocate the packet, do a memcpy from an internal buffer, then return.
There is also the flag meant to signal that the encoder will keep a reference 
to the packet around, which more or less implies it will be read later in the 
encoding process.

The doxy for avcodec_encode_video2(), which allowed the user to provide their 
own buffers in the output packet, does not mention any kind of requirement for 
the data pointer, so I don't think we can say it's an allowed scenario here 
either.


Fair enough.  If the tricky cases aren't allowed then there is no problem :)


Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for the payload.


I think say that explicitly.  avcodec_default_get_encoder_buffer() does give 
you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data pointer" add 
anything of value to the doxy? If i don't mention any kind of alignment requirement, it's 
because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind, like the padding and 
the need for an AVBufferRef. But if you think it's worth adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(), and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of providing a buffer 
allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in flags then the packet may 
be reused
+ * (read and/or written to if it is writable) later by libavcodec.
+ *
+ * This callback must be thread-safe, as when frame multithreading is 
used, it may
+ * be called from multiple threads simultaneously.


Allowing simulatenous calls feels unexpectedly tricky.  Is it really necessary?


This was a suggestion by Lynne, i personally don't know. We support frame 
threading encoding (For intra-only codecs), but currently ff_alloc_packet2() 
does not seem to be 

Re: [FFmpeg-devel] [PATCH] nvenc: Use frameIntervalP instead of max_b_frames for offsetting output dts

2021-03-12 Thread Timo Rothenpieler

applied



smime.p7s
Description: S/MIME Cryptographic 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: add VKAPI hwcontext implementation

2021-03-12 Thread Mark Thompson

On 11/03/2021 22:09, suji.velupil...@broadcom.com wrote:

From: Suji Velupillai 

Initial commit to add VKAPI hardware accelerator implementation.
The depedency component vkil source code can be obtained from github
https://github.com/Broadcom/vkil

Signed-off-by: Suji Velupillai 
---
  configure  |   8 +-
  doc/APIchanges |   4 +
  libavutil/Makefile |   2 +
  libavutil/hwcontext.c  |   4 +
  libavutil/hwcontext.h  |   1 +
  libavutil/hwcontext_internal.h |   1 +
  libavutil/hwcontext_vkapi.c| 522 +
  libavutil/hwcontext_vkapi.h| 104 +++
  libavutil/pixdesc.c|   4 +
  libavutil/pixfmt.h |   6 +
  10 files changed, 655 insertions(+), 1 deletion(-)
  create mode 100644 libavutil/hwcontext_vkapi.c
  create mode 100644 libavutil/hwcontext_vkapi.h


Where has the "vkapi" name come from?  It seems to be consistently called 
"vkil" in that repository.

If you are making up the name for this, please consider making it less 
confusing:
* The standard Vulkan API already effectively owns the "vk" namespace prefix, 
and colliding with that in a related project is unhelpful.
  * Indeed, Vulkan already uses the "VKAPI" name in its headers when marking ABIs 
(see 
).
* Current search results for "vkapi" show it is also used by APIs for the VK 
social network.


... > diff --git a/libavutil/hwcontext_vkapi.h b/libavutil/hwcontext_vkapi.h
new file mode 100644
index 00..096602b42e
--- /dev/null
+++ b/libavutil/hwcontext_vkapi.h
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2018 Broadcom
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_HWCONTEXT_VKAPI_H
+#define AVUTIL_HWCONTEXT_VKAPI_H
+
+#include 
+
+#define VKAPI_METADATA_PLANE 4
+
+/**
+ * @file
+ * API-specific header for AV_HWDEVICE_TYPE_VKAPI.
+ */
+
+/**
+ * Allocated as AVHWDeviceContext.hwctx
+ */
+typedef struct VKAPIDeviceContext {
+/**
+ * Holds pointers to hardware specific functions
+ */
+vkil_api *ilapi;
+/**
+ * Internal functions definitions
+ */
+/**
+ * Get the hwprops reference from the AVFrame:data[3]
+ */
+int (*frame_ref_hwprops)(const AVFrame *frame, void *phw_surface_desc);
+/**
+ * Set the hwprops into AVFrame:data[3]
+ */
+int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface 
*hw_surface_desc);
+/**
+ * Get the hwprops from AVFrame:data[3]
+ */
+int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface 
*hw_surface_desc);
+/**
+ * Check if format is in an array
+ */
+int (*fmt_is_in)(int fmt, const int *fmts);
+/**
+ * Convert AVPixelFormat to VKAPI equivalent pixel format
+ */
+int (*av2vk_fmt)(enum AVPixelFormat pixel_format);
+/**
+ * Get no of buffer count reference in the hardware pool
+ */
+int (*get_pool_occupancy)(AVHWFramesContext *ctx);
+} VKAPIDeviceContext;


This structure has two uses:

* To be filled by the user when they already have an instance of the device and 
want to use it with libav*.
* To provide information to the user about an instance of the device created 
inside libav*.

To that end, the "ilapi" field makes sense to me (the user provides their vkil 
API handle), but they also need to provide some sort of reference to the actual device (a 
vkil_context handle, maybe?).

I don't think any of the other fields make sense; it looks like you are trying 
to expose some internals in a confusing way - why would a user creating this 
structure want to set those function pointers?


+/**
+ * Contains color information for hardware
+ */
+typedef struct VKAPIColorContext {
+enum AVColorRange range;
+enum AVColorPrimaries primaries;
+enum AVColorTransferCharacteristic trc;
+enum AVColorSpace space;
+enum AVChromaLocation chroma_location;
+} VKAPIColorContext;
+
+/**
+ * Allocated as AVHWFramesContext.hwctx
+ */
+typedef struct VKAPIFramesContext {
+/**
+ * Handle to a hardware frame context
+ */
+uint32_t handle;
+/**
+ * Hardware component port associated to the frame context
+ */
+   

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Michael Niedermayer
On Fri, Mar 12, 2021 at 04:59:16PM -0300, James Almer wrote:
> On 3/12/2021 4:46 PM, James Almer wrote:
> > On 3/12/2021 4:30 PM, Michael Niedermayer wrote:
> > > On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:
> > > > On 3/12/2021 1:32 PM, Michael Niedermayer wrote:
> > > > > On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
> > > > > > On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
> > > > > > > On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
> > > > > > > > On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> > > > > > > > > On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
> > > > > > > > > > On 2/21/2021 6:04 PM, James Almer wrote:
> > > > > > > > > > > On 2/21/2021 5:29 PM, Mark Thompson wrote:
> > > > > > > > > > > > On 21/02/2021 20:00, James Almer wrote:
> > > > > > > > > > > > > On 2/21/2021 4:13 PM, Mark Thompson wrote:
> > > > > > > > > > > > > > On 21/02/2021 17:35, James Almer wrote:
> > > > > > > > > > > > > > > This callback is functionally the same as 
> > > > > > > > > > > > > > > get_buffer2()
> > > > > > > > > > > > > > > is for decoders, and
> > > > > > > > > > > > > > > implements for the new encode API the 
> > > > > > > > > > > > > > > functionality of
> > > > > > > > > > > > > > > the old encode API had
> > > > > > > > > > > > > > > where the user could provide their own buffers.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Signed-off-by: James Almer 
> > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > > Used the names Lynne suggested this time, plus a 
> > > > > > > > > > > > > > > line
> > > > > > > > > > > > > > > about how the callback
> > > > > > > > > > > > > > > must be thread safe.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > >      
> > > > > > > > > > > > > > > libavcodec/avcodec.h
> > > > > > > > > > > > > > > | 45
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > >       libavcodec/codec.h   |  8 ---
> > > > > > > > > > > > > > >       libavcodec/encode.c  | 54
> > > > > > > > > > > > > > > +++-
> > > > > > > > > > > > > > >       libavcodec/encode.h  |  8 +++
> > > > > > > > > > > > > > >       libavcodec/options.c |  1 +
> > > > > > > > > > > > > > >       5 files changed, 112 insertions(+), 4 
> > > > > > > > > > > > > > > deletions(-)
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > diff --git a/libavcodec/avcodec.h 
> > > > > > > > > > > > > > > b/libavcodec/avcodec.h
> > > > > > > > > > > > > > > index 7dbf083a24..e60eb16ce1 100644
> > > > > > > > > > > > > > > --- a/libavcodec/avcodec.h
> > > > > > > > > > > > > > > +++ b/libavcodec/avcodec.h
> > > > > > > > > > > > > > > @@ -513,6 +513,11 @@ typedef struct 
> > > > > > > > > > > > > > > AVProducerReferenceTime {
> > > > > > > > > > > > > > >        */
> > > > > > > > > > > > > > >       #define AV_GET_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > > > > +/**
> > > > > > > > > > > > > > > + * The encoder will keep a reference to the 
> > > > > > > > > > > > > > > packet and
> > > > > > > > > > > > > > > may reuse it later.
> > > > > > > > > > > > > > > + */
> > > > > > > > > > > > > > > +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > >       struct AVCodecInternal;
> > > > > > > > > > > > > > >       /**
> > > > > > > > > > > > > > > @@ -2346,6 +2351,39 @@ typedef struct 
> > > > > > > > > > > > > > > AVCodecContext {
> > > > > > > > > > > > > > >        * - encoding: set by user
> > > > > > > > > > > > > > >        */
> > > > > > > > > > > > > > >       int export_side_data;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +    /**
> > > > > > > > > > > > > > > + * This callback is called at the beginning 
> > > > > > > > > > > > > > > of each
> > > > > > > > > > > > > > > packet to get a data
> > > > > > > > > > > > > > > + * buffer for it.
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * The following field will be set in the 
> > > > > > > > > > > > > > > packet
> > > > > > > > > > > > > > > before this callback is
> > > > > > > > > > > > > > > + * called:
> > > > > > > > > > > > > > > + * - size
> > > > > > > > > > > > > > > + * This callback must use the above value to
> > > > > > > > > > > > > > > calculate the required buffer size,
> > > > > > > > > > > > > > > + * which must padded by at least
> > > > > > > > > > > > > > > AV_INPUT_BUFFER_PADDING_SIZE bytes.
> > > > > > > > > > > > > > > + *
> > > > > > > > > > > > > > > + * This
> > > > > > > > > > > > > > > callback must fill
> > > > > > > > > > > > > > > the following fields
> > > > > > > > > > > > > > > in the packet:
> > > > > > > > > > > > > > > + * - data
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Is the data pointer allowed to be in write-only 
> > > > > > > > > > > > > > memory?
> > > > > > > 

[FFmpeg-devel] [PATCH] nvenc: Use frameIntervalP instead of max_b_frames for offsetting output dts

2021-03-12 Thread Martin Storsjö
If b-frames were enabled implicitly (if max_b_frames wasn't set by
the caller at all, since a0949d0bcb0eee2f3fffcf9a4810c0295d14c0dc),
we wouldn't offset dts at all, producing invalid pts/dts combinations
(causing loud warnings by ffmpeg, or muxer errors if passed without
an extra cleanup pass).

Instead use frameIntervalP for offsetting, which should always be
accurate.
---
 libavcodec/nvenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index a061dee880..fbf55ebc9d 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -1921,7 +1921,7 @@ static int nvenc_set_timestamp(AVCodecContext *avctx,
 pkt->pts = params->outputTimeStamp;
 pkt->dts = timestamp_queue_dequeue(ctx->timestamp_list);
 
-pkt->dts -= FFMAX(avctx->max_b_frames, 0) * FFMAX(avctx->ticks_per_frame, 
1);
+pkt->dts -= FFMAX(ctx->encode_config.frameIntervalP - 1, 0) * 
FFMAX(avctx->ticks_per_frame, 1);
 
 return 0;
 }
-- 
2.24.3 (Apple Git-128)

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

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

Re: [FFmpeg-devel] [PATCH v2] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

2021-03-12 Thread Derek Buitenhuis
On 12/03/2021 18:49, Martin Storsjö wrote:
> LGTM. The commit message still talks only about the major brand though.

It seems at some point we added an unrelated test, seek-extra-mp4, which uses
an invalid dash-branded (under compatible brands) file generated by 
libavformat...
We mux correctly now, and for mos of git history, but it seems we maybe muxed
incorrectly at some point?

What's the best course of action here? Fix the sample with a few filename?
Drop compatible_brands (even though AFAIK, this is the correct way to handle
this)?

(Sample added in 37e8edc9f51545ad91cbdf7dbe796af93f011abe by Dale - I don't
know how he made it, but it was made with libavformat.)

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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Michael Niedermayer:
>> On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
>>> On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
 On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
> On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
>> On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
>>> On 2/21/2021 6:04 PM, James Almer wrote:
 On 2/21/2021 5:29 PM, Mark Thompson wrote:
> On 21/02/2021 20:00, James Almer wrote:
>> On 2/21/2021 4:13 PM, Mark Thompson wrote:
>>> On 21/02/2021 17:35, James Almer wrote:
 This callback is functionally the same as get_buffer2()
 is for decoders, and
 implements for the new encode API the functionality of
 the old encode API had
 where the user could provide their own buffers.

 Signed-off-by: James Almer 
 ---
 Used the names Lynne suggested this time, plus a line
 about how the callback
 must be thread safe.

     libavcodec/avcodec.h | 45 
     libavcodec/codec.h   |  8 ---
     libavcodec/encode.c  | 54
 +++-
     libavcodec/encode.h  |  8 +++
     libavcodec/options.c |  1 +
     5 files changed, 112 insertions(+), 4 deletions(-)

 diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
 index 7dbf083a24..e60eb16ce1 100644
 --- a/libavcodec/avcodec.h
 +++ b/libavcodec/avcodec.h
 @@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
      */
     #define AV_GET_BUFFER_FLAG_REF (1 << 0)
 +/**
 + * The encoder will keep a reference to the packet and
 may reuse it later.
 + */
 +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
 +
     struct AVCodecInternal;
     /**
 @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
      * - encoding: set by user
      */
     int export_side_data;
 +
 +    /**
 + * This callback is called at the beginning of each
 packet to get a data
 + * buffer for it.
 + *
 + * The following field will be set in the packet
 before this callback is
 + * called:
 + * - size
 + * This callback must use the above value to
 calculate the required buffer size,
 + * which must padded by at least
 AV_INPUT_BUFFER_PADDING_SIZE bytes.
 + *
 + * This callback must fill the following fields in the packet:
 + * - data
>>>
>>> Is the data pointer allowed to be in write-only memory?
>>
>> I'm not sure what the use case for this would be, so probably no?
>
> The two use-cases I see for this API are:
>
> * You want to avoid a copy when combining the output with something
> else.  E.g. you pass a pointer to the block of memory following
> where you are going to put your header data (for something you are
> going to send over the network, say).
>
> * You want to avoid a copy when passing the output directly to
> something external.  E.g. you pass a pointer to a memory-mapped
> device buffer (such as a V4L2 buffer, say).
>
> In the second case, write-only memory on an external device seems
> possible, as does memory which is, say, readable but uncached, so
> reading it is a really bad idea.

 Allowing the second case would depend on how encoders behave. Some may
 attempt to read data already written to the output packet. It's not 
 like
 all of them allocate the packet, do a memcpy from an internal buffer,
 then return.
 There is also the flag meant to signal that the encoder will keep a
 reference to the packet around, which more or less implies it will be
 read later in the encoding process.

 The doxy for avcodec_encode_video2(), which allowed the user to provide
 their own buffers in the output packet, does not mention any kind of
 requirement for the data pointer, so I don't think we can say it's an
 allowed scenario here either.

>
>>> Does it have any alignment requirements?
>>
>> No, just padding. AVPacket doesn't require alignment for the payload.
>
> I think say that explicitly.  avcodec_default_get_encoder_buffer()
> does give you 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 5:11 PM, Andreas Rheinhardt wrote:

James Almer:

On 3/12/2021 4:46 PM, James Almer wrote:

On 3/12/2021 4:30 PM, Michael Niedermayer wrote:

On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:

On 3/12/2021 1:32 PM, Michael Niedermayer wrote:

On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:

On 3/11/2021 1:35 PM, Michael Niedermayer wrote:

On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:

On 3/10/2021 5:18 PM, Michael Niedermayer wrote:

On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2()
is for decoders, and
implements for the new encode API the functionality of
the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line
about how the callback
must be thread safe.

       libavcodec/avcodec.h | 45

       libavcodec/codec.h   |  8 ---
       libavcodec/encode.c  | 54
+++-
       libavcodec/encode.h  |  8 +++
       libavcodec/options.c |  1 +
       5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct
AVProducerReferenceTime {
        */
       #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and
may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
       struct AVCodecInternal;
       /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
        * - encoding: set by user
        */
       int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each
packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet
before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to
calculate the required buffer size,
+ * which must padded by at least
AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in
the packet:
+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so
probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with
something
else.  E.g. you pass a pointer to the block of memory following
where you are going to put your header data (for something
you are
going to send over the network, say).

* You want to avoid a copy when passing the output directly to
something external.  E.g. you pass a pointer to a memory-mapped
device buffer (such as a V4L2 buffer, say).

In the second case, write-only memory on an external device
seems
possible, as does memory which is, say, readable but
uncached, so
reading it is a really bad idea.


Allowing the second case would depend on how encoders behave.
Some may
attempt to read data already written to the output packet.
It's not like
all of them allocate the packet, do a memcpy from an internal
buffer,
then return.
There is also the flag meant to signal that the encoder will
keep a
reference to the packet around, which more or less implies it
will be
read later in the encoding process.

The doxy for avcodec_encode_video2(), which allowed the user
to provide
their own buffers in the output packet, does not mention any
kind of
requirement for the data pointer, so I don't think we can say
it's an
allowed scenario here either.




Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for
the payload.


I think say that explicitly.
avcodec_default_get_encoder_buffer()
does give you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data
pointer" add
anything of value to the doxy? If i don't mention any kind of
alignment
requirement, it's because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind,
like the
padding and the need for an AVBufferRef. But if you think
it's worth
adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef
structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(),
and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then
get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of
providing a buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in
flags then the packet may be reused
+ * 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Andreas Rheinhardt
James Almer:
> On 3/12/2021 4:46 PM, James Almer wrote:
>> On 3/12/2021 4:30 PM, Michael Niedermayer wrote:
>>> On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:
 On 3/12/2021 1:32 PM, Michael Niedermayer wrote:
> On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
>> On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
>>> On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
 On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
>> On 2/21/2021 6:04 PM, James Almer wrote:
>>> On 2/21/2021 5:29 PM, Mark Thompson wrote:
 On 21/02/2021 20:00, James Almer wrote:
> On 2/21/2021 4:13 PM, Mark Thompson wrote:
>> On 21/02/2021 17:35, James Almer wrote:
>>> This callback is functionally the same as get_buffer2()
>>> is for decoders, and
>>> implements for the new encode API the functionality of
>>> the old encode API had
>>> where the user could provide their own buffers.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>> Used the names Lynne suggested this time, plus a line
>>> about how the callback
>>> must be thread safe.
>>>
>>>       libavcodec/avcodec.h | 45
>>> 
>>>       libavcodec/codec.h   |  8 ---
>>>       libavcodec/encode.c  | 54
>>> +++-
>>>       libavcodec/encode.h  |  8 +++
>>>       libavcodec/options.c |  1 +
>>>       5 files changed, 112 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 7dbf083a24..e60eb16ce1 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -513,6 +513,11 @@ typedef struct
>>> AVProducerReferenceTime {
>>>        */
>>>       #define AV_GET_BUFFER_FLAG_REF (1 << 0)
>>> +/**
>>> + * The encoder will keep a reference to the packet and
>>> may reuse it later.
>>> + */
>>> +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
>>> +
>>>       struct AVCodecInternal;
>>>       /**
>>> @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
>>>        * - encoding: set by user
>>>        */
>>>       int export_side_data;
>>> +
>>> +    /**
>>> + * This callback is called at the beginning of each
>>> packet to get a data
>>> + * buffer for it.
>>> + *
>>> + * The following field will be set in the packet
>>> before this callback is
>>> + * called:
>>> + * - size
>>> + * This callback must use the above value to
>>> calculate the required buffer size,
>>> + * which must padded by at least
>>> AV_INPUT_BUFFER_PADDING_SIZE bytes.
>>> + *
>>> + * This callback must fill the following fields in
>>> the packet:
>>> + * - data
>>
>> Is the data pointer allowed to be in write-only memory?
>
> I'm not sure what the use case for this would be, so
> probably no?

 The two use-cases I see for this API are:

 * You want to avoid a copy when combining the output with
 something
 else.  E.g. you pass a pointer to the block of memory following
 where you are going to put your header data (for something
 you are
 going to send over the network, say).

 * You want to avoid a copy when passing the output directly to
 something external.  E.g. you pass a pointer to a memory-mapped
 device buffer (such as a V4L2 buffer, say).

 In the second case, write-only memory on an external device
 seems
 possible, as does memory which is, say, readable but
 uncached, so
 reading it is a really bad idea.
>>>
>>> Allowing the second case would depend on how encoders behave.
>>> Some may
>>> attempt to read data already written to the output packet.
>>> It's not like
>>> all of them allocate the packet, do a memcpy from an internal
>>> buffer,
>>> then return.
>>> There is also the flag meant to signal that the encoder will
>>> keep a
>>> reference to the packet around, which 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 4:42 PM, Michael Niedermayer wrote:

On Fri, Mar 12, 2021 at 08:30:23PM +0100, Michael Niedermayer wrote:

On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:

On 3/12/2021 1:32 PM, Michael Niedermayer wrote:

On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:

On 3/11/2021 1:35 PM, Michael Niedermayer wrote:

On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:

On 3/10/2021 5:18 PM, Michael Niedermayer wrote:

On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2()
is for decoders, and
implements for the new encode API the functionality of
the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line
about how the callback
must be thread safe.

  libavcodec/avcodec.h | 45 
  libavcodec/codec.h   |  8 ---
  libavcodec/encode.c  | 54
+++-
  libavcodec/encode.h  |  8 +++
  libavcodec/options.c |  1 +
  5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
   */
  #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and
may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
  struct AVCodecInternal;
  /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
   * - encoding: set by user
   */
  int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each
packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet
before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to
calculate the required buffer size,
+ * which must padded by at least
AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with something
else.  E.g. you pass a pointer to the block of memory following
where you are going to put your header data (for something you are
going to send over the network, say).

* You want to avoid a copy when passing the output directly to
something external.  E.g. you pass a pointer to a memory-mapped
device buffer (such as a V4L2 buffer, say).

In the second case, write-only memory on an external device seems
possible, as does memory which is, say, readable but uncached, so
reading it is a really bad idea.


Allowing the second case would depend on how encoders behave. Some may
attempt to read data already written to the output packet. It's not like
all of them allocate the packet, do a memcpy from an internal buffer,
then return.
There is also the flag meant to signal that the encoder will keep a
reference to the packet around, which more or less implies it will be
read later in the encoding process.

The doxy for avcodec_encode_video2(), which allowed the user to provide
their own buffers in the output packet, does not mention any kind of
requirement for the data pointer, so I don't think we can say it's an
allowed scenario here either.




Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for the payload.


I think say that explicitly.  avcodec_default_get_encoder_buffer()
does give you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data pointer" add
anything of value to the doxy? If i don't mention any kind of alignment
requirement, it's because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind, like the
padding and the need for an AVBufferRef. But if you think it's worth
adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef
structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(),
and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then
get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of
providing a buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in
flags then the packet may be reused
+ * (read and/or written to if it is writable) 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 4:46 PM, James Almer wrote:

On 3/12/2021 4:30 PM, Michael Niedermayer wrote:

On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:

On 3/12/2021 1:32 PM, Michael Niedermayer wrote:

On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:

On 3/11/2021 1:35 PM, Michael Niedermayer wrote:

On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:

On 3/10/2021 5:18 PM, Michael Niedermayer wrote:

On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2()
is for decoders, and
implements for the new encode API the functionality of
the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line
about how the callback
must be thread safe.

      libavcodec/avcodec.h | 45 


      libavcodec/codec.h   |  8 ---
      libavcodec/encode.c  | 54
+++-
      libavcodec/encode.h  |  8 +++
      libavcodec/options.c |  1 +
      5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
       */
      #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and
may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
      struct AVCodecInternal;
      /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
       * - encoding: set by user
       */
      int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each
packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet
before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to
calculate the required buffer size,
+ * which must padded by at least
AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in 
the packet:

+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so 
probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with 
something

else.  E.g. you pass a pointer to the block of memory following
where you are going to put your header data (for something 
you are

going to send over the network, say).

* You want to avoid a copy when passing the output directly to
something external.  E.g. you pass a pointer to a memory-mapped
device buffer (such as a V4L2 buffer, say).

In the second case, write-only memory on an external device 
seems
possible, as does memory which is, say, readable but 
uncached, so

reading it is a really bad idea.


Allowing the second case would depend on how encoders behave. 
Some may
attempt to read data already written to the output packet. 
It's not like
all of them allocate the packet, do a memcpy from an internal 
buffer,

then return.
There is also the flag meant to signal that the encoder will 
keep a
reference to the packet around, which more or less implies it 
will be

read later in the encoding process.

The doxy for avcodec_encode_video2(), which allowed the user 
to provide
their own buffers in the output packet, does not mention any 
kind of
requirement for the data pointer, so I don't think we can say 
it's an

allowed scenario here either.




Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for the 
payload.


I think say that explicitly.  
avcodec_default_get_encoder_buffer()

does give you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data 
pointer" add
anything of value to the doxy? If i don't mention any kind of 
alignment

requirement, it's because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind, like 
the
padding and the need for an AVBufferRef. But if you think it's 
worth

adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef
structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(),
and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then
get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of
providing a buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in
flags then the packet may be reused
+ * (read and/or written to if it is writable) 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 4:30 PM, Michael Niedermayer wrote:

On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:

On 3/12/2021 1:32 PM, Michael Niedermayer wrote:

On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:

On 3/11/2021 1:35 PM, Michael Niedermayer wrote:

On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:

On 3/10/2021 5:18 PM, Michael Niedermayer wrote:

On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2()
is for decoders, and
implements for the new encode API the functionality of
the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line
about how the callback
must be thread safe.

  libavcodec/avcodec.h | 45 
  libavcodec/codec.h   |  8 ---
  libavcodec/encode.c  | 54
+++-
  libavcodec/encode.h  |  8 +++
  libavcodec/options.c |  1 +
  5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
   */
  #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and
may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
  struct AVCodecInternal;
  /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
   * - encoding: set by user
   */
  int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each
packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet
before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to
calculate the required buffer size,
+ * which must padded by at least
AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with something
else.  E.g. you pass a pointer to the block of memory following
where you are going to put your header data (for something you are
going to send over the network, say).

* You want to avoid a copy when passing the output directly to
something external.  E.g. you pass a pointer to a memory-mapped
device buffer (such as a V4L2 buffer, say).

In the second case, write-only memory on an external device seems
possible, as does memory which is, say, readable but uncached, so
reading it is a really bad idea.


Allowing the second case would depend on how encoders behave. Some may
attempt to read data already written to the output packet. It's not like
all of them allocate the packet, do a memcpy from an internal buffer,
then return.
There is also the flag meant to signal that the encoder will keep a
reference to the packet around, which more or less implies it will be
read later in the encoding process.

The doxy for avcodec_encode_video2(), which allowed the user to provide
their own buffers in the output packet, does not mention any kind of
requirement for the data pointer, so I don't think we can say it's an
allowed scenario here either.




Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for the payload.


I think say that explicitly.  avcodec_default_get_encoder_buffer()
does give you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data pointer" add
anything of value to the doxy? If i don't mention any kind of alignment
requirement, it's because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind, like the
padding and the need for an AVBufferRef. But if you think it's worth
adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef
structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(),
and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then
get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of
providing a buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in
flags then the packet may be reused
+ * (read and/or written to if it is writable) later
by libavcodec.
+ *
+ * This callback must be thread-safe, as 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Michael Niedermayer
On Fri, Mar 12, 2021 at 08:30:23PM +0100, Michael Niedermayer wrote:
> On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:
> > On 3/12/2021 1:32 PM, Michael Niedermayer wrote:
> > > On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
> > > > On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
> > > > > On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
> > > > > > On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> > > > > > > On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
> > > > > > > > On 2/21/2021 6:04 PM, James Almer wrote:
> > > > > > > > > On 2/21/2021 5:29 PM, Mark Thompson wrote:
> > > > > > > > > > On 21/02/2021 20:00, James Almer wrote:
> > > > > > > > > > > On 2/21/2021 4:13 PM, Mark Thompson wrote:
> > > > > > > > > > > > On 21/02/2021 17:35, James Almer wrote:
> > > > > > > > > > > > > This callback is functionally the same as 
> > > > > > > > > > > > > get_buffer2()
> > > > > > > > > > > > > is for decoders, and
> > > > > > > > > > > > > implements for the new encode API the functionality of
> > > > > > > > > > > > > the old encode API had
> > > > > > > > > > > > > where the user could provide their own buffers.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Signed-off-by: James Almer 
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > Used the names Lynne suggested this time, plus a line
> > > > > > > > > > > > > about how the callback
> > > > > > > > > > > > > must be thread safe.
> > > > > > > > > > > > > 
> > > > > > > > > > > > >  libavcodec/avcodec.h | 45 
> > > > > > > > > > > > > 
> > > > > > > > > > > > >  libavcodec/codec.h   |  8 ---
> > > > > > > > > > > > >  libavcodec/encode.c  | 54
> > > > > > > > > > > > > +++-
> > > > > > > > > > > > >  libavcodec/encode.h  |  8 +++
> > > > > > > > > > > > >  libavcodec/options.c |  1 +
> > > > > > > > > > > > >  5 files changed, 112 insertions(+), 4 
> > > > > > > > > > > > > deletions(-)
> > > > > > > > > > > > > 
> > > > > > > > > > > > > diff --git a/libavcodec/avcodec.h 
> > > > > > > > > > > > > b/libavcodec/avcodec.h
> > > > > > > > > > > > > index 7dbf083a24..e60eb16ce1 100644
> > > > > > > > > > > > > --- a/libavcodec/avcodec.h
> > > > > > > > > > > > > +++ b/libavcodec/avcodec.h
> > > > > > > > > > > > > @@ -513,6 +513,11 @@ typedef struct 
> > > > > > > > > > > > > AVProducerReferenceTime {
> > > > > > > > > > > > >   */
> > > > > > > > > > > > >  #define AV_GET_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > > +/**
> > > > > > > > > > > > > + * The encoder will keep a reference to the packet 
> > > > > > > > > > > > > and
> > > > > > > > > > > > > may reuse it later.
> > > > > > > > > > > > > + */
> > > > > > > > > > > > > +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > > +
> > > > > > > > > > > > >  struct AVCodecInternal;
> > > > > > > > > > > > >  /**
> > > > > > > > > > > > > @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
> > > > > > > > > > > > >   * - encoding: set by user
> > > > > > > > > > > > >   */
> > > > > > > > > > > > >  int export_side_data;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +    /**
> > > > > > > > > > > > > + * This callback is called at the beginning of 
> > > > > > > > > > > > > each
> > > > > > > > > > > > > packet to get a data
> > > > > > > > > > > > > + * buffer for it.
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * The following field will be set in the packet
> > > > > > > > > > > > > before this callback is
> > > > > > > > > > > > > + * called:
> > > > > > > > > > > > > + * - size
> > > > > > > > > > > > > + * This callback must use the above value to
> > > > > > > > > > > > > calculate the required buffer size,
> > > > > > > > > > > > > + * which must padded by at least
> > > > > > > > > > > > > AV_INPUT_BUFFER_PADDING_SIZE bytes.
> > > > > > > > > > > > > + *
> > > > > > > > > > > > > + * This callback must fill the following fields 
> > > > > > > > > > > > > in the packet:
> > > > > > > > > > > > > + * - data
> > > > > > > > > > > > 
> > > > > > > > > > > > Is the data pointer allowed to be in write-only memory?
> > > > > > > > > > > 
> > > > > > > > > > > I'm not sure what the use case for this would be, so 
> > > > > > > > > > > probably no?
> > > > > > > > > > 
> > > > > > > > > > The two use-cases I see for this API are:
> > > > > > > > > > 
> > > > > > > > > > * You want to avoid a copy when combining the output with 
> > > > > > > > > > something
> > > > > > > > > > else.  E.g. you pass a pointer to the block of memory 
> > > > > > > > > > following
> > > > > > > > > > where you are going to put your header data (for something 
> > > > > > > > > > you are
> > > > > > > > > > going to send over the network, say).
> > > > > > > > > > 
> > > > > > > > > > * You want to avoid a copy when passing 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Michael Niedermayer
On Fri, Mar 12, 2021 at 02:03:52PM -0300, James Almer wrote:
> On 3/12/2021 1:32 PM, Michael Niedermayer wrote:
> > On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
> > > On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
> > > > On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
> > > > > On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> > > > > > On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
> > > > > > > On 2/21/2021 6:04 PM, James Almer wrote:
> > > > > > > > On 2/21/2021 5:29 PM, Mark Thompson wrote:
> > > > > > > > > On 21/02/2021 20:00, James Almer wrote:
> > > > > > > > > > On 2/21/2021 4:13 PM, Mark Thompson wrote:
> > > > > > > > > > > On 21/02/2021 17:35, James Almer wrote:
> > > > > > > > > > > > This callback is functionally the same as get_buffer2()
> > > > > > > > > > > > is for decoders, and
> > > > > > > > > > > > implements for the new encode API the functionality of
> > > > > > > > > > > > the old encode API had
> > > > > > > > > > > > where the user could provide their own buffers.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: James Almer 
> > > > > > > > > > > > ---
> > > > > > > > > > > > Used the names Lynne suggested this time, plus a line
> > > > > > > > > > > > about how the callback
> > > > > > > > > > > > must be thread safe.
> > > > > > > > > > > > 
> > > > > > > > > > > >  libavcodec/avcodec.h | 45 
> > > > > > > > > > > > 
> > > > > > > > > > > >  libavcodec/codec.h   |  8 ---
> > > > > > > > > > > >  libavcodec/encode.c  | 54
> > > > > > > > > > > > +++-
> > > > > > > > > > > >  libavcodec/encode.h  |  8 +++
> > > > > > > > > > > >  libavcodec/options.c |  1 +
> > > > > > > > > > > >  5 files changed, 112 insertions(+), 4 deletions(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > > > > > > > index 7dbf083a24..e60eb16ce1 100644
> > > > > > > > > > > > --- a/libavcodec/avcodec.h
> > > > > > > > > > > > +++ b/libavcodec/avcodec.h
> > > > > > > > > > > > @@ -513,6 +513,11 @@ typedef struct 
> > > > > > > > > > > > AVProducerReferenceTime {
> > > > > > > > > > > >   */
> > > > > > > > > > > >  #define AV_GET_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > +/**
> > > > > > > > > > > > + * The encoder will keep a reference to the packet and
> > > > > > > > > > > > may reuse it later.
> > > > > > > > > > > > + */
> > > > > > > > > > > > +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > > > +
> > > > > > > > > > > >  struct AVCodecInternal;
> > > > > > > > > > > >  /**
> > > > > > > > > > > > @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
> > > > > > > > > > > >   * - encoding: set by user
> > > > > > > > > > > >   */
> > > > > > > > > > > >  int export_side_data;
> > > > > > > > > > > > +
> > > > > > > > > > > > +    /**
> > > > > > > > > > > > + * This callback is called at the beginning of each
> > > > > > > > > > > > packet to get a data
> > > > > > > > > > > > + * buffer for it.
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * The following field will be set in the packet
> > > > > > > > > > > > before this callback is
> > > > > > > > > > > > + * called:
> > > > > > > > > > > > + * - size
> > > > > > > > > > > > + * This callback must use the above value to
> > > > > > > > > > > > calculate the required buffer size,
> > > > > > > > > > > > + * which must padded by at least
> > > > > > > > > > > > AV_INPUT_BUFFER_PADDING_SIZE bytes.
> > > > > > > > > > > > + *
> > > > > > > > > > > > + * This callback must fill the following fields in 
> > > > > > > > > > > > the packet:
> > > > > > > > > > > > + * - data
> > > > > > > > > > > 
> > > > > > > > > > > Is the data pointer allowed to be in write-only memory?
> > > > > > > > > > 
> > > > > > > > > > I'm not sure what the use case for this would be, so 
> > > > > > > > > > probably no?
> > > > > > > > > 
> > > > > > > > > The two use-cases I see for this API are:
> > > > > > > > > 
> > > > > > > > > * You want to avoid a copy when combining the output with 
> > > > > > > > > something
> > > > > > > > > else.  E.g. you pass a pointer to the block of memory 
> > > > > > > > > following
> > > > > > > > > where you are going to put your header data (for something 
> > > > > > > > > you are
> > > > > > > > > going to send over the network, say).
> > > > > > > > > 
> > > > > > > > > * You want to avoid a copy when passing the output directly to
> > > > > > > > > something external.  E.g. you pass a pointer to a 
> > > > > > > > > memory-mapped
> > > > > > > > > device buffer (such as a V4L2 buffer, say).
> > > > > > > > > 
> > > > > > > > > In the second case, write-only memory on an external device 
> > > > > > > > > seems
> > > > > > > > > possible, as does memory which is, say, readable but 

Re: [FFmpeg-devel] [PATCH] avutil/frame: ensure the frame is writable in av_frame_copy()

2021-03-12 Thread James Almer

On 3/10/2021 2:13 PM, James Almer wrote:

Signed-off-by: James Almer 
---
  libavutil/frame.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index eab51b6a32..ec79d053e1 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -800,6 +800,8 @@ int av_frame_copy(AVFrame *dst, const AVFrame *src)
  {
  if (dst->format != src->format || dst->format < 0)
  return AVERROR(EINVAL);
+if (!av_frame_is_writable(dst))
+return AVERROR(EINVAL);
  
  if (dst->width > 0 && dst->height > 0)

  return frame_copy_video(dst, src);


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

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

Re: [FFmpeg-devel] [PATCH v2] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

2021-03-12 Thread Martin Storsjö

On Fri, 12 Mar 2021, Derek Buitenhuis wrote:


Files with the "dash" major brand are guaranteed to only have a single
initialization range for the whole file. We can check this and stop
appropriately - which is useful, as the existing check to see if the
offset reaches the end of the file is not sufficient. For example,
Premiere creates valid self-initializing ISOBMFF files that contain
a single 'uuid' box appended at the end, after the last 'moof', and
this check does not catch that, which causes, for example, probing
to traverse the entire file (every single 'moof'), which is extremely
slow over a network, which is the main place these self-initializing
fragmented ISOBMFF files would be used. This is the same behavior that
was addressed in 2ff3c466eca66bb8eb32bb41a4ce70fe285e3ea0 for live-style
fragmented files.

Signed-off-by: Derek Buitenhuis 
---
Updated the commit message too, to reflect that the files are actually
a YouTube-muxed file that was opened in Premiere, which appends XMP
metadata in a 'uuid' box to files it opens. Gross.
---
libavformat/mov.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 23b0ead01e..0eb52ab6e6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5052,6 +5052,8 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
AVStream *ref_st = NULL;
MOVStreamContext *sc, *ref_sc = NULL;
AVRational timescale;
+AVDictionaryEntry *major_brand = av_dict_get(c->fc->metadata, 
"major_brand", NULL, AV_DICT_MATCH_CASE);
+AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata, 
"compatible_brands", NULL, AV_DICT_MATCH_CASE);

version = avio_r8(pb);
if (version > 1) {
@@ -5120,7 +5122,13 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
sc->has_sidx = 1;

// See if the remaining bytes are just an mfra which we can ignore.
-is_complete = offset == stream_size;
+//
+// Also check to see if the file is Self-initializing, which guarantees
+// only a single initialization range is present for the whole file.
+// See: ISO-IEC 23009-1 Section 6.3.5.
+is_complete = offset == stream_size ||
+  (major_brand && !strncmp(major_brand->value, "dash", 4)) ||
+  (compatible_brands && strstr(compatible_brands->value, 
"dash"));
if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) {
int64_t ret;
int64_t original_pos = avio_tell(pb);
--
2.30.0


LGTM. The commit message still talks only about the major brand though.

// Martin

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

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

[FFmpeg-devel] [PATCH] Fix wrong time_base bug

2021-03-12 Thread Donough Liu
Set time_base to inv of frame rate is only valid when input video have fixed 
frame rate.

Signed-off-by: Donough Liu 
---
 doc/examples/transcoding.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/examples/transcoding.c b/doc/examples/transcoding.c
index 5aff08c135..d6fed63421 100644
--- a/doc/examples/transcoding.c
+++ b/doc/examples/transcoding.c
@@ -170,7 +170,7 @@ static int open_output_file(const char *filename)
 else
 enc_ctx->pix_fmt = dec_ctx->pix_fmt;
 /* video time_base can be set to whatever is handy and 
supported by encoder */
-enc_ctx->time_base = av_inv_q(dec_ctx->framerate);
+enc_ctx->time_base = dec_ctx->time_base;
 } else {
 enc_ctx->sample_rate = dec_ctx->sample_rate;
 enc_ctx->channel_layout = dec_ctx->channel_layout;
-- 
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] Fix wrong time_base bug

2021-03-12 Thread ldm0
From: ldm0 

Set time_base to inv of frame rate is only valid when input video have fixed 
frame rate.

Signed-off-by: ldm0 
---
 doc/examples/transcoding.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/examples/transcoding.c b/doc/examples/transcoding.c
index 5aff08c135..d6fed63421 100644
--- a/doc/examples/transcoding.c
+++ b/doc/examples/transcoding.c
@@ -170,7 +170,7 @@ static int open_output_file(const char *filename)
 else
 enc_ctx->pix_fmt = dec_ctx->pix_fmt;
 /* video time_base can be set to whatever is handy and 
supported by encoder */
-enc_ctx->time_base = av_inv_q(dec_ctx->framerate);
+enc_ctx->time_base = dec_ctx->time_base;
 } else {
 enc_ctx->sample_rate = dec_ctx->sample_rate;
 enc_ctx->channel_layout = dec_ctx->channel_layout;
-- 
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 1/2] avfilter/vf_uspp: Fix leak of qp-table on error

2021-03-12 Thread Michael Niedermayer
On Fri, Mar 12, 2021 at 03:24:06PM +0100, Andreas Rheinhardt wrote:
> Fixes Coverity issue #1473500.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---

should be ok



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

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


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

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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: Reindentation

2021-03-12 Thread James Almer

On 3/12/2021 1:54 PM, Andreas Rheinhardt wrote:

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/decode.c | 49 ++---
  1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 73cc3def5f..495e9e8b23 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1723,7 +1723,6 @@ static int add_metadata_from_side_data(const AVPacket 
*avpkt, AVFrame *frame)
  int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
  {
  AVPacket *pkt = avctx->internal->last_pkt_props;
-int i;
  static const struct {
  enum AVPacketSideDataType packet;
  enum AVFrameSideDataType frame;
@@ -1744,36 +1743,36 @@ int ff_decode_frame_props(AVCodecContext *avctx, 
AVFrame *frame)
  av_fifo_generic_read(avctx->internal->pkt_props,
   pkt, sizeof(*pkt), NULL);
  
-frame->pts = pkt->pts;

+frame->pts = pkt->pts;
  #if FF_API_PKT_PTS
  FF_DISABLE_DEPRECATION_WARNINGS
-frame->pkt_pts = pkt->pts;
+frame->pkt_pts = pkt->pts;
  FF_ENABLE_DEPRECATION_WARNINGS
  #endif
-frame->pkt_pos  = pkt->pos;
-frame->pkt_duration = pkt->duration;
-frame->pkt_size = pkt->size;
-
-for (i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
-buffer_size_t size;
-uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, 
);
-if (packet_sd) {
-AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
-   sd[i].frame,
-   size);
-if (!frame_sd)
-return AVERROR(ENOMEM);
-
-memcpy(frame_sd->data, packet_sd, size);
-}
+frame->pkt_pos  = pkt->pos;
+frame->pkt_duration = pkt->duration;
+frame->pkt_size = pkt->size;
+
+for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
+buffer_size_t size;
+uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, );
+if (packet_sd) {
+AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
+   sd[i].frame,
+   size);
+if (!frame_sd)
+return AVERROR(ENOMEM);
+
+memcpy(frame_sd->data, packet_sd, size);
  }
-add_metadata_from_side_data(pkt, frame);
+}
+add_metadata_from_side_data(pkt, frame);
  
-if (pkt->flags & AV_PKT_FLAG_DISCARD) {

-frame->flags |= AV_FRAME_FLAG_DISCARD;
-} else {
-frame->flags = (frame->flags & ~AV_FRAME_FLAG_DISCARD);
-}
+if (pkt->flags & AV_PKT_FLAG_DISCARD) {
+frame->flags |= AV_FRAME_FLAG_DISCARD;
+} else {
+frame->flags = (frame->flags & ~AV_FRAME_FLAG_DISCARD);
+}
  frame->reordered_opaque = avctx->reordered_opaque;
  
  if (frame->color_primaries == AVCOL_PRI_UNSPECIFIED)


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

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/avpacket: add av_packet_resize()

2021-03-12 Thread James Almer

On 3/12/2021 2:09 PM, Marton Balint wrote:



On Thu, 11 Mar 2021, James Almer wrote:


On 3/11/2021 7:40 PM, Marton Balint wrote:



On Thu, 11 Mar 2021, James Almer wrote:


On 3/11/2021 1:11 PM, Marton Balint wrote:



On Thu, 11 Mar 2021, James Almer wrote:


This function acts as a replacement for both av_grow_packet() and
av_shrink_packet(), the latter which is now deprecated and will be 
removed as

it does not correctly handle non-writable packets.


I don't think this is a good idea, av_shrink_packet cannot fail, 
av_grow_packet can. By using the same function you are losing the 
information if the end result should be checked or not.


I'm not sure i follow. av_shrink_packet() is not being changed at 
all, just deprecated, scheduled for removal, and its use discouraged.


But why are you complicating code with mandatory return value checking?


Because unlike av_shrink_packet(), av_packet_resize() is more thorough 
when handling AVPackets and allows new usage scenarios that were not 
allowed with the former, thus it can fail.


Because as soon as a function returns an error, you have to check it, 
and forward it upwards.


Is error checking that much of a problem? I don't understand why it 
bothers people so much.




Maybe i should have split this in two, one to add av_packet_resize() 
and one to deprecate av_shrink_packet(), to avoid confusion.


In any case, the fact av_shrink_packet() cannot fail is the reason 
I'm getting rid of it. It's zeroing the padding without bothering to 
check if the packet is writable at all.


Add an assert to it then.


We only assert on internal bugs, not invalid arguments passed by 
callers to a public function. The asserts would need to be added above 
each av_shrink_packet() call.


There are a lot of cases when we assert for bad API usage. See 
av_frame_ref() or av_frame_move_ref(). We assert if the dst is not empty.


And that's only for our internal uses of the function. It does nothing 
to the issue of it being public API that can't properly handle 
AVPackets in their current form.


> Shrinking a non-writable packet seems bad API usage anyways.

I get a packet from a demuxer. It contains two independent portions 
(NALUs, OBUs, etc) i want to separate in order to feed them to 
something like a multi threaded decoder. And so I create a new 
reference to it, resulting in two packets pointing to the same data. I 
shrink one to only contain the first portion, and i add the required 
byte offset to the data pointer and subtract it to the size field on 
the second packet so it contains only the second portion.
The result if i use av_packet_resize() will be two valid, properly 
padded packets containing their respective expected data, but if i use 
av_shrink_packet(), the result will be the packet with the second 
portion featuring padding bytes worth of data zeroed at the start of 
its payload.


This looks like an unfortunate example, since you are:
- adding a reference to the packet, so you don't have to duplicate data
- and then want to add zero padding to the partial packet, so you will
duplicate data.
And I think the padding does not have to be zero for the partial packet.


The padding exists AFAIK because something, like an optimized bitstream 
reader that tries to process several bytes at the same time, may end up 
reading or writing pass the reported end of the buffer.
That means that if reading and it's not all zeroes, it could in theory 
have unpredictable results. Hence why everything always zeroes the 
padding, even av_shrink_packet().


And yes, what you describe is what some bitstream filters and the 
matroska demuxer do. They just create several packet references pointing 
to the same data buffer, but using different offsets for the data 
pointer. They all have "padding", but in many cases said padding is the 
beginning of the payload of another packet, and that's not ideal.






I'm sure there are other similar valid scenarios where attempting to 
shrink a non writable packet is not "bad API usage".




If you want to shrink a writable packet, then maybe you don't even 
need zero padding, because the buffer possibly already contains some 
defined value, and the main reason of zero padding is avoiding 
reading uninitialized data...


If i allocate a payload of size 1024, ultimately fill only 512 bytes, 
then resize it to that value (typical scenario in lavf demuxers), if i 
don't zero the bytes after the 512 offset then they will remain 
uninitialized.


I am not sure I see your point here, if the data after the padding is 
uninitalized, that is not a problem. I made a typo above, and meant 
non-writable packet in my comment. And my reasoning is that if a packet 
is non-writable, it already contains initialized data with a zero 
padding. If you shrink that by reducing pkt->size only, you will not 
have uninitialized data, only the padding will not be zeroed. And that 
is preferable to copying the data only for zeroing the padding, because 
- 

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_uspp: Fix leak of packet side data

2021-03-12 Thread Michael Niedermayer
On Fri, Mar 12, 2021 at 03:24:07PM +0100, Andreas Rheinhardt wrote:
> The uspp filter uses a special option ("no_bitstream") of
> the Snow encoder to suppress it from generating output.
> The filter therefore did not unref the packet after usage,
> believing it to be blank. But this is wrong, as the Snow encoder
> attaches quality stats side data to the packet.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---

LGTM


> If there were a fate test for this filter, this would have been found
> and fixed long ago.



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

Republics decline into democracies and democracies degenerate into
despotisms. -- 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] CUDA - make it work for multiple GPU architectures

2021-03-12 Thread Timo Rothenpieler

On 12.03.2021 16:14, Patrick Ecord wrote:

Tried that and ran configure and it failed with Option '--ptx (-ptx)' is not allowed 
when compiling for multiple GPU architectures"

So I removed the `-ptx` flag and I was able to run configure and make and make 
install without any errors.


FFmpeg embeds the ptx assembly code. Removing the -ptx option WILL 
breaks any and all CUDA filters.



Tested by converting Big Buck Bunny and it played fine.
ffmpeg -y -vsync 0 -hwaccel cuda -hwaccel_output_format cuda -i 
./Big_Buck_Bunny_1080_10s_30MB.mp4 -c:a copy -c:v h264_nvenc -b:v 5M output.mp4


This only works because you are not actually using any CUDA filters.
There is zero need for nvcc support for that commandline.

On top of that, just use clang and don't bother with the Nvidia SDK 
unless you are developing filters.




smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/avpacket: add av_packet_resize()

2021-03-12 Thread Marton Balint



On Thu, 11 Mar 2021, James Almer wrote:


On 3/11/2021 7:40 PM, Marton Balint wrote:



On Thu, 11 Mar 2021, James Almer wrote:


On 3/11/2021 1:11 PM, Marton Balint wrote:



On Thu, 11 Mar 2021, James Almer wrote:


This function acts as a replacement for both av_grow_packet() and
av_shrink_packet(), the latter which is now deprecated and will be 
removed as

it does not correctly handle non-writable packets.


I don't think this is a good idea, av_shrink_packet cannot fail, 
av_grow_packet can. By using the same function you are losing the 
information if the end result should be checked or not.


I'm not sure i follow. av_shrink_packet() is not being changed at all, 
just deprecated, scheduled for removal, and its use discouraged.


But why are you complicating code with mandatory return value checking?


Because unlike av_shrink_packet(), av_packet_resize() is more thorough 
when handling AVPackets and allows new usage scenarios that were not 
allowed with the former, thus it can fail.


Because as soon as a function returns an error, you have to check it, 
and forward it upwards.


Is error checking that much of a problem? I don't understand why it 
bothers people so much.




Maybe i should have split this in two, one to add av_packet_resize() 
and one to deprecate av_shrink_packet(), to avoid confusion.


In any case, the fact av_shrink_packet() cannot fail is the reason I'm 
getting rid of it. It's zeroing the padding without bothering to check 
if the packet is writable at all.


Add an assert to it then.


We only assert on internal bugs, not invalid arguments passed by callers 
to a public function. The asserts would need to be added above each 
av_shrink_packet() call.


There are a lot of cases when we assert for bad API usage. See 
av_frame_ref() or av_frame_move_ref(). We assert if the dst is not empty.


And that's only for our internal uses of the 
function. It does nothing to the issue of it being public API that can't 
properly handle AVPackets in their current form.


> Shrinking a non-writable packet seems bad API usage anyways.

I get a packet from a demuxer. It contains two independent portions 
(NALUs, OBUs, etc) i want to separate in order to feed them to something 
like a multi threaded decoder. And so I create a new reference to it, 
resulting in two packets pointing to the same data. I shrink one to only 
contain the first portion, and i add the required byte offset to the 
data pointer and subtract it to the size field on the second packet so 
it contains only the second portion.
The result if i use av_packet_resize() will be two valid, properly 
padded packets containing their respective expected data, but if i use 
av_shrink_packet(), the result will be the packet with the second 
portion featuring padding bytes worth of data zeroed at the start of its 
payload.


This looks like an unfortunate example, since you are:
- adding a reference to the packet, so you don't have to duplicate data
- and then want to add zero padding to the partial packet, so you will
duplicate data.
And I think the padding does not have to be zero for the partial packet.



I'm sure there are other similar valid scenarios where attempting to 
shrink a non writable packet is not "bad API usage".




If you want to shrink a writable packet, then maybe you don't even need 
zero padding, because the buffer possibly already contains some defined 
value, and the main reason of zero padding is avoiding reading 
uninitialized data...


If i allocate a payload of size 1024, ultimately fill only 512 bytes, 
then resize it to that value (typical scenario in lavf demuxers), if i 
don't zero the bytes after the 512 offset then they will remain 
uninitialized.


I am not sure I see your point here, if the data after the padding is 
uninitalized, that is not a problem. I made a typo above, and meant 
non-writable packet in my comment. And my reasoning is that if a packet is 
non-writable, it already contains initialized data with a zero padding. If 
you shrink that by reducing pkt->size only, you will not have 
uninitialized data, only the padding will not be zeroed. And that is 
preferable to copying the data only for zeroing the padding, because - as 
far as I know - the padding does not have to be zeroed, it only have to be 
initialized.


I agree that it is not nice that av_shrink_packet() writes something to 
the packet, people may not think about it and misuse it instead of 
directly decreaseing pkt->size when they need a partial packet. That is 
why I suggested the assert(). One might argue that it kind of breaks 
behaviour, but I'd say it does not break it too much, and writing to a 
non-writable packet was broken in the first place.


Alternatively we can change av_shrink_packet() to only zero the padding if 
the packet is writable, which has the benefit that it will do what people 
generally expect, no matter if you throw a writable or a non-writable 
packet to it.


A third alternative is to 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread James Almer

On 3/12/2021 1:32 PM, Michael Niedermayer wrote:

On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:

On 3/11/2021 1:35 PM, Michael Niedermayer wrote:

On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:

On 3/10/2021 5:18 PM, Michael Niedermayer wrote:

On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:

On 2/21/2021 6:04 PM, James Almer wrote:

On 2/21/2021 5:29 PM, Mark Thompson wrote:

On 21/02/2021 20:00, James Almer wrote:

On 2/21/2021 4:13 PM, Mark Thompson wrote:

On 21/02/2021 17:35, James Almer wrote:

This callback is functionally the same as get_buffer2()
is for decoders, and
implements for the new encode API the functionality of
the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
Used the names Lynne suggested this time, plus a line
about how the callback
must be thread safe.

     libavcodec/avcodec.h | 45 
     libavcodec/codec.h   |  8 ---
     libavcodec/encode.c  | 54
+++-
     libavcodec/encode.h  |  8 +++
     libavcodec/options.c |  1 +
     5 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
      */
     #define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and
may reuse it later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
     struct AVCodecInternal;
     /**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
      * - encoding: set by user
      */
     int export_side_data;
+
+    /**
+ * This callback is called at the beginning of each
packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet
before this callback is
+ * called:
+ * - size
+ * This callback must use the above value to
calculate the required buffer size,
+ * which must padded by at least
AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data


Is the data pointer allowed to be in write-only memory?


I'm not sure what the use case for this would be, so probably no?


The two use-cases I see for this API are:

* You want to avoid a copy when combining the output with something
else.  E.g. you pass a pointer to the block of memory following
where you are going to put your header data (for something you are
going to send over the network, say).

* You want to avoid a copy when passing the output directly to
something external.  E.g. you pass a pointer to a memory-mapped
device buffer (such as a V4L2 buffer, say).

In the second case, write-only memory on an external device seems
possible, as does memory which is, say, readable but uncached, so
reading it is a really bad idea.


Allowing the second case would depend on how encoders behave. Some may
attempt to read data already written to the output packet. It's not like
all of them allocate the packet, do a memcpy from an internal buffer,
then return.
There is also the flag meant to signal that the encoder will keep a
reference to the packet around, which more or less implies it will be
read later in the encoding process.

The doxy for avcodec_encode_video2(), which allowed the user to provide
their own buffers in the output packet, does not mention any kind of
requirement for the data pointer, so I don't think we can say it's an
allowed scenario here either.




Does it have any alignment requirements?


No, just padding. AVPacket doesn't require alignment for the payload.


I think say that explicitly.  avcodec_default_get_encoder_buffer()
does give you aligned memory, even though it isn't needed.


Would saying "There's no alignment requirement for the data pointer" add
anything of value to the doxy? If i don't mention any kind of alignment
requirement, it's because there isn't any, and it's implicit.
I listed the requirements the user needs to keep in mind, like the
padding and the need for an AVBufferRef. But if you think it's worth
adding, then sure.




+ * - buf must contain a pointer to an AVBufferRef
structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(),
and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then
get_encoder_buffer() must call
+ * avcodec_default_get_encoder_buffer() instead of
providing a buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in
flags then the packet may be reused
+ * (read and/or written to if it is writable) later
by libavcodec.
+ *
+ * This callback must be thread-safe, as when frame
multithreading is used, it may
+ * be called from multiple threads simultaneously.


Allowing simulatenous 

[FFmpeg-devel] [PATCH 2/2] avcodec/decode: Reindentation

2021-03-12 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/decode.c | 49 ++---
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 73cc3def5f..495e9e8b23 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1723,7 +1723,6 @@ static int add_metadata_from_side_data(const AVPacket 
*avpkt, AVFrame *frame)
 int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
 {
 AVPacket *pkt = avctx->internal->last_pkt_props;
-int i;
 static const struct {
 enum AVPacketSideDataType packet;
 enum AVFrameSideDataType frame;
@@ -1744,36 +1743,36 @@ int ff_decode_frame_props(AVCodecContext *avctx, 
AVFrame *frame)
 av_fifo_generic_read(avctx->internal->pkt_props,
  pkt, sizeof(*pkt), NULL);
 
-frame->pts = pkt->pts;
+frame->pts = pkt->pts;
 #if FF_API_PKT_PTS
 FF_DISABLE_DEPRECATION_WARNINGS
-frame->pkt_pts = pkt->pts;
+frame->pkt_pts = pkt->pts;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
-frame->pkt_pos  = pkt->pos;
-frame->pkt_duration = pkt->duration;
-frame->pkt_size = pkt->size;
-
-for (i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
-buffer_size_t size;
-uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, 
);
-if (packet_sd) {
-AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
-   sd[i].frame,
-   size);
-if (!frame_sd)
-return AVERROR(ENOMEM);
-
-memcpy(frame_sd->data, packet_sd, size);
-}
+frame->pkt_pos  = pkt->pos;
+frame->pkt_duration = pkt->duration;
+frame->pkt_size = pkt->size;
+
+for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
+buffer_size_t size;
+uint8_t *packet_sd = av_packet_get_side_data(pkt, sd[i].packet, );
+if (packet_sd) {
+AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
+   sd[i].frame,
+   size);
+if (!frame_sd)
+return AVERROR(ENOMEM);
+
+memcpy(frame_sd->data, packet_sd, size);
 }
-add_metadata_from_side_data(pkt, frame);
+}
+add_metadata_from_side_data(pkt, frame);
 
-if (pkt->flags & AV_PKT_FLAG_DISCARD) {
-frame->flags |= AV_FRAME_FLAG_DISCARD;
-} else {
-frame->flags = (frame->flags & ~AV_FRAME_FLAG_DISCARD);
-}
+if (pkt->flags & AV_PKT_FLAG_DISCARD) {
+frame->flags |= AV_FRAME_FLAG_DISCARD;
+} else {
+frame->flags = (frame->flags & ~AV_FRAME_FLAG_DISCARD);
+}
 frame->reordered_opaque = avctx->reordered_opaque;
 
 if (frame->color_primaries == AVCOL_PRI_UNSPECIFIED)
-- 
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 1/2] avcodec/decode: Remove always-true check

2021-03-12 Thread Andreas Rheinhardt
Forgotten in 1fd76277708cf83572ba243e98f9e848c652f83d.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/decode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index c7dbf7b791..73cc3def5f 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1744,7 +1744,6 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
*frame)
 av_fifo_generic_read(avctx->internal->pkt_props,
  pkt, sizeof(*pkt), NULL);
 
-if (pkt) {
 frame->pts = pkt->pts;
 #if FF_API_PKT_PTS
 FF_DISABLE_DEPRECATION_WARNINGS
@@ -1775,7 +1774,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
 } else {
 frame->flags = (frame->flags & ~AV_FRAME_FLAG_DISCARD);
 }
-}
 frame->reordered_opaque = avctx->reordered_opaque;
 
 if (frame->color_primaries == AVCOL_PRI_UNSPECIFIED)
-- 
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 v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Andreas Rheinhardt
Michael Niedermayer:
> On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
>> On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
>>> On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
 On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
>> On 2/21/2021 6:04 PM, James Almer wrote:
>>> On 2/21/2021 5:29 PM, Mark Thompson wrote:
 On 21/02/2021 20:00, James Almer wrote:
> On 2/21/2021 4:13 PM, Mark Thompson wrote:
>> On 21/02/2021 17:35, James Almer wrote:
>>> This callback is functionally the same as get_buffer2()
>>> is for decoders, and
>>> implements for the new encode API the functionality of
>>> the old encode API had
>>> where the user could provide their own buffers.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>> Used the names Lynne suggested this time, plus a line
>>> about how the callback
>>> must be thread safe.
>>>
>>>     libavcodec/avcodec.h | 45 
>>>     libavcodec/codec.h   |  8 ---
>>>     libavcodec/encode.c  | 54
>>> +++-
>>>     libavcodec/encode.h  |  8 +++
>>>     libavcodec/options.c |  1 +
>>>     5 files changed, 112 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 7dbf083a24..e60eb16ce1 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
>>>      */
>>>     #define AV_GET_BUFFER_FLAG_REF (1 << 0)
>>> +/**
>>> + * The encoder will keep a reference to the packet and
>>> may reuse it later.
>>> + */
>>> +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
>>> +
>>>     struct AVCodecInternal;
>>>     /**
>>> @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
>>>      * - encoding: set by user
>>>      */
>>>     int export_side_data;
>>> +
>>> +    /**
>>> + * This callback is called at the beginning of each
>>> packet to get a data
>>> + * buffer for it.
>>> + *
>>> + * The following field will be set in the packet
>>> before this callback is
>>> + * called:
>>> + * - size
>>> + * This callback must use the above value to
>>> calculate the required buffer size,
>>> + * which must padded by at least
>>> AV_INPUT_BUFFER_PADDING_SIZE bytes.
>>> + *
>>> + * This callback must fill the following fields in the packet:
>>> + * - data
>>
>> Is the data pointer allowed to be in write-only memory?
>
> I'm not sure what the use case for this would be, so probably no?

 The two use-cases I see for this API are:

 * You want to avoid a copy when combining the output with something
 else.  E.g. you pass a pointer to the block of memory following
 where you are going to put your header data (for something you are
 going to send over the network, say).

 * You want to avoid a copy when passing the output directly to
 something external.  E.g. you pass a pointer to a memory-mapped
 device buffer (such as a V4L2 buffer, say).

 In the second case, write-only memory on an external device seems
 possible, as does memory which is, say, readable but uncached, so
 reading it is a really bad idea.
>>>
>>> Allowing the second case would depend on how encoders behave. Some may
>>> attempt to read data already written to the output packet. It's not like
>>> all of them allocate the packet, do a memcpy from an internal buffer,
>>> then return.
>>> There is also the flag meant to signal that the encoder will keep a
>>> reference to the packet around, which more or less implies it will be
>>> read later in the encoding process.
>>>
>>> The doxy for avcodec_encode_video2(), which allowed the user to provide
>>> their own buffers in the output packet, does not mention any kind of
>>> requirement for the data pointer, so I don't think we can say it's an
>>> allowed scenario here either.
>>>

>> Does it have any alignment requirements?
>
> No, just padding. AVPacket doesn't require alignment for the payload.

 I think say that explicitly.  avcodec_default_get_encoder_buffer()
 does give you aligned memory, even though it isn't needed.
>>>
>>> Would saying "There's no alignment requirement for the data pointer" add
>>> 

Re: [FFmpeg-devel] [PATCH v2] avcodec: add a get_encoder_buffer() callback to AVCodecContext

2021-03-12 Thread Michael Niedermayer
On Thu, Mar 11, 2021 at 02:18:36PM -0300, James Almer wrote:
> On 3/11/2021 1:35 PM, Michael Niedermayer wrote:
> > On Wed, Mar 10, 2021 at 05:59:11PM -0300, James Almer wrote:
> > > On 3/10/2021 5:18 PM, Michael Niedermayer wrote:
> > > > On Mon, Feb 22, 2021 at 07:27:34PM -0300, James Almer wrote:
> > > > > On 2/21/2021 6:04 PM, James Almer wrote:
> > > > > > On 2/21/2021 5:29 PM, Mark Thompson wrote:
> > > > > > > On 21/02/2021 20:00, James Almer wrote:
> > > > > > > > On 2/21/2021 4:13 PM, Mark Thompson wrote:
> > > > > > > > > On 21/02/2021 17:35, James Almer wrote:
> > > > > > > > > > This callback is functionally the same as get_buffer2()
> > > > > > > > > > is for decoders, and
> > > > > > > > > > implements for the new encode API the functionality of
> > > > > > > > > > the old encode API had
> > > > > > > > > > where the user could provide their own buffers.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: James Almer 
> > > > > > > > > > ---
> > > > > > > > > > Used the names Lynne suggested this time, plus a line
> > > > > > > > > > about how the callback
> > > > > > > > > > must be thread safe.
> > > > > > > > > > 
> > > > > > > > > >     libavcodec/avcodec.h | 45 
> > > > > > > > > > 
> > > > > > > > > >     libavcodec/codec.h   |  8 ---
> > > > > > > > > >     libavcodec/encode.c  | 54
> > > > > > > > > > +++-
> > > > > > > > > >     libavcodec/encode.h  |  8 +++
> > > > > > > > > >     libavcodec/options.c |  1 +
> > > > > > > > > >     5 files changed, 112 insertions(+), 4 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > > > > > index 7dbf083a24..e60eb16ce1 100644
> > > > > > > > > > --- a/libavcodec/avcodec.h
> > > > > > > > > > +++ b/libavcodec/avcodec.h
> > > > > > > > > > @@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime 
> > > > > > > > > > {
> > > > > > > > > >      */
> > > > > > > > > >     #define AV_GET_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > +/**
> > > > > > > > > > + * The encoder will keep a reference to the packet and
> > > > > > > > > > may reuse it later.
> > > > > > > > > > + */
> > > > > > > > > > +#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
> > > > > > > > > > +
> > > > > > > > > >     struct AVCodecInternal;
> > > > > > > > > >     /**
> > > > > > > > > > @@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
> > > > > > > > > >      * - encoding: set by user
> > > > > > > > > >      */
> > > > > > > > > >     int export_side_data;
> > > > > > > > > > +
> > > > > > > > > > +    /**
> > > > > > > > > > + * This callback is called at the beginning of each
> > > > > > > > > > packet to get a data
> > > > > > > > > > + * buffer for it.
> > > > > > > > > > + *
> > > > > > > > > > + * The following field will be set in the packet
> > > > > > > > > > before this callback is
> > > > > > > > > > + * called:
> > > > > > > > > > + * - size
> > > > > > > > > > + * This callback must use the above value to
> > > > > > > > > > calculate the required buffer size,
> > > > > > > > > > + * which must padded by at least
> > > > > > > > > > AV_INPUT_BUFFER_PADDING_SIZE bytes.
> > > > > > > > > > + *
> > > > > > > > > > + * This callback must fill the following fields in the 
> > > > > > > > > > packet:
> > > > > > > > > > + * - data
> > > > > > > > > 
> > > > > > > > > Is the data pointer allowed to be in write-only memory?
> > > > > > > > 
> > > > > > > > I'm not sure what the use case for this would be, so probably 
> > > > > > > > no?
> > > > > > > 
> > > > > > > The two use-cases I see for this API are:
> > > > > > > 
> > > > > > > * You want to avoid a copy when combining the output with 
> > > > > > > something
> > > > > > > else.  E.g. you pass a pointer to the block of memory following
> > > > > > > where you are going to put your header data (for something you are
> > > > > > > going to send over the network, say).
> > > > > > > 
> > > > > > > * You want to avoid a copy when passing the output directly to
> > > > > > > something external.  E.g. you pass a pointer to a memory-mapped
> > > > > > > device buffer (such as a V4L2 buffer, say).
> > > > > > > 
> > > > > > > In the second case, write-only memory on an external device seems
> > > > > > > possible, as does memory which is, say, readable but uncached, so
> > > > > > > reading it is a really bad idea.
> > > > > > 
> > > > > > Allowing the second case would depend on how encoders behave. Some 
> > > > > > may
> > > > > > attempt to read data already written to the output packet. It's not 
> > > > > > like
> > > > > > all of them allocate the packet, do a memcpy from an internal 
> > > > > > buffer,
> > > > > > then return.
> > > > > > There is also the flag meant to signal that the encoder will keep a
> > > > > > reference to the packet around, which more or less implies it will 
> > > > > 

[FFmpeg-devel] [PATCH] CUDA - make it work for multiple GPU architectures

2021-03-12 Thread Patrick Ecord
Hello, 

My friend was running into issues trying to compile ffpmeg with cuda support so 
I tried to replicate the issue on machine with my 1070.

Started by following Nvidia’s guide for compiling with CUDA support -
https://docs.nvidia.com/video-technologies/video-codec-sdk/ffmpeg-with-nvidia-gpu/

It uses the wrong flag (`-–enable-cuda-sdk` instead of `--enable-cuda-nvcc`) 
got that figured out.

Then when I tried to run ./configure with the right flag I got `nvcc fatal : 
Unsupported gpu architecture 'compute_30'`

Googled that and found this github issue where one person suggested changing 
the `nvccflags_default` flags and they said - "I went with 75 because I'm on 
Turing architecture”
https://github.com/NVIDIA/cuda-samples/issues/46 

Started looking around for what flags I would want to use and I found this 
webpage that listed what cards were supported by which CUDA versions.
https://arnon.dk/matching-sm-architectures-arch-and-gencode-for-various-nvidia-cards/

I had just installed nvcc off Nvidia’s site and it came with CUDA 11 and there 
was a section that had flags for CUDA 11 with compatibility for "V100 and T4 
Turing cards, but also support newer RTX 3080 and other Ampere cards”.

Also according to that person’s site a lot of the older cards got dropped with 
CUDA 8, 9, 10 and now 11 these flags should cover Maxwell and up

```
-arch=sm_52 \ 
-gencode=arch=compute_52,code=sm_52 \ 
-gencode=arch=compute_60,code=sm_60 \ 
-gencode=arch=compute_61,code=sm_61 \ 
-gencode=arch=compute_70,code=sm_70 \ 
-gencode=arch=compute_75,code=sm_75 \
-gencode=arch=compute_80,code=sm_80 \
-gencode=arch=compute_86,code=sm_86 \
-gencode=arch=compute_86,code=compute_86
```

Tried that and ran configure and it failed with Option '--ptx (-ptx)' is not 
allowed when compiling for multiple GPU architectures"

So I removed the `-ptx` flag and I was able to run configure and make and make 
install without any errors.

Tested by converting Big Buck Bunny and it played fine.
ffmpeg -y -vsync 0 -hwaccel cuda -hwaccel_output_format cuda -i 
./Big_Buck_Bunny_1080_10s_30MB.mp4 -c:a copy -c:v h264_nvenc -b:v 5M output.mp4

Other stuff - 
I am not really a CUDA expert so I am not sure if this is the "correct" way so 
let me know if there is a better way of doing it.
I haven't tried timing it to see if there is a slow down from supporting 
multiple architectures and not using the -ptx flag.
I saw there were also flags for clang, haven't tried messing with that yet my 
understanding is you can pass the flag multiple times.
"You can pass --cuda-gpu-arch multiple times to compile for multiple archs." - 
https://llvm.org/docs/CompileCudaWithLLVM.html

Wanted to send what I had and see what you all think, 
Thanks

---
configure | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index d11942fced..d9e4eff592 100755
--- a/configure
+++ b/configure
@@ -4344,7 +4344,7 @@ fi

if enabled cuda_nvcc; then
nvcc_default="nvcc"
-nvccflags_default="-gencode arch=compute_30,code=sm_30 -O2"
+nvccflags_default="-arch=sm_52 -gencode=arch=compute_52,code=sm_52 
-gencode=arch=compute_60,code=sm_60 -gencode=arch=compute_61,code=sm_61 
-gencode=arch=compute_70,code=sm_70 -gencode=arch=compute_75,code=sm_75 
-gencode=arch=compute_80,code=sm_80 -gencode=arch=compute_86,code=sm_86 
-gencode=arch=compute_86,code=compute_86"
else
nvcc_default="clang"
nvccflags_default="--cuda-gpu-arch=sm_30 -O2"
@@ -6240,7 +6240,7 @@ else
fi

if enabled cuda_nvcc; then
-nvccflags="$nvccflags -ptx"
+nvccflags="$nvccflags"
else
nvccflags="$nvccflags -S -nocudalib -nocudainc --cuda-device-only 
-Wno-c++11-narrowing -include ${source_link}/compat/cuda/cuda_runtime.h"
check_nvcc cuda_llvm
-- 
2.29.2

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

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

[FFmpeg-devel] [PATCH v2] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

2021-03-12 Thread Derek Buitenhuis
Files with the "dash" major brand are guaranteed to only have a single
initialization range for the whole file. We can check this and stop
appropriately - which is useful, as the existing check to see if the
offset reaches the end of the file is not sufficient. For example,
Premiere creates valid self-initializing ISOBMFF files that contain
a single 'uuid' box appended at the end, after the last 'moof', and
this check does not catch that, which causes, for example, probing
to traverse the entire file (every single 'moof'), which is extremely
slow over a network, which is the main place these self-initializing
fragmented ISOBMFF files would be used. This is the same behavior that
was addressed in 2ff3c466eca66bb8eb32bb41a4ce70fe285e3ea0 for live-style
fragmented files.

Signed-off-by: Derek Buitenhuis 
---
Updated the commit message too, to reflect that the files are actually
a YouTube-muxed file that was opened in Premiere, which appends XMP
metadata in a 'uuid' box to files it opens. Gross.
---
 libavformat/mov.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 23b0ead01e..0eb52ab6e6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5052,6 +5052,8 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 AVStream *ref_st = NULL;
 MOVStreamContext *sc, *ref_sc = NULL;
 AVRational timescale;
+AVDictionaryEntry *major_brand = av_dict_get(c->fc->metadata, 
"major_brand", NULL, AV_DICT_MATCH_CASE);
+AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata, 
"compatible_brands", NULL, AV_DICT_MATCH_CASE);
 
 version = avio_r8(pb);
 if (version > 1) {
@@ -5120,7 +5122,13 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 sc->has_sidx = 1;
 
 // See if the remaining bytes are just an mfra which we can ignore.
-is_complete = offset == stream_size;
+//
+// Also check to see if the file is Self-initializing, which guarantees
+// only a single initialization range is present for the whole file.
+// See: ISO-IEC 23009-1 Section 6.3.5.
+is_complete = offset == stream_size ||
+  (major_brand && !strncmp(major_brand->value, "dash", 4)) ||
+  (compatible_brands && strstr(compatible_brands->value, 
"dash"));
 if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) {
 int64_t ret;
 int64_t original_pos = avio_tell(pb);
-- 
2.30.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] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

2021-03-12 Thread Derek Buitenhuis
On 12/03/2021 11:20, Martin Storsjö wrote:
> LGTM, I guess the only question that remains is whether "dash" in the 
> compatible brands can imply the same too. (Our own muxer never sets "dash" 
> as the major brand, only as a compatible brand, and that only if we only 
> produce one global sidx.)

I *think* should probably be checking both major_brand and compatible_brands,
actually, yeah. Based on the wording in ISO/ETC 14496-12, I think this is
the case.

I will send a v2.

- Derek
___
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] avfilter/vf_uspp: Fix leak of packet side data

2021-03-12 Thread Andreas Rheinhardt
The uspp filter uses a special option ("no_bitstream") of
the Snow encoder to suppress it from generating output.
The filter therefore did not unref the packet after usage,
believing it to be blank. But this is wrong, as the Snow encoder
attaches quality stats side data to the packet.

Signed-off-by: Andreas Rheinhardt 
---
If there were a fate test for this filter, this would have been found
and fixed long ago.

 libavfilter/vf_uspp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/vf_uspp.c b/libavfilter/vf_uspp.c
index b77edeb244..523e47c811 100644
--- a/libavfilter/vf_uspp.c
+++ b/libavfilter/vf_uspp.c
@@ -257,6 +257,7 @@ static void filter(USPPContext *p, uint8_t *dst[3], uint8_t 
*src[3],
 av_log(p->avctx_enc[i], AV_LOG_ERROR, "Encoding failed\n");
 continue;
 }
+av_packet_unref();
 
 p->frame_dec = p->avctx_enc[i]->coded_frame;
 
-- 
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 1/2] avfilter/vf_uspp: Fix leak of qp-table on error

2021-03-12 Thread Andreas Rheinhardt
Fixes Coverity issue #1473500.

Signed-off-by: Andreas Rheinhardt 
---
Why does the new qp API actually need to allocate the qp table (instead
of reusing the side-data qp)?

 libavfilter/vf_uspp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_uspp.c b/libavfilter/vf_uspp.c
index 8b39f53c3d..b77edeb244 100644
--- a/libavfilter/vf_uspp.c
+++ b/libavfilter/vf_uspp.c
@@ -425,6 +425,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 out = ff_get_video_buffer(outlink, aligned_w, aligned_h);
 if (!out) {
 av_frame_free();
+if (qp_table != uspp->non_b_qp_table)
+av_free(qp_table);
 return AVERROR(ENOMEM);
 }
 av_frame_copy_props(out, in);
-- 
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 1/2] avformat/adp, svs: Remove redundant av_shrink_packet()

2021-03-12 Thread James Almer

On 3/12/2021 7:53 AM, Andreas Rheinhardt wrote:

av_get_packet() already makes sure that the packet size is accurate
and that the packet data is zero-padded even when one could not read as
much as desired.

Signed-off-by: Andreas Rheinhardt 
---
  libavformat/adp.c | 8 ++--
  libavformat/svs.c | 1 -
  2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/libavformat/adp.c b/libavformat/adp.c
index 8668c78fe4..b0ee09753e 100644
--- a/libavformat/adp.c
+++ b/libavformat/adp.c
@@ -75,13 +75,9 @@ static int adp_read_packet(AVFormatContext *s, AVPacket *pkt)
  return AVERROR_EOF;
  
  ret = av_get_packet(s->pb, pkt, size);

+if (ret < 0)
+return ret;
  
-if (ret != size) {

-if (ret < 0) {
-return ret;
-}
-av_shrink_packet(pkt, ret);
-}
  pkt->stream_index = 0;
  
  return ret;

diff --git a/libavformat/svs.c b/libavformat/svs.c
index d4285ed306..8be26c5bc3 100644
--- a/libavformat/svs.c
+++ b/libavformat/svs.c
@@ -79,7 +79,6 @@ static int svs_read_packet(AVFormatContext *s, AVPacket *pkt)
  if (ret != 32 * 256) {
  if (ret < 0)
  return ret;
-av_shrink_packet(pkt, ret);
  pkt->flags &= ~AV_PKT_FLAG_CORRUPT;
  }
  pkt->stream_index = 0;


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

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

Re: [FFmpeg-devel] [PATCH 7/7] avformat: add Changelog entry for librist and bump minor

2021-03-12 Thread Paul B Mahol
On Fri, Mar 12, 2021 at 12:21 AM Marton Balint  wrote:

>
>
> On Sun, 7 Mar 2021, Marton Balint wrote:
>
> >
> >
> > On Sun, 7 Mar 2021, Paul B Mahol wrote:
> >
> >> On Sun, Mar 7, 2021 at 9:02 AM Marton Balint  wrote:
> >>
> >>>
> >>>
> >>> On Sun, 7 Mar 2021, Paul B Mahol wrote:
> >>>
> >>> > On Sun, Mar 7, 2021 at 1:57 AM James Almer 
> wrote:
> >>> >
> >>> >> On 3/6/2021 9:51 PM, Paul B Mahol wrote:
> >>> >> > On Sun, Mar 7, 2021 at 1:35 AM Marton Balint 
> wrote:
> >>> >> >
> >>> >> >>
> >>> >> >>
> >>> >> >> On Sun, 7 Mar 2021, Paul B Mahol wrote:
> >>> >> >>
> >>> >> >>> How you tested this?
> >>> >> >>
> >>> >> >> with the librist tools.
> >>> >> >>
> >>> >> >>>
> >>> >> >>> And why you have not asked nicely before taking working on this?
> >>> >> >>
> >>> >> >> You wrote you have given up. It is your behaviour which is not
> very
> >>> >> nice.
> >>> >> >> After your continuous ignorance of my requests, I try to help
> your
> >>> work
> >>> >> >> get merged, and this is your feedback...
> >>> >> >>
> >>> >> >
> >>> >> >
> >>> >> > Why are you so aggressive? Other devs raised issues that looks not
> >>> >> resolved.
> >>> >>
> >>> >> I don't see aggressiveness from his part. I see you making things
> >>> >> incredibly more complex than they should be.
> >>> >>
> >>> >
> >>> > The code in functional part is not much different from initial
> versions.
> >>> >
> >>> > So I really doubt that fifo filling up issue have been properly
> >>> addressed.
> >>>
> >>> I can take a look at that if you provide command lines to reproduce.
> >>>
> >>
> >> I couldn't reproduce it, so you have to ask others.
> >
> > I found one issue in librist code which can be related and might cause
> > ffmpeg not to recover from a fifo fillup. I have reported it here:
> >
> > https://code.videolan.org/rist/librist/-/issues/93
>
> I'd love to see rist support in 4.4, so maybe we should apply this series
> as is and fix any remaining issues later...
>


Not gonna block it.



>
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
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/mxfdec: Fix leak on error

2021-03-12 Thread Andreas Rheinhardt
It was introduced in d3d9b1fc8e2dfc8b4d66c9916ab7221062ff4660;
Fixes Coverity issue #733800.

Signed-off-by: Andreas Rheinhardt 
---
I have no testcase for this; but hopefully Michael can test it with the
testcase that led to d3d9b1fc8e2dfc8b4d66c9916ab7221062ff4660 in the
first place?
(And I always thought fuzzing samples were small. How does it come that
it is close to INT64_MAX?)

 libavformat/mxfdec.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index d7213bda30..2e9d7d713a 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2909,8 +2909,11 @@ static int mxf_read_local_tags(MXFContext *mxf, 
KLVPacket *klv, MXFMetadataReadF
 int size = avio_rb16(pb); /* KLV specified by 0x53 */
 int64_t next = avio_tell(pb);
 UID uid = {0};
-if (next < 0 || next > INT64_MAX - size)
+if (next < 0 || next > INT64_MAX - size) {
+if (meta)
+mxf_free_metadataset(, 1);
 return next < 0 ? next : AVERROR_INVALIDDATA;
+}
 next += size;
 
 av_log(mxf->fc, AV_LOG_TRACE, "local tag %#04x size %d\n", tag, 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 1/2] avformat/mxfdec: Don't use wrong type of pointer

2021-03-12 Thread Andreas Rheinhardt
If one of the two results of a ternary conditional is a pointer to void,
the type of the whole conditional operator is a pointer to void, even
when the other possible result is not a pointer to void. This loophole
in the type system has allowed mxf_read_local_tags to have a pointer of
type pointer to MXFMetadataSet that actually points to an MXFContext.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/mxfdec.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index bb00838a3f..d7213bda30 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2889,13 +2889,20 @@ static int mxf_metadataset_init(MXFMetadataSet *ctx, 
enum MXFMetadataSetType typ
 static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, 
MXFMetadataReadFunc *read_child, int ctx_size, enum MXFMetadataSetType type)
 {
 AVIOContext *pb = mxf->fc->pb;
-MXFMetadataSet *ctx = ctx_size ? av_mallocz(ctx_size) : mxf;
 uint64_t klv_end = avio_tell(pb) + klv->length;
+MXFMetadataSet *meta;
+void *ctx;
 
-if (!ctx)
-return AVERROR(ENOMEM);
-if (ctx_size)
-mxf_metadataset_init(ctx, type);
+if (ctx_size) {
+meta = av_mallocz(ctx_size);
+if (!meta)
+return AVERROR(ENOMEM);
+ctx  = meta;
+mxf_metadataset_init(meta, type);
+} else {
+meta = NULL;
+ctx  = mxf;
+}
 while (avio_tell(pb) + 4 < klv_end && !avio_feof(pb)) {
 int ret;
 int tag = avio_rb16(pb);
@@ -2922,20 +2929,19 @@ static int mxf_read_local_tags(MXFContext *mxf, 
KLVPacket *klv, MXFMetadataReadF
 }
 }
 }
-if (ctx_size && tag == 0x3C0A) {
-avio_read(pb, ctx->uid, 16);
+if (meta && tag == 0x3C0A) {
+avio_read(pb, meta->uid, 16);
 } else if ((ret = read_child(ctx, pb, tag, size, uid, -1)) < 0) {
-if (ctx_size)
-mxf_free_metadataset(, 1);
+if (meta)
+mxf_free_metadataset(, 1);
 return ret;
 }
 
 /* Accept the 64k local set limit being exceeded (Avid). Don't accept
  * it extending past the end of the KLV though (zzuf5.mxf). */
 if (avio_tell(pb) > klv_end) {
-if (ctx_size) {
-mxf_free_metadataset(, 1);
-}
+if (meta)
+mxf_free_metadataset(, 1);
 
 av_log(mxf->fc, AV_LOG_ERROR,
"local tag %#04x extends past end of local set @ 
%#"PRIx64"\n",
@@ -2944,7 +2950,7 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket 
*klv, MXFMetadataReadF
 } else if (avio_tell(pb) <= next)   /* only seek forward, else this 
can loop for a long time */
 avio_seek(pb, next, SEEK_SET);
 }
-return ctx_size ? mxf_add_metadata_set(mxf, ) : 0;
+return meta ? mxf_add_metadata_set(mxf, ) : 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".

Re: [FFmpeg-devel] [PATCH] avformat/mov: Properly consider if the file is self-initializing when marking sidx reading complete

2021-03-12 Thread Martin Storsjö

On Wed, 10 Mar 2021, Derek Buitenhuis wrote:


Files with the "dash" major brand are guaranteed to only have a single
initialization range for the whole file. We can check this and stop
appropriately - which is useful, as the existing check to see if the
offset reaches the end of the file is not sufficient. For example,
YouTube creates valid self-initializing ISOBMFF files that contain
a single 'uuid' box appended at the end, after the last 'moof', and
this check does not catch that, which causes, for example, probing
to traverse the entire file (every single 'moof'), which is extremely
slow over a network, which is the main place these self-initializing
fragmented ISOBMFF files would be used. This is the same behavior that
was addressed in 2ff3c466eca66bb8eb32bb41a4ce70fe285e3ea0 for live-style
fragmented files.

Signed-off-by: Derek Buitenhuis 
---
libavformat/mov.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1c07cff6b5..67eae24e02 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5052,6 +5052,7 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
AVStream *ref_st = NULL;
MOVStreamContext *sc, *ref_sc = NULL;
AVRational timescale;
+AVDictionaryEntry *major_brand = av_dict_get(c->fc->metadata, 
"major_brand", NULL, AV_DICT_MATCH_CASE);

version = avio_r8(pb);
if (version > 1) {
@@ -5120,7 +5121,11 @@ static int mov_read_sidx(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
sc->has_sidx = 1;

// See if the remaining bytes are just an mfra which we can ignore.
-is_complete = offset == stream_size;
+//
+// Also check to see if the file is Self-initializing, which guarantees
+// only a single initialization range is present for the whole file.
+// See: ISO-IEC 23009-1 Section 6.3.5.
+is_complete = offset == stream_size || (major_brand && !strncmp(major_brand->value, 
"dash", 4));
if (!is_complete && (pb->seekable & AVIO_SEEKABLE_NORMAL)) {
int64_t ret;
int64_t original_pos = avio_tell(pb);
--
2.30.0


LGTM, I guess the only question that remains is whether "dash" in the 
compatible brands can imply the same too. (Our own muxer never sets "dash" 
as the major brand, only as a compatible brand, and that only if we only 
produce one global sidx.)


// Martin

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

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

Re: [FFmpeg-devel] GSOC 2021 project Guided Filter

2021-03-12 Thread Steven Liu


> 2021年3月12日 下午6:27,Shubham Sahoo  写道:
> 
> Hi everyone,
> 
> I'm Shubham Sahoo, an undergraduate student in Electronics and Electrical
> Communication Engineering from India. I would like to work on the project
> "Guided Filter" in GSOC 2021. Can anyone please guide me with the next
> steps, the qualification tasks, and the project information?
1. You could try understand these rules under the link:
https://summerofcode.withgoogle.com/get-started/

2. And understand Guided Filter Calculation:
For example: http://kaiminghe.com/eccv10/

3. And try to implement a filter for that.


> 
> 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".

Thanks

Steven Liu



___
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/utils: Fix confusing return value for ff_read_packet()

2021-03-12 Thread Andreas Rheinhardt
Currently, ff_read_packet() sometimes forwards the return value of
AVInputFormat.read_packet() (which should be zero on success, but isn't
for all demuxers) and sometimes it overwrites this with zero.
Furthermore, it uses two variables, one for the read_packet return value
and one for other errors, which is a bit confusing; it is also
unnecessary given that the documentation explicitly states that
ff_read_packet() never returns positive values. Returning a positive
value would lead to leaks with some callers (namely asfrtp_parse_packet
and estimate_timings_from_pts). So always return zero in case of
success.

(This behaviour stems from a time before av_read_packet sanitized
the return value of read_packet at all: It was added in commit
626004690c23c981f67228ea325dde3f35193988 and was unnecessary since
88b00723906f68b7563214c30333e4dddf78.)

Signed-off-by: Andreas Rheinhardt 
---
asfrtp_parse_packet contains this:

for (;;) {
int i;

res = ff_read_packet(rt->asf_ctx, pkt);
rt->asf_pb_pos = avio_tell(pb);
if (res != 0)
break;
for (i = 0; i < s->nb_streams; i++) {
if (s->streams[i]->id == 
rt->asf_ctx->streams[pkt->stream_index]->id) {
pkt->stream_index = i;
return 1; // FIXME: return 0 if last packet
}
}
av_packet_unref(pkt);
}

return res == 1 ? -1 : res;

It's the very last line that bothers me: ff_read_packet and
av_read_packet (its predecessor that was used when this code has
originally been added) are not allowed to return anything > 0 and
they didn't for the asf demuxer. Does someone see a hidden intent
in this or was it just wrong from the beginning?

 libavformat/utils.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 8573117694..a3de4af9a1 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -804,7 +804,7 @@ static int update_wrap_reference(AVFormatContext *s, 
AVStream *st, int stream_in
 
 int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
-int ret, i, err;
+int err, i;
 AVStream *st;
 
 pkt->data = NULL;
@@ -828,17 +828,17 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 }
 }
 
-ret = s->iformat->read_packet(s, pkt);
-if (ret < 0) {
+err = s->iformat->read_packet(s, pkt);
+if (err < 0) {
 av_packet_unref(pkt);
 
 /* Some demuxers return FFERROR_REDO when they consume
data and discard it (ignored streams, junk, extradata).
We must re-call the demuxer to get the real packet. */
-if (ret == FFERROR_REDO)
+if (err == FFERROR_REDO)
 continue;
-if (!pktl || ret == AVERROR(EAGAIN))
-return ret;
+if (!pktl || err == AVERROR(EAGAIN))
+return err;
 for (i = 0; i < s->nb_streams; i++) {
 st = s->streams[i];
 if (st->probe_packets || st->internal->request_probe > 0)
@@ -892,7 +892,7 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 pkt->dts = pkt->pts = av_rescale_q(av_gettime(), AV_TIME_BASE_Q, 
st->time_base);
 
 if (!pktl && st->internal->request_probe <= 0)
-return ret;
+return 0;
 
 err = avpriv_packet_list_put(>internal->raw_packet_buffer,
  >internal->raw_packet_buffer_end,
-- 
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 1/2] avformat/adp, svs: Remove redundant av_shrink_packet()

2021-03-12 Thread Andreas Rheinhardt
av_get_packet() already makes sure that the packet size is accurate
and that the packet data is zero-padded even when one could not read as
much as desired.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/adp.c | 8 ++--
 libavformat/svs.c | 1 -
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/libavformat/adp.c b/libavformat/adp.c
index 8668c78fe4..b0ee09753e 100644
--- a/libavformat/adp.c
+++ b/libavformat/adp.c
@@ -75,13 +75,9 @@ static int adp_read_packet(AVFormatContext *s, AVPacket *pkt)
 return AVERROR_EOF;
 
 ret = av_get_packet(s->pb, pkt, size);
+if (ret < 0)
+return ret;
 
-if (ret != size) {
-if (ret < 0) {
-return ret;
-}
-av_shrink_packet(pkt, ret);
-}
 pkt->stream_index = 0;
 
 return ret;
diff --git a/libavformat/svs.c b/libavformat/svs.c
index d4285ed306..8be26c5bc3 100644
--- a/libavformat/svs.c
+++ b/libavformat/svs.c
@@ -79,7 +79,6 @@ static int svs_read_packet(AVFormatContext *s, AVPacket *pkt)
 if (ret != 32 * 256) {
 if (ret < 0)
 return ret;
-av_shrink_packet(pkt, ret);
 pkt->flags &= ~AV_PKT_FLAG_CORRUPT;
 }
 pkt->stream_index = 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] GSOC 2021 project Guided Filter

2021-03-12 Thread Shubham Sahoo
Hi everyone,

I'm Shubham Sahoo, an undergraduate student in Electronics and Electrical
Communication Engineering from India. I would like to work on the project
"Guided Filter" in GSOC 2021. Can anyone please guide me with the next
steps, the qualification tasks, and the project information?

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".

Re: [FFmpeg-devel] [PATCH] lavu: add VKAPI hwcontext implementation

2021-03-12 Thread Jean-Baptiste Kempf
On Fri, 12 Mar 2021, at 11:28, Jean-Baptiste Kempf wrote:
> On Fri, 12 Mar 2021, at 11:17, Steven Liu wrote:
> > > 2021年3月12日 下午5:12,Jean-Baptiste Kempf  写道:
> > > 
> > > On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:
> > >> Initial commit to add VKAPI hardware accelerator implementation.
> > >> The depedency component vkil source code can be obtained from github
> > >> https://github.com/Broadcom/vkil
> > > 
> > > This has no license, no readme, no description.
> > I saw the License in source code: 
> 
> License in a file is not the same as in the core of the project.

Not to mention that if Apache2 it is, this makes the code LGPLv3.
-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
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: add VKAPI hwcontext implementation

2021-03-12 Thread Jean-Baptiste Kempf
On Fri, 12 Mar 2021, at 11:17, Steven Liu wrote:
> > 2021年3月12日 下午5:12,Jean-Baptiste Kempf  写道:
> > 
> > On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:
> >> Initial commit to add VKAPI hardware accelerator implementation.
> >> The depedency component vkil source code can be obtained from github
> >> https://github.com/Broadcom/vkil
> > 
> > This has no license, no readme, no description.
> I saw the License in source code: 

License in a file is not the same as in the core of the project.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
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: add VKAPI hwcontext implementation

2021-03-12 Thread Steven Liu


> 2021年3月12日 下午5:12,Jean-Baptiste Kempf  写道:
> 
> On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:
>> Initial commit to add VKAPI hardware accelerator implementation.
>> The depedency component vkil source code can be obtained from github
>> https://github.com/Broadcom/vkil
> 
> This has no license, no readme, no description.
I saw the License in source code: 
https://github.com/Broadcom/vkil/blob/master/src/vk_buffers.h

/* SPDX-License-Identifier: Apache-2.0 */
/*
 * Copyright 2018-2020 Broadcom.
 */

Apache-2.0 ?
> 
> What is the hardware supported?
> Does it work for rPI?
> 
> -- 
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu



___
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: add VKAPI hwcontext implementation

2021-03-12 Thread Jean-Baptiste Kempf
On Thu, 11 Mar 2021, at 23:09, suji.velupil...@broadcom.com wrote:
> Initial commit to add VKAPI hardware accelerator implementation.
> The depedency component vkil source code can be obtained from github
> https://github.com/Broadcom/vkil

This has no license, no readme, no description.

What is the hardware supported?
Does it work for rPI?

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
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 v1] lavfi/qsvvpp: support async depth

2021-03-12 Thread Wang, Fei W
On Fri, 2021-03-12 at 06:20 +, Xiang, Haihao wrote:
> On Wed, 2021-01-27 at 09:42 +0800, Fei Wang wrote:
> > Async depth will allow qsv filter cache few frames, and avoid force
> > switch and end filter task frame by frame. This change will improve
> > performance for some multi-task case, for example 1:N transcode(
> > decode + vpp + encode) with all QSV plugins.
> > 
> > Signed-off-by: Fei Wang 
> > ---
> >  libavfilter/qsvvpp.c | 147 ++-
> > 
> >  libavfilter/qsvvpp.h |  42 -
> >  libavfilter/vf_deinterlace_qsv.c |   8 --
> >  libavfilter/vf_vpp_qsv.c |  75 +---
> >  4 files changed, 187 insertions(+), 85 deletions(-)
> > 
> > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
> > index f216b3f248..2e824e67e7 100644
> > --- a/libavfilter/qsvvpp.c
> > +++ b/libavfilter/qsvvpp.c
> > @@ -27,6 +27,7 @@
> >  #include "libavutil/hwcontext_qsv.h"
> >  #include "libavutil/time.h"
> >  #include "libavutil/pixdesc.h"
> > +#include "libavutil/fifo.h"
> >  
> >  #include "internal.h"
> >  #include "qsvvpp.h"
> > @@ -37,37 +38,6 @@
> >  #define IS_OPAQUE_MEMORY(mode) (mode & MFX_MEMTYPE_OPAQUE_FRAME)
> >  #define IS_SYSTEM_MEMORY(mode) (mode & MFX_MEMTYPE_SYSTEM_MEMORY)
> >  
> > -typedef struct QSVFrame {
> > -AVFrame  *frame;
> > -mfxFrameSurface1 *surface;
> > -mfxFrameSurface1  surface_internal;  /* for system memory */
> > -struct QSVFrame  *next;
> > -} QSVFrame;
> > -
> > -/* abstract struct for all QSV filters */
> > -struct QSVVPPContext {
> > -mfxSession  session;
> > -int (*filter_frame) (AVFilterLink *outlink, AVFrame *frame);/*
> > callback
> > */
> > -enum AVPixelFormat  out_sw_format;   /* Real output format */
> > -mfxVideoParam   vpp_param;
> > -mfxFrameInfo   *frame_infos; /* frame info for each
> > input */
> > -
> > -/* members related to the input/output surface */
> > -int in_mem_mode;
> > -int out_mem_mode;
> > -QSVFrame   *in_frame_list;
> > -QSVFrame   *out_frame_list;
> > -int nb_surface_ptrs_in;
> > -int nb_surface_ptrs_out;
> > -mfxFrameSurface1  **surface_ptrs_in;
> > -mfxFrameSurface1  **surface_ptrs_out;
> > -
> > -/* MFXVPP extern parameters */
> > -mfxExtOpaqueSurfaceAlloc opaque_alloc;
> > -mfxExtBuffer  **ext_buffers;
> > -int nb_ext_buffers;
> > -};
> > -
> >  static const mfxHandleType handle_types[] = {
> >  MFX_HANDLE_VA_DISPLAY,
> >  MFX_HANDLE_D3D9_DEVICE_MANAGER,
> > @@ -336,9 +306,9 @@ static int fill_frameinfo_by_link(mfxFrameInfo
> > *frameinfo,
> > AVFilterLink *link)
> >  static void clear_unused_frames(QSVFrame *list)
> >  {
> >  while (list) {
> > -if (list->surface && !list->surface->Data.Locked) {
> > -list->surface = NULL;
> > +if (list->used && !list->queued && !list-
> > >surface.Data.Locked) {
> >  av_frame_free(>frame);
> > +list->used = 0;
> >  }
> >  list = list->next;
> >  }
> > @@ -361,8 +331,10 @@ static QSVFrame *get_free_frame(QSVFrame
> > **list)
> >  QSVFrame *out = *list;
> >  
> >  for (; out; out = out->next) {
> > -if (!out->surface)
> > +if (!out->used) {
> > +out->used = 1;
> >  break;
> > +}
> >  }
> >  
> >  if (!out) {
> > @@ -371,6 +343,7 @@ static QSVFrame *get_free_frame(QSVFrame
> > **list)
> >  av_log(NULL, AV_LOG_ERROR, "Can't alloc new output
> > frame.\n");
> >  return NULL;
> >  }
> > +out->used  = 1;
> >  out->next  = *list;
> >  *list  = out;
> >  }
> > @@ -402,7 +375,7 @@ static QSVFrame *submit_frame(QSVVPPContext *s,
> > AVFilterLink *inlink, AVFrame *p
> >  return NULL;
> >  }
> >  qsv_frame->frame   = av_frame_clone(picref);
> > -qsv_frame->surface = (mfxFrameSurface1 *)qsv_frame->frame-
> > >data[3];
> > +qsv_frame->surface = *(mfxFrameSurface1 *)qsv_frame-
> > >frame->data[3];
> >  } else {
> >  /* make a copy if the input is not padded as libmfx
> > requires */
> >  if (picref->height & 31 || picref->linesize[0] & 31) {
> > @@ -425,27 +398,26 @@ static QSVFrame *submit_frame(QSVVPPContext
> > *s,
> > AVFilterLink *inlink, AVFrame *p
> >  qsv_frame->frame = av_frame_clone(picref);
> >  
> >  if (map_frame_to_surface(qsv_frame->frame,
> > -_frame->surface_internal) < 0)
> > {
> > + _frame->surface) < 0) {
> >  av_log(ctx, AV_LOG_ERROR, "Unsupported frame.\n");
> >  return NULL;
> >  }
> > -qsv_frame->surface = _frame->surface_internal;
> >  }
> >  
> > -qsv_frame->surface->Info   = s-
> > > 

Re: [FFmpeg-devel] [PATCH 15/23] dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation

2021-03-12 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年3月12日 15:59
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 15/23]
> dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> 
> Guo, Yejun:
> >
> >
> >> -Original Message-
>  -Original Message-
>  From: ffmpeg-devel  On Behalf Of
>  Andreas Rheinhardt
>  Sent: 2021年3月11日 5:55
>  To: ffmpeg-devel@ffmpeg.org
>  Cc: Andreas Rheinhardt 
>  Subject: [FFmpeg-devel] [PATCH 15/23]
>  dnn/dnn_backend_native_layer_conv2d: Join two arrays, avoid allocation
> 
>  Fixes Coverity issue #1473507.
> 
>  Signed-off-by: Andreas Rheinhardt 
>  ---
>   libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 10 +-
>   1 file changed, 5 insertions(+), 5 deletions(-)
>   }
> 
>   //release memory
>  -av_freep(_id);
>  -
>   for (int i = 0; i < thread_num; i++){
>   av_freep(_param[i]);
>   }
> >>>
> >>> LGMT, and just one question: shall we reduce the allocation as less as
> >> possible? thanks.
> >>>
> >> If it is doable without too much effort as is here, then the answer is
> >> yes. Actually, one could go even further than what this patchset did:
> >> One could keep the thread_param array instead of constantly allocating
> >> and freeing it. With much more effort, one could even keep the threads.
> >
> > thanks, and is the reason that: we need to check the return value for
> dynamic
> > allocation, and also need some effort to make sure to free it for all 
> > possible
> > paths in the code followed?
> >
> Yes, reducing the amount of allocations also reduces the amount of
> checks and frees one has to perform. You can see the latter in this
> patchset, namely in this patch and in the one that stopped allocating
> thread_param separately (which enabled to remove the loop for freeing them).

got it, 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".