Re: [FFmpeg-devel] [PATCH 3/6] lavc/qsvdec: Replace current parser with MFXVideoDECODE_DecodeHeader()

2019-08-15 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Zhong Li
> Sent: Tuesday, August 13, 2019 2:11 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Li, Zhong ; Rogozhkin, Dmitry V
> 
> Subject: [FFmpeg-devel] [PATCH 3/6] lavc/qsvdec: Replace current parser
> with MFXVideoDECODE_DecodeHeader()
> 
> Using MSDK parser can improve qsv decoder pass rate in some cases (E.g:
> sps declares a wrong level_idc, smaller than it should be).
> And it is necessary for adding new qsv decoders such as MJPEG and VP9 since
> current parser can't provide enough information.
> Actually using MFXVideoDECODE_DecodeHeader() was disscussed at
> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-July/175734.html and
> merged as commit 1acb19d, but was overwritten when merged libav patches
> (commit: 1f26a23) without any explain.
> 
> Split decode header from decode_init, and call it for everyframe to detect
> format/resoultion change. It can fix some regression issues such as hevc
> 10bits decoding.
> 
> Signed-off-by: Zhong Li 
> Signed-off-by: Dmitry Rogozhkin 
> ---
>  libavcodec/qsvdec.c   | 184
> --
>  libavcodec/qsvdec.h   |   2 +
>  libavcodec/qsvdec_h2645.c |   1 +
>  libavcodec/qsvdec_other.c |   1 +
>  4 files changed, 100 insertions(+), 88 deletions(-)
> 
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index
> 46aa2d6..7e48c83 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -147,19 +147,21 @@ static int check_dec_param(AVCodecContext
> *avctx, QSVContext *q, mfxVideoParam *
>  return 1;
>  }
> 
> -static int qsv_decode_init(AVCodecContext *avctx, QSVContext *q)
> +static int qsv_decode_preinit(AVCodecContext *avctx, QSVContext *q,
> +enum AVPixelFormat pix_fmt, mfxVideoParam *param)
>  {
> -const AVPixFmtDescriptor *desc;
>  mfxSession session = NULL;
>  int iopattern = 0;
> -mfxVideoParam param = { 0 };
> -int frame_width  = avctx->coded_width;
> -int frame_height = avctx->coded_height;
>  int ret;
> +enum AVPixelFormat pix_fmts[3] = {
> +AV_PIX_FMT_QSV, /* opaque format in case of video memory
> output */
> +pix_fmt,/* system memory format obtained from
> bitstream parser */
> +AV_PIX_FMT_NONE };
> 
> -desc = av_pix_fmt_desc_get(avctx->sw_pix_fmt);
> -if (!desc)
> -return AVERROR_BUG;
> +ret = ff_get_format(avctx, pix_fmts);
> +if (ret < 0) {
> +q->orig_pix_fmt = avctx->pix_fmt = AV_PIX_FMT_NONE;
> +return ret;
> +}
> 
>  if (!q->async_fifo) {
>  q->async_fifo = av_fifo_alloc(q->async_depth *
> qsv_fifo_item_size()); @@ -197,54 +199,72 @@ static int
> qsv_decode_init(AVCodecContext *avctx, QSVContext *q)
>  return ret;
>  }
> 
> -ret = ff_qsv_codec_id_to_mfx(avctx->codec_id);
> -if (ret < 0)
> -return ret;
> +param->IOPattern   = q->iopattern;
> +param->AsyncDepth  = q->async_depth;
> +param->ExtParam= q->ext_buffers;
> +param->NumExtParam = q->nb_ext_buffers;
> 
> -param.mfx.CodecId  = ret;
> -param.mfx.CodecProfile = ff_qsv_profile_to_mfx(avctx->codec_id,
> avctx->profile);
> -param.mfx.CodecLevel   = ff_qsv_level_to_mfx(avctx->codec_id,
> avctx->level);
> -
> -param.mfx.FrameInfo.BitDepthLuma   = desc->comp[0].depth;
> -param.mfx.FrameInfo.BitDepthChroma = desc->comp[0].depth;
> -param.mfx.FrameInfo.Shift  = desc->comp[0].depth > 8;
> -param.mfx.FrameInfo.FourCC = q->fourcc;
> -param.mfx.FrameInfo.Width  = frame_width;
> -param.mfx.FrameInfo.Height = frame_height;
> -param.mfx.FrameInfo.ChromaFormat   =
> MFX_CHROMAFORMAT_YUV420;
> -
> -switch (avctx->field_order) {
> -case AV_FIELD_PROGRESSIVE:
> -param.mfx.FrameInfo.PicStruct =
> MFX_PICSTRUCT_PROGRESSIVE;
> -break;
> -case AV_FIELD_TT:
> -param.mfx.FrameInfo.PicStruct = MFX_PICSTRUCT_FIELD_TFF;
> -break;
> -case AV_FIELD_BB:
> -param.mfx.FrameInfo.PicStruct = MFX_PICSTRUCT_FIELD_BFF;
> -break;
> -default:
> -param.mfx.FrameInfo.PicStruct = MFX_PICSTRUCT_UNKNOWN;
> -break;
> -}
> +return 0;
> + }
> 
> -param.IOPattern   = q->iopattern;
> -param.AsyncDepth  = q->async_depth;
> -param.ExtParam= q->ext_buffers;
> -param.NumExtParam = q->nb_ext_buffers;
> +static int qsv_decode_init(AVCodecContext *avctx, QSVContext *q,
> +mfxVideoParam *param) {
> +int ret;
> 
> -if (!check_dec_param(avctx, q, ¶m)) {
> -//Just give a warning instead of an error since it is still decodable
> possibly.
> -av_log(avctx, AV_LOG_WARNING,
> -   "Current input bitstream is not supported by QSV
> decoder.\n");
> -}
> +avctx->width= param->mfx.FrameInfo.CropW;
> +avctx->height   = param->mfx.FrameInfo.CropH;
> +avctx->coded_width  = param->mfx.FrameInfo.Width;
> +avctx->coded_heigh

Re: [FFmpeg-devel] [PATCH 2/5] avcodec/vc1: Fix overflow in aspect?ratio calculation

2019-08-15 Thread Michael Niedermayer
On Tue, Aug 13, 2019 at 10:56:03AM +0200, Jean-Baptiste Kempf wrote:
> Are those values even acceptable according to spec?

Which spec ?
SMPTE 421M ?
no, i think not

wmv3image (which is what the fuzzer tested)
iam not sure if we have a spec about it.


> Shouldn't 16k*16K be the maximum allowed?

I can add a test, ask for a sample and fail instead.
I guess being more restrictive on this here is a good thing
so ill post a new patch with that

Thanks

[...]

-- 
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 v1] lavf/mov: Fix timestamp rescale on sidx atom

2019-08-15 Thread myp...@gmail.com
On Thu, Aug 15, 2019 at 12:49 PM Jun Li  wrote:
>
> On Thu, Jun 20, 2019 at 2:02 AM Jun Li  wrote:
>
> >
> >
> > On Tue, May 21, 2019 at 1:05 AM Jun Li  wrote:
> >
> >>
> >>
> >> On Thu, May 16, 2019 at 1:00 AM Jun Li  wrote:
> >>
> >>>
> >>>
> >>> On Sun, May 12, 2019 at 7:44 PM Jun Li  wrote:
> >>>
> 
> 
>  On Fri, May 10, 2019 at 7:25 PM Jun Li  wrote:
> 
> >
> > On Thu, May 9, 2019 at 2:08 AM Jun Li  wrote:
> >
> >> Fix #5090
> >> Fix the timestamp rescale issue, from sidx timebase to
> >> stream's timebase.
> >> ---
> >>  libavformat/mov.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index 78f692872b..d058855e6c 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -5017,7 +5017,7 @@ static int mov_read_sidx(MOVContext *c,
> >> AVIOContext *pb, MOVAtom atom)
> >>  return AVERROR_PATCHWELCOME;
> >>  }
> >>  avio_rb32(pb); // sap_flags
> >> -timestamp = av_rescale_q(pts, st->time_base, timescale);
> >> +timestamp = av_rescale_q(pts, timescale, st->time_base);
> >>
> >>  index = update_frag_index(c, offset);
> >>  frag_stream_info = get_frag_stream_info(&c->frag_index,
> >> index, track_id);
> >> --
> >> 2.17.1
> >>
> >
> > Ping
> >
> 
>  This change is for fix the issue of calculating sidx_pts.
>  Sidx box has "earliest_presentation_time", used as pts of  the referent
>  track, sidx also has timescale field.
>  So the operation should convert from sidx's timescale to track's
>  timescale, this patch is for addressing this, as well as fixing #5090.
> 
>  Of course this is based on my understanding, so please correct me if I
>  am wrong. Thanks !
> 
> 
> >>> Ping.
> >>> I believe this is a bug and triggered whenever sidx box's timescale is
> >>> different from track's timescale.
> >>> Created this kind of content and verified that ffplay couldn't play
> >>> while VLC plays well.
> >>> Then I checked VLC's implementation:
> >>>
> >>> https://github.com/videolan/vlc/blob/5609c1b41d6fbca6323103619c6139caf7bc9e6e/modules/demux/mp4/mp4.c#L4735
> >>>
> >>> Hope someone could help to have a review ? Thanks ! :)
> >>>
> >>> Best Regards,
> >>> -Jun
> >>>
> >>>
>  Best Regards,
>  Jun
> 
> >>>
> >> Ping x 3
> >>
> >
> > Ping x 4.
> > I believe this is an obvious bug and happened whenever sidx box's
> > timescale is different from track's timescale.
> > I created this kind of content and verified that ffplay couldn't play
> > while VLC plays well.
> > This is  VLC's implementation:
> >
> > https://github.com/videolan/vlc/blob/5609c1b41d6fbca6323103619c6139caf7bc9e6e/modules/demux/mp4/mp4.c#L4735
> >
>
> Ping x 5
Tested and verified with ffplay/ffprobe, now the sample video DTS is
monotonically increasing without wrap around.
___
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/8] avcodec/flicvideo: Optimize and Simplify FLI_COPY in flic_decode_frame_24BPP() by using bytestream2_get_buffer()

2019-08-15 Thread Tomas Härdin
ons 2019-08-14 klockan 19:42 +0200 skrev Michael Niedermayer:
> On Wed, Aug 14, 2019 at 05:21:49PM +0200, Tomas Härdin wrote:
> > mån 2019-08-12 klockan 21:17 +0200 skrev Michael Niedermayer:
> > > Fixes: Timeout (31sec  -> 22sec)
> > 
> > Is this a large test case? 22sec still sounds excessive
> 
> the input is about 240kbyte so 22sec is not great but thats what you get
> copying large frames around

Is there any way to take ownership of the buffer? The best copy is the
one you avoid.. Of course that won't work if there's more than one copy
chunk.

Come to think of it, any codec is "vulnerable" to these kinds of
crafted files. Eh

/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] libavformat/mxfenc: Allow more bitrates for NTSC IMX50

2019-08-15 Thread Tomas Härdin
ons 2019-08-14 klockan 22:18 +0200 skrev Thomas Mundt:
> Hi Tomas,
> 
> Am Mi., 14. Aug. 2019 um 12:42 Uhr schrieb Tomas Härdin  > :
> > tis 2019-08-13 klockan 22:03 +0200 skrev Thomas Mundt:
> > > Hi,
> > > 
> > > attached patch fixes ticket #8077.
> > > Please comment.
> > 
> > Probably OK, bitrates lower than 5000 are fine in D-10 according to
> > S356m.
> > 
> > > } else if ((sc->video_bit_rate >= 4840) && (sc->video_bit_rate <=
> > > 5000) && (mxf->time_base.den != 25)) {
> > 
> > You could drop the extra parentheses, else it should be fine.
> > 
> 
> New patch attached.

Looks OK. I'll push in a few days if no one else has any comments

> > The real fix is of course to add an explicit CBR mode to lavc, but
> > that's a bit more involved than this fix.
> 
> IMX is being used less and less. Maybe it´s not worth the effort.

It's not the only case where CBR MPEG-2 is desireable I think.
Certainly outside my concern however. Maybe something for x262? Since
x264 has such a mode I wouldn't be surprised if x262 does as well.

/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] lavfi/qsvvpp: disable pass through mode if format changed

2019-08-15 Thread Zhong Li
Fix tiket#8065

Signed-off-by: Zhong Li 
---
 libavfilter/vf_vpp_qsv.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
index f18513359a..858587411d 100644
--- a/libavfilter/vf_vpp_qsv.c
+++ b/libavfilter/vf_vpp_qsv.c
@@ -312,7 +312,9 @@ static int config_output(AVFilterLink *outlink)
 } else
 in_format = inlink->format;
 
-param.out_sw_format  = (vpp->out_format == AV_PIX_FMT_NONE) ? in_format : 
vpp->out_format;
+if (vpp->out_format == AV_PIX_FMT_NONE)
+vpp->out_format = in_format;
+param.out_sw_format  = vpp->out_format;
 
 if (vpp->use_crop) {
 crop.in_idx = 0;
@@ -454,7 +456,7 @@ static int config_output(AVFilterLink *outlink)
 
 if (vpp->use_frc || vpp->use_crop || vpp->deinterlace || vpp->denoise ||
 vpp->detail || vpp->procamp || vpp->rotate || vpp->hflip ||
-inlink->w != outlink->w || inlink->h != outlink->h)
+inlink->w != outlink->w || inlink->h != outlink->h || in_format != 
vpp->out_format)
 return ff_qsvvpp_create(ctx, &vpp->qsv, ¶m);
 else {
 av_log(ctx, AV_LOG_VERBOSE, "qsv vpp pass through mode.\n");
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 1/2] opusdsp: adjust and optimize C function to match assembly

2019-08-15 Thread Lynne
The C and asm versions behaved differently _outside_ of the codec.

The C version returned pre-multiplied 'state' for the next execution
to use right away, while the assembly version outputted non-multiplied
'state' for the next execution to multiply to save instructions.
Since the initial state when initialized or seeking is always 0,
and since C and asm versions were never mixed, there was no issue.

However, comparing outputs directly in checkasm doesn't work without
dividing the initial state by CELT_EMPH_COEFF and multiplying the
returned state by CELT_EMPH_COEFF for the assembly function.

Since its actually faster to do this in C as well, copy the behavior the
asm versions use. As a reminder, add a note explaining the differences
between libopus on coefficient init.

>From b7f2fc24387310cf12d57dbe1ce06f0284a2a390 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Thu, 15 Aug 2019 11:13:35 +0100
Subject: [PATCH 1/2] opusdsp: adjust and optimize C function to match assembly

The C and asm versions behaved differently _outside_ of the codec.

The C version returned pre-multiplied 'state' for the next execution
to use right away, while the assembly version outputted non-multiplied
'state' for the next execution to multiply to save instructions.
Since the initial state when initialized or seeking is always 0,
and since C and asm versions were never mixed, there was no issue.

However, comparing outputs directly in checkasm doesn't work without
dividing the initial state by CELT_EMPH_COEFF and multiplying the
returned state by CELT_EMPH_COEFF for the assembly function.

Since its actually faster to do this in C as well, copy the behavior the
asm versions use. As a reminder, the initial state 0 is divided by
CELT_EMPH_COEFF on seek and init (just in case in the future this is
changed, its technically more correct to init with CELT_EMPH_COEFF than 0,
however when seeking this will result in more audiable pops, unlike with 0
where the output gets in sync over a few samples).
---
 libavcodec/opus_celt.c |  6 +-
 libavcodec/opusdsp.c   | 11 +++
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/libavcodec/opus_celt.c b/libavcodec/opus_celt.c
index 4655172b09..9dbeff1927 100644
--- a/libavcodec/opus_celt.c
+++ b/libavcodec/opus_celt.c
@@ -507,7 +507,11 @@ void ff_celt_flush(CeltFrame *f)
 memset(block->pf_gains_old, 0, sizeof(block->pf_gains_old));
 memset(block->pf_gains_new, 0, sizeof(block->pf_gains_new));
 
-block->emph_coeff = 0.0;
+/* libopus uses CELT_EMPH_COEFF on init, but 0 is better since there's
+ * a lesser discontinuity when seeking.
+ * The deemphasis functions differ from libopus in that they require
+ * an initial state divided by the coefficient. */
+block->emph_coeff = 0.0f / CELT_EMPH_COEFF;
 }
 f->seed = 0;
 
diff --git a/libavcodec/opusdsp.c b/libavcodec/opusdsp.c
index 0e179c98c9..08df87ffbe 100644
--- a/libavcodec/opusdsp.c
+++ b/libavcodec/opusdsp.c
@@ -43,15 +43,10 @@ static void postfilter_c(float *data, int period, float *gains, int len)
 
 static float deemphasis_c(float *y, float *x, float coeff, int len)
 {
-float state = coeff;
+for (int i = 0; i < len; i++)
+coeff = y[i] = x[i] + coeff*CELT_EMPH_COEFF;
 
-for (int i = 0; i < len; i++) {
-const float tmp = x[i] + state;
-state = tmp * CELT_EMPH_COEFF;
-y[i] = tmp;
-}
-
-return state;
+return coeff;
 }
 
 av_cold void ff_opus_dsp_init(OpusDSP *ctx)
-- 
2.23.0.rc1

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

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

[FFmpeg-devel] [PATCH 2/2] checkasm: add opusdsp tests

2019-08-15 Thread Lynne
Patch attached.

>From 399dd39c2e99ee3aa3d284019eb8c01abbdedcc0 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Thu, 15 Aug 2019 11:42:19 +0100
Subject: [PATCH 2/2] checkasm: add opusdsp tests

---
 tests/checkasm/Makefile   |   1 +
 tests/checkasm/checkasm.c |   3 ++
 tests/checkasm/checkasm.h |   1 +
 tests/checkasm/opusdsp.c  | 103 ++
 tests/fate/checkasm.mak   |   1 +
 5 files changed, 109 insertions(+)
 create mode 100644 tests/checkasm/opusdsp.c

diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
index f5780eedb2..0112ff603e 100644
--- a/tests/checkasm/Makefile
+++ b/tests/checkasm/Makefile
@@ -22,6 +22,7 @@ AVCODECOBJS-$(CONFIG_DCA_DECODER)   += synth_filter.o
 AVCODECOBJS-$(CONFIG_EXR_DECODER)   += exrdsp.o
 AVCODECOBJS-$(CONFIG_HUFFYUV_DECODER)   += huffyuvdsp.o
 AVCODECOBJS-$(CONFIG_JPEG2000_DECODER)  += jpeg2000dsp.o
+AVCODECOBJS-$(CONFIG_OPUS_DECODER)  += opusdsp.o
 AVCODECOBJS-$(CONFIG_PIXBLOCKDSP)   += pixblockdsp.o
 AVCODECOBJS-$(CONFIG_HEVC_DECODER)  += hevc_add_res.o hevc_idct.o hevc_sao.o
 AVCODECOBJS-$(CONFIG_UTVIDEO_DECODER)   += utvideodsp.o
diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index 3e2ec377be..d9a5c7f401 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -130,6 +130,9 @@ static const struct {
 #if CONFIG_LLVIDENCDSP
 { "llviddspenc", checkasm_check_llviddspenc },
 #endif
+#if CONFIG_OPUS_DECODER
+{ "opusdsp", checkasm_check_opusdsp },
+#endif
 #if CONFIG_PIXBLOCKDSP
 { "pixblockdsp", checkasm_check_pixblockdsp },
 #endif
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index aed15b5fa4..fdf9eeb75d 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -64,6 +64,7 @@ void checkasm_check_jpeg2000dsp(void);
 void checkasm_check_llviddsp(void);
 void checkasm_check_llviddspenc(void);
 void checkasm_check_nlmeans(void);
+void checkasm_check_opusdsp(void);
 void checkasm_check_pixblockdsp(void);
 void checkasm_check_sbrdsp(void);
 void checkasm_check_synth_filter(void);
diff --git a/tests/checkasm/opusdsp.c b/tests/checkasm/opusdsp.c
new file mode 100644
index 00..279a92e5c6
--- /dev/null
+++ b/tests/checkasm/opusdsp.c
@@ -0,0 +1,103 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU 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 "libavcodec/opusdsp.h"
+
+#include "checkasm.h"
+
+#define randomize_float(buf, len)   \
+do {\
+for (int i = 0; i < len; i++) { \
+float f = (float)rnd() / (UINT_MAX >> 5) - 16.0f;   \
+buf[i] = f; \
+}   \
+} while (0)
+
+#define EPS 0.005
+#define MAX_SIZE (960)
+
+/* period is between 15 and 1022, inclusive */
+static void test_postfilter(int period)
+{
+LOCAL_ALIGNED(16, float, data0, [MAX_SIZE + 1024]);
+LOCAL_ALIGNED(16, float, data1, [MAX_SIZE + 1024]);
+
+/* This filter can explode very easily, so use a tapset from the codec.
+ * In the codec these are usually multiplied by at least 0.09375f,
+ * so its outside the largest filter value, but the filter is still stable
+ * so use it. */
+float gains[3] = { 0.3066406250f, 0.2170410156f, 0.1296386719f };
+
+/* The codec will always call with an offset which is aligned once
+ * (period + 2) is subtracted, but here we have to align it outselves. */
+int offset = FFALIGN(period + 2, 4);
+
+declare_func(void, float *data, int period, float *gains, int len);
+
+randomize_float(data0, MAX_SIZE + 1024);
+memcpy(data1, data0, (MAX_SIZE + 1024)*sizeof(float));
+
+call_ref(data0 + offset, period, gains, MAX_SIZE);
+call_new(data1 + offset, period, gains, MAX_SIZE);
+
+if (!float_near_abs_eps_array(data0 + offset, data1 + offset, EPS, MAX_SIZE))
+fail();
+bench_new(data1 + offset, period, gains, MAX_SIZE);
+}
+
+static void test_deemphasis(void)
+{
+LOCAL_ALIGNED(16, float, src, [FFALIGN(MAX_SIZE, 4)]);
+LOCAL_ALIGNED(16, float, dst0, [FFALIGN(MAX_SIZE, 4)]);
+LOCAL_ALIGNED(16, float, dst1, [FFALIGN(MAX_SIZE, 4)]);
+fl

Re: [FFmpeg-devel] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Vittorio Giovara
On Wed, Aug 14, 2019 at 10:11 PM Reimar Döffinger 
wrote:

> On 14.08.2019, at 11:45, Paul B Mahol  wrote:
> > I strongly disagree with you. Why some people have subscription to
> security
> > mailing list and I'm not allowed also?
>
> Long version, explaining to the best of my knowledge and memory:
> The people on it are on it because at some point it was considered
> necessary to have them on it.
> You have not brought an argument why the project would need you to be on
> it in order to deal with issues in a satisfactory way.
> Generally only basic technical skills are needed, enough to discuss the
> issue and potentially hand over to the maintainer. And whoever is involved
> in the releases is generally needed.
> Beyond that I would describe it as a PR function (giving a polite and
> level headed response to a security researcher claiming something that is
> obvious nonsense to an FFmpeg developer tends to make things go much
> smoother), which I would have assumed to not be among your aspirations.
> It's definitely not about a "right" or a "priviledge" or having "earned"
> it, it's about need.
> And when possible a bit of trust (the personal kind, not just the "not
> malicious" kind which is of course an absolute requirement), though that
> might be more relevant for projects like Linux where really bad stuff
> causing stress and possibly conflicts tends to hit. Flame wars is the last
> thing one needs in the middle of dealing with a bad issue.
>
> TL;DR is probably: one doesn't end up on security lists by asking to be on
> it but by persons Y and Z saying "we should/need to have person X on the
> list".
> I am not aware of any such wishes (though admittedly I wouldn't be the one
> contacted about it I guess).
>

I think being on the security list may have some professional implications
too: if you use ffmpeg in your $dayjob, being notified of security problem
in ffmpeg, and acting upon it before the fix lands in the tree, may be
crucial. I think Paul is lamenting the fact that being selected for the
security list is extremely arbitrary and there is no process described on
how to joining it.
-- 
Vittorio
___
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] libavformat/mxfenc: Allow more bitrates for NTSC IMX50

2019-08-15 Thread Thomas Mundt
Am Do., 15. Aug. 2019 um 11:01 Uhr schrieb Tomas Härdin :

> ons 2019-08-14 klockan 22:18 +0200 skrev Thomas Mundt:
> > Hi Tomas,
> >
> > Am Mi., 14. Aug. 2019 um 12:42 Uhr schrieb Tomas Härdin <
> tjop...@acc.umu.se
> > > :
> > > tis 2019-08-13 klockan 22:03 +0200 skrev Thomas Mundt:
> > > > Hi,
> > > >
> > > > attached patch fixes ticket #8077.
> > > > Please comment.
> > >
> > > Probably OK, bitrates lower than 5000 are fine in D-10 according to
> > > S356m.
> > >
> > > > } else if ((sc->video_bit_rate >= 4840) && (sc->video_bit_rate <=
> > > > 5000) && (mxf->time_base.den != 25)) {
> > >
> > > You could drop the extra parentheses, else it should be fine.
> > >
> >
> > New patch attached.
>
> Looks OK. I'll push in a few days if no one else has any comments
>

Thanks. Would you mind porting it to branches 4.1 and 4.2?


> > > The real fix is of course to add an explicit CBR mode to lavc, but
> > > that's a bit more involved than this fix.
> >
> > IMX is being used less and less. Maybe it´s not worth the effort.
>
> It's not the only case where CBR MPEG-2 is desireable I think.
> Certainly outside my concern however. Maybe something for x262? Since
> x264 has such a mode I wouldn't be surprised if x262 does as well.
>

Yeah, a CBR option would definitely make it easier for users.
So far I have not dealt with x262.

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

[FFmpeg-devel] [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread Tomas Härdin
Hi

Attached patch detects when the cinepak encoder has generated a
keyframe on its own, and pushes the generation of forced keyframes into
the future. This is similar to what msvideo1enc.c does.

Example:

$ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
cinepak out-before.avi
$ make
$ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
cinepak out-after.avi

out-before.avi: 667338
 out-after.avi: 660928

Aside: what is -keyint_min for exactly? It seems to be used as a max in
msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
minimum and maximum respectively.. Feels like the relevant encoders
need to be brought into line (assuming it doesn't break user scripts)

/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

2019-08-15 Thread Tomas Härdin
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
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/cinepak.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
> > index aeb15de0ed..62eb794332 100644
> > --- a/libavcodec/cinepak.c
> > +++ b/libavcodec/cinepak.c
> > @@ -356,6 +356,9 @@ static int cinepak_predecode_check
> > (CinepakContext *s)
> >  if (s->size < 10 + s->sega_film_skip_bytes + num_strips * 12)
> >  return AVERROR_INVALIDDATA;
> >  
> > +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'd replace s->avctx->height in the expression with the height of the
first strip, to not constrain things too much.

/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] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread James Almer
On 8/15/2019 11:34 AM, Tomas Härdin wrote:
> Hi
> 
> Attached patch detects when the cinepak encoder has generated a
> keyframe on its own, and pushes the generation of forced keyframes into
> the future. This is similar to what msvideo1enc.c does.
> 
> Example:
> 
> $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
> cinepak out-before.avi
> $ make
> $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
> cinepak out-after.avi
> 
> out-before.avi: 667338
>  out-after.avi: 660928
> 
> Aside: what is -keyint_min for exactly? It seems to be used as a max in
> msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
> minimum and maximum respectively.. Feels like the relevant encoders
> need to be brought into line (assuming it doesn't break user scripts)

Afaik, keyint_min is minimum interval/distance between keyframes. So
yes, if it's being used as max distance then it's wrong, as that's what
gop_size is for.

> 
> /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 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 8/8] avcodec/nuv: Avoid duplicating frames

2019-08-15 Thread James Almer
On 8/12/2019 4:17 PM, Michael Niedermayer wrote:
> Fixes: Timeout (14sec -> 133ms)
> Fixes: 
> 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> Fixes: 
> 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
>  (35sec ->0.5sec)
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/nuv.c  | 37 ++---
>  tests/ref/fate/nuv-rtjpeg |  1 -
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> index 75b14bce5b..0952b537b7 100644
> --- a/libavcodec/nuv.c
> +++ b/libavcodec/nuv.c
> @@ -42,6 +42,8 @@ typedef struct NuvContext {
>  unsigned char *decomp_buf;
>  uint32_t lq[64], cq[64];
>  RTJpegContext rtj;
> +int need_flush;
> +AVPacket flush_pkt;
>  } NuvContext;
>  
>  static const uint8_t fallback_lquant[] = {
> @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = {
>  99, 99, 99, 99, 99, 99, 99, 99
>  };
>  
> +static void decode_flush(AVCodecContext *avctx){
> +NuvContext *s = avctx->priv_data;
> +
> +s->need_flush = 0;
> +}
> +
>  /**
>   * @brief copy frame data from buffer to AVFrame, handling stride.
>   * @param f destination AVFrame
> @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  NUV_COPY_LAST = 'L'
>  } comptype;
>  
> +if (!avpkt->data) {
> +if (c->need_flush) {
> +c->need_flush = 0;
> +if ((ret = ff_reget_buffer(avctx, c->pic)) < 0)
> +return ret;
> +c->pic->pkt_pos  = c->flush_pkt.pos;
> +c->pic->pkt_duration = c->flush_pkt.duration;
> +c->pic->pkt_dts  = c->flush_pkt.dts;
> +c->pic->pkt_pts  =
> +c->pic->pts  = c->flush_pkt.pts;
> +if ((ret = av_frame_ref(data, c->pic)) < 0)
> +return ret;
> +*got_frame = 1;

Can't this be split off and shared in some way instead of implementing
it for every decoder that needs to remove duplicate frames? Or maybe
handle it in the generic decode.c code.

> +}
> +return 0;
> +}
> +c->flush_pkt = *avpkt;
> +c->pic->pkt_dts = c->flush_pkt.dts;
> +
> +
>  if (buf_size < 12) {
>  av_log(avctx, AV_LOG_ERROR, "coded frame too small\n");
>  return AVERROR_INVALIDDATA;
> @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  }
>  break;
>  case NUV_COPY_LAST:
> -keyframe = 0;
> -break;
> +c->need_flush = 1;
> +return buf_size;
>  default:
>  keyframe = 1;
>  break;
> @@ -313,6 +341,7 @@ retry:
>  if ((result = av_frame_ref(picture, c->pic)) < 0)
>  return result;
>  
> +c->need_flush = 0;
>  *got_frame = 1;
>  return orig_size;
>  }
> @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = {
>  .init   = decode_init,
>  .close  = decode_end,
>  .decode = decode_frame,
> -.capabilities   = AV_CODEC_CAP_DR1,
> +.flush  = decode_flush,
> +.caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS,
> +.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
>  .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
>  };
> diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg
> index b6f3b080dc..0914b985ec 100644
> --- a/tests/ref/fate/nuv-rtjpeg
> +++ b/tests/ref/fate/nuv-rtjpeg
> @@ -6,7 +6,6 @@
>  0,118,118,0,   460800, 0x54aedafe
>  0,152,152,0,   460800, 0xb7aa8b56
>  0,177,177,0,   460800, 0x283ea3b5
> -0,202,202,0,   460800, 0x283ea3b5
>  0,235,235,0,   460800, 0x10e577de
>  0,269,269,0,   460800, 0x4e091ee2
>  0,302,302,0,   460800, 0x2ea88828
> 

___
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] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread Tomas Härdin
tor 2019-08-15 klockan 12:54 -0300 skrev James Almer:
> On 8/15/2019 11:34 AM, Tomas Härdin wrote:
> > Hi
> > 
> > Attached patch detects when the cinepak encoder has generated a
> > keyframe on its own, and pushes the generation of forced keyframes into
> > the future. This is similar to what msvideo1enc.c does.
> > 
> > Example:
> > 
> > $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
> > cinepak out-before.avi
> > $ make
> > $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
> > cinepak out-after.avi
> > 
> > out-before.avi: 667338
> >  out-after.avi: 660928
> > 
> > Aside: what is -keyint_min for exactly? It seems to be used as a max in
> > msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
> > minimum and maximum respectively.. Feels like the relevant encoders
> > need to be brought into line (assuming it doesn't break user scripts)
> 
> Afaik, keyint_min is minimum interval/distance between keyframes. So
> yes, if it's being used as max distance then it's wrong, as that's what
> gop_size is for.

Make sense. But why is gop_size < keyint_min by default then? 12 vs 25.
I think the x264 people have complained multiple times about the
default gop_size setting even

/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] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Reimar Döffinger
On 15.08.2019, at 13:15, Vittorio Giovara  wrote:
> I think being on the security list may have some professional implications
> too: if you use ffmpeg in your $dayjob, being notified of security problem
> in ffmpeg, and acting upon it before the fix lands in the tree, may be
> crucial. I think Paul is lamenting the fact that being selected for the
> security list is extremely arbitrary and there is no process described on
> how to joining it.

Sorry, but just any $dayjob I really don't see relevant at all.
If there is a huge user of AND major contributor to FFmpeg with vastly higher 
risk of attack that is hard to mitigate in any other way they might have an 
argument. I.e. if there is a NEED because it is the only way to protect a 
significant user/number of users.
But it still most likely is a misuse. The security list is about receiving 
reports and responding to it from our side.
Using it to forewarn users would either mean letting a large number of people 
on it (I hope we agree that is obviously stupid) or disadvantaging > 99% of our 
users.
If someone has concerns in this area and I'm sure there's ways for them to 
contribute.
I still don't see it would need access to the security list though, but it 
might lead to being invited.

Of course this is just my opinion and I am happy to learn:
are there other projects describing such a process?
For the Linux kernel I only know about such a thing for the list that is for 
communicating and aligning with distributions.
Something comparable does not currently exist for FFmpeg.
___
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] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread James Almer
On 8/15/2019 2:13 PM, Tomas Härdin wrote:
> tor 2019-08-15 klockan 12:54 -0300 skrev James Almer:
>> On 8/15/2019 11:34 AM, Tomas Härdin wrote:
>>> Hi
>>>
>>> Attached patch detects when the cinepak encoder has generated a
>>> keyframe on its own, and pushes the generation of forced keyframes into
>>> the future. This is similar to what msvideo1enc.c does.
>>>
>>> Example:
>>>
>>> $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
>>> cinepak out-before.avi
>>> $ make
>>> $ ./ffmpeg -i fate-suite/zmbv/zmbv_32bit.avi -keyint_min 20 -vcodec
>>> cinepak out-after.avi
>>>
>>> out-before.avi: 667338
>>>  out-after.avi: 660928
>>>
>>> Aside: what is -keyint_min for exactly? It seems to be used as a max in
>>> msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
>>> minimum and maximum respectively.. Feels like the relevant encoders
>>> need to be brought into line (assuming it doesn't break user scripts)
>>
>> Afaik, keyint_min is minimum interval/distance between keyframes. So
>> yes, if it's being used as max distance then it's wrong, as that's what
>> gop_size is for.
> 
> Make sense. But why is gop_size < keyint_min by default then? 12 vs 25.

I have no idea.

> I think the x264 people have complained multiple times about the
> default gop_size setting even

The libx264 wrapper changes both to -1 in libavcodec, which means x264's
own defaults will be used instead. So if they complained then i guess it
was changed accordingly.

> 
> /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 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] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Paul B Mahol
On Thu, Aug 15, 2019 at 7:20 PM Reimar Döffinger 
wrote:

> On 15.08.2019, at 13:15, Vittorio Giovara 
> wrote:
> > I think being on the security list may have some professional
> implications
> > too: if you use ffmpeg in your $dayjob, being notified of security
> problem
> > in ffmpeg, and acting upon it before the fix lands in the tree, may be
> > crucial. I think Paul is lamenting the fact that being selected for the
> > security list is extremely arbitrary and there is no process described on
> > how to joining it.
>
> Sorry, but just any $dayjob I really don't see relevant at all.
> If there is a huge user of AND major contributor to FFmpeg with vastly
> higher risk of attack that is hard to mitigate in any other way they might
> have an argument. I.e. if there is a NEED because it is the only way to
> protect a significant user/number of users.
> But it still most likely is a misuse. The security list is about receiving
> reports and responding to it from our side.
> Using it to forewarn users would either mean letting a large number of
> people on it (I hope we agree that is obviously stupid) or disadvantaging >
> 99% of our users.
> If someone has concerns in this area and I'm sure there's ways for them to
> contribute.
> I still don't see it would need access to the security list though, but it
> might lead to being invited.
>
> Of course this is just my opinion and I am happy to learn:
> are there other projects describing such a process?
> For the Linux kernel I only know about such a thing for the list that is
> for communicating and aligning with distributions.
> Something comparable does not currently exist for FFmpeg.
>

So you, as developer are higher valued and more useful than other
developers?

This is discrimination.


> ___
> 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] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Juan De León
On Wed, Aug 14, 2019 at 12:11 PM Juan De León  wrote:

> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/encode_info.c |  68 
>  libavutil/encode_info.h | 112 
>  libavutil/frame.c   |   1 +
>  libavutil/frame.h   |   7 +++
>  5 files changed, 190 insertions(+)
>  create mode 100644 libavutil/encode_info.c
>  create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
>\
>dict.h\
>display.h \
>downmix_info.h\
> +  encode_info.h \
>encryption_info.h \
>error.h   \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
>   \
> dict.o   \
> display.o\
> downmix_info.o   \
> +   encode_info.o\
> encryption_info.o\
> error.o  \
> eval.o   \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..351333cf43
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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 "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> +
>  AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index 00..ae0a6563dc
> --- /dev/null
> +++ b/libavutil/encode_info.h
> @@ -0,0 +1,112 @@
> +/*
> + * This file is part of FFmpeg.
> 

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Paul B Mahol
On Thu, Aug 15, 2019 at 7:43 PM Juan De León <
juandl-at-google@ffmpeg.org> wrote:

> On Wed, Aug 14, 2019 at 12:11 PM Juan De León  wrote:
>
> > AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> > AV_FRAME_DATA_ENCODE_INFO.
> > The structure stores quantization index for each plane, DC/AC deltas
> > for luma and chroma planes, and an array of AVEncodeInfoBlock type
> > denoting position, size, and delta quantizer for each block in the
> > frame.
> > Can be extended to support extraction of other block information.
> >
> > Signed-off-by: Juan De León 
> > ---
> >  libavutil/Makefile  |   2 +
> >  libavutil/encode_info.c |  68 
> >  libavutil/encode_info.h | 112 
> >  libavutil/frame.c   |   1 +
> >  libavutil/frame.h   |   7 +++
> >  5 files changed, 190 insertions(+)
> >  create mode 100644 libavutil/encode_info.c
> >  create mode 100644 libavutil/encode_info.h
> >
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 57e6e3d7e8..37cfb099e9 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -24,6 +24,7 @@ HEADERS = adler32.h
> >\
> >dict.h
> \
> >display.h
>  \
> >downmix_info.h
> \
> > +  encode_info.h
>  \
> >encryption_info.h
>  \
> >error.h
>  \
> >eval.h
> \
> > @@ -111,6 +112,7 @@ OBJS = adler32.o
> >   \
> > dict.o
>  \
> > display.o
> \
> > downmix_info.o
>  \
> > +   encode_info.o
> \
> > encryption_info.o
> \
> > error.o
> \
> > eval.o
>  \
> > diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> > new file mode 100644
> > index 00..351333cf43
> > --- /dev/null
> > +++ b/libavutil/encode_info.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * 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 "libavutil/encode_info.h"
> > +#include "libavutil/mem.h"
> > +
> > +static int init_encode_info_data(AVEncodeInfoFrame *info, int
> nb_blocks) {
> > +info->nb_blocks = nb_blocks;
> > +info->block_size = sizeof(AVEncodeInfoBlock);
> > +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> > +
> > +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> > +info->plane_q[i] = -1;
> > +
> > +return 0;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> > +{
> > +AVEncodeInfoFrame *ptr;
> > +//AVEncodeInfoFrame already allocates size for one element of
> > AVEncodeInfoBlock
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +ptr = av_mallocz(size);
> > +if (!ptr)
> > +return NULL;
> > +
> > +init_encode_info_data(ptr, nb_blocks);
> > +
> > +return ptr;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> > nb_blocks)
> > +{
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +AVFrameSideData *sd = av_frame_new_side_data(frame,
> > +
> >  AV_FRAME_DATA_ENCODE_INFO,
> > + size);
> > +if (!sd)
> > +return NULL;
> > +
> > +memset(sd->data, 0, size);
> > +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> > +
> > +return (AVEncodeInfoFrame*)sd->data;
> > +}
> > diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> > new file mode 100644
> > index 00..ae0a6563dc
> > --- /dev/null
> > +++ b/libavutil/encode_info.h
> > @@ -0,0 +1,112 @@
> > +/*
> > + * 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 b

[FFmpeg-devel] [PATCH] Add assembly support for -fsanitize=hwaddress tagged globals.

2019-08-15 Thread Peter Collingbourne
As of LLVM r368102, Clang will set a pointer tag in bits 56-63 of the
address of a global when compiling with -fsanitize=hwaddress. This requires
an adjustment to assembly code that takes the address of such globals: the
code cannot use the regular R_AARCH64_ADR_PREL_PG_HI21 relocation to refer
to the global, since the tag would take the address out of range. Instead,
the code must use the non-checking (_NC) variant of the relocation (the
link-time check is substituted by a runtime check).

This change makes the necessary adjustment in the movrel macro, where it is
needed when compiling with -fsanitize=hwaddress.
---
 libavutil/aarch64/asm.S | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavutil/aarch64/asm.S b/libavutil/aarch64/asm.S
index 5c329430fd..3ac2ba0d52 100644
--- a/libavutil/aarch64/asm.S
+++ b/libavutil/aarch64/asm.S
@@ -32,6 +32,10 @@
 #   define FUNC #
 #endif
 
+#ifndef __has_feature
+#   define __has_feature(x) 0
+#endif
+
 .macro  function name, export=0, align=2
 .macro endfunc
 ELF .size   \name, . - \name
@@ -94,7 +98,11 @@ ELF .size   \name, . - \name
 add \rd, \rd, :lo12:\val+(\offset)
 .endif
 #elif CONFIG_PIC
+#   if __has_feature(hwaddress_sanitizer)
+adrp\rd, :pg_hi21_nc:\val+(\offset)
+#   else
 adrp\rd, \val+(\offset)
+#   endif
 add \rd, \rd, :lo12:\val+(\offset)
 #else
 ldr \rd, =\val+\offset
-- 
2.23.0.rc1.153.gdeed80330f-goog

___
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] Change libaom default to crf=28.

2019-08-15 Thread Elliott Karpilovsky
On Wed, Aug 14, 2019 at 5:35 PM James Zern
 wrote:
>
> Hi,
>
> On Tue, Aug 13, 2019 at 8:23 PM elliottk
>  wrote:
> >
> > Current default is 256kbps, which produces inconsistent
> > results (too high for low-res, too low for hi-res).
> > Use CRF instead, which will adapt.
> > ---
> >  libavcodec/libaomenc.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
>
> I think this is OK as it's similar to what is done for x264/5 [1].
>
> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > index 9b4fb3b4eb..a18d11c8aa 100644
> > --- a/libavcodec/libaomenc.c
> > +++ b/libavcodec/libaomenc.c
> > @@ -575,10 +575,10 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >  if (enccfg.rc_end_usage == AOM_CQ) {
> >  enccfg.rc_target_bitrate = 100;
> >  } else {
> > -avctx->bit_rate = enccfg.rc_target_bitrate * 1000;
> > +enccfg.rc_end_usage = AOM_Q;
> > +ctx->crf = 28;
> >
>
> Can we take a library default here or does it default to bitrate and
> have cq_level cleared?
I believe the library defaults to bitrate without a CQ level.
>
> >  av_log(avctx, AV_LOG_WARNING,
> > -   "Neither bitrate nor constrained quality specified, 
> > using default bitrate of %dkbit/sec\n",
> > -   enccfg.rc_target_bitrate);
> > +   "Neither bitrate nor constrained quality specified, 
> > using default CRF of 28\n");
> >
>
> You may want to use the variable to produce the output in case it changes.
Good suggestion. I will update the patch.
>
> [1] https://bugs.chromium.org/p/aomedia/issues/detail?id=2219#c9
> ___
> 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] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Nicolas George
Juan De León (12019-08-15):
> Ping. Does anyone have any more feedback?
> If not, can anyone review this for pushing.

Less than 24 hours feel a bit short to get impatient.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH 2/2] convert_from_tensorflow.py: support conv2d with dilation

2019-08-15 Thread Pedro Arthur
Pushed.

Em qua, 14 de ago de 2019 às 03:37, Guo, Yejun  escreveu:
>
>
>
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> > Pedro Arthur
> > Sent: Wednesday, August 14, 2019 12:09 AM
> > To: FFmpeg development discussions and patches 
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] convert_from_tensorflow.py: support
> > conv2d with dilation
> >
> > LGTM.
> > Should push soon.
>
> thanks.
>
> >
> > BTW I just noticed that the tensorflow backend is failling to load SR
> > filter models.
> >
> > $ python tools/python/convert.py sr_models/srcnn.pb
> > $ ./ffmpeg -i input.jpg -vf
> > sr=model=srcnn.model:dnn_backend=tensorflow out_srcnn_tf.png
> >
> > The above command fails.
> > It seems commit ccbab41039af424237eaac5c302c293ab97540f8 is the
> > problem. I thought I had tested it but clearly I made a mistake
> > somewhere in the process.
> > I suppose you have the .pb files to test it, but let me know if you need 
> > them.
>
> yes, I have the .pb files. I missed the patch for such support. Will refine 
> and send out soon.
>
>
> >
> > Em sex, 9 de ago de 2019 às 12:25, Guo, Yejun 
> > escreveu:
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Guo, Yejun
> > > > Sent: Tuesday, July 30, 2019 9:26 AM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Guo, Yejun 
> > > > Subject: [PATCH 2/2] convert_from_tensorflow.py: support conv2d with
> > dilation
> > > >
> > > > conv2d with dilation > 1 generates tens of nodes in graph, it is not
> > > > easy to parse each node one by one, so we do special tricks to parse
> > > > the conv2d layer.
> > > >
> > > > Signed-off-by: Guo, Yejun 
> > > > ---
> > > >  tools/python/convert_from_tensorflow.py | 80
> > > > -
> > > >  1 file changed, 59 insertions(+), 21 deletions(-)
> > >
> > > this patch set asks for review, thanks.
> > >
> > > I've locally finished more patches to improve dnn module, plan to send 
> > > more
> > them set by set, since the patches have dependency.
> > >
> > > Just in case you are interested in these new patches, I've uploaded to
> > https://github.com/guoyejun/ffmpeg/tree/dnn0809.
> > > for your convenient, I also copy the oneline log here for each patch (from
> > newer to older) with 4 patch sets.
> > >
> > > 7eced90 libavfilter/dnn: support multiple outputs for native mode
> > > 28a7054 libavfilter/dnn/dnn_backend_native: find the input operand
> > according to input name
> > >
> > > 256e657 FATE/dnn: add unit test for layer maximum
> > > 8c616a0 libavfilter/dnn: add layer maximum for native mode.
> > >
> > > 8ec6c0c FATE/dnn: add unit test for dnn depth_to_space layer
> > > 09ef108 libavfilter/dnn: separate depth_to_space layer from
> > dnn_backend_native.c to a new file
> > > c65b59d FATE/dnn: add unit test for dnn conv2d layer
> > > a5d69a7 libavfilter/dnn: separate conv2d layer from dnn_backend_native.c 
> > > to
> > a new file
> > >
> > > 202d323 dnn: export operand info in python script and load in c code
> > > 3c706a0 dnn: change .model file format to put layer number at the end of 
> > > file
> > > 0256731 dnn: introduce dnn operand (in c code) to hold operand infos 
> > > within
> > network
> > >
> > >
> > > Besides continuous dnn improvement, I also plan to add two generic video
> > filters for dnn.
> > > - a generic filter to process the content of AVFrame with different dnn
> > networks.
> > > and so the current specific filters such as vf_sr (some changes needed) 
> > > and
> > vf_derain are no longer needed, since they can be
> > > included in this specific filter. And of course, in practice I'll not 
> > > remove them.
> > >
> > > - a generic filter to analyze the content of AVFrame to generate some side
> > data with different dnn networks. The content of AVFrame does not change.
> > > The application, which invokes the filter with a given dnn network, has 
> > > the
> > responsibility/knowledge to parse the side data (analyze result).
> > >
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Nicolas George
Juan De León (12019-08-14):
> In that case, I believe documenting the size of the array and behaviour of
> undefined indexes should be enough. Have the pointers return NULL,
> and let the user handle the result, instead of stopping the execution.

I disagree. Either drop the check altogether or make it an assert. A
NULL return is a compromise that is worse than either.

> I would prefer to discuss the actual data structure for now.

This is whai I can comment on.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avutil: Add Simple loop detector

2019-08-15 Thread Nicolas George
Lynne (12019-08-14):
> I can't find a single hint of aggression in my email. I'm being direct and 
> factual.
> If you see this as aggression you shouldn't read any specifications or 
> reports, you'll find yourself very offended.

Was this supposed to be not aggressive either?

> I couldn't help but perceive the discussion you were having and your 
> intentions to post another version as ignoring others' opinions.

When some people want X and some people want not-X, somebody has to
decide. That is not ignoring, that is making an arbitration.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Alex Sukhanov
On Thu, Aug 15, 2019 at 11:07 AM Nicolas George  wrote:

> Juan De León (12019-08-15):
> > Ping. Does anyone have any more feedback?
> > If not, can anyone review this for pushing.
>
> Less than 24 hours feel a bit short to get impatient.
>
> Regards,
>
> --
>   Nicolas George
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Time is important here. Juan's internship is ending soon. Juan has been
working with Lynne on this design. Since there is back and forth emails in
this thread, it would be very helpful for Juan if Nicolas and Lynne get to
some agreement, rather than have intern trying to satisfy them while having
different views.
Juan needs clear guidance from ffmpeg community here. Please help.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Nicolas George
Alex Sukhanov (12019-08-15):
> Time is important here. Juan's internship is ending soon. Juan has been
> working with Lynne on this design. Since there is back and forth emails in
> this thread, it would be very helpful for Juan if Nicolas and Lynne get to
> some agreement, rather than have intern trying to satisfy them while having
> different views.
> Juan needs clear guidance from ffmpeg community here. Please help.

I have not seen Lynne's arguments. And I have stated and argued my
position: either drop the test or make it an assert. All other solution
is inferior.

Regards,

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

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

Re: [FFmpeg-devel] [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread Tomas Härdin
tor 2019-08-15 klockan 14:20 -0300 skrev James Almer:
> On 8/15/2019 2:13 PM, Tomas Härdin wrote:
> > tor 2019-08-15 klockan 12:54 -0300 skrev James Almer:
> > > On 8/15/2019 11:34 AM, Tomas Härdin wrote:
> > > > 
> > > > Aside: what is -keyint_min for exactly? It seems to be used as a max in
> > > > msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
> > > > minimum and maximum respectively.. Feels like the relevant encoders
> > > > need to be brought into line (assuming it doesn't break user scripts)
> > > 
> > > Afaik, keyint_min is minimum interval/distance between keyframes. So
> > > yes, if it's being used as max distance then it's wrong, as that's what
> > > gop_size is for.
> > 
> > Make sense. But why is gop_size < keyint_min by default then? 12 vs 25.
> 
> I have no idea.
> 
> > I think the x264 people have complained multiple times about the
> > default gop_size setting even
> 
> The libx264 wrapper changes both to -1 in libavcodec, which means x264's
> own defaults will be used instead. So if they complained then i guess it
> was changed accordingly.

Didn't know encoders could have their own defaults. Updated patch
attached. I added some sanity checks on -keyint_min and -g as well.

I couldn't find rl's contact information, hopefully they are subscribed
to this list.

/Tomas
From 49d7ac02d35acbf3e30555eef215ae3a15c06684 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= 
Date: Thu, 15 Aug 2019 16:20:23 +0200
Subject: [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as
 keyframe and reset curframe

This allows the encoder to make smarter choices for future frames,
while still keeping keyframe distances within [keyint_min, gop_size].
Defaults and checks prevent nonsensical keyint_min / gop_size combinations.
---
 libavcodec/cinepakenc.c | 54 +
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/libavcodec/cinepakenc.c b/libavcodec/cinepakenc.c
index 93917fafe8..396d3e5273 100644
--- a/libavcodec/cinepakenc.c
+++ b/libavcodec/cinepakenc.c
@@ -112,7 +112,7 @@ typedef struct CinepakEncContext {
 enum AVPixelFormat pix_fmt;
 int w, h;
 int frame_buf_size;
-int curframe, keyint;
+int curframe, keyint_min, keyint_max;
 AVLFG randctx;
 uint64_t lambda;
 int *codebook_input;
@@ -168,6 +168,16 @@ static av_cold int cinepak_encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 
+if (avctx->keyint_min <= 0) {
+av_log(avctx, AV_LOG_ERROR, "-keyint_min must be >= 1\n");
+return AVERROR(EINVAL);
+}
+
+if (avctx->keyint_min > avctx->gop_size) {
+av_log(avctx, AV_LOG_ERROR, "-keyint_min > -g is not allowed. Did you mean -g?\n");
+return AVERROR(EINVAL);
+}
+
 if (!(s->last_frame = av_frame_alloc()))
 return AVERROR(ENOMEM);
 if (!(s->best_frame = av_frame_alloc()))
@@ -213,7 +223,8 @@ static av_cold int cinepak_encode_init(AVCodecContext *avctx)
 s->h  = avctx->height;
 s->frame_buf_size = frame_buf_size;
 s->curframe   = 0;
-s->keyint = avctx->keyint_min;
+s->keyint_min = avctx->keyint_min;
+s->keyint_max = avctx->gop_size;
 s->pix_fmt= avctx->pix_fmt;
 
 // set up AVFrames
@@ -871,7 +882,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe,
 uint8_t *last_data[4], int last_linesize[4],
 uint8_t *data[4], int linesize[4],
 uint8_t *scratch_data[4], int scratch_linesize[4],
-unsigned char *buf, int64_t *best_score)
+unsigned char *buf, int64_t *best_score, CinepakMode *best_mode)
 {
 int64_t score = 0;
 int best_size = 0;
@@ -970,6 +981,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe,
 
 if (best_size == 0 || score < *best_score) {
 *best_score = score;
+*best_mode = mode;
 best_size = encode_mode(s, h,
 scratch_data, scratch_linesize,
 last_data, last_linesize, &info,
@@ -1000,13 +1012,15 @@ static int write_cvid_header(CinepakEncContext *s, unsigned char *buf,
 }
 
 static int rd_frame(CinepakEncContext *s, const AVFrame *frame,
-int isakeyframe, unsigned char *buf, int buf_size)
+int isakeyframe, unsigned char *buf, int buf_size,
+int *has_mc_out)
 {
 int num_strips, strip, i, y, nexty, size, temp_size, best_size;
 uint8_t *last_data[4], *data[4], *scratch_data[4];
 int  last_linesize[4],  linesize[4],  scratch_linesize[4];
 int64_t best_score = 0, score, score_temp;
 int best_nstrips;
+int has_mc;
 
 if (s->pix_fmt == AV_PIX_FMT_RGB24) {
 int x;
@@ -1067,9 +1081,11 @@ static int rd_frame(CinepakEncContext *s

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Lynne
Aug 15, 2019, 19:47 by geo...@nsup.org:

> Alex Sukhanov (12019-08-15):
>
>> Time is important here. Juan's internship is ending soon. Juan has been
>> working with Lynne on this design. Since there is back and forth emails in
>> this thread, it would be very helpful for Juan if Nicolas and Lynne get to
>> some agreement, rather than have intern trying to satisfy them while having
>> different views.
>> Juan needs clear guidance from ffmpeg community here. Please help.
>>
>
> I have not seen Lynne's arguments. And I have stated and argued my
> position: either drop the test or make it an assert. All other solution
> is inferior.
>

I don't really have an opinion on this, but I don't think users would even 
check the
return value as long as they loop over the specified nb_blocks. So I'm fine
with an assert, but I'd prefer to drop the test altogether.

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

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

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread James Almer
On 8/15/2019 3:47 PM, Nicolas George wrote:
> Alex Sukhanov (12019-08-15):
>> Time is important here. Juan's internship is ending soon. Juan has been
>> working with Lynne on this design. Since there is back and forth emails in
>> this thread, it would be very helpful for Juan if Nicolas and Lynne get to
>> some agreement, rather than have intern trying to satisfy them while having
>> different views.
>> Juan needs clear guidance from ffmpeg community here. Please help.
> 
> I have not seen Lynne's arguments. And I have stated and argued my
> position: either drop the test or make it an assert. All other solution
> is inferior.
> 
> Regards,

Go with the assert. It's not worth wasting time discussing this further.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Juan De León
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.

Signed-off-by: Juan De León 
---
 libavutil/Makefile  |   2 +
 libavutil/encode_info.c |  68 +
 libavutil/encode_info.h | 110 
 libavutil/frame.c   |   1 +
 libavutil/frame.h   |   7 +++
 5 files changed, 188 insertions(+)
 create mode 100644 libavutil/encode_info.c
 create mode 100644 libavutil/encode_info.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h   
  \
   dict.h\
   display.h \
   downmix_info.h\
+  encode_info.h \
   encryption_info.h \
   error.h   \
   eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o   \
display.o\
downmix_info.o   \
+   encode_info.o\
encryption_info.o\
error.o  \
eval.o   \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..351333cf43
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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 "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of 
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) + 
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int 
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) + 
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..93d0e3fb56
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,110 @@
+/*
+ * 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; 

[FFmpeg-devel] [PATCH] Change libaom default to crf=28.

2019-08-15 Thread elliottk
Current default is 256kbps, which produces inconsistent
results (too high for low-res, too low for hi-res).
Use CRF instead, which will adapt.
---
 libavcodec/libaomenc.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 9b4fb3b4eb..621e897672 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -575,10 +575,11 @@ static av_cold int aom_init(AVCodecContext *avctx,
 if (enccfg.rc_end_usage == AOM_CQ) {
 enccfg.rc_target_bitrate = 100;
 } else {
-avctx->bit_rate = enccfg.rc_target_bitrate * 1000;
+enccfg.rc_end_usage = AOM_Q;
+ctx->crf = 28;
 av_log(avctx, AV_LOG_WARNING,
-   "Neither bitrate nor constrained quality specified, using 
default bitrate of %dkbit/sec\n",
-   enccfg.rc_target_bitrate);
+   "Neither bitrate nor constrained quality specified, using 
default CRF of %d\n",
+   ctx->crf);
 }
 }
 
@@ -1091,7 +1092,7 @@ static const AVOption options[] = {
 };
 
 static const AVCodecDefault defaults[] = {
-{ "b",  "256*1000" },
+{ "b", "0" },
 { "qmin", "-1" },
 { "qmax", "-1" },
 { "g","-1" },
-- 
2.23.0.rc1.153.gdeed80330f-goog

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avutil: Add Simple loop detector

2019-08-15 Thread Lynne
Aug 15, 2019, 19:15 by geo...@nsup.org:

> Lynne (12019-08-14):
>
>> I can't find a single hint of aggression in my email. I'm being direct and 
>> factual.
>> If you see this as aggression you shouldn't read any specifications or 
>> reports, you'll find yourself very offended.
>>
>
> Was this supposed to be not aggressive either?
>

Yes, as it happens you don't even need to write anything aggressive to be 
mistaken as such.
Try reading your response in 3 ways: try speaking in a slightly lower monotonic 
voice, or try acting confused when you speak as if you didn't pay attention, 
and finally try putting a hard emphasis on on "this" and "not" while speaking 
quickly.
You'll hear a sarcastic remark, a genuine question, and a rude threat.

Should I tell you off for being aggressive when you could be genuinely 
confused? Or perhaps am I mistaking your sarcasm for being confused and 
educating you unnecessarily? Would this reply be considered an educational 
response, or maybe a sarcastic mockery? The possibilities are all too many
My point being is your own misconceptions color your view of everything, and 
you only have yourself to blame if you misread someone. And I do believe you're 
both misreading me, and will continue to do so despite what I've said and would 
say. Should I take that as a sign of aggression? Perhaps, if after saying twice 
in a row what my intentions were.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 4/7] avcodec/vc1_block: Check the return code from vc1_decode_p_block()

2019-08-15 Thread Michael Niedermayer
Fixes: left shift of negative value -1
Fixes: 
16424/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMV3_fuzzer-5656579055026176
Fixes: 
16358/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1IMAGE_fuzzer-5714436358144000

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

diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
index 775e3c516b..514206f6d2 100644
--- a/libavcodec/vc1_block.c
+++ b/libavcodec/vc1_block.c
@@ -1385,6 +1385,8 @@ static int vc1_decode_p_mb(VC1Context *v)
 pat = vc1_decode_p_block(v, 
v->block[v->cur_blk_idx][block_map[i]], i, mquant, ttmb, first_block,
  s->dest[dst_idx] + off, (i & 4) ? 
s->uvlinesize : s->linesize,
  CONFIG_GRAY && (i & 4) && 
(s->avctx->flags & AV_CODEC_FLAG_GRAY), &block_tt);
+if (pat < 0)
+return pat;
 block_cbp |= pat << (i << 2);
 if (!v->ttmbf && ttmb < 8)
 ttmb = -1;
@@ -1488,6 +1490,8 @@ static int vc1_decode_p_mb(VC1Context *v)
  (i & 4) ? s->uvlinesize : 
s->linesize,
  CONFIG_GRAY && (i & 4) && 
(s->avctx->flags & AV_CODEC_FLAG_GRAY),
  &block_tt);
+if (pat < 0)
+return pat;
 block_cbp |= pat << (i << 2);
 if (!v->ttmbf && ttmb < 8)
 ttmb = -1;
@@ -1698,6 +1702,8 @@ static int vc1_decode_p_mb_intfr(VC1Context *v)
  first_block, s->dest[dst_idx] + 
off,
  (i & 4) ? s->uvlinesize : 
(s->linesize << fieldtx),
  CONFIG_GRAY && (i & 4) && 
(s->avctx->flags & AV_CODEC_FLAG_GRAY), &block_tt);
+if (pat < 0)
+return pat;
 block_cbp |= pat << (i << 2);
 if (!v->ttmbf && ttmb < 8)
 ttmb = -1;
@@ -1834,6 +1840,8 @@ static int vc1_decode_p_mb_intfi(VC1Context *v)
  (i & 4) ? s->uvlinesize : s->linesize,
  CONFIG_GRAY && (i & 4) && 
(s->avctx->flags & AV_CODEC_FLAG_GRAY),
  &block_tt);
+if (pat < 0)
+return pat;
 block_cbp |= pat << (i << 2);
 if (!v->ttmbf && ttmb < 8)
 ttmb = -1;
@@ -1853,7 +1861,7 @@ static int vc1_decode_p_mb_intfi(VC1Context *v)
 
 /** Decode one B-frame MB (in Main profile)
  */
-static void vc1_decode_b_mb(VC1Context *v)
+static int vc1_decode_b_mb(VC1Context *v)
 {
 MpegEncContext *s = &v->s;
 GetBitContext *gb = &s->gb;
@@ -1919,7 +1927,7 @@ static void vc1_decode_b_mb(VC1Context *v)
 bmvtype = BMV_TYPE_INTERPOLATED;
 ff_vc1_pred_b_mv(v, dmv_x, dmv_y, direct, bmvtype);
 vc1_b_mc(v, dmv_x, dmv_y, direct, bmvtype);
-return;
+return 0;
 }
 if (direct) {
 cbp = get_vlc2(&v->s.gb, v->cbpcy_vlc->table, VC1_CBPCY_P_VLC_BITS, 2);
@@ -1936,7 +1944,7 @@ static void vc1_decode_b_mb(VC1Context *v)
 /* no coded blocks - effectively skipped */
 ff_vc1_pred_b_mv(v, dmv_x, dmv_y, direct, bmvtype);
 vc1_b_mc(v, dmv_x, dmv_y, direct, bmvtype);
-return;
+return 0;
 }
 if (s->mb_intra && !mb_has_coeffs) {
 GET_MQUANT();
@@ -1951,7 +1959,7 @@ static void vc1_decode_b_mb(VC1Context *v)
 /* interpolated skipped block */
 ff_vc1_pred_b_mv(v, dmv_x, dmv_y, direct, bmvtype);
 vc1_b_mc(v, dmv_x, dmv_y, direct, bmvtype);
-return;
+return 0;
 }
 }
 ff_vc1_pred_b_mv(v, dmv_x, dmv_y, direct, bmvtype);
@@ -1995,20 +2003,23 @@ static void vc1_decode_b_mb(VC1Context *v)
   i & 4 ? s->uvlinesize
 : s->linesize);
 } else if (val) {
-vc1_decode_p_block(v, s->block[i], i, mquant, ttmb,
-   first_block, s->dest[dst_idx] + off,
-   (i & 4) ? s->uvlinesize : s->linesize,
-   CONFIG_GRAY && (i & 4) && (s->avctx->flags & 
AV_CODEC_FLAG_GRAY), NULL);
+int pat = vc1_decode_p_block(v, s->block[i], i, mquant, ttmb,
+ first_block, s->dest[dst_

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

2019-08-15 Thread Michael Niedermayer
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] [PATCH 2/7] avcodec/vc1_block: Check for double escapes

2019-08-15 Thread Michael Niedermayer
Fixes: out of array read
Fixes: 
16331/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMV3IMAGE_fuzzer-5672735195267072

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

diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
index 94184b0873..775e3c516b 100644
--- a/libavcodec/vc1_block.c
+++ b/libavcodec/vc1_block.c
@@ -526,7 +526,7 @@ static int vc1_decode_ac_coeff(VC1Context *v, int *last, 
int *skip,
 int escape = decode210(gb);
 if (escape != 2) {
 index = get_vlc2(gb, ff_vc1_ac_coeff_table[codingset].table, 
AC_VLC_BITS, 3);
-if (index < 0)
+if (index >= ff_vc1_ac_sizes[codingset] - 1U)
 return AVERROR_INVALIDDATA;
 run   = vc1_index_decode_table[codingset][index][0];
 level = vc1_index_decode_table[codingset][index][1];
-- 
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 6/7] tools/target_dec_fuzzer: adjust pixel threshold for TRUEMOTION2, as it allows coding gigantic images on tiny input

2019-08-15 Thread Michael Niedermayer
Fixes: Timeout (137sec -> 6sec)
Fixes: 
16090/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-5674245178261504

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

diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index d83039417c..c5fc4f0537 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -173,6 +173,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t 
size) {
 case AV_CODEC_ID_MSRLE: maxpixels /= 16;  break;
 case AV_CODEC_ID_QTRLE: maxpixels /= 16;  break;
 case AV_CODEC_ID_GIF:   maxpixels /= 16;  break;
+case AV_CODEC_ID_TRUEMOTION2: maxpixels /= 1024; break;
 // Performs slow frame rescaling in C
 case AV_CODEC_ID_GDV:   maxpixels /= 256; break;
 // Postprocessing in C
-- 
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/7] avcodec/vc1dec: Require res_sprite for wmv3images

2019-08-15 Thread Michael Niedermayer
non res_sprite leads to decoder delay which leads to assertion failure
Fixes: Assertion failure
Fixes: 
16402/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMV3IMAGE_fuzzer-5704510034411520
Fixes: left shift of 1073741824 by 1 places cannot be represented in type 'int'
Fixes: 
16425/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMV3IMAGE_fuzzer-5692858838810624

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 2636ea701f..c2f8391714 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -455,6 +455,11 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
 if ((ret = ff_vc1_decode_sequence_header(avctx, v, &gb)) < 0)
   return ret;
 
+if (avctx->codec_id == AV_CODEC_ID_WMV3IMAGE && !v->res_sprite) {
+avpriv_request_sample(avctx, "Non sprite WMV3IMAGE");
+return AVERROR_PATCHWELCOME;
+}
+
 count = avctx->extradata_size*8 - get_bits_count(&gb);
 if (count > 0) {
 av_log(avctx, AV_LOG_INFO, "Extra data: %i bits left, value: %X\n",
-- 
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 7/7] avcodec/truemotion2: Fix multiple integer overflows in

2019-08-15 Thread Michael Niedermayer
Fixes: signed integer overflow: 1795032576 + 598344192 cannot be represented in 
type 'int'
Fixes: 
16196/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-5636723419119616

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

diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
index 5d6dfc24c3..27c876fd7d 100644
--- a/libavcodec/truemotion2.c
+++ b/libavcodec/truemotion2.c
@@ -619,7 +619,7 @@ static inline void tm2_null_res_block(TM2Context *ctx, 
AVFrame *pic, int bx, int
 ct = ctx->D[0] + ctx->D[1] + ctx->D[2] + ctx->D[3];
 
 if (bx > 0)
-left = last[-1] - ct;
+left = last[-1] - (unsigned)ct;
 else
 left = 0;
 
@@ -630,7 +630,7 @@ static inline void tm2_null_res_block(TM2Context *ctx, 
AVFrame *pic, int bx, int
 last[2] = right - (diff >> 2);
 last[3] = right;
 {
-int tp = left;
+unsigned tp = left;
 
 ctx->D[0] = (tp + (ct >> 2)) - left;
 left += ctx->D[0];
-- 
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 5/7] avcodec/anm: Check input size for a frame with just a stop code

2019-08-15 Thread Michael Niedermayer
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];
-- 
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 v2] avformat/http: Fixes #7975. Before reusing the connection the headers are checked to see if the server is closing the connection.

2019-08-15 Thread Ian Klassen
Hi,

Sorry Moritz, I somehow missed your feedback. Here's an updated patch with
the fixed formatting. I also set up a server that you can test with. Try:

ffmpeg -i test.mp4 -hls_flags +append_list -hls_time 6 -method PUT
-http_persistent 1 http://45.33.124.115/stream.m3u8

The server has persistent connections turned off so you'll see the errors
appear but they should be resolved once you apply the patch.

Thanks.

Ian

---
 libavformat/http.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 579debc..dcbd33a 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -332,15 +332,22 @@ int ff_http_do_new_request(URLContext *h, const char
*uri)
 return AVERROR(EINVAL);
 }

+if (!s->end_header) {
+int new_location;
+http_read_header(h, &new_location);
+if (s->willclose) {
+ret = ffurl_closep(&s->hd);
+if (ret < 0)
+return ret;
+}
+}
+
 if (!s->end_chunked_post) {
 ret = http_shutdown(h, h->flags);
 if (ret < 0)
 return ret;
 }

-if (s->willclose)
-return AVERROR_EOF;
-
 s->end_chunked_post = 0;
 s->chunkend  = 0;
 s->off   = 0;
-- 
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

2019-08-15 Thread Nicolas George
Juan De León (12019-08-15):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
> 
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/encode_info.c |  68 +
>  libavutil/encode_info.h | 110 
>  libavutil/frame.c   |   1 +
>  libavutil/frame.h   |   7 +++
>  5 files changed, 188 insertions(+)
>  create mode 100644 libavutil/encode_info.c
>  create mode 100644 libavutil/encode_info.h
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h 
> \
>dict.h\
>display.h \
>downmix_info.h\
> +  encode_info.h \
>encryption_info.h \
>error.h   \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o  
>   \
> dict.o   \
> display.o\
> downmix_info.o   \
> +   encode_info.o\
> encryption_info.o\
> error.o  \
> eval.o   \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..351333cf43
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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 "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of 
> AVEncodeInfoBlock

> +size_t size = sizeof(AVEncodeInfoFrame) + 
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);

This can overflow.

> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int 
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) + 
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index 00

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

2019-08-15 Thread 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
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/cinepak.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
> > > index aeb15de0ed..62eb794332 100644
> > > --- a/libavcodec/cinepak.c
> > > +++ b/libavcodec/cinepak.c
> > > @@ -356,6 +356,9 @@ static int cinepak_predecode_check
> > > (CinepakContext *s)
> > >  if (s->size < 10 + s->sega_film_skip_bytes + num_strips * 12)
> > >  return AVERROR_INVALIDDATA;
> > >  
> > > +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.


> 
> I'd replace s->avctx->height in the expression with the height of the
> first strip, to not constrain things too much.

ill post a patch doing that 

thx


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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- 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] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Reimar Döffinger


On 15.08.2019, at 19:38, Paul B Mahol  wrote:

> On Thu, Aug 15, 2019 at 7:20 PM Reimar Döffinger 
> wrote:
> 
>> On 15.08.2019, at 13:15, Vittorio Giovara 
>> wrote:
>>> I think being on the security list may have some professional
>> implications
>>> too: if you use ffmpeg in your $dayjob, being notified of security
>> problem
>>> in ffmpeg, and acting upon it before the fix lands in the tree, may be
>>> crucial. I think Paul is lamenting the fact that being selected for the
>>> security list is extremely arbitrary and there is no process described on
>>> how to joining it.
>> 
>> Sorry, but just any $dayjob I really don't see relevant at all.
>> If there is a huge user of AND major contributor to FFmpeg with vastly
>> higher risk of attack that is hard to mitigate in any other way they might
>> have an argument. I.e. if there is a NEED because it is the only way to
>> protect a significant user/number of users.
>> But it still most likely is a misuse. The security list is about receiving
>> reports and responding to it from our side.
>> Using it to forewarn users would either mean letting a large number of
>> people on it (I hope we agree that is obviously stupid) or disadvantaging >
>> 99% of our users.
>> If someone has concerns in this area and I'm sure there's ways for them to
>> contribute.
>> I still don't see it would need access to the security list though, but it
>> might lead to being invited.
>> 
>> Of course this is just my opinion and I am happy to learn:
>> are there other projects describing such a process?
>> For the Linux kernel I only know about such a thing for the list that is
>> for communicating and aligning with distributions.
>> Something comparable does not currently exist for FFmpeg.
>> 
> 
> So you, as developer are higher valued and more useful than other
> developers?

I have no idea where you get that from anything I said, do you think the bus 
driver is higher valued and more useful than anyone else on the bus because 
they don't let just anyone who wants drive it?
___
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] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Reimar Döffinger
On 15.08.2019, at 13:15, Vittorio Giovara  wrote:
>  if you use ffmpeg in your $dayjob, being notified of security problem
> in ffmpeg, and acting upon it before the fix lands in the tree, may be
> crucial.

I realize I only responded to this specific part only in the context of this 
discussion.
Which might give the wrong impression.
I'd LOVE for someone to come up with documentation and criteria and then create 
and managing a mailing
list of important and trusted USERS (which might overlap with developers, but 
I'd expect it to be more people in admin/deployment positions, or DevOps or 
such), similarly to what the Linux kernel has.
And a guideline for when it would be used.
I would expect anyone handling security issues would make an effort to have 
that list involved as appropriate and be happy to have a way to give a heads-up 
to critical users (correct me if I'm wrong on that).
But it would need one or more volunteers to do the work.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/dnn/dnn_backend_tf: add tf.pad support for tensorflow backend with native model.

2019-08-15 Thread Guo, Yejun


> -Original Message-
> From: Guo, Yejun
> Sent: Wednesday, August 14, 2019 3:06 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Guo, Yejun 
> Subject: [PATCH 2/2] libavfilter/dnn/dnn_backend_tf: add tf.pad support for
> tensorflow backend with native model.
> 
> Signed-off-by: Guo, Yejun 
> ---
>  libavfilter/dnn/dnn_backend_tf.c | 47 
> 
>  1 file changed, 19 insertions(+), 28 deletions(-)
> 
> diff --git a/libavfilter/dnn/dnn_backend_tf.c 
> b/libavfilter/dnn/dnn_backend_tf.c
> index ca7434a..75dbe5e 100644
> --- a/libavfilter/dnn/dnn_backend_tf.c
> +++ b/libavfilter/dnn/dnn_backend_tf.c
> @@ -27,6 +27,7 @@
>  #include "dnn_backend_native.h"
>  #include "libavformat/avio.h"
>  #include "libavutil/avassert.h"
> +#include "dnn_backend_native_layer_pad.h"
> 
>  #include 
> 
> @@ -347,23 +348,8 @@ static DNNReturnType
> add_depth_to_space_layer(TFModel *tf_model, TF_Operation **
>  return DNN_SUCCESS;
>  }
> 
> -static int calculate_pad(const ConvolutionalNetwork *conv_network)
> -{
> -ConvolutionalParams *params;
> -int32_t layer;
> -int pad = 0;
> -
> -for (layer = 0; layer < conv_network->layers_num; ++layer){
> -if (conv_network->layers[layer].type == CONV){
> -params = (ConvolutionalParams
> *)conv_network->layers[layer].params;
> -pad += params->kernel_size >> 1;
> -}
> -}
> -
> -return pad;
> -}
> -
> -static DNNReturnType add_pad_op(TFModel *tf_model, TF_Operation
> **cur_op, const int32_t pad)
> +static DNNReturnType add_pad_layer(TFModel *tf_model, TF_Operation
> **cur_op,
> +  LayerPadParams
> *params, const int layer)
>  {
>  TF_Operation *op;
>  TF_Tensor *tensor;
> @@ -374,14 +360,21 @@ static DNNReturnType add_pad_op(TFModel
> *tf_model, TF_Operation **cur_op, const
> 
>  input.index = 0;
> 
> -op_desc = TF_NewOperation(tf_model->graph, "Const", "pads");
> +char name_buffer[NAME_BUFFER_SIZE];

just found there is a compile warning for this variable, will fix it and send 
out v2 patch, thanks.

> +snprintf(name_buffer, NAME_BUFFER_SIZE, "pad%d", layer);
___
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] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe

2019-08-15 Thread James Almer
On 8/15/2019 3:55 PM, Tomas Härdin wrote:
> tor 2019-08-15 klockan 14:20 -0300 skrev James Almer:
>> On 8/15/2019 2:13 PM, Tomas Härdin wrote:
>>> tor 2019-08-15 klockan 12:54 -0300 skrev James Almer:
 On 8/15/2019 11:34 AM, Tomas Härdin wrote:
> Aside: what is -keyint_min for exactly? It seems to be used as a max in
> msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as
> minimum and maximum respectively.. Feels like the relevant encoders
> need to be brought into line (assuming it doesn't break user scripts)
 Afaik, keyint_min is minimum interval/distance between keyframes. So
 yes, if it's being used as max distance then it's wrong, as that's what
 gop_size is for.
>>> Make sense. But why is gop_size < keyint_min by default then? 12 vs 25.
>> I have no idea.
>>
>>> I think the x264 people have complained multiple times about the
>>> default gop_size setting even
>> The libx264 wrapper changes both to -1 in libavcodec, which means x264's
>> own defaults will be used instead. So if they complained then i guess it
>> was changed accordingly.
> Didn't know encoders could have their own defaults. Updated patch
> attached. I added some sanity checks on -keyint_min and -g as well.
> 
> I couldn't find rl's contact information, hopefully they are subscribed
> to this list.
> 
> /Tomas
> 
> 
> 0001-cinepakenc-When-rd_frame-doesn-t-use-MC-mark-pkt-as-.patch
> 
> From 49d7ac02d35acbf3e30555eef215ae3a15c06684 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= 
> Date: Thu, 15 Aug 2019 16:20:23 +0200
> Subject: [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as
>  keyframe and reset curframe
> 
> This allows the encoder to make smarter choices for future frames,
> while still keeping keyframe distances within [keyint_min, gop_size].
> Defaults and checks prevent nonsensical keyint_min / gop_size combinations.
> ---
>  libavcodec/cinepakenc.c | 54 +
>  1 file changed, 44 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/cinepakenc.c b/libavcodec/cinepakenc.c
> index 93917fafe8..396d3e5273 100644
> --- a/libavcodec/cinepakenc.c
> +++ b/libavcodec/cinepakenc.c
> @@ -112,7 +112,7 @@ typedef struct CinepakEncContext {
>  enum AVPixelFormat pix_fmt;
>  int w, h;
>  int frame_buf_size;
> -int curframe, keyint;
> +int curframe, keyint_min, keyint_max;
>  AVLFG randctx;
>  uint64_t lambda;
>  int *codebook_input;
> @@ -168,6 +168,16 @@ static av_cold int cinepak_encode_init(AVCodecContext 
> *avctx)
>  return AVERROR(EINVAL);
>  }
>  
> +if (avctx->keyint_min <= 0) {
> +av_log(avctx, AV_LOG_ERROR, "-keyint_min must be >= 1\n");
> +return AVERROR(EINVAL);
> +}
> +
> +if (avctx->keyint_min > avctx->gop_size) {
> +av_log(avctx, AV_LOG_ERROR, "-keyint_min > -g is not allowed. Did 
> you mean -g?\n");

Don't use option names in error messages, and especially not with CLI
formatting. Use keyint_min and gop_size instead, which are the names of
the AVCodecContext fields.

And the suggestion at the end is IMO not really needed.

> +return AVERROR(EINVAL);
> +}
> +
>  if (!(s->last_frame = av_frame_alloc()))
>  return AVERROR(ENOMEM);
>  if (!(s->best_frame = av_frame_alloc()))
> @@ -213,7 +223,8 @@ static av_cold int cinepak_encode_init(AVCodecContext 
> *avctx)
>  s->h  = avctx->height;
>  s->frame_buf_size = frame_buf_size;
>  s->curframe   = 0;
> -s->keyint = avctx->keyint_min;
> +s->keyint_min = avctx->keyint_min;
> +s->keyint_max = avctx->gop_size;
>  s->pix_fmt= avctx->pix_fmt;
>  
>  // set up AVFrames
> @@ -871,7 +882,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, 
> int keyframe,
>  uint8_t *last_data[4], int last_linesize[4],
>  uint8_t *data[4], int linesize[4],
>  uint8_t *scratch_data[4], int scratch_linesize[4],
> -unsigned char *buf, int64_t *best_score)
> +unsigned char *buf, int64_t *best_score, CinepakMode 
> *best_mode)
>  {
>  int64_t score = 0;
>  int best_size = 0;
> @@ -970,6 +981,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, 
> int keyframe,
>  
>  if (best_size == 0 || score < *best_score) {
>  *best_score = score;
> +*best_mode = mode;
>  best_size = encode_mode(s, h,
>  scratch_data, scratch_linesize,
>  last_data, last_linesize, &info,
> @@ -1000,13 +1012,15 @@ static int write_cvid_header(CinepakEncContext *s, 
> unsigned char *buf,
>  }
>  
>  static int rd_frame(CinepakEncContext *s, const AVFrame *frame,
> -int isakeyframe, unsigned char *buf, int buf_size)
> +int isakey

[FFmpeg-devel] [PATCH V2 1/2] libavfilter/dnn/dnn_backend_tf: fix typo that variable uninitialized.

2019-08-15 Thread Guo, Yejun
if it is initialized randomly, the tensorflow lib will report
error message such as:
Attempt to add output -7920 of depth_to_space4 not in range [0, 1) to node with 
type Identity

Signed-off-by: Guo, Yejun 
---
 libavfilter/dnn/dnn_backend_tf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/dnn/dnn_backend_tf.c b/libavfilter/dnn/dnn_backend_tf.c
index ba959ae..ca7434a 100644
--- a/libavfilter/dnn/dnn_backend_tf.c
+++ b/libavfilter/dnn/dnn_backend_tf.c
@@ -490,6 +490,7 @@ static DNNReturnType load_native_model(TFModel *tf_model, 
const char *model_file
 
 op_desc = TF_NewOperation(tf_model->graph, "Identity", "y");
 input.oper = op;
+input.index = 0;
 TF_AddInput(op_desc, input);
 TF_FinishOperation(op_desc, tf_model->status);
 if (TF_GetCode(tf_model->status) != TF_OK){
-- 
2.7.4

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

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

[FFmpeg-devel] [PATCH V2 2/2] libavfilter/dnn/dnn_backend_tf: add tf.pad support for tensorflow backend with native model.

2019-08-15 Thread Guo, Yejun
Signed-off-by: Guo, Yejun 
---
 libavfilter/dnn/dnn_backend_tf.c | 48 
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/libavfilter/dnn/dnn_backend_tf.c b/libavfilter/dnn/dnn_backend_tf.c
index ca7434a..626fba9 100644
--- a/libavfilter/dnn/dnn_backend_tf.c
+++ b/libavfilter/dnn/dnn_backend_tf.c
@@ -27,6 +27,7 @@
 #include "dnn_backend_native.h"
 #include "libavformat/avio.h"
 #include "libavutil/avassert.h"
+#include "dnn_backend_native_layer_pad.h"
 
 #include 
 
@@ -347,23 +348,8 @@ static DNNReturnType add_depth_to_space_layer(TFModel 
*tf_model, TF_Operation **
 return DNN_SUCCESS;
 }
 
-static int calculate_pad(const ConvolutionalNetwork *conv_network)
-{
-ConvolutionalParams *params;
-int32_t layer;
-int pad = 0;
-
-for (layer = 0; layer < conv_network->layers_num; ++layer){
-if (conv_network->layers[layer].type == CONV){
-params = (ConvolutionalParams *)conv_network->layers[layer].params;
-pad += params->kernel_size >> 1;
-}
-}
-
-return pad;
-}
-
-static DNNReturnType add_pad_op(TFModel *tf_model, TF_Operation **cur_op, 
const int32_t pad)
+static DNNReturnType add_pad_layer(TFModel *tf_model, TF_Operation **cur_op,
+  LayerPadParams *params, const 
int layer)
 {
 TF_Operation *op;
 TF_Tensor *tensor;
@@ -372,16 +358,21 @@ static DNNReturnType add_pad_op(TFModel *tf_model, 
TF_Operation **cur_op, const
 int32_t *pads;
 int64_t pads_shape[] = {4, 2};
 
-input.index = 0;
+char name_buffer[NAME_BUFFER_SIZE];
+snprintf(name_buffer, NAME_BUFFER_SIZE, "pad%d", layer);
 
-op_desc = TF_NewOperation(tf_model->graph, "Const", "pads");
+op_desc = TF_NewOperation(tf_model->graph, "Const", name_buffer);
 TF_SetAttrType(op_desc, "dtype", TF_INT32);
 tensor = TF_AllocateTensor(TF_INT32, pads_shape, 2, 4 * 2 * 
sizeof(int32_t));
 pads = (int32_t *)TF_TensorData(tensor);
-pads[0] = 0;   pads[1] = 0;
-pads[2] = pad; pads[3] = pad;
-pads[4] = pad; pads[5] = pad;
-pads[6] = 0;   pads[7] = 0;
+pads[0] = params->paddings[0][0];
+pads[1] = params->paddings[0][1];
+pads[2] = params->paddings[1][0];
+pads[3] = params->paddings[1][1];
+pads[4] = params->paddings[2][0];
+pads[5] = params->paddings[2][1];
+pads[6] = params->paddings[3][0];
+pads[7] = params->paddings[3][1];
 TF_SetAttrTensor(op_desc, "value", tensor, tf_model->status);
 if (TF_GetCode(tf_model->status) != TF_OK){
 return DNN_ERROR;
@@ -393,6 +384,7 @@ static DNNReturnType add_pad_op(TFModel *tf_model, 
TF_Operation **cur_op, const
 
 op_desc = TF_NewOperation(tf_model->graph, "MirrorPad", "mirror_pad");
 input.oper = *cur_op;
+input.index = 0;
 TF_AddInput(op_desc, input);
 input.oper = op;
 TF_AddInput(op_desc, input);
@@ -418,7 +410,6 @@ static DNNReturnType load_native_model(TFModel *tf_model, 
const char *model_file
 int32_t *transpose_perm;
 int64_t transpose_perm_shape[] = {4};
 int64_t input_shape[] = {1, -1, -1, -1};
-int32_t pad;
 DNNReturnType layer_add_res;
 DNNModel *native_model = NULL;
 ConvolutionalNetwork *conv_network;
@@ -429,7 +420,6 @@ static DNNReturnType load_native_model(TFModel *tf_model, 
const char *model_file
 }
 
 conv_network = (ConvolutionalNetwork *)native_model->model;
-pad = calculate_pad(conv_network);
 tf_model->graph = TF_NewGraph();
 tf_model->status = TF_NewStatus();
 
@@ -448,10 +438,6 @@ static DNNReturnType load_native_model(TFModel *tf_model, 
const char *model_file
 CLEANUP_ON_ERROR(tf_model);
 }
 
-if (add_pad_op(tf_model, &op, pad) != DNN_SUCCESS){
-CLEANUP_ON_ERROR(tf_model);
-}
-
 op_desc = TF_NewOperation(tf_model->graph, "Const", "transpose_perm");
 TF_SetAttrType(op_desc, "dtype", TF_INT32);
 tensor = TF_AllocateTensor(TF_INT32, transpose_perm_shape, 1, 4 * 
sizeof(int32_t));
@@ -479,6 +465,10 @@ static DNNReturnType load_native_model(TFModel *tf_model, 
const char *model_file
 layer_add_res = add_depth_to_space_layer(tf_model, &op,
  (DepthToSpaceParams 
*)conv_network->layers[layer].params, layer);
 break;
+case MIRROR_PAD:
+layer_add_res = add_pad_layer(tf_model, &op,
+  (LayerPadParams 
*)conv_network->layers[layer].params, layer);
+break;
 default:
 CLEANUP_ON_ERROR(tf_model);
 }
-- 
2.7.4

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

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

[FFmpeg-devel] [PATCH 04/11] avformat/utils: Try to reuse AvBufferRef

2019-08-15 Thread Andreas Rheinhardt
In case the parser flag PARSER_FLAG_COMPLETE_FRAMES is set, it is common
that every input packet generates exactly one output packet whose
underlying data coincides with the data of the input packet. If in this
situation the input packet buffer is reference counted (as it usually is),
the current code used to create a new reference to the underlying AVBuffer
for the output packet. The old reference would be discarded afterwards
when the input packet gets unreferenced. This has been changed: If it is
known that no more output packets result from the current input packet,
the input packet's reference is directly transferred to the last output
packet, thus saving one malloc+free.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/utils.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index b57e680089..64516a6a81 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1478,10 +1478,17 @@ static int parse_packet(AVFormatContext *s, AVPacket 
*pkt, int stream_index)
  * to data in it and not in the parser's internal buffer. */
 /* XXX: Ensure this is the case with all parsers when 
st->parser->flags
  * is PARSER_FLAG_COMPLETE_FRAMES and check for that instead? */
-out_pkt.buf = av_buffer_ref(pkt->buf);
-if (!out_pkt.buf) {
-ret = AVERROR(ENOMEM);
-goto fail;
+if (size > 0) {
+out_pkt.buf = av_buffer_ref(pkt->buf);
+if (!out_pkt.buf) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+} else {
+/* flush_pkt does not have buf set, so size == 0 means
+ * that this is the last iteration of this loop. */
+out_pkt.buf = pkt->buf;
+pkt->buf= NULL;
 }
 } else {
 ret = av_packet_make_refcounted(&out_pkt);
-- 
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 02/11] avformat/matroskadec: Remove unnecessary check

2019-08-15 Thread Andreas Rheinhardt
870e7552 introduced validating the lace sizes when they are parsed and
removed the old check; yet when merging this libav commit in 6902c3ac,
the old check for whether the frame extends beyond the frame has been kept.
It is unnecessary and has been removed.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 25f26da074..5901dbd221 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2996,10 +2996,10 @@ static void matroska_clear_queue(MatroskaDemuxContext 
*matroska)
 }
 
 static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
-int *buf_size, int type,
+int size, int type,
 uint32_t **lace_buf, int *laces)
 {
-int n, size = *buf_size;
+int n;
 uint8_t *data = *buf;
 uint32_t *lace_size;
 
@@ -3095,7 +3095,6 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 }
 
 *buf  = data;
-*buf_size = size;
 
 return 0;
 }
@@ -3589,7 +3588,7 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 }
 }
 
-res = matroska_parse_laces(matroska, &data, &size, (flags & 0x06) >> 1,
+res = matroska_parse_laces(matroska, &data, size, (flags & 0x06) >> 1,
&lace_size, &laces);
 
 if (res)
@@ -3613,11 +3612,6 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 for (n = 0; n < laces; n++) {
 int64_t lace_duration = block_duration*(n+1) / laces - 
block_duration*n / laces;
 
-if (lace_size[n] > size) {
-av_log(matroska->ctx, AV_LOG_ERROR, "Invalid packet size\n");
-break;
-}
-
 if ((st->codecpar->codec_id == AV_CODEC_ID_RA_288 ||
  st->codecpar->codec_id == AV_CODEC_ID_COOK   ||
  st->codecpar->codec_id == AV_CODEC_ID_SIPR   ||
@@ -3649,7 +3643,6 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 if (timecode != AV_NOPTS_VALUE)
 timecode = lace_duration ? timecode + lace_duration : 
AV_NOPTS_VALUE;
 data += lace_size[n];
-size -= lace_size[n];
 }
 
 end:
-- 
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 08/11] avcodec/h264_parser: Reindent after previous commits

2019-08-15 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_parser.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 188ba41c0b..669a9a5ff7 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -306,10 +306,10 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 break;
 }
 
-av_fast_padded_malloc(&rbsp->rbsp_buffer, &rbsp->rbsp_buffer_alloc_size, 
src_length);
-if (!rbsp->rbsp_buffer)
-return AVERROR(ENOMEM);
-rbsp->rbsp_buffer_size = 0;
+av_fast_padded_malloc(&rbsp->rbsp_buffer, 
&rbsp->rbsp_buffer_alloc_size, src_length);
+if (!rbsp->rbsp_buffer)
+return AVERROR(ENOMEM);
+rbsp->rbsp_buffer_size = 0;
 
 consumed = ff_h2645_extract_rbsp(buf + buf_index, src_length, rbsp, 
&nal, 1);
 if (consumed < 0)
@@ -364,31 +364,31 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 }
 
 if (p->ps.pps != (const PPS*)p->ps.pps_list[pps_id]->data) {
-av_buffer_unref(&p->ps.pps_ref);
-p->ps.pps_ref = av_buffer_ref(p->ps.pps_list[pps_id]);
-if (!p->ps.pps_ref) {
-p->ps.pps = NULL;
-goto unref_sps;
-}
-p->ps.pps = (const PPS*)p->ps.pps_ref->data;
+av_buffer_unref(&p->ps.pps_ref);
+p->ps.pps_ref = av_buffer_ref(p->ps.pps_list[pps_id]);
+if (!p->ps.pps_ref) {
+p->ps.pps = NULL;
+goto unref_sps;
+}
+p->ps.pps = (const PPS*)p->ps.pps_ref->data;
 }
 
 if (!p->ps.sps_list[p->ps.pps->sps_id]) {
 av_log(avctx, AV_LOG_ERROR,
"non-existing SPS %u referenced\n", p->ps.pps->sps_id);
 unref_sps:
-av_buffer_unref(&p->ps.sps_ref);
-p->ps.sps = NULL;
+av_buffer_unref(&p->ps.sps_ref);
+p->ps.sps = NULL;
 goto fail;
 }
 if (p->ps.sps != (const 
SPS*)p->ps.sps_list[p->ps.pps->sps_id]->data) {
-av_buffer_unref(&p->ps.sps_ref);
-p->ps.sps_ref = av_buffer_ref(p->ps.sps_list[p->ps.pps->sps_id]);
-if (!p->ps.sps_ref) {
-p->ps.sps = NULL;
-goto fail;
-}
-p->ps.sps = (const SPS*)p->ps.sps_ref->data;
+av_buffer_unref(&p->ps.sps_ref);
+p->ps.sps_ref = 
av_buffer_ref(p->ps.sps_list[p->ps.pps->sps_id]);
+if (!p->ps.sps_ref) {
+p->ps.sps = NULL;
+goto fail;
+}
+p->ps.sps = (const SPS*)p->ps.sps_ref->data;
 }
 
 sps = p->ps.sps;
-- 
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 01/11] avformat/matroskadec: Simplify control flow of parsing laces

2019-08-15 Thread Andreas Rheinhardt
Up until now, when an error happened in one of the inner loops in
matroska_parse_laces, a variable designated for the return value has
been set to an error value and break has been used to exit the
current loop/case. This was done so that the end of matroska_parse_laces
is reached, because said function allocates memory which is later used
and freed in the calling function and passed at the end of
matroska_parse_laces.
But there is actually no reason to pass it at the end: This commit sets
the address of the newly allocated memory as soon as it is known and
exits the subfunction as soon as an error is encountered.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 30 --
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4e20f15792..25f26da074 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2999,7 +2999,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 int *buf_size, int type,
 uint32_t **lace_buf, int *laces)
 {
-int res = 0, n, size = *buf_size;
+int n, size = *buf_size;
 uint8_t *data = *buf;
 uint32_t *lace_size;
 
@@ -3018,6 +3018,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 data += 1;
 size -= 1;
 lace_size = av_malloc_array(*laces, sizeof(*lace_size));
+*lace_buf = lace_size;
 if (!lace_size)
 return AVERROR(ENOMEM);
 
@@ -3026,13 +3027,12 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 {
 uint8_t temp;
 uint32_t total = 0;
-for (n = 0; res == 0 && n < *laces - 1; n++) {
+for (n = 0; n < *laces - 1; n++) {
 lace_size[n] = 0;
 
 while (1) {
 if (size <= total) {
-res = AVERROR_INVALIDDATA;
-break;
+return AVERROR_INVALIDDATA;
 }
 temp  = *data;
 total+= temp;
@@ -3044,8 +3044,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 }
 }
 if (size <= total) {
-res = AVERROR_INVALIDDATA;
-break;
+return AVERROR_INVALIDDATA;
 }
 
 lace_size[n] = size - total;
@@ -3054,8 +3053,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 
 case 0x2: /* fixed-size lacing */
 if (size % (*laces)) {
-res = AVERROR_INVALIDDATA;
-break;
+return AVERROR_INVALIDDATA;
 }
 for (n = 0; n < *laces; n++)
 lace_size[n] = size / *laces;
@@ -3069,21 +3067,19 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 if (n < 0 || num > INT_MAX) {
 av_log(matroska->ctx, AV_LOG_INFO,
"EBML block data error\n");
-res = n<0 ? n : AVERROR_INVALIDDATA;
-break;
+return n<0 ? n : AVERROR_INVALIDDATA;
 }
 data += n;
 size -= n;
 total = lace_size[0] = num;
-for (n = 1; res == 0 && n < *laces - 1; n++) {
+for (n = 1; n < *laces - 1; n++) {
 int64_t snum;
 int r;
 r = matroska_ebmlnum_sint(matroska, data, size, &snum);
 if (r < 0 || lace_size[n - 1] + snum > (uint64_t)INT_MAX) {
 av_log(matroska->ctx, AV_LOG_INFO,
"EBML block data error\n");
-res = r<0 ? r : AVERROR_INVALIDDATA;
-break;
+return r<0 ? r : AVERROR_INVALIDDATA;
 }
 data+= r;
 size-= r;
@@ -3091,8 +3087,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 total   += lace_size[n];
 }
 if (size <= total) {
-res = AVERROR_INVALIDDATA;
-break;
+return AVERROR_INVALIDDATA;
 }
 lace_size[*laces - 1] = size - total;
 break;
@@ -3100,10 +3095,9 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 }
 
 *buf  = data;
-*lace_buf = lace_size;
 *buf_size = size;
 
-return res;
+return 0;
 }
 
 static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
@@ -3537,7 +3531,7 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 int res = 0;
 AVStream *st;
 int16_t block_time;
-uint32_t *lace_size = NULL;
+uint32_t *lace_size;
 int n, flags, laces = 0;
 uint64_t num;
 int trust_default_duration = 1;
-- 
2.21.0

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

[FFmpeg-devel] [PATCH 10/11] avcodec/h264_parser: Don't keep full DSP Context

2019-08-15 Thread Andreas Rheinhardt
We only need the function to search for start codes from it.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_parser.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 604cddada5..68eb5939db 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -50,10 +50,10 @@
 typedef struct H264ParseContext {
 ParseContext pc;
 H264ParamSets ps;
-H264DSPContext h264dsp;
 H264POCContext poc;
 H264SEIContext sei;
 H2645RBSP rbsp;
+int (*startcode_find_candidate)(const uint8_t *buf, int size);
 int is_avc;
 int nal_length_size;
 int got_first;
@@ -97,7 +97,7 @@ static int h264_find_frame_end(H264ParseContext *p, const 
uint8_t *buf,
 }
 
 if (state == 7) {
-i += p->h264dsp.startcode_find_candidate(buf + i, next_avc - i);
+i += p->startcode_find_candidate(buf + i, next_avc - i);
 if (i < next_avc)
 state = 2;
 } else if (state <= 2) {
@@ -705,10 +705,13 @@ static void h264_close(AVCodecParserContext *s)
 static av_cold int init(AVCodecParserContext *s)
 {
 H264ParseContext *p = s->priv_data;
+H264DSPContext h264dsp;
+
+ff_h264dsp_init(&h264dsp, 8, 1);
+p->startcode_find_candidate = h264dsp.startcode_find_candidate;
 
 p->reference_dts = AV_NOPTS_VALUE;
 p->last_frame_num = INT_MAX;
-ff_h264dsp_init(&p->h264dsp, 8, 1);
 return 0;
 }
 
-- 
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 06/11] avcodec/h264_parser: Use smaller buffer

2019-08-15 Thread Andreas Rheinhardt
The H.264 parser currently allocates a buffer of the size of the input
frame for every input frame to store the unescaped RBSP of a NAL unit
(it is actually only used if a 0x03 escape is encountered at all).
For performance reasons only a small part of slices is actually
unescaped; and given that an unescaped NAL unit is only used once (if
ever), the really needed size of the buffer is the maximum of the
possible length of a NAL unit in front of the first slice and the length of
the part of the first slice that is actually unescaped.
For AVC (mp4/Matroska) content, the length of a NAL unit is known, so
that if the NAL units in front of the slices are small (as they usually
are), said maximum is small so that one can reduce memory usage by
allocating less.
For Annex B, this is unfortunately not so, because the code has to
assume the worst wrt the NAL unit sizes (it even assumes that an
access unit delimiter extends until the end of the input buffer).

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_parser.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index c200a2ab8e..5f7aea0c76 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -268,11 +268,6 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 if (!buf_size)
 return 0;
 
-av_fast_padded_malloc(&rbsp->rbsp_buffer, &rbsp->rbsp_buffer_alloc_size, 
buf_size);
-if (!rbsp->rbsp_buffer)
-return AVERROR(ENOMEM);
-rbsp->rbsp_buffer_size = 0;
-
 buf_index = 0;
 next_avc  = p->is_avc ? 0 : buf_size;
 for (;;) {
@@ -310,6 +305,12 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 }
 break;
 }
+
+av_fast_padded_malloc(&rbsp->rbsp_buffer, &rbsp->rbsp_buffer_alloc_size, 
src_length);
+if (!rbsp->rbsp_buffer)
+return AVERROR(ENOMEM);
+rbsp->rbsp_buffer_size = 0;
+
 consumed = ff_h2645_extract_rbsp(buf + buf_index, src_length, rbsp, 
&nal, 1);
 if (consumed < 0)
 break;
-- 
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 07/11] avcodec/h264_parser: Try to avoid (un)referencing

2019-08-15 Thread Andreas Rheinhardt
When a slice is encountered, the H.264 parser up until now always
unreferenced and reset the currently active SPS and PPS; immediately
afterwards, the currently active parameter sets are set which includes
referencing them. Given that it is not uncommon for the active parameter
sets to change only seldomly, most of the time the new active parameter
sets will be the old ones. Therefore this commit checks for this and
only unreferences an active parameter set if it changed.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_parser.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 5f7aea0c76..188ba41c0b 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -363,25 +363,33 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 goto fail;
 }
 
+if (p->ps.pps != (const PPS*)p->ps.pps_list[pps_id]->data) {
 av_buffer_unref(&p->ps.pps_ref);
-av_buffer_unref(&p->ps.sps_ref);
-p->ps.pps = NULL;
-p->ps.sps = NULL;
 p->ps.pps_ref = av_buffer_ref(p->ps.pps_list[pps_id]);
-if (!p->ps.pps_ref)
-goto fail;
+if (!p->ps.pps_ref) {
+p->ps.pps = NULL;
+goto unref_sps;
+}
 p->ps.pps = (const PPS*)p->ps.pps_ref->data;
+}
 
 if (!p->ps.sps_list[p->ps.pps->sps_id]) {
 av_log(avctx, AV_LOG_ERROR,
"non-existing SPS %u referenced\n", p->ps.pps->sps_id);
+unref_sps:
+av_buffer_unref(&p->ps.sps_ref);
+p->ps.sps = NULL;
 goto fail;
 }
-
+if (p->ps.sps != (const 
SPS*)p->ps.sps_list[p->ps.pps->sps_id]->data) {
+av_buffer_unref(&p->ps.sps_ref);
 p->ps.sps_ref = av_buffer_ref(p->ps.sps_list[p->ps.pps->sps_id]);
-if (!p->ps.sps_ref)
+if (!p->ps.sps_ref) {
+p->ps.sps = NULL;
 goto fail;
+}
 p->ps.sps = (const SPS*)p->ps.sps_ref->data;
+}
 
 sps = p->ps.sps;
 
-- 
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 09/11] avcodec/h264_parser: Remove wrong comment

2019-08-15 Thread Andreas Rheinhardt
From section 7.4.3.3 ("Decoded reference picture marking semantics")
of the H.264 specifications:
"The content of the decoded reference picture marking syntax structure
shall be the same in all slice headers of the primary coded picture.
When one or more redundant coded pictures are present, the content of
the decoded reference picture marking syntax structure shall be the same
in all slice headers of a redundant coded picture with a particular value
of redundant_pic_cnt."
And regarding redundant coded pictures:
"[T]he content of the decoded reference picture marking syntax structure
in a redundant coded picture is constrained in the way that the marking
status of reference pictures and the value of frame_num after the decoded
reference picture marking process in clause 8.2.5 must be identical
regardless whether the primary coded picture or any redundant coded
picture of the access unit would be decoded."
So just looking at the first slice header is fine.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_parser.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 669a9a5ff7..604cddada5 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -468,10 +468,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 if (ret < 0)
 goto fail;
 
-/* Continue parsing to check if MMCO_RESET is present.
- * FIXME: MMCO_RESET could appear in non-first slice.
- *Maybe, we should parse all undisposable non-IDR slice of 
this
- *picture until encountering MMCO_RESET in a slice of it. 
*/
+/* Continue parsing to check if MMCO_RESET is present. */
 if (nal.ref_idc && nal.type != H264_NAL_IDR_SLICE) {
 got_reset = scan_mmco_reset(s, &nal.gb, avctx);
 if (got_reset < 0)
-- 
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 05/11] avcodec/h264_parser: Reuse the RBSP buffer

2019-08-15 Thread Andreas Rheinhardt
Up until now, the H.264 parser has allocated a new buffer for every
frame in order to store the unescaped RBSP in case the part of the RBSP
that will be unescaped contains 0x03 escape bytes. This is expensive
and this commit changes this: The buffer is now kept and reused.

For an AVC file with an average framesize of 15304 B (without in-band
parameter sets etc.) and 132044 frames (131072 of which ended up in the
results) this reduced the average time used by the H.264 parser from
87708 decicycles (excluding about 1100-1200 skips in each iteration)
to 16963 decicycles (excluding about 10-15 skips in each iteration).
The test has been iterated 10 times.

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

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 5f9a9c46ef..c200a2ab8e 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -53,6 +53,7 @@ typedef struct H264ParseContext {
 H264DSPContext h264dsp;
 H264POCContext poc;
 H264SEIContext sei;
+H2645RBSP rbsp;
 int is_avc;
 int nal_length_size;
 int got_first;
@@ -246,7 +247,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
   const uint8_t * const buf, int buf_size)
 {
 H264ParseContext *p = s->priv_data;
-H2645RBSP rbsp = { NULL };
+H2645RBSP *rbsp = &p->rbsp;
 H2645NAL nal = { NULL };
 int buf_index, next_avc;
 unsigned int pps_id;
@@ -267,9 +268,10 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 if (!buf_size)
 return 0;
 
-av_fast_padded_malloc(&rbsp.rbsp_buffer, &rbsp.rbsp_buffer_alloc_size, 
buf_size);
-if (!rbsp.rbsp_buffer)
+av_fast_padded_malloc(&rbsp->rbsp_buffer, &rbsp->rbsp_buffer_alloc_size, 
buf_size);
+if (!rbsp->rbsp_buffer)
 return AVERROR(ENOMEM);
+rbsp->rbsp_buffer_size = 0;
 
 buf_index = 0;
 next_avc  = p->is_avc ? 0 : buf_size;
@@ -308,7 +310,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 }
 break;
 }
-consumed = ff_h2645_extract_rbsp(buf + buf_index, src_length, &rbsp, 
&nal, 1);
+consumed = ff_h2645_extract_rbsp(buf + buf_index, src_length, rbsp, 
&nal, 1);
 if (consumed < 0)
 break;
 
@@ -554,18 +556,15 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 p->last_frame_num = p->poc.frame_num;
 }
 
-av_freep(&rbsp.rbsp_buffer);
 return 0; /* no need to evaluate the rest */
 }
 }
 if (q264) {
-av_freep(&rbsp.rbsp_buffer);
 return 0;
 }
 /* didn't find a picture! */
 av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size 
%d\n", buf_size);
 fail:
-av_freep(&rbsp.rbsp_buffer);
 return -1;
 }
 
@@ -690,6 +689,8 @@ static void h264_close(AVCodecParserContext *s)
 ParseContext *pc = &p->pc;
 
 av_freep(&pc->buffer);
+av_freep(&p->rbsp.rbsp_buffer);
+p->rbsp.rbsp_buffer_alloc_size = 0;
 
 ff_h264_sei_uninit(&p->sei);
 ff_h264_ps_uninit(&p->ps);
-- 
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 03/11] avformat/matroskadec: Improve frame size parsing error messages

2019-08-15 Thread Andreas Rheinhardt
When parsing the sizes of the frames in a lace fails, sometimes no
error message was raised (e.g. when using xiph or fixed-size lacing).
Only EBML lacing generated error messages (which were wrongly declared
as AV_LOG_INFO), but even here not all errors resulted in an error
message. So add a generic error message to catch them all.

Moreover, if parsing one of the EBML numbers fails, ebml_read_num already
emits its own error messages, so that all that is needed is a generic error
message to indicate that this happened during parsing the sizes of the
frames in a block; in other words, the error messages specific to
parsing EBML lace numbers can be and have been removed.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 5901dbd221..c9fe795dcd 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3064,11 +3064,10 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 uint64_t num;
 uint64_t total;
 n = matroska_ebmlnum_uint(matroska, data, size, &num);
-if (n < 0 || num > INT_MAX) {
-av_log(matroska->ctx, AV_LOG_INFO,
-   "EBML block data error\n");
-return n<0 ? n : AVERROR_INVALIDDATA;
-}
+if (n < 0)
+return n;
+if (num > INT_MAX)
+return AVERROR_INVALIDDATA;
 data += n;
 size -= n;
 total = lace_size[0] = num;
@@ -3076,11 +3075,10 @@ static int matroska_parse_laces(MatroskaDemuxContext 
*matroska, uint8_t **buf,
 int64_t snum;
 int r;
 r = matroska_ebmlnum_sint(matroska, data, size, &snum);
-if (r < 0 || lace_size[n - 1] + snum > (uint64_t)INT_MAX) {
-av_log(matroska->ctx, AV_LOG_INFO,
-   "EBML block data error\n");
-return r<0 ? r : AVERROR_INVALIDDATA;
-}
+if (r < 0)
+return r;
+if (lace_size[n - 1] + snum > (uint64_t)INT_MAX)
+return AVERROR_INVALIDDATA;
 data+= r;
 size-= r;
 lace_size[n] = lace_size[n - 1] + snum;
@@ -3591,8 +3589,10 @@ static int matroska_parse_block(MatroskaDemuxContext 
*matroska, AVBufferRef *buf
 res = matroska_parse_laces(matroska, &data, size, (flags & 0x06) >> 1,
&lace_size, &laces);
 
-if (res)
+if (res < 0) {
+av_log(matroska->ctx, AV_LOG_ERROR, "Error parsing frame sizes.\n");
 goto end;
+}
 
 if (track->audio.samplerate == 8000) {
 // If this is needed for more codecs, then add them here
-- 
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 11/11] avformat/internal: Remove packet for extract_extradata

2019-08-15 Thread Andreas Rheinhardt
The effective lifetime of the packet does not extend beyond the
extract_extradata in libavformat/utils.c, so the packet can simply be
put on the stack there. This allows to remove the allocation and the
corresponding frees.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/internal.h |  1 -
 libavformat/utils.c| 10 +-
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index d6a039c497..2574d2da0c 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -182,7 +182,6 @@ struct AVStreamInternal {
  * supported) */
 struct {
 AVBSFContext *bsf;
-AVPacket *pkt;
 int inited;
 } extract_extradata;
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 64516a6a81..d3cd4b4167 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3491,10 +3491,6 @@ static int extract_extradata_init(AVStream *st)
 if (!ret)
 goto finish;
 
-sti->extract_extradata.pkt = av_packet_alloc();
-if (!sti->extract_extradata.pkt)
-return AVERROR(ENOMEM);
-
 ret = av_bsf_alloc(f, &sti->extract_extradata.bsf);
 if (ret < 0)
 goto fail;
@@ -3516,14 +3512,13 @@ finish:
 return 0;
 fail:
 av_bsf_free(&sti->extract_extradata.bsf);
-av_packet_free(&sti->extract_extradata.pkt);
 return ret;
 }
 
 static int extract_extradata(AVStream *st, AVPacket *pkt)
 {
 AVStreamInternal *sti = st->internal;
-AVPacket *pkt_ref;
+AVPacket pkt1, *pkt_ref = &pkt1;
 int ret;
 
 if (!sti->extract_extradata.inited) {
@@ -3535,7 +3530,6 @@ static int extract_extradata(AVStream *st, AVPacket *pkt)
 if (sti->extract_extradata.inited && !sti->extract_extradata.bsf)
 return 0;
 
-pkt_ref = sti->extract_extradata.pkt;
 ret = av_packet_ref(pkt_ref, pkt);
 if (ret < 0)
 return ret;
@@ -4171,7 +4165,6 @@ find_stream_info_err:
 avcodec_close(ic->streams[i]->internal->avctx);
 av_freep(&ic->streams[i]->info);
 av_bsf_free(&ic->streams[i]->internal->extract_extradata.bsf);
-av_packet_free(&ic->streams[i]->internal->extract_extradata.pkt);
 }
 if (ic->pb)
 av_log(ic, AV_LOG_DEBUG, "After avformat_find_stream_info() pos: 
%"PRId64" bytes read:%"PRId64" seeks:%d frames:%d\n",
@@ -4369,7 +4362,6 @@ static void free_stream(AVStream **pst)
 }
 av_freep(&st->internal->priv_pts);
 av_bsf_free(&st->internal->extract_extradata.bsf);
-av_packet_free(&st->internal->extract_extradata.pkt);
 }
 av_freep(&st->internal);
 
-- 
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".

Re: [FFmpeg-devel] [PATCH 05/11] avcodec/h264_parser: Reuse the RBSP buffer

2019-08-15 Thread Kieran Kunhya
On Fri, 16 Aug 2019 at 04:20, Andreas Rheinhardt <
andreas.rheinha...@gmail.com> wrote:

> Up until now, the H.264 parser has allocated a new buffer for every
> frame in order to store the unescaped RBSP in case the part of the RBSP
> that will be unescaped contains 0x03 escape bytes. This is expensive
> and this commit changes this: The buffer is now kept and reused.
>
> For an AVC file with an average framesize of 15304 B (without in-band
> parameter sets etc.) and 132044 frames (131072 of which ended up in the
> results) this reduced the average time used by the H.264 parser from
> 87708 decicycles (excluding about 1100-1200 skips in each iteration)
> to 16963 decicycles (excluding about 10-15 skips in each iteration).
> The test has been iterated 10 times.
>

If I understand correctly, this patch means if you have a large frame (or
large NAL) you are stuck with the large allocation of memory until the
decoder is closed.
Not sure if you have read the discussion here
https://patchwork.ffmpeg.org/patch/5631/

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

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

Re: [FFmpeg-devel] [PATCH 05/11] avcodec/h264_parser: Reuse the RBSP buffer

2019-08-15 Thread Andreas Rheinhardt
Kieran Kunhya:
> On Fri, 16 Aug 2019 at 04:20, Andreas Rheinhardt <
> andreas.rheinha...@gmail.com> wrote:
> 
>> Up until now, the H.264 parser has allocated a new buffer for every
>> frame in order to store the unescaped RBSP in case the part of the RBSP
>> that will be unescaped contains 0x03 escape bytes. This is expensive
>> and this commit changes this: The buffer is now kept and reused.
>>
>> For an AVC file with an average framesize of 15304 B (without in-band
>> parameter sets etc.) and 132044 frames (131072 of which ended up in the
>> results) this reduced the average time used by the H.264 parser from
>> 87708 decicycles (excluding about 1100-1200 skips in each iteration)
>> to 16963 decicycles (excluding about 10-15 skips in each iteration).
>> The test has been iterated 10 times.
>>
> 
> If I understand correctly, this patch means if you have a large frame (or
> large NAL) you are stuck with the large allocation of memory until the
> decoder is closed.
> Not sure if you have read the discussion here
> https://patchwork.ffmpeg.org/patch/5631/
> 
> Kieran
> 
I am aware of said discussion; and also of your solution [1] to it. It
actually does exactly the same as I propose for the parser: It keeps a
single buffer that does not shrink. Given that this is used for the
decoders (and for cbs_h2645), I didn't deem this to be problematic.
(There is clearly no quadratic memory usage and what Derek warns about
(with huge NALs at specific positions) is a no issue for both.)

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/219100.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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