Re: [FFmpeg-devel] [PATCH v2] lavu/hwcontext_vaapi: Add vaapi_drm_format_map support for x2rgb10

2023-08-09 Thread Xiang, Haihao 
On Wo, 2023-08-09 at 14:45 +0200, David Rosca wrote:
> Support for allocating frames with x2rgb10 format was added
> in c00264f5013, this adds support for importing DMA-BUFs.
> 
> v2: Fix #ifdef -> #if

Could you add the update history in annotate, not git commit log ?

Thanks
Haihao

> ---
>  libavutil/hwcontext_vaapi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 6c3a227ddd..558fed94c6 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -1048,6 +1048,9 @@ static const struct {
>  #if defined(VA_FOURCC_Y412) && defined(DRM_FORMAT_XVYU12_16161616)
>  DRM_MAP(Y412, 1, DRM_FORMAT_XVYU12_16161616),
>  #endif
> +#if defined(VA_FOURCC_X2R10G10B10) && defined(DRM_FORMAT_XRGB2101010)
> +    DRM_MAP(X2R10G10B10, 1, DRM_FORMAT_XRGB2101010),
> +#endif
>  };
>  #undef DRM_MAP
>  

___
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 v3] avcodec/cbs_vp8: Add support for VP8 codec bitstream READ methods

2023-08-09 Thread Xiang, Haihao 
On Ma, 2023-07-31 at 02:14 +, Dai, Jianhui J wrote:
> 
> 
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of Dai,
> > Jianhui J
> > Sent: Tuesday, June 20, 2023 9:42 AM
> > To: FFmpeg development discussions and patches 
> > Subject: Re: [FFmpeg-devel] [PATCH v3] avcodec/cbs_vp8: Add support for VP8
> > codec bitstream READ methods
> > 
> > 
> > 
> > > -Original Message-
> > > From: Dai, Jianhui J
> > > Sent: Thursday, June 8, 2023 11:18 AM
> > > To: FFmpeg development discussions and patches
> > > 
> > > Subject: RE: [PATCH v3] avcodec/cbs_vp8: Add support for VP8 codec
> > > bitstream READ methods
> > > 
> > > 
> > > 
> > > > -Original Message-
> > > > From: ffmpeg-devel  On Behalf Of
> > > > Dai, Jianhui J
> > > > Sent: Tuesday, May 30, 2023 4:00 PM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: [FFmpeg-devel] [PATCH v3] avcodec/cbs_vp8: Add support for
> > > > VP8 codec bitstream READ methods
> > > > 
> > > > This commit adds VP8 into cbs supported codec list, and enables the
> > > > `trace_headers` bitstream filters to support VP8, besides existing
> > > > AV1, H.264,
> > > > H.265 and VP9. It can be useful to debug VP8 stream issues.
> > > > 
> > > > Only the READ methods `read_unit` and `split_fragment` are
> > > > implemented, the WRITE methods `write_unit` and `assemble_fragment`
> > > > return `AVERROR_PATCHWELCOME` error code. It is because the CBS VP8
> > > > WRITE is unlikely used by any applications at the moment. The WRITE
> > > > methods can be added later if there are real requirments.
> > > > 
> > > > TESTS: ffmpeg -i fate-suite/vp8/frame_size_change.webm -vcodec copy
> > > > -bsf:v trace_headers -f null -
> > > > 
> > > > Signed-off-by: Jianhui Dai 
> > > > ---
> > > >  configure    |   4 +-
> > > >  libavcodec/Makefile  |   1 +
> > > >  libavcodec/cbs.c |   6 +
> > > >  libavcodec/cbs_internal.h    |   1 +
> > > >  libavcodec/cbs_vp8.c | 360 +++
> > > >  libavcodec/cbs_vp8.h | 112 +
> > > >  libavcodec/cbs_vp8_syntax_template.c | 224 +
> > > >  7 files changed, 707 insertions(+), 1 deletion(-)  create mode
> > > > 100644 libavcodec/cbs_vp8.c  create mode 100644 libavcodec/cbs_vp8.h
> > > > create mode
> > > > 100644 libavcodec/cbs_vp8_syntax_template.c
> > > > 
> > > > diff --git a/configure b/configure
> > > > index bb7be67676..b8960d2639 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -2432,6 +2432,7 @@ CONFIG_EXTRA="
> > > >  cbs_h265
> > > >  cbs_jpeg
> > > >  cbs_mpeg2
> > > > +    cbs_vp8
> > > >  cbs_vp9
> > > >  deflate_wrapper
> > > >  dirac_parse
> > > > @@ -2713,6 +2714,7 @@ cbs_h264_select="cbs"
> > > >  cbs_h265_select="cbs"
> > > >  cbs_jpeg_select="cbs"
> > > >  cbs_mpeg2_select="cbs"
> > > > +cbs_vp8_select="cbs"
> > > >  cbs_vp9_select="cbs"
> > > >  dct_select="rdft"
> > > >  deflate_wrapper_deps="zlib"
> > > > @@ -3284,7 +3286,7 @@ h264_redundant_pps_bsf_select="cbs_h264"
> > > >  hevc_metadata_bsf_select="cbs_h265"
> > > >  mjpeg2jpeg_bsf_select="jpegtables"
> > > >  mpeg2_metadata_bsf_select="cbs_mpeg2"
> > > > -trace_headers_bsf_select="cbs"
> > > > +trace_headers_bsf_select="cbs cbs_vp8"
> > > >  vp9_metadata_bsf_select="cbs_vp9"
> > > > 
> > > >  # external libraries
> > > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile index
> > > > 3cfb7e..1c4f0da1d2 100644
> > > > --- a/libavcodec/Makefile
> > > > +++ b/libavcodec/Makefile
> > > > @@ -78,6 +78,7 @@ OBJS-$(CONFIG_CBS_H264)    += cbs_h2645.o
> > > > cbs_sei.o h2645_parse.o
> > > >  OBJS-$(CONFIG_CBS_H265)    += cbs_h2645.o cbs_sei.o
> > h2645_parse.o
> > > >  OBJS-$(CONFIG_CBS_JPEG)    += cbs_jpeg.o
> > > >  OBJS-$(CONFIG_CBS_MPEG2)   += cbs_mpeg2.o
> > > > +OBJS-$(CONFIG_CBS_VP8) += cbs_vp8.o vpx_rac.o
> > > >  OBJS-$(CONFIG_CBS_VP9) += cbs_vp9.o
> > > >  OBJS-$(CONFIG_CRYSTALHD)   += crystalhd.o
> > > >  OBJS-$(CONFIG_DCT) += dct.o dct32_fixed.o
> > > > dct32_float.o
> > > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index
> > > > 504197e06d..c77110abb1
> > > > 100644
> > > > --- a/libavcodec/cbs.c
> > > > +++ b/libavcodec/cbs.c
> > > > @@ -46,6 +46,9 @@ static const CodedBitstreamType *const
> > > > cbs_type_table[] = {  #if CONFIG_CBS_MPEG2
> > > >  _cbs_type_mpeg2,
> > > >  #endif
> > > > +#if CONFIG_CBS_VP8
> > > > +    _cbs_type_vp8,
> > > > +#endif
> > > >  #if CONFIG_CBS_VP9
> > > >  _cbs_type_vp9,
> > > >  #endif
> > > > @@ -67,6 +70,9 @@ const enum AVCodecID ff_cbs_all_codec_ids[] = {
> > > > #if
> > > > CONFIG_CBS_MPEG2
> > > >  AV_CODEC_ID_MPEG2VIDEO,
> > > >  #endif
> > > > +#if CONFIG_CBS_VP8
> > > > +    AV_CODEC_ID_VP8,
> > > > +#endif
> > > >  #if CONFIG_CBS_VP9
> > > >  AV_CODEC_ID_VP9,
> > > >  #endif
> > > > diff

Re: [FFmpeg-devel] [PATCH v3 6/6] lavc/vaapi_encode: Add VAAPI AV1 encoder

2023-08-09 Thread Wang, Fei W 
On Mon, 2023-08-07 at 22:21 +0100, Mark Thompson wrote:
> On 03/08/2023 07:01, fei.w.wang-at-intel@ffmpeg.org wrote:
> > From: Fei Wang 
> > 
> > Signed-off-by: Fei Wang 
> > ---
> >   Changelog |1 +
> >   configure |3 +
> >   doc/encoders.texi |   13 +
> >   libavcodec/Makefile   |1 +
> >   libavcodec/allcodecs.c|1 +
> >   libavcodec/vaapi_encode.c |  125 +++-
> >   libavcodec/vaapi_encode.h |   12 +
> >   libavcodec/vaapi_encode_av1.c | 1229
> > +
> >   libavcodec/version.h  |2 +-
> >   9 files changed, 1368 insertions(+), 19 deletions(-)
> >   create mode 100644 libavcodec/vaapi_encode_av1.c
> 
> I assume this is tested on Intel hardware.  Is it tested on AMD as
> well?  (Apparently working in recent Mesa.)

AMD tested the patchset months ago:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22585

This patch changed a little compare with the version in Cartwheel,
@Ruijing, Could you help to review this version in ML? To avoid the
diffs break you. Thanks.

> 
> 
> > diff --git a/Changelog b/Changelog
> > index bbda4f4fd4..e86f742cd3 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -27,6 +27,7 @@ version :
> >   - Bitstream filter for converting VVC from MP4 to Annex B
> >   - scale_vt filter for videotoolbox
> >   - transpose_vt filter for videotoolbox
> > +- VAAPI AV1 encoder
> >   
> >   version 6.0:
> >   - Radiance HDR image support
> > diff --git a/configure b/configure
> > index 99388e7664..68a238a819 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3322,6 +3322,8 @@ av1_qsv_decoder_select="qsvdec"
> >   av1_qsv_encoder_select="qsvenc"
> >   av1_qsv_encoder_deps="libvpl"
> >   av1_amf_encoder_deps="amf"
> > +av1_vaapi_encoder_deps="VAEncPictureParameterBufferAV1"
> > +av1_vaapi_encoder_select="cbs_av1 vaapi_encode"
> >   
> >   # parsers
> >   aac_parser_select="adts_header mpeg4audio"
> > @@ -7106,6 +7108,7 @@ if enabled vaapi; then
> >   check_type "va/va.h va/va_enc_jpeg.h"
> > "VAEncPictureParameterBufferJPEG"
> >   check_type "va/va.h
> > va/va_enc_vp8.h"  "VAEncPictureParameterBufferVP8"
> >   check_type "va/va.h
> > va/va_enc_vp9.h"  "VAEncPictureParameterBufferVP9"
> > +check_type "va/va.h
> > va/va_enc_av1.h"  "VAEncPictureParameterBufferAV1"
> >   fi
> >   
> >   if enabled_all opencl libdrm ; then
> > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > index 25d6b7f09e..fb331ebd8e 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -3991,6 +3991,19 @@ Average variable bitrate.
> >   Each encoder also has its own specific options:
> >   @table @option
> >   
> > +@item av1_vaapi
> > +@option{profile} sets the value of @emph{seq_profile}.
> > +@option{tier} sets the value of @emph{seq_tier}.
> > +@option{level} sets the value of @emph{seq_level_idx}.
> > +
> > +@table @option
> > +@item tiles
> > +Set the number of tiles to encode the input video with, as columns
> > x rows.
> > +(default is 1x1).
> 
> Probably needs some clarification that large resolutions must be
> split into tiles?  Maybe an "auto" value for this (meaning use as few
> as possible), and let explicit "1x1" fail if the resolution is too
> large.
> 
> > +@item tile_groups
> > +Set tile groups number (default is 1).
> 
> Meaning what?  It splits into tile group OBUs containing equal
> numbers of tiles, or of equal size in bits, or something else?
> 
> > +@end table
> > +
> >   @item h264_vaapi
> >   @option{profile} sets the value of @emph{profile_idc} and the
> > @emph{constraint_set*_flag}s.
> >   @option{level} sets the value of @emph{level_idc}.
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index a6b2ecbb22..473afb4471 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -258,6 +258,7 @@ OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  +=
> > mediacodecdec.o
> >   OBJS-$(CONFIG_AV1_MEDIACODEC_ENCODER)  += mediacodecenc.o
> >   OBJS-$(CONFIG_AV1_NVENC_ENCODER)   += nvenc_av1.o nvenc.o
> >   OBJS-$(CONFIG_AV1_QSV_ENCODER) += qsvenc_av1.o
> > +OBJS-$(CONFIG_AV1_VAAPI_ENCODER)   += vaapi_encode_av1.o
> > av1_levels.o
> >   OBJS-$(CONFIG_AVRN_DECODER)+= avrndec.o
> >   OBJS-$(CONFIG_AVRP_DECODER)+= r210dec.o
> >   OBJS-$(CONFIG_AVRP_ENCODER)+= r210enc.o
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index 8775d15a4f..c43c1d7b48 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -844,6 +844,7 @@ extern const FFCodec ff_av1_nvenc_encoder;
> >   extern const FFCodec ff_av1_qsv_decoder;
> >   extern const FFCodec ff_av1_qsv_encoder;
> >   extern const FFCodec ff_av1_amf_encoder;
> > +extern const FFCodec ff_av1_vaapi_encoder;
> >   extern const FFCodec ff_libopenh264_encoder;
> >   extern const FFCodec ff_libopenh264_decoder;
> >   extern const FFCodec ff_h264_amf_encoder;
> > diff --git

Re: [FFmpeg-devel] [PATCH v7 0/5] JPEG XL Parser (and bug fixes)

2023-08-09 Thread Leo Izen 

On 8/2/23 16:33, Leo Izen wrote:

Changes from v6:
- Added dummy stub libavformat/jpegxl_parse.c to fix shared builds


Bumping for review. I know the commit message in patch 3/5 is wrong, but 
I can fix that on merge, or v8 if a v8 is necessary.


- Leo Izen

___
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] new audio filter and misc improvements

2023-08-09 Thread Paul B Mahol 
Patches attached.
From 2d8c330f543397642fa1afe0a01a67155008d4e1 Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Wed, 9 Aug 2023 21:53:04 +0200
Subject: [PATCH 3/3] avfilter/af_adeclick: do not output pointless message

Signed-off-by: Paul B Mahol 
---
 libavfilter/af_adeclick.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavfilter/af_adeclick.c b/libavfilter/af_adeclick.c
index fd0438d252..3401028c6b 100644
--- a/libavfilter/af_adeclick.c
+++ b/libavfilter/af_adeclick.c
@@ -721,9 +721,10 @@ static av_cold void uninit(AVFilterContext *ctx)
 {
 AudioDeclickContext *s = ctx->priv;
 
-av_log(ctx, AV_LOG_INFO, "Detected %s in %"PRId64" of %"PRId64" samples (%g%%).\n",
-   filter_modes[s->mode], s->detected_errors,
-   s->nb_samples, 100. * s->detected_errors / s->nb_samples);
+if (s->nb_samples > 0)
+av_log(ctx, AV_LOG_INFO, "Detected %s in %"PRId64" of %"PRId64" samples (%g%%).\n",
+   filter_modes[s->mode], s->detected_errors,
+   s->nb_samples, 100. * s->detected_errors / s->nb_samples);
 
 av_audio_fifo_free(s->fifo);
 av_audio_fifo_free(s->efifo);
-- 
2.39.1

From 0a6de9b1482ece4402b1c7274501b43e43f5f56a Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Wed, 9 Aug 2023 21:49:19 +0200
Subject: [PATCH 2/3] avfilter/af_adeclick: refactor and cleanup

Signed-off-by: Paul B Mahol 
---
 libavfilter/af_adeclick.c | 136 +++---
 1 file changed, 67 insertions(+), 69 deletions(-)

diff --git a/libavfilter/af_adeclick.c b/libavfilter/af_adeclick.c
index 1137dbe25b..fd0438d252 100644
--- a/libavfilter/af_adeclick.c
+++ b/libavfilter/af_adeclick.c
@@ -119,7 +119,6 @@ static int config_input(AVFilterLink *inlink)
 {
 AVFilterContext *ctx = inlink->dst;
 AudioDeclickContext *s = ctx->priv;
-int i;
 
 s->pts = AV_NOPTS_VALUE;
 s->window_size = inlink->sample_rate * s->w / 1000.;
@@ -134,7 +133,7 @@ static int config_input(AVFilterLink *inlink)
 s->window_func_lut = av_calloc(s->window_size, sizeof(*s->window_func_lut));
 if (!s->window_func_lut)
 return AVERROR(ENOMEM);
-for (i = 0; i < s->window_size; i++)
+for (int i = 0; i < s->window_size; i++)
 s->window_func_lut[i] = sin(M_PI * i / s->window_size) *
 (1. - (s->overlap / 100.)) * M_PI_2;
 
@@ -167,7 +166,7 @@ static int config_input(AVFilterLink *inlink)
 if (!s->chan)
 return AVERROR(ENOMEM);
 
-for (i = 0; i < inlink->ch_layout.nb_channels; i++) {
+for (int i = 0; i < inlink->ch_layout.nb_channels; i++) {
 DeclickChannel *c = >chan[i];
 
 c->detection = av_calloc(s->window_size, sizeof(*c->detection));
@@ -189,13 +188,13 @@ static int config_input(AVFilterLink *inlink)
 static void autocorrelation(const double *input, int order, int size,
 double *output, double scale)
 {
-int i, j;
-
-for (i = 0; i <= order; i++) {
+for (int i = 0; i <= order; i++) {
+const double *ninput = input + i;
+const int isize = size - i;
 double value = 0.;
 
-for (j = i; j < size; j++)
-value += input[j] * input[j - i];
+for (int j = 0; j < isize; j++)
+value += ninput[j] * input[j];
 
 output[i] = value * scale;
 }
@@ -205,7 +204,6 @@ static double autoregression(const double *samples, int ar_order,
  int nb_samples, double *k, double *r, double *a)
 {
 double alpha;
-int i, j;
 
 memset(a, 0, ar_order * sizeof(*a));
 
@@ -214,23 +212,23 @@ static double autoregression(const double *samples, int ar_order,
 /* Levinson-Durbin algorithm */
 k[0] = a[0] = -r[1] / r[0];
 alpha = r[0] * (1. - k[0] * k[0]);
-for (i = 1; i < ar_order; i++) {
+for (int i = 1; i < ar_order; i++) {
 double epsilon = 0.;
 
-for (j = 0; j < i; j++)
+for (int j = 0; j < i; j++)
 epsilon += a[j] * r[i - j];
 epsilon += r[i + 1];
 
 k[i] = -epsilon / alpha;
 alpha *= (1. - k[i] * k[i]);
-for (j = i - 1; j >= 0; j--)
+for (int j = i - 1; j >= 0; j--)
 k[j] = a[j] + k[i] * a[i - j - 1];
-for (j = 0; j <= i; j++)
+for (int j = 0; j <= i; j++)
 a[j] = k[j];
 }
 
 k[0] = 1.;
-for (i = 1; i <= ar_order; i++)
+for (int i = 1; i <= ar_order; i++)
 k[i] = a[i - 1];
 
 return sqrt(alpha);
@@ -238,9 +236,7 @@ static double autoregression(const double *samples, int ar_order,
 
 static int isfinite_array(double *samples, int nb_samples)
 {
-int i;
-
-for (i = 0; i < nb_samples; i++)
+for (int i = 0; i < nb_samples; i++)
 if (!isfinite(samples[i]))
 return 0;
 
@@ -272,14 +268,12 @@ static int find_index(int *index, int value, int size)
 
 static int factorization(double *matrix, int n)
 {
-int i, j, k;
-
-for (i = 0; i < n;

Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread Paul B Mahol 
On Wed, Aug 9, 2023 at 11:15 PM James Almer  wrote:

> On 8/9/2023 6:20 PM, Paul B Mahol wrote:
> > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> mich...@niedermayer.cc>
> > wrote:
> >
> >> Hi Paul
> >>
> >> On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> >>> This is not correct, and please stop writing such patches. Thanks.
> >>
> >> If there is a problem in the bugfix, please explain what the problem is.
> >> If you do not like the specific fix, you can fix it differently too or
> >> tell me what you prefer.
> >> Simply saying "no" with no explanation repeatedly is rude
> >>
> >
> > Patch breaks valid files.
>
> Can those files be added to FATE alongside a test?
>

Sure, once SDR files have been removed.


>
> >
> >
> >>
> >> thank you
> >>
> >> [...]
> >> --
> >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >>
> >> I have often repented speaking, but never of holding my tongue.
> >> -- Xenocrates
> >> ___
> >> 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".

Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread James Almer 

On 8/9/2023 6:20 PM, Paul B Mahol wrote:

On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer 
wrote:


Hi Paul

On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:

This is not correct, and please stop writing such patches. Thanks.


If there is a problem in the bugfix, please explain what the problem is.
If you do not like the specific fix, you can fix it differently too or
tell me what you prefer.
Simply saying "no" with no explanation repeatedly is rude



Patch breaks valid files.


Can those files be added to FATE alongside a test?






thank you

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
___
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".

Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread Paul B Mahol 
On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer 
wrote:

> Hi Paul
>
> On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > This is not correct, and please stop writing such patches. Thanks.
>
> If there is a problem in the bugfix, please explain what the problem is.
> If you do not like the specific fix, you can fix it differently too or
> tell me what you prefer.
> Simply saying "no" with no explanation repeatedly is rude
>

Patch breaks valid files.


>
> thank you
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
> ___
> 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 v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread Michael Niedermayer 
Hi Paul

On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> This is not correct, and please stop writing such patches. Thanks.

If there is a problem in the bugfix, please explain what the problem is.
If you do not like the specific fix, you can fix it differently too or
tell me what you prefer.
Simply saying "no" with no explanation repeatedly is rude

thank you

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread Paul B Mahol 
This is not correct, and please stop writing such patches. Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation

2023-08-09 Thread Michael Niedermayer 
Fixes: Timeout
Fixes: 
60867/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MV30_fuzzer-6381933108527104
Fixes: 
30147/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MV30_fuzzer-5549246684200960

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

diff --git a/libavcodec/mv30.c b/libavcodec/mv30.c
index 0b19534b00..addf188c92 100644
--- a/libavcodec/mv30.c
+++ b/libavcodec/mv30.c
@@ -411,6 +411,8 @@ static int decode_intra(AVCodecContext *avctx, 
GetBitContext *gb, AVFrame *frame
 mgb = *gb;
 if (get_bits_left(gb) < s->mode_size * 8)
 return AVERROR_INVALIDDATA;
+if (s->mode_size * 8 < (avctx->height + 15)/16 * ((avctx->width + 15)/16) 
* 12)
+return AVERROR_INVALIDDATA;
 
 skip_bits_long(gb, s->mode_size * 8);
 
@@ -476,6 +478,9 @@ static int decode_inter(AVCodecContext *avctx, 
GetBitContext *gb,
 int ret, cnt = 0;
 int flags = 0;
 
+if (get_bits_left(gb) < (mask_size + s->mode_size) * 8)
+return AVERROR_INVALIDDATA;
+
 if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
 return ret;
 
-- 
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 v2] avcodec/dovi_rpu: verify RPU data CRC32

2023-08-09 Thread quietvoid 


On 09/08/2023 13.08, James Almer wrote:

On 8/9/2023 2:05 PM, quietvoid wrote:

The Dolby Vision RPU contains a CRC32 to validate the payload against.
The implementation is CRC32/MPEG-2.

The CRC is only verified with the AV_EF_CRCCHECK flag.

Signed-off-by: quietvoid 
---
  libavcodec/dovi_rpu.c | 45 ---
  libavcodec/dovi_rpu.h |  3 ++-
  libavcodec/hevcdec.c  |  3 ++-
  3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index dd38936552..9ada0aceec 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
   */
    #include "libavutil/buffer.h"
+#include "libavutil/crc.h"
    #include "dovi_rpu.h"
  #include "golomb.h"
@@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext 
*gb, const AVDOVIRpuDataHeader *

} \
  } while (0)
  -int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t 
rpu_size)
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t 
rpu_size,

+  int err_recognition)
  {
  AVDOVIRpuDataHeader *hdr = >header;
  GetBitContext *gb = &(GetBitContext){0};
  DOVIVdrRef *vdr;
  int ret;
  +    size_t actual_rpu_size;
+    uint8_t trailing_zeroes = 0;
+
  uint8_t nal_prefix;
  uint8_t rpu_type;
  uint8_t vdr_seq_info_present;
@@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const 
uint8_t *rpu, size_t rpu_size)

  uint8_t use_prev_vdr_rpu;
  uint8_t use_nlq;
  uint8_t profile;
-    if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+
+    uint32_t rpu_data_crc32;
+    uint32_t computed_crc32;
+
+    for (int i = rpu_size - 1; i > 0; i--) {
+    if (!rpu[i]) {
+    trailing_zeroes++;
+    } else {
+    break;
+    }
+    }
+
+    actual_rpu_size = rpu_size - trailing_zeroes;
+
+    /* Exclude trailing byte (0x80) from reader */
+    if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0)
  return ret;
    /* RPU header, common values */
@@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const 
uint8_t *rpu, size_t rpu_size)

  color->source_diagonal = get_bits(gb, 10);
  }
  -    /* FIXME: verify CRC32, requires implementation of 
AV_CRC_32_MPEG_2 */

+    /* Skip unsupported until CRC32 */
+    skip_bits_long(gb, get_bits_left(gb) - 32);


This can be done after the err_recognition check.


+
+    if (err_recognition & AV_EF_CRCCHECK) {


Do instead

if (!(err_recognition & AV_EF_CRCCHECK))
    return 0;

To remove one level of indentation.
Thanks, sent v3: 
https://ffmpeg.org/pipermail/ffmpeg-devel/2023-August/313141.html

___
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 v3] avcodec/dovi_rpu: verify RPU data CRC32

2023-08-09 Thread quietvoid 
The Dolby Vision RPU contains a CRC32 to validate the payload against.
The implementation is CRC32/MPEG-2.

The CRC is only verified with the AV_EF_CRCCHECK flag.

Signed-off-by: quietvoid 
---
 libavcodec/dovi_rpu.c | 46 ---
 libavcodec/dovi_rpu.h |  3 ++-
 libavcodec/hevcdec.c  |  3 ++-
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index dd38936552..1dfeee7564 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
  */
 
 #include "libavutil/buffer.h"
+#include "libavutil/crc.h"
 
 #include "dovi_rpu.h"
 #include "golomb.h"
@@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext *gb, 
const AVDOVIRpuDataHeader *
 }  
 \
 } while (0)
 
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition)
 {
 AVDOVIRpuDataHeader *hdr = >header;
 GetBitContext *gb = &(GetBitContext){0};
 DOVIVdrRef *vdr;
 int ret;
 
+size_t actual_rpu_size;
+uint8_t trailing_zeroes = 0;
+
 uint8_t nal_prefix;
 uint8_t rpu_type;
 uint8_t vdr_seq_info_present;
@@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 uint8_t use_prev_vdr_rpu;
 uint8_t use_nlq;
 uint8_t profile;
-if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+
+uint32_t rpu_data_crc32;
+uint32_t computed_crc32;
+
+for (int i = rpu_size - 1; i > 0; i--) {
+if (!rpu[i]) {
+trailing_zeroes++;
+} else {
+break;
+}
+}
+
+actual_rpu_size = rpu_size - trailing_zeroes;
+
+/* Exclude trailing byte (0x80) from reader */
+if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0)
 return ret;
 
 /* RPU header, common values */
@@ -440,7 +460,27 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 color->source_diagonal = get_bits(gb, 10);
 }
 
-/* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */
+if (!(err_recognition & AV_EF_CRCCHECK))
+return 0;
+
+/* Skip unsupported until CRC32 */
+skip_bits_long(gb, get_bits_left(gb) - 32);
+
+rpu_data_crc32 = get_bits_long(gb, 32);
+
+/* Verify CRC32, buffer excludes the prefix, CRC32 and trailing byte */
+computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE),
+   -1, rpu + 1, actual_rpu_size - 6));
+
+if (rpu_data_crc32 != computed_crc32) {
+av_log(s->logctx, AV_LOG_ERROR,
+   "RPU CRC mismatch! Expected %"PRIu32", received %"PRIu32"\n",
+   rpu_data_crc32, computed_crc32);
+
+if (err_recognition & AV_EF_EXPLODE)
+goto fail;
+}
+
 return 0;
 
 fail:
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index f6ca5bbbc5..2b993a72c6 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -77,7 +77,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const 
AVDOVIDecoderConfigurationRecord *
  *
  * Returns 0 or an error code.
  */
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size);
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition);
 
 /**
  * Attach the decoded AVDOVIMetadata as side data to an AVFrame.
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index df40c91ba6..81b1a84625 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3182,7 +3182,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t 
*buf, int length)
 return AVERROR(ENOMEM);
 memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2);
 
-ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2);
+ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2,
+s->avctx->err_recognition);
 if (ret < 0) {
 av_buffer_unref(>rpu_buf);
 av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL unit.\n");
-- 
2.41.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/dovi_rpu: verify RPU data CRC32

2023-08-09 Thread James Almer 

On 8/9/2023 2:05 PM, quietvoid wrote:

The Dolby Vision RPU contains a CRC32 to validate the payload against.
The implementation is CRC32/MPEG-2.

The CRC is only verified with the AV_EF_CRCCHECK flag.

Signed-off-by: quietvoid 
---
  libavcodec/dovi_rpu.c | 45 ---
  libavcodec/dovi_rpu.h |  3 ++-
  libavcodec/hevcdec.c  |  3 ++-
  3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index dd38936552..9ada0aceec 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
   */
  
  #include "libavutil/buffer.h"

+#include "libavutil/crc.h"
  
  #include "dovi_rpu.h"

  #include "golomb.h"
@@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext *gb, 
const AVDOVIRpuDataHeader *
  } 
  \
  } while (0)
  
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)

+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition)
  {
  AVDOVIRpuDataHeader *hdr = >header;
  GetBitContext *gb = &(GetBitContext){0};
  DOVIVdrRef *vdr;
  int ret;
  
+size_t actual_rpu_size;

+uint8_t trailing_zeroes = 0;
+
  uint8_t nal_prefix;
  uint8_t rpu_type;
  uint8_t vdr_seq_info_present;
@@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
  uint8_t use_prev_vdr_rpu;
  uint8_t use_nlq;
  uint8_t profile;
-if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+
+uint32_t rpu_data_crc32;
+uint32_t computed_crc32;
+
+for (int i = rpu_size - 1; i > 0; i--) {
+if (!rpu[i]) {
+trailing_zeroes++;
+} else {
+break;
+}
+}
+
+actual_rpu_size = rpu_size - trailing_zeroes;
+
+/* Exclude trailing byte (0x80) from reader */
+if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0)
  return ret;
  
  /* RPU header, common values */

@@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
  color->source_diagonal = get_bits(gb, 10);
  }
  
-/* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */

+/* Skip unsupported until CRC32 */
+skip_bits_long(gb, get_bits_left(gb) - 32);


This can be done after the err_recognition check.


+
+if (err_recognition & AV_EF_CRCCHECK) {


Do instead

if (!(err_recognition & AV_EF_CRCCHECK))
return 0;

To remove one level of indentation.


+rpu_data_crc32 = get_bits_long(gb, 32);
+
+/* Verify CRC32, buffer excludes the prefix, CRC32 and trailing byte */
+computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE),
+   -1, rpu + 1, actual_rpu_size - 6));
+
+if (rpu_data_crc32 != computed_crc32) {
+av_log(s->logctx, AV_LOG_ERROR,
+   "RPU CRC mismatch! Expected %"PRIu32", received 
%"PRIu32"\n",
+   rpu_data_crc32, computed_crc32);
+
+if (err_recognition & AV_EF_EXPLODE)
+goto fail;
+}
+}
+
  return 0;
  
  fail:

diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index f6ca5bbbc5..2b993a72c6 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -77,7 +77,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const 
AVDOVIDecoderConfigurationRecord *
   *
   * Returns 0 or an error code.
   */
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size);
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition);
  
  /**

   * Attach the decoded AVDOVIMetadata as side data to an AVFrame.
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index df40c91ba6..81b1a84625 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3182,7 +3182,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t 
*buf, int length)
  return AVERROR(ENOMEM);
  memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2);
  
-ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2);

+ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2,
+s->avctx->err_recognition);
  if (ret < 0) {
  av_buffer_unref(>rpu_buf);
  av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL 
unit.\n");

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

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

[FFmpeg-devel] [PATCH v2] avcodec/dovi_rpu: verify RPU data CRC32

2023-08-09 Thread quietvoid 
The Dolby Vision RPU contains a CRC32 to validate the payload against.
The implementation is CRC32/MPEG-2.

The CRC is only verified with the AV_EF_CRCCHECK flag.

Signed-off-by: quietvoid 
---
 libavcodec/dovi_rpu.c | 45 ---
 libavcodec/dovi_rpu.h |  3 ++-
 libavcodec/hevcdec.c  |  3 ++-
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index dd38936552..9ada0aceec 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
  */
 
 #include "libavutil/buffer.h"
+#include "libavutil/crc.h"
 
 #include "dovi_rpu.h"
 #include "golomb.h"
@@ -191,13 +192,17 @@ static inline int64_t get_se_coef(GetBitContext *gb, 
const AVDOVIRpuDataHeader *
 }  
 \
 } while (0)
 
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition)
 {
 AVDOVIRpuDataHeader *hdr = >header;
 GetBitContext *gb = &(GetBitContext){0};
 DOVIVdrRef *vdr;
 int ret;
 
+size_t actual_rpu_size;
+uint8_t trailing_zeroes = 0;
+
 uint8_t nal_prefix;
 uint8_t rpu_type;
 uint8_t vdr_seq_info_present;
@@ -205,7 +210,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 uint8_t use_prev_vdr_rpu;
 uint8_t use_nlq;
 uint8_t profile;
-if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+
+uint32_t rpu_data_crc32;
+uint32_t computed_crc32;
+
+for (int i = rpu_size - 1; i > 0; i--) {
+if (!rpu[i]) {
+trailing_zeroes++;
+} else {
+break;
+}
+}
+
+actual_rpu_size = rpu_size - trailing_zeroes;
+
+/* Exclude trailing byte (0x80) from reader */
+if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0)
 return ret;
 
 /* RPU header, common values */
@@ -440,7 +460,26 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 color->source_diagonal = get_bits(gb, 10);
 }
 
-/* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */
+/* Skip unsupported until CRC32 */
+skip_bits_long(gb, get_bits_left(gb) - 32);
+
+if (err_recognition & AV_EF_CRCCHECK) {
+rpu_data_crc32 = get_bits_long(gb, 32);
+
+/* Verify CRC32, buffer excludes the prefix, CRC32 and trailing byte */
+computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE),
+   -1, rpu + 1, actual_rpu_size - 6));
+
+if (rpu_data_crc32 != computed_crc32) {
+av_log(s->logctx, AV_LOG_ERROR,
+   "RPU CRC mismatch! Expected %"PRIu32", received 
%"PRIu32"\n",
+   rpu_data_crc32, computed_crc32);
+
+if (err_recognition & AV_EF_EXPLODE)
+goto fail;
+}
+}
+
 return 0;
 
 fail:
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index f6ca5bbbc5..2b993a72c6 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -77,7 +77,8 @@ void ff_dovi_update_cfg(DOVIContext *s, const 
AVDOVIDecoderConfigurationRecord *
  *
  * Returns 0 or an error code.
  */
-int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size);
+int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
+  int err_recognition);
 
 /**
  * Attach the decoded AVDOVIMetadata as side data to an AVFrame.
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index df40c91ba6..81b1a84625 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3182,7 +3182,8 @@ static int decode_nal_units(HEVCContext *s, const uint8_t 
*buf, int length)
 return AVERROR(ENOMEM);
 memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2);
 
-ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2);
+ret = ff_dovi_rpu_parse(>dovi_ctx, nal->data + 2, nal->size - 2,
+s->avctx->err_recognition);
 if (ret < 0) {
 av_buffer_unref(>rpu_buf);
 av_log(s->avctx, AV_LOG_WARNING, "Error parsing DOVI NAL unit.\n");
-- 
2.41.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 v4 2/4] mpegts: Stash original PTS for SCTE-35 sections for processing later

2023-08-09 Thread Devin Heitmueller 
Hi Kieran,

Thanks for your review.

On Wed, Aug 9, 2023 at 9:55 AM Kieran Kunhya  wrote:
> How is this frame accurate? Surely "last_pcr" can be up to 100ms out. You 
> need to actually be interpolating the true value in order to be frame 
> accurate (not saying this is easy/doable in FFmpeg). But at the same time 
> inaccurate splices aren't great either.

So it's worth noting that the patch I've proposed doesn't change the
existing logic in terms of how the timestamp is determined.  The patch
in question simply makes the existing timestamp available as side
data.

Second, in most cases the accuracy of the timestamp for the SCTE
message isn't actually that important for frame accuracy.  It's the
splice time that is important (usually specified as a PTS).  And hence
even if the timestamp of the SCTE message is off by a bit you can
still have frame accurate splicing.

Now it's true that the splice-immediate case does benefit by the value
being more accurate.  I have a separate patch which better tracks the
video PTS and uses that as the basis for specifying the SCTE-35
timestamp value (and that's what I use in production).  I will be
looking to submit that as a separate patch, but didn't want to muddy
the waters by introducing it in this patch series (where I'm not
trying to tackle that problem).

In short, this patch series does significantly improve the situation,
even though it doesn't attempt to tackle the problem of the SCTE-35
timestamp not being as accurate as it could be.

Devin


--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmuel...@ltnglobal.com
___
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] What is FFmpeg and what should it be

2023-08-09 Thread Paul B Mahol 
On Wed, Aug 9, 2023 at 5:59 PM Michael Niedermayer 
wrote:

> On Tue, Aug 08, 2023 at 09:53:11PM +0300, Rémi Denis-Courmont wrote:
> > Le tiistaina 8. elokuuta 2023, 18.22.49 EEST Michael Niedermayer a écrit
> :
> > > > > That is missing that people suggest a path forward but
> > > > > with too few details to easily walk that path.
> > > >
> > > > Uh, I hate to state the patently obvious, but if "no path forward is
> > > > needed", then there should logically be _no_ "details to walk [a]
> path".
> > > > Conversely, if avradio does not belong in FFmpeg, as Kieran, Tomas
> and
> > > > others have been arguing, then there is no path forward to be given
> on
> > > > FFmpeg-devel.
> > > >
> > > >
> > > > And besides I don't think it's even fair to state that "too few
> details"
> > > > were given. People did suggest making this a new separate project
> > > > properly isolated from FFmpeg internals, and/or joining efforts with
> > > > existing OSS SDR projects rather than FFmpeg. Some specific projects
> have
> > > > even been cited.
> > > >
> > > > As far as FFmpeg(-devel) is concerned, I can't think how it
> could/should
> > > > reasonably get any more specific than that.
> > >
> > > The saying goes, one cannot win an Argument on the Internet.
> > > So, iam not trying to, but
> > >
> > > IIRC, a while ago you said iam obliged to work on FFmpeg. Thats
> > > simply not the case.
> >
> > I have made some preposterous statements in my dark past, but I am
> pretty sure
> > that I didn't make any statement to that effect, no.
>
> Not litterally, but i think you implied it in a reply to nicolas:
> Or at least thats how i read it, maybe i just misunderstood, this is really
> not important, just for completeness, here is what i refered to:
>
> in "Re: [FFmpeg-devel] [PATCH v2] avformat: add Software Defined Radio
> support"
> > Unless you have commitments that we are not privy of, nobody can tell
> > you how you are supposed to be spending your time and skills.
>
> FYI https://fflabs.eu/about/
>
>
FYI  https://fflabs.eu/legal-mentions/

Thanks

thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Never trust a computer, one day, it may think you are the virus. -- Compn
> ___
> 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] What is FFmpeg and what should it be

2023-08-09 Thread Michael Niedermayer 
On Tue, Aug 08, 2023 at 09:53:11PM +0300, Rémi Denis-Courmont wrote:
> Le tiistaina 8. elokuuta 2023, 18.22.49 EEST Michael Niedermayer a écrit :
> > > > That is missing that people suggest a path forward but
> > > > with too few details to easily walk that path.
> > > 
> > > Uh, I hate to state the patently obvious, but if "no path forward is
> > > needed", then there should logically be _no_ "details to walk [a] path".
> > > Conversely, if avradio does not belong in FFmpeg, as Kieran, Tomas and
> > > others have been arguing, then there is no path forward to be given on
> > > FFmpeg-devel.
> > > 
> > > 
> > > And besides I don't think it's even fair to state that "too few details"
> > > were given. People did suggest making this a new separate project
> > > properly isolated from FFmpeg internals, and/or joining efforts with
> > > existing OSS SDR projects rather than FFmpeg. Some specific projects have
> > > even been cited.
> > > 
> > > As far as FFmpeg(-devel) is concerned, I can't think how it could/should
> > > reasonably get any more specific than that.
> > 
> > The saying goes, one cannot win an Argument on the Internet.
> > So, iam not trying to, but
> > 
> > IIRC, a while ago you said iam obliged to work on FFmpeg. Thats
> > simply not the case.
> 
> I have made some preposterous statements in my dark past, but I am pretty 
> sure 
> that I didn't make any statement to that effect, no.

Not litterally, but i think you implied it in a reply to nicolas:
Or at least thats how i read it, maybe i just misunderstood, this is really
not important, just for completeness, here is what i refered to:

in "Re: [FFmpeg-devel] [PATCH v2] avformat: add Software Defined Radio 
support"
> Unless you have commitments that we are not privy of, nobody can tell
> you how you are supposed to be spending your time and skills.

FYI https://fflabs.eu/about/

thx

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

Never trust a computer, one day, it may think you are the virus. -- Compn


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

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

[FFmpeg-devel] [PATCH] avcodec/dovi_rpu: verify RPU data CRC32

2023-08-09 Thread quietvoid 
The Dolby Vision RPU contains a CRC32 to validate the payload against.
It must be an identical match for the metadata to be used.

The implementation is CRC32/MPEG-2.

Signed-off-by: quietvoid 
---
 libavcodec/dovi_rpu.c | 37 +++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index dd38936552..00f811e3a6 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -22,6 +22,7 @@
  */
 
 #include "libavutil/buffer.h"
+#include "libavutil/crc.h"
 
 #include "dovi_rpu.h"
 #include "golomb.h"
@@ -198,6 +199,9 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 DOVIVdrRef *vdr;
 int ret;
 
+size_t actual_rpu_size;
+uint8_t trailing_zeroes = 0;
+
 uint8_t nal_prefix;
 uint8_t rpu_type;
 uint8_t vdr_seq_info_present;
@@ -205,7 +209,21 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 uint8_t use_prev_vdr_rpu;
 uint8_t use_nlq;
 uint8_t profile;
-if ((ret = init_get_bits8(gb, rpu, rpu_size)) < 0)
+uint32_t rpu_data_crc32;
+uint32_t computed_crc32;
+
+for (int i = rpu_size - 1; i > 0; i--) {
+if (!rpu[i]) {
+trailing_zeroes++;
+} else {
+break;
+}
+}
+
+actual_rpu_size = rpu_size - trailing_zeroes;
+
+/* Exclude trailing byte (0x80) from reader */
+if ((ret = init_get_bits8(gb, rpu, actual_rpu_size - 1)) < 0)
 return ret;
 
 /* RPU header, common values */
@@ -440,7 +458,22 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, 
size_t rpu_size)
 color->source_diagonal = get_bits(gb, 10);
 }
 
-/* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPEG_2 */
+/* Skip unsupported until CRC32 */
+skip_bits_long(gb, get_bits_left(gb) - 32);
+
+rpu_data_crc32 = get_bits_long(gb, 32);
+
+/* Verify CRC32, the buffer excludes the prefix, CRC32 and trailing byte */
+computed_crc32 = av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IEEE), -1,
+rpu + 1, actual_rpu_size - 6));
+
+if (rpu_data_crc32 != computed_crc32) {
+av_log(s->logctx, AV_LOG_ERROR,
+   "Invalid RPU CRC32: Received %"PRIu32", expected %"PRIu32"\n",
+   computed_crc32, rpu_data_crc32);
+goto fail;
+}
+
 return 0;
 
 fail:
-- 
2.41.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 v4 2/4] mpegts: Stash original PTS for SCTE-35 sections for processing later

2023-08-09 Thread Kieran Kunhya 
On Mon, 31 Jul 2023 at 09:38, Devin Heitmueller <
devin.heitmuel...@ltnglobal.com> wrote:

> We need the original PTS value in order to do subsequent processing,
> so set it as packet side data.
>
> Signed-off-by: Devin Heitmueller 
> ---
>  libavformat/mpegts.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 0b3edda..a1b2420 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -1783,8 +1783,17 @@ static void scte_data_cb(MpegTSFilter *filter,
> const uint8_t *section,
>  prg = av_find_program_from_stream(ts->stream, NULL, idx);
>  if (prg && prg->pcr_pid != -1 && prg->discard != AVDISCARD_ALL) {
>  MpegTSFilter *f = ts->pids[prg->pcr_pid];
> -if (f && f->last_pcr != -1)
> +if (f && f->last_pcr != -1) {
> +AVTransportTimestamp *transport_ts;
>  ts->pkt->pts = ts->pkt->dts = f->last_pcr/300;
> +transport_ts = (AVTransportTimestamp *)
> av_packet_new_side_data(ts->pkt,
> +
>   AV_PKT_DATA_TRANSPORT_TIMESTAMP,
> +
>   sizeof(AVTransportTimestamp));
> +if (transport_ts) {
> +transport_ts->pts = ts->pkt->pts;
> +transport_ts->time_base = av_make_q(1, 9);
> +}
> +}
>  }
>  ts->stop_parse = 1;
>

How is this frame accurate? Surely "last_pcr" can be up to 100ms out. You
need to actually be interpolating the true value in order to be frame
accurate (not saying this is easy/doable in FFmpeg). But at the same time
inaccurate splices aren't great either.

Kieran
___
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/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

2023-08-09 Thread Andreas Rheinhardt 
James Almer:
> On 8/9/2023 7:08 AM, Nicolas George wrote:
>> Andreas Rheinhardt (12023-08-06):
>>> The AVBPrint API guarantees that the string buffer is always
>>> zero-terminated; in order to honour this guarantee, there
>>> obviously must be a string buffer at all and it must have
>>> a size >= 1. Therefore av_bprint_init_for_buffer() treats
>>> passing a NULL buffer or size == 0 as invalid data that
>>> leads to undefined behaviour, namely NPD in case NULL is provided
>>> or a write to a buffer of size 0 in case size == 0.
>>>
>>> But it would be easy to support this, namely by using the internal
>>> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
>>>
>>> There is a reason to allow this: Several functions like
>>> av_channel_(description|name) are actually wrappers
>>> around corresponding AVBPrint functions. They accept user
>>> provided buffers and are supposed to return the required
>>> size of the buffer, which would allow the user to call
>>> it once to get the required buffer size and call it once
>>> more after having allocated the buffer.
>>> If av_bprint_init_for_buffer() treats size == 0 as invalid,
>>> all these users would need to check for this themselves
>>> and basically add the same codeblock that this patch
>>> adds to av_bprint_init_for_buffer().
>>>
>>> This change is in line with e.g. snprintf() which also allows
>>> the pointer to be NULL in case size is zero.
>>>
>>> This fixes Coverity issues #1503074, #1503076 and #1503082;
>>> all of these issues are about providing NULL to the channel-layout
>>> functions that are wrappers around AVBPrint versions.
>>>
>>> Signed-off-by: Andreas Rheinhardt 
>>> ---
>>> Missing lavu minor version bump.
>>
>> Looks good to me.
>>
>> The other patches in the series too, but I do not maintain the channel
>> layouts.
>>
>> Regards,
> 
> The layout patches are ok too.

Ok, then I'll already apply them tomorrow. Thanks for the reviews.

- 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/hwcontext_vaapi: Add vaapi_drm_format_map support for x2rgb10

2023-08-09 Thread David Rosca 
On Wed, Aug 9, 2023 at 2:35 PM Rémi Denis-Courmont  wrote:
>
>
>
> Le 9 août 2023 15:02:45 GMT+03:00, David Rosca  a écrit :
> >Support for allocating frames with x2rgb10 format was added
> >in c00264f5013, this adds support for importing DMA-BUFs.
> >---
> > libavutil/hwcontext_vaapi.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> >index 6c3a227ddd..63544ce476 100644
> >--- a/libavutil/hwcontext_vaapi.c
> >+++ b/libavutil/hwcontext_vaapi.c
> >@@ -1048,6 +1048,9 @@ static const struct {
> > #if defined(VA_FOURCC_Y412) && defined(DRM_FORMAT_XVYU12_16161616)
> > DRM_MAP(Y412, 1, DRM_FORMAT_XVYU12_16161616),
> > #endif
> >+#ifdef defined(VA_FOURCC_X2R10G10B10) && defined(DRM_FORMAT_XRGB2101010)
> >+DRM_MAP(X2R10G10B10, 1, DRM_FORMAT_XRGB2101010),
> >+#endif
>
> That syntax is ostensibly wrong. Did you test the patch?

Sorry, I missed that when it was just

#ifdef VA_FOURCC_X2R10G10B10

copied from the other place.
Fixed in v2.

>
> > };
> > #undef DRM_MAP
> >
> ___
> 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 v2] lavu/hwcontext_vaapi: Add vaapi_drm_format_map support for x2rgb10

2023-08-09 Thread David Rosca 
Support for allocating frames with x2rgb10 format was added
in c00264f5013, this adds support for importing DMA-BUFs.

v2: Fix #ifdef -> #if
---
 libavutil/hwcontext_vaapi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 6c3a227ddd..558fed94c6 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -1048,6 +1048,9 @@ static const struct {
 #if defined(VA_FOURCC_Y412) && defined(DRM_FORMAT_XVYU12_16161616)
 DRM_MAP(Y412, 1, DRM_FORMAT_XVYU12_16161616),
 #endif
+#if defined(VA_FOURCC_X2R10G10B10) && defined(DRM_FORMAT_XRGB2101010)
+DRM_MAP(X2R10G10B10, 1, DRM_FORMAT_XRGB2101010),
+#endif
 };
 #undef DRM_MAP
 
-- 
2.41.0

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

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

Re: [FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: Add vaapi_drm_format_map support for x2rgb10

2023-08-09 Thread Rémi Denis-Courmont 


Le 9 août 2023 15:02:45 GMT+03:00, David Rosca  a écrit :
>Support for allocating frames with x2rgb10 format was added
>in c00264f5013, this adds support for importing DMA-BUFs.
>---
> libavutil/hwcontext_vaapi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
>index 6c3a227ddd..63544ce476 100644
>--- a/libavutil/hwcontext_vaapi.c
>+++ b/libavutil/hwcontext_vaapi.c
>@@ -1048,6 +1048,9 @@ static const struct {
> #if defined(VA_FOURCC_Y412) && defined(DRM_FORMAT_XVYU12_16161616)
> DRM_MAP(Y412, 1, DRM_FORMAT_XVYU12_16161616),
> #endif
>+#ifdef defined(VA_FOURCC_X2R10G10B10) && defined(DRM_FORMAT_XRGB2101010)
>+DRM_MAP(X2R10G10B10, 1, DRM_FORMAT_XRGB2101010),
>+#endif

That syntax is ostensibly wrong. Did you test the patch?

> };
> #undef DRM_MAP
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: Add vaapi_drm_format_map support for x2rgb10

2023-08-09 Thread David Rosca 
Support for allocating frames with x2rgb10 format was added
in c00264f5013, this adds support for importing DMA-BUFs.
---
 libavutil/hwcontext_vaapi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 6c3a227ddd..63544ce476 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -1048,6 +1048,9 @@ static const struct {
 #if defined(VA_FOURCC_Y412) && defined(DRM_FORMAT_XVYU12_16161616)
 DRM_MAP(Y412, 1, DRM_FORMAT_XVYU12_16161616),
 #endif
+#ifdef defined(VA_FOURCC_X2R10G10B10) && defined(DRM_FORMAT_XRGB2101010)
+DRM_MAP(X2R10G10B10, 1, DRM_FORMAT_XRGB2101010),
+#endif
 };
 #undef DRM_MAP
 
-- 
2.41.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 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

2023-08-09 Thread James Almer 

On 8/9/2023 7:08 AM, Nicolas George wrote:

Andreas Rheinhardt (12023-08-06):

The AVBPrint API guarantees that the string buffer is always
zero-terminated; in order to honour this guarantee, there
obviously must be a string buffer at all and it must have
a size >= 1. Therefore av_bprint_init_for_buffer() treats
passing a NULL buffer or size == 0 as invalid data that
leads to undefined behaviour, namely NPD in case NULL is provided
or a write to a buffer of size 0 in case size == 0.

But it would be easy to support this, namely by using the internal
buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.

There is a reason to allow this: Several functions like
av_channel_(description|name) are actually wrappers
around corresponding AVBPrint functions. They accept user
provided buffers and are supposed to return the required
size of the buffer, which would allow the user to call
it once to get the required buffer size and call it once
more after having allocated the buffer.
If av_bprint_init_for_buffer() treats size == 0 as invalid,
all these users would need to check for this themselves
and basically add the same codeblock that this patch
adds to av_bprint_init_for_buffer().

This change is in line with e.g. snprintf() which also allows
the pointer to be NULL in case size is zero.

This fixes Coverity issues #1503074, #1503076 and #1503082;
all of these issues are about providing NULL to the channel-layout
functions that are wrappers around AVBPrint versions.

Signed-off-by: Andreas Rheinhardt 
---
Missing lavu minor version bump.


Looks good to me.

The other patches in the series too, but I do not maintain the channel
layouts.

Regards,


The layout patches are ok too.
___
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/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

2023-08-09 Thread Nicolas George 
Andreas Rheinhardt (12023-08-06):
> The AVBPrint API guarantees that the string buffer is always
> zero-terminated; in order to honour this guarantee, there
> obviously must be a string buffer at all and it must have
> a size >= 1. Therefore av_bprint_init_for_buffer() treats
> passing a NULL buffer or size == 0 as invalid data that
> leads to undefined behaviour, namely NPD in case NULL is provided
> or a write to a buffer of size 0 in case size == 0.
> 
> But it would be easy to support this, namely by using the internal
> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
> 
> There is a reason to allow this: Several functions like
> av_channel_(description|name) are actually wrappers
> around corresponding AVBPrint functions. They accept user
> provided buffers and are supposed to return the required
> size of the buffer, which would allow the user to call
> it once to get the required buffer size and call it once
> more after having allocated the buffer.
> If av_bprint_init_for_buffer() treats size == 0 as invalid,
> all these users would need to check for this themselves
> and basically add the same codeblock that this patch
> adds to av_bprint_init_for_buffer().
> 
> This change is in line with e.g. snprintf() which also allows
> the pointer to be NULL in case size is zero.
> 
> This fixes Coverity issues #1503074, #1503076 and #1503082;
> all of these issues are about providing NULL to the channel-layout
> functions that are wrappers around AVBPrint versions.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Missing lavu minor version bump.

Looks good to me.

The other patches in the series too, but I do not maintain the channel
layouts.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH 1/5] avformat/matroskaenc: Support rotations

2023-08-09 Thread Andreas Rheinhardt 
Andreas Rheinhardt:
> Matroska supports orthogonal transformations (both pure rotations
> as well as reflections) via its 3D-projection elements, namely
> ProjectionPoseYaw (for a horizontal reflection) as well as
> ProjectionPoseRoll (for rotations). This commit adds support
> for this.
> 
> Support for this in the demuxer has been added in
> 937bb6bbc1e8654633737e69e403e95a37113058 and
> the sample used in the matroska-dovi-write-config8 FATE-test
> includes a displaymatrix indicating a rotation which is now
> properly written and read, thereby providing coverage for
> the relevant code in the muxer as well as the demuxer.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Honestly, I am not really sure how to handle the floating-point
> inaccuracies here (in atan2).
> 
>  libavformat/matroskaenc.c  | 100 +
>  tests/ref/fate/matroska-dovi-write-config8 |  13 ++-
>  2 files changed, 94 insertions(+), 19 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 41e13b273d..c1f40b26e6 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1403,25 +1403,75 @@ static void mkv_write_video_color(EbmlWriter *writer, 
> const AVStream *st,
>  }
>  
>  #define MAX_VIDEO_PROJECTION_ELEMS 6
> -static void mkv_write_video_projection(AVFormatContext *s, EbmlWriter 
> *writer,
> -   const AVStream *st, uint8_t private[])
> +static void mkv_handle_rotation(void *logctx, const AVStream *st,
> +double *yaw, double *roll)
> +{
> +const int32_t *matrix =
> +(const int32_t*)av_stream_get_side_data(st, 
> AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +
> +if (!matrix)
> +return;
> +
> +/* Check whether this is an affine transformation */
> +if (matrix[2] || matrix[5])
> +goto ignore;
> +
> +/* This together with the checks below test whether
> + * the upper-left 2x2 matrix is nonsingular. */
> +if (!matrix[0] && !matrix[1])
> +goto ignore;
> +
> +/* We ignore the translation part of the matrix (matrix[6] and matrix[7])
> + * as well as any scaling, i.e. we only look at the upper left 2x2 
> matrix.
> + * We only accept matrices that are an exact multiple of an orthogonal 
> one.
> + * Apart from the multiple, every such matrix can be obtained by
> + * potentially flipping in the x-direction (corresponding to yaw = 180)
> + * followed by a rotation of (say) an angle phi in the counterclockwise
> + * direction. The upper-left 2x2 matrix then looks like this:
> + * | (+/-)cos(phi) (-/+)sin(phi) |
> + * scale * | |
> + * |  sin(phi)  cos(phi) |
> + * The first set of signs in the first row apply in case of no flipping,
> + * the second set applies in case of flipping. */
> +
> +/* The casts to int64_t are needed because -INT32_MIN doesn't fit
> + * in an int32_t. */
> +if (matrix[0] == matrix[4] && -(int64_t)matrix[1] == matrix[3]) {
> +/* No flipping case */
> +*yaw = 0;
> +} else if (-(int64_t)matrix[0] == matrix[4] && matrix[1] == matrix[3]) {
> +/* Horizontal flip */
> +*yaw = 180;
> +} else {
> +ignore:
> +av_log(logctx, AV_LOG_INFO, "Ignoring display matrix indicating "
> +   "non-orthogonal transformation.\n");
> +return;
> +}
> +*roll = 180 / M_PI * atan2(matrix[3], matrix[4]);
> +
> +/* We do not write a ProjectionType element indicating "rectangular",
> + * because this is the default value. */
> +}
> +
> +static int mkv_handle_spherical(void *logctx, EbmlWriter *writer,
> +const AVStream *st, uint8_t private[],
> +double *yaw, double *pitch, double *roll)
>  {
>  const AVSphericalMapping *spherical =
>  (const AVSphericalMapping *)av_stream_get_side_data(st, 
> AV_PKT_DATA_SPHERICAL,
>  NULL);
>  
>  if (!spherical)
> -return;
> +return 0;
>  
>  if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR  &&
>  spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE &&
>  spherical->projection != AV_SPHERICAL_CUBEMAP) {
> -av_log(s, AV_LOG_WARNING, "Unknown projection type\n");
> -return;
> +av_log(logctx, AV_LOG_WARNING, "Unknown projection type\n");
> +return 0;
>  }
>  
> -ebml_writer_open_master(writer, MATROSKA_ID_VIDEOPROJECTION);
> -
>  switch (spherical->projection) {
>  case AV_SPHERICAL_EQUIRECTANGULAR:
>  case AV_SPHERICAL_EQUIRECTANGULAR_TILE:
> @@ -1455,17 +1505,33 @@ static void 
> mkv_write_video_projection(AVFormatContext *s, EbmlWriter *writer,
>  av_assert0(0);
>  }
>  
> -if (spherical->yaw)
> -ebml_writer_add_float(writer,

Re: [FFmpeg-devel] [PATCH 2/7] avutil/bprint: Allow size == 0 in av_bprint_init_for_buffer()

2023-08-09 Thread Andreas Rheinhardt 
Andreas Rheinhardt:
> The AVBPrint API guarantees that the string buffer is always
> zero-terminated; in order to honour this guarantee, there
> obviously must be a string buffer at all and it must have
> a size >= 1. Therefore av_bprint_init_for_buffer() treats
> passing a NULL buffer or size == 0 as invalid data that
> leads to undefined behaviour, namely NPD in case NULL is provided
> or a write to a buffer of size 0 in case size == 0.
> 
> But it would be easy to support this, namely by using the internal
> buffer with AV_BPRINT_SIZE_COUNT_ONLY in case size == 0.
> 
> There is a reason to allow this: Several functions like
> av_channel_(description|name) are actually wrappers
> around corresponding AVBPrint functions. They accept user
> provided buffers and are supposed to return the required
> size of the buffer, which would allow the user to call
> it once to get the required buffer size and call it once
> more after having allocated the buffer.
> If av_bprint_init_for_buffer() treats size == 0 as invalid,
> all these users would need to check for this themselves
> and basically add the same codeblock that this patch
> adds to av_bprint_init_for_buffer().
> 
> This change is in line with e.g. snprintf() which also allows
> the pointer to be NULL in case size is zero.
> 
> This fixes Coverity issues #1503074, #1503076 and #1503082;
> all of these issues are about providing NULL to the channel-layout
> functions that are wrappers around AVBPrint versions.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Missing lavu minor version bump.
> 
>  libavutil/bprint.c | 5 +
>  libavutil/bprint.h | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> index 23998a8b02..4e9571715c 100644
> --- a/libavutil/bprint.c
> +++ b/libavutil/bprint.c
> @@ -84,6 +84,11 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, 
> unsigned size_max)
>  
>  void av_bprint_init_for_buffer(AVBPrint *buf, char *buffer, unsigned size)
>  {
> +if (size == 0) {
> +av_bprint_init(buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
> +return;
> +}
> +
>  buf->str  = buffer;
>  buf->len  = 0;
>  buf->size = size;
> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> index f27d30f723..8559745478 100644
> --- a/libavutil/bprint.h
> +++ b/libavutil/bprint.h
> @@ -144,6 +144,9 @@ void av_bprint_init(AVBPrint *buf, unsigned size_init, 
> unsigned size_max);
>   * Init a print buffer using a pre-existing buffer.
>   *
>   * The buffer will not be reallocated.
> + * In case size equals zero, the AVBPrint will be initialized to use
> + * the internal buffer as if using AV_BPRINT_SIZE_COUNT_ONLY with
> + * av_bprint_init().
>   *
>   * @param buf buffer structure to init
>   * @param buffer  byte buffer to use for the string data

Ping for this patch (and the rest of this patchset). Will apply patches
2-5 in two days unless there are objections; will apply the rest today.

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