Re: [FFmpeg-devel] [PATCH v5 2/3] ibavfilter/vf_cover_rect.c: change the cover_rect function for support cover frame can be changed

2019-06-16 Thread Michael Niedermayer
On Sat, Jun 15, 2019 at 12:48:25AM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavfilter/vf_cover_rect.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> index 898debf..b66c40b 100644
> --- a/libavfilter/vf_cover_rect.c
> +++ b/libavfilter/vf_cover_rect.c
> @@ -71,24 +71,25 @@ static int config_input(AVFilterLink *inlink)
>  return 0;
>  }
>  
> -static void cover_rect(CoverContext *cover, AVFrame *in, int offx, int offy)
> +static void cover_rect(AVFrame *cover_frame, AVFrame *in, int offx, int offy)
>  {
>  int x, y, p;
>  
>  for (p = 0; p < 3; p++) {
>  uint8_t *data = in->data[p] + (offx>>!!p) + (offy>>!!p) * 
> in->linesize[p];
> -const uint8_t *src = cover->cover_frame->data[p];
> -int w = AV_CEIL_RSHIFT(cover->cover_frame->width , !!p);
> -int h = AV_CEIL_RSHIFT(cover->cover_frame->height, !!p);
> +const uint8_t *src = cover_frame->data[p];
> +int w = AV_CEIL_RSHIFT(cover_frame->width , !!p);
> +int h = AV_CEIL_RSHIFT(cover_frame->height, !!p);
>  for (y = 0; y < h; y++) {
>  for (x = 0; x < w; x++) {
>  data[x] = src[x];
>  }
>  data += in->linesize[p];
> -src += cover->cover_frame->linesize[p];
> +src += cover_frame->linesize[p];
>  }
>  }

>  }
> +
>  static void blur(CoverContext *cover, AVFrame *in, int offx, int offy)
>  {
>  int x, y, p;
> @@ -139,6 +140,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  AVDictionaryEntry *ex, *ey, *ew, *eh;
>  int x = -1, y = -1, w = -1, h = -1;
>  char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr = NULL;
> +AVFrame *cover_frame = NULL;
>  
>  ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL, AV_DICT_MATCH_CASE);
>  ey = av_dict_get(in->metadata, "lavfi.rect.y", NULL, AV_DICT_MATCH_CASE);
> @@ -174,6 +176,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  if (cover->cover_frame) {
>  if (w != cover->cover_frame->width || h != 
> cover->cover_frame->height)
>  return AVERROR(EINVAL);
> +cover_frame = cover->cover_frame;
>  }
>  
>  cover->width  = w;
> @@ -186,8 +189,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  
>  if (cover->mode == MODE_BLUR) {
>  blur (cover, in, x, y);

> -} else {
> -cover_rect(cover, in, x, y);
> +} else if (cover_frame) {
> +cover_rect(cover_frame, in, x, y);

replacing a field from the context by a local variable cannot make a field
null that was not before


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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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 v4] libavfilter/vf_cover_rect: support for cover image with more pixel format and different width and height

2019-06-16 Thread Michael Niedermayer
On Sun, Jun 16, 2019 at 07:11:27AM +0800, Lance Wang wrote:
> On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer 
> wrote:
> 
> > On Fri, Jun 14, 2019 at 11:52:47PM +0800, Lance Wang wrote:
> > > On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer
> > 
> > > wrote:
[...]
> >
> > >
> > >
> > > >
> > > >
> > > > > +if (!cover->match_frame || (w != cover->match_frame->width
> > || h
> > > > != cover->match_frame->height
> > > > > +|| in_format != cover->match_frame->format)) {
> > > > > +if (cover->match_frame)
> > > > > +av_freep(&cover->match_frame->data[0]);
> > > > > +else if (!(cover->match_frame = av_frame_alloc()))
> > > > > +return AVERROR(ENOMEM);
> > > > > +
> > > >
> > > > > +if ((ret = ff_scale_image(cover->match_frame->data,
> > > > cover->match_frame->linesize,
> > > > > +w, h, in_format,
> > cover->cover_frame->data,
> > > > cover->cover_frame->linesize,
> > > > > +cover->cover_frame->width,
> > > > cover->cover_frame->height,
> > > > > +cover->cover_frame->format, ctx)) < 0)
> > > > > +return AVERROR(ENOMEM);
> > > >
> > > > This sort of reimplements the scale filter and it
> > > > doesnt consider some parameters like AVCOL_RANGE*
> > > >
> > >
> > > the ff_scale_image is implemented and used by other alike place,  I try
> > to
> > > reuse the function anyway.
> > > If we need other parameters for scale, we may improve the function later,
> > > now it's OK for my testing as
> > > the cover image is logo mostly.
> >
> > I think one could argue that the scale filter should be used for converting
> > this way the cover image would become one externally vissible input. (which
> > then subsequently also could change over time and not just be a static
> > image ...)
> >
> >
> For now, the code is simple and prefer to use it.  How about to improve all
> other function which use ff_scale_image in future.

if you add no new ff_scale_image(), sure theres no need to clean it up
but if you add more bad code, no thats not ok IMO. 

Thanks

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


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 v5 1/3] doc/filters.texi: Fix the confusing description for find_rect and cover_rect command

2019-06-16 Thread Michael Niedermayer
On Sat, Jun 15, 2019 at 12:48:24AM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  doc/filters.texi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index ec1c7c7..4d48068 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -10166,7 +10166,7 @@ Specifies the rectangle in which to search.
>  
>  @itemize
>  @item
> -Generate a representative palette of a given video using @command{ffmpeg}:
> +Cover a rectangular object by the supplied image of a given video using 
> @command{ffmpeg}:
>  @example
>  ffmpeg -i file.ts -vf find_rect=newref.pgm,cover_rect=cover.jpg:mode=cover 
> new.mkv
>  @end example
> @@ -10200,7 +10200,7 @@ Default value is @var{blur}.
>  
>  @itemize
>  @item
> -Generate a representative palette of a given video using @command{ffmpeg}:
> +Cover a rectangular object by the supplied image of a given video using 
> @command{ffmpeg}:
>  @example
>  ffmpeg -i file.ts -vf find_rect=newref.pgm,cover_rect=cover.jpg:mode=cover 
> new.mkv
>  @end example

will apply

thanks

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

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


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

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

Re: [FFmpeg-devel] [PATCH V1 3/3] lavfi/af_asetnsamples: Remove the redundant condition check

2019-06-16 Thread Michael Niedermayer
On Sun, Jun 16, 2019 at 01:13:38AM +0800, Jun Zhao wrote:
> From: Jun Zhao 
> 
> Redundant condition: '!A || B' is equivalent to '!A || (A && B)' but
> more clearly.
> 
> Signed-off-by: Jun Zhao 
> ---
>  libavfilter/af_asetnsamples.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libavfilter/af_asetnsamples.c b/libavfilter/af_asetnsamples.c
> index a7e424f..bbc391a 100644
> --- a/libavfilter/af_asetnsamples.c
> +++ b/libavfilter/af_asetnsamples.c
> @@ -67,7 +67,7 @@ static int activate(AVFilterContext *ctx)
>  return ret;
>  
>  if (ret > 0) {
> -if ((!s->pad || (s->pad && frame->nb_samples == s->nb_out_samples))) 
> {
> +if (!s->pad || frame->nb_samples == s->nb_out_samples) {
>  ret = ff_filter_frame(outlink, frame);
>  if (ff_inlink_queued_samples(inlink) >= s->nb_out_samples)
>  ff_filter_set_ready(ctx, 100);

LGTM

thx

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

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk



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 2/2] avformat/rpl: Allow a file to have audio, but not video

2019-06-16 Thread Michael Niedermayer
On Fri, Jun 14, 2019 at 08:51:34PM +0100, Cameron Cawley wrote:
> Signed-off-by: Cameron Cawley 
> ---
>  libavformat/rpl.c | 78 
> +++
>  1 file changed, 44 insertions(+), 34 deletions(-)

a fate test for this probably would make sense

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

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


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

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

Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags to FF_DECODE_ERROR_CONCEALMENT_ACTIVE in case of concealed errors

2019-06-16 Thread Michael Niedermayer
On Fri, Jun 14, 2019 at 07:43:46AM -0700, Amir Pauker wrote:
> set AVFrame decode_error_flags to FF_DECODE_ERROR_CONCEALMENT_ACTIVE in case
> h->slice_ctx->er.error_occurred is set after the call to 
> ff_h264_execute_decode_slices.
> This allows the user to detect concealed decoding errors in the call to 
> avcodec_receive_frame
> 
> Signed-off-by: Amir Pauker 
> ---
>  libavcodec/h264dec.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 837c3b7..98b7d79 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const 
> uint8_t *buf, int buf_size)
>  if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
>  goto end;
>  
> +// set decode_error_flags to allow users to detect concealed decoding 
> errors
> +if ((ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr) {
> +h->cur_pic_ptr->f->decode_error_flags |= 
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
> +

it would be more robust to set the flag around where the
"concealing %d DC, %d AC, %d MV errors in %c frame\" message is printed

That is when concealing actually happens

thanks

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

Avoid a single point of failure, be that a person or equipment.


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] add FF_DECODE_ERROR_CONCEALMENT_ACTIVE flag for AVFrame.decode_error_flags

2019-06-16 Thread Michael Niedermayer
On Fri, Jun 14, 2019 at 07:35:44AM -0700, Amir Pauker wrote:
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE is set when the decoded frame has error(s) 
> but the returned value from
> avcodec_receive_frame is zero i.e. concealed errors
> 
> Signed-off-by: Amir Pauker 
> ---
>  doc/APIchanges  | 3 +++
>  libavutil/frame.h   | 1 +
>  libavutil/version.h | 2 +-
>  3 files changed, 5 insertions(+), 1 deletion(-)

will apply 

thanks


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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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

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

Re: [FFmpeg-devel] [PATCH 4/5] avcodec/bink: Reorder operations in init to avoid memleak on error

2019-06-16 Thread Michael Niedermayer
On Sun, Jun 16, 2019 at 12:37:32PM +1000, Peter Ross wrote:
> On Sun, Jun 16, 2019 at 12:00:55AM +0200, Michael Niedermayer wrote:
> > Fixes: Direct leak of 536 byte(s) in 1 object(s)
> > Fixes: 
> > 15266/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINK_fuzzer-5629530426834944
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/bink.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/bink.c b/libavcodec/bink.c
> > index d0f1b39321..8392bbeeb0 100644
> > --- a/libavcodec/bink.c
> > +++ b/libavcodec/bink.c
> > @@ -1333,13 +1333,13 @@ static av_cold int decode_init(AVCodecContext 
> > *avctx)
> >  }
> >  c->avctx = avctx;
> >  
> > +if ((ret = av_image_check_size(avctx->width, avctx->height, 0, avctx)) 
> > < 0)
> > +return ret;
> > +
> >  c->last = av_frame_alloc();
> >  if (!c->last)
> >  return AVERROR(ENOMEM);
> >  
> > -if ((ret = av_image_check_size(avctx->width, avctx->height, 0, avctx)) 
> > < 0)
> > -return ret;
> > -
> >  avctx->pix_fmt = c->has_alpha ? AV_PIX_FMT_YUVA420P : 
> > AV_PIX_FMT_YUV420P;
> >  avctx->color_range = c->version == 'k' ? AVCOL_RANGE_JPEG : 
> > AVCOL_RANGE_MPEG;
> >  
> > -- 
> > 2.21.0
> 
> lgtm. please apply.

will apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

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

Re: [FFmpeg-devel] [PATCH v6] avformat/ifv: added support for ifv cctv files

2019-06-16 Thread Swaraj Hota
On Sun 16 Jun, 2019, 8:24 AM Peter Ross,  wrote:

> On Mon, Jun 10, 2019 at 09:25:27AM +0530, Swaraj Hota wrote:
> > Fixes ticket #2956.
> >
> > Signed-off-by: Swaraj Hota 
> > ---
> > Added entry in "doc/general.texi".
> > ---
> >  Changelog|   1 +
> >  doc/general.texi |   2 +
> >  libavformat/Makefile |   1 +
> >  libavformat/allformats.c |   1 +
> >  libavformat/ifv.c| 304 +++
> >  libavformat/version.h|   4 +-
> >  6 files changed, 311 insertions(+), 2 deletions(-)
> >  create mode 100644 libavformat/ifv.c
>
> i think this is now ready to push.
>
> can you do that? if this for gsoc2019, does it first need blessing from
> mentor?
>

I don't think that's required. ':D

otherwise i will push in a couple of days.
>

Thank you!
___
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 v6] avformat/ifv: added support for ifv cctv files

2019-06-16 Thread Paul B Mahol
On 6/16/19, Swaraj Hota  wrote:
> On Sun 16 Jun, 2019, 8:24 AM Peter Ross,  wrote:
>
>> On Mon, Jun 10, 2019 at 09:25:27AM +0530, Swaraj Hota wrote:
>> > Fixes ticket #2956.
>> >
>> > Signed-off-by: Swaraj Hota 
>> > ---
>> > Added entry in "doc/general.texi".
>> > ---
>> >  Changelog|   1 +
>> >  doc/general.texi |   2 +
>> >  libavformat/Makefile |   1 +
>> >  libavformat/allformats.c |   1 +
>> >  libavformat/ifv.c| 304 +++
>> >  libavformat/version.h|   4 +-
>> >  6 files changed, 311 insertions(+), 2 deletions(-)
>> >  create mode 100644 libavformat/ifv.c
>>
>> i think this is now ready to push.
>>
>> can you do that? if this for gsoc2019, does it first need blessing from
>> mentor?
>>
>
> I don't think that's required. ':D

It is required, for sure. Mentor did not replied at all.

>
> otherwise i will push in a couple of days.
>>
>
> Thank you!
> ___
> 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 v4] libavfilter/vf_cover_rect: support for cover image with more pixel format and different width and height

2019-06-16 Thread Lance Wang
On Sun, Jun 16, 2019 at 3:26 PM Michael Niedermayer 
wrote:

> On Sun, Jun 16, 2019 at 07:11:27AM +0800, Lance Wang wrote:
> > On Sun, Jun 16, 2019 at 6:20 AM Michael Niedermayer
> 
> > wrote:
> >
> > > On Fri, Jun 14, 2019 at 11:52:47PM +0800, Lance Wang wrote:
> > > > On Fri, Jun 14, 2019 at 5:31 PM Michael Niedermayer
> > > 
> > > > wrote:
> [...]
> > >
> > > >
> > > >
> > > > >
> > > > >
> > > > > > +if (!cover->match_frame || (w !=
> cover->match_frame->width
> > > || h
> > > > > != cover->match_frame->height
> > > > > > +|| in_format !=
> cover->match_frame->format)) {
> > > > > > +if (cover->match_frame)
> > > > > > +av_freep(&cover->match_frame->data[0]);
> > > > > > +else if (!(cover->match_frame = av_frame_alloc()))
> > > > > > +return AVERROR(ENOMEM);
> > > > > > +
> > > > >
> > > > > > +if ((ret = ff_scale_image(cover->match_frame->data,
> > > > > cover->match_frame->linesize,
> > > > > > +w, h, in_format,
> > > cover->cover_frame->data,
> > > > > cover->cover_frame->linesize,
> > > > > > +cover->cover_frame->width,
> > > > > cover->cover_frame->height,
> > > > > > +cover->cover_frame->format, ctx)) <
> 0)
> > > > > > +return AVERROR(ENOMEM);
> > > > >
> > > > > This sort of reimplements the scale filter and it
> > > > > doesnt consider some parameters like AVCOL_RANGE*
> > > > >
> > > >
> > > > the ff_scale_image is implemented and used by other alike place,  I
> try
> > > to
> > > > reuse the function anyway.
> > > > If we need other parameters for scale, we may improve the function
> later,
> > > > now it's OK for my testing as
> > > > the cover image is logo mostly.
> > >
> > > I think one could argue that the scale filter should be used for
> converting
> > > this way the cover image would become one externally vissible input.
> (which
> > > then subsequently also could change over time and not just be a static
> > > image ...)
> > >
> > >
> > For now, the code is simple and prefer to use it.  How about to improve
> all
> > other function which use ff_scale_image in future.
>
> if you add no new ff_scale_image(), sure theres no need to clean it up
> but if you add more bad code, no thats not ok IMO.
>
>
OK,  if not use ff_scale_image function, what's better recommended to reuse
the vf_scale complete function simply?



> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Asymptotically faster algorithms should always be preferred if you have
> asymptotical amounts of data
> ___
> 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 v5 2/3] ibavfilter/vf_cover_rect.c: change the cover_rect function for support cover frame can be changed

2019-06-16 Thread Lance Wang
On Sun, Jun 16, 2019 at 3:24 PM Michael Niedermayer 
wrote:

> On Sat, Jun 15, 2019 at 12:48:25AM +0800, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> >
> > Signed-off-by: Limin Wang 
> > ---
> >  libavfilter/vf_cover_rect.c | 17 ++---
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > index 898debf..b66c40b 100644
> > --- a/libavfilter/vf_cover_rect.c
> > +++ b/libavfilter/vf_cover_rect.c
> > @@ -71,24 +71,25 @@ static int config_input(AVFilterLink *inlink)
> >  return 0;
> >  }
> >
> > -static void cover_rect(CoverContext *cover, AVFrame *in, int offx, int
> offy)
> > +static void cover_rect(AVFrame *cover_frame, AVFrame *in, int offx, int
> offy)
> >  {
> >  int x, y, p;
> >
> >  for (p = 0; p < 3; p++) {
> >  uint8_t *data = in->data[p] + (offx>>!!p) + (offy>>!!p) *
> in->linesize[p];
> > -const uint8_t *src = cover->cover_frame->data[p];
> > -int w = AV_CEIL_RSHIFT(cover->cover_frame->width , !!p);
> > -int h = AV_CEIL_RSHIFT(cover->cover_frame->height, !!p);
> > +const uint8_t *src = cover_frame->data[p];
> > +int w = AV_CEIL_RSHIFT(cover_frame->width , !!p);
> > +int h = AV_CEIL_RSHIFT(cover_frame->height, !!p);
> >  for (y = 0; y < h; y++) {
> >  for (x = 0; x < w; x++) {
> >  data[x] = src[x];
> >  }
> >  data += in->linesize[p];
> > -src += cover->cover_frame->linesize[p];
> > +src += cover_frame->linesize[p];
> >  }
> >  }
>
> >  }
> > +
> >  static void blur(CoverContext *cover, AVFrame *in, int offx, int offy)
> >  {
> >  int x, y, p;
> > @@ -139,6 +140,7 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >  AVDictionaryEntry *ex, *ey, *ew, *eh;
> >  int x = -1, y = -1, w = -1, h = -1;
> >  char *xendptr = NULL, *yendptr = NULL, *wendptr = NULL, *hendptr =
> NULL;
> > +AVFrame *cover_frame = NULL;
> >
> >  ex = av_dict_get(in->metadata, "lavfi.rect.x", NULL,
> AV_DICT_MATCH_CASE);
> >  ey = av_dict_get(in->metadata, "lavfi.rect.y", NULL,
> AV_DICT_MATCH_CASE);
> > @@ -174,6 +176,7 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >  if (cover->cover_frame) {
> >  if (w != cover->cover_frame->width || h !=
> cover->cover_frame->height)
> >  return AVERROR(EINVAL);
> > +cover_frame = cover->cover_frame;
> >  }
> >
> >  cover->width  = w;
> > @@ -186,8 +189,8 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
> >
> >  if (cover->mode == MODE_BLUR) {
> >  blur (cover, in, x, y);
>
> > -} else {
> > -cover_rect(cover, in, x, y);
> > +} else if (cover_frame) {
> > +cover_rect(cover_frame, in, x, y);
>
> replacing a field from the context by a local variable cannot make a field
> null that was not before
>

Yes,  no need to check here.  I'll update if the ff_scale_image function
issue can be fixed.



>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 1/4] avcodec/apedec: Fix multiple integer overflows in predictor_update_filter()

2019-06-16 Thread Michael Niedermayer
Fixes: signed integer overflow: -829262115 + -1410750414 cannot be represented 
in type 'int'
Fixes: 
15251/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5651742252859392

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

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 15eb416ba4..2428c3cb64 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -1124,10 +1124,10 @@ static av_always_inline int 
predictor_update_filter(APEPredictor *p,
 p->buf[delayA - 1] = p->buf[delayA] - p->buf[delayA - 1];
 p->buf[adaptA - 1] = APESIGN(p->buf[delayA - 1]);
 
-predictionA = p->buf[delayA] * p->coeffsA[filter][0] +
-  p->buf[delayA - 1] * p->coeffsA[filter][1] +
-  p->buf[delayA - 2] * p->coeffsA[filter][2] +
-  p->buf[delayA - 3] * p->coeffsA[filter][3];
+predictionA = p->buf[delayA] * (unsigned)p->coeffsA[filter][0] +
+  p->buf[delayA - 1] * (unsigned)p->coeffsA[filter][1] +
+  p->buf[delayA - 2] * (unsigned)p->coeffsA[filter][2] +
+  p->buf[delayA - 3] * (unsigned)p->coeffsA[filter][3];
 
 /*  Apply a scaled first-order filter compression */
 p->buf[delayB] = p->filterA[filter ^ 1] - ((p->filterB[filter] * 31) 
>> 5);
@@ -1136,13 +1136,13 @@ static av_always_inline int 
predictor_update_filter(APEPredictor *p,
 p->buf[adaptB - 1] = APESIGN(p->buf[delayB - 1]);
 p->filterB[filter] = p->filterA[filter ^ 1];
 
-predictionB = p->buf[delayB] * p->coeffsB[filter][0] +
-  p->buf[delayB - 1] * p->coeffsB[filter][1] +
-  p->buf[delayB - 2] * p->coeffsB[filter][2] +
-  p->buf[delayB - 3] * p->coeffsB[filter][3] +
-  p->buf[delayB - 4] * p->coeffsB[filter][4];
+predictionB = p->buf[delayB] * (unsigned)p->coeffsB[filter][0] +
+  p->buf[delayB - 1] * (unsigned)p->coeffsB[filter][1] +
+  p->buf[delayB - 2] * (unsigned)p->coeffsB[filter][2] +
+  p->buf[delayB - 3] * (unsigned)p->coeffsB[filter][3] +
+  p->buf[delayB - 4] * (unsigned)p->coeffsB[filter][4];
 
-p->lastA[filter] = decoded + ((predictionA + (predictionB >> 1)) >> 10);
+p->lastA[filter] = decoded + ((int)((unsigned)predictionA + (predictionB 
>> 1)) >> 10);
 p->filterA[filter] = p->lastA[filter] + ((p->filterA[filter] * 31) >> 5);
 
 sign = APESIGN(decoded);
-- 
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 2/4] avcodec/apedec: Add k < 24 check to the only k++ case which lacks such a check

2019-06-16 Thread Michael Niedermayer
Fixes: 
15255/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5718831688843264
Fixes: left shift of 1 by 31 places cannot be represented in type 'int'

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

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 2428c3cb64..3558a5b708 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -460,7 +460,7 @@ static inline void update_rice(APERice *rice, unsigned int 
x)
 
 if (rice->ksum < lim)
 rice->k--;
-else if (rice->ksum >= (1 << (rice->k + 5)))
+else if (rice->ksum >= (1 << (rice->k + 5)) && rice->k < 24)
 rice->k++;
 }
 
-- 
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 3/4] avcodec/apedec: Fix various integer overflows

2019-06-16 Thread Michael Niedermayer
Fixes: signed integer overflow: -538976267 * 31 cannot be represented in type 
'int'
Fixes: left shift of 65312 by 16 places cannot be represented in type 'int'
Fixes: 
15255/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5718831688843264

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

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 3558a5b708..61ebfdafd5 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -554,7 +554,7 @@ static inline int ape_decode_value_3990(APEContext *ctx, 
APERice *rice)
 overflow = range_get_symbol(ctx, counts_3980, counts_diff_3980);
 
 if (overflow == (MODEL_ELEMENTS - 1)) {
-overflow  = range_decode_bits(ctx, 16) << 16;
+overflow  = (unsigned)range_decode_bits(ctx, 16) << 16;
 overflow |= range_decode_bits(ctx, 16);
 }
 
@@ -1130,7 +1130,7 @@ static av_always_inline int 
predictor_update_filter(APEPredictor *p,
   p->buf[delayA - 3] * (unsigned)p->coeffsA[filter][3];
 
 /*  Apply a scaled first-order filter compression */
-p->buf[delayB] = p->filterA[filter ^ 1] - ((p->filterB[filter] * 31) 
>> 5);
+p->buf[delayB] = p->filterA[filter ^ 1] - ((int)(p->filterB[filter] * 
31U) >> 5);
 p->buf[adaptB] = APESIGN(p->buf[delayB]);
 p->buf[delayB - 1] = p->buf[delayB] - p->buf[delayB - 1];
 p->buf[adaptB - 1] = APESIGN(p->buf[delayB - 1]);
@@ -1143,7 +1143,7 @@ static av_always_inline int 
predictor_update_filter(APEPredictor *p,
   p->buf[delayB - 4] * (unsigned)p->coeffsB[filter][4];
 
 p->lastA[filter] = decoded + ((int)((unsigned)predictionA + (predictionB 
>> 1)) >> 10);
-p->filterA[filter] = p->lastA[filter] + ((p->filterA[filter] * 31) >> 5);
+p->filterA[filter] = p->lastA[filter] + ((int)(p->filterA[filter] * 31U) 
>> 5);
 
 sign = APESIGN(decoded);
 p->coeffsA[filter][0] += p->buf[adaptA] * sign;
-- 
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 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Michael Niedermayer
Fixes: left shift of negative value -4
Fixes: signed integer overflow: -15091694 * 167 cannot be represented in type 
'int'
Fixes: signed integer overflow: 1898547155 + 453967445 cannot be represented in 
type 'int'
Fixes: 
15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688

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

diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
index 61ebfdafd5..f1bfc634c2 100644
--- a/libavcodec/apedec.c
+++ b/libavcodec/apedec.c
@@ -859,22 +859,22 @@ static av_always_inline int filter_3800(APEPredictor *p,
 return predictionA;
 }
 d2 =  p->buf[delayA];
-d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
-d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) << 3);
+d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
+d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) * 8);
 d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
 d4 =  p->buf[delayB];
 
-predictionA = d0 * p->coeffsA[filter][0] +
-  d1 * p->coeffsA[filter][1] +
-  d2 * p->coeffsA[filter][2];
+predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
+  d1 * (unsigned)p->coeffsA[filter][1] +
+  d2 * (unsigned)p->coeffsA[filter][2];
 
 sign = APESIGN(decoded);
 p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
 p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
 p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
 
-predictionB = d3 * p->coeffsB[filter][0] -
-  d4 * p->coeffsB[filter][1];
+predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
+  d4 * (unsigned)p->coeffsB[filter][1];
 p->lastA[filter] = decoded + (predictionA >> 11);
 sign = APESIGN(p->lastA[filter]);
 p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
@@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer, int 
order, int shift, int len
 dotprod = 0;
 sign = APESIGN(buffer[i]);
 for (j = 0; j < order; j++) {
-dotprod += delay[j] * coeffs[j];
+dotprod += delay[j] * (unsigned)coeffs[j];
 coeffs[j] += ((delay[j] >> 31) | 1) * sign;
 }
 buffer[i] -= dotprod >> shift;
-- 
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 2/5] avcodec/atrac9dec: Check conditions before apply_band_extension() to avoid out of array read in initialization of unused variables

2019-06-16 Thread Lynne
Jun 15, 2019, 11:00 PM by mich...@niedermayer.cc:

> Fixes: global-buffer-overflow
> Fixes: 
> 15247/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5671602181636096
>
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/atrac9dec.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> index 805d46f3b8..5401d6e19e 100644
> --- a/libavcodec/atrac9dec.c
> +++ b/libavcodec/atrac9dec.c
> @@ -535,9 +535,6 @@ static inline void apply_band_extension(ATRAC9Context *s, 
> ATRAC9BlockData *b,
>  at9_q_unit_to_coeff_idx[g_units[3]],
>  };
>  
> -if (!b->has_band_ext || !b->has_band_ext_data)
> -return;
> -
>  for (int ch = 0; ch <= stereo; ch++) {
>  ATRAC9ChannelData *c = &b->channel[ch];
>  
> @@ -741,7 +738,9 @@ static int atrac9_decode_block(ATRAC9Context *s, 
> GetBitContext *gb,
>  
>  apply_intensity_stereo(s, b, stereo);
>  apply_scalefactors(s, b, stereo);
> -apply_band_extension  (s, b, stereo);
> +
> +if (b->has_band_ext && b->has_band_ext_data)
> +apply_band_extension  (s, b, stereo); 
>

False positive as usual, q_unit_cnt can't be anything out of array since its 
looked up from
at9_tab_band_q_unit_map.
I'd really appreciate it if you stopped fixing complaint messages from 
automated tools.
Especially from overflows and fuzzing timeouts. The latter are completely 
useless and
often make the code look worse and weird, and the former are all useless except 
when
outside of DSP code (e.g. malloc). And most of our code is DSP.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Lynne
Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:

> Fixes: left shift of negative value -4
> Fixes: signed integer overflow: -15091694 * 167 cannot be represented in type 
> 'int'
> Fixes: signed integer overflow: 1898547155 + 453967445 cannot be represented 
> in type 'int'
> Fixes: 
> 15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688
>
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/apedec.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 61ebfdafd5..f1bfc634c2 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -859,22 +859,22 @@ static av_always_inline int filter_3800(APEPredictor *p,
>  return predictionA;
>  }
>  d2 =  p->buf[delayA];
> -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
> -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) << 3);
> +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
> +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) * 8);
>  d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
>  d4 =  p->buf[delayB];
>  
> -predictionA = d0 * p->coeffsA[filter][0] +
> -  d1 * p->coeffsA[filter][1] +
> -  d2 * p->coeffsA[filter][2];
> +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
> +  d1 * (unsigned)p->coeffsA[filter][1] +
> +  d2 * (unsigned)p->coeffsA[filter][2];
>  
>  sign = APESIGN(decoded);
>  p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
>  p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
>  p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
>  
> -predictionB = d3 * p->coeffsB[filter][0] -
> -  d4 * p->coeffsB[filter][1];
> +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
> +  d4 * (unsigned)p->coeffsB[filter][1];
>  p->lastA[filter] = decoded + (predictionA >> 11);
>  sign = APESIGN(p->lastA[filter]);
>  p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
> @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer, int 
> order, int shift, int len
>  dotprod = 0;
>  sign = APESIGN(buffer[i]);
>  for (j = 0; j < order; j++) {
> -dotprod += delay[j] * coeffs[j];
> +dotprod += delay[j] * (unsigned)coeffs[j];
>  coeffs[j] += ((delay[j] >> 31) | 1) * sign;
>  }
>  buffer[i] -= dotprod >> shift;
>

This code is DSP, overflows should be allowed in case input is corrupt.
This isn't even the best way to solve this, you can just make the array types 
unsigned.

NAK to this patchset and all future patchsets fixing overflows in DSPs and 
timeouts until
you explain exactly how this is useful besides fixing spam from a rarely useful 
tool that
Google refuses to fix. Derek tried to ask them to ignore timeouts and they did 
nothing.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH V1 2/2] doc/muxers: fix and update docs for hls_playlist_type

2019-06-16 Thread Jun Zhao
From: Jun Zhao 

fix and update docs for hls_playlist_type

Signed-off-by: Jun Zhao 
---
 doc/muxers.texi |   16 ++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 50147c4..b2fbe3f 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -896,14 +896,26 @@ before they have been added to the m3u8 playlist.
 
 @end table
 
-@item hls_playlist_type event
+@item hls_playlist_type @var{int}
+Set the HLS playlist type, Default value is none.
+
+Possible values:
+@table @option
+
+@item none (default)
+NOT contain the EXT-X-PLAYLIST-TYPE tag, usually in the Live playlist, that
+tag allows Media Segments to be removed.
+
+@item event
 Emit @code{#EXT-X-PLAYLIST-TYPE:EVENT} in the m3u8 header. Forces
 @option{hls_list_size} to 0; the playlist can only be appended to.
 
-@item hls_playlist_type vod
+@item vod
 Emit @code{#EXT-X-PLAYLIST-TYPE:VOD} in the m3u8 header. Forces
 @option{hls_list_size} to 0; the playlist must not change.
 
+@end table
+
 @item method
 Use the given HTTP method to create the hls files.
 @example
-- 
1.7.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 V1 1/2] lavf/hlsenc: Update suboption for hls_playlist_type

2019-06-16 Thread Jun Zhao
From: Jun Zhao 

Update suboption for hls_playlist_type

Signed-off-by: Jun Zhao 
---
 libavformat/hlsenc.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 9884f74..6c0d0a5 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2945,6 +2945,7 @@ static const AVOption options[] = {
 #endif
 {"strftime_mkdir", "create last directory component in strftime-generated 
filename", OFFSET(use_localtime_mkdir), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, E 
},
 {"hls_playlist_type", "set the HLS playlist type", OFFSET(pl_type), 
AV_OPT_TYPE_INT, {.i64 = PLAYLIST_TYPE_NONE }, 0, PLAYLIST_TYPE_NB-1, E, 
"pl_type" },
+{"none", "Not contain the EXT-X-PLAYLIST-TYPE tag", 0, AV_OPT_TYPE_CONST, 
{.i64 = PLAYLIST_TYPE_NONE }, INT_MIN, INT_MAX, E, "pl_type" },
 {"event", "EVENT playlist", 0, AV_OPT_TYPE_CONST, {.i64 = 
PLAYLIST_TYPE_EVENT }, INT_MIN, INT_MAX, E, "pl_type" },
 {"vod", "VOD playlist", 0, AV_OPT_TYPE_CONST, {.i64 = PLAYLIST_TYPE_VOD }, 
INT_MIN, INT_MAX, E, "pl_type" },
 {"method", "set the HTTP method(default: PUT)", OFFSET(method), 
AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,E},
-- 
1.7.1

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

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

Re: [FFmpeg-devel] [PATCH V1 1/2] lavf/hlsenc: Update suboption for hls_playlist_type

2019-06-16 Thread Steven Liu


> 在 2019年6月16日,19:42,Jun Zhao  写道:
> 
> From: Jun Zhao 
> 
> Update suboption for hls_playlist_type
> 
> Signed-off-by: Jun Zhao 
> ---
> libavformat/hlsenc.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 9884f74..6c0d0a5 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2945,6 +2945,7 @@ static const AVOption options[] = {
> #endif
> {"strftime_mkdir", "create last directory component in strftime-generated 
> filename", OFFSET(use_localtime_mkdir), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, 
> E },
> {"hls_playlist_type", "set the HLS playlist type", OFFSET(pl_type), 
> AV_OPT_TYPE_INT, {.i64 = PLAYLIST_TYPE_NONE }, 0, PLAYLIST_TYPE_NB-1, E, 
> "pl_type" },
> +{"none", "Not contain the EXT-X-PLAYLIST-TYPE tag", 0, 
> AV_OPT_TYPE_CONST, {.i64 = PLAYLIST_TYPE_NONE }, INT_MIN, INT_MAX, E, 
> "pl_type" },
This option is same as don’t use hls_playlist_type, is it?
> 
> {"event", "EVENT playlist", 0, AV_OPT_TYPE_CONST, {.i64 = 
> PLAYLIST_TYPE_EVENT }, INT_MIN, INT_MAX, E, "pl_type" },
> {"vod", "VOD playlist", 0, AV_OPT_TYPE_CONST, {.i64 = PLAYLIST_TYPE_VOD 
> }, INT_MIN, INT_MAX, E, "pl_type" },
> {"method", "set the HTTP method(default: PUT)", OFFSET(method), 
> AV_OPT_TYPE_STRING, {.str = NULL},  0, 0,E},
> -- 
> 1.7.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".

Thanks
Steven





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

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

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: unknown codec triggers error

2019-06-16 Thread Reimar Döffinger
On 14.06.2019, at 17:01, James Almer  wrote:

> On 6/14/2019 11:52 AM, Reimar Döffinger wrote:
>> 
>> 
>> On 14.06.2019, at 03:15, Chris Cunningham  wrote:
>> 
>>> Only "succeed" to read a header if the codec is valid. Otherwise
>>> return AVERROR_INVALIDDATA.
>> 
>> That doesn't sound right to me, an unknown codec in (possibly) a single 
>> stream is not an error.
>> I understood the discussion more to say the if it's an unknown codec, we 
>> should not try to override valid codec configuration with a broken one.
> 
> I did request this change, seeing that returning codec_id none in this
> scenario results in a crash at a later point due to conflicting parameters.
> 
> Do you suggest we should limit the change to only reject any duplicate
> header that may show up after the first one (and before the first data
> packet)?

I don't know or understand the details, but I understood the suggestion as "do 
not override a valid codec ID to NONE".
Either way, I would have suggested only skipping the affected stream - but I 
admit I have not checked how far-reaching it is to return an error here.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Reimar Döffinger


On 16.06.2019, at 12:30, Lynne  wrote:

> Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:
> 
>> Fixes: left shift of negative value -4
>> Fixes: signed integer overflow: -15091694 * 167 cannot be represented in 
>> type 'int'
>> Fixes: signed integer overflow: 1898547155 + 453967445 cannot be represented 
>> in type 'int'
>> Fixes: 
>> 15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688
>> 
>> Found-by: continuous fuzzing process 
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: Michael Niedermayer 
>> ---
>> libavcodec/apedec.c | 16 
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>> index 61ebfdafd5..f1bfc634c2 100644
>> --- a/libavcodec/apedec.c
>> +++ b/libavcodec/apedec.c
>> @@ -859,22 +859,22 @@ static av_always_inline int filter_3800(APEPredictor 
>> *p,
>> return predictionA;
>> }
>> d2 =  p->buf[delayA];
>> -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
>> -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) << 3);
>> +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
>> +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) * 8);
>> d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
>> d4 =  p->buf[delayB];
>> 
>> -predictionA = d0 * p->coeffsA[filter][0] +
>> -  d1 * p->coeffsA[filter][1] +
>> -  d2 * p->coeffsA[filter][2];
>> +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
>> +  d1 * (unsigned)p->coeffsA[filter][1] +
>> +  d2 * (unsigned)p->coeffsA[filter][2];
>> 
>> sign = APESIGN(decoded);
>> p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
>> p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
>> p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
>> 
>> -predictionB = d3 * p->coeffsB[filter][0] -
>> -  d4 * p->coeffsB[filter][1];
>> +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
>> +  d4 * (unsigned)p->coeffsB[filter][1];
>> p->lastA[filter] = decoded + (predictionA >> 11);
>> sign = APESIGN(p->lastA[filter]);
>> p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
>> @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer, int 
>> order, int shift, int len
>> dotprod = 0;
>> sign = APESIGN(buffer[i]);
>> for (j = 0; j < order; j++) {
>> -dotprod += delay[j] * coeffs[j];
>> +dotprod += delay[j] * (unsigned)coeffs[j];
>> coeffs[j] += ((delay[j] >> 31) | 1) * sign;
>> }
>> buffer[i] -= dotprod >> shift;
>> 
> 
> This code is DSP, overflows should be allowed in case input is corrupt.

Then get the C standard changed or use a different language.
But in C overflows on signed types absolutely are not Ok.
___
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 v6] avformat/ifv: added support for ifv cctv files

2019-06-16 Thread Swaraj Hota
On Sun, Jun 16, 2019 at 2:12 PM Paul B Mahol  wrote:

> On 6/16/19, Swaraj Hota  wrote:
> > On Sun 16 Jun, 2019, 8:24 AM Peter Ross,  wrote:
> >
> >> On Mon, Jun 10, 2019 at 09:25:27AM +0530, Swaraj Hota wrote:
> >> > Fixes ticket #2956.
> >> >
> >> > Signed-off-by: Swaraj Hota 
> >> > ---
> >> > Added entry in "doc/general.texi".
> >> > ---
> >> >  Changelog|   1 +
> >> >  doc/general.texi |   2 +
> >> >  libavformat/Makefile |   1 +
> >> >  libavformat/allformats.c |   1 +
> >> >  libavformat/ifv.c| 304
> +++
> >> >  libavformat/version.h|   4 +-
> >> >  6 files changed, 311 insertions(+), 2 deletions(-)
> >> >  create mode 100644 libavformat/ifv.c
> >>
> >> i think this is now ready to push.
> >>
> >> can you do that? if this for gsoc2019, does it first need blessing from
> >> mentor?
> >>
> >
> > I don't think that's required. ':D
>
> It is required, for sure. Mentor did not replied at all.
>

Sorry I didn't know that. But he did tell me before (personally) to get it
pushed iirc. I guess he is busy right now. Anyway, I think it would be fair
to wait for a few days and then push if no further objections?
Or if it is absolutely necessary then it's your call, we can wait for him.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Paul B Mahol
On 6/16/19, Reimar Döffinger  wrote:
>
>
> On 16.06.2019, at 12:30, Lynne  wrote:
>
>> Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:
>>
>>> Fixes: left shift of negative value -4
>>> Fixes: signed integer overflow: -15091694 * 167 cannot be represented in
>>> type 'int'
>>> Fixes: signed integer overflow: 1898547155 + 453967445 cannot be
>>> represented in type 'int'
>>> Fixes:
>>> 15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688
>>>
>>> Found-by: continuous fuzzing process
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer 
>>> ---
>>> libavcodec/apedec.c | 16 
>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>>> index 61ebfdafd5..f1bfc634c2 100644
>>> --- a/libavcodec/apedec.c
>>> +++ b/libavcodec/apedec.c
>>> @@ -859,22 +859,22 @@ static av_always_inline int
>>> filter_3800(APEPredictor *p,
>>> return predictionA;
>>> }
>>> d2 =  p->buf[delayA];
>>> -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
>>> -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) <<
>>> 3);
>>> +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
>>> +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) *
>>> 8);
>>> d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
>>> d4 =  p->buf[delayB];
>>>
>>> -predictionA = d0 * p->coeffsA[filter][0] +
>>> -  d1 * p->coeffsA[filter][1] +
>>> -  d2 * p->coeffsA[filter][2];
>>> +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
>>> +  d1 * (unsigned)p->coeffsA[filter][1] +
>>> +  d2 * (unsigned)p->coeffsA[filter][2];
>>>
>>> sign = APESIGN(decoded);
>>> p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
>>> p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
>>> p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
>>>
>>> -predictionB = d3 * p->coeffsB[filter][0] -
>>> -  d4 * p->coeffsB[filter][1];
>>> +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
>>> +  d4 * (unsigned)p->coeffsB[filter][1];
>>> p->lastA[filter] = decoded + (predictionA >> 11);
>>> sign = APESIGN(p->lastA[filter]);
>>> p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
>>> @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer,
>>> int order, int shift, int len
>>> dotprod = 0;
>>> sign = APESIGN(buffer[i]);
>>> for (j = 0; j < order; j++) {
>>> -dotprod += delay[j] * coeffs[j];
>>> +dotprod += delay[j] * (unsigned)coeffs[j];
>>> coeffs[j] += ((delay[j] >> 31) | 1) * sign;
>>> }
>>> buffer[i] -= dotprod >> shift;
>>>
>>
>> This code is DSP, overflows should be allowed in case input is corrupt.
>
> Then get the C standard changed or use a different language.
> But in C overflows on signed types absolutely are not Ok.

I disagree, overflows in DSP are normal.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2] libavcodec/videotoolboxenc: Fix compilation broken on macOS 10.12

2019-06-16 Thread Richard Kern

> On Jun 15, 2019, at 7:54 PM, Lance Wang  wrote:
> 
>> On Fri, Jun 7, 2019 at 11:13 PM  wrote:
>> 
>> From: Limin Wang 
>> 
>> Signed-off-by: Limin Wang 
>> ---
>> libavcodec/videotoolboxenc.c | 5 +
>> 1 file changed, 5 insertions(+)
>> 
>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> index 3665581..f8ccdea 100644
>> --- a/libavcodec/videotoolboxenc.c
>> +++ b/libavcodec/videotoolboxenc.c
>> @@ -39,6 +39,11 @@
>> enum { kCMVideoCodecType_HEVC = 'hvc1' };
>> #endif
>> 
>> +#if !HAVE_KCVPIXELFORMATTYPE_420YPCBCR10BIPLANARVIDEORANGE
>> +enum { kCVPixelFormatType_420YpCbCr10BiPlanarFullRange = 'xf20' };
>> +enum { kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange = 'x420' };
>> +#endif
>> +
>> 
> 
> 
> Anybody can check the patch is OK or not,  I'm glad that the FFmpeg master
> will be build on my old Mac pro system without self patch.
> Or apply below patch if you prefer to.
> https://patchwork.ffmpeg.org/patch/13109/
> 

I’ll look at it tomorrow and push. 

> 
> 
>> typedef OSStatus (*getParameterSetAtIndex)(CMFormatDescriptionRef
>> videoDesc,
>>size_t parameterSetIndex,
>>const uint8_t
>> **parameterSetPointerOut,
>> --
>> 2.6.4
>> 
>> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
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/6] avcodec/rv10: Avoid calculating undefined value that is unused

2019-06-16 Thread Michael Niedermayer
Fixes: shift exponent 64 is too large for 32-bit type 'int'
Fixes: 
15253/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RV10_fuzzer-5671114300194816

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

diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 595e217519..8f4497b9e0 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -646,7 +646,7 @@ static int rv10_decode_packet(AVCodecContext *avctx, const 
uint8_t *buf,
 
 // Repeat the slice end check from ff_h263_decode_mb with our active
 // bitstream size
-if (ret != SLICE_ERROR) {
+if (ret != SLICE_ERROR && active_bits_size >= get_bits_count(&s->gb)) {
 int v = show_bits(&s->gb, 16);
 
 if (get_bits_count(&s->gb) + 16 > active_bits_size)
-- 
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 4/6] avcodec/bintext: Check font height

2019-06-16 Thread Michael Niedermayer
Fixes: division by zero
Fixes: 
15257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINTEXT_fuzzer-5757352881422336

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

diff --git a/libavcodec/bintext.c b/libavcodec/bintext.c
index 49b75c9e27..1aeed21f51 100644
--- a/libavcodec/bintext.c
+++ b/libavcodec/bintext.c
@@ -63,6 +63,10 @@ static av_cold int decode_init(AVCodecContext *avctx)
 av_log(avctx, AV_LOG_ERROR, "not enough extradata\n");
 return AVERROR_INVALIDDATA;
 }
+if (!s->font_height) {
+av_log(avctx, AV_LOG_ERROR, "invalid font height\n");
+return AVERROR_INVALIDDATA;
+}
 } else {
 s->font_height = 8;
 s->flags = 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 5/6] avcodec/ffwavesynth: Check ts_end - ts_start for overflow

2019-06-16 Thread Michael Niedermayer
Fixes: signed integer overflow: 2314885530818453536 - -8926099139098304480 
cannot be represented in type 'long'
Fixes: 
15259/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FFWAVESYNTH_fuzzer-5764366093254656

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

diff --git a/libavcodec/ffwavesynth.c b/libavcodec/ffwavesynth.c
index 9d055e4019..a66113972b 100644
--- a/libavcodec/ffwavesynth.c
+++ b/libavcodec/ffwavesynth.c
@@ -267,7 +267,10 @@ static int wavesynth_parse_extradata(AVCodecContext *avc)
 in->type = AV_RL32(edata + 16);
 in->channels = AV_RL32(edata + 20);
 edata += 24;
-if (in->ts_start < cur_ts || in->ts_end <= in->ts_start)
+if (in->ts_start < cur_ts ||
+in->ts_end <= in->ts_start ||
+(uint64_t)in->ts_end - in->ts_start > INT64_MAX
+)
 return AVERROR(EINVAL);
 cur_ts = in->ts_start;
 dt = in->ts_end - in->ts_start;
-- 
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 3/6] avcodec/tta: Fix undefined shift

2019-06-16 Thread Michael Niedermayer
Fixes: left shift of negative value -4483
Fixes: 
15256/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TTA_fuzzer-5738691617619968

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

diff --git a/libavcodec/tta.c b/libavcodec/tta.c
index 8f097b3bcc..c7702610b6 100644
--- a/libavcodec/tta.c
+++ b/libavcodec/tta.c
@@ -372,7 +372,7 @@ static int tta_decode_frame(AVCodecContext *avctx, void 
*data,
 // shift samples for 24-bit sample format
 int32_t *samples = (int32_t *)frame->data[0];
 for (i = 0; i < framelen * s->channels; i++)
-*samples++ <<= 8;
+*samples++ *= 256;
 // reset decode buffer
 s->decode_buffer = NULL;
 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 6/6] avcodec/vc1dsp: Avoid undefined shifts in vc1_v_s_overlap_c / vc1_h_s_overlap_c

2019-06-16 Thread Michael Niedermayer
Fixes: left shift of negative value -13
Fixes: 
15260/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VC1_fuzzer-5702076048343040

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

diff --git a/libavcodec/vc1dsp.c b/libavcodec/vc1dsp.c
index 778b811f1a..c25a6f3adf 100644
--- a/libavcodec/vc1dsp.c
+++ b/libavcodec/vc1dsp.c
@@ -95,10 +95,10 @@ static void vc1_v_s_overlap_c(int16_t *top, int16_t *bottom)
 d1 = a - d;
 d2 = a - d + b - c;
 
-top[48]   = ((a << 3) - d1 + rnd1) >> 3;
-top[56]   = ((b << 3) - d2 + rnd2) >> 3;
-bottom[0] = ((c << 3) + d2 + rnd1) >> 3;
-bottom[8] = ((d << 3) + d1 + rnd2) >> 3;
+top[48]   = ((a * 8) - d1 + rnd1) >> 3;
+top[56]   = ((b * 8) - d2 + rnd2) >> 3;
+bottom[0] = ((c * 8) + d2 + rnd1) >> 3;
+bottom[8] = ((d * 8) + d1 + rnd2) >> 3;
 
 bottom++;
 top++;
@@ -122,10 +122,10 @@ static void vc1_h_s_overlap_c(int16_t *left, int16_t 
*right, int left_stride, in
 d1 = a - d;
 d2 = a - d + b - c;
 
-left[6]  = ((a << 3) - d1 + rnd1) >> 3;
-left[7]  = ((b << 3) - d2 + rnd2) >> 3;
-right[0] = ((c << 3) + d2 + rnd1) >> 3;
-right[1] = ((d << 3) + d1 + rnd2) >> 3;
+left[6]  = ((a * 8) - d1 + rnd1) >> 3;
+left[7]  = ((b * 8) - d2 + rnd2) >> 3;
+right[0] = ((c * 8) + d2 + rnd1) >> 3;
+right[1] = ((d * 8) + d1 + rnd2) >> 3;
 
 right += right_stride;
 left  += left_stride;
-- 
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 2/6] avcodec/qdmc: Fix integer overflows in PRNG

2019-06-16 Thread Michael Niedermayer
Fixes: signed integer overflow: 214013 * 2531011 cannot be represented in type 
'int'
Fixes: 
15254/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_QDMC_fuzzer-5698137026461696

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

diff --git a/libavcodec/qdmc.c b/libavcodec/qdmc.c
index 8f5b7b920d..8bea1552e1 100644
--- a/libavcodec/qdmc.c
+++ b/libavcodec/qdmc.c
@@ -577,9 +577,9 @@ static void add_noise(QDMCContext *s, int ch, int 
current_subframe)
 for (j = 2; j < s->subframe_size - 1; j++) {
 float rnd_re, rnd_im;
 
-s->rndval = 214013 * s->rndval + 2531011;
+s->rndval = 214013U * s->rndval + 2531011;
 rnd_im = ((s->rndval & 0x7FFF) - 16384.0f) * 0.30517578f * 
s->noise2_buffer[j];
-s->rndval = 214013 * s->rndval + 2531011;
+s->rndval = 214013U * s->rndval + 2531011;
 rnd_re = ((s->rndval & 0x7FFF) - 16384.0f) * 0.30517578f * 
s->noise2_buffer[j];
 im[j  ] += rnd_im;
 re[j  ] += rnd_re;
-- 
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 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Lynne
Jun 16, 2019, 4:12 PM by one...@gmail.com:

> On 6/16/19, Reimar Döffinger  wrote:
>
>>
>>
>> On 16.06.2019, at 12:30, Lynne  wrote:
>>
>>> Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:
>>>
 Fixes: left shift of negative value -4
 Fixes: signed integer overflow: -15091694 * 167 cannot be represented in
 type 'int'
 Fixes: signed integer overflow: 1898547155 + 453967445 cannot be
 represented in type 'int'
 Fixes:
 15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688

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

 diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
 index 61ebfdafd5..f1bfc634c2 100644
 --- a/libavcodec/apedec.c
 +++ b/libavcodec/apedec.c
 @@ -859,22 +859,22 @@ static av_always_inline int
 filter_3800(APEPredictor *p,
 return predictionA;
 }
 d2 =  p->buf[delayA];
 -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
 -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) <<
 3);
 +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
 +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) *
 8);
 d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
 d4 =  p->buf[delayB];

 -predictionA = d0 * p->coeffsA[filter][0] +
 -  d1 * p->coeffsA[filter][1] +
 -  d2 * p->coeffsA[filter][2];
 +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
 +  d1 * (unsigned)p->coeffsA[filter][1] +
 +  d2 * (unsigned)p->coeffsA[filter][2];

 sign = APESIGN(decoded);
 p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
 p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
 p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;

 -predictionB = d3 * p->coeffsB[filter][0] -
 -  d4 * p->coeffsB[filter][1];
 +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
 +  d4 * (unsigned)p->coeffsB[filter][1];
 p->lastA[filter] = decoded + (predictionA >> 11);
 sign = APESIGN(p->lastA[filter]);
 p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
 @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer,
 int order, int shift, int len
 dotprod = 0;
 sign = APESIGN(buffer[i]);
 for (j = 0; j < order; j++) {
 -dotprod += delay[j] * coeffs[j];
 +dotprod += delay[j] * (unsigned)coeffs[j];
 coeffs[j] += ((delay[j] >> 31) | 1) * sign;
 }
 buffer[i] -= dotprod >> shift;

>>>
>>> This code is DSP, overflows should be allowed in case input is corrupt.
>>>
>>
>> Then get the C standard changed or use a different language.
>> But in C overflows on signed types absolutely are not Ok.
>>
>
> I disagree, overflows in DSP are normal.
>

In some codecs like VP9 they're normative IIRC.
The C standard says they're undefined operations, and what the implementation
decides to do is something we don't care about as the input is invalid anyway 
and we
can't salvage 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] [PATCH 2/5] avcodec/atrac9dec: Check conditions before apply_band_extension() to avoid out of array read in initialization of unused variables

2019-06-16 Thread Michael Niedermayer
On Sun, Jun 16, 2019 at 12:20:35PM +0200, Lynne wrote:
> Jun 15, 2019, 11:00 PM by mich...@niedermayer.cc:
> 
> > Fixes: global-buffer-overflow
> > Fixes: 
> > 15247/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5671602181636096
> >
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/atrac9dec.c | 7 +++
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> > index 805d46f3b8..5401d6e19e 100644
> > --- a/libavcodec/atrac9dec.c
> > +++ b/libavcodec/atrac9dec.c
> > @@ -535,9 +535,6 @@ static inline void apply_band_extension(ATRAC9Context 
> > *s, ATRAC9BlockData *b,
> >  at9_q_unit_to_coeff_idx[g_units[3]],
> >  };
> >  
> > -if (!b->has_band_ext || !b->has_band_ext_data)
> > -return;
> > -
> >  for (int ch = 0; ch <= stereo; ch++) {
> >  ATRAC9ChannelData *c = &b->channel[ch];
> >  
> > @@ -741,7 +738,9 @@ static int atrac9_decode_block(ATRAC9Context *s, 
> > GetBitContext *gb,
> >  
> >  apply_intensity_stereo(s, b, stereo);
> >  apply_scalefactors(s, b, stereo);
> > -apply_band_extension  (s, b, stereo);
> > +
> > +if (b->has_band_ext && b->has_band_ext_data)
> > +apply_band_extension  (s, b, stereo); 
> >
> 
> False positive as usual, q_unit_cnt can't be anything out of array since its 
> looked up from
> at9_tab_band_q_unit_map.
> I'd really appreciate it if you stopped fixing complaint messages from 
> automated tools.
> Especially from overflows and fuzzing timeouts. The latter are completely 
> useless and
> often make the code look worse and weird, and the former are all useless 
> except when
> outside of DSP code (e.g. malloc). And most of our code is DSP.

Calm down please, ill explain how this is reading out of array

In fact there seem to be more ways than i realized before that this can read
out of array, so i will post 2 more patches to fix this more completely

First q_unit_cnt is only set from at9_tab_band_q_unit_map if you are lucky as
the code is conditional on a bit read from the bitstream.

Second the values in at9_tab_band_q_unit_map start like this 0, 4, 8, 10, 12

The code reading out of array is this:

const int g_units[4] = { /* A, B, C, total units */
b->q_unit_cnt,
at9_tab_band_ext_group[b->q_unit_cnt - 13][0],
at9_tab_band_ext_group[b->q_unit_cnt - 13][1],

if q_unit_cnt is less than 13 the index is negative and that reads out of the 
array
teh value is not used later in the sample but the program could have crashed 
already

It is very good that we discuss this here though as the fix from the patch
does not appear to be enough. There are more pathes that can lead to this.

Thanks

PS: If anyone has atrac9 samples for testing, or could add atrac9 samples to 
FATE
that would be very helpfull in ensuring that none of these changes break 
anything

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

It is what and why we do it that matters, not just one of them.


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

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

Re: [FFmpeg-devel] [PATCH 4/5] avcodec/bink: Reorder operations in init to avoid memleak on error

2019-06-16 Thread James Almer
On 6/15/2019 7:00 PM, Michael Niedermayer wrote:
> Fixes: Direct leak of 536 byte(s) in 1 object(s)
> Fixes: 
> 15266/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINK_fuzzer-5629530426834944
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/bink.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/bink.c b/libavcodec/bink.c
> index d0f1b39321..8392bbeeb0 100644
> --- a/libavcodec/bink.c
> +++ b/libavcodec/bink.c
> @@ -1333,13 +1333,13 @@ static av_cold int decode_init(AVCodecContext *avctx)
>  }
>  c->avctx = avctx;
>  
> +if ((ret = av_image_check_size(avctx->width, avctx->height, 0, avctx)) < 
> 0)
> +return ret;
> +
>  c->last = av_frame_alloc();
>  if (!c->last)
>  return AVERROR(ENOMEM);
>  
> -if ((ret = av_image_check_size(avctx->width, avctx->height, 0, avctx)) < 
> 0)
> -return ret;
> -
>  avctx->pix_fmt = c->has_alpha ? AV_PIX_FMT_YUVA420P : AV_PIX_FMT_YUV420P;
>  avctx->color_range = c->version == 'k' ? AVCOL_RANGE_JPEG : 
> AVCOL_RANGE_MPEG;

This can also be fixed by adding the FF_CODEC_CAP_INIT_CLEANUP flag to
caps_internal instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Michael Niedermayer
On Sun, Jun 16, 2019 at 08:31:37PM +0200, Lynne wrote:
> Jun 16, 2019, 4:12 PM by one...@gmail.com:
> 
> > On 6/16/19, Reimar Döffinger  wrote:
> >
> >>
> >>
> >> On 16.06.2019, at 12:30, Lynne  wrote:
> >>
> >>> Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:
> >>>
>  Fixes: left shift of negative value -4
>  Fixes: signed integer overflow: -15091694 * 167 cannot be represented in
>  type 'int'
>  Fixes: signed integer overflow: 1898547155 + 453967445 cannot be
>  represented in type 'int'
>  Fixes:
>  15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688
> 
>  Found-by: continuous fuzzing process
>  https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>  Signed-off-by: Michael Niedermayer 
>  ---
>  libavcodec/apedec.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
>  diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>  index 61ebfdafd5..f1bfc634c2 100644
>  --- a/libavcodec/apedec.c
>  +++ b/libavcodec/apedec.c
>  @@ -859,22 +859,22 @@ static av_always_inline int
>  filter_3800(APEPredictor *p,
>  return predictionA;
>  }
>  d2 =  p->buf[delayA];
>  -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
>  -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) <<
>  3);
>  +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
>  +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) *
>  8);
>  d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
>  d4 =  p->buf[delayB];
> 
>  -predictionA = d0 * p->coeffsA[filter][0] +
>  -  d1 * p->coeffsA[filter][1] +
>  -  d2 * p->coeffsA[filter][2];
>  +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
>  +  d1 * (unsigned)p->coeffsA[filter][1] +
>  +  d2 * (unsigned)p->coeffsA[filter][2];
> 
>  sign = APESIGN(decoded);
>  p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
>  p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
>  p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
> 
>  -predictionB = d3 * p->coeffsB[filter][0] -
>  -  d4 * p->coeffsB[filter][1];
>  +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
>  +  d4 * (unsigned)p->coeffsB[filter][1];
>  p->lastA[filter] = decoded + (predictionA >> 11);
>  sign = APESIGN(p->lastA[filter]);
>  p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
>  @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer,
>  int order, int shift, int len
>  dotprod = 0;
>  sign = APESIGN(buffer[i]);
>  for (j = 0; j < order; j++) {
>  -dotprod += delay[j] * coeffs[j];
>  +dotprod += delay[j] * (unsigned)coeffs[j];
>  coeffs[j] += ((delay[j] >> 31) | 1) * sign;
>  }
>  buffer[i] -= dotprod >> shift;
> 
> >>>
> >>> This code is DSP, overflows should be allowed in case input is corrupt.
> >>>
> >>
> >> Then get the C standard changed or use a different language.
> >> But in C overflows on signed types absolutely are not Ok.
> >>
> >
> > I disagree, overflows in DSP are normal.
> >
> 
> In some codecs like VP9 they're normative IIRC.
> The C standard says they're undefined operations, and what the implementation
> decides to do is something we don't care about as the input is invalid anyway 
> and we
> can't salvage it.

ISO/IEC 9899:2017
  3.4.3
1 undefined behavior
  behavior, upon use of a nonportable or erroneous program construct or of 
erroneous data, for which
  this International Standard imposes no requirements
2 Note 1 to entry: Possible undefined behavior ranges from ignoring the 
situation completely with unpredictable results,
  to behaving during translation or program execution in a documented manner 
characteristic of the environment (with or
  without the issuance of a diagnostic message), to terminating a translation 
or execution (with the issuance of a diagnostic
  message).
3 EXAMPLE An example of undefined behavior is the behavior on integer overflow.

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

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

[FFmpeg-devel] [PATCH 2/2] avcodec/atrac9dec: Check q_unit_cnt in parse_band_ext()

2019-06-16 Thread Michael Niedermayer
Fixes: global-buffer-overflow
Fixes: 
15247/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5671602181636096

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

diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
index 11b683d136..ff4991a573 100644
--- a/libavcodec/atrac9dec.c
+++ b/libavcodec/atrac9dec.c
@@ -202,6 +202,8 @@ static inline int parse_band_ext(ATRAC9Context *s, 
ATRAC9BlockData *b,
 int ext_band = 0;
 
 if (b->has_band_ext) {
+if (b->q_unit_cnt < 13)
+return AVERROR_INVALIDDATA;
 ext_band = at9_tab_band_ext_group[b->q_unit_cnt - 13][2];
 if (stereo) {
 b->channel[1].band_ext = get_bits(gb, 2);
-- 
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 1/2] avcodec/atrac9dec: Check that the reused block has succeeded initilization

2019-06-16 Thread Michael Niedermayer
Fixes: global-buffer-overflow
Fixes: 
15247/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ATRAC9_fuzzer-5671602181636096

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg

Signed-off-by: Michael Niedermayer 
---
 libavcodec/atrac9dec.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
index 805d46f3b8..11b683d136 100644
--- a/libavcodec/atrac9dec.c
+++ b/libavcodec/atrac9dec.c
@@ -71,6 +71,8 @@ typedef struct ATRAC9BlockData {
 int cpe_base_channel;
 int is_signs[30];
 
+int reuseable;
+
 } ATRAC9BlockData;
 
 typedef struct ATRAC9Context {
@@ -668,6 +670,7 @@ static int atrac9_decode_block(ATRAC9Context *s, 
GetBitContext *gb,
 if (!reuse_params) {
 int stereo_band, ext_band;
 const int min_band_count = s->samplerate_idx > 7 ? 1 : 3;
+b->reuseable = 0;
 b->band_count = get_bits(gb, 4) + min_band_count;
 b->q_unit_cnt = at9_tab_band_q_unit_map[b->band_count];
 
@@ -699,6 +702,11 @@ static int atrac9_decode_block(ATRAC9Context *s, 
GetBitContext *gb,
 }
 b->band_ext_q_unit = at9_tab_band_q_unit_map[ext_band];
 }
+b->reuseable = 1;
+}
+if (!b->reuseable) {
+av_log(s->avctx, AV_LOG_ERROR, "invalid block reused!\n");
+return AVERROR_INVALIDDATA;
 }
 
 /* Calculate bit alloc gradient */
-- 
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 4/4] avcodec/apedec: Fix multiple integer overflows in filter_3800()

2019-06-16 Thread Reimar Döffinger
On 16.06.2019, at 17:12, Paul B Mahol  wrote:

> On 6/16/19, Reimar Döffinger  wrote:
>> 
>> 
>> On 16.06.2019, at 12:30, Lynne  wrote:
>> 
>>> Jun 16, 2019, 10:57 AM by mich...@niedermayer.cc:
>>> 
 Fixes: left shift of negative value -4
 Fixes: signed integer overflow: -15091694 * 167 cannot be represented in
 type 'int'
 Fixes: signed integer overflow: 1898547155 + 453967445 cannot be
 represented in type 'int'
 Fixes:
 15258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5759095564402688
 
 Found-by: continuous fuzzing process
 https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
 Signed-off-by: Michael Niedermayer 
 ---
 libavcodec/apedec.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)
 
 diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
 index 61ebfdafd5..f1bfc634c2 100644
 --- a/libavcodec/apedec.c
 +++ b/libavcodec/apedec.c
 @@ -859,22 +859,22 @@ static av_always_inline int
 filter_3800(APEPredictor *p,
 return predictionA;
 }
 d2 =  p->buf[delayA];
 -d1 = (p->buf[delayA] - p->buf[delayA - 1]) << 1;
 -d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) <<
 3);
 +d1 = (p->buf[delayA] - p->buf[delayA - 1]) * 2;
 +d0 =  p->buf[delayA] + ((p->buf[delayA - 2] - p->buf[delayA - 1]) *
 8);
 d3 =  p->buf[delayB] * 2 - p->buf[delayB - 1];
 d4 =  p->buf[delayB];
 
 -predictionA = d0 * p->coeffsA[filter][0] +
 -  d1 * p->coeffsA[filter][1] +
 -  d2 * p->coeffsA[filter][2];
 +predictionA = d0 * (unsigned)p->coeffsA[filter][0] +
 +  d1 * (unsigned)p->coeffsA[filter][1] +
 +  d2 * (unsigned)p->coeffsA[filter][2];
 
 sign = APESIGN(decoded);
 p->coeffsA[filter][0] += (((d0 >> 30) & 2) - 1) * sign;
 p->coeffsA[filter][1] += (((d1 >> 28) & 8) - 4) * sign;
 p->coeffsA[filter][2] += (((d2 >> 28) & 8) - 4) * sign;
 
 -predictionB = d3 * p->coeffsB[filter][0] -
 -  d4 * p->coeffsB[filter][1];
 +predictionB = d3 * (unsigned)p->coeffsB[filter][0] -
 +  d4 * (unsigned)p->coeffsB[filter][1];
 p->lastA[filter] = decoded + (predictionA >> 11);
 sign = APESIGN(p->lastA[filter]);
 p->coeffsB[filter][0] += (((d3 >> 29) & 4) - 2) * sign;
 @@ -902,7 +902,7 @@ static void long_filter_high_3800(int32_t *buffer,
 int order, int shift, int len
 dotprod = 0;
 sign = APESIGN(buffer[i]);
 for (j = 0; j < order; j++) {
 -dotprod += delay[j] * coeffs[j];
 +dotprod += delay[j] * (unsigned)coeffs[j];
 coeffs[j] += ((delay[j] >> 31) | 1) * sign;
 }
 buffer[i] -= dotprod >> shift;
 
>>> 
>>> This code is DSP, overflows should be allowed in case input is corrupt.
>> 
>> Then get the C standard changed or use a different language.
>> But in C overflows on signed types absolutely are not Ok.
> 
> I disagree, overflows in DSP are normal.

It's simply incorrect code.
Yes, you will probably get lucky for a long time.
But there are ISAs that trap on overflow, and gcc can trap on overflow.
That already makes these cases a denial-of-service issue.
However it gets far worse than that.
All it takes is for code to do something like
if (x < 2048) some_array[x] = y;
(lots of code followed by DSP code doing)
x * (1 << 24)

and now the compiler can and actually might really remove the if condition and 
you have an exploitable buffer overflow.
People have been bitten by such issues often enough (because due to LTO 
compiler optimizations of this kind have potentially unlimited scope, whereas 
reviewer have almost not chance of catching something this subtle) that such 
assumptions violating the C standard should be avoided.
(besides the more immediate point that fuzzing does catch quite real issues, 
and leaving such well-maybe-it's-ok cases in prevents finding real issues - on 
its own it might be an argument to change to tools, but due to the above it is 
reasonable to consider the tools doing the right thing).
___
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/4] avcodec/hevc_ps: Fix integer overflow with num_tile_rows

2019-06-16 Thread Song, Ruiling
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Sunday, June 16, 2019 6:07 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/4] avcodec/hevc_ps: Fix integer
> overflow with num_tile_rows
> 
> On Sat, Jun 15, 2019 at 03:07:13PM +, Song, Ruiling wrote:
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > > Of Michael Niedermayer
> > > Sent: Friday, June 14, 2019 2:33 AM
> > > To: FFmpeg development discussions and patches  > > de...@ffmpeg.org>
> > > Subject: [FFmpeg-devel] [PATCH 2/4] avcodec/hevc_ps: Fix integer
> overflow
> > > with num_tile_rows
> > >
> > > Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in
> > > type 'int'
> > > Fixes: 14880/clusterfuzz-testcase-minimized-
> > > ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5130977304641536
> > >
> > > Found-by: continuous fuzzing process https://github.com/google/oss-
> > > fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/hevc_ps.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> > > index 80df417e4f..0ed6682bb4 100644
> > > --- a/libavcodec/hevc_ps.c
> > > +++ b/libavcodec/hevc_ps.c
> > > @@ -1596,7 +1596,7 @@ int ff_hevc_decode_nal_pps(GetBitContext
> *gb,
> > > AVCodecContext *avctx,
> > >  if (pps->num_tile_rows <= 0 ||
> > >  pps->num_tile_rows >= sps->height) {
> > >  av_log(avctx, AV_LOG_ERROR, "num_tile_rows_minus1 out of
> > > range: %d\n",
> > > -   pps->num_tile_rows - 1);
> > > +   pps->num_tile_rows - 1U);
> > I think the machine code generated here should be the same, right?
> > So you just tell fuzzer "I am doing subtraction between unsigned numbers",
> to make it happy?
> 
> its likely the same machine code, yes. A compiler might produce different
> code
> that break in case of the overflow though ...
Ok, it seems num_tile_columns also need such kind of change.

> 
> thx
> 
> [...]
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When the tyrant has disposed of foreign enemies by conquest or treaty, and
> there is nothing more to fear from them, then he is always stirring up
> some war or other, in order that the people may require a leader. -- Plato
___
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/4] avcodec/hevc_ps: Fix integer overflow with num_tile_rows

2019-06-16 Thread James Almer
On 6/13/2019 3:32 PM, Michael Niedermayer wrote:
> Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 
> 'int'
> Fixes: 
> 14880/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5130977304641536
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/hevc_ps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index 80df417e4f..0ed6682bb4 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -1596,7 +1596,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, 
> AVCodecContext *avctx,
>  if (pps->num_tile_rows <= 0 ||
>  pps->num_tile_rows >= sps->height) {
>  av_log(avctx, AV_LOG_ERROR, "num_tile_rows_minus1 out of range: 
> %d\n",
> -   pps->num_tile_rows - 1);
> +   pps->num_tile_rows - 1U);

The proper fix for this is making pps->num_tile_rows/cols unsigned. The
minimum allowed value for num_tile_{rows,cols}_minus1 is 0.

>  ret = AVERROR_INVALIDDATA;
>  goto err;
>  }
> 

___
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 00/18] cbs: Use ff_bsf_get_packet_ref etc.

2019-06-16 Thread Andreas Rheinhardt
Hello,

this patchset is mainly about switching the bitstream filters using cbs
from ff_bsf_get_packet to ff_bsf_get_packet_ref (except trace_headers,
which also uses it). But in the course of doing so, I also found several
other (usually small) things to improve and this is included here.

This partially conflicts with my earlier patchset [1]; I will rebase it
on top of this and resend it.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-June/244952.html

Andreas Rheinhardt (18):
  cbs: Allow non-blank packets in ff_cbs_write_packet
  av1_metadata: Avoid allocations and copies of packet structures
  filter_units: Unref packet on failure
  filter_units: Don't use fake loop
  filter_units: Reindent after previous commit
  filter_units: Avoid allocations and copies of packet structures
  av1/h264_metadata, filter_units: Count down when deleting units
  h264_metadata: Avoid allocations and copies of packet structures
  h264_metadata: Localize code for display orientation
  cbs: Remove superfluous checks for ff_cbs_delete_unit
  cbs_h264, h264_metadata: Deleting SEI messages never fails
  h264_redundant_pps: Avoid allocations and copies of packet structures
  h264_redundant_pps: Fix looping over an access unit's units
  h265_metadata: Avoid allocations and copies of packet structures
  h265_metadata: Correct error check
  mpeg2_metadata: Avoid allocations and copies of packet structures
  mpeg2_metadata: Localize inserting of sequence display extensions
  vp9_metadata: Avoid allocations and copies of packet structures

 libavcodec/av1_metadata_bsf.c   | 28 +++--
 libavcodec/cbs.c|  3 +-
 libavcodec/cbs.h| 12 +++-
 libavcodec/cbs_h264.h   |  8 +--
 libavcodec/cbs_h2645.c  | 12 ++--
 libavcodec/filter_units_bsf.c   | 62 ---
 libavcodec/h264_metadata_bsf.c  | 95 +
 libavcodec/h264_redundant_pps_bsf.c | 25 +++-
 libavcodec/h265_metadata_bsf.c  | 18 ++
 libavcodec/mpeg2_metadata_bsf.c | 50 ++-
 libavcodec/vp9_metadata_bsf.c   | 16 ++---
 11 files changed, 121 insertions(+), 208 deletions(-)

-- 
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 15/18] h265_metadata: Correct error check

2019-06-16 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h265_metadata_bsf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h265_metadata_bsf.c b/libavcodec/h265_metadata_bsf.c
index 2fb74aea5a..0dd7634774 100644
--- a/libavcodec/h265_metadata_bsf.c
+++ b/libavcodec/h265_metadata_bsf.c
@@ -289,7 +289,7 @@ static int h265_metadata_filter(AVBSFContext *bsf, AVPacket 
*pkt)
 
 err = ff_cbs_insert_unit_content(ctx->cbc, au,
  0, HEVC_NAL_AUD, aud, NULL);
-if (err) {
+if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to insert AUD.\n");
 goto fail;
 }
-- 
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 17/18] mpeg2_metadata: Localize inserting of sequence display extensions

2019-06-16 Thread Andreas Rheinhardt
If a new sequence display extension had to be added, this was up until
now done at two places: One where a sequence display extension was
initialized with default values and one where the actual sequence
display extension was inserted into the fragment. This division of
labour is unnecessary and pointless; it has been changed.

Furthermore, if a sequence display extension has to be added, the
earlier code set some fields to their default value twice. This has been
changed, too.

Signed-off-by: Andreas Rheinhardt 
---
This patch of course partially overlaps with [1]. Will rebase later.

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-June/244952.html
 libavcodec/mpeg2_metadata_bsf.c | 34 -
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/libavcodec/mpeg2_metadata_bsf.c b/libavcodec/mpeg2_metadata_bsf.c
index 082137d786..6779ffd4c4 100644
--- a/libavcodec/mpeg2_metadata_bsf.c
+++ b/libavcodec/mpeg2_metadata_bsf.c
@@ -53,7 +53,7 @@ static int mpeg2_metadata_update_fragment(AVBSFContext *bsf,
 MPEG2RawSequenceHeader*sh = NULL;
 MPEG2RawSequenceExtension *se = NULL;
 MPEG2RawSequenceDisplayExtension *sde = NULL;
-int i, se_pos, add_sde = 0;
+int i, se_pos;
 
 for (i = 0; i < frag->nb_units; i++) {
 if (frag->units[i].type == MPEG2_START_SEQUENCE_HEADER) {
@@ -115,7 +115,7 @@ static int mpeg2_metadata_update_fragment(AVBSFContext *bsf,
 ctx->transfer_characteristics >= 0 ||
 ctx->matrix_coefficients  >= 0) {
 if (!sde) {
-add_sde = 1;
+int err;
 ctx->sequence_display_extension.extension_start_code =
 MPEG2_START_EXTENSION;
 ctx->sequence_display_extension.extension_start_code_identifier =
@@ -135,6 +135,16 @@ static int mpeg2_metadata_update_fragment(AVBSFContext 
*bsf,
 .display_vertical_size =
 se->vertical_size_extension << 12 | 
sh->vertical_size_value,
 };
+
+err = ff_cbs_insert_unit_content(ctx->cbc, frag, se_pos + 1,
+ MPEG2_START_EXTENSION,
+ &ctx->sequence_display_extension,
+ NULL);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to insert new sequence "
+   "display extension.\n");
+return err;
+}
 }
 
 if (ctx->video_format >= 0)
@@ -147,32 +157,12 @@ static int mpeg2_metadata_update_fragment(AVBSFContext 
*bsf,
 
 if (ctx->colour_primaries >= 0)
 sde->colour_primaries = ctx->colour_primaries;
-else if (add_sde)
-sde->colour_primaries = 2;
 
 if (ctx->transfer_characteristics >= 0)
 sde->transfer_characteristics = ctx->transfer_characteristics;
-else if (add_sde)
-sde->transfer_characteristics = 2;
 
 if (ctx->matrix_coefficients >= 0)
 sde->matrix_coefficients = ctx->matrix_coefficients;
-else if (add_sde)
-sde->matrix_coefficients = 2;
-}
-}
-
-if (add_sde) {
-int err;
-
-err = ff_cbs_insert_unit_content(ctx->cbc, frag, se_pos + 1,
- MPEG2_START_EXTENSION,
- &ctx->sequence_display_extension,
- NULL);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to insert new sequence "
-   "display extension.\n");
-return err;
 }
 }
 
-- 
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 13/18] h264_redundant_pps: Fix looping over an access unit's units

2019-06-16 Thread Andreas Rheinhardt
When looping over an access unit's units in positive direction and
deleting some of them, one needs to make sure that a unit that is at
the position of a unit that just got deleted gets checked, too.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_redundant_pps_bsf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/h264_redundant_pps_bsf.c 
b/libavcodec/h264_redundant_pps_bsf.c
index f8b0f9ac0a..8405738c4b 100644
--- a/libavcodec/h264_redundant_pps_bsf.c
+++ b/libavcodec/h264_redundant_pps_bsf.c
@@ -95,6 +95,8 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 av_log(bsf, AV_LOG_VERBOSE, "Deleting redundant PPS "
"at %"PRId64".\n", pkt->pts);
 ff_cbs_delete_unit(ctx->input, au, i);
+i--;
+continue;
 }
 }
 if (nal->type == H264_NAL_SLICE ||
-- 
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/18] filter_units: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes filter_units to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props (or, in case of
passthrough, to av_packet_move_ref).

(b) has been made possible by the recent changes to ff_cbs_write_packet.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/filter_units_bsf.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index 8c501e1726..f3691a5755 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -98,24 +98,20 @@ invalid:
 return AVERROR(EINVAL);
 }
 
-static int filter_units_filter(AVBSFContext *bsf, AVPacket *out)
+static int filter_units_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 FilterUnitsContext  *ctx = bsf->priv_data;
 CodedBitstreamFragment *frag = &ctx->fragment;
-AVPacket *in = NULL;
 int err, i, j;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-if (ctx->mode == NOOP) {
-av_packet_move_ref(out, in);
-av_packet_free(&in);
+if (ctx->mode == NOOP)
 return 0;
-}
 
-err = ff_cbs_read_packet(ctx->cbc, frag, in);
+err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -139,21 +135,16 @@ static int filter_units_filter(AVBSFContext *bsf, 
AVPacket *out)
 goto fail;
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, frag);
+err = ff_cbs_write_packet(ctx->cbc, pkt, frag);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 fail:
 if (err < 0)
-av_packet_unref(out);
+av_packet_unref(pkt);
 ff_cbs_fragment_reset(ctx->cbc, frag);
-av_packet_free(&in);
 
 return err;
 }
-- 
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 16/18] mpeg2_metadata: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes mpeg2_metadata to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

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

diff --git a/libavcodec/mpeg2_metadata_bsf.c b/libavcodec/mpeg2_metadata_bsf.c
index ba3a74afda..082137d786 100644
--- a/libavcodec/mpeg2_metadata_bsf.c
+++ b/libavcodec/mpeg2_metadata_bsf.c
@@ -179,18 +179,17 @@ static int mpeg2_metadata_update_fragment(AVBSFContext 
*bsf,
 return 0;
 }
 
-static int mpeg2_metadata_filter(AVBSFContext *bsf, AVPacket *out)
+static int mpeg2_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 MPEG2MetadataContext *ctx = bsf->priv_data;
-AVPacket *in = NULL;
 CodedBitstreamFragment *frag = &ctx->fragment;
 int err;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->cbc, frag, in);
+err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -202,23 +201,18 @@ static int mpeg2_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 goto fail;
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, frag);
+err = ff_cbs_write_packet(ctx->cbc, pkt, frag);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->cbc, frag);
 
 if (err < 0)
-av_packet_unref(out);
-av_packet_free(&in);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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/18] av1/h264_metadata, filter_units: Count down when deleting units

2019-06-16 Thread Andreas Rheinhardt
When testing whether a particular unit should be kept or discarded, it
is best to start at the very last unit of a fragment and count down,
because that way a unit that will eventually be deleted won't be
memmoved during earlier deletions; and frag/au->nb_units need only be
evaluated once in this case and the counter is automatically correct
when a unit got deleted.

It also works for double loops, i.e. when looping over all SEI messages
in all SEI units of an access unit.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/av1_metadata_bsf.c  |  3 +--
 libavcodec/filter_units_bsf.c  |  6 ++
 libavcodec/h264_metadata_bsf.c | 18 ++
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index e294d7a24e..842b80c201 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -160,14 +160,13 @@ static int av1_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 }
 
 if (ctx->delete_padding) {
-for (i = 0; i < frag->nb_units; i++) {
+for (i = frag->nb_units - 1; i >= 0; i--) {
 if (frag->units[i].type == AV1_OBU_PADDING) {
 err = ff_cbs_delete_unit(ctx->cbc, frag, i);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to delete Padding 
OBU.\n");
 goto fail;
 }
---i;
 }
 }
 }
diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index f3691a5755..380f23e5a7 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -117,16 +117,14 @@ static int filter_units_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 goto fail;
 }
 
-for (i = 0; i < frag->nb_units; i++) {
+for (i = frag->nb_units - 1; i >= 0; i--) {
 for (j = 0; j < ctx->nb_types; j++) {
 if (frag->units[i].type == ctx->type_list[j])
 break;
 }
 if (ctx->mode == REMOVE ? j <  ctx->nb_types
-: j >= ctx->nb_types) {
+: j >= ctx->nb_types)
 ff_cbs_delete_unit(ctx->cbc, frag, i);
---i;
-}
 }
 
 if (frag->nb_units == 0) {
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index ae54929b85..b95d2341dc 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -428,7 +428,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 }
 
 if (ctx->delete_filler) {
-for (i = 0; i < au->nb_units; i++) {
+for (i = au->nb_units - 1; i >= 0; i--) {
 if (au->units[i].type == H264_NAL_FILLER_DATA) {
 // Filler NAL units.
 err = ff_cbs_delete_unit(ctx->cbc, au, i);
@@ -437,7 +437,6 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
"filler NAL.\n");
 goto fail;
 }
---i;
 continue;
 }
 
@@ -445,7 +444,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 // Filler SEI messages.
 H264RawSEI *sei = au->units[i].content;
 
-for (j = 0; j < sei->payload_count; j++) {
+for (j = sei->payload_count - 1; j >= 0; j--) {
 if (sei->payload[j].payload_type ==
 H264_SEI_TYPE_FILLER_PAYLOAD) {
 err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
@@ -455,10 +454,6 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
"filler SEI message.\n");
 goto fail;
 }
-// Renumbering might have happened, start again at
-// the same NAL unit position.
---i;
-break;
 }
 }
 }
@@ -466,13 +461,13 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 
 if (ctx->display_orientation != PASS) {
-for (i = 0; i < au->nb_units; i++) {
+for (i = au->nb_units - 1; i >= 0; i--) {
 H264RawSEI *sei;
 if (au->units[i].type != H264_NAL_SEI)
 continue;
 sei = au->units[i].content;
 
-for (j = 0; j < sei->payload_count; j++) {
+for (j = sei->payload_count - 1; j >= 0; j--) {
 H264RawSEIDisplayOrientation *disp;
 int32_t *matrix;
 
@@ -490,8 +485,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
"display orientation SEI message.\n");
 goto fail;
 }
---i;
-break;
+continue;
 }

[FFmpeg-devel] [PATCH 12/18] h264_redundant_pps: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes h264_redundant_pps to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_redundant_pps_bsf.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/libavcodec/h264_redundant_pps_bsf.c 
b/libavcodec/h264_redundant_pps_bsf.c
index 8526b5b4c4..f8b0f9ac0a 100644
--- a/libavcodec/h264_redundant_pps_bsf.c
+++ b/libavcodec/h264_redundant_pps_bsf.c
@@ -66,19 +66,18 @@ static int 
h264_redundant_pps_fixup_slice(H264RedundantPPSContext *ctx,
 return 0;
 }
 
-static int h264_redundant_pps_filter(AVBSFContext *bsf, AVPacket *out)
+static int h264_redundant_pps_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 H264RedundantPPSContext *ctx = bsf->priv_data;
-AVPacket *in;
 CodedBitstreamFragment *au = &ctx->access_unit;
 int au_has_sps;
 int err, i;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->input, au, in);
+err = ff_cbs_read_packet(ctx->input, au, pkt);
 if (err < 0)
 goto fail;
 
@@ -94,7 +93,7 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
AVPacket *out)
 goto fail;
 if (!au_has_sps) {
 av_log(bsf, AV_LOG_VERBOSE, "Deleting redundant PPS "
-   "at %"PRId64".\n", in->pts);
+   "at %"PRId64".\n", pkt->pts);
 ff_cbs_delete_unit(ctx->input, au, i);
 }
 }
@@ -105,21 +104,15 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
-err = ff_cbs_write_packet(ctx->output, out, au);
-if (err < 0)
-goto fail;
-
-
-err = av_packet_copy_props(out, in);
+err = ff_cbs_write_packet(ctx->output, pkt, au);
 if (err < 0)
 goto fail;
 
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->output, au);
-av_packet_free(&in);
 if (err < 0)
-av_packet_unref(out);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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 18/18] vp9_metadata: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes vp9_metadata to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

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

diff --git a/libavcodec/vp9_metadata_bsf.c b/libavcodec/vp9_metadata_bsf.c
index b79f08af6c..1bde1b96aa 100644
--- a/libavcodec/vp9_metadata_bsf.c
+++ b/libavcodec/vp9_metadata_bsf.c
@@ -37,18 +37,17 @@ typedef struct VP9MetadataContext {
 } VP9MetadataContext;
 
 
-static int vp9_metadata_filter(AVBSFContext *bsf, AVPacket *out)
+static int vp9_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 VP9MetadataContext *ctx = bsf->priv_data;
-AVPacket *in = NULL;
 CodedBitstreamFragment *frag = &ctx->fragment;
 int err, i;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->cbc, frag, in);
+err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -74,23 +73,18 @@ static int vp9_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 }
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, frag);
+err = ff_cbs_write_packet(ctx->cbc, pkt, frag);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->cbc, frag);
 
 if (err < 0)
-av_packet_unref(out);
-av_packet_free(&in);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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/18] filter_units: Unref packet on failure

2019-06-16 Thread Andreas Rheinhardt
According to the API, the packet structure a bsf receives must not be
touched on failure, yet filter_units nevertheless did it.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/filter_units_bsf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index bc2ca288dd..0876693c81 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -153,6 +153,8 @@ static int filter_units_filter(AVBSFContext *bsf, AVPacket 
*out)
 goto fail;
 
 fail:
+if (err < 0)
+av_packet_unref(out);
 ff_cbs_fragment_reset(ctx->cbc, frag);
 av_packet_free(&in);
 
-- 
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/18] h264_metadata: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes h264_metadata to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_metadata_bsf.c | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index b95d2341dc..18c5ae807d 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -283,21 +283,20 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
 return 0;
 }
 
-static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out)
+static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 H264MetadataContext *ctx = bsf->priv_data;
-AVPacket *in = NULL;
 CodedBitstreamFragment *au = &ctx->access_unit;
 int err, i, j, has_sps;
 H264RawAUD aud;
 uint8_t *displaymatrix_side_data = NULL;
 size_t displaymatrix_side_data_size = 0;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->cbc, au, in);
+err = ff_cbs_read_packet(ctx->cbc, au, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -518,7 +517,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*out)
 int size;
 int write = 0;
 
-data = av_packet_get_side_data(in, AV_PKT_DATA_DISPLAYMATRIX, &size);
+data = av_packet_get_side_data(pkt, AV_PKT_DATA_DISPLAYMATRIX, &size);
 if (data && size >= 9 * sizeof(int32_t)) {
 int32_t matrix[9];
 int hflip, vflip;
@@ -578,18 +577,14 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, au);
+err = ff_cbs_write_packet(ctx->cbc, pkt, au);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 if (displaymatrix_side_data) {
-err = av_packet_add_side_data(out, AV_PKT_DATA_DISPLAYMATRIX,
+err = av_packet_add_side_data(pkt, AV_PKT_DATA_DISPLAYMATRIX,
   displaymatrix_side_data,
   displaymatrix_side_data_size);
 if (err) {
@@ -608,8 +603,7 @@ fail:
 av_freep(&displaymatrix_side_data);
 
 if (err < 0)
-av_packet_unref(out);
-av_packet_free(&in);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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/18] cbs: Allow non-blank packets in ff_cbs_write_packet

2019-06-16 Thread Andreas Rheinhardt
Up until now, ff_cbs_write_packet always initialized the packet
structure it received without documenting this behaviour; furthermore,
the packet's buffer would (on success) be overwritten with the new
buffer without unreferencing the old. This meant that the input packet
had to be either clean (otherwise there would be memleaks) in which case
the initialization is redundant or uninitialized. ff_cbs_write_packet
was never used with uninitialized packets, so the initialization was
redundant. Worse yet, it forced callers to use more than one packet and
made it difficult to add side-data to a packet designated for output,
because said side-data could only be attached after the call to
ff_cbs_write_packet.

This has been changed. It is now allowed to use a non-blank packet.
The currently existing buffer will be unreferenced and replaced by
the new one, as will be the accompanying fields (i.e. data and size).
The rest isn't touched at all.

This change will enable us to use only one packet in the bitstream
filters that rely on CBS.

This commit also updates the documentation of ff_cbs_write_extradata
and ff_cbs_write_packet (to better describe existing behaviour and in
the latter case to also describe the new behaviour).

Signed-off-by: Andreas Rheinhardt 
---
I could also have made it unref the packet's buffer at the beginning;
this would have the advantage that the packet's buffer would be freed
after the units have been rewritten (if they are rewritten) and after
the fragment's buffer has been unreferenced, so that maximum memory
consumption would decrease. It would also be in line with all current
callers of ff_cbs_write_packet, but maybe it wouldn't be what a future
caller wants. What do you think? 
 libavcodec/cbs.c |  3 ++-
 libavcodec/cbs.h | 10 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 0260ba6f67..47679eca1b 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -357,7 +357,8 @@ int ff_cbs_write_packet(CodedBitstreamContext *ctx,
 if (!buf)
 return AVERROR(ENOMEM);
 
-av_init_packet(pkt);
+av_buffer_unref(&pkt->buf);
+
 pkt->buf  = buf;
 pkt->data = frag->data;
 pkt->size = frag->data_size;
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 967dcd1468..5260a39c63 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -297,7 +297,8 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
 /**
  * Write the bitstream of a fragment to the extradata in codec parameters.
  *
- * This replaces any existing extradata in the structure.
+ * Modifies context and fragment as ff_cbs_write_fragment_data does and
+ * replaces any existing extradata in the structure.
  */
 int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
AVCodecParameters *par,
@@ -305,6 +306,13 @@ int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
 
 /**
  * Write the bitstream of a fragment to a packet.
+ *
+ * Modifies context and fragment as ff_cbs_write_fragment_data does.
+ *
+ * On success, the packet's buf is unreferenced and its buf, data and
+ * size fields are set to the corresponding values from the newly updated
+ * fragment; other fields are not touched.  On failure, the packet is not
+ * touched at all.
  */
 int ff_cbs_write_packet(CodedBitstreamContext *ctx,
 AVPacket *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 04/18] filter_units: Don't use fake loop

2019-06-16 Thread Andreas Rheinhardt
According to the BSF API, when a BSF is finished with an input packet,
it should return AVERROR(EAGAIN) to signal that another packet should be
sent to the BSF via av_bsf_send_packet that the actual BSF can receive
via ff_bsf_get_packet[_ref]. filter_units on the other hand simply called
ff_bsf_get_packet again if the first packet received didn't result in
any output. This call of course returned AVERROR(EAGAIN) which was
returned, but it is nevertheless better to not include a fake loop.

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

diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index 0876693c81..a787933f0a 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -105,7 +105,6 @@ static int filter_units_filter(AVBSFContext *bsf, AVPacket 
*out)
 AVPacket *in = NULL;
 int err, i, j;
 
-while (1) {
 err = ff_bsf_get_packet(bsf, &in);
 if (err < 0)
 return err;
@@ -134,12 +133,10 @@ static int filter_units_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
-if (frag->nb_units > 0)
-break;
-
+if (frag->nb_units == 0) {
 // Don't return packets with nothing in them.
-av_packet_free(&in);
-ff_cbs_fragment_reset(ctx->cbc, frag);
+err = AVERROR(EAGAIN);
+goto fail;
 }
 
 err = ff_cbs_write_packet(ctx->cbc, out, frag);
-- 
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/18] filter_units: Reindent after previous commit

2019-06-16 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/filter_units_bsf.c | 46 +--
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c
index a787933f0a..8c501e1726 100644
--- a/libavcodec/filter_units_bsf.c
+++ b/libavcodec/filter_units_bsf.c
@@ -105,33 +105,33 @@ static int filter_units_filter(AVBSFContext *bsf, 
AVPacket *out)
 AVPacket *in = NULL;
 int err, i, j;
 
-err = ff_bsf_get_packet(bsf, &in);
-if (err < 0)
-return err;
+err = ff_bsf_get_packet(bsf, &in);
+if (err < 0)
+return err;
 
-if (ctx->mode == NOOP) {
-av_packet_move_ref(out, in);
-av_packet_free(&in);
-return 0;
-}
+if (ctx->mode == NOOP) {
+av_packet_move_ref(out, in);
+av_packet_free(&in);
+return 0;
+}
 
-err = ff_cbs_read_packet(ctx->cbc, frag, in);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
-goto fail;
-}
+err = ff_cbs_read_packet(ctx->cbc, frag, in);
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
+goto fail;
+}
 
-for (i = 0; i < frag->nb_units; i++) {
-for (j = 0; j < ctx->nb_types; j++) {
-if (frag->units[i].type == ctx->type_list[j])
-break;
-}
-if (ctx->mode == REMOVE ? j <  ctx->nb_types
-: j >= ctx->nb_types) {
-ff_cbs_delete_unit(ctx->cbc, frag, i);
---i;
-}
+for (i = 0; i < frag->nb_units; i++) {
+for (j = 0; j < ctx->nb_types; j++) {
+if (frag->units[i].type == ctx->type_list[j])
+break;
+}
+if (ctx->mode == REMOVE ? j <  ctx->nb_types
+: j >= ctx->nb_types) {
+ff_cbs_delete_unit(ctx->cbc, frag, i);
+--i;
 }
+}
 
 if (frag->nb_units == 0) {
 // Don't return packets with nothing in them.
-- 
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/18] av1_metadata: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes av1_metadata to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

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

diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index fe208feaf5..e294d7a24e 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -116,19 +116,18 @@ static int 
av1_metadata_update_sequence_header(AVBSFContext *bsf,
 return 0;
 }
 
-static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *out)
+static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 AV1MetadataContext *ctx = bsf->priv_data;
-AVPacket *in = NULL;
 CodedBitstreamFragment *frag = &ctx->access_unit;
 AV1RawOBU td, *obu;
 int err, i;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->cbc, frag, in);
+err = ff_cbs_read_packet(ctx->cbc, frag, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -173,23 +172,18 @@ static int av1_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, frag);
+err = ff_cbs_write_packet(ctx->cbc, pkt, frag);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->cbc, frag);
 
 if (err < 0)
-av_packet_unref(out);
-av_packet_free(&in);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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 14/18] h265_metadata: Avoid allocations and copies of packet structures

2019-06-16 Thread Andreas Rheinhardt
This commit changes h265_metadata to (a) use ff_bsf_get_packet_ref
instead of ff_bsf_get_packet (thereby avoiding one malloc and free per
filtered packet) and (b) to use only one packet structure at all,
thereby avoiding a call to av_packet_copy_props.

(b) has been made possible by the recent changes to ff_cbs_write_packet.

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

diff --git a/libavcodec/h265_metadata_bsf.c b/libavcodec/h265_metadata_bsf.c
index 0683cc2f9d..2fb74aea5a 100644
--- a/libavcodec/h265_metadata_bsf.c
+++ b/libavcodec/h265_metadata_bsf.c
@@ -230,18 +230,17 @@ static int h265_metadata_update_sps(AVBSFContext *bsf,
 return 0;
 }
 
-static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *out)
+static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
 H265MetadataContext *ctx = bsf->priv_data;
-AVPacket *in = NULL;
 CodedBitstreamFragment *au = &ctx->access_unit;
 int err, i;
 
-err = ff_bsf_get_packet(bsf, &in);
+err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
 return err;
 
-err = ff_cbs_read_packet(ctx->cbc, au, in);
+err = ff_cbs_read_packet(ctx->cbc, au, pkt);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
 goto fail;
@@ -310,23 +309,18 @@ static int h265_metadata_filter(AVBSFContext *bsf, 
AVPacket *out)
 }
 }
 
-err = ff_cbs_write_packet(ctx->cbc, out, au);
+err = ff_cbs_write_packet(ctx->cbc, pkt, au);
 if (err < 0) {
 av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
 goto fail;
 }
 
-err = av_packet_copy_props(out, in);
-if (err < 0)
-goto fail;
-
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->cbc, au);
 
 if (err < 0)
-av_packet_unref(out);
-av_packet_free(&in);
+av_packet_unref(pkt);
 
 return err;
 }
-- 
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 10/18] cbs: Remove superfluous checks for ff_cbs_delete_unit

2019-06-16 Thread Andreas Rheinhardt
ff_cbs_delete_unit never fails if the index of the unit to delete is
valid; document this behaviour explicitly and remove the checks for
whether ff_cbs_delete_unit failed, because all the callers of
ff_cbs_delete_unit already make sure that the index is valid.

Signed-off-by: Andreas Rheinhardt 
---
Several callers already ignored to check the return value.

 libavcodec/av1_metadata_bsf.c   | 9 ++---
 libavcodec/cbs.h| 2 ++
 libavcodec/h264_metadata_bsf.c  | 7 +--
 libavcodec/h264_redundant_pps_bsf.c | 4 +---
 4 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
index 842b80c201..dafddced63 100644
--- a/libavcodec/av1_metadata_bsf.c
+++ b/libavcodec/av1_metadata_bsf.c
@@ -161,13 +161,8 @@ static int av1_metadata_filter(AVBSFContext *bsf, AVPacket 
*pkt)
 
 if (ctx->delete_padding) {
 for (i = frag->nb_units - 1; i >= 0; i--) {
-if (frag->units[i].type == AV1_OBU_PADDING) {
-err = ff_cbs_delete_unit(ctx->cbc, frag, i);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to delete Padding 
OBU.\n");
-goto fail;
-}
-}
+if (frag->units[i].type == AV1_OBU_PADDING)
+ff_cbs_delete_unit(ctx->cbc, frag, i);
 }
 }
 
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 5260a39c63..e1e6055ceb 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -380,6 +380,8 @@ int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
 
 /**
  * Delete a unit from a fragment and free all memory it uses.
+ *
+ * Never returns failure if position is >= 0 and < frag->nb_units.
  */
 int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index f7ca1f0f09..d05b75be14 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -428,12 +428,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 for (i = au->nb_units - 1; i >= 0; i--) {
 if (au->units[i].type == H264_NAL_FILLER_DATA) {
 // Filler NAL units.
-err = ff_cbs_delete_unit(ctx->cbc, au, i);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to delete "
-   "filler NAL.\n");
-goto fail;
-}
+ff_cbs_delete_unit(ctx->cbc, au, i);
 continue;
 }
 
diff --git a/libavcodec/h264_redundant_pps_bsf.c 
b/libavcodec/h264_redundant_pps_bsf.c
index db8717d69a..8526b5b4c4 100644
--- a/libavcodec/h264_redundant_pps_bsf.c
+++ b/libavcodec/h264_redundant_pps_bsf.c
@@ -95,9 +95,7 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
AVPacket *out)
 if (!au_has_sps) {
 av_log(bsf, AV_LOG_VERBOSE, "Deleting redundant PPS "
"at %"PRId64".\n", in->pts);
-err = ff_cbs_delete_unit(ctx->input, au, i);
-if (err < 0)
-goto fail;
+ff_cbs_delete_unit(ctx->input, au, i);
 }
 }
 if (nal->type == H264_NAL_SLICE ||
-- 
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/18] h264_metadata: Localize code for display orientation

2019-06-16 Thread Andreas Rheinhardt
The recent changes to h264_metadata (enabled by the recent changes to
ff_cbs_write_packet) made it possible to add side_data to the output
packet at any place, not only after the output packet has been written
and the properties of the input packet copied. This means that one can
now localize the code to add display orientation side-data to the packet
to the place dealing with said display-orientation.

Furthermore, the documentation of av_display_rotation_set states that
the matrix will be fully overwritten by it, so there is no need to
allocate it with av_mallocz.

Signed-off-by: Andreas Rheinhardt 
---
That the display orientation code itself is very buggy has not been
fixed yet.

 libavcodec/h264_metadata_bsf.c | 33 -
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 18c5ae807d..f7ca1f0f09 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -289,8 +289,6 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*pkt)
 CodedBitstreamFragment *au = &ctx->access_unit;
 int err, i, j, has_sps;
 H264RawAUD aud;
-uint8_t *displaymatrix_side_data = NULL;
-size_t displaymatrix_side_data_size = 0;
 
 err = ff_bsf_get_packet_ref(bsf, pkt);
 if (err < 0)
@@ -487,7 +485,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket 
*pkt)
 continue;
 }
 
-matrix = av_mallocz(9 * sizeof(int32_t));
+matrix = av_malloc(9 * sizeof(int32_t));
 if (!matrix) {
 err = AVERROR(ENOMEM);
 goto fail;
@@ -499,11 +497,17 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 av_display_matrix_flip(matrix, disp->hor_flip, disp->ver_flip);
 
 // If there are multiple display orientation messages in an
-// access unit then ignore all but the first one.
-av_freep(&displaymatrix_side_data);
-
-displaymatrix_side_data  = (uint8_t*)matrix;
-displaymatrix_side_data_size = 9 * sizeof(int32_t);
+// access unit, then the last one added to the packet (i.e.
+// the first one in the access unit) will prevail.
+err = av_packet_add_side_data(pkt, AV_PKT_DATA_DISPLAYMATRIX,
+  (uint8_t*)matrix,
+  9 * sizeof(int32_t));
+if (err < 0) {
+av_log(bsf, AV_LOG_ERROR, "Failed to attach extracted "
+   "displaymatrix side data to packet.\n");
+av_freep(matrix);
+goto fail;
+}
 }
 }
 }
@@ -583,24 +587,11 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 goto fail;
 }
 
-if (displaymatrix_side_data) {
-err = av_packet_add_side_data(pkt, AV_PKT_DATA_DISPLAYMATRIX,
-  displaymatrix_side_data,
-  displaymatrix_side_data_size);
-if (err) {
-av_log(bsf, AV_LOG_ERROR, "Failed to attach extracted "
-   "displaymatrix side data to packet.\n");
-goto fail;
-}
-displaymatrix_side_data = NULL;
-}
-
 ctx->done_first_au = 1;
 
 err = 0;
 fail:
 ff_cbs_fragment_reset(ctx->cbc, au);
-av_freep(&displaymatrix_side_data);
 
 if (err < 0)
 av_packet_unref(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 11/18] cbs_h264, h264_metadata: Deleting SEI messages never fails

2019-06-16 Thread Andreas Rheinhardt
Deleting a unit from a fragment in CBS only fails if there is no unit
in the fragment corresponding to the position given as argument to
ff_cbs_delete_unit. Given that ff_cbs_h264_delete_sei_message asserts
this to be so, we know that the call to ff_cbs_delete_unit can never
fail and hence ff_cbs_h264_delete_sei_message doesn't need a return
value at all. The existing checks for these return values can be deleted.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/cbs_h264.h  |  8 
 libavcodec/cbs_h2645.c | 12 +---
 libavcodec/h264_metadata_bsf.c | 21 +
 3 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index a31be298ba..f63c19ffc0 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -479,9 +479,9 @@ int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx,
  * Deletes from nal_unit, which must be an SEI NAL unit.  If this is the
  * last message in nal_unit, also deletes it from access_unit.
  */
-int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
-   CodedBitstreamFragment *access_unit,
-   CodedBitstreamUnit *nal_unit,
-   int position);
+void ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+CodedBitstreamFragment *access_unit,
+CodedBitstreamUnit *nal_unit,
+int position);
 
 #endif /* AVCODEC_CBS_H264_H */
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 0456937710..a3bad83736 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1644,10 +1644,10 @@ int ff_cbs_h264_add_sei_message(CodedBitstreamContext 
*ctx,
 return 0;
 }
 
-int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
-   CodedBitstreamFragment *au,
-   CodedBitstreamUnit *nal,
-   int position)
+void ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+CodedBitstreamFragment *au,
+CodedBitstreamUnit *nal,
+int position)
 {
 H264RawSEI *sei = nal->content;
 
@@ -1664,7 +1664,7 @@ int ff_cbs_h264_delete_sei_message(CodedBitstreamContext 
*ctx,
 }
 av_assert0(i < au->nb_units && "NAL unit not in access unit.");
 
-return ff_cbs_delete_unit(ctx, au, i);
+ff_cbs_delete_unit(ctx, au, i);
 } else {
 cbs_h264_free_sei_payload(&sei->payload[position]);
 
@@ -1672,7 +1672,5 @@ int ff_cbs_h264_delete_sei_message(CodedBitstreamContext 
*ctx,
 memmove(sei->payload + position,
 sei->payload + position + 1,
 (sei->payload_count - position) * sizeof(*sei->payload));
-
-return 0;
 }
 }
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index d05b75be14..c7969f1152 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -438,15 +438,9 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 
 for (j = sei->payload_count - 1; j >= 0; j--) {
 if (sei->payload[j].payload_type ==
-H264_SEI_TYPE_FILLER_PAYLOAD) {
-err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
- &au->units[i], j);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to delete "
-   "filler SEI message.\n");
-goto fail;
-}
-}
+H264_SEI_TYPE_FILLER_PAYLOAD)
+ff_cbs_h264_delete_sei_message(ctx->cbc, au,
+   &au->units[i], j);
 }
 }
 }
@@ -470,13 +464,8 @@ static int h264_metadata_filter(AVBSFContext *bsf, 
AVPacket *pkt)
 
 if (ctx->display_orientation == REMOVE ||
 ctx->display_orientation == INSERT) {
-err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
- &au->units[i], j);
-if (err < 0) {
-av_log(bsf, AV_LOG_ERROR, "Failed to delete "
-   "display orientation SEI message.\n");
-goto fail;
-}
+ff_cbs_h264_delete_sei_message(ctx->cbc, au,
+   &au->units[i], j);
 continue;
 }
 
-- 
2.21.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffm

Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: fix ref-frame size judging method

2019-06-16 Thread yancen
Hi, RonaldHave you see the reply of Haihao?
He mentioned that decode_frame_header() has use s->s.refs[s->s.h.refidx[i]].f 
without safe check. 
For this suggestion, We must avoid the point on vp9.c 794-795.Could I restore 
the null point in decode_frame_header()? 
Thanks,Cen
 原始信息 由: "Xiang, Haihao"  日期: 2019/5/27 
 12:53  (GMT+08:00) 收件人: rsbul...@gmail.com, ffmpeg-devel@ffmpeg.org 抄送: "Yan, 
CenX"  主题: Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: fix 
ref-frame size judging method 
On Mon, 2019-05-27 at 04:44 +, Xiang, Haihao wrote:
> On Thu, 2019-05-23 at 08:12 -0400, Ronald S. Bultje wrote:
> Hi guys,
> 
> On Wed, May 22, 2019 at 11:14 PM 严小复  mryan...@gmail.com>> wrote:
> code”, I'm little confused about the red word,would you mean encode process
> need validity checks or there need to check the reference id of each frame?
> 
> And this patch will act on decode process, all references have already been
> appointed by the stream.
> 
> No. As said before, the *decode* process needs such checks.
> 
> But since you don't seem to understand, let me try to be more helpful.
> 
> If you have an array of references, and we refer to that as s->s.refs[8], in
> which some reference is missing, e.g. s->s.refs[5] is NULL (but the rest is
> fine).
> 

Do you mean s->s.refs[5].f ? s->s.refs[5] is not a pointer.

> 
> Now let's also say that we have a header in s->s.h in which there is an array
> of reference indices s->s.h.refidx[7] assigned as per the "spec". Let's
> imagine that we encounter a frame in which s->s.refs[5] is used as an active
> reference in this frame, for example s->s.h.refidx[0] = 5. Right now, with the
> code in git/master, we abort decoding. Your patch will make it continue
> decoding.
> 

If so, even without Cen's patch, there is still a similar issue because the
reference is used without any check in decode_frame_header () in vp9.c line 794-
795

    AVFrame *ref = s->s.refs[s->s.h.refidx[i]].f;
    int refw = ref->width, refh = ref->height;

So I presumed s->s.refs[s->s.h.refidx[i]].f is assigned somewhere, otherwise we
must add a check here.

> 
> 
> So now, imagine that we encounter, in this frame, an inter block in which we
> use this reference (so b->ref[0] = 0, which means s->s.h.refidx[b->ref[0]] =
> 5), and have prediction code that does something like vp9_mc_template.c line
> 39:
> 
> ThreadFrame *tref1 = &s->s.refs[s->s.h.refidx[b->ref[0]]];
> 
> Then later on this code will call a function which may in some cases be called
> mc_luma_dir() as per vp9_mc_template.c line 413, which is implemented in
> vp9recon.c line 298 in the function called mc_luma_unscaled() (see #define on
> line 383 of same file). This function now calls a DSP function in line 331:
> 
> mc[!!mx][!!my](dst, dst_stride, ref, ref_stride, bh, mx << 1, my << 1);
> 
> which directly accesses the reference pixels (see e.g. vp9dsp_template.c line
> 2044).
> 
> Note how especially *none of these functions* check anywhere whether s-
> >s.refs[s->s.h.refidx[b->ref[0]]] (or tref1) is NULL. They don't do that,
> because ... the header check already did that, so the check was redundant.
> 
> But! You are *removing* that header check, so in this brave new world which
> will exist after applying your patch, what will happen is that I can craft a
> special stream where s->s.refs[5] becomes NULL, then send a subsequent frame
> using refs[5] by having s->s.h.refidx[0] = 5, then have a frame in which the
> block uses inter coding and uses reference 0 so that this causes access into
> s->s.refs[s->s.h.refidx[b->ref[0, which is a NULL pointer, which is
> undefined behaviour by the C standard, which means a bad person could craft
> such a stream that would allow this bad person to break into your computer and
> steal all your data. In more straightforward cases, it might also crash
> Firefox and VLC, which use the FFmpeg VP9 decoder.
> 
> Note also that having your data stolen or your application crashing is
> considered *bad*. We *don't want that*. Therefore, as I've tried to say a few
> times already, if you remove the header check, you need to add a per-block
> check instead, so that Firefox/VLC don't crash and so that bad persons cannot
> steal your data.
> 
> Please add such a check and test using a fuzzer that the check prevents
> crashes as described above. Similar checks may be needed in other places also,
> since there's multiple places where the software decoder does MC and accesses
> references. Good luck finding these places by reading the code and fuzzing
> away.
> 
> HTH, and please ask more questions (on IRC please, #ffmpeg-devel on Freenode)
> if this is still unclear.
> Ronald
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-de

Re: [FFmpeg-devel] [PATCH 0/5 v2] New version

2019-06-16 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Hello,
> 
> I have added the necessary numbers for the index version (which you can
> find at https://github.com/mkver/FFmpeg/commits/start_6 if you care) to
> the commit message of the first patch and have also adapted the
> overread check as you suggested (no performance impact whatsoever
> from this).
> 
> - Andreas
> 
> Andreas Rheinhardt (5):
>   startcode: Use common macro and switch to pointer arithmetic
>   startcode: Switch to aligned reads
>   startcode: Stop overreading
>   startcode: Don't return false positives
>   startcode: Filter out non-startcodes earlier
> 
>  libavcodec/h264dsp.h   |   7 +--
>  libavcodec/startcode.c | 133 ++---
>  libavcodec/vc1dsp.h|   6 +-
>  3 files changed, 117 insertions(+), 29 deletions(-)
> 
Ping.

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

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

Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: add support for AVC Trellis

2019-06-16 Thread Fu, Linjie


> -Original Message-
> From: myp...@gmail.com [mailto:myp...@gmail.com]
> Sent: Wednesday, June 12, 2019 16:14
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/vaapi_encode: add support for
> AVC Trellis
> 
> On Wed, Jun 12, 2019 at 3:28 PM Linjie Fu  wrote:
> >
> > Add support for VAAPI AVC Trellis Quantization with limitation:
> > - VA-API version >= (1, 0, 0)
> >
> > Use option "-trellis off/I/P/B" to disable or enable Trellis
> > quantization for I/P/B frames.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> > [v2]: Since nonstandard struct for VAEncMiscParameterQuantization is
> > fixed: https://github.com/intel/libva/issues/265
> > update patch based on:
> >
> http://git.ffmpeg.org/gitweb/ffmpeg.git/commit/2880a32c668023bfee47450
> 95c885450d547ae45
> >  libavcodec/vaapi_encode.c  | 48
> ++
> >  libavcodec/vaapi_encode.h  |  9 +--
> >  libavcodec/vaapi_encode_h264.c |  9 +++
> >  3 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > index dd2a24de04..fbfbe78c6b 100644
> > --- a/libavcodec/vaapi_encode.c
> > +++ b/libavcodec/vaapi_encode.c
> > @@ -1671,6 +1671,48 @@ rc_mode_found:
> >  return 0;
> >  }
> >
> > +static av_cold int vaapi_encode_init_quantization(AVCodecContext
> *avctx)
> > +{
> > +#if VA_CHECK_VERSION(1, 0, 0)
> > +VAAPIEncodeContext *ctx = avctx->priv_data;
> > +VAStatus vas;
> > +VAConfigAttrib attr = { VAConfigAttribEncQuantization };
> > +int trellis = ctx->trellis;
> > +
> > +vas = vaGetConfigAttributes(ctx->hwctx->display,
> > +ctx->va_profile,
> > +ctx->va_entrypoint,
> > +&attr, 1);
> > +if (vas != VA_STATUS_SUCCESS) {
> > +av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "
> > +   "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
> > +return AVERROR_EXTERNAL;
> > +}
> > +
> > +if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||
> > +attr.value == VA_ENC_QUANTIZATION_NONE) {
> > +av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is
> not "
> > +"supported: will use default quantization.\n");
> > +} else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){
> > +av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");
> > +
> > +ctx->quantization_params = (VAEncMiscParameterQuantization) {
> > +.quantization_flags.value = trellis,
> > +};
> > +
> > +vaapi_encode_add_global_param(avctx,
> > +  VAEncMiscParameterTypeQuantization,
> > +  &ctx->quantization_params,
> > +  sizeof(ctx->quantization_params));
> > +}
> > +#else
> > +av_log(avctx, AV_LOG_WARNING, "The encode quantization option
> (Trellis) is "
> > +   "not supported with this VAAPI version.\n");
> > +#endif
> > +
> > +return 0;
> > +}
> > +
> >  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext
> *avctx)
> >  {
> >  VAAPIEncodeContext *ctx = avctx->priv_data;
> > @@ -2132,6 +2174,12 @@ av_cold int
> ff_vaapi_encode_init(AVCodecContext *avctx)
> >  if (err < 0)
> >  goto fail;
> >
> > +if (ctx->trellis) {
> > +err = vaapi_encode_init_quantization(avctx);
> > +if (err < 0)
> > +goto fail;
> > +}
> > +
> >  if (avctx->compression_level >= 0) {
> >  err = vaapi_encode_init_quality(avctx);
> >  if (err < 0)
> > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> > index eeec06036b..b24735da59 100644
> > --- a/libavcodec/vaapi_encode.h
> > +++ b/libavcodec/vaapi_encode.h
> > @@ -37,7 +37,7 @@ struct VAAPIEncodePicture;
> >
> >  enum {
> >  MAX_CONFIG_ATTRIBUTES  = 4,
> > -MAX_GLOBAL_PARAMS  = 4,
> > +MAX_GLOBAL_PARAMS  = 5,
> >  MAX_DPB_SIZE   = 16,
> >  MAX_PICTURE_REFERENCES = 2,
> >  MAX_REORDER_DELAY  = 16,
> > @@ -220,6 +220,9 @@ typedef struct VAAPIEncodeContext {
> >  // Packed headers which will actually be sent.
> >  unsigned intva_packed_headers;
> >
> > +// Quantization mode
> > +int trellis;
> > +
> >  // Configuration attributes to use when creating va_config.
> >  VAConfigAttrib  config_attributes[MAX_CONFIG_ATTRIBUTES];
> >  int  nb_config_attributes;
> > @@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {
> >  #if VA_CHECK_VERSION(0, 36, 0)
> >  VAEncMiscParameterBufferQualityLevel quality_params;
> >  #endif
> > -
> > +#if VA_CHECK_VERSION(1, 0, 0)
> > +VAEncMiscParameterQuantization quantization_params;
> > +#endif
> >  // Per-sequence parameter structure
> (VAEncSequenceParameterBuffer*).
> >  vo

[FFmpeg-devel] [PATCH v3, 1/2] lavc/vaapi_encode: add support for AVC Trellis

2019-06-16 Thread Linjie Fu
Trellis quantization is an algorithm that can improve data compression
in DCT-based encoding methods. It reduces the size of some DCT
coefficients while recovering others to take their place.

Trellis quantization effectively finds the optimal quantization for
each block to maximize the PSNR relative to bitrate.

Add support for VAAPI AVC Trellis Quantization with limitation:
- VA-API version >= (1, 5, 0)

Use option "-trellis off/I/P/B" to disable or enable Trellis
quantization for I/P/B frames.

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_encode.c | 47 +++
 libavcodec/vaapi_encode.h | 19 +---
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index dd2a24de04..394c824069 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1671,6 +1671,47 @@ rc_mode_found:
 return 0;
 }
 
+static av_cold int vaapi_encode_init_quantization(AVCodecContext *avctx)
+{
+#if VA_CHECK_VERSION(1, 5, 0)
+VAAPIEncodeContext *ctx = avctx->priv_data;
+VAStatus vas;
+VAConfigAttrib attr = { VAConfigAttribEncQuantization };
+
+vas = vaGetConfigAttributes(ctx->hwctx->display,
+ctx->va_profile,
+ctx->va_entrypoint,
+&attr, 1);
+if (vas != VA_STATUS_SUCCESS) {
+av_log(avctx, AV_LOG_ERROR, "Failed to query quantization "
+   "config attribute: %d (%s).\n", vas, vaErrorStr(vas));
+return AVERROR_EXTERNAL;
+}
+
+if (attr.value == VA_ATTRIB_NOT_SUPPORTED ||
+attr.value == VA_ENC_QUANTIZATION_NONE) {
+av_log(avctx, AV_LOG_WARNING, "Special Quantization attribute is not "
+"supported: will use default quantization.\n");
+} else if (attr.value == VA_ENC_QUANTIZATION_TRELLIS_SUPPORTED){
+av_log(avctx, AV_LOG_VERBOSE, "Quantization Trellis supported.\n");
+
+ctx->quantization_params = (VAEncMiscParameterQuantization) {
+.quantization_flags.value = ctx->trellis,
+};
+
+vaapi_encode_add_global_param(avctx,
+  VAEncMiscParameterTypeQuantization,
+  &ctx->quantization_params,
+  sizeof(ctx->quantization_params));
+}
+#else
+av_log(avctx, AV_LOG_WARNING, "The encode quantization option (Trellis) is 
"
+   "not supported with this VAAPI version.\n");
+#endif
+
+return 0;
+}
+
 static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
@@ -2132,6 +2173,12 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
 if (err < 0)
 goto fail;
 
+if (ctx->trellis) {
+err = vaapi_encode_init_quantization(avctx);
+if (err < 0)
+goto fail;
+}
+
 if (avctx->compression_level >= 0) {
 err = vaapi_encode_init_quality(avctx);
 if (err < 0)
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index eeec06036b..9b7f29a2d2 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -37,7 +37,7 @@ struct VAAPIEncodePicture;
 
 enum {
 MAX_CONFIG_ATTRIBUTES  = 4,
-MAX_GLOBAL_PARAMS  = 4,
+MAX_GLOBAL_PARAMS  = 5,
 MAX_DPB_SIZE   = 16,
 MAX_PICTURE_REFERENCES = 2,
 MAX_REORDER_DELAY  = 16,
@@ -176,6 +176,9 @@ typedef struct VAAPIEncodeContext {
 // Desired B frame reference depth.
 int desired_b_depth;
 
+// Quantization mode
+int trellis;
+
 // Explicitly set RC mode (otherwise attempt to pick from
 // available modes).
 int explicit_rc_mode;
@@ -256,7 +259,9 @@ typedef struct VAAPIEncodeContext {
 #if VA_CHECK_VERSION(0, 36, 0)
 VAEncMiscParameterBufferQualityLevel quality_params;
 #endif
-
+#if VA_CHECK_VERSION(1, 5, 0)
+VAEncMiscParameterQuantization quantization_params;
+#endif
 // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
 void   *codec_sequence_params;
 
@@ -418,7 +423,15 @@ int ff_vaapi_encode_close(AVCodecContext *avctx);
 { "b_depth", \
   "Maximum B-frame reference depth", \
   OFFSET(common.desired_b_depth), AV_OPT_TYPE_INT, \
-  { .i64 = 1 }, 1, INT_MAX, FLAGS }
+  { .i64 = 1 }, 1, INT_MAX, FLAGS }, \
+{ "trellis", \
+  "Trellis Quantization", \
+  OFFSET(common.trellis), AV_OPT_TYPE_FLAGS, \
+  { .i64 = 0}, 0, INT_MAX, FLAGS, "trellis"}, \
+  { "off",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, 
FLAGS, "trellis"}, \
+  { "I", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX, 
FLAGS, "trellis"}, \
+  { "P", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 }, INT_MIN, INT_MAX, 
FLAGS, "trellis"}, \
+  { "B", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 8 }, INT_MIN, INT_MAX,

[FFmpeg-devel] [PATCH 2/2] doc/encoders.texi: add docs for trellis

2019-06-16 Thread Linjie Fu
Signed-off-by: Linjie Fu 
---
 doc/encoders.texi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index eefd124751..2e54dcc15f 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2899,6 +2899,13 @@ will refer only to P- or I-frames.  When set to greater 
values multiple layers
 of B-frames will be present, frames in each layer only referring to frames in
 higher layers.
 
+@item trellis
+Trellis quantization can improve data compression in DCT-based encoding 
methods.
+It reduces the size of some DCT coefficients while recovering others to take 
their
+place. This process can increase quality by effectively finding the optimal
+quantization for each block to maximize the PSNR relative to bitrate.
+Set to off/I/P/B to disable or enable trellis quantization for I/P/B frames.
+
 @item rc_mode
 Set the rate control mode to use.  A given driver may only support a subset of
 modes.
-- 
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".