date:20210416

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffprobe: Remove check on show_frames and show_packets in XML writer

2021-04-16 Thread Tobias Rapp


On 13.04.2021 08:54, Tobias Rapp wrote:

On 31.03.2021 12:13, Tobias Rapp wrote:

The "packets_and_frames" element has been added to ffprobe.xsd in
0c9f0da0f7656059e9bd41931d250aafddf35ea3 but apparently removing the
check in ffprobe.c has been forgotten.

Signed-off-by: Tobias Rapp 
---
  fftools/ffprobe.c | 7 ---
  1 file changed, 7 deletions(-)

[..]


BTW, this can be tested using a command-line like:

ffprobe -show_streams -show_packets -show_format -show_frames \
   tests/data/ffprobe-test.nut -noprivate -of xml=q=1:x=1 | \
   xmllint --noout --schema doc/ffprobe.xsd -

If there are no objections I'd like to apply both patches at the end of 
this week.


Applied both patches.

Regards,
Tobias

___
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] Added bandwidth parameter manual configurare in HLS master playlist

2021-04-16 Thread Dhanish Vijayan

---
 libavformat/hlsenc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 7d97ce1789..957eb609a0 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -183,6 +183,7 @@ typedef struct VariantStream {
 const char *sgroup;   /* subtitle group name */
 const char *ccgroup;  /* closed caption group name */
 const char *varname;  /* variant name */
+int bandwidth;/* bandwidth for the variant */
 } VariantStream;
 
 typedef struct ClosedCaptionsStream {
@@ -1492,6 +1493,10 @@ static int create_master_playlist(AVFormatContext *s,
 bandwidth += get_stream_bit_rate(aud_st);
 bandwidth += bandwidth / 10;
 
+if (vs->bandwidth){
+bandwidth = vs->bandwidth;
+}
+
 ccgroup = NULL;
 if (vid_st && vs->ccgroup) {
 /* check if this group name is available in the cc map string */
@@ -2088,6 +2093,9 @@ static int parse_variant_stream_mapstring(AVFormatContext 
*s)
   (!av_strncasecmp(val, "1", strlen("1";
 hls->has_default_key = 1;
 continue;
+} else if (av_strstart(keyval, "bandwidth:", &val)) {
+vs->bandwidth  = strtoimax(val, NULL, 10);
+continue;
 } else if (av_strstart(keyval, "name:", &val)) {
 vs->varname  = val;
 continue;
-- 
2.25.1

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

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

[FFmpeg-devel] [PATCH] fate/ffprobe: Verify ffprobe XML output against schema file

2021-04-16 Thread Tobias Rapp

Adds schema validation for ffprobe XML output so that updating the
ffprobe.xsd file upon changes to ffprobe is not forgotten. This was
suggested by Marton Balint in:
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/278428.html

The schema FATE test is only run if xmllint command is available.

Signed-off-by: Tobias Rapp 
---
 configure  |  3 +++
 tests/fate/ffprobe.mak |  6 +
 tests/ref/fate/ffprobe_xsd | 57 ++
 3 files changed, 66 insertions(+)
 create mode 100644 tests/ref/fate/ffprobe_xsd

diff --git a/configure b/configure
index d7a3f50..b8ad7f9 100755
--- a/configure
+++ b/configure
@@ -2341,6 +2341,7 @@ HAVE_LIST="
 perl
 pod2man
 texi2html
+xmllint
 "
 
 # options emitted with CONFIG_ prefix but not available on the command line
@@ -6599,6 +6600,7 @@ disabled makeinfo_html && texi2html --help 2> /dev/null | 
grep -q 'init-file' &&
 perl -v> /dev/null 2>&1 && enable perl  || disable perl
 pod2man --help > /dev/null 2>&1 && enable pod2man   || disable pod2man
 rsync --help 2> /dev/null | grep -q 'contimeout' && enable rsync_contimeout || 
disable rsync_contimeout
+xmllint --version  > /dev/null 2>&1 && enable xmllint   || disable xmllint
 
 # check V4L2 codecs available in the API
 if enabled v4l2_m2m; then
@@ -7365,6 +7367,7 @@ echo "perl enabled  ${perl-no}"
 echo "pod2man enabled   ${pod2man-no}"
 echo "makeinfo enabled  ${makeinfo-no}"
 echo "makeinfo supports HTML${makeinfo_html-no}"
+echo "xmllint enabled   ${xmllint-no}"
 test -n "$random_seed" &&
 echo "random seed   ${random_seed}"
 echo
diff --git a/tests/fate/ffprobe.mak b/tests/fate/ffprobe.mak
index c867beb..d2abe8a 100644
--- a/tests/fate/ffprobe.mak
+++ b/tests/fate/ffprobe.mak
@@ -29,6 +29,12 @@ FATE_FFPROBE-$(CONFIG_AVDEVICE) += fate-ffprobe_xml
 fate-ffprobe_xml: $(FFPROBE_TEST_FILE)
 fate-ffprobe_xml: CMD = run $(FFPROBE_COMMAND) -of xml
 
+FATE_FFPROBE_SCHEMA-$(CONFIG_AVDEVICE) += fate-ffprobe_xsd
+fate-ffprobe_xsd: $(FFPROBE_TEST_FILE)
+fate-ffprobe_xsd: CMD = run $(FFPROBE_COMMAND) -noprivate -of xml=q=1:x=1 | \
+   xmllint --schema $(SRC_PATH)/doc/ffprobe.xsd -
+
+FATE_FFPROBE-$(HAVE_XMLLINT) += $(FATE_FFPROBE_SCHEMA-yes)
 FATE_FFPROBE += $(FATE_FFPROBE-yes)
 
 fate-ffprobe: $(FATE_FFPROBE)
diff --git a/tests/ref/fate/ffprobe_xsd b/tests/ref/fate/ffprobe_xsd
new file mode 100644
index 000..cb3413e
--- /dev/null
+++ b/tests/ref/fate/ffprobe_xsd
@@ -0,0 +1,57 @@
+
+http://www.w3.org/2001/XMLSchema-instance"; 
xmlns:ffprobe="http://www.ffmpeg.org/schema/ffprobe"; 
xsi:schemaLocation="http://www.ffmpeg.org/schema/ffprobe ffprobe.xsd">
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-- 
2.7.4


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

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

Re: [FFmpeg-devel] [PATCH] avformat/utils: Combine identical statements

2021-04-16 Thread Andreas Rheinhardt

Andreas Rheinhardt:
> This would only make a difference in case the first attempt to
> initialize the encoder failed and the second succeeded. The only
> reason I can think of for this to happen is that the options (in
> particular the codec whitelist) are not used for the second try
> and that obviously implies that we should not even try a second time
> to open the decoder.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/utils.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index d9971d7fd3..d4ec3d0190 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3747,16 +3747,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  if (ic->codec_whitelist)
>  av_dict_set(options ? &options[i] : &thread_opt, 
> "codec_whitelist", ic->codec_whitelist, 0);
>  
> -/* Ensure that subtitle_header is properly set. */
> -if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE
> -&& codec && !avctx->codec) {
> -if (avcodec_open2(avctx, codec, options ? &options[i] : 
> &thread_opt) < 0)
> -av_log(ic, AV_LOG_WARNING,
> -   "Failed to open codec in %s\n",__FUNCTION__);
> -}
> -
>  // Try to just open decoders, in case this is enough to get 
> parameters.
> -if (!has_codec_parameters(st, NULL) && st->internal->request_probe 
> <= 0) {
> +// Also ensure that subtitle_header is properly set.
> +if (!has_codec_parameters(st, NULL) && st->internal->request_probe 
> <= 0 ||
> +st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) {
>  if (codec && !avctx->codec)
>  if (avcodec_open2(avctx, codec, options ? &options[i] : 
> &thread_opt) < 0)
>  av_log(ic, AV_LOG_WARNING,
> 
Will apply tomorrow unless there are objections.

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

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

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: Remove unnecessary function calls

2021-04-16 Thread Andreas Rheinhardt

Andreas Rheinhardt:
> ffio_fill() is used when initially writing unknown length elements;
> yet it can happen that the amount of bytes written by it is zero in
> which case it is of course unnecessary to ever call it. Whether it is
> possible to know this during compiletime depends upon how aggressively
> the compiler inlines function calls (i.e. if it inlines calls to
> start_ebml_master() where the upper bound for the size of the element
> implies that the size will be written on one byte) and this depends upon
> optimization settings. It is not the aim of this patch to inline all
> calls where it is known that ffio_fill() will be unnecessary, but merely
> to make compilers that inline such calls aware of the fact that writing
> zero bytes with ffio_fill() is unnecessary. To this end
> av_builtin_constant_p() is used to check whether the size is a
> compiletime constant.
> 
> For GCC 10 this made a difference at -O3 only: The size of .text
> decreased from 0x747F (with 29 calls to ffio_fill(), eight of which
> use size zero) to 0x7337 (with 21 calls to ffio_fill(), zero of which
> use size zero).
> For Clang 11 it made a difference at -O2 and -O3: At -O2, the size of
> .text decreased from 0x879C to 0x871C (with eight calls to ffio_fill()
> eliminated); at -O3 the size of .text decreased from 0xAF2F to 0xAEBF.
> Once again, eight calls to ffio_fill() with size zero have been
> eliminated.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/matroskaenc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 3649ac25a2..0141fb0b8d 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -195,6 +195,8 @@ static void put_ebml_size_unknown(AVIOContext *pb, int 
> bytes)
>  {
>  av_assert0(bytes <= 8);
>  avio_w8(pb, 0x1ff >> bytes);
> +if (av_builtin_constant_p(bytes) && bytes == 1)
> +return;
>  ffio_fill(pb, 0xff, bytes - 1);
>  }
>  
> 
Will apply tomorrow unless there are  objections.

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/jpeglsdec: Don't allocate+free JPEGLSState for every frame

2021-04-16 Thread Andreas Rheinhardt

Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/jpeglsdec.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
> index 69980eaa49..92df81600b 100644
> --- a/libavcodec/jpeglsdec.c
> +++ b/libavcodec/jpeglsdec.c
> @@ -45,6 +45,11 @@
>   */
>  //#define JLS_BROKEN
>  
> +typedef struct JpegLSDecodeContext {
> +MJpegDecodeContext mjpeg;
> +JLSState state;
> +} JpegLSDecodeContext;
> +
>  /**
>   * Decode LSE block with initialization parameters
>   */
> @@ -350,7 +355,7 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int 
> near,
>  {
>  int i, t = 0;
>  uint8_t *zero, *last, *cur;
> -JLSState *state;
> +JLSState *const state = &((JpegLSDecodeContext*)s)->state;
>  int off = 0, stride = 1, width, shift, ret = 0;
>  int decoded_height = 0;
>  
> @@ -360,12 +365,8 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int 
> near,
>  last = zero;
>  cur  = s->picture_ptr->data[0];
>  
> -state = av_mallocz(sizeof(JLSState));
> -if (!state) {
> -av_free(zero);
> -return AVERROR(ENOMEM);
> -}
>  /* initialize JPEG-LS state from JPEG parameters */
> +memset(state, 0, sizeof(*state));
>  state->near   = near;
>  state->bpp= (s->bits < 2) ? 2 : s->bits;
>  state->maxval = s->maxval;
> @@ -537,7 +538,6 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int 
> near,
>  }
>  
>  end:
> -av_free(state);
>  av_free(zero);
>  
>  return ret;
> @@ -548,7 +548,7 @@ AVCodec ff_jpegls_decoder = {
>  .long_name  = NULL_IF_CONFIG_SMALL("JPEG-LS"),
>  .type   = AVMEDIA_TYPE_VIDEO,
>  .id = AV_CODEC_ID_JPEGLS,
> -.priv_data_size = sizeof(MJpegDecodeContext),
> +.priv_data_size = sizeof(JpegLSDecodeContext),
>  .init   = ff_mjpeg_decode_init,
>  .close  = ff_mjpeg_decode_end,
>  .receive_frame  = ff_mjpeg_receive_frame,
> 
Will apply tomorrow unless there are objections.

- Andreas

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

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

Re: [FFmpeg-devel] [PATCH 1/2] Include attributes.h directly

2021-04-16 Thread Andreas Rheinhardt

Andreas Rheinhardt:
> Some files currently rely on libavutil/cpu.h to include it for them;
> yet said file won't use include it any more after the currently
> deprecated functions are removed, so include attributes.h directly.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/aarch64/aacpsdsp_init_aarch64.c  | 1 +
>  libavcodec/aarch64/opusdsp_init.c   | 1 +
>  libavcodec/arm/flacdsp_init_arm.c   | 1 +
>  libavcodec/arm/mpegvideo_arm.c  | 1 +
>  libavcodec/arm/mpegvideoencdsp_init_arm.c   | 1 +
>  libavcodec/arm/sbcdsp_init_arm.c| 1 +
>  libavcodec/mips/aacdec_mips.c   | 1 +
>  libavcodec/mips/cabac.h | 1 +
>  libavcodec/mips/fft_mips.c  | 1 +
>  libavcodec/mips/fmtconvert_mips.c   | 1 +
>  libavcodec/mips/h263dsp_init_mips.c | 1 +
>  libavcodec/mips/h264chroma_init_mips.c  | 1 +
>  libavcodec/mips/h264dsp_init_mips.c | 1 +
>  libavcodec/mips/h264pred_init_mips.c| 1 +
>  libavcodec/mips/h264qpel_init_mips.c| 1 +
>  libavcodec/mips/idctdsp_init_mips.c | 1 +
>  libavcodec/mips/lsp_mips.h  | 1 +
>  libavcodec/mips/me_cmp_init_mips.c  | 1 +
>  libavcodec/mips/mpegvideo_init_mips.c   | 1 +
>  libavcodec/mips/mpegvideoencdsp_init_mips.c | 1 +
>  libavcodec/mips/vc1dsp_mmi.c| 2 +-
>  libavcodec/mips/vp8dsp_mmi.c| 1 +
>  libavcodec/mips/vp9dsp_init_mips.c  | 1 +
>  libavcodec/mips/xvididct_init_mips.c| 1 +
>  libavcodec/neon/mpegvideo.c | 1 +
>  libavcodec/ppc/fft_init.c   | 1 +
>  libavcodec/ppc/mpegvideodsp.c   | 1 +
>  libavcodec/ppc/vp8dsp_altivec.c | 1 +
>  libavcodec/x86/aacencdsp_init.c | 1 +
>  libavcodec/x86/celt_pvq_init.c  | 1 +
>  libavcodec/x86/fdct.c   | 1 +
>  libavcodec/x86/flacdsp_init.c   | 1 +
>  libavcodec/x86/mdct15_init.c| 1 +
>  libavcodec/x86/opusdsp_init.c   | 1 +
>  libavcodec/x86/sbcdsp_init.c| 1 +
>  libavcodec/x86/snowdsp.c| 1 +
>  libavcodec/x86/takdsp_init.c| 1 +
>  libavcodec/x86/ttadsp_init.c| 1 +
>  libavcodec/x86/ttaencdsp_init.c | 1 +
>  libavcodec/x86/v210-init.c  | 1 +
>  libavcodec/x86/v210enc_init.c   | 1 +
>  libavcodec/x86/vc1dsp_mmx.c | 1 +
>  libavcodec/x86/vp56_arith.h | 2 ++
>  libavcodec/x86/vp9dsp_init.h| 1 +
>  libavfilter/aarch64/vf_nlmeans_init.c   | 1 +
>  libavutil/mips/libm_mips.h  | 2 ++
>  libavutil/x86/lls_init.c| 1 +
>  libswresample/aarch64/resample_init.c   | 1 +
>  libswresample/arm/resample_init.c   | 1 +
>  libswresample/x86/audio_convert_init.c  | 1 +
>  libswresample/x86/rematrix_init.c   | 1 +
>  libswresample/x86/resample_init.c   | 1 +
>  libswscale/aarch64/swscale.c| 1 +
>  libswscale/arm/swscale.c| 1 +
>  libswscale/ppc/swscale_ppc_template.c   | 1 +
>  libswscale/x86/hscale_fast_bilinear_simd.c  | 1 +
>  56 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/aarch64/aacpsdsp_init_aarch64.c 
> b/libavcodec/aarch64/aacpsdsp_init_aarch64.c
> index 5e7e19bba4..cc9e4d79bd 100644
> --- a/libavcodec/aarch64/aacpsdsp_init_aarch64.c
> +++ b/libavcodec/aarch64/aacpsdsp_init_aarch64.c
> @@ -18,6 +18,7 @@
>  
>  #include "config.h"
>  
> +#include "libavutil/attributes.h"
>  #include "libavutil/aarch64/cpu.h"
>  #include "libavcodec/aacpsdsp.h"
>  
> diff --git a/libavcodec/aarch64/opusdsp_init.c 
> b/libavcodec/aarch64/opusdsp_init.c
> index cc6a1b672d..bb6d71b66b 100644
> --- a/libavcodec/aarch64/opusdsp_init.c
> +++ b/libavcodec/aarch64/opusdsp_init.c
> @@ -18,6 +18,7 @@
>  
>  #include "config.h"
>  
> +#include "libavutil/attributes.h"
>  #include "libavutil/aarch64/cpu.h"
>  #include "libavcodec/opusdsp.h"
>  
> diff --git a/libavcodec/arm/flacdsp_init_arm.c 
> b/libavcodec/arm/flacdsp_init_arm.c
> index 564e3dc79b..c4a6e3a535 100644
> --- a/libavcodec/arm/flacdsp_init_arm.c
> +++ b/libavcodec/arm/flacdsp_init_arm.c
> @@ -18,6 +18,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include "libavutil/attributes.h"
>  #include "libavcodec/flacdsp.h"
>  #include "config.h"
>  
> diff --git a/libavcodec/arm/mpegvideo_arm.c b/libavcodec/arm/mpegvideo_arm.c
> index 918be16d03..008ef18eea 100644
> --- a/libavcodec/arm/mpegvideo_arm.c
> +++ b/libavcodec/arm/mpegvideo_arm.c
> @@ -18,6 +18,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include "libavutil/attributes.h"
>  #include "libavutil/internal.h"
>  #include "libavutil/arm/cpu.h"
>  #include "libavcodec/avcodec.h"
> diff --git a/libavcodec/arm/mpe

[FFmpeg-devel] [PATCH 1/1] avcodec/nvenc: move lossless presets after new ones

2021-04-16 Thread Martin Pulec

A simplier alternative to this patch would be replacing the condition in
nvenc.c:
if (ctx->preset >= PRESET_LOSSLESS_DEFAULT && ret <= 0) {
with
if ((ctx->preset >= PRESET_LOSSLESS_DEFAULT && (ctx->preset <= 
PRESET_LOSSLESS_HP))
&& ret <= 0) {

But a comment in the preset enum suggests keeping lossless presets at the end
so that I've followed that advice.

Martin Pulec (1):
  avcodec/nvenc: move lossless presets after new ones

 libavcodec/nvenc.h  | 4 ++--
 libavcodec/nvenc_h264.c | 2 +-
 libavcodec/nvenc_hevc.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.27.0

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

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

[FFmpeg-devel] [PATCH 1/1] avcodec/nvenc: move lossless presets after new ones

2021-04-16 Thread Martin Pulec

A check in nvenc.c checks for NV_ENC_CAPS_SUPPORT_LOSSLESS_ENCODE if
preset >= PRESET_LOSSLESS_DEFAULT which was true for the new presets. As
a result, the use of new presets (P1-P7) fail if the card doesn't
support lossless encoding.
---
 libavcodec/nvenc.h  | 4 ++--
 libavcodec/nvenc_h264.c | 2 +-
 libavcodec/nvenc_hevc.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index fefc5f7f0b..fd69b7809f 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -103,8 +103,6 @@ enum {
 PRESET_LOW_LATENCY_DEFAULT ,
 PRESET_LOW_LATENCY_HQ ,
 PRESET_LOW_LATENCY_HP,
-PRESET_LOSSLESS_DEFAULT, // lossless presets must be the last ones
-PRESET_LOSSLESS_HP,
 #ifdef NVENC_HAVE_NEW_PRESETS
 PRESET_P1,
 PRESET_P2,
@@ -114,6 +112,8 @@ enum {
 PRESET_P6,
 PRESET_P7,
 #endif
+PRESET_LOSSLESS_DEFAULT, // lossless presets must be the last ones
+PRESET_LOSSLESS_HP,
 };
 
 enum {
diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
index 4c2585876e..113840a672 100644
--- a/libavcodec/nvenc_h264.c
+++ b/libavcodec/nvenc_h264.c
@@ -27,7 +27,7 @@
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
 #ifdef NVENC_HAVE_NEW_PRESETS
-{ "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 }, PRESET_DEFAULT, PRESET_P7,  
VE, "preset" },
+{ "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 }, PRESET_DEFAULT, 
PRESET_LOSSLESS_HP, VE, "preset" },
 #else
 { "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_MEDIUM }, PRESET_DEFAULT, 
PRESET_LOSSLESS_HP, VE, "preset" },
 #endif
diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
index 441e7871d2..46e4798a6f 100644
--- a/libavcodec/nvenc_hevc.c
+++ b/libavcodec/nvenc_hevc.c
@@ -27,7 +27,7 @@
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
 #ifdef NVENC_HAVE_NEW_PRESETS
-{ "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 }, PRESET_DEFAULT, PRESET_P7,  
VE, "preset" },
+{ "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 }, PRESET_DEFAULT, 
PRESET_LOSSLESS_HP, VE, "preset" },
 #else
 { "preset",   "Set the encoding preset",OFFSET(preset),
   AV_OPT_TYPE_INT,   { .i64 = PRESET_MEDIUM }, PRESET_DEFAULT, 
PRESET_LOSSLESS_HP, VE, "preset" },
 #endif
-- 
2.27.0

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

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

Re: [FFmpeg-devel] [PATCH] avformat/mpegts: set correct extradata size for Opus streams

2021-04-16 Thread James Almer


On 4/15/2021 1:02 AM, James Almer wrote:

map_type 0 is always 19 bytes, whereas map_type 1 and 255 are 21 + channel
count bytes.

Should fix ticket #9190.

Signed-off-by: James Almer 
---
  libavformat/mpegts.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 6e0d9d7496..5343a14e38 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2030,6 +2030,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, 
AVStream *st, int stream_type
  st->codecpar->extradata[19] = 
opus_stream_cnt[channel_config_code];
  st->codecpar->extradata[20] = 
opus_coupled_stream_cnt[channel_config_code];
  memcpy(&st->codecpar->extradata[21], 
opus_channel_map[channels - 1], channels);
+st->codecpar->extradata_size = 19 + 
(st->codecpar->extradata[18] ? 2 + channels : 0);
  } else {
  avpriv_request_sample(fc, "Opus in MPEG-TS - channel_config_code 
> 0x8");
  }


Will amend the patches with FATE changes and apply the set.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/libwebpenc_animencoder: stop propagating bogus empty packets

2021-04-16 Thread James Almer


On 4/10/2021 11:12 PM, James Almer wrote:

Packets must have at least one of data or side_data. If none are available,
then got_packet must not be signaled.

The generic encode code already discarded these empty packets, but it's better
just not propagating them at all.

Signed-off-by: James Almer 
---
This patchset supersedes "avcodec/libwebpenc_animencoder: Don't return pkt
without data/side-data" and "avformat/webpenc: Don't treat zero-sized packets
as invalid".

  libavcodec/libwebpenc_animencoder.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavcodec/libwebpenc_animencoder.c 
b/libavcodec/libwebpenc_animencoder.c
index 7f35a0b939..633af2e925 100644
--- a/libavcodec/libwebpenc_animencoder.c
+++ b/libavcodec/libwebpenc_animencoder.c
@@ -102,10 +102,9 @@ static int libwebp_anim_encode_frame(AVCodecContext 
*avctx, AVPacket *pkt,
  goto end;
  }
  
-pkt->pts = pkt->dts = frame->pts;

  s->prev_frame_pts = frame->pts;  // Save for next frame.
  ret = 0;
-*got_packet = 1;
+*got_packet = 0;
  
  end:

  WebPPictureFree(pic);


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

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

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_astats: Remove fraction part of integer metadata entries

2021-04-16 Thread Tobias Rapp


On 09.04.2021 09:58, Tobias Rapp wrote:

On 08.04.2021 11:34, Nicolas George wrote:

Anton Khirnov (12021-04-08):

Does this mean that there are no stability guarantees for metadata
exported by filters?


We can have stability for the components that are good enough to be
stable, and no stability yet for components that need enhancing.


Indeed I should at least increment the minor (or micro?) version for 
libavfilter. And for the metadata changes my goal was to make some 
obvious things like "lavfi.astats.Bit_depth=24.00" less weird. I 
didn't dare to touch other things. As the astats filter metadata is not 
tested by FATE my guess was that stability is not yet a high priority.


BTW: After the change metadata key names match what is documented in
http://ffmpeg.org/ffmpeg-filters.html#astats-1 regarding underscores and 
the "Overall" prefix.


Have added an increment of the minor version in libavfilter/version.h 
locally. So I guess this patch should be fine now?


Regards,
Tobias

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/libaomdec: export frame pict_type

2021-04-16 Thread James Almer


On 4/13/2021 9:14 PM, James Almer wrote:

Should fix ticket #9180

Signed-off-by: James Almer 
---
  libavcodec/libaomdec.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
index 1fc0a0001d..6de3bcc5c3 100644
--- a/libavcodec/libaomdec.c
+++ b/libavcodec/libaomdec.c
@@ -161,6 +161,7 @@ static int aom_decode(AVCodecContext *avctx, void *data, 
int *got_frame,
  AVFrame *picture  = data;
  const void *iter  = NULL;
  struct aom_image *img;
+aom_codec_frame_flags_t av_unused flags;
  int ret;
  
  if (aom_codec_decode(&ctx->decoder, avpkt->data, avpkt->size, NULL) !=

@@ -198,6 +199,19 @@ static int aom_decode(AVCodecContext *avctx, void *data, 
int *got_frame,
  if ((ret = ff_get_buffer(avctx, picture, 0)) < 0)
  return ret;
  
+#ifdef AOM_CTRL_AOMD_GET_FRAME_FLAGS

+ret = aom_codec_control(&ctx->decoder, AOMD_GET_FRAME_FLAGS, &flags);
+if (ret == AOM_CODEC_OK) {
+picture->key_frame = !!(flags & AOM_FRAME_IS_KEY);
+if (flags & (AOM_FRAME_IS_KEY | AOM_FRAME_IS_INTRAONLY))
+picture->pict_type = AV_PICTURE_TYPE_I;
+else if (flags & AOM_FRAME_IS_SWITCH)
+picture->pict_type = AV_PICTURE_TYPE_SP;
+else
+picture->pict_type = AV_PICTURE_TYPE_P;
+}
+#endif
+
  av_reduce(&picture->sample_aspect_ratio.num,
&picture->sample_aspect_ratio.den,
picture->height * img->r_w,


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

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

Re: [FFmpeg-devel] [PATCH] avformat/movenc: fix writing dOps atoms

2021-04-16 Thread James Almer


On 4/15/2021 1:39 AM, James Almer wrote:

Don't blindly copy all bytes in extradata past ChannelMappingFamily. Instead
check if ChannelMappingFamily is not 0 and then only write the correct amount
of bytes from ChannelMappingTable, as defined in the spec[1].

Should fix ticket #9190.

[1] https://opus-codec.org/docs/opus_in_isobmff.html#4.3.2

Signed-off-by: James Almer 
---
Compared to "avformat/mpegts: set correct extradata size for Opus streams",
which only ensured the mpets demuxer would output something the mov muxer would
not mishandle, this is a proper fix for the ticket, making the muxer spec
compliant.

  libavformat/movenc.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0b620a802d..8e6ed817d8 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -797,6 +797,7 @@ static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack 
*track)
  static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack 
*track)
  {
  int64_t pos = avio_tell(pb);
+int channels, channel_map;
  avio_wb32(pb, 0);
  ffio_wfourcc(pb, "dOps");
  avio_w8(pb, 0); /* Version */
@@ -807,12 +808,22 @@ static int mov_write_dops_tag(AVFormatContext *s, 
AVIOContext *pb, MOVTrack *tra
  /* extradata contains an Ogg OpusHead, other than byte-ordering and
 OpusHead's preceeding magic/version, OpusSpecificBox is currently
 identical. */
-avio_w8(pb, AV_RB8(track->par->extradata + 9)); /* OuputChannelCount */
+channels = AV_RB8(track->par->extradata + 9);
+channel_map = AV_RB8(track->par->extradata + 18);
+
+avio_w8(pb, channels); /* OuputChannelCount */
  avio_wb16(pb, AV_RL16(track->par->extradata + 10)); /* PreSkip */
  avio_wb32(pb, AV_RL32(track->par->extradata + 12)); /* InputSampleRate */
  avio_wb16(pb, AV_RL16(track->par->extradata + 16)); /* OutputGain */
+avio_w8(pb, channel_map); /* ChannelMappingFamily */
  /* Write the rest of the header out without byte-swapping. */
-avio_write(pb, track->par->extradata + 18, track->par->extradata_size - 
18);
+if (channel_map) {
+if (track->par->extradata_size < 21 + channels) {
+av_log(s, AV_LOG_ERROR, "invalid extradata size\n");
+return AVERROR_INVALIDDATA;
+}
+avio_write(pb, track->par->extradata + 19, 2 + channels); /* 
ChannelMappingTable */
+}
  
  return update_size(pb, pos);

  }


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

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] avformat/webpenc: don't assume animated webp streams will have more than one packet

2021-04-16 Thread Carl Eugen Hoyos



> Am 16.04.2021 um 16:15 schrieb James Almer :
> 
> ffmpeg | branch: master | James Almer  | Sat Apr 10 
> 22:53:34 2021 -0300| [55d667d86af7d13fc5aa2b4071a5b97eb10e2da6] | committer: 
> James Almer
> 
> avformat/webpenc: don't assume animated webp streams will have more than one 
> packet
> 
> The libwebp_animencoder returns a single packet with the entire animated
> stream, as that's what the external library produces. As such, only ensure the
> stream was produced by said encoder (or propagated by a demuxer, once support
> is added) when attempting to write the requested loop value.
> 
> Fixes ticket #9179.
> 
> Signed-off-by: James Almer 
> 
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=55d667d86af7d13fc5aa2b4071a5b97eb10e2da6
> ---
> 
> libavformat/webpenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
> index ed8325c02d..ca4ffc4e2d 100644
> --- a/libavformat/webpenc.c
> +++ b/libavformat/webpenc.c
> @@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
> WebpContext *w = s->priv_data;
> 
> if (w->using_webp_anim_encoder) {
> -if ((w->frame_count > 1) && w->loop) {  // Write loop count.
> +if (w->loop) {  // Write loop count.
> avio_seek(s->pb, 42, SEEK_SET);
> avio_wl16(s->pb, w->loop);
> }

I wonder now why you are asking for reviews, more so given that you tend to 
block patches you consider not good enough.

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

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] avformat/webpenc: don't assume animated webp streams will have more than one packet

2021-04-16 Thread James Almer


On 4/16/2021 11:36 AM, Carl Eugen Hoyos wrote:




Am 16.04.2021 um 16:15 schrieb James Almer :

ffmpeg | branch: master | James Almer  | Sat Apr 10 
22:53:34 2021 -0300| [55d667d86af7d13fc5aa2b4071a5b97eb10e2da6] | committer: James 
Almer

avformat/webpenc: don't assume animated webp streams will have more than one 
packet

The libwebp_animencoder returns a single packet with the entire animated
stream, as that's what the external library produces. As such, only ensure the
stream was produced by said encoder (or propagated by a demuxer, once support
is added) when attempting to write the requested loop value.

Fixes ticket #9179.

Signed-off-by: James Almer 


http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=55d667d86af7d13fc5aa2b4071a5b97eb10e2da6

---

libavformat/webpenc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
index ed8325c02d..ca4ffc4e2d 100644
--- a/libavformat/webpenc.c
+++ b/libavformat/webpenc.c
@@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
 WebpContext *w = s->priv_data;

 if (w->using_webp_anim_encoder) {
-if ((w->frame_count > 1) && w->loop) {  // Write loop count.
+if (w->loop) {  // Write loop count.
 avio_seek(s->pb, 42, SEEK_SET);
 avio_wl16(s->pb, w->loop);
 }


I wonder now why you are asking for reviews, more so given that you tend to 
block patches you consider not good enough.

Carl Eugen


I sent this patch a week ago. The only comment was your suggestion to 
move the loop count writing to write_packet(). That's still a 
possibility, but it's orthogonal to the regression this patch fixed, 
given it's a change to enable this muxer to work with non-seekable output.

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

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread Michael Niedermayer

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:
> On 4/15/2021 5:44 PM, Michael Niedermayer wrote:
> > Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot be 
> > represented in type 'int'
> > Fixes: 
> > 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >   libavformat/rmdec.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> > index fc3bff4859..af032ed90a 100644
> > --- a/libavformat/rmdec.c
> > +++ b/libavformat/rmdec.c
> > @@ -269,9 +269,9 @@ static int rm_read_audio_stream_info(AVFormatContext 
> > *s, AVIOContext *pb,
> >   case DEINT_ID_INT4:
> >   if (ast->coded_framesize > ast->audio_framesize ||
> >   sub_packet_h <= 1 ||
> > -ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 
> > 1)) * ast->audio_framesize)
> > +ast->coded_framesize * (uint64_t)sub_packet_h > (2 + 
> > (sub_packet_h & 1)) * ast->audio_framesize)
> 
> This check seems superfluous with the one below right after it.
> ast->coded_framesize * sub_packet_h must be equal to 2 *
> ast->audio_framesize. It can be removed.
> 
> >   return AVERROR_INVALIDDATA;
> > -if (ast->coded_framesize * sub_packet_h != 
> > 2*ast->audio_framesize) {
> > +if (ast->coded_framesize * (uint64_t)sub_packet_h != 
> > 2*ast->audio_framesize) {
> >   avpriv_request_sample(s, "mismatching interleaver 
> > parameters");
> >   return AVERROR_INVALIDDATA;
> >   }
> 
> How about something like
> 
> > diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> > index fc3bff4859..09880ee3fe 100644
> > --- a/libavformat/rmdec.c
> > +++ b/libavformat/rmdec.c
> > @@ -269,7 +269,7 @@ static int rm_read_audio_stream_info(AVFormatContext 
> > *s, AVIOContext *pb,
> >  case DEINT_ID_INT4:
> >  if (ast->coded_framesize > ast->audio_framesize ||
> >  sub_packet_h <= 1 ||
> > -ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 
> > 1)) * ast->audio_framesize)
> > +ast->audio_framesize > INT_MAX / sub_packet_h)
> >  return AVERROR_INVALIDDATA;
> >  if (ast->coded_framesize * sub_packet_h != 
> > 2*ast->audio_framesize) {
> >  avpriv_request_sample(s, "mismatching interleaver 
> > parameters");
> 
> Instead?

The 2 if() execute different things, the 2nd requests a sample, the first
not. I think this suggestion would change when we request a sample

thx

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

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott



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] avformat/wavdec: Fix reading files with id3v2 apic before fmt tag

2021-04-16 Thread Andreas Rheinhardt

In this case the cover images will get the stream index 0, violating
the hardcoded assumption that this is the index of the audio stream.

Fixes #8540; regression since f5aad350d3695b5b16e7d135154a4c61e4dce9d8.

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

diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index 8214ab8498..6e5843 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -49,6 +49,7 @@ typedef struct WAVDemuxContext {
 const AVClass *class;
 int64_t data_end;
 int w64;
+AVStream *ast, *vst;
 int64_t smv_data_ofs;
 int smv_block_size;
 int smv_frames_per_jpeg;
@@ -77,7 +78,7 @@ static const AVOption demux_options[] = {
 
 static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav)
 {
-if (CONFIG_SPDIF_DEMUXER && s->streams[0]->codecpar->codec_tag == 1) {
+if (CONFIG_SPDIF_DEMUXER && wav->ast->codecpar->codec_tag == 1) {
 enum AVCodecID codec;
 int len = 1<<16;
 int ret = ffio_ensure_seekback(s->pb, len);
@@ -92,7 +93,7 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext 
*wav)
 if (len >= 0) {
 ret = ff_spdif_probe(buf, len, &codec);
 if (ret > AVPROBE_SCORE_EXTENSION) {
-s->streams[0]->codecpar->codec_id = codec;
+wav->ast->codecpar->codec_id = codec;
 wav->spdif = 1;
 }
 }
@@ -180,6 +181,7 @@ static int wav_parse_fmt_tag(AVFormatContext *s, int64_t 
size, AVStream **st)
 *st = avformat_new_stream(s, NULL);
 if (!*st)
 return AVERROR(ENOMEM);
+wav->ast = *st;
 
 ret = ff_get_wav_header(s, pb, (*st)->codecpar, size, wav->rifx);
 if (ret < 0)
@@ -196,6 +198,7 @@ static int wav_parse_fmt_tag(AVFormatContext *s, int64_t 
size, AVStream **st)
 static int wav_parse_xma2_tag(AVFormatContext *s, int64_t size, AVStream **st)
 {
 AVIOContext *pb = s->pb;
+WAVDemuxContext *wav = s->priv_data;
 int version, num_streams, i, channels = 0, ret;
 
 if (size < 36)
@@ -204,6 +207,7 @@ static int wav_parse_xma2_tag(AVFormatContext *s, int64_t 
size, AVStream **st)
 *st = avformat_new_stream(s, NULL);
 if (!*st)
 return AVERROR(ENOMEM);
+wav->ast = *st;
 
 (*st)->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
 (*st)->codecpar->codec_id   = AV_CODEC_ID_XMA2;
@@ -484,6 +488,7 @@ static int wav_read_header(AVFormatContext *s)
 vst = avformat_new_stream(s, NULL);
 if (!vst)
 return AVERROR(ENOMEM);
+wav->vst = vst;
 avio_r8(pb);
 vst->id = 1;
 vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
@@ -693,23 +698,29 @@ static int wav_read_packet(AVFormatContext *s, AVPacket 
*pkt)
 {
 int ret, size;
 int64_t left;
-AVStream *st;
 WAVDemuxContext *wav = s->priv_data;
+AVStream *st = wav->ast;
 
-if (CONFIG_SPDIF_DEMUXER && wav->spdif == 1)
-return ff_spdif_read_packet(s, pkt);
+if (CONFIG_SPDIF_DEMUXER && wav->spdif == 1) {
+ret = ff_spdif_read_packet(s, pkt);
+if (ret < 0)
+return ret;
+pkt->stream_index = st->index;
+return 0;
+}
 
 if (wav->smv_data_ofs > 0) {
 int64_t audio_dts, video_dts;
+AVStream *vst = wav->vst;
 smv_retry:
-audio_dts = (int32_t)s->streams[0]->cur_dts;
-video_dts = (int32_t)s->streams[1]->cur_dts;
+audio_dts = (int32_t)st->cur_dts;
+video_dts = (int32_t)vst->cur_dts;
 
 if (audio_dts != AV_NOPTS_VALUE && video_dts != AV_NOPTS_VALUE) {
 /*We always return a video frame first to get the pixel format 
first*/
 wav->smv_last_stream = wav->smv_given_first ?
-av_compare_ts(video_dts, s->streams[1]->time_base,
-  audio_dts, s->streams[0]->time_base) > 0 : 0;
+av_compare_ts(video_dts, vst->time_base,
+  audio_dts,  st->time_base) > 0 : 0;
 wav->smv_given_first = 1;
 }
 wav->smv_last_stream = !wav->smv_last_stream;
@@ -732,7 +743,7 @@ smv_retry:
 pkt->duration = wav->smv_frames_per_jpeg;
 wav->smv_block++;
 
-pkt->stream_index = 1;
+pkt->stream_index = vst->index;
 smv_out:
 avio_seek(s->pb, old_pos, SEEK_SET);
 if (ret == AVERROR_EOF) {
@@ -743,8 +754,6 @@ smv_out:
 }
 }
 
-st = s->streams[0];
-
 left = wav->data_end - avio_tell(s->pb);
 if (wav->ignore_length)
 left = INT_MAX;
@@ -772,7 +781,7 @@ smv_out:
 ret  = av_get_packet(s->pb, pkt, size);
 if (ret < 0)
 return ret;
-pkt->stream_index = 0;
+pkt->stream_index = st->index;
 
 return ret;
 }
@@ -781,22 +790,25 @@ static int w

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot be 
represented in type 'int'
Fixes: 
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576

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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int rm_read_audio_stream_info(AVFormatContext *s, 
AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
-ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 1)) * 
ast->audio_framesize)
+ast->coded_framesize * (uint64_t)sub_packet_h > (2 + (sub_packet_h 
& 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


   return AVERROR_INVALIDDATA;
-if (ast->coded_framesize * sub_packet_h != 2*ast->audio_framesize) 
{
+if (ast->coded_framesize * (uint64_t)sub_packet_h != 
2*ast->audio_framesize) {
   avpriv_request_sample(s, "mismatching interleaver 
parameters");
   return AVERROR_INVALIDDATA;
   }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int rm_read_audio_stream_info(AVFormatContext *s, 
AVIOContext *pb,
  case DEINT_ID_INT4:
  if (ast->coded_framesize > ast->audio_framesize ||
  sub_packet_h <= 1 ||
-ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 1)) * 
ast->audio_framesize)
+ast->audio_framesize > INT_MAX / sub_packet_h)
  return AVERROR_INVALIDDATA;
  if (ast->coded_framesize * sub_packet_h != 
2*ast->audio_framesize) {
  avpriv_request_sample(s, "mismatching interleaver 
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the first
not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that 
matter? If it's considered an invalid scenario, do we really need a sample?


In any case, if you don't want more files where "ast->coded_framesize * 
sub_packet_h != 2*ast->audio_framesize" would print a sample request, 
then maybe something like the following could be used instead?



diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int rm_read_audio_stream_info(AVFormatContext *s, 
AVIOContext *pb,
 case DEINT_ID_INT4:
 if (ast->coded_framesize > ast->audio_framesize ||
 sub_packet_h <= 1 ||
+ast->audio_framesize > INT_MAX / sub_packet_h ||
 ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 1)) * 
ast->audio_framesize)
 return AVERROR_INVALIDDATA;
 if (ast->coded_framesize * sub_packet_h != 2*ast->audio_framesize) 
{
@@ -278,12 +279,16 @@ static int rm_read_audio_stream_info(AVFormatContext *s, 
AVIOContext *pb,
 break;
 case DEINT_ID_GENR:
 if (ast->sub_packet_size <= 0 ||
+ast->audio_framesize > INT_MAX / sub_packet_h ||
 ast->sub_packet_size > ast->audio_framesize)
 return AVERROR_INVALIDDATA;
 if (ast->audio_framesize % ast->sub_packet_size)
 return AVERROR_INVALIDDATA;
 break;
 case DEINT_ID_SIPR:
+if (ast->audio_framesize > INT_MAX / sub_packet_h)
+return AVERROR_INVALIDDATA;
+break;
 case DEINT_ID_INT0:
 case DEINT_ID_VBRS:
 case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int rm_read_audio_stream_info(AVFormatContext *s, 
AVIOContext *pb,
 ast->deint_id == DEINT_ID_GENR ||
 ast->deint_id == DEINT_ID_SIPR) {
 if (st->codecpar->block_align <= 0 ||
-ast->audio_framesize * (uint64_t)sub_packet_h > 
(unsigned)INT_MAX ||
 ast->audio_framesize * sub_packet_h < 
st->codecpar->block_align)
 return AVERROR_INVALIDDATA;
 if (av_new_packet(&ast->pkt, ast->audio_framesize * sub_packet_h) 
< 0)


Same amount of checks for all three deint ids, and no

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread Andreas Rheinhardt

James Almer:
> On 4/16/2021 4:04 PM, Michael Niedermayer wrote:
>> On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:
>>> On 4/15/2021 5:44 PM, Michael Niedermayer wrote:
 Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
 be represented in type 'int'
 Fixes:
 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576


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

 diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
 index fc3bff4859..af032ed90a 100644
 --- a/libavformat/rmdec.c
 +++ b/libavformat/rmdec.c
 @@ -269,9 +269,9 @@ static int
 rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
 -    ast->coded_framesize * sub_packet_h > (2 +
 (sub_packet_h & 1)) * ast->audio_framesize)
 +    ast->coded_framesize * (uint64_t)sub_packet_h > (2
 + (sub_packet_h & 1)) * ast->audio_framesize)
>>>
>>> This check seems superfluous with the one below right after it.
>>> ast->coded_framesize * sub_packet_h must be equal to 2 *
>>> ast->audio_framesize. It can be removed.
>>>
    return AVERROR_INVALIDDATA;
 -    if (ast->coded_framesize * sub_packet_h !=
 2*ast->audio_framesize) {
 +    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
 2*ast->audio_framesize) {
    avpriv_request_sample(s, "mismatching interleaver
 parameters");
    return AVERROR_INVALIDDATA;
    }
>>>
>>> How about something like
>>>
 diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
 index fc3bff4859..09880ee3fe 100644
 --- a/libavformat/rmdec.c
 +++ b/libavformat/rmdec.c
 @@ -269,7 +269,7 @@ static int
 rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
 -    ast->coded_framesize * sub_packet_h > (2 +
 (sub_packet_h & 1)) * ast->audio_framesize)
 +    ast->audio_framesize > INT_MAX / sub_packet_h)
   return AVERROR_INVALIDDATA;
   if (ast->coded_framesize * sub_packet_h !=
 2*ast->audio_framesize) {
   avpriv_request_sample(s, "mismatching interleaver
 parameters");
>>>
>>> Instead?
>>
>> The 2 if() execute different things, the 2nd requests a sample, the first
>> not. I think this suggestion would change when we request a sample
> 
> Why are we returning INVALIDDATA after requesting a sample, for that
> matter? If it's considered an invalid scenario, do we really need a sample?
> 
> In any case, if you don't want more files where "ast->coded_framesize *
> sub_packet_h != 2*ast->audio_framesize" would print a sample request,
> then maybe something like the following could be used instead?
> 
>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> index fc3bff4859..10c1699a81 100644
>> --- a/libavformat/rmdec.c
>> +++ b/libavformat/rmdec.c
>> @@ -269,6 +269,7 @@ static int
>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>  case DEINT_ID_INT4:
>>  if (ast->coded_framesize > ast->audio_framesize ||
>>  sub_packet_h <= 1 ||
>> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>>  ast->coded_framesize * sub_packet_h > (2 +
>> (sub_packet_h & 1)) * ast->audio_framesize)
>>  return AVERROR_INVALIDDATA;
>>  if (ast->coded_framesize * sub_packet_h !=
>> 2*ast->audio_framesize) {
>> @@ -278,12 +279,16 @@ static int
>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>  break;
>>  case DEINT_ID_GENR:
>>  if (ast->sub_packet_size <= 0 ||
>> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>>  ast->sub_packet_size > ast->audio_framesize)
>>  return AVERROR_INVALIDDATA;
>>  if (ast->audio_framesize % ast->sub_packet_size)
>>  return AVERROR_INVALIDDATA;
>>  break;
>>  case DEINT_ID_SIPR:
>> +    if (ast->audio_framesize > INT_MAX / sub_packet_h)

sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.

>> +    return AVERROR_INVALIDDATA;
>> +    break;
>>  case DEINT_ID_INT0:
>>  case DEINT_ID_VBRS:
>>  case DEINT_ID_VBRF:
>> @@ -296,7 +301,6 @@ static int
>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
be represented in type 'int'
Fixes:
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576


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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->coded_framesize * (uint64_t)sub_packet_h > (2
+ (sub_packet_h & 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


    return AVERROR_INVALIDDATA;
-    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
+    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
2*ast->audio_framesize) {
    avpriv_request_sample(s, "mismatching interleaver
parameters");
    return AVERROR_INVALIDDATA;
    }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->audio_framesize > INT_MAX / sub_packet_h)
   return AVERROR_INVALIDDATA;
   if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
   avpriv_request_sample(s, "mismatching interleaver
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the first
not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that
matter? If it's considered an invalid scenario, do we really need a sample?

In any case, if you don't want more files where "ast->coded_framesize *
sub_packet_h != 2*ast->audio_framesize" would print a sample request,
then maybe something like the following could be used instead?


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  case DEINT_ID_INT4:
  if (ast->coded_framesize > ast->audio_framesize ||
  sub_packet_h <= 1 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
@@ -278,12 +279,16 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  break;
  case DEINT_ID_GENR:
  if (ast->sub_packet_size <= 0 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->sub_packet_size > ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->audio_framesize % ast->sub_packet_size)
  return AVERROR_INVALIDDATA;
  break;
  case DEINT_ID_SIPR:
+    if (ast->audio_framesize > INT_MAX / sub_packet_h)


sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.


Ah, good catch. This also means av_new_packet() is potentially being 
called with 0 as size for these two codepaths.





+    return AVERROR_INVALIDDATA;
+    break;
  case DEINT_ID_INT0:
  case DEINT_ID_VBRS:
  case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  ast->deint_id == DEINT_ID_GENR ||
  ast->deint_id == DEINT_ID_SIPR) {
  if (st->codecpar->block_align <= 0 ||
-    ast->audio_framesize * (uint64_t)sub_packet_h

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 7:45 PM, James Almer wrote:

On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
be represented in type 'int'
Fixes:
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576 




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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->coded_framesize * (uint64_t)sub_packet_h > (2
+ (sub_packet_h & 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


    return AVERROR_INVALIDDATA;
-    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
+    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
2*ast->audio_framesize) {
    avpriv_request_sample(s, "mismatching interleaver
parameters");
    return AVERROR_INVALIDDATA;
    }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->audio_framesize > INT_MAX / sub_packet_h)
   return AVERROR_INVALIDDATA;
   if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
   avpriv_request_sample(s, "mismatching interleaver
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the 
first

not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that
matter? If it's considered an invalid scenario, do we really need a 
sample?


In any case, if you don't want more files where "ast->coded_framesize *
sub_packet_h != 2*ast->audio_framesize" would print a sample request,
then maybe something like the following could be used instead?


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  case DEINT_ID_INT4:
  if (ast->coded_framesize > ast->audio_framesize ||
  sub_packet_h <= 1 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
@@ -278,12 +279,16 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  break;
  case DEINT_ID_GENR:
  if (ast->sub_packet_size <= 0 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->sub_packet_size > ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->audio_framesize % ast->sub_packet_size)
  return AVERROR_INVALIDDATA;
  break;
  case DEINT_ID_SIPR:
+    if (ast->audio_framesize > INT_MAX / sub_packet_h)


sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.


Ah, good catch. This also means av_new_packet() is potentially being 
called with 0 as size for these two codepaths.


My bad, the check right before the av_new_packet() call makes ensures 
it's not.







+    return AVERROR_INVALIDDATA;
+    break;
  case DEINT_ID_INT0:
  case DEINT_ID_VBRS:
  case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  ast->deint_id == DEINT_ID_GENR ||
  ast->deint_id =

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 7:45 PM, James Almer wrote:

On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
be represented in type 'int'
Fixes:
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576 




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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->coded_framesize * (uint64_t)sub_packet_h > (2
+ (sub_packet_h & 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


    return AVERROR_INVALIDDATA;
-    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
+    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
2*ast->audio_framesize) {
    avpriv_request_sample(s, "mismatching interleaver
parameters");
    return AVERROR_INVALIDDATA;
    }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->audio_framesize > INT_MAX / sub_packet_h)
   return AVERROR_INVALIDDATA;
   if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
   avpriv_request_sample(s, "mismatching interleaver
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the 
first

not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that
matter? If it's considered an invalid scenario, do we really need a 
sample?


In any case, if you don't want more files where "ast->coded_framesize *
sub_packet_h != 2*ast->audio_framesize" would print a sample request,
then maybe something like the following could be used instead?


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  case DEINT_ID_INT4:
  if (ast->coded_framesize > ast->audio_framesize ||
  sub_packet_h <= 1 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
@@ -278,12 +279,16 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  break;
  case DEINT_ID_GENR:
  if (ast->sub_packet_size <= 0 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
  ast->sub_packet_size > ast->audio_framesize)
  return AVERROR_INVALIDDATA;
  if (ast->audio_framesize % ast->sub_packet_size)
  return AVERROR_INVALIDDATA;
  break;
  case DEINT_ID_SIPR:
+    if (ast->audio_framesize > INT_MAX / sub_packet_h)


sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.


Ah, good catch. This also means av_new_packet() is potentially being 
called with 0 as size for these two codepaths.





+    return AVERROR_INVALIDDATA;
+    break;
  case DEINT_ID_INT0:
  case DEINT_ID_VBRS:
  case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  ast->deint_id == DEINT_ID_GENR ||
  ast->deint_id == DEINT_ID_SIPR) {
  if (st->codecpar->block_align <= 0 ||
-

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread Andreas Rheinhardt

James Almer:
> On 4/16/2021 7:45 PM, James Almer wrote:
>> On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:
>>> James Almer:
 On 4/16/2021 4:04 PM, Michael Niedermayer wrote:
> On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:
>> On 4/15/2021 5:44 PM, Michael Niedermayer wrote:
>>> Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
>>> be represented in type 'int'
>>> Fixes:
>>> 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576
>>>
>>>
>>>
>>> Found-by: continuous fuzzing process
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer 
>>> ---
>>>     libavformat/rmdec.c | 4 ++--
>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index fc3bff4859..af032ed90a 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -269,9 +269,9 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>     case DEINT_ID_INT4:
>>>     if (ast->coded_framesize > ast->audio_framesize ||
>>>     sub_packet_h <= 1 ||
>>> -    ast->coded_framesize * sub_packet_h > (2 +
>>> (sub_packet_h & 1)) * ast->audio_framesize)
>>> +    ast->coded_framesize * (uint64_t)sub_packet_h > (2
>>> + (sub_packet_h & 1)) * ast->audio_framesize)
>>
>> This check seems superfluous with the one below right after it.
>> ast->coded_framesize * sub_packet_h must be equal to 2 *
>> ast->audio_framesize. It can be removed.
>>
>>>     return AVERROR_INVALIDDATA;
>>> -    if (ast->coded_framesize * sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>> +    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>>     avpriv_request_sample(s, "mismatching
>>> interleaver
>>> parameters");
>>>     return AVERROR_INVALIDDATA;
>>>     }
>>
>> How about something like
>>
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index fc3bff4859..09880ee3fe 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -269,7 +269,7 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>    case DEINT_ID_INT4:
>>>    if (ast->coded_framesize > ast->audio_framesize ||
>>>    sub_packet_h <= 1 ||
>>> -    ast->coded_framesize * sub_packet_h > (2 +
>>> (sub_packet_h & 1)) * ast->audio_framesize)
>>> +    ast->audio_framesize > INT_MAX / sub_packet_h)
>>>    return AVERROR_INVALIDDATA;
>>>    if (ast->coded_framesize * sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>>    avpriv_request_sample(s, "mismatching interleaver
>>> parameters");
>>
>> Instead?
>
> The 2 if() execute different things, the 2nd requests a sample, the
> first
> not. I think this suggestion would change when we request a sample

 Why are we returning INVALIDDATA after requesting a sample, for that
 matter? If it's considered an invalid scenario, do we really need a
 sample?

 In any case, if you don't want more files where "ast->coded_framesize *
 sub_packet_h != 2*ast->audio_framesize" would print a sample request,
 then maybe something like the following could be used instead?

> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index fc3bff4859..10c1699a81 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -269,6 +269,7 @@ static int
> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>   case DEINT_ID_INT4:
>   if (ast->coded_framesize > ast->audio_framesize ||
>   sub_packet_h <= 1 ||
> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>   ast->coded_framesize * sub_packet_h > (2 +
> (sub_packet_h & 1)) * ast->audio_framesize)
>   return AVERROR_INVALIDDATA;
>   if (ast->coded_framesize * sub_packet_h !=
> 2*ast->audio_framesize) {
> @@ -278,12 +279,16 @@ static int
> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>   break;
>   case DEINT_ID_GENR:
>   if (ast->sub_packet_size <= 0 ||
> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>   ast->sub_packet_size > ast->audio_framesize)
>   return AVERROR_INVALIDDATA;
>   if (ast->audio_framesize % ast->sub_packet_size)
>   return AVERROR_INVALIDDATA;
>

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 8:45 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 7:45 PM, James Almer wrote:

On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot
be represented in type 'int'
Fixes:
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576



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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
     case DEINT_ID_INT4:
     if (ast->coded_framesize > ast->audio_framesize ||
     sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->coded_framesize * (uint64_t)sub_packet_h > (2
+ (sub_packet_h & 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


     return AVERROR_INVALIDDATA;
-    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
+    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
2*ast->audio_framesize) {
     avpriv_request_sample(s, "mismatching
interleaver
parameters");
     return AVERROR_INVALIDDATA;
     }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->audio_framesize > INT_MAX / sub_packet_h)
    return AVERROR_INVALIDDATA;
    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
    avpriv_request_sample(s, "mismatching interleaver
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the
first
not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that
matter? If it's considered an invalid scenario, do we really need a
sample?

In any case, if you don't want more files where "ast->coded_framesize *
sub_packet_h != 2*ast->audio_framesize" would print a sample request,
then maybe something like the following could be used instead?


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   case DEINT_ID_INT4:
   if (ast->coded_framesize > ast->audio_framesize ||
   sub_packet_h <= 1 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
   ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
   return AVERROR_INVALIDDATA;
   if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
@@ -278,12 +279,16 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   break;
   case DEINT_ID_GENR:
   if (ast->sub_packet_size <= 0 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
   ast->sub_packet_size > ast->audio_framesize)
   return AVERROR_INVALIDDATA;
   if (ast->audio_framesize % ast->sub_packet_size)
   return AVERROR_INVALIDDATA;
   break;
   case DEINT_ID_SIPR:
+    if (ast->audio_framesize > INT_MAX / sub_packet_h)


sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.


Ah, good catch. This also means av_new_packet() is potentially being
called with 0 as size for these two codepaths.




+    return AVERROR_INVALIDDATA;
+    break;
   case DEINT_ID_INT0:
   case DEINT_ID_VBRS:
   case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
   ast->deint_id == DEINT_ID_GENR ||
   ast->deint

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread Andreas Rheinhardt

James Almer:
> On 4/16/2021 8:45 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 4/16/2021 7:45 PM, James Almer wrote:
 On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 4/16/2021 4:04 PM, Michael Niedermayer wrote:
>>> On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:
 On 4/15/2021 5:44 PM, Michael Niedermayer wrote:
> Fixes: runtime error: signed integer overflow: 65312 * 65535
> cannot
> be represented in type 'int'
> Fixes:
> 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576
>
>
>
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/rmdec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index fc3bff4859..af032ed90a 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -269,9 +269,9 @@ static int
> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>  case DEINT_ID_INT4:
>  if (ast->coded_framesize >
> ast->audio_framesize ||
>  sub_packet_h <= 1 ||
> -    ast->coded_framesize * sub_packet_h > (2 +
> (sub_packet_h & 1)) * ast->audio_framesize)
> +    ast->coded_framesize * (uint64_t)sub_packet_h
> > (2
> + (sub_packet_h & 1)) * ast->audio_framesize)

 This check seems superfluous with the one below right after it.
 ast->coded_framesize * sub_packet_h must be equal to 2 *
 ast->audio_framesize. It can be removed.

>  return AVERROR_INVALIDDATA;
> -    if (ast->coded_framesize * sub_packet_h !=
> 2*ast->audio_framesize) {
> +    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
> 2*ast->audio_framesize) {
>  avpriv_request_sample(s, "mismatching
> interleaver
> parameters");
>  return AVERROR_INVALIDDATA;
>  }

 How about something like

> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index fc3bff4859..09880ee3fe 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -269,7 +269,7 @@ static int
> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>     case DEINT_ID_INT4:
>     if (ast->coded_framesize > ast->audio_framesize ||
>     sub_packet_h <= 1 ||
> -    ast->coded_framesize * sub_packet_h > (2 +
> (sub_packet_h & 1)) * ast->audio_framesize)
> +    ast->audio_framesize > INT_MAX / sub_packet_h)
>     return AVERROR_INVALIDDATA;
>     if (ast->coded_framesize * sub_packet_h !=
> 2*ast->audio_framesize) {
>     avpriv_request_sample(s, "mismatching
> interleaver
> parameters");

 Instead?
>>>
>>> The 2 if() execute different things, the 2nd requests a sample, the
>>> first
>>> not. I think this suggestion would change when we request a sample
>>
>> Why are we returning INVALIDDATA after requesting a sample, for that
>> matter? If it's considered an invalid scenario, do we really need a
>> sample?
>>
>> In any case, if you don't want more files where
>> "ast->coded_framesize *
>> sub_packet_h != 2*ast->audio_framesize" would print a sample request,
>> then maybe something like the following could be used instead?
>>
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index fc3bff4859..10c1699a81 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -269,6 +269,7 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>    case DEINT_ID_INT4:
>>>    if (ast->coded_framesize > ast->audio_framesize ||
>>>    sub_packet_h <= 1 ||
>>> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>>>    ast->coded_framesize * sub_packet_h > (2 +
>>> (sub_packet_h & 1)) * ast->audio_framesize)
>>>    return AVERROR_INVALIDDATA;
>>>    if (ast->coded_framesize * sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>> @@ -278,12 +279,16 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>    break;
>>>    case DEINT_ID_GENR:
>>>    if (ast-

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread James Almer


On 4/16/2021 9:13 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 8:45 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 7:45 PM, James Almer wrote:

On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:

James Almer:

On 4/16/2021 4:04 PM, Michael Niedermayer wrote:

On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:

On 4/15/2021 5:44 PM, Michael Niedermayer wrote:

Fixes: runtime error: signed integer overflow: 65312 * 65535
cannot
be represented in type 'int'
Fixes:
32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576




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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..af032ed90a 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,9 +269,9 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
  case DEINT_ID_INT4:
  if (ast->coded_framesize >
ast->audio_framesize ||
  sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->coded_framesize * (uint64_t)sub_packet_h

(2

+ (sub_packet_h & 1)) * ast->audio_framesize)


This check seems superfluous with the one below right after it.
ast->coded_framesize * sub_packet_h must be equal to 2 *
ast->audio_framesize. It can be removed.


  return AVERROR_INVALIDDATA;
-    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
+    if (ast->coded_framesize * (uint64_t)sub_packet_h !=
2*ast->audio_framesize) {
  avpriv_request_sample(s, "mismatching
interleaver
parameters");
  return AVERROR_INVALIDDATA;
  }


How about something like


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..09880ee3fe 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,7 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
     case DEINT_ID_INT4:
     if (ast->coded_framesize > ast->audio_framesize ||
     sub_packet_h <= 1 ||
-    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
+    ast->audio_framesize > INT_MAX / sub_packet_h)
     return AVERROR_INVALIDDATA;
     if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
     avpriv_request_sample(s, "mismatching
interleaver
parameters");


Instead?


The 2 if() execute different things, the 2nd requests a sample, the
first
not. I think this suggestion would change when we request a sample


Why are we returning INVALIDDATA after requesting a sample, for that
matter? If it's considered an invalid scenario, do we really need a
sample?

In any case, if you don't want more files where
"ast->coded_framesize *
sub_packet_h != 2*ast->audio_framesize" would print a sample request,
then maybe something like the following could be used instead?


diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index fc3bff4859..10c1699a81 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -269,6 +269,7 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    case DEINT_ID_INT4:
    if (ast->coded_framesize > ast->audio_framesize ||
    sub_packet_h <= 1 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
    ast->coded_framesize * sub_packet_h > (2 +
(sub_packet_h & 1)) * ast->audio_framesize)
    return AVERROR_INVALIDDATA;
    if (ast->coded_framesize * sub_packet_h !=
2*ast->audio_framesize) {
@@ -278,12 +279,16 @@ static int
rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
    break;
    case DEINT_ID_GENR:
    if (ast->sub_packet_size <= 0 ||
+    ast->audio_framesize > INT_MAX / sub_packet_h ||
    ast->sub_packet_size > ast->audio_framesize)
    return AVERROR_INVALIDDATA;
    if (ast->audio_framesize % ast->sub_packet_size)
    return AVERROR_INVALIDDATA;
    break;
    case DEINT_ID_SIPR:
+    if (ast->audio_framesize > INT_MAX / sub_packet_h)


sub_packet_h has not been checked for being != 0 here and in the
DEINT_ID_GENR codepath.


Ah, good catch. This also means av_new_packet() is potentially being
called with 0 as size for these two codepaths.




+    return AVERROR_INVALIDDATA;
+    break;
    case DEINT_ID_INT0:
    case DEINT_ID_VBRS:
    case DEINT_ID_VBRF:
@@ -296,7 +301,6 @@ static int
rm_read_audio_stream_info(AVFormatContex

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

2021-04-16 Thread Andreas Rheinhardt

James Almer:
> On 4/16/2021 9:13 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 4/16/2021 8:45 PM, Andreas Rheinhardt wrote:
 James Almer:
> On 4/16/2021 7:45 PM, James Almer wrote:
>> On 4/16/2021 7:24 PM, Andreas Rheinhardt wrote:
>>> James Almer:
 On 4/16/2021 4:04 PM, Michael Niedermayer wrote:
> On Thu, Apr 15, 2021 at 06:22:10PM -0300, James Almer wrote:
>> On 4/15/2021 5:44 PM, Michael Niedermayer wrote:
>>> Fixes: runtime error: signed integer overflow: 65312 * 65535
>>> cannot
>>> be represented in type 'int'
>>> Fixes:
>>> 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576
>>>
>>>
>>>
>>>
>>>
>>> Found-by: continuous fuzzing process
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer 
>>> ---
>>>   libavformat/rmdec.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index fc3bff4859..af032ed90a 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -269,9 +269,9 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>   case DEINT_ID_INT4:
>>>   if (ast->coded_framesize >
>>> ast->audio_framesize ||
>>>   sub_packet_h <= 1 ||
>>> -    ast->coded_framesize * sub_packet_h > (2 +
>>> (sub_packet_h & 1)) * ast->audio_framesize)
>>> +    ast->coded_framesize * (uint64_t)sub_packet_h
 (2
>>> + (sub_packet_h & 1)) * ast->audio_framesize)
>>
>> This check seems superfluous with the one below right after it.
>> ast->coded_framesize * sub_packet_h must be equal to 2 *
>> ast->audio_framesize. It can be removed.
>>
>>>   return AVERROR_INVALIDDATA;
>>> -    if (ast->coded_framesize * sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>> +    if (ast->coded_framesize *
>>> (uint64_t)sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>>   avpriv_request_sample(s, "mismatching
>>> interleaver
>>> parameters");
>>>   return AVERROR_INVALIDDATA;
>>>   }
>>
>> How about something like
>>
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index fc3bff4859..09880ee3fe 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -269,7 +269,7 @@ static int
>>> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>>>  case DEINT_ID_INT4:
>>>  if (ast->coded_framesize >
>>> ast->audio_framesize ||
>>>  sub_packet_h <= 1 ||
>>> -    ast->coded_framesize * sub_packet_h > (2 +
>>> (sub_packet_h & 1)) * ast->audio_framesize)
>>> +    ast->audio_framesize > INT_MAX / sub_packet_h)
>>>  return AVERROR_INVALIDDATA;
>>>  if (ast->coded_framesize * sub_packet_h !=
>>> 2*ast->audio_framesize) {
>>>  avpriv_request_sample(s, "mismatching
>>> interleaver
>>> parameters");
>>
>> Instead?
>
> The 2 if() execute different things, the 2nd requests a sample,
> the
> first
> not. I think this suggestion would change when we request a sample

 Why are we returning INVALIDDATA after requesting a sample, for
 that
 matter? If it's considered an invalid scenario, do we really need a
 sample?

 In any case, if you don't want more files where
 "ast->coded_framesize *
 sub_packet_h != 2*ast->audio_framesize" would print a sample
 request,
 then maybe something like the following could be used instead?

> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index fc3bff4859..10c1699a81 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -269,6 +269,7 @@ static int
> rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>     case DEINT_ID_INT4:
>     if (ast->coded_framesize > ast->audio_framesize ||
>     sub_packet_h <= 1 ||
> +    ast->audio_framesize > INT_MAX / sub_packet_h ||
>     ast->coded_framesize * sub_packet_h > (2 +
> (sub_packet_h & 1)) * ast->audio_framesize)
>

[FFmpeg-devel] [PATCH v2] avformat/wavdec: Fix reading files with id3v2 apic before fmt tag

2021-04-16 Thread Andreas Rheinhardt

Up until now the cover images will get the stream index 0 in this case,
violating the hardcoded assumption that this is the index of the audio
stream. Fix this by creating the audio stream first; this is also in
line with the expectations of ff_pcm_read_seek() and
ff_spdif_read_packet(). It also simplifies the code to parse the fmt and
xma2 tags.

Fixes #8540; regression since f5aad350d3695b5b16e7d135154a4c61e4dce9d8.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wavdec.c | 78 ++--
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index 8214ab8498..791ae23b4a 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -49,6 +49,7 @@ typedef struct WAVDemuxContext {
 const AVClass *class;
 int64_t data_end;
 int w64;
+AVStream *vst;
 int64_t smv_data_ofs;
 int smv_block_size;
 int smv_frames_per_jpeg;
@@ -170,30 +171,26 @@ static void handle_stream_probing(AVStream *st)
 }
 }
 
-static int wav_parse_fmt_tag(AVFormatContext *s, int64_t size, AVStream **st)
+static int wav_parse_fmt_tag(AVFormatContext *s, int64_t size, AVStream *st)
 {
 AVIOContext *pb = s->pb;
 WAVDemuxContext *wav = s->priv_data;
 int ret;
 
 /* parse fmt header */
-*st = avformat_new_stream(s, NULL);
-if (!*st)
-return AVERROR(ENOMEM);
-
-ret = ff_get_wav_header(s, pb, (*st)->codecpar, size, wav->rifx);
+ret = ff_get_wav_header(s, pb, st->codecpar, size, wav->rifx);
 if (ret < 0)
 return ret;
-handle_stream_probing(*st);
+handle_stream_probing(st);
 
-(*st)->need_parsing = AVSTREAM_PARSE_FULL_RAW;
+st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
 
-avpriv_set_pts_info(*st, 64, 1, (*st)->codecpar->sample_rate);
+avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
 
 return 0;
 }
 
-static int wav_parse_xma2_tag(AVFormatContext *s, int64_t size, AVStream **st)
+static int wav_parse_xma2_tag(AVFormatContext *s, int64_t size, AVStream *st)
 {
 AVIOContext *pb = s->pb;
 int version, num_streams, i, channels = 0, ret;
@@ -201,13 +198,9 @@ static int wav_parse_xma2_tag(AVFormatContext *s, int64_t 
size, AVStream **st)
 if (size < 36)
 return AVERROR_INVALIDDATA;
 
-*st = avformat_new_stream(s, NULL);
-if (!*st)
-return AVERROR(ENOMEM);
-
-(*st)->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
-(*st)->codecpar->codec_id   = AV_CODEC_ID_XMA2;
-(*st)->need_parsing = AVSTREAM_PARSE_FULL_RAW;
+st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
+st->codecpar->codec_id   = AV_CODEC_ID_XMA2;
+st->need_parsing = AVSTREAM_PARSE_FULL_RAW;
 
 version = avio_r8(pb);
 if (version != 3 && version != 4)
@@ -216,26 +209,26 @@ static int wav_parse_xma2_tag(AVFormatContext *s, int64_t 
size, AVStream **st)
 if (size != (32 + ((version==3)?0:8) + 4*num_streams))
 return AVERROR_INVALIDDATA;
 avio_skip(pb, 10);
-(*st)->codecpar->sample_rate = avio_rb32(pb);
+st->codecpar->sample_rate = avio_rb32(pb);
 if (version == 4)
 avio_skip(pb, 8);
 avio_skip(pb, 4);
-(*st)->duration = avio_rb32(pb);
+st->duration = avio_rb32(pb);
 avio_skip(pb, 8);
 
 for (i = 0; i < num_streams; i++) {
 channels += avio_r8(pb);
 avio_skip(pb, 3);
 }
-(*st)->codecpar->channels = channels;
+st->codecpar->channels = channels;
 
-if ((*st)->codecpar->channels <= 0 || (*st)->codecpar->sample_rate <= 0)
+if (st->codecpar->channels <= 0 || st->codecpar->sample_rate <= 0)
 return AVERROR_INVALIDDATA;
 
-avpriv_set_pts_info(*st, 64, 1, (*st)->codecpar->sample_rate);
+avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
 
 avio_seek(pb, -size, SEEK_CUR);
-if ((ret = ff_get_extradata(s, (*st)->codecpar, pb, size)) < 0)
+if ((ret = ff_get_extradata(s, st->codecpar, pb, size)) < 0)
 return ret;
 
 return 0;
@@ -407,6 +400,11 @@ static int wav_read_header(AVFormatContext *s)
 
 }
 
+/* Create the audio stream now so that its index is always zero */
+st = avformat_new_stream(s, NULL);
+if (!st)
+return AVERROR(ENOMEM);
+
 for (;;) {
 AVStream *vst;
 size = next_tag(pb, &tag, wav->rifx);
@@ -418,7 +416,7 @@ static int wav_read_header(AVFormatContext *s)
 switch (tag) {
 case MKTAG('f', 'm', 't', ' '):
 /* only parse the first 'fmt ' tag found */
-if (!got_xma2 && !got_fmt && (ret = wav_parse_fmt_tag(s, size, 
&st)) < 0) {
+if (!got_xma2 && !got_fmt && (ret = wav_parse_fmt_tag(s, size, 
st)) < 0) {
 return ret;
 } else if (got_fmt)
 av_log(s, AV_LOG_WARNING, "found more than one 'fmt ' tag\n");
@@ -427,7 +425,7 @@ static int wav_read_header(AVFormatContext *s)
 break;
 case MKTAG('X', 'M', 'A', '2'):

Re: [FFmpeg-devel] [PATCH] avutil/cpu: Use HW_NCPUONLINE to detect # of online CPUs with OpenBSD

2021-04-16 Thread Brad Smith


ping.

On 4/3/2021 2:49 PM, Brad Smith wrote:

avutil/cpu: Use HW_NCPUONLINE to detect # of online CPUs with OpenBSD

Signed-off-by: Brad Smith 
---
  libavutil/cpu.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index 8e3576a1f3..9d249737df 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -291,6 +291,12 @@ int av_cpu_count(void)
  DWORD_PTR proc_aff, sys_aff;
  if (GetProcessAffinityMask(GetCurrentProcess(), &proc_aff, &sys_aff))
  nb_cpus = av_popcount64(proc_aff);
+#elif HAVE_SYSCTL && defined(HW_NCPUONLINE)
+int mib[2] = { CTL_HW, HW_NCPUONLINE };
+size_t len = sizeof(nb_cpus);
+
+if (sysctl(mib, 2, &nb_cpus, &len, NULL, 0) == -1)
+nb_cpus = 0;
  #elif HAVE_SYSCTL && defined(HW_NCPU)
  int mib[2] = { CTL_HW, HW_NCPU };
  size_t len = sizeof(nb_cpus);

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

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

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffprobe: Remove check on show_frames and show_packets in XML writer

[FFmpeg-devel] [PATCH] Added bandwidth parameter manual configurare in HLS master playlist

[FFmpeg-devel] [PATCH] fate/ffprobe: Verify ffprobe XML output against schema file

Re: [FFmpeg-devel] [PATCH] avformat/utils: Combine identical statements

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: Remove unnecessary function calls

Re: [FFmpeg-devel] [PATCH] avcodec/jpeglsdec: Don't allocate+free JPEGLSState for every frame

Re: [FFmpeg-devel] [PATCH 1/2] Include attributes.h directly

[FFmpeg-devel] [PATCH 1/1] avcodec/nvenc: move lossless presets after new ones

[FFmpeg-devel] [PATCH 1/1] avcodec/nvenc: move lossless presets after new ones

Re: [FFmpeg-devel] [PATCH] avformat/mpegts: set correct extradata size for Opus streams

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/libwebpenc_animencoder: stop propagating bogus empty packets

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_astats: Remove fraction part of integer metadata entries

Re: [FFmpeg-devel] [PATCH] avcodec/libaomdec: export frame pict_type

Re: [FFmpeg-devel] [PATCH] avformat/movenc: fix writing dOps atoms

Re: [FFmpeg-devel] [FFmpeg-cvslog] avformat/webpenc: don't assume animated webp streams will have more than one packet

Re: [FFmpeg-devel] [FFmpeg-cvslog] avformat/webpenc: don't assume animated webp streams will have more than one packet

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

[FFmpeg-devel] [PATCH] avformat/wavdec: Fix reading files with id3v2 apic before fmt tag

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

Re: [FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

[FFmpeg-devel] [PATCH v2] avformat/wavdec: Fix reading files with id3v2 apic before fmt tag

Re: [FFmpeg-devel] [PATCH] avutil/cpu: Use HW_NCPUONLINE to detect # of online CPUs with OpenBSD

30 matches

Site Navigation

Mail list logo

Footer information