Re: [FFmpeg-devel] [PATCH 1/3] avcodec/alsdec: Limit maximum channels to 64

[Thilo Borgmann]

Am 19.08.19 um 01:30 schrieb Michael Niedermayer:
> There seems to be no limit in the specification and upto 64k could be stored
> 64 is chooses as limit as thats also used for AAC and is what a int64 mask
> can handle
> 
> An alternative to this patch would be a max_channels variable

There's a conformance file containing 512 channels, that should be the default 
max value.

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

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

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/alsdec: Fix integer overflow in decode_var_block_data()

[Thilo Borgmann]

Am 19.08.19 um 01:30 schrieb Michael Niedermayer:
> Fixes: signed integer overflow: 1927975249 - -514719744 cannot be represented 
> in type 'int'
> Fixes: 
> 16413/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5651206856245248
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/alsdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
> index 425cf73be9..4794556aad 100644
> --- a/libavcodec/alsdec.c
> +++ b/libavcodec/alsdec.c
> @@ -953,7 +953,7 @@ static int decode_var_block_data(ALSDecContext *ctx, 
> ALSBlockData *bd)
>  
>  // reconstruct difference signal for prediction (joint-stereo)
>  if (bd->js_blocks && bd->raw_other) {
> -int32_t *left, *right;
> +uint32_t *left, *right;
>  
>  if (bd->raw_other > raw_samples) {  // D = R - L
>  left  = raw_samples;
> 

LGTM

-Thilo
___
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 v17 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

[Sun, Jing A]

-Original Message-
From: Sun, Jing A 
Sent: Monday, August 19, 2019 1:55 PM
To: ffmpeg-devel@ffmpeg.org
Cc: Sun, Jing A ; Huang, Zhengxu 
; Tmar, Hassene ; Jun Zhao 

Subject: [PATCH v17 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

Fixed memleak.

Regards,
Sun, Jing




___
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 v17 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

[Jing Sun]

Signed-off-by: Zhengxu Huang 
Signed-off-by: Hassene Tmar 
Signed-off-by: Jun Zhao 
Signed-off-by: Jing Sun 
---
 configure|   4 +
 libavcodec/Makefile  |   1 +
 libavcodec/allcodecs.c   |   1 +
 libavcodec/libsvt_hevc.c | 497 +++
 libavcodec/version.h |   2 +-
 5 files changed, 504 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/libsvt_hevc.c

diff --git a/configure b/configure
index c09c842..648ea80 100755
--- a/configure
+++ b/configure
@@ -264,6 +264,7 @@ External library support:
   --enable-libspeexenable Speex de/encoding via libspeex [no]
   --enable-libsrt  enable Haivision SRT protocol via libsrt [no]
   --enable-libssh  enable SFTP protocol via libssh [no]
+  --enable-libsvthevc  enable HEVC encoding via svt [no]
   --enable-libtensorflow   enable TensorFlow as a DNN module backend
for DNN based filters like sr [no]
   --enable-libtesseractenable Tesseract, needed for ocr filter [no]
@@ -1793,6 +1794,7 @@ EXTERNAL_LIBRARY_LIST="
 libspeex
 libsrt
 libssh
+libsvthevc
 libtensorflow
 libtesseract
 libtheora
@@ -3194,6 +3196,7 @@ libshine_encoder_select="audio_frame_queue"
 libspeex_decoder_deps="libspeex"
 libspeex_encoder_deps="libspeex"
 libspeex_encoder_select="audio_frame_queue"
+libsvt_hevc_encoder_deps="libsvthevc"
 libtheora_encoder_deps="libtheora"
 libtwolame_encoder_deps="libtwolame"
 libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
@@ -6269,6 +6272,7 @@ enabled libsoxr   && require libsoxr soxr.h 
soxr_create -lsoxr
 enabled libssh&& require_pkg_config libssh libssh libssh/sftp.h 
sftp_init
 enabled libspeex  && require_pkg_config libspeex speex speex/speex.h 
speex_decoder_init
 enabled libsrt&& require_pkg_config libsrt "srt >= 1.3.0" 
srt/srt.h srt_socket
+enabled libsvthevc&& require_pkg_config libsvthevc SvtHevcEnc EbApi.h 
EbInitHandle
 enabled libtensorflow && require libtensorflow tensorflow/c/c_api.h 
TF_Version -ltensorflow
 enabled libtesseract  && require_pkg_config libtesseract tesseract 
tesseract/capi.h TessBaseAPICreate
 enabled libtheora && require libtheora theora/theoraenc.h th_info_init 
-ltheoraenc -ltheoradec -logg
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3cd73fb..d39f568 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -991,6 +991,7 @@ OBJS-$(CONFIG_LIBOPUS_ENCODER)+= libopusenc.o 
libopus.o \
 OBJS-$(CONFIG_LIBSHINE_ENCODER)   += libshine.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)   += libspeexdec.o
 OBJS-$(CONFIG_LIBSPEEX_ENCODER)   += libspeexenc.o
+OBJS-$(CONFIG_LIBSVT_HEVC_ENCODER)+= libsvt_hevc.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)  += libtheoraenc.o
 OBJS-$(CONFIG_LIBTWOLAME_ENCODER) += libtwolame.o
 OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER) += libvo-amrwbenc.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index d2f9a39..d8788a7 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -707,6 +707,7 @@ extern AVCodec ff_librsvg_decoder;
 extern AVCodec ff_libshine_encoder;
 extern AVCodec ff_libspeex_encoder;
 extern AVCodec ff_libspeex_decoder;
+extern AVCodec ff_libsvt_hevc_encoder;
 extern AVCodec ff_libtheora_encoder;
 extern AVCodec ff_libtwolame_encoder;
 extern AVCodec ff_libvo_amrwbenc_encoder;
diff --git a/libavcodec/libsvt_hevc.c b/libavcodec/libsvt_hevc.c
new file mode 100644
index 000..24ab0ff
--- /dev/null
+++ b/libavcodec/libsvt_hevc.c
@@ -0,0 +1,497 @@
+/*
+* Scalable Video Technology for HEVC encoder library plugin
+*
+* Copyright (c) 2019 Intel Corporation
+*
+* This file is part of FFmpeg.
+*
+* FFmpeg is free software; you can redistribute it and/or
+* modify it under the terms of the GNU Lesser General Public
+* License as published by the Free Software Foundation; either
+* version 2.1 of the License, or (at your option) any later version.
+*
+* FFmpeg is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+*/
+
+#include "EbApi.h"
+
+#include "libavutil/common.h"
+#include "libavutil/frame.h"
+#include "libavutil/opt.h"
+
+#include "internal.h"
+#include "avcodec.h"
+
+typedef enum eos_status {
+EOS_NOT_REACHED = 0,
+EOS_SENT,
+EOS_RECEIVED
+}EOS_STATUS;
+
+typedef struct SvtContext {
+AVClass *class;
+
+EB_H265_ENC_CONFIGURATION enc_params;
+EB_COMPONENTTYPE *svt_handle;
+EB_BUFFERHEADERTYPE in_buf;
+uint8_t *in_data;
+EOS_STATUS eos_flag;
+
+// User options.
+ 

[FFmpeg-devel] [PATCH v17 2/2] doc: Add libsvt_hevc encoder docs

[Jing Sun]

Add docs for libsvt_hevc encoder in encoders.texi and general.texi

Signed-off-by: Jun Zhao 
Signed-off-by: Zhengxu Huang 
Signed-off-by: Hassene Tmar 
Signed-off-by: Jing Sun 
---
 doc/encoders.texi | 149 ++
 doc/general.texi  |   8 +++
 2 files changed, 157 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index eefd124..aa5bcda 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1640,6 +1640,155 @@ Set maximum NAL size in bytes.
 Allow skipping frames to hit the target bitrate if set to 1.
 @end table
 
+@section libsvt_hevc
+
+Scalable Video Technology for HEVC (SVT-HEVC) encoder wrapper.
+
+This encoder requires the presence of the headers and
+library during configuration. You need to explicitly configure the
+build with @code{--enable-libsvthevc}. The library is detected using
+@command{pkg-config}.
+
+For more information about the library see
+@url{https://github.com/intel/SVT-HEVC.git}.
+
+@subsection Options
+
+The following FFmpeg global options affect the configurations of the
+libsvt_hevc encoder:
+
+@table @option
+@item b  (@emph{bitrate})
+Set the bitrate (as a number of bits per second). Default is 7M.
+
+@item g  / @option{gop_size}
+Set the GOP size. Default is -2 (unspecified).
+
+@item flags +cgop
+Enable closed GOP.
+
+@item qmin (@emph{min-q})
+Default is 10
+
+@item qmax (@emph{max-q})
+Default is 48
+
+Set minimum/maximum quantisation values.  Valid range is from 0 to 51
+(Only used when bit rate control mode @option{rc} is set to 1(vbr) mode.
+It is required that qmax >= qmin).
+
+@item profile (@emph{profile})
+Set profile restrictions. Can assume one of the following possible values:
+
+@table @samp
+@item main
+main profile
+@item main10
+main10 profile
+@item rext
+rext profile
+@end table
+
+Default is 1 (main).
+
+@item level (@emph{level})
+
+@option{level} sets the value of @emph{level}.
+Set level (level_idc). Default is 0 (to be determined by the encoder).
+
+@end table
+
+The encoder also has its own specific options:
+
+@table @option
+@item aud (@emph{aud})
+Enable use of access unit delimiters when set to 1. Default is 0 (Off).
+
+@item hielevel
+Set hierarchical levels. Can assume one of the following possible values:
+
+@table @samp
+@item flat
+flat more
+@item 1 level
+Minigop size is 2^1
+@item 2 level
+Minigop size is 2^2
+@item 3 level
+Minigop size is 2^3
+@end table
+
+Default is 3 level.
+
+@item la_depth
+Set look-ahead depth, depending on @option{rc}: for @var{vbr}, it's recommended
+to unset it and use the default value (the intra period); for @var{cqp}, better
+specify the look-ahead depth.
+
+The range is @var{-1-256}. Default is -1 (unset and the default value to be 
used).
+
+@item preset
+Set the quality vs density tradeoff point at which the encoding is to be 
performed.
+Higher perset value, higher density and lower quality.
+
+The range is @var{0-12}. Default is 9.
+
+@item tier
+Set @emph{general_tier_flag}.  This may affect the level chosen for the stream
+if it is not explicitly specified. Can assume one of the following possible 
values:
+
+@table @samp
+@item main
+main tier
+@item high
+high tier
+@end table
+
+Default is 1 (main).
+
+@item rc
+Set bit rate control mode. Can assume one of the following possible values:
+
+@table @samp
+@item cqp
+Constant QP (CQP) mode
+@item vbr
+Variable Bit Rate (VBR) mode
+@end table
+
+Default is 0 (cqp).
+
+@item forced_idr
+Force keyframes to be IDR if set to >=0 (the value sets headers insertion 
interval). Default is -1 (CRA).
+
+@item asm_type
+Auto select highest supported asm if set to 1 or C only if 0. Default is 1.
+
+@item qp
+Initial quantization parameter for the intra pictures used when
+@option{rc} is cqp mode. The range is from @var{0-51}. Default is 32.
+
+@item sc_detection
+Enables or disables the scene change detection algorithm. Default is 0 
(disabled).
+
+@item tune
+Set quality tuning mode. Can assume one of the following possible values:
+
+@table @samp
+@item sq
+Visually optimized mode
+@item oq
+PSNR / SSIM optimized mode
+@item vmaf
+VMAF optimized mode
+@end table
+
+Default is 1 (oq).
+
+@item bl_mode
+Enables or disables Random Access Prediction. Default is 0 (disabled).
+@end table
+
 @section libtheora
 
 libtheora Theora encoder wrapper.
diff --git a/doc/general.texi b/doc/general.texi
index 3c0c803..fa9cd31 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -243,6 +243,14 @@ FFmpeg can use the OpenJPEG libraries for 
decoding/encoding J2K videos.  Go to
 instructions.  To enable using OpenJPEG in FFmpeg, pass 
@code{--enable-libopenjpeg} to
 @file{./configure}.
 
+@section Scalable Video Technology for HEVC
+
+FFmpeg can make use of the SVT-HEVC library for HEVC encoding.
+
+Go to @url{https://github.com/intel/SVT-HEVC.git} and follow the instructions
+for installing the library. Pass @code{--enable-libsvthevc} to configure to
+enable it.
+
 @section TwoLAME
 
 FFmpeg can make use of the 

Re: [FFmpeg-devel] [PATCH] hevc_mp4toannexb: Do not duplicate parameter sets

[Andriy Gelman]

Andreas, 

On Tue, 13. Aug 06:24, Andreas Rheinhardt wrote:
> Andriy Gelman:
> > Andreas,
> > 
> > On Sun, 21. Jul 10:47, Andreas Rheinhardt wrote:
> >> Andriy Gelman:
> >>> From: Andriy Gelman 
> >>>
> >>> Fixes #7799
> >>>
> >>> Currently, the mp4toannexb filter always inserts extradata at the start
> >>> of each IRAP unit. This can lead to duplication of parameter sets if the
> >>> demuxed packet from mdat atom already contains a version of the
> >>> parameters.
> >>>
> >>> As in ticket #7799 this can also lead to decoding errors when the
> >>> parameter sets of the IRAP frames are different from the ones stored in
> >>> extradata.
> >>>
> >>> This commit avoids duplicating the parameter sets if they are already
> >>> present in the demuxed packet.
> >>>
> >>> This commit also makes an update to the hevc-bsf-mp4toannexb fate
> >>> test since the result before this patch contained duplicate vps/sps/pps
> >>> nal units.
> >>> ---
> >>>  libavcodec/hevc_mp4toannexb_bsf.c | 9 -
> >>>  tests/fate/hevc.mak   | 2 +-
> >>>  2 files changed, 9 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libavcodec/hevc_mp4toannexb_bsf.c 
> >>> b/libavcodec/hevc_mp4toannexb_bsf.c
> >>> index 09bce5b34c..5c27306b09 100644
> >>> --- a/libavcodec/hevc_mp4toannexb_bsf.c
> >>> +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> >>> @@ -123,6 +123,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, 
> >>> AVPacket *out)
> >>>  
> >>>  int got_irap = 0;
> >>>  int i, ret = 0;
> >>> +int vps_detected, sps_detected, pps_detected = 0;
> >>>  
> >> You are only initiallizing pps_detected.
> >>
> >>>  ret = ff_bsf_get_packet(ctx, );
> >>>  if (ret < 0)
> >>> @@ -146,9 +147,15 @@ static int hevc_mp4toannexb_filter(AVBSFContext 
> >>> *ctx, AVPacket *out)
> >>>  
> >>>  nalu_type = (bytestream2_peek_byte() >> 1) & 0x3f;
> >>>  
> >>> +switch (nalu_type) {
> >>> +  case HEVC_NAL_VPS: vps_detected = 1; break;
> >>> +  case HEVC_NAL_SPS: sps_detected = 1; break;
> >>> +  case HEVC_NAL_PPS: pps_detected = 1; break;
> >>> +}
> >>> +
> >>>  /* prepend extradata to IRAP frames */
> >>>  is_irap   = nalu_type >= 16 && nalu_type <= 23;
> >>> -add_extradata = is_irap && !got_irap;
> >>> +add_extradata = is_irap && !got_irap && !(vps_detected && 
> >>> sps_detected && pps_detected);
> >>>  extra_size= add_extradata * ctx->par_out->extradata_size;
> >>>  got_irap |= is_irap;
> >>>  
> >> There are two things that I don't like about this approach:
> >> 1. Image an input file like this: VPS, SPS and PPS in extradata and
> >> then after a few GOPs there is an inband PPS as part of a VPS, SPS and
> >> PPS combination where the PPS differs from the PPS in the extradata.
> >> After this change in parameter sets there are no further inband
> >> parameter sets. With your proposal, the extradata would again be
> >> inserted into access units with IRAP frames after the change in
> >> extradata and it would be the old, outdated extradata, thereby
> >> breaking decoding (the original mp4 file would probably not be able
> >> fully seekable in this case, but that would be another story).
> >> 2. If the sample in #7799 contained only the parameter set that
> >> actually changed inband, your proposal would still add a whole
> >> VPS+SPS+PPS combination and therefore still make the sample unplayable.
> >>
> > 
> > Thanks for your feedback. 
> > I believe I'm quite close to finishing the new patch. 
> > 
> > The general workflow in the updated mp4toannexb_filter function is: 
> >   1. Convert input avcc packet to annexb.
> >   2. Split the annexb bytestream into NAL units using 
> > ff_h2645_packet_splits function
> This function is quite slow (it checks the whole NAL unit for 0x03
> escapes and if it finds any, it unescapes the whole NAL unit, i.e. it
> copies the NAL unit with the 0x03 escapes removed). Do you limit
> unescaping to the relevant NAL units only?

Yes I agree, ff_h2645_packet_splits is not the best option.
We only need the escaped version for the SPS parameter set.

I hope you can review the current version. I'll work on
writing a custom ff_h2645_packet_splits after your feedback.

> >   3. If VPS/SPS/PPS nal units are present, parse them to get their id and 
> > reference to higher level parameter set.
> >   4. New VPS/SPS/PPS parameter sets are compared their 'cached version'. 
> > The cached versions are stored in a struct similar to HEVCParamSets. If the 
> > new parameter set is
> >   different, then update the cached version.
> >   5. Update extradata if any of the cached parameter sets have changed.
> If I am not mistaken, then you are not allowed to modify the extradata
> after init. All you could do is send side data of
> AV_PKT_DATA_NEW_EXTRADATA type. But it is probably enough to simply
> add the new extradata in-band.
> 
I have changed this. The original extradata is not modified. 

> >   6. 

Re: [FFmpeg-devel] [PATCH v1] lavf/hlsenc: fix one warning: unused variable 'filename' [-Wunused-variable]

[Liu Steven]

> 在 2019年8月19日，上午9:42，lance.lmw...@gmail.com 写道：
> 
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
> libavformat/hlsenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index f6f9c8161d..149b2a3bd0 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1571,7 +1571,7 @@ static int hls_start(AVFormatContext *s, VariantStream 
> *vs)
> AVDictionary *options = NULL;
> const char *proto = NULL;
> int use_temp_file = 0;
> -char *filename, iv_string[KEYSIZE*2 + 1];
> +char iv_string[KEYSIZE*2 + 1];
> int err = 0;
> 
> if (c->flags & HLS_SINGLE_FILE) {
> -- 
> 2.21.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".


LGTM


Thanks
Steven

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

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

[FFmpeg-devel] [PATCH v2] hevc_mp4toannexb: Insert correct parameter sets before IRAP

[Andriy Gelman]

From: Andriy Gelman 

Fixes #7799

Currently, the mp4toannexb filter always inserts the same extradata at
the start of the first IRAP unit. As in ticket #7799, this can lead to
decoding errors if modified parameter sets are signalled in-band.

This commit keeps track of the vps/sps/pps parameter sets during the
conversion. The correct combination is inserted at the start of the
first IRAP unit instead of the original extradata. SEI prefix nal units
are also cached and inserted after the parameter sets.

This commit also makes an update to the hevc-bsf-mp4toannexb fate
test since the result before this patch contained duplicate vps/sps/pps
parameter sets in-band.
---
 libavcodec/hevc_mp4toannexb_bsf.c | 475 +++---
 tests/fate/hevc.mak   |   2 +-
 2 files changed, 437 insertions(+), 40 deletions(-)

diff --git a/libavcodec/hevc_mp4toannexb_bsf.c 
b/libavcodec/hevc_mp4toannexb_bsf.c
index 09bce5b34c..7f3d68252d 100644
--- a/libavcodec/hevc_mp4toannexb_bsf.c
+++ b/libavcodec/hevc_mp4toannexb_bsf.c
@@ -28,19 +28,212 @@
 #include "bsf.h"
 #include "bytestream.h"
 #include "hevc.h"
-
-#define MIN_HEVCC_LENGTH 23
+#include "h2645_parse.h"
+#include "hevc_ps.h"
+#include "golomb.h"
+
+#define MIN_HEVCC_LENGTH23
+#define PROFILE_WITHOUT_IDC_BITS88
+#define IS_IRAP(s)  ((s)->type >= 16 && (s)->type <= 23)
+#define IS_PARAMSET(s)  ((s)->type >= 32 && (s)->type <= 34)
+
+#define WRITE_NAL(pkt, prev_size, in_buffer) do {\
+AV_WB32((pkt)->data + (prev_size), 1);   \
+prev_size += 4;  \
+memcpy((pkt)->data + (prev_size), (in_buffer)->data, (in_buffer)->size); \
+prev_size += (in_buffer)->size;  \
+} while (0)
+
+typedef struct Param {
+AVBufferRef *raw_data; /*store a copy of the raw data to construct 
extradata*/
+int ref;  /*stores the ref of the higher level parameter set*/
+} Param;
+
+/*modified version of HEVCParamSets to store bytestream and reference to 
previous level*/
+typedef struct ParamSets {
+Param vps_list[HEVC_MAX_VPS_COUNT];
+Param sps_list[HEVC_MAX_SPS_COUNT];
+Param pps_list[HEVC_MAX_PPS_COUNT];
+
+AVBufferRef *sei_prefix;
+} ParamSets;
 
 typedef struct HEVCBSFContext {
-uint8_t  length_size;
-int  extradata_parsed;
+uint8_t  length_size;
+int  extradata_parsed;
+ParamSetsps; /*make own of version of HEVCParamSets store copy of the 
bytestream*/
 } HEVCBSFContext;
 
+
+static int parse_vps(AVBSFContext* ctx, H2645NAL *nal)
+{
+int vps_id = 0;
+Param *param_ptr;
+
+HEVCBSFContext *s = ctx->priv_data;
+ParamSets *ps = >ps;
+
+GetBitContext *gb = >gb;
+int nal_size  = nal->raw_size;
+
+vps_id = get_bits(gb, 4);
+if (vps_id >= HEVC_MAX_VPS_COUNT) {
+av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_id);
+return AVERROR_INVALIDDATA;
+}
+
+param_ptr = >vps_list[vps_id];
+/*init raw_data if needed*/
+if (!param_ptr->raw_data) {
+param_ptr->raw_data = av_buffer_allocz(nal_size);
+param_ptr->ref  = 0;
+if (!param_ptr->raw_data)
+return AVERROR(ENOMEM);
+}
+
+if (param_ptr->raw_data && param_ptr->raw_data->size == nal_size &&
+!memcmp(param_ptr->raw_data->data, nal->raw_data, nal_size)) {
+av_log(ctx, AV_LOG_DEBUG, "Parsed VPS id: %d. Copy already exists in 
parameter set\n", vps_id);
+} else {
+AVBufferRef *vps_buf = av_buffer_allocz(nal_size);
+if (!vps_buf)
+return AVERROR(ENOMEM);
+
+/*copy raw data into vps_buf buffer and replace existing copy in ps*/
+memcpy(vps_buf->data, nal->raw_data, nal_size);
+av_buffer_unref(_ptr->raw_data);
+param_ptr->raw_data = vps_buf;
+}
+return 0;
+}
+
+static int parse_sps(AVBSFContext *ctx, H2645NAL *nal)
+{
+int sps_id, vps_ref, max_sub_layers_minus1;
+int i;
+Param *param_ptr;
+
+HEVCBSFContext *s = (HEVCBSFContext*)ctx->priv_data;
+ParamSets *ps = >ps;
+
+GetBitContext *gb = >gb;
+int nal_size  = nal->raw_size;
+
+uint8_t sub_layer_profile_present_flag[HEVC_MAX_SUB_LAYERS];
+uint8_t sub_layer_level_present_flag[HEVC_MAX_SUB_LAYERS];
+
+vps_ref = get_bits(gb, 4);
+if (vps_ref >= HEVC_MAX_VPS_COUNT) {
+av_log(ctx, AV_LOG_ERROR, "VPS id out of range: %d\n", vps_ref);
+return AVERROR_INVALIDDATA;
+}
+
+max_sub_layers_minus1 = get_bits(gb, 3);
+skip_bits1(gb);   /*sps_temporal_id_nesting_flag*/
+skip_bits(gb, PROFILE_WITHOUT_IDC_BITS); /*profile_tier_level*/
+skip_bits(gb, 8); /*general_level_idc*/
+
+for (i = 0; i < max_sub_layers_minus1; ++i) {
+sub_layer_profile_present_flag[i] = get_bits1(gb);
+sub_layer_level_present_flag[i]   = 

Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: grow packet if vaMapBuffer returns multiple buffers

[Fu, Linjie]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Tuesday, August 6, 2019 16:12
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>; Mark Thompson 
> Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: grow packet if
> vaMapBuffer returns multiple buffers
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Fu, Linjie
> > Sent: Tuesday, June 4, 2019 17:11
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Mark Thompson 
> > Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: grow packet
> if
> > vaMapBuffer returns multiple buffers
> >
> > > -Original Message-
> > > From: Fu, Linjie
> > > Sent: Friday, May 31, 2019 08:35
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Fu, Linjie 
> > > Subject: [PATCH,v2] lavc/vaapi_encode: grow packet if vaMapBuffer
> > returns
> > > multiple buffers
> > >
> > > It seems that VA_CODED_BUF_STATUS_SINGLE_NALU allows driver to
> > map
> > > buffer for each slice.
> > >
> > > Currently, assigning new buffer for pkt when multiple buffer returns
> > > from vaMapBuffer will cover the previous encoded pkt data and lead
> > > to encode issues.
> > >
> > > Iterate through the buf_list first to find out the total buffer size
> > > needed for the pkt, allocate the whole pkt to avoid repeated reallocation
> > > and memcpy, then copy data from each buf to pkt.
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > > [v2]: allocate the whole pkt to avoid repeated reallocation
> > > and memcpy
> > >
> > >  libavcodec/vaapi_encode.c | 18 +-
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > > index 2dda451..9c9e5dd 100644
> > > --- a/libavcodec/vaapi_encode.c
> > > +++ b/libavcodec/vaapi_encode.c
> > > @@ -489,6 +489,8 @@ static int vaapi_encode_output(AVCodecContext
> > > *avctx,
> > >  VAAPIEncodeContext *ctx = avctx->priv_data;
> > >  VACodedBufferSegment *buf_list, *buf;
> > >  VAStatus vas;
> > > +int total_size = 0;
> > > +uint8_t *ptr;
> > >  int err;
> > >
> > >  err = vaapi_encode_wait(avctx, pic);
> > > @@ -505,15 +507,21 @@ static int
> vaapi_encode_output(AVCodecContext
> > > *avctx,
> > >  goto fail;
> > >  }
> > >
> > > +for (buf = buf_list; buf; buf = buf->next)
> > > +total_size += buf->size;
> > > +
> > > +err = av_new_packet(pkt, total_size);
> > > +ptr = pkt->data;
> > > +
> > > +if (err < 0)
> > > +goto fail_mapped;
> > > +
> > >  for (buf = buf_list; buf; buf = buf->next) {
> > >  av_log(avctx, AV_LOG_DEBUG, "Output buffer: %u bytes "
> > > "(status %08x).\n", buf->size, buf->status);
> > >
> > > -err = av_new_packet(pkt, buf->size);
> > > -if (err < 0)
> > > -goto fail_mapped;
> > > -
> > > -memcpy(pkt->data, buf->buf, buf->size);
> > > +memcpy(ptr, buf->buf, buf->size);
> > > +ptr += buf->size;
> > >  }
> > >
> > >  if (pic->type == PICTURE_TYPE_IDR)
> > > --
> > > 2.7.4
> >
> > Ping?
> > This can fix the encoding issue for vaapi_vp8:
> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -f
> > rawvideo -pix_fmt yuv420p -s:v 1280x720 -i ./input_I420.yuv -vf
> > 'format=nv12,hwupload' -c:v vp8_vaapi -rc_mode CQP -g 1 -global_quality
> 14
> > -loop_filter_sharpness 0 -loop_filter_level 0 -vframes 10 -y out.ivf
> 
> Media driver will return 2 buffers in fact. The first buffer is
> VACodedBufferSegment
> buffer which includes encoded output. And the second buffer is extra
> VACodedBufferVP9Status.
> https://github.com/intel/media-driver/issues/624
> 
> Fixes vp8 encoding issues for core platforms(Kaby Lake) and could be verified.
> Ping?
> 
Another ping?
Had passed the regression patch check in 
https://github.com/intel-media-ci/ffmpeg/pull/44

- linjie
___
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 v1] lavf/hlsenc: fix one warning: unused variable 'filename' [-Wunused-variable]

[lance . lmwang]

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavformat/hlsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index f6f9c8161d..149b2a3bd0 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1571,7 +1571,7 @@ static int hls_start(AVFormatContext *s, VariantStream 
*vs)
 AVDictionary *options = NULL;
 const char *proto = NULL;
 int use_temp_file = 0;
-char *filename, iv_string[KEYSIZE*2 + 1];
+char iv_string[KEYSIZE*2 + 1];
 int err = 0;
 
 if (c->flags & HLS_SINGLE_FILE) {
-- 
2.21.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 v1] avcodec/h264_parse: retry decoding SPS with complete NAL

[Jun Li]

Fix #6591
The content has no rbsp_stop_one_bit for ending the SPS, that
causes the decoding SPS failure, results decoding frame failure as well.
The patch is just adding a retry with complete NALU.
---
 libavcodec/h264_parse.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index ac31f54e07..a2267a0610 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -376,8 +376,14 @@ static int decode_extradata_ps(const uint8_t *data, int 
size, H264ParamSets *ps,
 switch (nal->type) {
 case H264_NAL_SPS:
 ret = ff_h264_decode_seq_parameter_set(>gb, logctx, ps, 0);
-if (ret < 0)
-goto fail;
+if (ret < 0) {
+GetBitContext tmp_gb = nal->gb;
+av_log(logctx, AV_LOG_DEBUG,
+   "SPS decoding failure (maybe missing rbsp_stop_one_bit), 
trying again with the complete NAL\n");
+init_get_bits8(_gb, nal->raw_data + 1, nal->raw_size - 1);
+if ((ret = ff_h264_decode_seq_parameter_set(_gb, logctx, 
ps, 0)) < 0)
+goto fail;
+}
 break;
 case H264_NAL_PPS:
 ret = ff_h264_decode_picture_parameter_set(>gb, logctx, ps,
-- 
2.17.1

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

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

Re: [FFmpeg-devel] [PATCH] lavf/vf_deinterlace_vaapi: flush queued frame in field mode

[Fu, Linjie]

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Aman Gupta
> Sent: Wednesday, August 14, 2019 10:05
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavf/vf_deinterlace_vaapi: flush
> queued frame in field mode
> 
> On Tue, Aug 13, 2019 at 7:01 PM Fu, Linjie  wrote:
> 
> > > -Original Message-
> > > From: Fu, Linjie
> > > Sent: Friday, August 2, 2019 17:54
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Fu, Linjie 
> > > Subject: [PATCH] lavf/vf_deinterlace_vaapi: flush queued frame in field
> > > mode
> > >
> > > Add deint_vaapi_request_frame for deinterlace_vaapi, send NULL frame
> > > to flush the queued frame.
> > >
> > > Fix the frame drop issue in field mode:
> > >
> > > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -v verbose -
> c:v
> > > h264 -i ./input.h264 -vf 'format=nv12|vaapi,hwupload,
> > > deinterlace_vaapi=mode=bob:rate=field,hwdownload,format=nv12'
> > > -pix_fmt yuv420p -f rawvideo -vsync passthrough -y dump.yuv
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > >  libavfilter/vf_deinterlace_vaapi.c | 46
> > > --
> > >  1 file changed, 39 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/libavfilter/vf_deinterlace_vaapi.c
> > > b/libavfilter/vf_deinterlace_vaapi.c
> > > index 72d0349..c811327 100644
> > > --- a/libavfilter/vf_deinterlace_vaapi.c
> > > +++ b/libavfilter/vf_deinterlace_vaapi.c
> > > @@ -48,6 +48,9 @@ typedef struct DeintVAAPIContext {
> > >  intqueue_count;
> > >  AVFrame   *frame_queue[MAX_REFERENCES];
> > >  intextra_delay_for_timestamps;
> > > +
> > > +inteof;
> > > +intprev_pts;
> > >  } DeintVAAPIContext;
> > >
> > >  static const char *deint_vaapi_mode_name(int mode)
> > > @@ -190,11 +193,16 @@ static int deint_vaapi_filter_frame(AVFilterLink
> > > *inlink, AVFrame *input_frame)
> > >  void *filter_params_addr = NULL;
> > >  int err, i, field, current_frame_index;
> > >
> > > -av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u
> > (%"PRId64").\n",
> > > -   av_get_pix_fmt_name(input_frame->format),
> > > -   input_frame->width, input_frame->height, input_frame->pts);
> > > +// NULL frame is used to flush the queue in field mode
> > > +if (input_frame)
> > > +av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u
> > > (%"PRId64").\n",
> > > +av_get_pix_fmt_name(input_frame->format),
> > > +input_frame->width, input_frame->height, input_frame->pts);
> > > +else if (ctx->field_rate == 1) {
> > > +return 0;
> > > +}
> > >
> > > -if (ctx->queue_count < ctx->queue_depth) {
> > > +if (input_frame && ctx->queue_count < ctx->queue_depth) {
> > >  ctx->frame_queue[ctx->queue_count++] = input_frame;
> > >  if (ctx->queue_count < ctx->queue_depth) {
> > >  // Need more reference surfaces before we can continue.
> > > @@ -291,6 +299,8 @@ static int deint_vaapi_filter_frame(AVFilterLink
> > *inlink,
> > > AVFrame *input_frame)
> > >  if (ctx->field_rate == 2) {
> > >  if (field == 0)
> > >  output_frame->pts = 2 * input_frame->pts;
> > > +else if (ctx->eof)
> > > +output_frame->pts = 3 * input_frame->pts -
> > ctx->prev_pts;
> > >  else
> > >  output_frame->pts = input_frame->pts +
> > >  ctx->frame_queue[current_frame_index + 1]->pts;
> > > @@ -306,6 +316,8 @@ static int deint_vaapi_filter_frame(AVFilterLink
> > *inlink,
> > > AVFrame *input_frame)
> > >  break;
> > >  }
> > >
> > > +ctx->prev_pts = input_frame->pts;
> > > +
> > >  return err;
> > >
> > >  fail:
> > > @@ -315,6 +327,25 @@ fail:
> > >  return err;
> > >  }
> > >
> > > +static int deint_vaapi_request_frame(AVFilterLink *link)
> > > +{
> > > +AVFilterContext *avctx = link->src;
> > > +DeintVAAPIContext *ctx   = avctx->priv;
> > > +int ret;
> > > +
> > > +if (ctx->eof)
> > > +return AVERROR_EOF;
> > > +
> > > +ret = ff_request_frame(link->src->inputs[0]);
> > > +if (ret == AVERROR_EOF) {
> > > +ctx->eof = 1;
> > > +deint_vaapi_filter_frame(link->src->inputs[0], NULL);
> > > +} else if (ret < 0)
> > > +return ret;
> > > +
> > > +return 0;
> > > +}
> > > +
> > >  static av_cold int deint_vaapi_init(AVFilterContext *avctx)
> > >  {
> > >  VAAPIVPPContext *vpp_ctx = avctx->priv;
> > > @@ -376,9 +407,10 @@ static const AVFilterPad deint_vaapi_inputs[] = {
> > >
> > >  static const AVFilterPad deint_vaapi_outputs[] = {
> > >  {
> > > -.name = "default",
> > > -.type = AVMEDIA_TYPE_VIDEO,
> > > -.config_props = _vaapi_config_output,
> > > +.name   = "default",
> > > +.type   

Re: [FFmpeg-devel] [PATCH v4] avfilter/vaapi: add overlay_vaapi filter

[Zachary Zhou]

Hi Mark,

Do you have any comments on this patch ?

Thanks,

Zachary


On 7/11/19 5:29 PM, Zachary Zhou wrote:

---
  configure  |   1 +
  libavfilter/Makefile   |   1 +
  libavfilter/allfilters.c   |   1 +
  libavfilter/vf_overlay_vaapi.c | 424 +
  4 files changed, 427 insertions(+)
  create mode 100644 libavfilter/vf_overlay_vaapi.c

diff --git a/configure b/configure
index 32fc26356c..f469e6a3b1 100755
--- a/configure
+++ b/configure
@@ -3478,6 +3478,7 @@ openclsrc_filter_deps="opencl"
  overlay_opencl_filter_deps="opencl"
  overlay_qsv_filter_deps="libmfx"
  overlay_qsv_filter_select="qsvvpp"
+overlay_vaapi_filter_deps="vaapi"
  owdenoise_filter_deps="gpl"
  pan_filter_deps="swresample"
  perspective_filter_deps="gpl"
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 07ea8d7edc..ccaad0d6a4 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -311,6 +311,7 @@ OBJS-$(CONFIG_OVERLAY_FILTER)+= 
vf_overlay.o framesync.o
  OBJS-$(CONFIG_OVERLAY_OPENCL_FILTER) += vf_overlay_opencl.o opencl.o \
  opencl/overlay.o framesync.o
  OBJS-$(CONFIG_OVERLAY_QSV_FILTER)+= vf_overlay_qsv.o framesync.o
+OBJS-$(CONFIG_OVERLAY_VAAPI_FILTER)  += vf_overlay_vaapi.o framesync.o 
vaapi_vpp.o
  OBJS-$(CONFIG_OWDENOISE_FILTER)  += vf_owdenoise.o
  OBJS-$(CONFIG_PAD_FILTER)+= vf_pad.o
  OBJS-$(CONFIG_PALETTEGEN_FILTER) += vf_palettegen.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 9c846b1ddd..27ee1df78b 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -295,6 +295,7 @@ extern AVFilter ff_vf_oscilloscope;
  extern AVFilter ff_vf_overlay;
  extern AVFilter ff_vf_overlay_opencl;
  extern AVFilter ff_vf_overlay_qsv;
+extern AVFilter ff_vf_overlay_vaapi;
  extern AVFilter ff_vf_owdenoise;
  extern AVFilter ff_vf_pad;
  extern AVFilter ff_vf_palettegen;
diff --git a/libavfilter/vf_overlay_vaapi.c b/libavfilter/vf_overlay_vaapi.c
new file mode 100644
index 00..9fffa0fcb9
--- /dev/null
+++ b/libavfilter/vf_overlay_vaapi.c
@@ -0,0 +1,424 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include 
+
+#include "libavutil/avassert.h"
+#include "libavutil/mem.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+
+#include "avfilter.h"
+#include "framesync.h"
+#include "formats.h"
+#include "internal.h"
+#include "vaapi_vpp.h"
+
+typedef struct OverlayVAAPIContext {
+VAAPIVPPContext  vpp_ctx; // must be the first field
+FFFrameSync  fs;
+int  overlay_x;
+int  overlay_y;
+int  overlay_w;
+int  overlay_h;
+floatoverlay_alpha;
+} OverlayVAAPIContext;
+
+static int overlay_vaapi_query_formats(AVFilterContext *ctx)
+{
+int i;
+int ret;
+
+static const enum AVPixelFormat main_in_fmts[] = {
+AV_PIX_FMT_VAAPI,
+AV_PIX_FMT_NONE
+};
+static const enum AVPixelFormat out_pix_fmts[] = {
+AV_PIX_FMT_VAAPI,
+AV_PIX_FMT_NONE
+};
+
+for (i = 0; i < ctx->nb_inputs; i++) {
+ret = ff_formats_ref(ff_make_format_list(main_in_fmts), 
>inputs[i]->out_formats);
+if (ret < 0)
+return ret;
+}
+
+ret = ff_formats_ref(ff_make_format_list(out_pix_fmts), 
>outputs[0]->in_formats);
+if (ret < 0)
+return ret;
+
+return 0;
+}
+
+static int overlay_vaapi_render(AVFilterContext *avctx,
+VAProcPipelineParameterBuffer *params,
+VAProcPipelineParameterBuffer *subpic_params,
+VASurfaceID output_surface)
+{
+VABufferID params_id;
+VABufferID subpic_params_id;
+VAStatus vas;
+int err = 0;
+VAAPIVPPContext *ctx   = avctx->priv;
+
+vas = vaBeginPicture(ctx->hwctx->display,
+ ctx->va_context, output_surface);
+if (vas != VA_STATUS_SUCCESS) {
+av_log(avctx, AV_LOG_ERROR, "Failed to attach new picture: "
+   "%d (%s).\n", vas, vaErrorStr(vas));
+err = AVERROR(EIO);
+goto fail;
+}
+
+vas = 

[FFmpeg-devel] [PATCH 2/3] avcodec/alsdec: Fix integer overflow in decode_var_block_data()

[Michael Niedermayer]

Fixes: signed integer overflow: 1927975249 - -514719744 cannot be represented 
in type 'int'
Fixes: 
16413/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5651206856245248

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

diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 425cf73be9..4794556aad 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -953,7 +953,7 @@ static int decode_var_block_data(ALSDecContext *ctx, 
ALSBlockData *bd)
 
 // reconstruct difference signal for prediction (joint-stereo)
 if (bd->js_blocks && bd->raw_other) {
-int32_t *left, *right;
+uint32_t *left, *right;
 
 if (bd->raw_other > raw_samples) {  // D = R - L
 left  = raw_samples;
-- 
2.22.1

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

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

[FFmpeg-devel] [PATCH 3/3] avcodec/atrac9dec: Check block_align

[Michael Niedermayer]

Fixes: Infinite loop
Fixes: 
16260/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5676365617037312

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

diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
index 4ea6ff0f31..46e60ca998 100644
--- a/libavcodec/atrac9dec.c
+++ b/libavcodec/atrac9dec.c
@@ -839,6 +839,11 @@ static av_cold int atrac9_decode_init(AVCodecContext 
*avctx)
 
 av_lfg_init(>lfg, 0xFBADF00D);
 
+if (avctx->block_align <= 0) {
+av_log(avctx, AV_LOG_ERROR, "Invalid block align\n");
+return AVERROR_INVALIDDATA;
+}
+
 if (avctx->extradata_size != 12) {
 av_log(avctx, AV_LOG_ERROR, "Invalid extradata length!\n");
 return AVERROR_INVALIDDATA;
-- 
2.22.1

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

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

[FFmpeg-devel] [PATCH 1/3] avcodec/alsdec: Limit maximum channels to 64

[Michael Niedermayer]

There seems to be no limit in the specification and upto 64k could be stored
64 is chooses as limit as thats also used for AAC and is what a int64 mask
can handle

An alternative to this patch would be a max_channels variable

Fixes: OOM
Fixes: 
16200/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5764788793114624

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

diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 26c496c769..425cf73be9 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -43,6 +43,8 @@
 
 #include 
 
+#define MAX_CHANNELS 64
+
 /** Rice parameters and corresponding index offsets for decoding the
  *  indices of scaled PARCOR values. The table chosen is set globally
  *  by the encoder and stored in ALSSpecificConfig.
@@ -348,6 +350,11 @@ static av_cold int read_specific_config(ALSDecContext *ctx)
 if (als_id != MKBETAG('A','L','S','\0'))
 return AVERROR_INVALIDDATA;
 
+if (avctx->channels > MAX_CHANNELS) {
+avpriv_request_sample(avctx, "Huge number of channels\n");
+return AVERROR_PATCHWELCOME;
+}
+
 ctx->cur_frame_length = sconf->frame_length;
 
 // read channel config
-- 
2.22.1

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

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

Re: [FFmpeg-devel] [PATCH 2/3] avutil/imgutils: remove dead assignment

[Michael Niedermayer]

On Sat, Aug 17, 2019 at 09:41:04PM +0200, Marton Balint wrote:
> Signed-off-by: Marton Balint 
> ---
>  libavutil/imgutils.c | 1 -
>  1 file changed, 1 deletion(-)

LGTM

thx

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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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

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

Re: [FFmpeg-devel] [CALL] New FFmpeg developers meeting

[Thomas Volkert]

On 18.08.19 18:25, Tomas Härdin wrote:

lör 2019-08-17 klockan 11:48 +0200 skrev Paul B Mahol:

Hi,

When and how to organize this?

Physical meeting or over the internets? IBC is coming up, that might be
a good opportunity. Depending on form, attending remotely via mumble or
jingle might also be a good idea.


I would be interested in an informal physical meeting at the IBC.

Best regards,
Thomas.
___
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/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 02:49:39PM +0200, Tomas Härdin wrote:
> sön 2019-08-18 klockan 14:18 +0200 skrev Michael Niedermayer:
> > On Sun, Aug 18, 2019 at 01:40:01PM +0200, Tomas Härdin wrote:
> > > sön 2019-08-18 klockan 12:19 +0200 skrev Michael Niedermayer:
> > > > On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > > > > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
> > > > > 
> > > > > > and yes i too wish there was a magic fix but i think most things 
> > > > > > that
> > > > > > look like magic fixes have a fatal flaw. But maybe iam missing 
> > > > > > something
> > > > > > in fact i hope that iam missing something and that there is a magic 
> > > > > > fix
> > > > > > 
> > > > > 
> > > > > Magic fix is enabling reference counted frames in fuzzer.
> > > > 
> > > > That is covered by the part below which you maybe did not read
> > > > 
> > > > > > PS: if you think of changing the API, i dont think its the API.
> > > > > > I mean every user application will read the frames it receives, so
> > > > > > even if inside the decoder we just return the same frame with 2 
> > > > > > pixels
> > > > > > different the user doesnt know this and has to read the whole frame.
> > > > > > The problem is moved around but its still there.
> > > 
> > > Copying is still slower than not copying. Enabling refcounting fixes
> > > the timeout issue here, and will likely silence a whole bunch of false
> > > positives for this class of files.
> > 
> > it makes probably sense to enable ref counting but we should
> > immedeatly in the next or a previous commit make the fuzzer read the frames
> > from the  decoder. Thats what basically every user app would do.
> > Otherwise we would have a bunch of issues closed and then reopened
> > later.
> 
> Why should we care how much work the user does? We're fuzzing lavc
> here, not user programs. Certain use cases are decode-only (ffprobe
> -show_frames for example)

thats a valid point of view

The user though has few options if she gets many frames, she can just
continue processing or stop, she doesnt know how (in)valid the stream is

OTOH libavcodec knows the codec bitstream, and can check various things

so just moving the responsibility to the user app would move it where
its much harder to fix well
That is currently, we could export all kinds of metadata about the
amount of errors. Then a user app could decide what to do.
It would become duplicated code between user apps though...


> 
> > an alternative viewpoint to this would be to set the refcounting flag
> > from the input so the fuzzer itself has control over it and we test
> > both codepathes. This would improve coverage
> 
> Not a bad idea. But as this example shows, not refcounting has
> performance implications. The fuzzer should be more lenient with
> timeout in that case, imo.

this is possible, yes

Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: do not ignore bad size errors

[Tomas Härdin]

lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint:
> The return value was unintentionally lost after
> 00a2652df3bf25a27d174cc67ed508b5317cb115.
> 
> Signed-off-by: Marton Balint 
> ---
>  libavformat/mxfdec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index bb72fb9841..397f820b3f 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket 
> *pkt)
>  } else {
>  if ((size = next_ofs - pos) <= 0) {
>  av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", 
> size);
> -ret = AVERROR_INVALIDDATA;
> -goto skip;
> +mxf->current_klv_data = (KLVPacket){{0}};
> +return AVERROR_INVALIDDATA;

Should maybe ask for a sample. Else looks OK

/Tomas

___
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] [CALL] New FFmpeg developers meeting

[Tomas Härdin]

lör 2019-08-17 klockan 11:48 +0200 skrev Paul B Mahol:
> Hi,
> 
> When and how to organize this?

Physical meeting or over the internets? IBC is coming up, that might be
a good opportunity. Depending on form, attending remotely via mumble or
jingle might also be a good idea.

/Tomas

___
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 v1] lavf/dump: dump the vbv_delay with N/A instead of 18446744073709551615

[lance . lmwang]

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavformat/dump.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavformat/dump.c b/libavformat/dump.c
index a3791a3..56814ff 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -321,13 +321,16 @@ static void dump_cpb(void *ctx, AVPacketSideData *sd)
 
 av_log(ctx, AV_LOG_INFO,
 #if FF_API_UNSANITIZED_BITRATES
-   "bitrate max/min/avg: %d/%d/%d buffer size: %d vbv_delay: %"PRIu64,
+   "bitrate max/min/avg: %d/%d/%d buffer size: %d ",
 #else
-   "bitrate max/min/avg: %"PRId64"/%"PRId64"/%"PRId64" buffer size: %d 
vbv_delay: %"PRIu64,
+   "bitrate max/min/avg: %"PRId64"/%"PRId64"/%"PRId64" buffer size: %d 
",
 #endif
cpb->max_bitrate, cpb->min_bitrate, cpb->avg_bitrate,
-   cpb->buffer_size,
-   cpb->vbv_delay);
+   cpb->buffer_size);
+if (cpb->vbv_delay == UINT64_MAX)
+av_log(ctx, AV_LOG_INFO, "vbv_delay: N/A");
+else
+av_log(ctx, AV_LOG_INFO, "vbv_delay: %"PRIu64"", cpb->vbv_delay);
 }
 
 static void dump_mastering_display_metadata(void *ctx, AVPacketSideData* sd) {
-- 
2.6.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 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Tomas Härdin]

sön 2019-08-18 klockan 14:18 +0200 skrev Michael Niedermayer:
> On Sun, Aug 18, 2019 at 01:40:01PM +0200, Tomas Härdin wrote:
> > sön 2019-08-18 klockan 12:19 +0200 skrev Michael Niedermayer:
> > > On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > > > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
> > > > 
> > > > > and yes i too wish there was a magic fix but i think most things that
> > > > > look like magic fixes have a fatal flaw. But maybe iam missing 
> > > > > something
> > > > > in fact i hope that iam missing something and that there is a magic 
> > > > > fix
> > > > > 
> > > > 
> > > > Magic fix is enabling reference counted frames in fuzzer.
> > > 
> > > That is covered by the part below which you maybe did not read
> > > 
> > > > > PS: if you think of changing the API, i dont think its the API.
> > > > > I mean every user application will read the frames it receives, so
> > > > > even if inside the decoder we just return the same frame with 2 pixels
> > > > > different the user doesnt know this and has to read the whole frame.
> > > > > The problem is moved around but its still there.
> > 
> > Copying is still slower than not copying. Enabling refcounting fixes
> > the timeout issue here, and will likely silence a whole bunch of false
> > positives for this class of files.
> 
> it makes probably sense to enable ref counting but we should
> immedeatly in the next or a previous commit make the fuzzer read the frames
> from the  decoder. Thats what basically every user app would do.
> Otherwise we would have a bunch of issues closed and then reopened
> later.

Why should we care how much work the user does? We're fuzzing lavc
here, not user programs. Certain use cases are decode-only (ffprobe
-show_frames for example)

> an alternative viewpoint to this would be to set the refcounting flag
> from the input so the fuzzer itself has control over it and we test
> both codepathes. This would improve coverage

Not a bad idea. But as this example shows, not refcounting has
performance implications. The fuzzer should be more lenient with
timeout in that case, imo.

> > It would be beneficial to have a consistent way of signalling that a
> > frame didn't change, since a bunch of codecs have that property.
> > Currently it's a mix of discontinuous timestamps, longer frame
> > durations and repeating identical frames.
> 
> yes, i strongly agree.
>
> > > so feeding really crazy resolutions into a decoder requires some
> > > small but proportional amout of input bytes.
> > > double the width and the minimum input bytes double sort of.
> > 
> > Lavc is already very lenient with what it accepts. How do you detect
> > the difference between "this packet is too small to decode to an entire
> > frame" from "this packet is too small but we could still get a few MBs
> > out of it"?
> 
> In reality this is actually not hard because a frame that is smaller
> than the minimum valid size is generally for many codecs so small 
> it really wont contain anything usefull to decode.
> And we have discard_damaged_percentage where the user can tune it too.
> Patches using discard_damaged_percentage are sometimes objected too
> though so it is not consistently used.

I remain skeptical of this, since it makes the parsing more complicated
and slower, and increases mental load. There's a sliding scale of
strictness here, and if I try to implement even something basic as "the
length in the header should match the length of the actual data" then
FATE breaks. We can of course change FATE to accept that changed
strictness.

I get the feeling whenver we put in checks like this it's only a matter
of time before the fuzzers find out some other way to trip up the
decoders. This is why I advocate for maximum strictness unless there's
a good reason to not be strict.

/Tomas

___
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/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 01:40:01PM +0200, Tomas Härdin wrote:
> sön 2019-08-18 klockan 12:19 +0200 skrev Michael Niedermayer:
> > On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
> > > 
> > > wrote:
> > > 
> > > > On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> > > > > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > > > > 
> > > > > I did some investigation, it is indeed ff_reget_buffer(). It copies 
> > > > > the
> > > > > frame data for some reason. The fix is simple in this case: just call
> > > > > ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
> > > > > same frame.
> > > > > 
> > > > > > As I said on IRC, this class of problems will exist for every codec.
> > > > > > Cinepak is easy to decode, even at these resolutions. Just imagine 
> > > > > > what
> > > > > > will happens when someone feeds in a 65535x209 av1 stream..
> > > > > 
> > > > > And related to this, ff_reget_buffer() is used for a lot of these
> > > > > codecs which only overwrite pixels in the old frame. flicvideo, 
> > > > > gifdec,
> > > > > msrle, roqvideodec and others probably have the same flaw.
> > > > 
> > > > not calling any form of *get_buffer per frame breaks decoding into
> > > > user supplied buffers.
> > > > 
> > > > If you check the documentation of the get_buffer2 callback
> > > > 
> > > > " This callback is called at the beginning of each frame to get data
> > > > buffer(s) for it."
> > > > 
> > > > That would not be possible if its just called once in init
> 
> Sorry, I'm a bit rusty on lavc internals.
> 
> > > > and yes i too wish there was a magic fix but i think most things that
> > > > look like magic fixes have a fatal flaw. But maybe iam missing something
> > > > in fact i hope that iam missing something and that there is a magic fix
> > > > 
> > > 
> > > Magic fix is enabling reference counted frames in fuzzer.
> > 
> > That is covered by the part below which you maybe did not read
> > 
> > > > PS: if you think of changing the API, i dont think its the API.
> > > > I mean every user application will read the frames it receives, so
> > > > even if inside the decoder we just return the same frame with 2 pixels
> > > > different the user doesnt know this and has to read the whole frame.
> > > > The problem is moved around but its still there.
> 
> Copying is still slower than not copying. Enabling refcounting fixes
> the timeout issue here, and will likely silence a whole bunch of false
> positives for this class of files.

it makes probably sense to enable ref counting but we should
immedeatly in the next or a previous commit make the fuzzer read the frames
from the  decoder. Thats what basically every user app would do.
Otherwise we would have a bunch of issues closed and then reopened
later.

an alternative viewpoint to this would be to set the refcounting flag
from the input so the fuzzer itself has control over it and we test
both codepathes. This would improve coverage


> 
> It would be beneficial to have a consistent way of signalling that a
> frame didn't change, since a bunch of codecs have that property.
> Currently it's a mix of discontinuous timestamps, longer frame
> durations and repeating identical frames.

yes, i strongly agree.


> 
> > so feeding really crazy resolutions into a decoder requires some
> > small but proportional amout of input bytes.
> > double the width and the minimum input bytes double sort of.
> 
> Lavc is already very lenient with what it accepts. How do you detect
> the difference between "this packet is too small to decode to an entire
> frame" from "this packet is too small but we could still get a few MBs
> out of it"?

In reality this is actually not hard because a frame that is smaller
than the minimum valid size is generally for many codecs so small 
it really wont contain anything usefull to decode.
And we have discard_damaged_percentage where the user can tune it too.
Patches using discard_damaged_percentage are sometimes objected too
though so it is not consistently used.

Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


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

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

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Tomas Härdin]

sön 2019-08-18 klockan 12:19 +0200 skrev Michael Niedermayer:
> On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
> > 
> > wrote:
> > 
> > > On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> > > > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > > > 
> > > > I did some investigation, it is indeed ff_reget_buffer(). It copies the
> > > > frame data for some reason. The fix is simple in this case: just call
> > > > ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
> > > > same frame.
> > > > 
> > > > > As I said on IRC, this class of problems will exist for every codec.
> > > > > Cinepak is easy to decode, even at these resolutions. Just imagine 
> > > > > what
> > > > > will happens when someone feeds in a 65535x209 av1 stream..
> > > > 
> > > > And related to this, ff_reget_buffer() is used for a lot of these
> > > > codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
> > > > msrle, roqvideodec and others probably have the same flaw.
> > > 
> > > not calling any form of *get_buffer per frame breaks decoding into
> > > user supplied buffers.
> > > 
> > > If you check the documentation of the get_buffer2 callback
> > > 
> > > " This callback is called at the beginning of each frame to get data
> > > buffer(s) for it."
> > > 
> > > That would not be possible if its just called once in init

Sorry, I'm a bit rusty on lavc internals.

> > > and yes i too wish there was a magic fix but i think most things that
> > > look like magic fixes have a fatal flaw. But maybe iam missing something
> > > in fact i hope that iam missing something and that there is a magic fix
> > > 
> > 
> > Magic fix is enabling reference counted frames in fuzzer.
> 
> That is covered by the part below which you maybe did not read
> 
> > > PS: if you think of changing the API, i dont think its the API.
> > > I mean every user application will read the frames it receives, so
> > > even if inside the decoder we just return the same frame with 2 pixels
> > > different the user doesnt know this and has to read the whole frame.
> > > The problem is moved around but its still there.

Copying is still slower than not copying. Enabling refcounting fixes
the timeout issue here, and will likely silence a whole bunch of false
positives for this class of files.

It would be beneficial to have a consistent way of signalling that a
frame didn't change, since a bunch of codecs have that property.
Currently it's a mix of discontinuous timestamps, longer frame
durations and repeating identical frames.

> so feeding really crazy resolutions into a decoder requires some
> small but proportional amout of input bytes.
> double the width and the minimum input bytes double sort of.

Lavc is already very lenient with what it accepts. How do you detect
the difference between "this packet is too small to decode to an entire
frame" from "this packet is too small but we could still get a few MBs
out of it"?

FTR I've been in favor of rejecting broken files for a while now, and
I'm in favor of something like checking that the number of bytes in all
strips add up to some minimum. This can be combined with a pre-parsing
step that also rejects the input if any chunk has incorrect size.

> codecs OTOH which allow coding a whole frame in 10bytes input
> independant of the resolution behave worse in this way 
> as that can produce orders of magnitude more load per bandwidth
> the attacker needs.

Users are ultimately responsible for limiting how much resources their
programs can use. As you say, getting high amplification isn't hard.
Not even ffprobe is safe from this. I've been in talks with the
peertube people about this very topic not too long ago.

Stuff like this is why I've been harping on about parsers and verifying
things, and being very specific about what we accept. Unfortunately
FFculture seems to be against this.

/Tomas

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

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

Re: [FFmpeg-devel] [PATCH 1/7] avcodec/vc1: Check for excessive resolution

[Paul B Mahol]

NAK

On Thu, Aug 15, 2019 at 11:51 PM Michael Niedermayer 
wrote:

> Fixes: overflow in aspect ratio calculation
> Fixes: signed integer overflow: 393215 * 14594 cannot be represented in
> type 'int'
> Fixes:
> 15728/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMV3IMAGE_fuzzer-5661588893204480
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> :
> Michael Niedermayer 
> ---
>  libavcodec/vc1dec.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index 9519864c55..2636ea701f 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -426,6 +426,11 @@ static av_cold int vc1_decode_init(AVCodecContext
> *avctx)
>  GetBitContext gb;
>  int ret;
>
> +if (avctx->width > 1 << 14 || avctx->height > 1 << 14) {
> +avpriv_request_sample(avctx, "Huge resolution");
> +return AVERROR_PATCHWELCOME;
> +}
> +
>  /* save the container output size for WMImage */
>  v->output_width  = avctx->width;
>  v->output_height = avctx->height;
> --
> 2.22.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 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/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks

[Stanislav Ionascu]

Hi,

thanks for looking into this.

On Sun, Aug 18, 2019 at 4:55 AM Andreas Rheinhardt
 wrote:
>
> Hello,
>
> I am no expert on mov (and so this should definitely be looked at from
> someone who is), but I have some points:
>
> Stanislav Ionascu:
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index 1ea9b807e6..2a397c909a 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -2473,25 +2473,58 @@ static int
> matroska_parse_tracks(AVFormatContext *s)
> >  } else if (!strcmp(track->codec_id, "V_QUICKTIME") &&
> > (track->codec_priv.size >= 21)  &&
> > (track->codec_priv.data)) {
> > +MOVStreamContext *msc;
> > +MOVContext *mc = NULL;
> > +void *priv_data;
> > +int nb_streams;
>
> You could initialize nb_streams and priv_data here. And the
> initialization of mc is unnecessary.

Done.

>
> >  int ret = get_qt_codec(track, , _id);
> >  if (ret < 0)
> >  return ret;
> > -if (codec_id == AV_CODEC_ID_NONE &&
> AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI ")) {
> > -fourcc = MKTAG('S','V','Q','3');
> > -codec_id = ff_codec_get_id(ff_codec_movvideo_tags,
> fourcc);
> > +mc = av_mallocz(sizeof(*mc));
> > +if (!mc)
> > +return AVERROR(ENOMEM);
> > +priv_data = st->priv_data;
> > +nb_streams = s->nb_streams;
> > +mc->fc = s;
> > +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> > +if (!msc) {
> > +av_free(mc);
> > +st->priv_data = priv_data;
> > +return AVERROR(ENOMEM);
> >  }
> > +ffio_init_context(, track->codec_priv.data,
> > +  track->codec_priv.size,
> > +  0, NULL, NULL, NULL, NULL);
> > +
> > +/* ff_mov_read_stsd_entries updates stream s->nb_streams-1,
> > + * so set it temporarily to indicate which stream to
> update. */
> > +s->nb_streams = st->index + 1;
> > +ff_mov_read_stsd_entries(mc, , 1);
>
> Is it intentional that you don't check the return value here?.

Added the check.

>
> > +
> > +/* copy palette from MOVStreamContext */
> > +track->has_palette = msc->has_palette;
> > +if (track->has_palette) {
> > +/* leave bit_depth = -1, to reuse
> bits_per_coded_sample  */
> > +memcpy(track->palette, msc->palette, AVPALETTE_COUNT);
>
> In case the track has a palette, your patch would only copy 256 bytes
> of it, but a palette consists of 256 uint32_t. (This link
> https://drive.google.com/drive/folders/0B3_pEBoLs0faWElmM2FnLTZYNlk
> from the commit message that added the palette stuff seems to still
> work if you need samples for this.)

Indeed. I've corrected that. Also remuxed all mov's into mkv's, and
verified that they all still work.

>
> > +}
> > +
> > +av_free(msc);
> > +av_free(mc);
> > +st->priv_data = priv_data;
> > +s->nb_streams = nb_streams;
> > +fourcc = st->codecpar->codec_tag;
> > +codec_id = st->codecpar->codec_id;
> > +
> > +av_log(matroska->ctx, AV_LOG_TRACE,
> > +   "mov FourCC found %s.\n", av_fourcc2str(fourcc));
> > +
> >  if (codec_id == AV_CODEC_ID_NONE)
> >  av_log(matroska->ctx, AV_LOG_ERROR,
> > -   "mov FourCC not found %s.\n",
> av_fourcc2str(fourcc));
>
> If the codec_id turns out to be AV_CODEC_ID_NONE, two strings will be
> output (at least on loglevel trace): "mov FourCC found %s.\n" and then
> "mov FourCC not found %s.\n". The first of these is superfluous in
> this case.

Removed it, as stsd parser will also trace the FourCC code.

>
> > -if (track->codec_priv.size >= 86) {
> > -bit_depth = AV_RB16(track->codec_priv.data + 82);
> > -ffio_init_context(, track->codec_priv.data,
> > -  track->codec_priv.size,
> > -  0, NULL, NULL, NULL, NULL);
> > -if (ff_get_qtpalette(codec_id, , track->palette)) {
>
> If I am not extremely mistaken, then there is no need to include
> qtpalette.h any more after removing this function call.

Yes. Removed as it's not necessary.

>
> > -bit_depth &= 0x1F;
> > -track->has_palette = 1;
> > -}
> > +"mov FourCC not found %s.\n",
> av_fourcc2str(fourcc));
> > +
> > +// dvh1 in mkv is likely HEVC
> > +if (fourcc == MKTAG('d','v','h','1')) {
> > +codec_id = AV_CODEC_ID_HEVC;
>
> Your changes for dvh1 should probably be moved to a separate commit.

Removed. 

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Paul B Mahol]

On Sun, Aug 18, 2019 at 12:19 PM Michael Niedermayer 
wrote:

> On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer
> 
> > wrote:
> >
> > > On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> > > > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > > > > lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > > > > > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > > > > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > > > > >
> > > > > > > I feel I should point out that being conservative here is at
> odds
> > > with
> > > > > > > the general "best effort" approach taken in this project.
> These toy
> > > > > > > codecs are useful as illustrative examples of this
> contradiction.
> > > I'm
> > > > > > > sure there are many more examples of files that can cause
> ffmpeg
> > > to do
> > > > > > > a lot more work than expected, for almost every codec. I know
> > > afl-fuzz
> > > > > > > is likely to find out that it can make the ZMBV decoder do a
> lot of
> > > > > > > work for a small input file, because I've seen it do that with
> > > gzip.
> > > > > > >
> > > > > > > The user base for cinepak is of course miniscule, so I doubt
> > > anyone's
> > > > > > > going to complain that their broken CVID files don't work any
> > > more. I
> > > > > > > certainly don't care since cinepakenc only puts out valid
> files.
> > > > > > > But
> > > > > > > again, for other formats we're just going to have to tell
> users to
> > > put
> > > > > > > ffmpeg inside a sandbox and stick a CPU limit on it. Even
> ffprobe
> > > is
> > > > > > > vulnerable to DoS-y things.
> > > > > >
> > > > > > yes
> > > > > >
> > > > > > the question ATM is just what to do here about this codec ?
> > > > > > apply the patch ?
> > > > > > change it ?
> > > > >
> > > > > Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> > > > > wouldn't call decoding that @ 263 fps particularly slow
> > > > >
> > > > > Second, it's not the decoder which is slow. If I comment out the
> > > > > "*got_frame = 1;" then the test also runs fast. I'm not sure what
> > > > > happens elsewhere with the decoded buffer, but I suspect there's a
> > > > > bunch of useless malloc()/memset()ing going on. Maybe the decoder
> is
> > > > > using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not
> sure.
> > > >
> > > > I did some investigation, it is indeed ff_reget_buffer(). It copies
> the
> > > > frame data for some reason. The fix is simple in this case: just call
> > > > ff_get_buffer() once in cinepak_decode_init() and keep overwriting
> the
> > > > same frame.
> > > >
> > > > > As I said on IRC, this class of problems will exist for every
> codec.
> > > > > Cinepak is easy to decode, even at these resolutions. Just imagine
> what
> > > > > will happens when someone feeds in a 65535x209 av1 stream..
> > > >
> > > > And related to this, ff_reget_buffer() is used for a lot of these
> > > > codecs which only overwrite pixels in the old frame. flicvideo,
> gifdec,
> > > > msrle, roqvideodec and others probably have the same flaw.
> > >
> > > not calling any form of *get_buffer per frame breaks decoding into
> > > user supplied buffers.
> > >
> > > If you check the documentation of the get_buffer2 callback
> > >
> > > " This callback is called at the beginning of each frame to get data
> > > buffer(s) for it."
> > >
> > > That would not be possible if its just called once in init
> > >
> > > and yes i too wish there was a magic fix but i think most things that
> > > look like magic fixes have a fatal flaw. But maybe iam missing
> something
> > > in fact i hope that iam missing something and that there is a magic fix
> > >
> >
> > Magic fix is enabling reference counted frames in fuzzer.
>
> That is covered by the part below which you maybe did not read
>
>
> >
> >
> > >
> > > PS: if you think of changing the API, i dont think its the API.
>
> > > I mean every user application will read the frames it receives, so
> > > even if inside the decoder we just return the same frame with 2 pixels
> > > different the user doesnt know this and has to read the whole frame.
> > > The problem is moved around but its still there.
>
> This above
>
> Its a very specific feature of the fuzzer currently that it does not
> read the returned frame. But basically every real application will
> read it, it would be rather pointless to decode if its not read.
>
>
No, you are very mistaken and confused.


> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I do not agree with what you have to say, but I'll defend to the death your
> right to say it. -- Voltaire
> ___
> 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 

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
> wrote:
> 
> > On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> > > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > > > lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > > > > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > > > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > > > >
> > > > > > I feel I should point out that being conservative here is at odds
> > with
> > > > > > the general "best effort" approach taken in this project. These toy
> > > > > > codecs are useful as illustrative examples of this contradiction.
> > I'm
> > > > > > sure there are many more examples of files that can cause ffmpeg
> > to do
> > > > > > a lot more work than expected, for almost every codec. I know
> > afl-fuzz
> > > > > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > > > > work for a small input file, because I've seen it do that with
> > gzip.
> > > > > >
> > > > > > The user base for cinepak is of course miniscule, so I doubt
> > anyone's
> > > > > > going to complain that their broken CVID files don't work any
> > more. I
> > > > > > certainly don't care since cinepakenc only puts out valid files.
> > > > > > But
> > > > > > again, for other formats we're just going to have to tell users to
> > put
> > > > > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe
> > is
> > > > > > vulnerable to DoS-y things.
> > > > >
> > > > > yes
> > > > >
> > > > > the question ATM is just what to do here about this codec ?
> > > > > apply the patch ?
> > > > > change it ?
> > > >
> > > > Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> > > > wouldn't call decoding that @ 263 fps particularly slow
> > > >
> > > > Second, it's not the decoder which is slow. If I comment out the
> > > > "*got_frame = 1;" then the test also runs fast. I'm not sure what
> > > > happens elsewhere with the decoded buffer, but I suspect there's a
> > > > bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> > > > using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.
> > >
> > > I did some investigation, it is indeed ff_reget_buffer(). It copies the
> > > frame data for some reason. The fix is simple in this case: just call
> > > ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
> > > same frame.
> > >
> > > > As I said on IRC, this class of problems will exist for every codec.
> > > > Cinepak is easy to decode, even at these resolutions. Just imagine what
> > > > will happens when someone feeds in a 65535x209 av1 stream..
> > >
> > > And related to this, ff_reget_buffer() is used for a lot of these
> > > codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
> > > msrle, roqvideodec and others probably have the same flaw.
> >
> > not calling any form of *get_buffer per frame breaks decoding into
> > user supplied buffers.
> >
> > If you check the documentation of the get_buffer2 callback
> >
> > " This callback is called at the beginning of each frame to get data
> > buffer(s) for it."
> >
> > That would not be possible if its just called once in init
> >
> > and yes i too wish there was a magic fix but i think most things that
> > look like magic fixes have a fatal flaw. But maybe iam missing something
> > in fact i hope that iam missing something and that there is a magic fix
> >
> 
> Magic fix is enabling reference counted frames in fuzzer.

That is covered by the part below which you maybe did not read


> 
> 
> >
> > PS: if you think of changing the API, i dont think its the API.

> > I mean every user application will read the frames it receives, so
> > even if inside the decoder we just return the same frame with 2 pixels
> > different the user doesnt know this and has to read the whole frame.
> > The problem is moved around but its still there.

This above

Its a very specific feature of the fuzzer currently that it does not
read the returned frame. But basically every real application will 
read it, it would be rather pointless to decode if its not read.

Thanks

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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

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

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 02:35:45AM +0200, Tomas Härdin wrote:
> lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > > fre 2019-08-16 klockan 00:50 +0200 skrev Michael Niedermayer:
> > > > > On Thu, Aug 15, 2019 at 04:43:19PM +0200, Tomas Härdin wrote:
> > > > > > ons 2019-08-14 klockan 12:32 +0200 skrev Tomas Härdin:
> > > > > > > mån 2019-08-12 klockan 21:17 +0200 skrev Michael Niedermayer:
> > > > > > > > Fixes: Timeout (12sec -> 32ms)
> > > > > > > > Fixes: 16078/clusterfuzz-testcase-minimized-
> > > > > > > > ffmpeg_AV_CODEC_ID_CINEPAK_fuzzer-5695832885559296
> > > > > > > > [...]
> > > > > > > > +if (s->size < (s->avctx->width * s->avctx->height) / 
> > > > > > > > (4*4*8))
> > > > > > > > +return AVERROR_INVALIDDATA;
> > > > > > > 
> > > > > > > This is wrong if num_strips == 0, and if the MB area is != 0 mod 
> > > > > > > 8. You
> > > > > > > could merge it with the check above into something like:
> > > > > > > 
> > > > > > > if (s->size < 10 + s->sega_film_skip_bytes + num_strips * 12 +
> > > > > > > (num_strips ? ((s->avctx->width * s->avctx->height) / 16 + 
> > > > > > > 7)/8 :
> > > > > > > 0)) {
> > > > > > > return AVERROR_INVALIDDATA;
> > > > > > > }
> > > > > > > 
> > > > > > > The check further down could also check each strip's size, not 
> > > > > > > just the
> > > > > > > first one.
> > > > > > 
> > > > > > Actually, thinking a bit more about this, I suspect it might be 
> > > > > > legal
> > > > > > to have strips that don't cover the entire frame. It would 
> > > > > > certainly be
> > > > > > good to support that, for optimizing skip frames. Not sure what old
> > > > > > decoders make of that however. A skip could potentially be encoded 
> > > > > > in
> > > > > > 22 + (width/4 + 7)/8 bytes while still being compatible.
> > > > > 
> > > > > i was asking myself the same questions before writing the patch, and
> > > > > in the "spec" there is
> > > > > "Each frame is segmented into 4x4 pixel blocks, and each block is 
> > > > > coded using either 1 or 4 vectors."
> > > > > "Each" meaning no holes to me. If thats actually true for all real 
> > > > > files
> > > > > that i do not know.
> > > > 
> > > > We should keep in mind the spec is fine with there being zero strips.
> > > > It's only if one wants to support certain decoders that there will/must
> > > > be one or more strips.
> > > 
> > > I've been playing around with the Windows 3.1 cinepak decoder, and it
> > > seems one indeed has to code every MB even if they don't change. I
> > > suspect the reason is because that decoder uses double buffering and
> > > they wanted to avoid doing more memcpy()s than absolutely necessary. If
> > > one tries to play tricks like coding strips that are only 4x4 pixels to
> > > indicate a frame without changes then parts of the video will be
> > > replaced by garbage. So demanding number_of_bits >= number_of_mbs is
> > > certainly in line with that decoder.
> > > 
> > > I feel I should point out that being conservative here is at odds with
> > > the general "best effort" approach taken in this project. These toy
> > > codecs are useful as illustrative examples of this contradiction. I'm
> > > sure there are many more examples of files that can cause ffmpeg to do
> > > a lot more work than expected, for almost every codec. I know afl-fuzz
> > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > work for a small input file, because I've seen it do that with gzip.
> > > 
> > > The user base for cinepak is of course miniscule, so I doubt anyone's
> > > going to complain that their broken CVID files don't work any more. I
> > > certainly don't care since cinepakenc only puts out valid files. 
> > > But
> > > again, for other formats we're just going to have to tell users to put
> > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe is
> > > vulnerable to DoS-y things.
> > 
> > yes
> > 
> > the question ATM is just what to do here about this codec ?
> > apply the patch ?
> > change it ?
> 
> Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> wouldn't call decoding that @ 263 fps particularly slow
> 
> Second, it's not the decoder which is slow. If I comment out the
> "*got_frame = 1;" then the test also runs fast. I'm not sure what
> happens elsewhere with the decoded buffer, but I suspect there's a
> bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.
> 
> As I said on IRC, this class of problems will exist for every codec.
> Cinepak is easy to decode, even at these resolutions. Just imagine what

> will happens when someone feeds in a 65535x209 av1 stream..

i dont know about av1 specifically without checking the specs
but with most of the mainstream codecs. areas not covered by 
slices are 

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Paul B Mahol]

On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer 
wrote:

> On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > > lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > > > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > > >
> > > > > I feel I should point out that being conservative here is at odds
> with
> > > > > the general "best effort" approach taken in this project. These toy
> > > > > codecs are useful as illustrative examples of this contradiction.
> I'm
> > > > > sure there are many more examples of files that can cause ffmpeg
> to do
> > > > > a lot more work than expected, for almost every codec. I know
> afl-fuzz
> > > > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > > > work for a small input file, because I've seen it do that with
> gzip.
> > > > >
> > > > > The user base for cinepak is of course miniscule, so I doubt
> anyone's
> > > > > going to complain that their broken CVID files don't work any
> more. I
> > > > > certainly don't care since cinepakenc only puts out valid files.
> > > > > But
> > > > > again, for other formats we're just going to have to tell users to
> put
> > > > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe
> is
> > > > > vulnerable to DoS-y things.
> > > >
> > > > yes
> > > >
> > > > the question ATM is just what to do here about this codec ?
> > > > apply the patch ?
> > > > change it ?
> > >
> > > Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> > > wouldn't call decoding that @ 263 fps particularly slow
> > >
> > > Second, it's not the decoder which is slow. If I comment out the
> > > "*got_frame = 1;" then the test also runs fast. I'm not sure what
> > > happens elsewhere with the decoded buffer, but I suspect there's a
> > > bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> > > using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.
> >
> > I did some investigation, it is indeed ff_reget_buffer(). It copies the
> > frame data for some reason. The fix is simple in this case: just call
> > ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
> > same frame.
> >
> > > As I said on IRC, this class of problems will exist for every codec.
> > > Cinepak is easy to decode, even at these resolutions. Just imagine what
> > > will happens when someone feeds in a 65535x209 av1 stream..
> >
> > And related to this, ff_reget_buffer() is used for a lot of these
> > codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
> > msrle, roqvideodec and others probably have the same flaw.
>
> not calling any form of *get_buffer per frame breaks decoding into
> user supplied buffers.
>
> If you check the documentation of the get_buffer2 callback
>
> " This callback is called at the beginning of each frame to get data
> buffer(s) for it."
>
> That would not be possible if its just called once in init
>
> and yes i too wish there was a magic fix but i think most things that
> look like magic fixes have a fatal flaw. But maybe iam missing something
> in fact i hope that iam missing something and that there is a magic fix
>

Magic fix is enabling reference counted frames in fuzzer.


>
> PS: if you think of changing the API, i dont think its the API.
> I mean every user application will read the frames it receives, so
> even if inside the decoder we just return the same frame with 2 pixels
> different the user doesnt know this and has to read the whole frame.
> The problem is moved around but its still there.
>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> During times of universal deceit, telling the truth becomes a
> revolutionary act. -- George Orwell
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [v3] avformat/flvdec: delete unused code

[Michael Niedermayer]

On Wed, Aug 14, 2019 at 11:07:18AM +0800, leozhang wrote:
> Reviewed-by: Carl Eugen Hoyos 
> Signed-off-by: leozhang 
> ---
>  libavformat/flvdec.c | 17 -
>  1 file changed, 17 deletions(-)

probably ok

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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

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

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote:
> sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> > lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > > 
> > > > I feel I should point out that being conservative here is at odds with
> > > > the general "best effort" approach taken in this project. These toy
> > > > codecs are useful as illustrative examples of this contradiction. I'm
> > > > sure there are many more examples of files that can cause ffmpeg to do
> > > > a lot more work than expected, for almost every codec. I know afl-fuzz
> > > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > > work for a small input file, because I've seen it do that with gzip.
> > > > 
> > > > The user base for cinepak is of course miniscule, so I doubt anyone's
> > > > going to complain that their broken CVID files don't work any more. I
> > > > certainly don't care since cinepakenc only puts out valid files. 
> > > > But
> > > > again, for other formats we're just going to have to tell users to put
> > > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe is
> > > > vulnerable to DoS-y things.
> > > 
> > > yes
> > > 
> > > the question ATM is just what to do here about this codec ?
> > > apply the patch ?
> > > change it ?
> > 
> > Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> > wouldn't call decoding that @ 263 fps particularly slow
> > 
> > Second, it's not the decoder which is slow. If I comment out the
> > "*got_frame = 1;" then the test also runs fast. I'm not sure what
> > happens elsewhere with the decoded buffer, but I suspect there's a
> > bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> > using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.
> 
> I did some investigation, it is indeed ff_reget_buffer(). It copies the
> frame data for some reason. The fix is simple in this case: just call
> ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
> same frame.
> 
> > As I said on IRC, this class of problems will exist for every codec.
> > Cinepak is easy to decode, even at these resolutions. Just imagine what
> > will happens when someone feeds in a 65535x209 av1 stream..
> 
> And related to this, ff_reget_buffer() is used for a lot of these
> codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
> msrle, roqvideodec and others probably have the same flaw.

not calling any form of *get_buffer per frame breaks decoding into
user supplied buffers.

If you check the documentation of the get_buffer2 callback

" This callback is called at the beginning of each frame to get data buffer(s) 
for it."

That would not be possible if its just called once in init

and yes i too wish there was a magic fix but i think most things that
look like magic fixes have a fatal flaw. But maybe iam missing something
in fact i hope that iam missing something and that there is a magic fix

PS: if you think of changing the API, i dont think its the API.
I mean every user application will read the frames it receives, so
even if inside the decoder we just return the same frame with 2 pixels
different the user doesnt know this and has to read the whole frame.
The problem is moved around but its still there.

Thanks

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

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


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

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

[Paul B Mahol]

On Sun, Aug 18, 2019 at 10:25 AM Michael Niedermayer 
wrote:

> On Sun, Aug 18, 2019 at 09:21:25AM +0200, Paul B Mahol wrote:
> > NAK
>
> What problem do you see in this patch ?
>
> What change do you suggest ?
> or what alternative fix for the issue do you suggest ?
>
> a DOS issue in png will have to be fixed, otherwise major
> users would have to use different libraries for *png and
> disable it in their libavcodec.
>

What other png libraries do this thing?


>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> There will always be a question for which you do not know the correct
> answer.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 7/7] avcodec/truemotion2: Fix multiple integer overflows in

[Michael Niedermayer]

On Sat, Aug 17, 2019 at 03:40:26PM +0200, Moritz Barsnick wrote:
> On Thu, Aug 15, 2019 at 23:49:15 +0200, Michael Niedermayer wrote:
> > Subject: [FFmpeg-devel] [PATCH 7/7] avcodec/truemotion2: Fix multiple 
> > integer overflows in
> 
> ... in what? Spurious " in"?

... overflows in tm2_null_res_block()

fixed locally

thx

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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

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

Re: [FFmpeg-devel] [PATCH 5/7] avcodec/anm: Check input size for a frame with just a stop code

[Michael Niedermayer]

On Fri, Aug 16, 2019 at 10:20:42PM +1000, Peter Ross wrote:
> On Thu, Aug 15, 2019 at 11:49:13PM +0200, Michael Niedermayer wrote:
> > Fixes: Timeout (11sec -> 6sec)
> > Fixes: 
> > 16344/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ANM_fuzzer-5673032000995328
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/anm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/anm.c b/libavcodec/anm.c
> > index ab6a3994e9..778f38413e 100644
> > --- a/libavcodec/anm.c
> > +++ b/libavcodec/anm.c
> > @@ -119,6 +119,9 @@ static int decode_frame(AVCodecContext *avctx,
> >  uint8_t *dst, *dst_end;
> >  int count, ret;
> >  
> > +if (buf_size < 7)
> > +return AVERROR_INVALIDDATA;
> > +
> >  if ((ret = ff_reget_buffer(avctx, s->frame)) < 0)
> >  return ret;
> >  dst = s->frame->data[0];
> 
> approve.

will apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


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

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

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Tomas Härdin]

sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > > 
> > > I feel I should point out that being conservative here is at odds with
> > > the general "best effort" approach taken in this project. These toy
> > > codecs are useful as illustrative examples of this contradiction. I'm
> > > sure there are many more examples of files that can cause ffmpeg to do
> > > a lot more work than expected, for almost every codec. I know afl-fuzz
> > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > work for a small input file, because I've seen it do that with gzip.
> > > 
> > > The user base for cinepak is of course miniscule, so I doubt anyone's
> > > going to complain that their broken CVID files don't work any more. I
> > > certainly don't care since cinepakenc only puts out valid files. 
> > > But
> > > again, for other formats we're just going to have to tell users to put
> > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe is
> > > vulnerable to DoS-y things.
> > 
> > yes
> > 
> > the question ATM is just what to do here about this codec ?
> > apply the patch ?
> > change it ?
> 
> Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> wouldn't call decoding that @ 263 fps particularly slow
> 
> Second, it's not the decoder which is slow. If I comment out the
> "*got_frame = 1;" then the test also runs fast. I'm not sure what
> happens elsewhere with the decoded buffer, but I suspect there's a
> bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.

I did some investigation, it is indeed ff_reget_buffer(). It copies the
frame data for some reason. The fix is simple in this case: just call
ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
same frame.

> As I said on IRC, this class of problems will exist for every codec.
> Cinepak is easy to decode, even at these resolutions. Just imagine what
> will happens when someone feeds in a 65535x209 av1 stream..

And related to this, ff_reget_buffer() is used for a lot of these
codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
msrle, roqvideodec and others probably have the same flaw.

Patched attached.

/Tomas
From c85f23ca4c42f9ce27f512be897214b8689c1c94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= 
Date: Sun, 18 Aug 2019 10:30:59 +0200
Subject: [PATCH] libavcodec/cinepak: Avoid copying frame data

We can just keep overwriting the same frame.
This speeds up the decoder, especially for very
large frames, since skip MBs are turned into nops.

clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CINEPAK_fuzzer-5695832885559296
32577 ms -> 42 ms
---
 libavcodec/cinepak.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
index aeb15de0ed..e3e6ecfdf0 100644
--- a/libavcodec/cinepak.c
+++ b/libavcodec/cinepak.c
@@ -424,6 +424,7 @@ static int cinepak_decode (CinepakContext *s)
 static av_cold int cinepak_decode_init(AVCodecContext *avctx)
 {
 CinepakContext *s = avctx->priv_data;
+int ret;
 
 s->avctx = avctx;
 s->width = (avctx->width + 3) & ~3;
@@ -444,6 +445,9 @@ static av_cold int cinepak_decode_init(AVCodecContext *avctx)
 if (!s->frame)
 return AVERROR(ENOMEM);
 
+if ((ret = ff_get_buffer(avctx, s->frame, 0)) < 0)
+return ret;
+
 return 0;
 }
 
@@ -473,9 +477,6 @@ static int cinepak_decode_frame(AVCodecContext *avctx,
 return ret;
 }
 
-if ((ret = ff_reget_buffer(avctx, s->frame)) < 0)
-return ret;
-
 if (s->palette_video) {
 int size;
 const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, );
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH 3/6] avcodec/pngdec: Optimize has_trns code

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 08:49:06AM +0200, Reimar Döffinger wrote:
> On 18.08.2019, at 01:28, Michael Niedermayer  wrote:
> 
> > 30M cycles -> 5M cycles
> 
> I see nothing wrong with it, but:
> You could save reviewers a lot of time if you gave them a hint of what the 
> change
> contains instead of them having to reverse-engineer.
> In this case for example something like
> "add inner loop specialisations for 2 bpp and 4 bpp"

added


> 
> > 
> > @@ -1363,19 +1364,38 @@ exit_loop:
> > unsigned x, y;
> > 
> > av_assert0(s->bit_depth > 1);
> > -
> 
> Cosmetic?

removed and reposted 

thx

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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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

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

[FFmpeg-devel] [PATCH] avcodec/pngdec: Optimize has_trns code

[Michael Niedermayer]

add inner loop specialisations for 2 bpp and 4 bpp
These are all cases for which i found testsamples.

30M cycles -> 5M cycles

Testcase: fate-rgbapng-4816
Testcase: 
16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824

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

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index cad5796545..2d6c1b218e 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -24,6 +24,7 @@
 #include "libavutil/avassert.h"
 #include "libavutil/bprint.h"
 #include "libavutil/imgutils.h"
+#include "libavutil/intreadwrite.h"
 #include "libavutil/stereo3d.h"
 #include "libavutil/mastering_display_metadata.h"
 
@@ -1367,15 +1368,35 @@ exit_loop:
 for (y = 0; y < s->height; ++y) {
 uint8_t *row = >image_buf[s->image_linesize * y];
 
-/* since we're updating in-place, we have to go from right to left 
*/
-for (x = s->width; x > 0; --x) {
-uint8_t *pixel = [s->bpp * (x - 1)];
-memmove(pixel, [raw_bpp * (x - 1)], raw_bpp);
+if (s->bpp == 2 && byte_depth == 1) {
+uint8_t *pixel = [2 * s->width - 1];
+uint8_t *rowp  = [1 * s->width - 1];
+int tcolor = s->transparent_color_be[0];
+for (x = s->width; x > 0; --x) {
+*pixel-- = *rowp == tcolor ? 0 : 0xff;
+*pixel-- = *rowp--;
+}
+} else if (s->bpp == 4 && byte_depth == 1) {
+uint8_t *pixel = [4 * s->width - 1];
+uint8_t *rowp  = [3 * s->width - 1];
+int tcolor = AV_RL24(s->transparent_color_be);
+for (x = s->width; x > 0; --x) {
+*pixel-- = AV_RL24(rowp-2) == tcolor ? 0 : 0xff;
+*pixel-- = *rowp--;
+*pixel-- = *rowp--;
+*pixel-- = *rowp--;
+}
+} else {
+/* since we're updating in-place, we have to go from right to 
left */
+for (x = s->width; x > 0; --x) {
+uint8_t *pixel = [s->bpp * (x - 1)];
+memmove(pixel, [raw_bpp * (x - 1)], raw_bpp);
 
-if (!memcmp(pixel, s->transparent_color_be, raw_bpp)) {
-memset([raw_bpp], 0, byte_depth);
-} else {
-memset([raw_bpp], 0xff, byte_depth);
+if (!memcmp(pixel, s->transparent_color_be, raw_bpp)) {
+memset([raw_bpp], 0, byte_depth);
+} else {
+memset([raw_bpp], 0xff, byte_depth);
+}
 }
 }
 }
-- 
2.22.1

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

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

[Michael Niedermayer]

On Sun, Aug 18, 2019 at 09:21:25AM +0200, Paul B Mahol wrote:
> NAK

What problem do you see in this patch ?

What change do you suggest ?
or what alternative fix for the issue do you suggest ?

a DOS issue in png will have to be fixed, otherwise major
users would have to use different libraries for *png and
disable it in their libavcodec.

Thanks

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

There will always be a question for which you do not know the correct answer.


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

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

Re: [FFmpeg-devel] EOF and IO error checking

[Michael Niedermayer]

On Sat, Aug 17, 2019 at 09:10:59PM +0200, Marton Balint wrote:
> Hi,
> 
> As you might now avio_feof() returns true both in case of actual EOF and in
> case of IO errors.
> 
> Some demuxers (matroska) have special handling for this exact reason, e.g.:
> 
> if (avio_feof(pb)) {
> if (pb->error) {
> return pb->error;
> } else {
> return AVERROR_EOF;
> }
> }
> 
> Most of the demuxers do not, so there is a real chance that IO errrors are
> mistaken for EOF.
> 
> What should we do about this?
> 
> a) Fix every demuxer to return IO error if there is one.
> 
> b) Add special code to ff_read_packet which checks if there is an error in
> the IO context and return that if there is?

> 
> c) Add code to ffmpeg.c which checks the IO context error code after every
> av_read_frame call?

This while generally correct is not guranteed to be correct.
The internal IO context may have an IO error without the demuxer having an
error. From the possibility of reconnects to redundancy to errors after
all essential parts of a file ...
so this may need some flag per demuxer that either declares this relation
true or a flag declaring it false


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

Those who are best at talking, realize last or never when they are wrong.


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

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

[Paul B Mahol]

NAK

On Sun, Aug 18, 2019 at 1:36 AM Michael Niedermayer 
wrote:

> Fixes: Timeout (70sec -> 243ms)
> Fixes:
> 16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> :
> Michael Niedermayer 
> ---
>  libavcodec/pngdec.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index 4ca4f7bdc1..6e6856ab3e 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -320,6 +320,15 @@ static void deloco_ ## NAME(TYPE *dst, int size, int
> alpha) \
>  YUV2RGB(rgb8, uint8_t)
>  YUV2RGB(rgb16, uint16_t)
>
> +static int percent_missing(PNGDecContext *s)
> +{
> +if (s->interlace_type) {
> +return 100 - 100 * s->pass / (NB_PASSES - 1);
> +} else {
> +return 100 - 100 * s->y / s->cur_h;
> +}
> +}
> +
>  /* process exactly one decompressed row */
>  static void png_handle_row(PNGDecContext *s)
>  {
> @@ -1354,6 +1363,9 @@ exit_loop:
>  return 0;
>  }
>
> +if (percent_missing(s) > avctx->discard_damaged_percentage)
> +return AVERROR_INVALIDDATA;
> +
>  if (s->bits_per_pixel <= 4)
>  handle_small_bpp(s, p);
>
> --
> 2.22.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 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/6] avcodec/pngdec: Optimize has_trns code

[Reimar Döffinger]

On 18.08.2019, at 01:28, Michael Niedermayer  wrote:

> 30M cycles -> 5M cycles

I see nothing wrong with it, but:
You could save reviewers a lot of time if you gave them a hint of what the 
change
contains instead of them having to reverse-engineer.
In this case for example something like
"add inner loop specialisations for 2 bpp and 4 bpp"

> 
> @@ -1363,19 +1364,38 @@ exit_loop:
> unsigned x, y;
> 
> av_assert0(s->bit_depth > 1);
> -

Cosmetic?
___
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".