Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc_hevc: add -pic_timing_sei option

[Xiang, Haihao]

On Wed, 2022-01-12 at 12:50 +0800, Haihao Xiang wrote:
> The SDK may insert picture timing SEI for hevc and the code to set mfx
> parameter has been added in qsvenc, however the corresponding option is
> missing in the hevc option array
> 
> Reviewed-by: Limin Wang 
> Signed-off-by: Haihao Xiang 
> ---
> v2: added option description in the doc
> 
>  doc/encoders.texi| 3 +++
>  libavcodec/qsvenc_hevc.c | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 7cc8be1209..1679f389d7 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3432,6 +3432,9 @@ Number of rows for tiled encoding.
>  
>  @item @var{aud}
>  Insert the Access Unit Delimiter NAL.
> +
> +@item @var{pic_timing_sei}
> +Insert picture timing SEI with pic_struct_syntax element.
>  @end table
>  
>  @subsection MPEG2 Options
> diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> index 08aba3011d..342b6bdea7 100644
> --- a/libavcodec/qsvenc_hevc.c
> +++ b/libavcodec/qsvenc_hevc.c
> @@ -251,6 +251,7 @@ static const AVOption options[] = {
>  { "tile_rows",  "Number of rows for tiled
> encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
> UINT16_MAX, VE },
>  { "recovery_point_sei", "Insert recovery point SEI
> messages",   OFFSET(qsv.recovery_point_sei),  AV_OPT_TYPE_INT, { .i64
> = -1 },   -1,  1, VE },
>  { "aud", "Insert the Access Unit Delimiter NAL", OFFSET(qsv.aud),
> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE},
> +{ "pic_timing_sei","Insert picture timing SEI with pic_struct_syntax
> element", OFFSET(qsv.pic_timing_sei), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE
> },
>  
>  { NULL },
>  };

Will apply

-Haihao

___
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] lavc/qsvenc: add encode support for screen content coding extension

[Xiang, Haihao]

On Wed, 2022-01-12 at 16:36 +0800, Haihao Xiang wrote:
> Enables HEVC Screen Content Coding extension support on ICL+ platform
> 
> Signed-off-by: Haihao Xiang 
> ---
> v2: rebased it against the latest master and added scc to the doc
> 
>  doc/encoders.texi| 3 +++
>  libavcodec/qsvenc.c  | 3 +++
>  libavcodec/qsvenc_hevc.c | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 7cc8be1209..78518629cd 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3411,12 +3411,15 @@ an internal session.
>  Depth of look ahead in number frames, available when extbrc option is
> enabled.
>  
>  @item @var{profile}
> +Set the encoding profile (scc requires libmfx >= 1.32).
> +
>  @table @samp
>  @item unknown
>  @item main
>  @item main10
>  @item mainsp
>  @item rext
> +@item scc
>  @end table
>  
>  @item @var{gpb}
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 4e7a15f060..269386624d 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -74,6 +74,9 @@ static const struct profile_names hevc_profiles[] = {
>  { MFX_PROFILE_HEVC_MAINSP,  "hevc
> mainsp"},
>  { MFX_PROFILE_HEVC_REXT,"hevc
> rext"  },
>  #endif
> +#if QSV_VERSION_ATLEAST(1, 32)
> +{ MFX_PROFILE_HEVC_SCC, "hevc
> scc"   },
> +#endif
>  };
>  
>  static const struct profile_names vp9_profiles[] = {
> diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> index 08aba3011d..07010c698e 100644
> --- a/libavcodec/qsvenc_hevc.c
> +++ b/libavcodec/qsvenc_hevc.c
> @@ -244,6 +244,9 @@ static const AVOption options[] = {
>  { "main10",  NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_HEVC_MAIN10  }, INT_MIN, INT_MAX, VE, "profile" },
>  { "mainsp",  NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_HEVC_MAINSP  }, INT_MIN, INT_MAX, VE, "profile" },
>  { "rext",NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_HEVC_REXT}, INT_MIN, INT_MAX, VE, "profile" },
> +#if QSV_VERSION_ATLEAST(1, 32)
> +{ "scc", NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_HEVC_SCC }, INT_MIN, INT_MAX, VE, "profile" },
> +#endif
>  
>  { "gpb", "1: GPB (generalized P/B frame); 0: regular P frame",
> OFFSET(qsv.gpb), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE},
>  

Will apply,

-Haihao

___
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] lavc/qsvenc: add tile encoding support for VP9

[Xiang, Haihao]

On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:
> Add -tile_rows and -tile_cols options to specify the number of tile
> rows and columns
> 
> Signed-off-by: Haihao Xiang 
> ---
> v2: add option descriptions in the doc
> 
>  doc/encoders.texi   |  6 ++
>  libavcodec/qsvenc.c |  4 
>  libavcodec/qsvenc.h |  1 +
>  libavcodec/qsvenc_vp9.c | 10 ++
>  4 files changed, 21 insertions(+)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 7cc8be1209..a4176089d5 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
>  @item profile2
>  @item profile3
>  @end table
> +
> +@item @var{tile_cols}
> +Number of columns for tiled encoding (requires libmfx >= 1.29).
> +
> +@item @var{tile_rows}
> +Number of rows for tiled encoding (requires libmfx  >= 1.29).
>  @end table
>  
>  @section snow
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 4e7a15f060..4cbc9ff4dc 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
>  q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
>  q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
>  q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
> +#if QSV_HAVE_EXT_VP9_TILES
> +q->extvp9param.NumTileColumns  = q->tile_cols;
> +q->extvp9param.NumTileRows = q->tile_rows;
> +#endif
>  q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer *)&q-
> >extvp9param;
>  }
>  #endif
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 31516b8e55..00ee52a5d1 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -41,6 +41,7 @@
>  
>  #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
>  #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
> +#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)
>  
>  #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
>  #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
> index 9329990d11..1168ddda0e 100644
> --- a/libavcodec/qsvenc_vp9.c
> +++ b/libavcodec/qsvenc_vp9.c
> @@ -73,6 +73,16 @@ static const AVOption options[] = {
>  { "profile2",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_VP9_2   },  INT_MIN,  INT_MAX,  VE,  "profile" },
>  { "profile3",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64 =
> MFX_PROFILE_VP9_3   },  INT_MIN,  INT_MAX,  VE,  "profile" },
>  
> +#if QSV_HAVE_EXT_VP9_TILES
> +/* The minimum tile width in luma pixels is 256, set maximum tile_cols to
> 32 for 8K video */
> +{ "tile_cols",  "Number of columns for tiled
> encoding",   OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 32,
> VE },
> +/* Set maximum tile_rows to 4 per VP9 spec */
> +{ "tile_rows",  "Number of rows for tiled
> encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 4,
> VE },
> +#else
> +{ "tile_cols",  "(not
> supported)",OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT,
> { .i64 = 0 }, 0, 0, VE },
> +{ "tile_rows",  "(not
> supported)",OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT,
> { .i64 = 0 }, 0, 0, VE },
> +#endif
> +
>  { NULL },
>  };
>  

Will apply,

-Haihao

___
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/videotoolbox: add support for memory mapping frames

[Anton Khirnov]

Quoting Cameron Gutman (2022-01-10 09:17:37)
> 
> > On Jan 9, 2022, at 3:24 AM, Anton Khirnov  wrote:
> > 
> > Quoting Cameron Gutman (2022-01-03 01:33:19)
> >> Signed-off-by: Cameron Gutman 
> >> ---
> >> libavutil/hwcontext_videotoolbox.c | 25 +
> >> 1 file changed, 25 insertions(+)
> >> 
> >> diff --git a/libavutil/hwcontext_videotoolbox.c 
> >> b/libavutil/hwcontext_videotoolbox.c
> >> index 0a8dbe9f33..026127d412 100644
> >> --- a/libavutil/hwcontext_videotoolbox.c
> >> +++ b/libavutil/hwcontext_videotoolbox.c
> >> @@ -711,6 +711,30 @@ fail:
> >> return err;
> >> }
> >> 
> >> +static int vt_map_from(AVHWFramesContext *hwfc, AVFrame *dst,
> >> +   const AVFrame *src, int flags)
> >> +{
> >> +int err;
> >> +
> >> +if (dst->format == AV_PIX_FMT_NONE)
> >> +dst->format = hwfc->sw_format;
> >> +else if (dst->format != hwfc->sw_format)
> >> +return AVERROR(ENOSYS);
> >> +
> >> +err = vt_map_frame(hwfc, dst, src, flags);
> >> +if (err)
> >> +return err;
> >> +
> >> +dst->width  = src->width;
> >> +dst->height = src->height;
> >> +
> >> +err = av_frame_copy_props(dst, src);
> >> +if (err)
> >> +return err;
> > 
> > Don't you need to unmap the frame in this error path?
> > 
> 
> Maybe? It's complicated...
> 
> The mapping is still written to dst and it will be unmapped when
> av_frame_unref() is called on dst. However, if the caller wants to try again
> with same dst frame for some reason, then it looks like it will leak the first
> "failed" mapping. AFAICT, another call to ff_hwframe_map_create() will
> overwrite dst->buf[0] without unrefing it first, but that makes sense given
> that the docs say "dst should be blank" (arguably that "should" ought to be a
> "must" in this case). However, this isn't the full story.
> 
> None of the existing map_from() implementations (VAAPI, DRM, DXVA2) actually
> unmap when av_frame_copy_props() fails. The only ones that don't have this bug
> are OpenCL and Vulkan, and that's only because they forgot to call
> av_frame_copy_props() entirely!
> 
> Ideally, you'd have nothing modified in dst when av_hwframe_map() fails,
> but that isn't the case in practice. Much of the mapping process involves
> irreversible changes to the dst frame, including overwriting dst->format,
> dst->width, dst->height, dst->data, dst->linesize, even partially copied
> frame properties prior to av_frame_copy_props() failing.
> 
> Given these limitations, it seems like the only sane thing to do with a dst
> frame after failure of av_hwframe_map() (other than ENOSYS) is to unref/free.
> The frame is basically in an undefined state after such a failure. If that's
> the case, then we don't actually have a leak here since av_frame_unref()
> will do the right thing and free the old mapping.

Right, but who will call this av_frame_unref(). The doxy for
av_hwframe_map() does not specify what precisely happens on failure, but
I would expect it to clean dst.

-- 
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 v2] lavc/qsvenc: add tile encoding support for VP9

[myp...@gmail.com]

On Mon, Jan 17, 2022 at 4:30 PM Xiang, Haihao
 wrote:
>
> On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:
> > Add -tile_rows and -tile_cols options to specify the number of tile
> > rows and columns
> >
> > Signed-off-by: Haihao Xiang 
> > ---
> > v2: add option descriptions in the doc
> >
> >  doc/encoders.texi   |  6 ++
> >  libavcodec/qsvenc.c |  4 
> >  libavcodec/qsvenc.h |  1 +
> >  libavcodec/qsvenc_vp9.c | 10 ++
> >  4 files changed, 21 insertions(+)
> >
> > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > index 7cc8be1209..a4176089d5 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
> >  @item profile2
> >  @item profile3
> >  @end table
> > +
> > +@item @var{tile_cols}
> > +Number of columns for tiled encoding (requires libmfx >= 1.29).
> > +
> > +@item @var{tile_rows}
> > +Number of rows for tiled encoding (requires libmfx  >= 1.29).
> >  @end table
> >
> >  @section snow
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > index 4e7a15f060..4cbc9ff4dc 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
> > QSVEncContext *q)
> >  q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
> >  q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
> >  q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
> > +#if QSV_HAVE_EXT_VP9_TILES
> > +q->extvp9param.NumTileColumns  = q->tile_cols;
> > +q->extvp9param.NumTileRows = q->tile_rows;
> > +#endif
> >  q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer 
> > *)&q-
> > >extvp9param;
> >  }
> >  #endif
> > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > index 31516b8e55..00ee52a5d1 100644
> > --- a/libavcodec/qsvenc.h
> > +++ b/libavcodec/qsvenc.h
> > @@ -41,6 +41,7 @@
> >
> >  #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
> >  #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
> > +#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)
> >
> >  #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
> >  #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> > diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
> > index 9329990d11..1168ddda0e 100644
> > --- a/libavcodec/qsvenc_vp9.c
> > +++ b/libavcodec/qsvenc_vp9.c
> > @@ -73,6 +73,16 @@ static const AVOption options[] = {
> >  { "profile2",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64 =
> > MFX_PROFILE_VP9_2   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> >  { "profile3",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64 =
> > MFX_PROFILE_VP9_3   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> >
> > +#if QSV_HAVE_EXT_VP9_TILES
> > +/* The minimum tile width in luma pixels is 256, set maximum tile_cols 
> > to
> > 32 for 8K video */
> > +{ "tile_cols",  "Number of columns for tiled
> > encoding",   OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 32,
> > VE },
> > +/* Set maximum tile_rows to 4 per VP9 spec */
> > +{ "tile_rows",  "Number of rows for tiled
> > encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 
> > 4,
> > VE },
> > +#else
> > +{ "tile_cols",  "(not
> > supported)",OFFSET(qsv.tile_cols),
> > AV_OPT_TYPE_INT,
> > { .i64 = 0 }, 0, 0, VE },
> > +{ "tile_rows",  "(not
> > supported)",OFFSET(qsv.tile_rows),
> > AV_OPT_TYPE_INT,
> > { .i64 = 0 }, 0, 0, VE },
> > +#endif
> > +
perfer one option like "-tile rows x cols" than two options like
"-tile_rows  row  -tile_cols  col"
> >  { NULL },
> >  };
> >
>
> Will apply,
>
> -Haihao
>
___
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] lavc/qsvenc: add tile encoding support for VP9

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> myp...@gmail.com
> Sent: Monday, January 17, 2022 11:36 AM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: add tile encoding support
> for VP9
> 
> On Mon, Jan 17, 2022 at 4:30 PM Xiang, Haihao
>  wrote:
> >
> > On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:
> > > Add -tile_rows and -tile_cols options to specify the number of tile
> > > rows and columns
> > >
> > > Signed-off-by: Haihao Xiang 
> > > ---
> > > v2: add option descriptions in the doc
> > >
> > >  doc/encoders.texi   |  6 ++
> > >  libavcodec/qsvenc.c |  4 
> > >  libavcodec/qsvenc.h |  1 +
> > >  libavcodec/qsvenc_vp9.c | 10 ++
> > >  4 files changed, 21 insertions(+)
> > >
> > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > index 7cc8be1209..a4176089d5 100644
> > > --- a/doc/encoders.texi
> > > +++ b/doc/encoders.texi
> > > @@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
> > >  @item profile2
> > >  @item profile3
> > >  @end table
> > > +
> > > +@item @var{tile_cols}
> > > +Number of columns for tiled encoding (requires libmfx >= 1.29).
> > > +
> > > +@item @var{tile_rows}
> > > +Number of rows for tiled encoding (requires libmfx  >= 1.29).
> > >  @end table
> > >
> > >  @section snow
> > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > > index 4e7a15f060..4cbc9ff4dc 100644
> > > --- a/libavcodec/qsvenc.c
> > > +++ b/libavcodec/qsvenc.c
> > > @@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
> > > QSVEncContext *q)
> > >  q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
> > >  q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
> > >  q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
> > > +#if QSV_HAVE_EXT_VP9_TILES
> > > +q->extvp9param.NumTileColumns  = q->tile_cols;
> > > +q->extvp9param.NumTileRows = q->tile_rows;
> > > +#endif
> > >  q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer
> *)&q-
> > > >extvp9param;
> > >  }
> > >  #endif
> > > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > > index 31516b8e55..00ee52a5d1 100644
> > > --- a/libavcodec/qsvenc.h
> > > +++ b/libavcodec/qsvenc.h
> > > @@ -41,6 +41,7 @@
> > >
> > >  #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
> > >  #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
> > > +#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)
> > >
> > >  #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
> > >  #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> > > diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
> > > index 9329990d11..1168ddda0e 100644
> > > --- a/libavcodec/qsvenc_vp9.c
> > > +++ b/libavcodec/qsvenc_vp9.c
> > > @@ -73,6 +73,16 @@ static const AVOption options[] = {
> > >  { "profile2",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64
> =
> > > MFX_PROFILE_VP9_2   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> > >  { "profile3",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64
> =
> > > MFX_PROFILE_VP9_3   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> > >
> > > +#if QSV_HAVE_EXT_VP9_TILES
> > > +/* The minimum tile width in luma pixels is 256, set maximum
> tile_cols to
> > > 32 for 8K video */
> > > +{ "tile_cols",  "Number of columns for tiled
> > > encoding",   OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
> 32,
> > > VE },
> > > +/* Set maximum tile_rows to 4 per VP9 spec */
> > > +{ "tile_rows",  "Number of rows for tiled
> > > encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 4,
> > > VE },
> > > +#else
> > > +{ "tile_cols",  "(not
> > > supported)",OFFSET(qsv.tile_cols),
> AV_OPT_TYPE_INT,
> > > { .i64 = 0 }, 0, 0, VE },
> > > +{ "tile_rows",  "(not
> > > supported)",OFFSET(qsv.tile_rows),
> AV_OPT_TYPE_INT,
> > > { .i64 = 0 }, 0, 0, VE },
> > > +#endif
> > > +
> perfer one option like "-tile rows x cols" than two options like

The example is invalid. (spaces)

> "-tile_rows  row  -tile_cols  col"

This way, the options are typed, have min, max and default values,
without needing any code to write and maintain.

if you mean "-tile rowsxcols", there's no option type for this, 
unless you would want to mis-use the video size type for it.
Otherwise you'd need to make it string and write all the parsing
and checking code for it. 
Do you think that it would be worth the effort?

Also, in most use cases, you will probably have rows or cols,
rather than both..

sw


___
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] lavc/qsvenc: add tile encoding support for VP9

[myp...@gmail.com]

On Mon, Jan 17, 2022 at 6:57 PM Soft Works  wrote:
>
>
>
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of
> > myp...@gmail.com
> > Sent: Monday, January 17, 2022 11:36 AM
> > To: FFmpeg development discussions and patches 
> > Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: add tile encoding 
> > support
> > for VP9
> >
> > On Mon, Jan 17, 2022 at 4:30 PM Xiang, Haihao
> >  wrote:
> > >
> > > On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:
> > > > Add -tile_rows and -tile_cols options to specify the number of tile
> > > > rows and columns
> > > >
> > > > Signed-off-by: Haihao Xiang 
> > > > ---
> > > > v2: add option descriptions in the doc
> > > >
> > > >  doc/encoders.texi   |  6 ++
> > > >  libavcodec/qsvenc.c |  4 
> > > >  libavcodec/qsvenc.h |  1 +
> > > >  libavcodec/qsvenc_vp9.c | 10 ++
> > > >  4 files changed, 21 insertions(+)
> > > >
> > > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > > index 7cc8be1209..a4176089d5 100644
> > > > --- a/doc/encoders.texi
> > > > +++ b/doc/encoders.texi
> > > > @@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
> > > >  @item profile2
> > > >  @item profile3
> > > >  @end table
> > > > +
> > > > +@item @var{tile_cols}
> > > > +Number of columns for tiled encoding (requires libmfx >= 1.29).
> > > > +
> > > > +@item @var{tile_rows}
> > > > +Number of rows for tiled encoding (requires libmfx  >= 1.29).
> > > >  @end table
> > > >
> > > >  @section snow
> > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > > > index 4e7a15f060..4cbc9ff4dc 100644
> > > > --- a/libavcodec/qsvenc.c
> > > > +++ b/libavcodec/qsvenc.c
> > > > @@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
> > > > QSVEncContext *q)
> > > >  q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
> > > >  q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
> > > >  q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
> > > > +#if QSV_HAVE_EXT_VP9_TILES
> > > > +q->extvp9param.NumTileColumns  = q->tile_cols;
> > > > +q->extvp9param.NumTileRows = q->tile_rows;
> > > > +#endif
> > > >  q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer
> > *)&q-
> > > > >extvp9param;
> > > >  }
> > > >  #endif
> > > > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > > > index 31516b8e55..00ee52a5d1 100644
> > > > --- a/libavcodec/qsvenc.h
> > > > +++ b/libavcodec/qsvenc.h
> > > > @@ -41,6 +41,7 @@
> > > >
> > > >  #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
> > > >  #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
> > > > +#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)
> > > >
> > > >  #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
> > > >  #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> > > > diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
> > > > index 9329990d11..1168ddda0e 100644
> > > > --- a/libavcodec/qsvenc_vp9.c
> > > > +++ b/libavcodec/qsvenc_vp9.c
> > > > @@ -73,6 +73,16 @@ static const AVOption options[] = {
> > > >  { "profile2",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64
> > =
> > > > MFX_PROFILE_VP9_2   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> > > >  { "profile3",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64
> > =
> > > > MFX_PROFILE_VP9_3   },  INT_MIN,  INT_MAX,  VE,  "profile" },
> > > >
> > > > +#if QSV_HAVE_EXT_VP9_TILES
> > > > +/* The minimum tile width in luma pixels is 256, set maximum
> > tile_cols to
> > > > 32 for 8K video */
> > > > +{ "tile_cols",  "Number of columns for tiled
> > > > encoding",   OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
> > 32,
> > > > VE },
> > > > +/* Set maximum tile_rows to 4 per VP9 spec */
> > > > +{ "tile_rows",  "Number of rows for tiled
> > > > encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 },
> > 0, 4,
> > > > VE },
> > > > +#else
> > > > +{ "tile_cols",  "(not
> > > > supported)",OFFSET(qsv.tile_cols),
> > AV_OPT_TYPE_INT,
> > > > { .i64 = 0 }, 0, 0, VE },
> > > > +{ "tile_rows",  "(not
> > > > supported)",OFFSET(qsv.tile_rows),
> > AV_OPT_TYPE_INT,
> > > > { .i64 = 0 }, 0, 0, VE },
> > > > +#endif
> > > > +
> > perfer one option like "-tile rows x cols" than two options like
>
> The example is invalid. (spaces)
>
> > "-tile_rows  row  -tile_cols  col"
>
> This way, the options are typed, have min, max and default values,
> without needing any code to write and maintain.
>
> if you mean "-tile rowsxcols", there's no option type for this,
> unless you would want to mis-use the video size type for it.
> Otherwise you'd need to make it string and write all the parsing
> and checking code for it.
> Do you think that it would be worth the effort?
You convinced me
>
> Also, in most use cases, you will probably have rows or cols,
> rather than both..
>
__

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Ronald S. Bultje]

Hi,

On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau 
wrote:

> Hi all,
>
> Recently I’ve submitted a patch that adds a config option to disable the
> caching of http redirects.
> We planned this as a workaround to the fact there’s a limit on the
> expiration that can be set on S3 pre-signed URLs.
> The idea was to have a service that generates signed S3 URLs and redirects
> to them, ffmpeg would hit this service
> on every seek/disconnect, and get a fresh S3 URL.
>
> We found that in some cases, ffmpeg seeks on every frame (probably due to
> some gap between the positions of
> video/audio frames in the file). In these cases, completely disabling the
> redirect caching becomes quite inefficient -
> our S3 signing service gets called ~20 times/sec by a single encoding task.
>
> The attached patch provides a more complete/correct implementation of
> redirect caching – the decision whether
> to cache/for how long is now determined according to the response from the
> server - status code (e.g. 301 vs 302),
> expires header & cache-control header. The behavior (detailed in the
> comment of the commit) is more aligned
> with how browsers handle it.
>
> In high level, these are the changes that were implemented in the patch –
>
>   1.  Added a dictionary on HTTPContext for keeping the cache – since I
> need to save both the target URL
> and the expiration, I’m formatting them together on a single string
> (“expiry;target-url”)
>   2.  Added a string on HTTPContext to save the value of the location
> header – this makes the existing
> flags new_location/location_changed redundant, and they were removed
>   3.  Added functions for parsing Cache-Control/Expires – for Expires I
> used the existing function for
> parsing cookie expiration time. The result is saved on a new integer on
> HTTPContent -
> a zero value means that no such header was encountered yet, a negative
> value means the response
> should not be cached.
> Once this patch is merged, IMHO we can remove the flag that I added in the
> previous patch (=always treat it as 0,
> and have the caching work based on the new dictionary). I will happily
> submit another patch for this.
>
> Thank you!
>

Will merge this in 1-2 days if there's no further comments.

Ronald
___
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] lavc/qsvenc: add tile encoding support for VP9

[James Almer]

On 1/17/2022 7:57 AM, Soft Works wrote:




-Original Message-
From: ffmpeg-devel  On Behalf Of
myp...@gmail.com
Sent: Monday, January 17, 2022 11:36 AM
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: add tile encoding support
for VP9

On Mon, Jan 17, 2022 at 4:30 PM Xiang, Haihao
 wrote:


On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:

Add -tile_rows and -tile_cols options to specify the number of tile
rows and columns

Signed-off-by: Haihao Xiang 
---
v2: add option descriptions in the doc

  doc/encoders.texi   |  6 ++
  libavcodec/qsvenc.c |  4 
  libavcodec/qsvenc.h |  1 +
  libavcodec/qsvenc_vp9.c | 10 ++
  4 files changed, 21 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 7cc8be1209..a4176089d5 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
  @item profile2
  @item profile3
  @end table
+
+@item @var{tile_cols}
+Number of columns for tiled encoding (requires libmfx >= 1.29).
+
+@item @var{tile_rows}
+Number of rows for tiled encoding (requires libmfx  >= 1.29).
  @end table

  @section snow
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 4e7a15f060..4cbc9ff4dc 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
QSVEncContext *q)
  q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
  q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
  q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
+#if QSV_HAVE_EXT_VP9_TILES
+q->extvp9param.NumTileColumns  = q->tile_cols;
+q->extvp9param.NumTileRows = q->tile_rows;
+#endif
  q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer

*)&q-

extvp9param;

  }
  #endif
diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
index 31516b8e55..00ee52a5d1 100644
--- a/libavcodec/qsvenc.h
+++ b/libavcodec/qsvenc.h
@@ -41,6 +41,7 @@

  #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
  #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
+#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)

  #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
  #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
index 9329990d11..1168ddda0e 100644
--- a/libavcodec/qsvenc_vp9.c
+++ b/libavcodec/qsvenc_vp9.c
@@ -73,6 +73,16 @@ static const AVOption options[] = {
  { "profile2",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64

=

MFX_PROFILE_VP9_2   },  INT_MIN,  INT_MAX,  VE,  "profile" },
  { "profile3",  NULL, 0,   AV_OPT_TYPE_CONST, { .i64

=

MFX_PROFILE_VP9_3   },  INT_MIN,  INT_MAX,  VE,  "profile" },

+#if QSV_HAVE_EXT_VP9_TILES
+/* The minimum tile width in luma pixels is 256, set maximum

tile_cols to

32 for 8K video */
+{ "tile_cols",  "Number of columns for tiled
encoding",   OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0,

32,

VE },
+/* Set maximum tile_rows to 4 per VP9 spec */
+{ "tile_rows",  "Number of rows for tiled
encoding",  OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 },

0, 4,

VE },
+#else
+{ "tile_cols",  "(not
supported)",OFFSET(qsv.tile_cols),

AV_OPT_TYPE_INT,

{ .i64 = 0 }, 0, 0, VE },
+{ "tile_rows",  "(not
supported)",OFFSET(qsv.tile_rows),

AV_OPT_TYPE_INT,

{ .i64 = 0 }, 0, 0, VE },
+#endif
+

perfer one option like "-tile rows x cols" than two options like


The example is invalid. (spaces)


"-tile_rows  row  -tile_cols  col"


This way, the options are typed, have min, max and default values,
without needing any code to write and maintain.

if you mean "-tile rowsxcols", there's no option type for this,
unless you would want to mis-use the video size type for it.
Otherwise you'd need to make it string and write all the parsing
and checking code for it.


We already have code for this. see libaomenc.


Do you think that it would be worth the effort?

Also, in most use cases, you will probably have rows or cols,
rather than both..

sw


___
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 001/281] Add a new channel layout API

[James Almer]

On 1/16/2022 7:54 PM, Marton Balint wrote:



On Sun, 16 Jan 2022, Nicolas George wrote:


James Almer (12022-01-12):

From: Anton Khirnov 

The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.

Deprecate the old API working with just uint64_t bitmasks.

Expanded and completed by Vittorio Giovara 
and James Almer .
Signed-off-by: Vittorio Giovara 
Signed-off-by: James Almer 
---
 libavutil/channel_layout.c | 629 -
 libavutil/channel_layout.h | 542 ++--
 libavutil/version.h    |   1 +
 3 files changed, 1069 insertions(+), 103 deletions(-)


Thank you. I have no fundamental objection to the design of the API as
it is, but the user interface and documentation is still missing, that
needs to be addressed before the patch goes in.

(But IIRC, Marton had other requirements, so let us wait for him to
weigh in.)


My extensible metadata idea was not popular, so I am willing to let it go.




+/**
+ * Initialize a channel layout from a given string description.
+ * The input string can be represented by:
+ *  - the formal channel layout name (returned by 
av_channel_layout_describe())

+ *  - single or multiple channel names (returned by av_channel_name()
+ *    or concatenated with "+")
+ *  - a hexadecimal value of a channel layout (eg. "0x4")
+ *  - the number of channels with default layout (eg. "5c")
+ *  - the number of unordered channels (eg. "4", "4C", or "4 channels")
+ *
+ * @param channel_layout input channel layout
+ * @param str string describing the channel layout
+ * @return 0 channel layout was detected, AVERROR_INVALIDATATA 
otherwise

+ */
+int av_channel_layout_from_string(AVChannelLayout *channel_layout,
+  const char *str);


The documentation for the syntax needs to be in the user documentation,
with examples, not just in the API documentation.


+/**
+ * This is the inverse function of @ref av_channel_name().
+ *
+ * @return the channel with the given name
+ * AV_CHAN_NONE when name does not identify a known channel
+ */
+enum AVChannel av_channel_from_string(const char *name);



+/**
+ * Get a channel described by the given string.
+ *
+ * This function accepts channel names in the same format as
+ * @ref av_channel_from_string().
+ *
+ * @param channel_layout input channel layout
+ * @return a channel described by the given string, or a negative 
AVERROR value.

+ */
+int av_channel_layout_channel_from_string(const AVChannelLayout 
*channel_layout,

+  const char *name);


This looks to be the preferred function for when the user will specify a
channel in a layout.

First, this fact should be stated clearly in the introduction of the
documentation. Otherwise, people will likely use other functions,
probably av_channel_layout_channel_from_index().

Second, the "name" argument cannot be just a name argument: the user
must be able to say "the third FC channel" or "the FC channel with name
'piano'". And probably both at once.

idx = av_channel_layout_channel_from_string(layout, "FC");
idx = av_channel_layout_channel_from_string(layout, "FC#2");
idx = av_channel_layout_channel_from_string(layout, "FC[piano]");
idx = av_channel_layout_channel_from_string(layout, "FC[piano]#2");

(I think it would be acceptable to limit the name, for example "names
with non-alphanumeric ASCII characters are not supported.)

And this need to go in the user documentation.

I am not sure if we also need a function to extract "all the FL channels
with name 'piano'".


Before discussing channel selection syntax, can we discuss serialization?

Should the serializaton and unserialization functions store/parse both 
the channel label and the channel designation? As far as I see right now 
it is kind of inconsistent: av_channel_layout_describe_print() prints 
the label (if exists), not the designation, but 
av_channel_layout_from_string() expects the designations only, never the 
custom label.


You're still thinking there's a distinction, when i already told you 
that there is none. I added the name field because people wanted to give 
non standard channel names, and maybe also change the standard ones too. 
It's not a label to go alongside a designation, it's *a* name.
There are about 20 channels that have a standard name from waveformat 
and extensions, while the rest lack one. You can obviously have a non 
standard speaker setup with 50 channels, and all those extra speakers 
can surely have a name based on their position (Say, RTFC, to refer to 
"right of top front center"), so the field lets you give it to them if 
that's convenient for you and you want a pretty print output of the 
layout without seeing things like USR49. That's it.


Yes, av_channel_layout_from_string() will not be able to parse the 
output of av_channel_layout_from_d

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/16/2022 8:27 AM, Nicolas George wrote:

James Almer (12022-01-12):

From: Anton Khirnov 

The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.

Deprecate the old API working with just uint64_t bitmasks.

Expanded and completed by Vittorio Giovara 
and James Almer .
Signed-off-by: Vittorio Giovara 
Signed-off-by: James Almer 
---
  libavutil/channel_layout.c | 629 -
  libavutil/channel_layout.h | 542 ++--
  libavutil/version.h|   1 +
  3 files changed, 1069 insertions(+), 103 deletions(-)


Thank you. I have no fundamental objection to the design of the API as
it is, but the user interface and documentation is still missing, that
needs to be addressed before the patch goes in.

(But IIRC, Marton had other requirements, so let us wait for him to
weigh in.)


+/**
+ * Initialize a channel layout from a given string description.
+ * The input string can be represented by:
+ *  - the formal channel layout name (returned by av_channel_layout_describe())
+ *  - single or multiple channel names (returned by av_channel_name()
+ *or concatenated with "+")
+ *  - a hexadecimal value of a channel layout (eg. "0x4")
+ *  - the number of channels with default layout (eg. "5c")
+ *  - the number of unordered channels (eg. "4", "4C", or "4 channels")
+ *
+ * @param channel_layout input channel layout
+ * @param str string describing the channel layout
+ * @return 0 channel layout was detected, AVERROR_INVALIDATATA otherwise
+ */
+int av_channel_layout_from_string(AVChannelLayout *channel_layout,
+  const char *str);


The documentation for the syntax needs to be in the user documentation,
with examples, not just in the API documentation.


I'll look into it.




+/**
+ * This is the inverse function of @ref av_channel_name().
+ *
+ * @return the channel with the given name
+ * AV_CHAN_NONE when name does not identify a known channel
+ */
+enum AVChannel av_channel_from_string(const char *name);



+/**
+ * Get a channel described by the given string.
+ *
+ * This function accepts channel names in the same format as
+ * @ref av_channel_from_string().
+ *
+ * @param channel_layout input channel layout
+ * @return a channel described by the given string, or a negative AVERROR 
value.
+ */
+int av_channel_layout_channel_from_string(const AVChannelLayout 
*channel_layout,
+  const char *name);


This looks to be the preferred function for when the user will specify a
channel in a layout.

First, this fact should be stated clearly in the introduction of the
documentation. Otherwise, people will likely use other functions,
probably av_channel_layout_channel_from_index().


And they can if they want to. It has a very specific purpose and it 
fulfills it.




Second, the "name" argument cannot be just a name argument: the user
must be able to say "the third FC channel" or "the FC channel with name
'piano'". And probably both at once.

idx = av_channel_layout_channel_from_string(layout, "FC");
idx = av_channel_layout_channel_from_string(layout, "FC#2");
idx = av_channel_layout_channel_from_string(layout, "FC[piano]");
idx = av_channel_layout_channel_from_string(layout, "FC[piano]#2");


Please, stop asking for this. It's an incredibly niche usecase you want 
for libavfilter, so you can and should implement it there. The API is 
there and you can use it, you don't need to overdesign these general 
purpose helpers to create these bizarre scenarios just to remove one or 
two filters from your chain.


You have the opaque field in both the layout and each channel. You can 
store a pointer there to anything you want at any point in your chain, 
like an internal and refcounted lavfi struct that stores all this 
information filters in the chain can parse and use.


This is a layout of speaker positions. If you want metadata, use metadata.



(I think it would be acceptable to limit the name, for example "names
with non-alphanumeric ASCII characters are not supported.)

And this need to go in the user documentation.

I am not sure if we also need a function to extract "all the FL channels
with name 'piano'".

Regards,


___
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 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> And they can if they want to. It has a very specific purpose and it fulfills
> it.

Just be clearer in the documentation.

> Please, stop asking for this. It's an incredibly niche usecase you want for
> libavfilter, so you can and should implement it there. The API is there and
> you can use it, you don't need to overdesign these general purpose helpers
> to create these bizarre scenarios just to remove one or two filters from
> your chain.

I will not stop asking for this, it is not specific to libavfilter: with
a quick search, I spot -map_channel that requires exactly the same
feature.

There may be filters that require an option to distinguish one channel
in particular; even if there are no now, there may be in the future. Can
you guarantee it does not and will not happen?

This use case happens at several places, therefore the parsing belongs
in a common API.

Regards,

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[Nicolas George]

Marton Balint (12022-01-16):
> Should the serializaton and unserialization functions store/parse both the
> channel label and the channel designation? As far as I see right now it is
> kind of inconsistent: av_channel_layout_describe_print() prints the label
> (if exists), not the designation, but av_channel_layout_from_string()
> expects the designations only, never the custom label.

Oh, excellent point. av_channel_layout_from_string() should be able to
parse the output of av_channel_layout_describe_bprint() in all cases.
And it should be covered by FATE.

Regards,

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 10:53 AM, Nicolas George wrote:

Marton Balint (12022-01-16):

Should the serializaton and unserialization functions store/parse both the
channel label and the channel designation? As far as I see right now it is
kind of inconsistent: av_channel_layout_describe_print() prints the label
(if exists), not the designation, but av_channel_layout_from_string()
expects the designations only, never the custom label.


Oh, excellent point. av_channel_layout_from_string() should be able to
parse the output of av_channel_layout_describe_bprint() in all cases.
And it should be covered by FATE.


Yes, which is why I'll make describe() stop looking at the name field.



Regards,


___
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 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> Yes, which is why I'll make describe() stop looking at the name field.

Unacceptable.

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 10:56 AM, Nicolas George wrote:

James Almer (12022-01-17):

Yes, which is why I'll make describe() stop looking at the name field.


Unacceptable.


Either that, or the field is removed. The opaque field is more than 
enough for your usecase. lavfi can and should use it to do everything 
you need.
Alternative, you can, at any time, extend this API yourself to cover all 
kinds of crazy musical instrumental scenarios. Create a new layout order 
and go nuts. But I'm not going to overdesign this API or its helpers 
just because you want less code in lavfi.


There are real world files and real world scenarios waiting for this API 
to land, unlike front center pianos and clarinets wishing they could 
skip one entry in your -filter_complex line, so it's not an excuse to 
delay it any longer.





___
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] avformat/imfdec: Use proper logcontext

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/imfdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
index 566a0fb792..d67c9b8898 100644
--- a/libavformat/imfdec.c
+++ b/libavformat/imfdec.c
@@ -200,7 +200,7 @@ static int parse_imf_asset_map_from_xml_dom(AVFormatContext 
*s,
elem_count + asset_map->asset_count,
sizeof(IMFAssetLocator));
 if (!tmp) {
-av_log(NULL, AV_LOG_ERROR, "Cannot allocate IMF asset locators\n");
+av_log(s, AV_LOG_ERROR, "Cannot allocate IMF asset locators\n");
 return AVERROR(ENOMEM);
 }
 asset_map->assets = tmp;
-- 
2.32.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 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 10:51 AM, Nicolas George wrote:

James Almer (12022-01-17):

And they can if they want to. It has a very specific purpose and it fulfills
it.


Just be clearer in the documentation.


Please, stop asking for this. It's an incredibly niche usecase you want for
libavfilter, so you can and should implement it there. The API is there and
you can use it, you don't need to overdesign these general purpose helpers
to create these bizarre scenarios just to remove one or two filters from
your chain.


I will not stop asking for this, it is not specific to libavfilter: with
a quick search, I spot -map_channel that requires exactly the same
feature.


-map_channel works perfectly with the current bitmask API, and the new one.



There may be filters that require an option to distinguish one channel
in particular; even if there are no now, there may be in the future. Can
you guarantee it does not and will not happen?


Look at their IDs? Look at their names? Give them names if you need to?
And again, you keep talking about filtering scenarios. Lavfi can and 
should handle all this on its own internally, just like it was designed 
to, and not pollute an API meant to define the position a channel is 
supposed to be.




This use case happens at several places, therefore the parsing belongs
in a common API.

Regards,


___
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] Website release notes for 5.0

[Lynne]

15 Jan 2022, 01:48 by d...@lynne.ee:

> 14 Jan 2022, 20:39 by d...@lynne.ee:
>
> Okay, the release will be officially out on 16:00 UTC+1 (CET) on Monday, the 
> 17th,
> so I'll push the update to the webpage then.
>

Website updated, so I think that makes the release officially announced.
___
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] [FFmpeg-cvslog] [ffmpeg-web] branch master updated. a23b3fe Website release notes for 5.0

[Gyan Doshi]

On 2022-01-17 08:30 pm, ffmpeg-...@ffmpeg.org wrote:

The branch, master has been updated
via  a23b3fe406dcabfbed5f5b31f3f07362173a10cb (commit)
   from  c362eec3ccd1ebe4dafc8500b70238d96aa4ba64 (commit)


- Log -
commit a23b3fe406dcabfbed5f5b31f3f07362173a10cb
Author: Lynne 
AuthorDate: Tue Jan 4 15:59:31 2022 +0100
Commit: Lynne 
CommitDate: Fri Jan 14 22:10:59 2022 +0100

 Website release notes for 5.0

diff --git a/src/download b/src/download
index b723674..7d78dd0 100644
--- a/src/download
+++ b/src/download
@@ -304,6 +304,42 @@ gpg: Good signature from "FFmpeg release signing key 
Changelog
+  https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/refs/heads/release/5.0:/RELEASE_NOTES";>Release 
Notes
+ 
+   
+
FFmpeg 4.4.1 "Rao"
  


diff --git a/src/index b/src/index
index 982fa33..62a514f 100644
--- a/src/index
+++ b/src/index
@@ -35,13 +35,73 @@
  News

  
+  January Nth, 2022, FFmpeg 5.0 "Lorentz"


"Nth"


+  
+FFmpeg 5.0 "Lorentz", a new
+major release, is now available! For this long-overdue release, a major 
effort
+underwent to remove the old encode/decode APIs and replace them with an
+N:M-based API, the entire libavresample library was removed, libswscale
+has a new, easier to use AVframe-based API, the Vulkan code was much 
improved,
+many new filters were added, including libplacebo integration, and finally,
+DoVi support was added, including tonemapping and remuxing. The default
+AAC encoder settings were also changed to improve quality.
+Some of the changelog highlights:
+  
+  
+ADPCM IMA Westwood encoder
+Westwood AUD muxer
+ADPCM IMA Acorn Replay decoder
+Argonaut Games CVG demuxer
+Argonaut Games CVG muxer
+Concatf protocol
+afwtdn audio filter
+audio and video segment filters
+Apple Graphics (SMC) encoder
+hsvkey and hsvhold video filters
+adecorrelate audio filter
+atilt audio filter
+grayworld video filter
+AV1 Low overhead bitstream format muxer
+swscale slice threading
+MSN Siren decoder
+scharr video filter
+apsyclip audio filter
+morpho video filter
+amr parser
+(a)latency filters
+GEM Raster image decoder
+asdr audio filter
+speex decoder
+limitdiff video filter
+xcorrelate video filter
+varblur video filter
+huesaturation video filter
+colorspectrum source video filter
+RTP packetizer for uncompressed video (RFC 4175)
+bitpacked encoder
+VideoToolbox VP9 hwaccel
+VideoToolbox ProRes hwaccel
+support loongarch.
+aspectralstats audio filter
+adynamicsmooth audio filter
+libplacebo filter
+vflip_vulkan, hflip_vulkan and flip_vulkan filters
+adynamicequalizer audio filter
+yadif_videotoolbox filter
+VideoToolbox ProRes encoder
+anlmf audio filter
+  
+  
+We strongly recommend users, distributors, and system integrators to
+upgrade unless they use current git master.
+  
+
June 19th, 2021, IRC

  We have a new IRC home at Libera Chat
  now! Feel free to join us at #ffmpeg and #ffmpeg-devel. More info at https://ffmpeg.org/contact.html#IRCChannels";>contact#IRCChannels

  
-

April 8th, 2021, FFmpeg 4.4 "Rao"

  FFmpeg 4.4 "Rao", a new

---

Summary of changes:
  src/download | 36 +++
  src/index| 62 +++-
  2 files changed, 97 insertions(+), 1 deletion(-)


hooks/post-receive


___
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] [FFmpeg-cvslog] [ffmpeg-web] branch master updated. a23b3fe Website release notes for 5.0

[Lynne]

17 Jan 2022, 16:04 by ffm...@gyani.pro:

>
>
> On 2022-01-17 08:30 pm, ffmpeg-...@ffmpeg.org wrote:
>
>> The branch, master has been updated
>>  via  a23b3fe406dcabfbed5f5b31f3f07362173a10cb (commit)
>>  from  c362eec3ccd1ebe4dafc8500b70238d96aa4ba64 (commit)
>>
>>
>> - Log -
>> commit a23b3fe406dcabfbed5f5b31f3f07362173a10cb
>> Author: Lynne 
>> AuthorDate: Tue Jan 4 15:59:31 2022 +0100
>> Commit: Lynne 
>> CommitDate: Fri Jan 14 22:10:59 2022 +0100
>>
>>  Website release notes for 5.0
>>
>> diff --git a/src/download b/src/download
>> index b723674..7d78dd0 100644
>> --- a/src/download
>> +++ b/src/download
>> @@ -304,6 +304,42 @@ gpg: Good signature from "FFmpeg release signing key 
>> >  and much faster bug fixes such as additional features and security patches.
>>  
>>  +  FFmpeg 5.0 "Lorentz"
>> +
>> +  
>> +5.0 was released on 2022-01-NN. It is the latest stable FFmpeg release
>>
>
> "NN"
>
>> +from the 5.0 release branch, which was cut from master on 2022-01-04.
>> +  
>> +  It includes the following library versions:
>> +  
>> +  
>> +libavutil  57. 17.100
>> +libavcodec 59. 18.100
>> +libavformat59. 16.100
>> +libavdevice59.  4.100
>> +libavfilter 8. 24.100
>> +libswscale  6.  4.100
>> +libswresample   4.  3.100
>> +libpostproc56.  3.100
>> +  
>> +
>> +  Download 
>> xz tarball
>> +  PGP 
>> signature
>> + 
>> +
>> +  > href="releases/ffmpeg-5.0.tar.bz2">Download bzip2 tarball
>> +  PGP 
>> signature
>> + 
>> +
>> +  Download 
>> gzip tarball
>> +  PGP 
>> signature
>> + 
>> +
>> +  > href="https://git.ffmpeg.org/gitweb/ffmpeg.git/shortlog/n5.0";>Changelog
>> +  > href="https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/refs/heads/release/5.0:/RELEASE_NOTES";>Release
>>  Notes
>> + 
>> +   
>> +
>>  FFmpeg 4.4.1 "Rao"
>>  
>> diff --git a/src/index b/src/index
>> index 982fa33..62a514f 100644
>> --- a/src/index
>> +++ b/src/index
>> @@ -35,13 +35,73 @@
>>  News
>>  
>>  +  January Nth, 2022, FFmpeg 5.0 "Lorentz"
>>
>
> "Nth"
>

Thanks, fixed both placeholders.
The only date I did check was the split-off date, so I thought
I fixed all.
___
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 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> -map_channel works perfectly with the current bitmask API, and the new one.

Not if there are several times the same channel with different labels.
It should.

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> Either that, or the field is removed.

Are we discussing together to design the best API possible or are you a
dictator making threats?

The label field stays, and the parse and stringify functions must be
reciprocal of each other. The API is unacceptable otherwise.

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 1:48 PM, Nicolas George wrote:

James Almer (12022-01-17):

-map_channel works perfectly with the current bitmask API, and the new one.


Not if there are several times the same channel with different labels.
It should.


Patches are always welcome for new or updated functionality.




___
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 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 1:50 PM, Nicolas George wrote:

James Almer (12022-01-17):

Either that, or the field is removed.


Are we discussing together to design the best API possible or are you a
dictator making threats?


Like you said below, the functions must be reciprocal, so they can't 
take that field into account. Removing the field is not necessary, but 
is another option.




The label field stays, and the parse and stringify functions must be
reciprocal of each other. The API is unacceptable otherwise.


___
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 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> Like you said below, the functions must be reciprocal, so they can't take
> that field into account.

A serializaion function is supposed to serialize the whole structure. I
will not accept less.

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 1:54 PM, Nicolas George wrote:

James Almer (12022-01-17):

Like you said below, the functions must be reciprocal, so they can't take
that field into account.


A serializaion function is supposed to serialize the whole structure. I
will not accept less.


It will print the whole structure, what makes you think it wont? It 
doesn't need custom names to be set to do that.





___
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 001/281] Add a new channel layout API

[Nicolas George]

James Almer (12022-01-17):
> It will print the whole structure, what makes you think it wont? It doesn't
> need custom names to be set to do that.

If it prints the whole structure, including the names, and if the
parsing function parses the whole result, including the names, it is
good. Anything else... needs more work.

-- 
  Nicolas George


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

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

[FFmpeg-devel] [PATCH 001/289 v4] Add a new channel layout API

[James Almer]

From: Anton Khirnov 

The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.

Deprecate the old API working with just uint64_t bitmasks.

Expanded and completed by Vittorio Giovara 
and James Almer .
Signed-off-by: Vittorio Giovara 
Signed-off-by: James Almer 
---
Changes since last version:

*Fixed av_channel_layout_from_string() to properly handle duplicate
 channels in the string.
*av_channel_layout_from_string() will no longer use custom names if
 present for a given channel in a Custom order layout as
 av_channel_layout_describe() can't parse them, and both functions
 must be reciprocal.
 Any changes to this can be done anytime in a backwards compatible
 way by anyone interested in doing so, but it needs to be thought
 carefully as these strings are printed in human readable output
 when reporting the layout all over the place.

*av_channel_layout_index_from_string() and
 av_channel_layout_channel_from_string() do still look for this field,
 and will use it to find the desired channel.
 Any changes to these functions to extend the supported syntax can be
 defined and designed later, and implemented by those interested in it
 as the need for them arises.

Will look into the documentation later.

 libavutil/channel_layout.c | 621 -
 libavutil/channel_layout.h | 545 ++--
 libavutil/version.h|   1 +
 3 files changed, 1064 insertions(+), 103 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index ac773a9e63..fbfa4aaa60 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -37,81 +37,151 @@ struct channel_name {
 };
 
 static const struct channel_name channel_names[] = {
- [0] = { "FL","front left"},
- [1] = { "FR","front right"   },
- [2] = { "FC","front center"  },
- [3] = { "LFE",   "low frequency" },
- [4] = { "BL","back left" },
- [5] = { "BR","back right"},
- [6] = { "FLC",   "front left-of-center"  },
- [7] = { "FRC",   "front right-of-center" },
- [8] = { "BC","back center"   },
- [9] = { "SL","side left" },
-[10] = { "SR","side right"},
-[11] = { "TC","top center"},
-[12] = { "TFL",   "top front left"},
-[13] = { "TFC",   "top front center"  },
-[14] = { "TFR",   "top front right"   },
-[15] = { "TBL",   "top back left" },
-[16] = { "TBC",   "top back center"   },
-[17] = { "TBR",   "top back right"},
-[29] = { "DL","downmix left"  },
-[30] = { "DR","downmix right" },
-[31] = { "WL","wide left" },
-[32] = { "WR","wide right"},
-[33] = { "SDL",   "surround direct left"  },
-[34] = { "SDR",   "surround direct right" },
-[35] = { "LFE2",  "low frequency 2"   },
-[36] = { "TSL",   "top side left" },
-[37] = { "TSR",   "top side right"},
-[38] = { "BFC",   "bottom front center"   },
-[39] = { "BFL",   "bottom front left" },
-[40] = { "BFR",   "bottom front right"},
+[AV_CHAN_FRONT_LEFT   ] = { "FL","front left"},
+[AV_CHAN_FRONT_RIGHT  ] = { "FR","front right"   },
+[AV_CHAN_FRONT_CENTER ] = { "FC","front center"  },
+[AV_CHAN_LOW_FREQUENCY] = { "LFE",   "low frequency" },
+[AV_CHAN_BACK_LEFT] = { "BL","back left" },
+[AV_CHAN_BACK_RIGHT   ] = { "BR","back right"},
+[AV_CHAN_FRONT_LEFT_OF_CENTER ] = { "FLC",   "front left-of-center"  },
+[AV_CHAN_FRONT_RIGHT_OF_CENTER] = { "FRC",   "front right-of-center" },
+[AV_CHAN_BACK_CENTER  ] = { "BC","back center"   },
+[AV_CHAN_SIDE_LEFT] = { "SL","side left" },
+[AV_CHAN_SIDE_RIGHT   ] = { "SR","side right"},
+[AV_CHAN_TOP_CENTER   ] = { "TC","top center"},
+[AV_CHAN_TOP_FRONT_LEFT   ] = { "TFL",   "top front left"},
+[AV_CHAN_TOP_FRONT_CENTER ] = { "TFC",   "top front center"  },
+[AV_CHAN_TOP_FRONT_RIGHT  ] = { "TFR",   "top front right"   },
+[AV_CHAN_TOP_BACK_LEFT] = { "TBL",   "top back left" },
+[AV_CHAN_TOP_BACK_CENTER  ] = { "TBC",   "top back center"   },
+[AV_CHAN_TOP_BACK_RIGHT   ] = { "TBR",   "top back right"},
+[AV_CHAN_STEREO_LEFT  ] = { "DL","downmix le

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[Marton Balint]

On Mon, 17 Jan 2022, James Almer wrote:




You're still thinking there's a distinction, when i already told you that 
there is none. I added the name field because people wanted to give non 
standard channel names, and maybe also change the standard ones too. It's not 
a label to go alongside a designation, it's *a* name.
There are about 20 channels that have a standard name from waveformat and 
extensions, while the rest lack one. You can obviously have a non standard 
speaker setup with 50 channels, and all those extra speakers can surely have 
a name based on their position (Say, RTFC, to refer to "right of top front 
center"), so the field lets you give it to them if that's convenient for you 
and you want a pretty print output of the layout without seeing things like 
USR49. That's it.


OK, but shouldn't the user be able to specify if it means a builtin name 
or a custom name when specifying a channel name?


That is why I suggested some additinal syntax for custom names 
in av_channel_layout_index_from_string() and 
av_channel_layout_channel_from_string(). Like "FL" is a builtin name, 
"@name" is a custom name.




Yes, av_channel_layout_from_string() will not be able to parse the output of 
av_channel_layout_from_describe() if you gave channels a custom name, since 
they are specifically from that other layout. There's no way around that, as 
we can't make describe() output some string that from_string() can interpret 
for those because then describe() will be useless for printing the layout for 
human readability purposes.
It is in fact a good reason to either remove the name field or stop making 
these helpers look at it, since describe() is meant to create strings 
from_string() can parse.


I personally would do just that and keep the opaque fields alone. Otherwise 
I'll make the helpers stop looking at it, since after your request, non 
standard channels can be addressed as USR#, so you can create layout from 
strings with them just fine.


Removing the custom names is fine with me. Maybe it's a compromise nobody 
is particularly happy about.


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

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

Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API

[James Almer]

On 1/17/2022 5:18 PM, Marton Balint wrote:



On Mon, 17 Jan 2022, James Almer wrote:




You're still thinking there's a distinction, when i already told you 
that there is none. I added the name field because people wanted to 
give non standard channel names, and maybe also change the standard 
ones too. It's not a label to go alongside a designation, it's *a* name.
There are about 20 channels that have a standard name from waveformat 
and extensions, while the rest lack one. You can obviously have a non 
standard speaker setup with 50 channels, and all those extra speakers 
can surely have a name based on their position (Say, RTFC, to refer to 
"right of top front center"), so the field lets you give it to them if 
that's convenient for you and you want a pretty print output of the 
layout without seeing things like USR49. That's it.


OK, but shouldn't the user be able to specify if it means a builtin name 
or a custom name when specifying a channel name?


That is why I suggested some additinal syntax for custom names in 
av_channel_layout_index_from_string() and 
av_channel_layout_channel_from_string(). Like "FL" is a builtin name, 
"@name" is a custom name.




Yes, av_channel_layout_from_string() will not be able to parse the 
output of av_channel_layout_from_describe() if you gave channels a 
custom name, since they are specifically from that other layout. 
There's no way around that, as we can't make describe() output some 
string that from_string() can interpret for those because then 
describe() will be useless for printing the layout for human 
readability purposes.
It is in fact a good reason to either remove the name field or stop 
making these helpers look at it, since describe() is meant to create 
strings from_string() can parse.


I personally would do just that and keep the opaque fields alone. 
Otherwise I'll make the helpers stop looking at it, since after your 
request, non standard channels can be addressed as USR#, so you can 
create layout from strings with them just fine.


Removing the custom names is fine with me. Maybe it's a compromise 
nobody is particularly happy about.


I will not remove it. It has its uses even if helpers don't make 
extended use of it at first.


Advanced syntax can be implemented later. See my comments in v4.



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

Re: [FFmpeg-devel] [PATCH 001/289 v4] Add a new channel layout API

[Marton Balint]

On Mon, 17 Jan 2022, James Almer wrote:

[...]


-static const char *get_channel_name(int channel_id)
+static const char *get_channel_name(enum AVChannel channel_id)
{
-if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
+if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names) ||
+!channel_names[channel_id].name)
return NULL;
return channel_names[channel_id].name;
}

-static const struct {
+void av_channel_name_bprint(AVBPrint *bp, enum AVChannel channel_id)
+{
+av_bprint_clear(bp);


Clearing should not be done here. Maybe the user wants to construct a
string, and the channel name is only a part of it. If not, the user can
clear the bprint buffer himself before calling this.


+
+if ((unsigned)channel_id < FF_ARRAY_ELEMS(channel_names))
+av_bprintf(bp, "%s", channel_names[channel_id].name);
+else
+av_bprintf(bp, "USR%d", channel_id);
+}
+
+int av_channel_name(char *buf, size_t buf_size, enum AVChannel channel_id)
+{
+AVBPrint bp;
+
+if (!buf && buf_size)
+return AVERROR(EINVAL);
+
+av_bprint_init_for_buffer(&bp, buf, buf_size);
+av_channel_name_bprint(&bp, channel_id);
+
+return bp.len;
+}
+
+void av_channel_description_bprint(AVBPrint *bp, enum AVChannel channel_id)
+{
+av_bprint_clear(bp);


Same here.


+
+if ((unsigned)channel_id < FF_ARRAY_ELEMS(channel_names))
+av_bprintf(bp, "%s", channel_names[channel_id].description);
+else
+av_bprintf(bp, "user %d", channel_id);
+}
+
+int av_channel_description(char *buf, size_t buf_size, enum AVChannel 
channel_id)
+{
+AVBPrint bp;
+
+if (!buf && buf_size)
+return AVERROR(EINVAL);
+
+av_bprint_init_for_buffer(&bp, buf, buf_size);
+av_channel_description_bprint(&bp, channel_id);
+
+return bp.len;
+}


[...]


+int av_channel_layout_from_string(AVChannelLayout *channel_layout,
+  const char *str)
+{
+int i;
+int channels = 0, native = 1;
+enum AVChannel highest_channel = AV_CHAN_NONE;
+const char *dup = str;
+char *end;
+uint64_t mask = 0;
+
+/* channel layout names */
+for (i = 0; i < FF_ARRAY_ELEMS(channel_layout_map); i++) {
+if (channel_layout_map[i].name && !strcmp(str, 
channel_layout_map[i].name)) {
+*channel_layout = channel_layout_map[i].layout;
+return 0;
+}
+}
+
+/* channel names */
+while (*dup) {
+char *chname = av_get_token(&dup, "+");
+if (!chname)
+return AVERROR(ENOMEM);
+if (*dup)
+dup++; // skip separator
+for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+if (channel_names[i].name && !strcmp(chname, 
channel_names[i].name)) {
+if (i < highest_channel || mask & (1ULL << i))
+native = 0; // Not a native layout, use a custom one
+highest_channel = i;
+mask |= 1ULL << i;
+break;
+}
+}
+
+if (i >= FF_ARRAY_ELEMS(channel_names)) {
+char *endptr = chname;
+enum AVChannel id = AV_CHAN_NONE;
+
+if (!strncmp(chname, "USR", 3)) {
+const char *p = chname + 3;
+id = strtol(p, &endptr, 0);
+}
+if (id < 0 || *endptr) {
+native = 0; // Unknown channel name
+channels = 0;
+mask = 0;
+av_free(chname);
+break;
+}
+if (id > 63)
+native = 0; // Not a native layout, use a custom one
+else {
+if (id < highest_channel || mask & (1ULL << id))
+native = 0; // Not a native layout, use a custom one
+highest_channel = id;
+mask |= 1ULL << id;
+}
+}
+channels++;
+av_free(chname);
+}
+if (mask && native) {
+av_channel_layout_from_mask(channel_layout, mask);
+return 0;
+}
+
+/* custom layout of channel names */
+if (channels && !native) {
+int idx = 0;
+
+channel_layout->u.map = av_calloc(channels, 
sizeof(*channel_layout->u.map));
+if (!channel_layout->u.map)
+return AVERROR(ENOMEM);
+
+channel_layout->order = AV_CHANNEL_ORDER_CUSTOM;
+channel_layout->nb_channels = channels;
+
+dup = str;
+while (*dup) {
+char *chname = av_get_token(&dup, "+");
+if (!chname) {
+av_freep(&channel_layout->u.map);
+return AVERROR(ENOMEM);
+}
+if (*dup)
+dup++; // skip separator
+for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+if (channel_names[i].name && !strcmp(chname, 
channel_names[i].name)) {
+channel_layout->u.map[idx++].id = i;
+break;
+  

[FFmpeg-devel] [PATCH 001/289 v5] Add a new channel layout API

[James Almer]

From: Anton Khirnov 

The new API is more extensible and allows for custom layouts.
More accurate information is exported, eg for decoders that do not
set a channel layout, lavc will not make one up for them.

Deprecate the old API working with just uint64_t bitmasks.

Expanded and completed by Vittorio Giovara 
and James Almer .
Signed-off-by: Vittorio Giovara 
Signed-off-by: James Almer 
---
Changes since last version:

*AVBprint based functions will append the string to the buffer instead
 of clearing it and writing it at the beginning.
*Fixed error return value in av_channel_layout_from_string().

Changes since v3:

*Fixed av_channel_layout_from_string() to properly handle duplicate
 channels in the string.
*av_channel_layout_from_string() will no longer use custom names if
 present for a given channel in a Custom order layout as
 av_channel_layout_describe() can't parse them, and both functions
 must be reciprocal.
 Any changes to this can be done anytime in a backwards compatible
 way by anyone interested in doing so, but it needs to be thought
 carefully as these strings are printed in human readable output
 when reporting the layout all over the place.

*av_channel_layout_index_from_string() and
 av_channel_layout_channel_from_string() do still look for this field,
 and will use it to find the desired channel.
 Any changes to these functions to extend the supported syntax can be
 defined and designed later, and implemented by those interested in it
 as the need for them arises.
 
 libavutil/channel_layout.c | 615 -
 libavutil/channel_layout.h | 550 +++--
 libavutil/version.h|   1 +
 3 files changed, 1063 insertions(+), 103 deletions(-)

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index ac773a9e63..c6a8dd5aec 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -37,81 +37,147 @@ struct channel_name {
 };
 
 static const struct channel_name channel_names[] = {
- [0] = { "FL","front left"},
- [1] = { "FR","front right"   },
- [2] = { "FC","front center"  },
- [3] = { "LFE",   "low frequency" },
- [4] = { "BL","back left" },
- [5] = { "BR","back right"},
- [6] = { "FLC",   "front left-of-center"  },
- [7] = { "FRC",   "front right-of-center" },
- [8] = { "BC","back center"   },
- [9] = { "SL","side left" },
-[10] = { "SR","side right"},
-[11] = { "TC","top center"},
-[12] = { "TFL",   "top front left"},
-[13] = { "TFC",   "top front center"  },
-[14] = { "TFR",   "top front right"   },
-[15] = { "TBL",   "top back left" },
-[16] = { "TBC",   "top back center"   },
-[17] = { "TBR",   "top back right"},
-[29] = { "DL","downmix left"  },
-[30] = { "DR","downmix right" },
-[31] = { "WL","wide left" },
-[32] = { "WR","wide right"},
-[33] = { "SDL",   "surround direct left"  },
-[34] = { "SDR",   "surround direct right" },
-[35] = { "LFE2",  "low frequency 2"   },
-[36] = { "TSL",   "top side left" },
-[37] = { "TSR",   "top side right"},
-[38] = { "BFC",   "bottom front center"   },
-[39] = { "BFL",   "bottom front left" },
-[40] = { "BFR",   "bottom front right"},
+[AV_CHAN_FRONT_LEFT   ] = { "FL","front left"},
+[AV_CHAN_FRONT_RIGHT  ] = { "FR","front right"   },
+[AV_CHAN_FRONT_CENTER ] = { "FC","front center"  },
+[AV_CHAN_LOW_FREQUENCY] = { "LFE",   "low frequency" },
+[AV_CHAN_BACK_LEFT] = { "BL","back left" },
+[AV_CHAN_BACK_RIGHT   ] = { "BR","back right"},
+[AV_CHAN_FRONT_LEFT_OF_CENTER ] = { "FLC",   "front left-of-center"  },
+[AV_CHAN_FRONT_RIGHT_OF_CENTER] = { "FRC",   "front right-of-center" },
+[AV_CHAN_BACK_CENTER  ] = { "BC","back center"   },
+[AV_CHAN_SIDE_LEFT] = { "SL","side left" },
+[AV_CHAN_SIDE_RIGHT   ] = { "SR","side right"},
+[AV_CHAN_TOP_CENTER   ] = { "TC","top center"},
+[AV_CHAN_TOP_FRONT_LEFT   ] = { "TFL",   "top front left"},
+[AV_CHAN_TOP_FRONT_CENTER ] = { "TFC",   "top front center"  },
+[AV_CHAN_TOP_FRONT_RIGHT  ] = { "TFR",   "top front right"   },
+[AV_CHAN_TOP_BACK_LEFT] = { "TBL",   "top back left" },
+[AV_CHAN_TOP_BACK_CENTER  ] = { "TBC",   "top back 

[FFmpeg-devel] avfilter/adelay - interactive commands

[deiwo deiwo]

Hello,
I would like to ask if it makes sense to implement ZeroMQ interactive
commands for the adelay filter, from the performance and memory point of
view.
If it does make sense then is it possible to malloc/free/realloc memory
within the command processing function? Can it cause noticeable delays in
the stream outputs?

I am new to the ffmpeg development so I am sorry if this kind of question
does not belong to this mailing list.

Thanks for the response,
David L
___
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 17/28] ffmpeg: move the mux queue into muxer private data

[Anton Khirnov]

Quoting Andreas Rheinhardt (2022-01-13 11:50:48)
> 
> My objections to adding a separately allocated muxing context and to
> this MuxStream have not changed. Both incur unnecessary allocations
> and indirections and (in case of the latter) loops;

I cannot imagine any remotely real situation where the cost of these
allocation and loops is anywhere close to being meaningful. So your
argument amounts to vastly premature optimization.

> the latter is also very unnatural. The patch here actually shows it:
> You only use the muxer context to get the MuxStream context
> corresponding to the OutputStream you are interested in:

I don't see how that shows any unnaturalness. As I said last time
already - the idea is to (eventually) split OutputStream into separate
encoding and muxing objects that can be used independently.

If you really insist I can wait until actually implementing that split,
but I'd rather avoid giant branches if possible.

> 
> >  for (i = 0; i < of->ctx->nb_streams; i++) {
> > +MuxStream *ms = &of->mux->streams[i];
> >  OutputStream *ost = output_streams[of->ost_index + i];
> 
> >  AVStream *st = ost->st;
> > +MuxStream *ms = &of->mux->streams[st->index];
> >  int ret;
> 
> Your aim of making sure what code can use/modify what parts can also be
> fulfilled by comments.

In my experience that simply does not work. People either don't read the
comments at all or read them once and forget. Did you see what happened
to all those fields in AVStream marked as "libavformat private use only"?

-- 
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 2/2] avformat/mov: Check size before subtraction

[Michael Niedermayer]

Fixes: signed integer overflow: -9223372036854775808 - 8 cannot be represented 
in type 'long'
Fixes: 
43542/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5237670148702208

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

diff --git a/libavformat/mov.c b/libavformat/mov.c
index e401cd39b5..063cc2bae2 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7329,6 +7329,8 @@ static int mov_read_default(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 if (a.size == 0) {
 a.size = atom.size - total_size + 8;
 }
+if (a.size < 0)
+break;
 a.size -= 8;
 if (a.size < 0)
 break;
-- 
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/2] avcodec/cfhd: Avoid signed integer overflow in coeff

[Michael Niedermayer]

Fixes: signed integer overflow: 15244032 * 256 cannot be represented in type 
'int'
Fixes: 
43504/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4865014842916864

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

diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
index 008a6360b6..ac7826250f 100644
--- a/libavcodec/cfhd.c
+++ b/libavcodec/cfhd.c
@@ -838,7 +838,7 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, 
int *got_frame,
 const uint16_t q = s->quantisation;
 
 for (i = 0; i < run; i++) {
-*coeff_data |= coeff * 256;
+*coeff_data |= coeff * 256U;
 *coeff_data++ *= q;
 }
 } else {
@@ -869,7 +869,7 @@ static int cfhd_decode(AVCodecContext *avctx, void *data, 
int *got_frame,
 const uint16_t q = s->quantisation;
 
 for (i = 0; i < run; i++) {
-*coeff_data |= coeff * 256;
+*coeff_data |= coeff * 256U;
 *coeff_data++ *= q;
 }
 } else {
-- 
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] [RFC] Next release and regressions

[Michael Niedermayer]

Hi

Trac lists 162 non closed bugs with keyword regression
https://trac.ffmpeg.org/query?status=new&status=open&status=reopened&keywords=~regression

Our next major release maybe will be next december

I suggest we try to reduce the number of regression bugs, and also to
add fate tests for as many of these when they are fixed as easily
possible.

Thanks


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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] [RFC] Next release and regressions

[Jean-Baptiste Kempf]

Heya,

On Mon, 17 Jan 2022, at 23:39, Michael Niedermayer wrote:
> Trac lists 162 non closed bugs with keyword regression
> https://trac.ffmpeg.org/query?status=new&status=open&status=reopened&keywords=~regression
>
> Our next major release maybe will be next december

When is 5.1.0 ? June/July?
If so, then reducing regressions would be very nice at that time.

jb
-- 
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] [RFC] Next release and regressions

[Michael Niedermayer]

On Mon, Jan 17, 2022 at 11:46:24PM +0100, Jean-Baptiste Kempf wrote:
> Heya,
> 
> On Mon, 17 Jan 2022, at 23:39, Michael Niedermayer wrote:
> > Trac lists 162 non closed bugs with keyword regression
> > https://trac.ffmpeg.org/query?status=new&status=open&status=reopened&keywords=~regression
> >
> > Our next major release maybe will be next december
> 
> When is 5.1.0 ? June/July?

Sounds like that would fit


> If so, then reducing regressions would be very nice at that time.

I think regressions should always be a thing to fix, not just
at time of a release.
Doing it just at a release will make the whole more entangled
and then when there are 30 issues one wants to fix and time for
only fixing 10 the fixes will be worse and people would skip
writing fate tests as thats the easiest to skip when time is
short

thx

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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] [RFC] Next release and regressions

[Marton Balint]

On Mon, 17 Jan 2022, Michael Niedermayer wrote:


Hi

Trac lists 162 non closed bugs with keyword regression
https://trac.ffmpeg.org/query?status=new&status=open&status=reopened&keywords=~regression

Our next major release maybe will be next december

I suggest we try to reduce the number of regression bugs, and also to
add fate tests for as many of these when they are fixed as easily
possible.


Regression fixes should be backported to stable branches, unless they are 
very invasive. I'd rather suggest a bug hunt of all issues categorized as 
important, mostly regressions, some crashes.


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

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

Re: [FFmpeg-devel] [PATCH] lavu/videotoolbox: add support for memory mapping frames

[Cameron Gutman]

> On Jan 17, 2022, at 3:07 AM, Anton Khirnov  wrote:
> 
> Quoting Cameron Gutman (2022-01-10 09:17:37)
>> 
>>> On Jan 9, 2022, at 3:24 AM, Anton Khirnov  wrote:
>>> 
>>> Quoting Cameron Gutman (2022-01-03 01:33:19)
 Signed-off-by: Cameron Gutman 
 ---
 libavutil/hwcontext_videotoolbox.c | 25 +
 1 file changed, 25 insertions(+)
 
 diff --git a/libavutil/hwcontext_videotoolbox.c 
 b/libavutil/hwcontext_videotoolbox.c
 index 0a8dbe9f33..026127d412 100644
 --- a/libavutil/hwcontext_videotoolbox.c
 +++ b/libavutil/hwcontext_videotoolbox.c
 @@ -711,6 +711,30 @@ fail:
return err;
 }
 
 +static int vt_map_from(AVHWFramesContext *hwfc, AVFrame *dst,
 +   const AVFrame *src, int flags)
 +{
 +int err;
 +
 +if (dst->format == AV_PIX_FMT_NONE)
 +dst->format = hwfc->sw_format;
 +else if (dst->format != hwfc->sw_format)
 +return AVERROR(ENOSYS);
 +
 +err = vt_map_frame(hwfc, dst, src, flags);
 +if (err)
 +return err;
 +
 +dst->width  = src->width;
 +dst->height = src->height;
 +
 +err = av_frame_copy_props(dst, src);
 +if (err)
 +return err;
>>> 
>>> Don't you need to unmap the frame in this error path?
>>> 
>> 
>> Maybe? It's complicated...
>> 
>> The mapping is still written to dst and it will be unmapped when
>> av_frame_unref() is called on dst. However, if the caller wants to try again
>> with same dst frame for some reason, then it looks like it will leak the 
>> first
>> "failed" mapping. AFAICT, another call to ff_hwframe_map_create() will
>> overwrite dst->buf[0] without unrefing it first, but that makes sense given
>> that the docs say "dst should be blank" (arguably that "should" ought to be a
>> "must" in this case). However, this isn't the full story.
>> 
>> None of the existing map_from() implementations (VAAPI, DRM, DXVA2) actually
>> unmap when av_frame_copy_props() fails. The only ones that don't have this 
>> bug
>> are OpenCL and Vulkan, and that's only because they forgot to call
>> av_frame_copy_props() entirely!
>> 
>> Ideally, you'd have nothing modified in dst when av_hwframe_map() fails,
>> but that isn't the case in practice. Much of the mapping process involves
>> irreversible changes to the dst frame, including overwriting dst->format,
>> dst->width, dst->height, dst->data, dst->linesize, even partially copied
>> frame properties prior to av_frame_copy_props() failing.
>> 
>> Given these limitations, it seems like the only sane thing to do with a dst
>> frame after failure of av_hwframe_map() (other than ENOSYS) is to unref/free.
>> The frame is basically in an undefined state after such a failure. If that's
>> the case, then we don't actually have a leak here since av_frame_unref()
>> will do the right thing and free the old mapping.
> 
> Right, but who will call this av_frame_unref(). The doxy for
> av_hwframe_map() does not specify what precisely happens on failure, but
> I would expect it to clean dst.
> 

Whoever allocated the frame will unref/free it when they are done with it.

For example, vf_hwmap.c does this:

dst = av_frame_alloc();
if (!dst) {
av_frame_free(&src);
return NULL;
}

err = av_hwframe_map(dst, src, ctx->mode);
if (err) {
av_log(avctx, AV_LOG_ERROR, "Failed to map frame to "
   "software: %d.\n", err);
av_frame_free(&src);
av_frame_free(&dst);
return NULL;
}

So the mapping will be freed in the call to av_frame_free(&dst) in this case.

Where we might run into issues with leaks is if someone does:

/* Try NV12 first */
dst->format = AV_PIX_FMT_NV12;
err = av_hwframe_map(dst, src, AV_HWFRAME_MAP_READ);
if (err) {
/* That didn't work. Now try YUV420P */
dst->format = AV_PIX_FMT_YUV420P;
err = av_hwframe_map(dst, src, AV_HWFRAME_MAP_READ);
}

Assuming that first av_hwframe_map() fails in av_frame_copy_props(), the second
call (if successful) will cause the first mapping to be leaked. For this case,
it makes sense to clear dst on failure.

The docs say "dst should be blank", so you could also argue that the second
call to av_hwframe_map() is incorrect, since it is not guaranteed that dst is
blank at that point (and today it is guaranteed _not_ to be blank for any of
the map_from() implementations in the case that av_frame_copy_props() fails).

If we do want dst to be cleared upon failure, a good place to do it might be in
av_hwframe_map() itself (since none of the existing map_from() implementations
do it themselves). That would formalize the behavior of cleaning dst on failure
rather than leaving the various hwcontext implementations to handle it how they
choose.

> -- 
> Anton Khirnov
>