Re: [FFmpeg-devel] [PATCH 3/3] avcodec/truemotion2: Check huffman code max bits

2018-11-18 Thread Michael Niedermayer
On Sun, Nov 18, 2018 at 11:29:10PM +0100, Tomas Härdin wrote:
> lör 2018-11-17 klockan 03:01 +0100 skrev Michael Niedermayer:
> > Fixes: Timeout
> > Fixes: 
> > 10984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-6643310750859264
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/truemotion2.c | 15 ++-
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> > index c583ff4032..5d12a70839 100644
> > --- a/libavcodec/truemotion2.c
> > +++ b/libavcodec/truemotion2.c
> > @@ -114,7 +114,7 @@ typedef struct TM2Huff {
> >  
> >  static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, 
> > TM2Huff *huff)
> 
> Since you seem to be changing what this function returns, a comment
> saying what it returns would be nice

will document this better

thanks 

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

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


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


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/truemotion2: fix integer overflows in tm2_low_chroma()

2018-11-18 Thread Michael Niedermayer
On Sun, Nov 18, 2018 at 11:32:21PM +0100, Tomas Härdin wrote:
> lör 2018-11-17 klockan 03:01 +0100 skrev Michael Niedermayer:
> > Fixes: 
> > 11295/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-4888953459572736
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/truemotion2.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> > index 58a577f53c..c583ff4032 100644
> > --- a/libavcodec/truemotion2.c
> > +++ b/libavcodec/truemotion2.c
> > @@ -484,7 +484,7 @@ static inline void tm2_high_chroma(int *data, int 
> > stride, int *last, unsigned *C
> >  }
> >  }
> >  
> > -static inline void tm2_low_chroma(int *data, int stride, int *clast, int 
> > *CD, int *deltas, int bx)
> > +static inline void tm2_low_chroma(int *data, int stride, int *clast, 
> > unsigned *CD, int *deltas, int bx)
> >  {
> >  int t;
> >  int l;
> > @@ -494,8 +494,8 @@ static inline void tm2_low_chroma(int *data, int 
> > stride, int *clast, int *CD, in
> >  prev = clast[-3];
> >  else
> >  prev = 0;
> > -t= (CD[0] + CD[1]) >> 1;
> > -l= (prev - CD[0] - CD[1] + clast[1]) >> 1;
> > +t= (int)(CD[0] + CD[1]) >> 1;
> 
> I presume the old code would overflow for sums exceeding INT_MAX. 

There were overflows in the sense of undefined behavior, yes


> Why
> then is there a cast to int before the shift and not after?

because shifts of signed and unsigned values produce different results

maybe the unsigned type confused you. CD is signed in a semantic sense its
stored in a unsigned type because otherwise this code has undefined behavior.
It would be better to use a type with different name but that had lead to
long arguments that went nowhere in the past. So i tend to use plain
unsigned now for these kind of cases as it seems that is what most people
prefer.

thx

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


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


Re: [FFmpeg-devel] [PATCH]configure: Add -Wno-char-subscripts

2018-11-18 Thread Carl Eugen Hoyos
2018-11-18 23:25 GMT+01:00, Tomas Härdin :
> sön 2018-11-18 klockan 04:40 +0100 skrev Carl Eugen Hoyos:
>> Hi!
>>
>> On systems with signed char, the compiler cannot distinguish between
>> (intended) int8_t used as subscript and unintended char, therefore the
>> warning doesn't help.
>
> Is this done anywhere in the codebase currently?

Not sure I understand correctly:
I don't think there is a "char" used as subscript (or we would see the
warning on standard x86 Linux systems), I suspect the "uint8_t" we
often use as subscripts are translated into "unsigned char" and
never trigger the warning but - afaict from looking at the code and
the many warnings on sunos - wherever FFmpeg uses "int8_t" as
subscript (which happens often in the code and I assume it happens
intentionally) the warning shows because of a translation into "char"
on such a system where char is signed.

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


Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support

2018-11-18 Thread Carl Eugen Hoyos
2018-11-18 23:50 GMT+01:00, Kieran O Leary :
> On Sun, 18 Nov 2018, 21:58 Martin Vignali 
>> I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli,
>> and I don't know, what the general "rules" of this project for multiple
>> pix_fmt output for the same input file.
>>
>> If user option for decoding precision need to be remove, i think
>> 422 need to stay in 10b decoding, and 444 need to be decode in
>> 12b.
>
> What happens if a 10-bit 444 file is decoded with 12-bit precision?
> Is this just a resampling up to 12-bit for no real benefit?

Iiuc, it is not possible to detect if the original input was 10 or 12 bit.

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


Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support

2018-11-18 Thread Carl Eugen Hoyos
2018-11-18 22:58 GMT+01:00, Martin Vignali :
> Le dim. 18 nov. 2018 à 01:57, Carl Eugen Hoyos  a
> écrit :
>
>> 2018-11-18 0:28 GMT+01:00, Martin Vignali :
>>
>> > 012 : Add 12b support for 444 by default,
>>
>> Is it slower?
>> I believe that once 12 bit decoding is not slower, it should
>> be the default for 422.
>>
>
> Yes 12b is much slower
>
> On a 422 HQ file :
> 10b decoding precision :
> frame= 3316 fps=280 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A
> speed=11.2x
> bench: utime=88.741s stime=1.257s rtime=11.833s
>
> 12b decoding precision :
> frame= 3316 fps=176 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A
> speed=7.02x
> bench: utime=145.096s stime=0.769s rtime=18.894s

Thank you for explaining and after rereading the ticket, I agree
that the defaults you suggested make sense.

>> > and add user option for setting decoding precision
>>
>> I wonder if this is necessary and correct: Calling applications
>> should choose the bit depth depending on their use case, if
>> ffmpeg (the cli) really doesn't support that, it is a missing
>> feature, different pix_fmts for one decoder (and input file) is
>> not new in FFmpeg, MPlayer had used this feature since
>> forever.
>
> I don't know, how to get the "wanted" pix_fmt output using
> ffmpeg cli, and I don't know, what the general "rules" of this
> project for multiple pix_fmt output for the same input file.

That's exactly what I am trying to say:
We both assume that there is a bug (or missing feature) in the
ffmpeg cli, but this should not be a reason to add a codec-specific
option, the cli should be fixed instead.

> If user option for decoding precision need to be remove,
> i think 422 need to stay in 10b decoding, and 444 need to be
> decode in 12b.

Then the simple option may be to hardcode this pix_fmts for the
moment, but maybe sorting them is already enough.

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


Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffprobe: fix max_bit_rate dump.

2018-11-18 Thread myp...@gmail.com
On Mon, Nov 19, 2018 at 6:03 AM Moritz Barsnick  wrote:
>
> > +if (dec_ctx->rc_max_rate > 0) print_val ("max_bit_rate", 
> > dec_ctx->rc_max_rate, unit_bit_per_second_str);
> > +else  print_str_opt("max_bit_rate", "N/A");
> >  if (dec_ctx && dec_ctx->bits_per_raw_sample > 0) 
> > print_fmt("bits_per_raw_sample", "%d", dec_ctx->bits_per_raw_sample);
>
> If the (now) second condition needs to check for validity of dec_ctx,
> shouldn't the new first one also need to check?
>
>
Ha, I guess I missed the check, will update, Thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support

2018-11-18 Thread Kieran O Leary
On Sun, 18 Nov 2018, 21:58 Martin Vignali 
> I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli, and
> I don't know, what the general "rules" of this project for multiple pix_fmt
> output for the same input file.
>
> If user option for decoding precision need to be remove,
> i think 422 need to stay in 10b decoding, and 444 need to be decode in 12b.
>

What happens if a 10-bit 444 file is decoded with 12-bit precision? Is this
just a resampling up to 12-bit for no real benefit?

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


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/truemotion2: fix integer overflows in tm2_low_chroma()

2018-11-18 Thread Tomas Härdin
lör 2018-11-17 klockan 03:01 +0100 skrev Michael Niedermayer:
> Fixes: 
> 11295/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-4888953459572736
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/truemotion2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> index 58a577f53c..c583ff4032 100644
> --- a/libavcodec/truemotion2.c
> +++ b/libavcodec/truemotion2.c
> @@ -484,7 +484,7 @@ static inline void tm2_high_chroma(int *data, int stride, 
> int *last, unsigned *C
>  }
>  }
>  
> -static inline void tm2_low_chroma(int *data, int stride, int *clast, int 
> *CD, int *deltas, int bx)
> +static inline void tm2_low_chroma(int *data, int stride, int *clast, 
> unsigned *CD, int *deltas, int bx)
>  {
>  int t;
>  int l;
> @@ -494,8 +494,8 @@ static inline void tm2_low_chroma(int *data, int stride, 
> int *clast, int *CD, in
>  prev = clast[-3];
>  else
>  prev = 0;
> -t= (CD[0] + CD[1]) >> 1;
> -l= (prev - CD[0] - CD[1] + clast[1]) >> 1;
> +t= (int)(CD[0] + CD[1]) >> 1;

I presume the old code would overflow for sums exceeding INT_MAX. Why
then is there a cast to int before the shift and not after?

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


Re: [FFmpeg-devel] [PATCH 3/3] avcodec/truemotion2: Check huffman code max bits

2018-11-18 Thread Tomas Härdin
lör 2018-11-17 klockan 03:01 +0100 skrev Michael Niedermayer:
> Fixes: Timeout
> Fixes: 
> 10984/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TRUEMOTION2_fuzzer-6643310750859264
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/truemotion2.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> index c583ff4032..5d12a70839 100644
> --- a/libavcodec/truemotion2.c
> +++ b/libavcodec/truemotion2.c
> @@ -114,7 +114,7 @@ typedef struct TM2Huff {
>  
>  static int tm2_read_tree(TM2Context *ctx, uint32_t prefix, int length, 
> TM2Huff *huff)

Since you seem to be changing what this function returns, a comment
saying what it returns would be nice

Don't have much else to say about the patch since I don't know this
format

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


Re: [FFmpeg-devel] [PATCH]configure: Add -Wno-char-subscripts

2018-11-18 Thread Tomas Härdin
sön 2018-11-18 klockan 04:40 +0100 skrev Carl Eugen Hoyos:
> Hi!
> 
> On systems with signed char, the compiler cannot distinguish between
> (intended) int8_t used as subscript and unintended char, therefore the
> warning doesn't help.

Is this done anywhere in the codebase currently? Either way, probably a
good idea

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


Re: [FFmpeg-devel] [PATCH] avutil/mem: Correct documentation of av_fast_*alloc(z)

2018-11-18 Thread Michael Niedermayer
On Sun, Nov 18, 2018 at 03:08:45PM +0100, Andreas Rheinhardt wrote:
> The current wording regarding size and min_size is completely wrong and
> ignores that min_size is indeed only a desired minimal size, not the
> actually allocated size.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavutil/mem.h | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)

will apply

thanks

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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


Re: [FFmpeg-devel] [PATCH 3/3] libaomenc: Drop unused noise-sensitivity option

2018-11-18 Thread Mark Thompson
On 18/11/18 18:34, Jan Ekström wrote:
> On Mon, Nov 5, 2018 at 4:53 PM Mark Thompson  wrote:
>>
>> ---
>> On 28/10/18 21:02, Carl Eugen Hoyos wrote:
>>> 2018-10-28 21:08 GMT+01:00, Mark Thompson :
 ---
 This was in the 4.0 release, unfortunately (and did nothing there as well).
>>>
>>> If it has never worked, it can be removed imo.
>>
>> Sure, here is a new version.
>>
>> Thanks,
>>
>> - Mark
>>
>>
>>  libavcodec/libaomenc.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
>> index c5458766cb..4cad053a48 100644
>> --- a/libavcodec/libaomenc.c
>> +++ b/libavcodec/libaomenc.c
>> @@ -71,7 +71,6 @@ typedef struct AOMEncoderContext {
>>  int crf;
>>  int static_thresh;
>>  int drop_threshold;
>> -int noise_sensitivity;
>>  uint64_t sse[4];
>>  int have_sse; /**< true if we have pending sse[] */
>>  uint64_t frame_number;
>> @@ -980,7 +979,6 @@ static const AVOption options[] = {
>>  { "crf",  "Select the quality for constant quality mode", 
>> offsetof(AOMContext, crf), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 63, VE },
>>  { "static-thresh","A change threshold on blocks below which they 
>> will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { 
>> .i64 = 0 }, 0, INT_MAX, VE },
>>  { "drop-threshold",   "Frame drop threshold", offsetof(AOMContext, 
>> drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE },
>> -{ "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), 
>> AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE},
>>  { "tiles","Tile columns x rows", OFFSET(tile_cols), 
>> AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, VE },
>>  { "tile-columns", "Log2 of number of tile columns to use", 
>> OFFSET(tile_cols_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>>  { "tile-rows","Log2 of number of tile rows to use",
>> OFFSET(tile_rows_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>> --
>> 2.19.1
>>
> 
> Only two references to noise_sensitivity outside of libvpxenc. LGTM.

Applied.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 2/5] fftools/ffprobe: fix max_bit_rate dump.

2018-11-18 Thread Moritz Barsnick
> +if (dec_ctx->rc_max_rate > 0) print_val ("max_bit_rate", 
> dec_ctx->rc_max_rate, unit_bit_per_second_str);
> +else  print_str_opt("max_bit_rate", "N/A");
>  if (dec_ctx && dec_ctx->bits_per_raw_sample > 0) 
> print_fmt("bits_per_raw_sample", "%d", dec_ctx->bits_per_raw_sample);

If the (now) second condition needs to check for validity of dec_ctx,
shouldn't the new first one also need to check?

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


Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support

2018-11-18 Thread Martin Vignali
Le dim. 18 nov. 2018 à 01:57, Carl Eugen Hoyos  a
écrit :

> 2018-11-18 0:28 GMT+01:00, Martin Vignali :
>
> > 012 : Add 12b support for 444 by default,
>
> Is it slower?
> I believe that once 12 bit decoding is not slower, it should
> be the default for 422.
>

Yes 12b is much slower

On a 422 HQ file :
10b decoding precision :
frame= 3316 fps=280 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A
speed=11.2x
bench: utime=88.741s stime=1.257s rtime=11.833s

12b decoding precision :
frame= 3316 fps=176 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A
speed=7.02x
bench: utime=145.096s stime=0.769s rtime=18.894s


>
> > and add user option for setting decoding precision
>
> I wonder if this is necessary and correct: Calling applications
> should choose the bit depth depending on their use case, if
> ffmpeg (the cli) really doesn't support that, it is a missing
> feature, different pix_fmts for one decoder (and input file) is
> not new in FFmpeg, MPlayer had used this feature since
> forever.
>
>

I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli, and
I don't know, what the general "rules" of this project for multiple pix_fmt
output for the same input file.

If user option for decoding precision need to be remove,
i think 422 need to stay in 10b decoding, and 444 need to be decode in 12b.

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


Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support

2018-11-18 Thread Martin Vignali
>
> Are all 444(4?) Encodings 12-bit? Also is there a way to verify if your
> file should be decoded as 10 or 12-bit? Or does this code automate this
> detection?
>
>
The auto mode of the patch, only use codec_tag to switch between 10b and
12b, no other value.
I don't think, prores have metadata for encoding precision.

Apple doc say : 444 are 12b and 422 are 10b, so these value by default
seems reasonable to me,
even if the file is not encode with that precision (like using ffmpeg
prores_aw/ks encoder)

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


Re: [FFmpeg-devel] [PATCH 4/6] h264_redundant_pps: Fix memleak in case of errors

2018-11-18 Thread Mark Thompson
On 09/11/18 05:31, Andreas Rheinhardt wrote:
> Now the fragment is uninitialized and the input packet freed in case of
> errors.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/h264_redundant_pps_bsf.c | 46 ++---
>  1 file changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/libavcodec/h264_redundant_pps_bsf.c 
> b/libavcodec/h264_redundant_pps_bsf.c
> index cc5a3060f5..79a11c0e30 100644
> --- a/libavcodec/h264_redundant_pps_bsf.c
> +++ b/libavcodec/h264_redundant_pps_bsf.c
> @@ -76,11 +76,11 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  
>  err = ff_bsf_get_packet(bsf, &in);
>  if (err < 0)
> -return err;
> +goto fail;

Not this one - you only want to call fragment_uninit() if it fails with a 
fragment in hand.

>  
>  err = ff_cbs_read_packet(ctx->input, au, in);
>  if (err < 0)
> -return err;
> +goto fail;
>  
>  au_has_sps = 0;
>  for (i = 0; i < au->nb_units; i++) {
> @@ -89,11 +89,15 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  if (nal->type == H264_NAL_SPS)
>  au_has_sps = 1;
>  if (nal->type == H264_NAL_PPS) {
> -h264_redundant_pps_fixup_pps(ctx, nal->content);
> +err = h264_redundant_pps_fixup_pps(ctx, nal->content);
> +if (err < 0)
> +goto fail;
>  if (!au_has_sps) {
>  av_log(ctx, AV_LOG_VERBOSE, "Deleting redundant PPS "
> "at %"PRId64".\n", in->pts);
> -ff_cbs_delete_unit(ctx->input, au, i);
> +err = ff_cbs_delete_unit(ctx->input, au, i);
> +if (err < 0)
> +goto fail;
>  }
>  }
>  if (nal->type == H264_NAL_SLICE ||
> @@ -105,17 +109,21 @@ static int h264_redundant_pps_filter(AVBSFContext *bsf, 
> AVPacket *out)
>  
>  err = ff_cbs_write_packet(ctx->output, out, au);
>  if (err < 0)
> -return err;
> +goto fail;
>  
> -ff_cbs_fragment_uninit(ctx->output, au);
>  
>  err = av_packet_copy_props(out, in);
>  if (err < 0)
> -return err;
> +goto fail;
>  
> +err = 0;
> +fail:
> +ff_cbs_fragment_uninit(ctx->output, au);
>  av_packet_free(&in);
> +if (err < 0)
> +av_packet_unref(out);
>  
> -return 0;
> +return err;
>  }
>  
>  static int h264_redundant_pps_init(AVBSFContext *bsf)
> @@ -126,11 +134,11 @@ static int h264_redundant_pps_init(AVBSFContext *bsf)
>  
>  err = ff_cbs_init(&ctx->input, AV_CODEC_ID_H264, bsf);
>  if (err < 0)
> -return err;
> +goto fail;
>  
>  err = ff_cbs_init(&ctx->output, AV_CODEC_ID_H264, bsf);
>  if (err < 0)
> -return err;
> +goto fail;
>  
>  ctx->global_pic_init_qp = 26;
>  
> @@ -138,25 +146,29 @@ static int h264_redundant_pps_init(AVBSFContext *bsf)
>  err = ff_cbs_read_extradata(ctx->input, au, bsf->par_in);
>  if (err < 0) {
>  av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");
> -return err;
> +goto fail;
>  }
>  
>  for (i = 0; i < au->nb_units; i++) {
> -if (au->units[i].type == H264_NAL_PPS)
> -h264_redundant_pps_fixup_pps(ctx, au->units[i].content);
> +if (au->units[i].type == H264_NAL_PPS) {
> +err = h264_redundant_pps_fixup_pps(ctx, 
> au->units[i].content);
> +if (err < 0)
> +goto fail;
> +}
>  }
>  
>  ctx->extradata_pic_init_qp = ctx->current_pic_init_qp;
>  err = ff_cbs_write_extradata(ctx->output, bsf->par_out, au);
>  if (err < 0) {
>  av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n");
> -return err;
> +goto fail;
>  }
> -
> -ff_cbs_fragment_uninit(ctx->output, au);
>  }
>  
> -return 0;
> +err = 0;
> +fail:
> +ff_cbs_fragment_uninit(ctx->output, au);
> +return err;
>  }
>  
>  static void h264_redundant_pps_flush(AVBSFContext *bsf)
> 

Applied with that changed.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 3/6] cbs_h2645: Create functions to make parameter sets writeable

2018-11-18 Thread Mark Thompson
On 09/11/18 05:31, Andreas Rheinhardt wrote:
> av_buffer_make_writable can't be used here, because a shallow copy isn't
> enough for parameter sets.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cbs_h264.h  |  8 
>  libavcodec/cbs_h2645.c | 16 
>  libavcodec/cbs_h265.h  |  9 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
> index 92277e4750..dc6019765b 100644
> --- a/libavcodec/cbs_h264.h
> +++ b/libavcodec/cbs_h264.h
> @@ -479,4 +479,12 @@ int ff_cbs_h264_delete_sei_message(CodedBitstreamContext 
> *ctx,
> CodedBitstreamUnit *nal_unit,
> int position);
>  
> +/**
> + * Create a writeable reference from a reference to a buffer that contains
> + * a parameter set of the kind implied by the function name.
> + */
> +int ff_cbs_h264_make_sps_writeable(AVBufferRef **buf);
> +int ff_cbs_h264_make_pps_writeable(AVBufferRef **buf);
> +
> +
>  #endif /* AVCODEC_CBS_H264_H */
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index e73706f2e6..a323bb2e8a 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -716,6 +716,22 @@ static int cbs_h26 ## h26n ## _replace_ ## 
> ps_var(CodedBitstreamContext *ctx, \
>  return AVERROR(ENOMEM); \
>  priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var ## 
> _ref[id]->data; \
>  return 0; \
> +} \
> + \
> +int ff_cbs_h26 ## h26n ## _make_ ## ps_var ## _writeable(AVBufferRef **buf) \
> +{ \
> +if (av_buffer_is_writable(*buf)) {\
> +return 0; \
> +} else { \
> +AVBufferRef *copy = cbs_h26 ## h26n ## _copy_ ## ps_var \
> +  ((H26 ## h26n ## Raw ## 
> ps_name*)(*buf)->data); \
> +if (copy) { \
> +av_buffer_unref(buf); \
> +*buf = copy; \
> +return 0; \
> +} else \
> +return AVERROR(ENOMEM); \
> +} \
>  }
>  
>  
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index cca1d7590b..9e951cd945 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -581,4 +581,13 @@ typedef struct CodedBitstreamH265Context {
>  } CodedBitstreamH265Context;
>  
>  
> +/**
> + * Create a writeable reference from a reference to a buffer that contains
> + * a parameter set of the kind implied by the function name.
> + */
> +int ff_cbs_h265_make_vps_writeable(AVBufferRef **buf);
> +int ff_cbs_h265_make_sps_writeable(AVBufferRef **buf);
> +int ff_cbs_h265_make_pps_writeable(AVBufferRef **buf);
> +
> +
>  #endif /* AVCODEC_CBS_H265_H */
> 

Would it be sensible to instead make the more general 
ff_cbs_make_unit_writeable(), calling a new function in CodedBitstreamType?  
You don't need to implement any additional cases (they can just return 
AVERROR_PATCHWELCOME if you call it on an unsupported unit), but I think the 
API would be nicer.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 2/6] cbs_h2645: Do a deep copy for parameter sets

2018-11-18 Thread Mark Thompson
On 09/11/18 05:31, Andreas Rheinhardt wrote:
> This commit solves dangling pointers problems when the content of a
> parameter set isn't refcounted in the beginning: Now a deep copy of the
> parameter sets is performed.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cbs_h2645.c | 59 +++---
>  1 file changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 37b0207420..e73706f2e6 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -674,7 +674,26 @@ static int 
> cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
>  return 0;
>  }
>  
> -#define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \
> +
> +#define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element, buffer) \
> +static AVBufferRef* cbs_h26 ## h26n ## _copy_ ## ps_var(const H26 ## h26n ## 
> Raw ## ps_name *source)\
> +{ \
> +H26 ## h26n ## Raw ## ps_name *copy; \
> +AVBufferRef *copy_ref; \
> +copy = av_malloc(sizeof(*source)); \
> +if (!copy) \
> +return NULL; \
> +memcpy(copy, source, sizeof(*source)); \
> +copy_ref = av_buffer_create((uint8_t*)copy, sizeof(*source), \
> +FREE(h26n, ps_var), NULL, 0); \
> +if (!copy_ref) { \
> +av_free(copy); \
> +return NULL; \
> +} \
> +cbs_h2645_copy_substructure(h26n, ps_name, ps_var, buffer) \
> +return copy_ref; \
> +} \

I think the copy function should be split out from replace_ps, since you're 
calling it from other contexts in the following patches.

> + \
>  static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext 
> *ctx, \
>CodedBitstreamUnit *unit)  
> \
>  { \
> @@ -692,21 +711,43 @@ static int cbs_h26 ## h26n ## _replace_ ## 
> ps_var(CodedBitstreamContext *ctx, \
>  if (unit->content_ref) \
>  priv->ps_var ## _ref[id] = av_buffer_ref(unit->content_ref); \
>  else \
> -priv->ps_var ## _ref[id] = av_buffer_alloc(sizeof(*ps_var)); \
> +priv->ps_var ## _ref[id] = cbs_h26 ## h26n ## _copy_ ## 
> ps_var(ps_var); \
>  if (!priv->ps_var ## _ref[id]) \
>  return AVERROR(ENOMEM); \
>  priv->ps_var[id] = (H26 ## h26n ## Raw ## ps_name *)priv->ps_var ## 
> _ref[id]->data; \
> -if (!unit->content_ref) \
> -memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \
>  return 0; \
>  }
>  
>  
> -cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id)
> -cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id)
> -cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id)
> -cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id)
> -cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id)
> +#define FREE(h26n, ps_var) NULL
> +#define cbs_h2645_copy_substructure(h26n, ps_name, ps_var, buffer)
> +cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id, )
> +#undef cbs_h2645_copy_substructure
> +#undef FREE
> +
> +#define FREE(h26n, ps_var) &cbs_h26 ## h26n ## _free_ ## ps_var
> +#define cbs_h2645_copy_substructure(h26n, ps_name, ps_var, buffer) \
> +if (source->buffer) { \
> +copy->buffer ## _ref = av_buffer_allocz(SIZE + 
> AV_INPUT_BUFFER_PADDING_SIZE); \
> +if (!copy->buffer) { \
> +av_buffer_unref(©_ref); \
> +return NULL; \
> +} \
> +copy->buffer = copy->buffer ## _ref->data; \
> +memcpy(copy->buffer, source->buffer, SIZE); \
> +}
> +
> +#define SIZE (copy->pic_size_in_map_units_minus1 + 1)
> +cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id, slice_group_id)
> +#undef SIZE
> +
> +#define SIZE ((copy->extension_data.bit_length + 7) / 8)
> +cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id, 
> extension_data.data)
> +cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id, 
> extension_data.data)
> +cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id, 
> extension_data.data)
> +#undef SIZE
> +#undef FREE

Could we perhaps make a single "internal buffers" argument here rather than the 
additional magic defines (SIZE, FREE, copy_substructure)?

Something like:

#define cbs_h2645_copy_ps_with_buffers(h26n, ps_name, ps_var, id_element, 
internal_buffers) \
...
struct {
uint8_t *data;
AVBufferRef *ref;
size_t   size;
} bufs[] = { internal_buffers };
...
for (i = 0; i < FF_ARRAY_ELEMS(bufs); i++) {
if (bufs[i].data) {

}
}

(Though maybe it needs offsetof() against the structure rather than passing in 
names, not sure.)

> +
>  
>  static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
>CodedBitstreamUnit *unit)
> 
Thanks,

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


Re: [FFmpeg-devel] [PATCH 1/6] cbs_h2645: Unify the free-functions via macro

2018-11-18 Thread Mark Thompson
On 09/11/18 05:31, Andreas Rheinhardt wrote:
> The similarity between several free-functions is exploited to create
> them via a common macro.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cbs_h2645.c | 55 --
>  1 file changed, 15 insertions(+), 40 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index e55bd00183..37b0207420 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -414,13 +414,22 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext 
> *gbc)
>  #undef allocate
>  
>  
> -static void cbs_h264_free_pps(void *unit, uint8_t *content)
> -{
> -H264RawPPS *pps = (H264RawPPS*)content;
> -av_buffer_unref(&pps->slice_group_id_ref);
> -av_freep(&content);
> +#define cbs_h2645_free(h26n, name, var, buffer) \
> +static void cbs_h26 ## h26n ## _free_ ## var(void *unit, uint8_t *content) \
> +{ \
> +H26 ## h26n ## Raw ## name *var = (H26 ## h26n ## Raw ## name*)content; \
> +av_buffer_unref(&var->buffer ## _ref); \
> +av_freep(&content); \
>  }
>  
> +cbs_h2645_free(4, PPS, pps, slice_group_id)
> +cbs_h2645_free(5, VPS, vps, extension_data.data)
> +cbs_h2645_free(5, SPS, sps, extension_data.data)
> +cbs_h2645_free(5, PPS, pps, extension_data.data)
> +
> +cbs_h2645_free(4, Slice, slice, data)
> +cbs_h2645_free(5, Slice, slice, data)
> +
>  static void cbs_h264_free_sei_payload(H264RawSEIPayload *payload)
>  {
>  switch (payload->payload_type) {
> @@ -452,41 +461,6 @@ static void cbs_h264_free_sei(void *unit, uint8_t 
> *content)
>  av_freep(&content);
>  }
>  
> -static void cbs_h264_free_slice(void *unit, uint8_t *content)
> -{
> -H264RawSlice *slice = (H264RawSlice*)content;
> -av_buffer_unref(&slice->data_ref);
> -av_freep(&content);
> -}
> -
> -static void cbs_h265_free_vps(void *unit, uint8_t *content)
> -{
> -H265RawVPS *vps = (H265RawVPS*)content;
> -av_buffer_unref(&vps->extension_data.data_ref);
> -av_freep(&content);
> -}
> -
> -static void cbs_h265_free_sps(void *unit, uint8_t *content)
> -{
> -H265RawSPS *sps = (H265RawSPS*)content;
> -av_buffer_unref(&sps->extension_data.data_ref);
> -av_freep(&content);
> -}
> -
> -static void cbs_h265_free_pps(void *unit, uint8_t *content)
> -{
> -H265RawPPS *pps = (H265RawPPS*)content;
> -av_buffer_unref(&pps->extension_data.data_ref);
> -av_freep(&content);
> -}
> -
> -static void cbs_h265_free_slice(void *unit, uint8_t *content)
> -{
> -H265RawSlice *slice = (H265RawSlice*)content;
> -av_buffer_unref(&slice->data_ref);
> -av_freep(&content);
> -}
> -
>  static void cbs_h265_free_sei_payload(H265RawSEIPayload *payload)
>  {
>  switch (payload->payload_type) {
> @@ -727,6 +701,7 @@ static int cbs_h26 ## h26n ## _replace_ ## 
> ps_var(CodedBitstreamContext *ctx, \
>  return 0; \
>  }
>  
> +
>  cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id)
>  cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id)
>  cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id)
> 

I'm not convinced that this is an improvement.  Those functions were previously 
simple and clear, while now they are a bit more obscure and I'm not sure you've 
gained anything except a slight saving in source code size.  The similarity 
between these functions that you're exploiting is that they free exactly one 
internal buffer, but otherwise they don't really have very much in common 
(unlike the replace_ps case).

If the aim is to avoid a bit of boilerplate, perhaps we could do something like:

#define cbs_h2645_free_unit(h26n, name, var, code) \
static void cbs_h26 ## h26n ## _free_ ## var(void *unit, uint8_t *content) \
{ \
H26 ## h26n ## Raw ## name *var = (H26 ## h26n ## Raw ## name*)content; \
do code while (0);
av_freep(&content); \
}

cbs_h2645_free_unit(4, PPS,   pps,   { av_buffer_unref(&pps->slice_group_id); })
cbs_h2645_free_unit(4, Slice, slice, { 
av_buffer_unref(&slice->extension_data.data_ref); })
cbs_h2645_free_unit(4, SEI,   sei,   {
int i;
for (i = 0; i < sei->payload_count; i++)
cbs_h264_free_sei_payload(&sei->payload[i]);
})

to exploit the commonality without hiding anything?  (Not necessarily exactly 
this, but something along those lines so the buffer behaviour is more clear 
than your version above.)

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


Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: don't abort when splitting Temporal Units containing OBUs with no obu_size field

2018-11-18 Thread James Almer
On 11/18/2018 3:41 PM, Mark Thompson wrote:
> On 15/11/18 04:27, James Almer wrote:
>> The ISOBMFF and Matroska specs allow the last OBU in a Sample/Block to have
>> obu_has_size_field equal to 0.
>>
>> Signed-off-by: James Almer 
>> ---
>> See https://0x0.st/sUsU.mp4
>>
>> It apparently can't be decoded with either aom or dav1d atm, but the latter
>> should be able to soon.
>> You can try remuxing it using the av1_metadata bsf after this patch while at 
>> it,
>> since the code to write obu_size fields unconditionally is still in place, 
>> and
>> that should make it decodable with either library.
> 
> I wonder whether there should be an option somewhere for omitting the last 
> size field when writing?  It would save a few bytes on every TU when using 
> any container which already has sizes.

Yeah, that'd be neat as an option in av1_metadata, with choices of
bypass and remove/add if present or not in last obu, much like the one
for TDs.

Only thing i worry about is how as i mentioned libaom doesn't seem to be
able to handle OBUs with no size field. That means any such generated
stream would fail to decode on pretty much everything, at least until
dav1d gets widespread adoption, or aom implements support for the feature.

> 
>> For some reason cbs_av1_read_unit() is already able to handle OBUs with no
>> obu_size field. It was only cbs_av1_split_fragment() bailing out with them.
> 
> That came from the annex B support, before we decided that AVPackets would 
> never contain annex B.
> 
>>  libavcodec/cbs_av1.c | 16 ++--
>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
>> index ed5211be19..e02bc7027a 100644
>> --- a/libavcodec/cbs_av1.c
>> +++ b/libavcodec/cbs_av1.c
>> @@ -788,13 +788,6 @@ static int cbs_av1_split_fragment(CodedBitstreamContext 
>> *ctx,
>>  if (err < 0)
>>  goto fail;
>>  
>> -if (!header.obu_has_size_field) {
>> -av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid OBU for raw "
>> -   "stream: size field must be present.\n");
>> -err = AVERROR_INVALIDDATA;
>> -goto fail;
>> -}
>> -
>>  if (get_bits_left(&gbc) < 8) {
>>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid OBU: fragment "
>> "too short (%zu bytes).\n", size);
>> @@ -802,9 +795,12 @@ static int cbs_av1_split_fragment(CodedBitstreamContext 
>> *ctx,
>>  goto fail;
>>  }
>>  
>> -err = cbs_av1_read_leb128(ctx, &gbc, "obu_size", &obu_size);
>> -if (err < 0)
>> -goto fail;
>> +if (header.obu_has_size_field) {
>> +err = cbs_av1_read_leb128(ctx, &gbc, "obu_size", &obu_size);
>> +if (err < 0)
>> +goto fail;
>> +} else
>> +obu_size = size - 1 - header.obu_extension_flag;
>>  
>>  pos = get_bits_count(&gbc);
>>  av_assert0(pos % 8 == 0 && pos / 8 <= size);
>>
> 
> LGTM.
> 
> Thanks,
> 
> - Mark

Pushed. Thanks!
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v5 1/4] lavc/libdavs2: fix sequence incomplete output error

2018-11-18 Thread Mark Thompson
On 02/11/18 13:30, hwren wrote:
> Signed-off-by: hwren 
> ---
>  libavcodec/libdavs2.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index cadf995..e36bfed 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -129,7 +129,17 @@ static int davs2_decode_frame(AVCodecContext *avctx, 
> void *data,
>  int   ret  = DAVS2_DEFAULT;
>  
>  if (!buf_size) {
> -return 0;
> +ret = davs2_decoder_flush(cad->decoder, &cad->headerset, 
> &cad->out_frame);
> +if (ret == DAVS2_END) {
> +return 0;
> +} else if (ret == DAVS2_GOT_FRAME) {
> +*got_frame = davs2_dump_frames(avctx, &cad->out_frame, 
> &cad->headerset, ret, frame);
> +davs2_decoder_frame_unref(cad->decoder, &cad->out_frame);
> +return ret;

This returns a library-internal value to the user, which doesn't look right.

> +} else {
> +av_log(avctx, AV_LOG_ERROR, "Decoder error: dump frames 
> failed\n");
> +return AVERROR_EXTERNAL;
> +}
>  }
>  
>  cad->packet.data = buf_ptr;
> 
On 02/11/18 13:30, hwren wrote:
> Signed-off-by: hwren 
> ---
>  libavcodec/libdavs2.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index 3e59d41..6e4bd50 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -147,15 +147,17 @@ static int davs2_decode_frame(AVCodecContext *avctx, 
> void *data,
>  if (!buf_size) {
>  ret = davs2_decoder_flush(cad->decoder, &cad->headerset, 
> &cad->out_frame);
>  if (ret == DAVS2_END) {
> -return 0;
> +ret = 0;
>  } else if (ret == DAVS2_GOT_FRAME) {
> -*got_frame = davs2_dump_frames(avctx, &cad->out_frame, 
> &cad->headerset, ret, frame);
> +ret = davs2_dump_frames(avctx, &cad->out_frame, &cad->headerset, 
> ret, frame);
>  davs2_decoder_frame_unref(cad->decoder, &cad->out_frame);
> -return ret;
> +if (ret == 0 || ret == 1) {
> +*got_frame = ret;
> +}
>  } else {
> -av_log(avctx, AV_LOG_ERROR, "Decoder error: dump frames 
> failed\n");
> -return AVERROR_EXTERNAL;
> +av_log(avctx, AV_LOG_ERROR, "Decoder error: flush frames 
> failed\n");

So does this one.

>  }
> +return ret;
>  }
>  
>  cad->packet.data = buf_ptr;
> @@ -174,8 +176,14 @@ static int davs2_decode_frame(AVCodecContext *avctx, 
> void *data,
>  ret = davs2_decoder_recv_frame(cad->decoder, &cad->headerset, 
> &cad->out_frame);
>  
>  if (ret != DAVS2_DEFAULT) {
> -*got_frame = davs2_dump_frames(avctx, &cad->out_frame, 
> &cad->headerset, ret, frame);
> +ret = davs2_dump_frames(avctx, &cad->out_frame, &cad->headerset, 
> ret, frame);
>  davs2_decoder_frame_unref(cad->decoder, &cad->out_frame);
> +if (ret == 0 || ret == 1) {
> +*got_frame = ret;
> +} else {
> +av_log(avctx, AV_LOG_ERROR, "Decoder error: dump frames 
> failed\n");
> +return ret;
> +}
>  }
>  
>  return buf_size;
> 

Patches 2 and 3 applied.

Thanks,

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


Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: don't abort when splitting Temporal Units containing OBUs with no obu_size field

2018-11-18 Thread Mark Thompson
On 15/11/18 04:27, James Almer wrote:
> The ISOBMFF and Matroska specs allow the last OBU in a Sample/Block to have
> obu_has_size_field equal to 0.
> 
> Signed-off-by: James Almer 
> ---
> See https://0x0.st/sUsU.mp4
> 
> It apparently can't be decoded with either aom or dav1d atm, but the latter
> should be able to soon.
> You can try remuxing it using the av1_metadata bsf after this patch while at 
> it,
> since the code to write obu_size fields unconditionally is still in place, and
> that should make it decodable with either library.

I wonder whether there should be an option somewhere for omitting the last size 
field when writing?  It would save a few bytes on every TU when using any 
container which already has sizes.

> For some reason cbs_av1_read_unit() is already able to handle OBUs with no
> obu_size field. It was only cbs_av1_split_fragment() bailing out with them.

That came from the annex B support, before we decided that AVPackets would 
never contain annex B.

>  libavcodec/cbs_av1.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index ed5211be19..e02bc7027a 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -788,13 +788,6 @@ static int cbs_av1_split_fragment(CodedBitstreamContext 
> *ctx,
>  if (err < 0)
>  goto fail;
>  
> -if (!header.obu_has_size_field) {
> -av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid OBU for raw "
> -   "stream: size field must be present.\n");
> -err = AVERROR_INVALIDDATA;
> -goto fail;
> -}
> -
>  if (get_bits_left(&gbc) < 8) {
>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid OBU: fragment "
> "too short (%zu bytes).\n", size);
> @@ -802,9 +795,12 @@ static int cbs_av1_split_fragment(CodedBitstreamContext 
> *ctx,
>  goto fail;
>  }
>  
> -err = cbs_av1_read_leb128(ctx, &gbc, "obu_size", &obu_size);
> -if (err < 0)
> -goto fail;
> +if (header.obu_has_size_field) {
> +err = cbs_av1_read_leb128(ctx, &gbc, "obu_size", &obu_size);
> +if (err < 0)
> +goto fail;
> +} else
> +obu_size = size - 1 - header.obu_extension_flag;
>  
>  pos = get_bits_count(&gbc);
>  av_assert0(pos % 8 == 0 && pos / 8 <= size);
> 

LGTM.

Thanks,

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


Re: [FFmpeg-devel] [PATCH 3/3] libaomenc: Drop unused noise-sensitivity option

2018-11-18 Thread Jan Ekström
On Mon, Nov 5, 2018 at 4:53 PM Mark Thompson  wrote:
>
> ---
> On 28/10/18 21:02, Carl Eugen Hoyos wrote:
> > 2018-10-28 21:08 GMT+01:00, Mark Thompson :
> >> ---
> >> This was in the 4.0 release, unfortunately (and did nothing there as well).
> >
> > If it has never worked, it can be removed imo.
>
> Sure, here is a new version.
>
> Thanks,
>
> - Mark
>
>
>  libavcodec/libaomenc.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index c5458766cb..4cad053a48 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -71,7 +71,6 @@ typedef struct AOMEncoderContext {
>  int crf;
>  int static_thresh;
>  int drop_threshold;
> -int noise_sensitivity;
>  uint64_t sse[4];
>  int have_sse; /**< true if we have pending sse[] */
>  uint64_t frame_number;
> @@ -980,7 +979,6 @@ static const AVOption options[] = {
>  { "crf",  "Select the quality for constant quality mode", 
> offsetof(AOMContext, crf), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 63, VE },
>  { "static-thresh","A change threshold on blocks below which they 
> will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { 
> .i64 = 0 }, 0, INT_MAX, VE },
>  { "drop-threshold",   "Frame drop threshold", offsetof(AOMContext, 
> drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE },
> -{ "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), 
> AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE},
>  { "tiles","Tile columns x rows", OFFSET(tile_cols), 
> AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, VE },
>  { "tile-columns", "Log2 of number of tile columns to use", 
> OFFSET(tile_cols_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>  { "tile-rows","Log2 of number of tile rows to use",
> OFFSET(tile_rows_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
> --
> 2.19.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Only two references to noise_sensitivity outside of libvpxenc. LGTM.

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


Re: [FFmpeg-devel] invalid PCM-DVD detection

2018-11-18 Thread Michael Niedermayer
On Fri, Nov 16, 2018 at 07:41:27PM +0300, Евгений wrote:
> 

>  mpeg.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 10489d03a741445401f3f6bfb49e536680db6cc9  0003-invalid-PCM-DVD-detection.patch
> From 8dffee21a78c17bf1e74f459869babe77041dfd2 Mon Sep 17 00:00:00 2001
> From: Evgeny 
> Date: Fri, 16 Nov 2018 12:38:01 +0300

> Subject: [PATCH 3/6] invalid PCM-DVD detection

This commit message is too terse, it should refer to the
sample and or some reference information

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


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


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg_er: fix clearing chroma blocks for 422 and 444

2018-11-18 Thread Michael Niedermayer
On Sat, Nov 17, 2018 at 11:39:08PM +0100, Marton Balint wrote:
> Fixes ticket #7494.
> 
> Signed-off-by: Marton Balint 
> ---
>  libavcodec/mpeg_er.c | 2 ++
>  1 file changed, 2 insertions(+)

probably ok

a fate test covering this case if its stable should be added too.

thx

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

No great genius has ever existed without some touch of madness. -- Aristotle


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


Re: [FFmpeg-devel] [PATCH 3/3] libaomenc: Drop unused noise-sensitivity option

2018-11-18 Thread Mark Thompson
On 05/11/18 14:52, Mark Thompson wrote:
> ---
> On 28/10/18 21:02, Carl Eugen Hoyos wrote:
>> 2018-10-28 21:08 GMT+01:00, Mark Thompson :
>>> ---
>>> This was in the 4.0 release, unfortunately (and did nothing there as well).
>>
>> If it has never worked, it can be removed imo.
> 
> Sure, here is a new version.

Ping.

- Mark


>  libavcodec/libaomenc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index c5458766cb..4cad053a48 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -71,7 +71,6 @@ typedef struct AOMEncoderContext {
>  int crf;
>  int static_thresh;
>  int drop_threshold;
> -int noise_sensitivity;
>  uint64_t sse[4];
>  int have_sse; /**< true if we have pending sse[] */
>  uint64_t frame_number;
> @@ -980,7 +979,6 @@ static const AVOption options[] = {
>  { "crf",  "Select the quality for constant quality mode", 
> offsetof(AOMContext, crf), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 63, VE },
>  { "static-thresh","A change threshold on blocks below which they 
> will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { 
> .i64 = 0 }, 0, INT_MAX, VE },
>  { "drop-threshold",   "Frame drop threshold", offsetof(AOMContext, 
> drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE },
> -{ "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), 
> AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE},
>  { "tiles","Tile columns x rows", OFFSET(tile_cols), 
> AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, VE },
>  { "tile-columns", "Log2 of number of tile columns to use", 
> OFFSET(tile_cols_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>  { "tile-rows","Log2 of number of tile rows to use",
> OFFSET(tile_rows_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] doc/encoders: Add libaom-av1

2018-11-18 Thread Mark Thompson
On 05/11/18 15:49, Gyan wrote:
> On Mon, Nov 5, 2018 at 9:04 PM Mark Thompson  wrote:
> 
>> On 05/11/18 15:20, Gyan wrote:
>>> On Mon, Nov 5, 2018 at 8:23 PM Mark Thompson  wrote:
>>>
 ---
 Updated to add some more explanation of the rate control mode selection.


  doc/encoders.texi | 108 ++
  1 file changed, 108 insertions(+)

 diff --git a/doc/encoders.texi b/doc/encoders.texi
 index 1ca0ef1543..f2e4199017 100644
 --- a/doc/encoders.texi
 +++ b/doc/encoders.texi
 @@ -1370,6 +1370,114 @@ makes it possible to store non-rgb pix_fmts.

  @end table

 +@section libaom-av1
 +
 +libaom AV1 encoder wrapper.
 +
 +Requires the presence of the libaom headers and library during
 +configuration.  You need to explicitly configure the build with
 +@code{--enable-libaom}.
 +
 +@subsection Options
 +
 +The wrapper supports the following standard libavcodec options:

>>>
>>> Do we really need to duplicate the text for the generic options? That
>> will
>>> lead to bloat if widely adopted.
>>
>> Which parts do you object to?  This text is trying to explain how the
>> standard options apply to libaom and what their defaults are.
>>
> 
> Makes sense. But items like g, the color props or profile are covered by
> the generic text. Maybe for completeness' sake, we should keep them here.
> 
> LGTM

Applied with minor changes - I removed the colour properties setting because it 
wasn't adding anything, and I added some more text to the GOP size option to 
describe the default behaviour.

Thanks,

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


Re: [FFmpeg-devel] [PATCH] mjpegdec: Fill raw huffman tables with default values too

2018-11-18 Thread Mark Thompson
On 13/11/18 00:24, myp...@gmail.com wrote:
> On Mon, Nov 12, 2018 at 12:26 AM Mark Thompson  wrote:
>>
>> These may be used by hwaccel decoders when the standard tables are not
>> otherwise available.  At the same time, clean up that code into an array
>> so it's a little less repetitive.
>> ---
>>  libavcodec/mjpegdec.c | 67 +--
>>  1 file changed, 39 insertions(+), 28 deletions(-)
>>
>> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
>> index 96c425515a..2f1635838a 100644
>> --- a/libavcodec/mjpegdec.c
>> +++ b/libavcodec/mjpegdec.c
>> @@ -73,34 +73,45 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table,
>>huff_code, 2, 2, huff_sym, 2, 2, use_static);
>>  }
>>
>> -static int build_basic_mjpeg_vlc(MJpegDecodeContext *s)
>> +static int init_default_huffman_tables(MJpegDecodeContext *s)
>>  {
>> -int ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance,
>> - avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
>> -return ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[0][1], avpriv_mjpeg_bits_dc_chrominance,
>> - avpriv_mjpeg_val_dc, 12, 0, 0)) < 0)
>> -return ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[1][0], avpriv_mjpeg_bits_ac_luminance,
>> - avpriv_mjpeg_val_ac_luminance, 251, 0, 1)) < 0)
>> -return ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[1][1], avpriv_mjpeg_bits_ac_chrominance,
>> - avpriv_mjpeg_val_ac_chrominance, 251, 0, 1)) < 0)
>> -return ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[2][0], avpriv_mjpeg_bits_ac_luminance,
>> - avpriv_mjpeg_val_ac_luminance, 251, 0, 0)) < 0)
>> -return ret;
>> -
>> -if ((ret = build_vlc(&s->vlcs[2][1], avpriv_mjpeg_bits_ac_chrominance,
>> - avpriv_mjpeg_val_ac_chrominance, 251, 0, 0)) < 0)
>> -return ret;
>> +static const struct {
>> +int class;
>> +int index;
>> +const uint8_t *bits;
>> +const uint8_t *values;
>> +int codes;
>> +int length;
>> +} ht[] = {
>> +{ 0, 0, avpriv_mjpeg_bits_dc_luminance,
>> +avpriv_mjpeg_val_dc, 12, 12 },
>> +{ 0, 1, avpriv_mjpeg_bits_dc_chrominance,
>> +avpriv_mjpeg_val_dc, 12, 12 },
>> +{ 1, 0, avpriv_mjpeg_bits_ac_luminance,
>> +avpriv_mjpeg_val_ac_luminance,   251, 162 },
>> +{ 1, 1, avpriv_mjpeg_bits_ac_chrominance,
>> +avpriv_mjpeg_val_ac_chrominance, 251, 162 },
>> +{ 2, 0, avpriv_mjpeg_bits_ac_luminance,
>> +avpriv_mjpeg_val_ac_luminance,   251, 162 },
>> +{ 2, 1, avpriv_mjpeg_bits_ac_chrominance,
>> +avpriv_mjpeg_val_ac_chrominance, 251, 162 },
>> +};
>> +int i, ret;
>> +
>> +for (i = 0; i < FF_ARRAY_ELEMS(ht); i++) {
>> +ret = build_vlc(&s->vlcs[ht[i].class][ht[i].index],
>> +ht[i].bits, ht[i].values, ht[i].codes,
>> +0, ht[i].class == 1);
>> +if (ret < 0)
>> +return ret;
>>
>> +if (ht[i].class < 2) {
>> +memcpy(s->raw_huffman_lengths[ht[i].class][ht[i].index],
>> +   ht[i].bits + 1, 16);
>> +memcpy(s->raw_huffman_values[ht[i].class][ht[i].index],
>> +   ht[i].values, ht[i].length);
>> +}
>> +}
>>
>>  return 0;
>>  }
>> @@ -151,7 +162,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
>>  avctx->colorspace = AVCOL_SPC_BT470BG;
>>  s->hwaccel_pix_fmt = s->hwaccel_sw_pix_fmt = AV_PIX_FMT_NONE;
>>
>> -if ((ret = build_basic_mjpeg_vlc(s)) < 0)
>> +if ((ret = init_default_huffman_tables(s)) < 0)
>>  return ret;
>>
>>  if (s->extern_huff) {
>> @@ -161,7 +172,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
>>  if (ff_mjpeg_decode_dht(s)) {
>>  av_log(avctx, AV_LOG_ERROR,
>> "error using external huffman table, switching back to 
>> internal\n");
>> -build_basic_mjpeg_vlc(s);
>> +init_default_huffman_tables(s);
>>  }
>>  }
>>  if (avctx->field_order == AV_FIELD_BB) { /* quicktime icefloe 019 */
>> --
>> 2.19.1
> LGTM

Applied.

Thanks,

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


[FFmpeg-devel] [PATCH 4/5] fftools/ffmpeg: delete the unused code.

2018-11-18 Thread Jun Zhao
There are come from 2012 ago and have never been used from this
time.

Signed-off-by: Jun Zhao 
---
 fftools/ffmpeg.c |9 +
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index a12208c..085d6d2 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1485,8 +1485,6 @@ static int reap_filters(int flush)
 av_rescale_q(filtered_frame->pts, filter_tb, 
enc->time_base) -
 av_rescale_q(start_time, AV_TIME_BASE_Q, enc->time_base);
 }
-//if (ost->source_index >= 0)
-//*filtered_frame= 
*input_streams[ost->source_index]->decoded_frame; //for me_threshold
 
 switch (av_buffersink_get_type(filter)) {
 case AVMEDIA_TYPE_VIDEO:
@@ -3340,7 +3338,7 @@ static int init_output_stream_encode(OutputStream *ost)
"if you want a different framerate.\n",
ost->file_index, ost->index);
 }
-//  ost->frame_rate = ist->st->avg_frame_rate.num ? 
ist->st->avg_frame_rate : (AVRational){25, 1};
+
 if (ost->enc->supported_framerates && !ost->force_fps) {
 int idx = av_find_nearest_q_idx(ost->frame_rate, 
ost->enc->supported_framerates);
 ost->frame_rate = ost->enc->supported_framerates[idx];
@@ -4879,11 +4877,6 @@ int main(int argc, char **argv)
 exit_program(1);
 }
 
-// if (nb_input_files == 0) {
-// av_log(NULL, AV_LOG_FATAL, "At least one input file must be 
specified\n");
-// exit_program(1);
-// }
-
 for (i = 0; i < nb_output_files; i++) {
 if (strcmp(output_files[i]->ctx->oformat->name, "rtp"))
 want_sdp = 0;
-- 
1.7.1

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


[FFmpeg-devel] [PATCH 5/5] lavc/kvazaar: fix auto thread flag in kvazaar wrapper.

2018-11-18 Thread Jun Zhao
Now the kvazaar warpper didn't setting the threads for kvazaar API,
and kavzaar will auto select the thread number.

Signed-off-by: Jun Zhao 
---
 libavcodec/libkvazaar.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index 5bc5b4e..8f50bef 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -293,7 +293,7 @@ AVCodec ff_libkvazaar_encoder = {
 .long_name= NULL_IF_CONFIG_SMALL("libkvazaar H.265 / HEVC"),
 .type = AVMEDIA_TYPE_VIDEO,
 .id   = AV_CODEC_ID_HEVC,
-.capabilities = AV_CODEC_CAP_DELAY,
+.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
 .pix_fmts = pix_fmts,
 
 .priv_class   = &class,
-- 
1.7.1

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


[FFmpeg-devel] [PATCH 2/5] fftools/ffprobe: fix max_bit_rate dump.

2018-11-18 Thread Jun Zhao
‘codec’ is deprecated in AVStream, so used the dec_ctx to dump
max_bit_rate in ffprobe. Clean the warning like:
"warning: ‘codec’ is deprecated [-Wdeprecated-declarations]"

Signed-off-by: Jun Zhao 
---
 fftools/ffprobe.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 544786e..280db78 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2622,10 +2622,8 @@ static int show_stream(WriterContext *w, AVFormatContext 
*fmt_ctx, int stream_id
 print_time("duration",stream->duration, &stream->time_base);
 if (par->bit_rate > 0) print_val("bit_rate", par->bit_rate, 
unit_bit_per_second_str);
 else   print_str_opt("bit_rate", "N/A");
-#if FF_API_LAVF_AVCTX
-if (stream->codec->rc_max_rate > 0) print_val ("max_bit_rate", 
stream->codec->rc_max_rate, unit_bit_per_second_str);
-elseprint_str_opt("max_bit_rate", "N/A");
-#endif
+if (dec_ctx->rc_max_rate > 0) print_val ("max_bit_rate", 
dec_ctx->rc_max_rate, unit_bit_per_second_str);
+else  print_str_opt("max_bit_rate", "N/A");
 if (dec_ctx && dec_ctx->bits_per_raw_sample > 0) 
print_fmt("bits_per_raw_sample", "%d", dec_ctx->bits_per_raw_sample);
 else 
print_str_opt("bits_per_raw_sample", "N/A");
 if (stream->nb_frames) print_fmt("nb_frames", "%"PRId64, 
stream->nb_frames);
-- 
1.7.1

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


[FFmpeg-devel] [PATCH 1/5] lavfi/buffersrc: Indent the code.

2018-11-18 Thread Jun Zhao
commit b0012de420f missed reindent.

Signed-off-by: Jun Zhao 
---
 libavfilter/buffersrc.c |   30 +++---
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index cd56f8c..0c12650 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -205,21 +205,21 @@ static int 
av_buffersrc_add_frame_internal(AVFilterContext *ctx,
 
 if (!(flags & AV_BUFFERSRC_FLAG_NO_CHECK_FORMAT)) {
 
-switch (ctx->outputs[0]->type) {
-case AVMEDIA_TYPE_VIDEO:
-CHECK_VIDEO_PARAM_CHANGE(ctx, s, frame->width, frame->height,
- frame->format);
-break;
-case AVMEDIA_TYPE_AUDIO:
-/* For layouts unknown on input but known on link after negotiation. */
-if (!frame->channel_layout)
-frame->channel_layout = s->channel_layout;
-CHECK_AUDIO_PARAM_CHANGE(ctx, s, frame->sample_rate, 
frame->channel_layout,
- frame->channels, frame->format);
-break;
-default:
-return AVERROR(EINVAL);
-}
+switch (ctx->outputs[0]->type) {
+case AVMEDIA_TYPE_VIDEO:
+CHECK_VIDEO_PARAM_CHANGE(ctx, s, frame->width, frame->height,
+ frame->format);
+break;
+case AVMEDIA_TYPE_AUDIO:
+/* For layouts unknown on input but known on link after 
negotiation. */
+if (!frame->channel_layout)
+frame->channel_layout = s->channel_layout;
+CHECK_AUDIO_PARAM_CHANGE(ctx, s, frame->sample_rate, 
frame->channel_layout,
+ frame->channels, frame->format);
+break;
+default:
+return AVERROR(EINVAL);
+}
 
 }
 
-- 
1.7.1

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


[FFmpeg-devel] [PATCH 3/5] fftools/ffprobe: Indent the code.

2018-11-18 Thread Jun Zhao
commit 196765a7cc4 missed the reindet.

Signed-off-by: Jun Zhao 
---
 fftools/ffprobe.c |   28 ++--
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 280db78..d589893 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2644,20 +2644,20 @@ static int show_stream(WriterContext *w, 
AVFormatContext *fmt_ctx, int stream_id
 } while (0)
 
 if (do_show_stream_disposition) {
-writer_print_section_header(w, in_program ? 
SECTION_ID_PROGRAM_STREAM_DISPOSITION : SECTION_ID_STREAM_DISPOSITION);
-PRINT_DISPOSITION(DEFAULT,  "default");
-PRINT_DISPOSITION(DUB,  "dub");
-PRINT_DISPOSITION(ORIGINAL, "original");
-PRINT_DISPOSITION(COMMENT,  "comment");
-PRINT_DISPOSITION(LYRICS,   "lyrics");
-PRINT_DISPOSITION(KARAOKE,  "karaoke");
-PRINT_DISPOSITION(FORCED,   "forced");
-PRINT_DISPOSITION(HEARING_IMPAIRED, "hearing_impaired");
-PRINT_DISPOSITION(VISUAL_IMPAIRED,  "visual_impaired");
-PRINT_DISPOSITION(CLEAN_EFFECTS,"clean_effects");
-PRINT_DISPOSITION(ATTACHED_PIC, "attached_pic");
-PRINT_DISPOSITION(TIMED_THUMBNAILS, "timed_thumbnails");
-writer_print_section_footer(w);
+writer_print_section_header(w, in_program ? 
SECTION_ID_PROGRAM_STREAM_DISPOSITION : SECTION_ID_STREAM_DISPOSITION);
+PRINT_DISPOSITION(DEFAULT,  "default");
+PRINT_DISPOSITION(DUB,  "dub");
+PRINT_DISPOSITION(ORIGINAL, "original");
+PRINT_DISPOSITION(COMMENT,  "comment");
+PRINT_DISPOSITION(LYRICS,   "lyrics");
+PRINT_DISPOSITION(KARAOKE,  "karaoke");
+PRINT_DISPOSITION(FORCED,   "forced");
+PRINT_DISPOSITION(HEARING_IMPAIRED, "hearing_impaired");
+PRINT_DISPOSITION(VISUAL_IMPAIRED,  "visual_impaired");
+PRINT_DISPOSITION(CLEAN_EFFECTS,"clean_effects");
+PRINT_DISPOSITION(ATTACHED_PIC, "attached_pic");
+PRINT_DISPOSITION(TIMED_THUMBNAILS, "timed_thumbnails");
+writer_print_section_footer(w);
 }
 
 if (do_show_stream_tags)
-- 
1.7.1

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


[FFmpeg-devel] [PATCH 0/5] Misc change

2018-11-18 Thread Jun Zhao
V1: - indet ffmpeg/buffersrc
- add auto threads flag for kvazaar
- delete the unused code from ffmpeg
- fix max_bit_rate dump warning for ffprobe

Jun Zhao (5):
  lavfi/buffersrc: Indent the code.
  fftools/ffprobe: fix max_bit_rate dump.
  fftools/ffprobe: Indent the code.
  fftools/ffmpeg: delete the unused code.
  lavc/kvazaar: fix auto thread flag in kvazaar wrapper.

 fftools/ffmpeg.c|9 +
 fftools/ffprobe.c   |   34 --
 libavcodec/libkvazaar.c |2 +-
 libavfilter/buffersrc.c |   30 +++---
 4 files changed, 33 insertions(+), 42 deletions(-)

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


[FFmpeg-devel] [PATCH] avutil/mem: Correct documentation of av_fast_*alloc(z)

2018-11-18 Thread Andreas Rheinhardt
The current wording regarding size and min_size is completely wrong and
ignores that min_size is indeed only a desired minimal size, not the
actually allocated size.

Signed-off-by: Andreas Rheinhardt 
---
 libavutil/mem.h | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/libavutil/mem.h b/libavutil/mem.h
index 7e0b12a8a7..55ae573ac9 100644
--- a/libavutil/mem.h
+++ b/libavutil/mem.h
@@ -363,10 +363,10 @@ av_alloc_size(2, 3) int av_reallocp_array(void *ptr, 
size_t nmemb, size_t size);
  * @endcode
  *
  * @param[in,out] ptr  Already allocated buffer, or `NULL`
- * @param[in,out] size Pointer to current size of buffer `ptr`. `*size` is
- * changed to `min_size` in case of success or 0 in
- * case of failure
- * @param[in] min_size New size of buffer `ptr`
+ * @param[in,out] size Pointer to the size of buffer `ptr`. `*size` is
+ * updated to the new allocated size, in particular 0
+ * in case of failure.
+ * @param[in] min_size Desired minimal size of buffer `ptr`
  * @return `ptr` if the buffer is large enough, a pointer to newly reallocated
  * buffer if the buffer was not large enough, or `NULL` in case of
  * error
@@ -397,10 +397,10 @@ void *av_fast_realloc(void *ptr, unsigned int *size, 
size_t min_size);
  * @param[in,out] ptr  Pointer to pointer to an already allocated buffer.
  * `*ptr` will be overwritten with pointer to new
  * buffer on success or `NULL` on failure
- * @param[in,out] size Pointer to current size of buffer `*ptr`. `*size` is
- * changed to `min_size` in case of success or 0 in
- * case of failure
- * @param[in] min_size New size of buffer `*ptr`
+ * @param[in,out] size Pointer to the size of buffer `*ptr`. `*size` is
+ * updated to the new allocated size, in particular 0
+ * in case of failure.
+ * @param[in] min_size Desired minimal size of buffer `*ptr`
  * @see av_realloc()
  * @see av_fast_mallocz()
  */
@@ -418,10 +418,10 @@ void av_fast_malloc(void *ptr, unsigned int *size, size_t 
min_size);
  * @param[in,out] ptr  Pointer to pointer to an already allocated buffer.
  * `*ptr` will be overwritten with pointer to new
  * buffer on success or `NULL` on failure
- * @param[in,out] size Pointer to current size of buffer `*ptr`. `*size` is
- * changed to `min_size` in case of success or 0 in
- * case of failure
- * @param[in] min_size New size of buffer `*ptr`
+ * @param[in,out] size Pointer to the size of buffer `*ptr`. `*size` is
+ * updated to the new allocated size, in particular 0
+ * in case of failure.
+ * @param[in] min_size Desired minimal size of buffer `*ptr`
  * @see av_fast_malloc()
  */
 void av_fast_mallocz(void *ptr, unsigned int *size, size_t min_size);
-- 
2.19.1

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


[FFmpeg-devel] [PATCH] avfilter: add rgbashift filter

2018-11-18 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 doc/filters.texi |  25 
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vf_chromashift.c | 226 +--
 4 files changed, 244 insertions(+), 9 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 77f57bfcd6..6b1bf8766b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14311,6 +14311,31 @@ trim=end=5,reverse
 @end example
 @end itemize
 
+@section rgbashift
+Shift R/G/B/A pixels horizontally and/or vertically.
+
+The filter accepts the following options:
+@table @option
+@item rh
+Set amount to shift red horizontally.
+@item rv
+Set amount to shift red vertically.
+@item gh
+Set amount to shift green horizontally.
+@item gv
+Set amount to shift green vertically.
+@item bh
+Set amount to shift blue horizontally.
+@item bv
+Set amount to shift blue vertically.
+@item ah
+Set amount to shift alpha horizontally.
+@item av
+Set amount to shift alpha vertically.
+@item edge
+Set edge mode, can be @var{smear}, default, or @var{warp}.
+@end table
+
 @section roberts
 Apply roberts cross operator to input video stream.
 
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 8f3fde8c27..4df22db2fc 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -330,6 +330,7 @@ OBJS-$(CONFIG_REMOVEGRAIN_FILTER)+= 
vf_removegrain.o
 OBJS-$(CONFIG_REMOVELOGO_FILTER) += bbox.o lswsutils.o lavfutils.o 
vf_removelogo.o
 OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += vf_repeatfields.o
 OBJS-$(CONFIG_REVERSE_FILTER)+= f_reverse.o
+OBJS-$(CONFIG_RGBASHIFT_FILTER)  += vf_chromashift.o
 OBJS-$(CONFIG_ROBERTS_FILTER)+= vf_convolution.o
 OBJS-$(CONFIG_ROBERTS_OPENCL_FILTER) += vf_convolution_opencl.o 
opencl.o \
 opencl/convolution.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 6f392b5632..7294486314 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -314,6 +314,7 @@ extern AVFilter ff_vf_removegrain;
 extern AVFilter ff_vf_removelogo;
 extern AVFilter ff_vf_repeatfields;
 extern AVFilter ff_vf_reverse;
+extern AVFilter ff_vf_rgbashift;
 extern AVFilter ff_vf_roberts;
 extern AVFilter ff_vf_roberts_opencl;
 extern AVFilter ff_vf_rotate;
diff --git a/libavfilter/vf_chromashift.c b/libavfilter/vf_chromashift.c
index f4c561e2d0..3e0557036a 100644
--- a/libavfilter/vf_chromashift.c
+++ b/libavfilter/vf_chromashift.c
@@ -35,8 +35,13 @@ typedef struct ChromaShiftContext {
 const AVClass *class;
 int cbh, cbv;
 int crh, crv;
+int rh, rv;
+int gh, gv;
+int bh, bv;
+int ah, av;
 int edge;
 
+int nb_planes;
 int depth;
 int height[4];
 int width[4];
@@ -44,12 +49,13 @@ typedef struct ChromaShiftContext {
 
 AVFrame *in;
 
+int is_rgbashift;
 int (*filter_slice)(AVFilterContext *ctx, void *arg, int jobnr, int 
nb_jobs);
 } ChromaShiftContext;
 
 static int query_formats(AVFilterContext *ctx)
 {
-static const enum AVPixelFormat pix_fmts[] = {
+static const enum AVPixelFormat yuv_pix_fmts[] = {
 AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVA422P, AV_PIX_FMT_YUVA420P,
 AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ440P, 
AV_PIX_FMT_YUVJ422P,AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ411P,
 AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV422P, 
AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUV410P,
@@ -62,6 +68,19 @@ static int query_formats(AVFilterContext *ctx)
 AV_PIX_FMT_YUVA420P16, AV_PIX_FMT_YUVA422P16, AV_PIX_FMT_YUVA444P16,
 AV_PIX_FMT_NONE
 };
+static const enum AVPixelFormat rgb_pix_fmts[] = {
+AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRAP, AV_PIX_FMT_GBRP9,
+AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12,
+AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16,
+AV_PIX_FMT_GBRAP10, AV_PIX_FMT_GBRAP12, AV_PIX_FMT_GBRAP16,
+AV_PIX_FMT_NONE
+};
+const enum AVPixelFormat *pix_fmts;
+
+if (!strcmp(ctx->filter->name, "rgbashift"))
+pix_fmts = rgb_pix_fmts;
+else
+pix_fmts = yuv_pix_fmts;
 
 AVFilterFormats *fmts_list = ff_make_format_list(pix_fmts);
 if (!fmts_list)
@@ -163,6 +182,156 @@ static int wrap_slice ## depth(AVFilterContext *ctx, void 
*arg, int jobnr, int n
 DEFINE_WRAP(8, uint8_t, 1)
 DEFINE_WRAP(16, uint16_t, 2)
 
+#define DEFINE_RGBASMEAR(depth, type, div) 
   \
+static int rgbasmear_slice ## depth(AVFilterContext *ctx, void *arg, int 
jobnr, int nb_jobs)  \
+{  
   \
+ChromaShiftContext *s = ctx->priv; 
   \
+AVFrame *in = s->in;   
   \
+AVFrame *out = arg;
   

Re: [FFmpeg-devel] [PATCH 4/4] lavf/dashenc: Fix AVDictionary leaks in case of various init errors.

2018-11-18 Thread Jeyapal, Karthick
Thanks for sending these excellent patches. The entire patchset looks good to 
me.
Also, many thanks for your patience and taking the earlier review comments in 
the right spirit.

Regards,
Karthick

On 11/17/18 11:10 PM, Andrey Semashev wrote:
> ---
>  libavformat/dashenc.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index f552503564..2c872f93a1 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -833,12 +833,12 @@ static int write_manifest(AVFormatContext *s, int final)
>  snprintf(temp_filename, sizeof(temp_filename), use_rename ? "%s.tmp" : 
> "%s", s->url);
>  set_http_options(&opts, c);
>  ret = dashenc_io_open(s, &c->mpd_out, temp_filename, &opts);
> +av_dict_free(&opts);
>  if (ret < 0) {
>  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
> temp_filename);
>  return ret;
>  }
>  out = c->mpd_out;
> -av_dict_free(&opts);
>  avio_printf(out, "\n");
>  avio_printf(out, " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n";
>  "\txmlns=\"urn:mpeg:dash:schema:mpd:2011\"\n"
> @@ -924,11 +924,11 @@ static int write_manifest(AVFormatContext *s, int final)
>  
>  set_http_options(&opts, c);
>  ret = dashenc_io_open(s, &c->m3u8_out, temp_filename, &opts);
> +av_dict_free(&opts);
>  if (ret < 0) {
>  av_log(s, AV_LOG_ERROR, "Unable to open %s for writing\n", 
> temp_filename);
>  return ret;
>  }
> -av_dict_free(&opts);
>  
>  ff_hls_write_playlist_version(c->m3u8_out, 7);
>  
> @@ -1122,9 +1122,9 @@ static int dash_init(AVFormatContext *s)
>  snprintf(filename, sizeof(filename), "%s%s", c->dirname, 
> os->initfile);
>  set_http_options(&opts, c);
>  ret = s->io_open(s, &os->out, filename, AVIO_FLAG_WRITE, &opts);
> +av_dict_free(&opts);
>  if (ret < 0)
>  return ret;
> -av_dict_free(&opts);
>  os->init_start_pos = 0;
>  
>  if (c->format_options_str) {
> @@ -1145,11 +1145,12 @@ static int dash_init(AVFormatContext *s)
>  av_dict_set_int(&opts, "dash_track_number", i + 1, 0);
>  av_dict_set_int(&opts, "live", 1, 0);
>  }
> -if ((ret = avformat_init_output(ctx, &opts)) < 0)
> +ret = avformat_init_output(ctx, &opts);
> +av_dict_free(&opts);
> +if (ret < 0)
>  return ret;
>  os->ctx_inited = 1;
>  avio_flush(ctx->pb);
> -av_dict_free(&opts);
>  
>  av_log(s, AV_LOG_VERBOSE, "Representation %d init segment will be 
> written to: %s\n", i, filename);
>  
> @@ -1553,9 +1554,9 @@ static int dash_write_packet(AVFormatContext *s, 
> AVPacket *pkt)
>   use_rename ? "%s.tmp" : "%s", os->full_path);
>  set_http_options(&opts, c);
>  ret = dashenc_io_open(s, &os->out, os->temp_path, &opts);
> +av_dict_free(&opts);
>  if (ret < 0)
>  return ret;
> -av_dict_free(&opts);
>  }
>  
>  //write out the data immediately in streaming mode

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