Re: [FFmpeg-devel] [PATCH 11/11] avutil/rational: use frexp rather than ad-hoc log to get floating point exponent

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 12:20 AM, Ganesh Ajjanagadde
 wrote:
> This simplifies and cleans up the code.
> Furthermore, it is much faster due to absence of the slow log computation.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavutil/rational.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavutil/rational.c b/libavutil/rational.c
> index 21d2bb7..81a9402 100644
> --- a/libavutil/rational.c
> +++ b/libavutil/rational.c
> @@ -106,14 +106,14 @@ AVRational av_sub_q(AVRational b, AVRational c)
>  AVRational av_d2q(double d, int max)
>  {
>  AVRational a;
> -#define LOG2  0.69314718055994530941723212145817656807550013436025
>  int exponent;
>  int64_t den;
>  if (isnan(d))
>  return (AVRational) { 0,0 };
>  if (fabs(d) > INT_MAX + 3LL)
>  return (AVRational) { d < 0 ? -1 : 1, 0 };
> -exponent = FFMAX( (int)(log(fabs(d) + 1e-20)/LOG2), 0);
> +frexp(d, );
> +exponent = FFMAX(exponent-1, 0);
>  den = 1LL << (61 - exponent);
>  // (int64_t)rint() and llrint() do not work with gcc on ia64 and sparc64
>  av_reduce(, , floor(d * den + 0.5), den, max);
> --
> 2.6.2
>

Just a note, so that this is not buried: wm4's comment (which I still
disagree with) does not apply to this patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
> This is more concise and conveys the intent better.
> Furthermore, it is likely more precise as well due to lack of floating
> point division.
> 
> Signed-off-by: Ganesh Ajjanagadde 

patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
qemu-arm
fate passes on all, i could test on actual arm hw if people think
that is needed

[...]
-- 
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


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

Re: [FFmpeg-devel] [PATCH 08/11] avfilter/avf_showvolume: use log10 instead of log()/M_LN10

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 8:47 AM, Ganesh Ajjanagadde 
wrote:

> On Thu, Oct 29, 2015 at 8:22 AM, Paul B Mahol  wrote:
> > On 10/29/15, Ganesh Ajjanagadde  wrote:
> >> This is likely more precise and conveys the intent better.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavfilter/avf_showvolume.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavfilter/avf_showvolume.c b/libavfilter/avf_showvolume.c
> >> index 95b5388..395375a 100644
> >> --- a/libavfilter/avf_showvolume.c
> >> +++ b/libavfilter/avf_showvolume.c
> >> @@ -197,7 +197,7 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame
> >> *insamples)
> >>  max = FFMAX(max, src[i]);
> >>
> >>  max = av_clipf(max, 0, 1);
> >> -values[VAR_VOLUME] = 20.0 * log(max) / M_LN10;
> >> +values[VAR_VOLUME] = 20.0 * log10(max);
> >>  values[VAR_CHANNEL] = c;
> >>  color = av_expr_eval(s->c_expr, values, NULL);
> >>
> >> --
> >> 2.6.2
> >
> > Have you checked which one is faster?
> > I really have no opinion on this but gain is neglible.
>
> No, I have not. I suspect, but have not confirmed that log10 is
> slightly slower:
>
> https://stackoverflow.com/questions/10810105/explanation-wanted-log10-faster-than-log-and-log2-but-only-with-o2-and-greater
> (janneb's comment). Keep in mind their benchmark did not include the
> cost of a division.


A good compiler (including yours) will merge the division into the multiply:
a * func(x) / c
is the same as:
(a / c) * func(x)

and the compiler can pre-calculate the output of a/c into a single constant.

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

Re: [FFmpeg-devel] [PATCHv2] all: fix enum definition for large values

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 7:55 AM, Ganesh Ajjanagadde 
wrote:

> On Thu, Oct 29, 2015 at 12:29 AM, Michael Niedermayer
>  wrote:
> > On Wed, Oct 28, 2015 at 07:02:42PM -0400, Ganesh Ajjanagadde wrote:
> >> On Wed, Oct 28, 2015 at 3:05 PM, Ronald S. Bultje 
> wrote:
> >> > Hi,
> >> >
> >> > On Wed, Oct 28, 2015 at 2:46 PM, Ganesh Ajjanagadde <
> gajjanaga...@gmail.com>
> >> > wrote:
> >> >>
> >> >> On Wed, Oct 28, 2015 at 2:39 PM, Ronald S. Bultje <
> rsbul...@gmail.com>
> >> >> wrote:
> >> >> > Hi,
> >> >> >
> >> >> > On Wed, Oct 28, 2015 at 2:31 PM, Ganesh Ajjanagadde
> >> >> > 
> >> >> > wrote:
> >> >> >>
> >> >> >> On Wed, Oct 28, 2015 at 11:38 AM, Ronald S. Bultje <
> rsbul...@gmail.com>
> >> >> >> wrote:
> >> >> >> > Hi,
> >> >> >> >
> >> >> >> > On Wed, Oct 28, 2015 at 11:00 AM, Ganesh Ajjanagadde
> >> >> >> >  wrote:
> >> >> >> >>
> >> >> >> >> On Wed, Oct 28, 2015 at 10:53 AM, Ronald S. Bultje
> >> >> >> >> 
> >> >> >> >> wrote:
> >> >> >> >> > Hi,
> >> >> >> >> >
> >> >> >> >> > On Wed, Oct 28, 2015 at 10:48 AM, Ganesh Ajjanagadde
> >> >> >> >> >  wrote:
> >> >> >> >> >>
> >> >> >> >> >> On Wed, Oct 28, 2015 at 10:34 AM, Ronald S. Bultje
> >> >> >> >> >> 
> >> >> >> >> >> wrote:
> >> >> >> >> >> > Hi,
> >> >> >> >> >> >
> >> >> >> >> >> > On Wed, Oct 28, 2015 at 8:20 AM, Ganesh Ajjanagadde
> >> >> >> >> >> > 
> >> >> >> >> >> > wrote:
> >> >> >> >> >> >>
> >> >> >> >> >> >> On Wed, Oct 28, 2015 at 7:00 AM, Ronald S. Bultje
> >> >> >> >> >> >> 
> >> >> >> >> >> >> wrote:
> >> >> >> >> >> >> > Hi,
> >> >> >> >> >> >> >
> >> >> >> >> >> >> > On Tue, Oct 27, 2015 at 10:58 PM, Ganesh Ajjanagadde
> >> >> >> >> >> >> >  wrote:
> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> On Tue, Oct 27, 2015 at 10:41 PM, Ronald S. Bultje
> >> >> >> >> >> >> >> 
> >> >> >> >> >> >> >> wrote:
> >> >> >> >> >> >> >> > Hi,
> >> >> >> >> >> >> >> >
> >> >> >> >> >> >> >> > On Tue, Oct 27, 2015 at 8:53 PM, Ganesh Ajjanagadde
> >> >> >> >> >> >> >> > 
> >> >> >> >> >> >> >> > wrote:
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> ISO C restricts enumerator values to the range of
> int.
> >> >> >> >> >> >> >> >> Thus
> >> >> >> >> >> >> >> >> (for
> >> >> >> >> >> >> >> >> instance)
> >> >> >> >> >> >> >> >> 0x8000
> >> >> >> >> >> >> >> >> unfortunately does not work, and throws a warning
> with
> >> >> >> >> >> >> >> >> -Wpedantic
> >> >> >> >> >> >> >> >> on
> >> >> >> >> >> >> >> >> clang 3.7.
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> This fixes it by using alternative expressions that
> >> >> >> >> >> >> >> >> result
> >> >> >> >> >> >> >> >> in
> >> >> >> >> >> >> >> >> identical
> >> >> >> >> >> >> >> >> values but do not have this issue.
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> Tested with FATE.
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> Signed-off-by: Ganesh Ajjanagadde
> >> >> >> >> >> >> >> >> 
> >> >> >> >> >> >> >> >> ---
> >> >> >> >> >> >> >> >>  libavcodec/dca_syncwords.h | 26
> >> >> >> >> >> >> >> >> --
> >> >> >> >> >> >> >> >>  libavformat/cinedec.c  |  2 +-
> >> >> >> >> >> >> >> >>  libavformat/mov_chan.c |  2 +-
> >> >> >> >> >> >> >> >>  3 files changed, 14 insertions(+), 16 deletions(-)
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> diff --git a/libavcodec/dca_syncwords.h
> >> >> >> >> >> >> >> >> b/libavcodec/dca_syncwords.h
> >> >> >> >> >> >> >> >> index 3466b6b..6981cb8 100644
> >> >> >> >> >> >> >> >> --- a/libavcodec/dca_syncwords.h
> >> >> >> >> >> >> >> >> +++ b/libavcodec/dca_syncwords.h
> >> >> >> >> >> >> >> >> @@ -19,19 +19,17 @@
> >> >> >> >> >> >> >> >>  #ifndef AVCODEC_DCA_SYNCWORDS_H
> >> >> >> >> >> >> >> >>  #define AVCODEC_DCA_SYNCWORDS_H
> >> >> >> >> >> >> >> >>
> >> >> >> >> >> >> >> >> -enum DCASyncwords {
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_BE= 0x7FFE8001U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_LE= 0xFE7F0180U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_14B_BE= 0x1FFFE800U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_14B_LE= 0xFF1F00E8U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_XCH= 0x5A5A5A5AU,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_XXCH   = 0x47004A03U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_X96= 0x1D95F262U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_XBR= 0x655E315EU,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_LBR= 0x0A801921U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_XLL= 0x41A29547U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_SUBSTREAM  = 0x64582025U,
> >> >> >> >> >> >> >> >> -DCA_SYNCWORD_SUBSTREAM_CORE = 0x02B09261U,
> >> >> >> >> >> >> >> 

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: add testsrc2 test source.

[Nicolas George]

Le sextidi 6 brumaire, an CCXXIV, Clement Boesch a écrit :
> We have testsrc and mptestsrc so... fftestsrc? or ffmire, miresrc,
> colorfuzz, rainbowlicornsrc, ... or it could also just be a "ffmpeg" mode
> option for testsrc so we could have all sort of mire templates (but that's
> going to be inconsistent with the other smptebars & friends).
> 
> I don't know.

None of these seem really better than testsrc2, they introduce a new
non-intuitive name. For now, I think I will stick to testsrc2.

> BTW, while I'm at it, I think a test source generating 10-12 bits video
> stream would be great for testing purpose. This mire looks like a good
> candidate.

This would be useful indeed. Unfortunately, all the drawutils code is really
8-bits-centric. If someone updates it to work with higher bit depths, then
this source would be able to somewhat make use of it: with a little change,
the diagonal gradient and the noise checkers can make use of the low order
bits.

For high bit depth, I suspect some kind of fractal or other mathematical
pattern generator would be more suited.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 08/11] avfilter/avf_showvolume: use log10 instead of log()/M_LN10

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:22 AM, Paul B Mahol  wrote:
> On 10/29/15, Ganesh Ajjanagadde  wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/avf_showvolume.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/avf_showvolume.c b/libavfilter/avf_showvolume.c
>> index 95b5388..395375a 100644
>> --- a/libavfilter/avf_showvolume.c
>> +++ b/libavfilter/avf_showvolume.c
>> @@ -197,7 +197,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
>> *insamples)
>>  max = FFMAX(max, src[i]);
>>
>>  max = av_clipf(max, 0, 1);
>> -values[VAR_VOLUME] = 20.0 * log(max) / M_LN10;
>> +values[VAR_VOLUME] = 20.0 * log10(max);
>>  values[VAR_CHANNEL] = c;
>>  color = av_expr_eval(s->c_expr, values, NULL);
>>
>> --
>> 2.6.2
>
> Have you checked which one is faster?
> I really have no opinion on this but gain is neglible.

No, I have not. I suspect, but have not confirmed that log10 is
slightly slower:
https://stackoverflow.com/questions/10810105/explanation-wanted-log10-faster-than-log-and-log2-but-only-with-o2-and-greater
(janneb's comment). Keep in mind their benchmark did not include the
cost of a division.

I also suspect that this is mostly an artifact of libm, see links:
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/dbl-64/e_log10.c;hb=HEAD
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/dbl-64/e_log2.c;hb=HEAD
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/dbl-64/e_log.c;hb=HEAD

Basically log10 uses _ieee754_log (essentially log) underneath. No one
has taken the trouble of creating a different polynomial, etc for
log10. I strongly suspect this is because most clients of libm anyway
use log instead of log10, which is arguably rarer. Furthermore,
improvements in _ieee754_log are the lowest hanging fruit for them,
since due to the above, any improvements there translate into
improvements in the other log functions.
Lastly, work has been done on optimizing log, and due to the above
architecture, log10 inherently relies on it:
https://developerblog.redhat.com/2015/01/02/improving-math-performance-in-glibc/
I am pretty sure that if the libm people spent the time to optimize
log10 (e.g by creating different polynomial, etc), the gap should
narrow.

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

Re: [FFmpeg-devel] [PATCH 04/11] avcodec/zmbvenc: use log2 instead of log()/M_LN2

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:02AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/zmbvenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c
> index df06e37..e832bed 100644
> --- a/libavcodec/zmbvenc.c
> +++ b/libavcodec/zmbvenc.c
> @@ -277,7 +277,7 @@ static av_cold int encode_init(AVCodecContext *avctx)
>  int lvl = 9;
>  
>  for(i=1; i<256; i++)
> -score_tab[i]= -i * log(i/(double)(ZMBV_BLOCK*ZMBV_BLOCK)) * 
> (256/M_LN2);
> +score_tab[i]= -i * log2(i/(double)(ZMBV_BLOCK*ZMBV_BLOCK)) * 256;

Interrestingly this changes the output from
ffmpeg -i matrixbench_mpeg2.mpg -vcodec zmbv -frames 30  -an -f framecrc -
for a few frames

the code change looks good though

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer  wrote:

> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
> > This is more concise and conveys the intent better.
> > Furthermore, it is likely more precise as well due to lack of floating
> > point division.
> >
> > Signed-off-by: Ganesh Ajjanagadde 
>
> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
> qemu-arm
> fate passes on all, i could test on actual arm hw if people think
> that is needed


I don't think that's needed.

Is there some way we can confirm that each of these files that is changed
includes libavutil/libm.h for the compatibility macros in case they're
lacking on the target system?

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

Re: [FFmpeg-devel] [PATCH] fate: add decimate test.

[Nicolas George]

Le quintidi 5 brumaire, an CCXXIV, Michael Niedermayer a écrit :
> seems failing as 32bit on 64bit x86 linux (works on 64bit)
> 
> --- ffmpeg/tests/ref/fate/filter-decimate-simple   2015-10-25 
> 23:19:29.618103426 +0100
> +++ tests/data/fate/filter-decimate-simple  2015-10-26 00:02:08.838157343 
> +0100
> @@ -2,17 +2,17 @@
>  0,  1,  1,1,   115200, 0x5e0f54f3
>  0,  2,  2,1,   115200, 0xd5c25c93
>  0,  3,  3,1,   115200, 0x372c655c
> -0,  5,  5,1,   115200, 0xedbaed7f
> +0,  4,  4,1,   115200, 0xedbaed7f
>  0,  6,  6,1,   115200, 0x8b2a40a6
>  0,  7,  7,1,   115200, 0x4e0547fe
> -0,  9,  9,1,   115200, 0xca5a3761
> +0,  8,  8,1,   115200, 0xca5a3761
>  0, 10, 10,1,   115200, 0xe7562c6f
>  0, 11, 11,1,   115200, 0x709f3960
> -0, 13, 13,1,   115200, 0x2f76d8f2
> +0, 12, 12,1,   115200, 0x2f76d8f2
>  0, 14, 14,1,   115200, 0x72045ce1
>  0, 15, 15,1,   115200, 0xa6b36e8e
> -0, 17, 17,1,   115200, 0x275a0a3a
> +0, 16, 16,1,   115200, 0x275a0a3a
>  0, 18, 18,1,   115200, 0x87f04127
>  0, 19, 19,1,   115200, 0x442d4f33
> -0, 21, 21,1,   115200, 0x8e892c86
> +0, 20, 20,1,   115200, 0x8e892c86
>  0, 22, 22,1,   115200, 0xb214258f
> Test filter-decimate-simple failed. Look at 
> tests/data/fate/filter-decimate-simple.err for details.
> make: *** [fate-filter-decimate-simple] Error 1

Thanks for catching this. It selects the same frame, but gives it a
different timestamp. I just noticed the timestamp computations are made
with doubles, and does not take the input frame timestamps into account. I
would say that this filter is broken in several ways, there is no need for
testing it before it is fixed. Patch dropped. 

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 5/8] tools/crypto_bench: add AES-CBC modes

[Nicolas George]

Le septidi 7 brumaire, an CCXXIV, Rodger Combs a écrit :
> ---
>  tools/crypto_bench.c | 140 
> +--
>  1 file changed, 137 insertions(+), 3 deletions(-)

Should be ok. Not sure all the modes are really needed since the operation
mode is quite orthogonal to the block cipher itself, but it does not matter
much.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 6/8] tools/crypto_bench: switch to OpenSSL's faster AES API

[Nicolas George]

Le septidi 7 brumaire, an CCXXIV, Rodger Combs a écrit :
> ---
>  tools/crypto_bench.c | 85 
> +---
>  1 file changed, 47 insertions(+), 38 deletions(-)

I do not know enough of libcrypto API to comment on the actual code. No
objection if it works. Could you include before/after numbers to the commit
message to show how faster it is?

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 4/8] tools/crypto_bench: add AES-192 and AES-256

[Nicolas George]

Le septidi 7 brumaire, an CCXXIV, Rodger Combs a écrit :
> ---
>  tools/crypto_bench.c | 82 
> 
>  1 file changed, 82 insertions(+)

Just a suggestion: maybe use macros to factor the code between the three key
sizes. That would probably reduce the next patch about the libcrypto faster
API. But since it was already approved, do not feel compelled.

Regards,

-- 
  Nicolas George


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

[FFmpeg-devel] [PATCH] vp9: update timestamps in ref files using multiple invisible frames.

[Ronald S. Bultje]

For the 10-show-existing-frame, the source file indeed has a timestamp
of 3 (or 100/33) for the second visible frame, so the fix appears to
work correctly. For the other, only the timebase is fixed, but again
appears to be correct now.
---
 tests/ref/fate/vp9-10-show-existing-frame | 2 +-
 tests/ref/fate/vp9-16-intra-only  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ref/fate/vp9-10-show-existing-frame 
b/tests/ref/fate/vp9-10-show-existing-frame
index 2885b00..6a2c904 100644
--- a/tests/ref/fate/vp9-10-show-existing-frame
+++ b/tests/ref/fate/vp9-10-show-existing-frame
@@ -4,7 +4,7 @@
 #tb 0: 1/30
 #stream#, dts,pts, duration, size, hash
 0,  0,  0,1,   152064, 18981342ec178e082519451062c3a67f
-0,  1,  1,1,   152064, 04ab9dbeac49ec31be58f6e671698e05
+0,  3,  3,1,   152064, 04ab9dbeac49ec31be58f6e671698e05
 0,  4,  4,1,   152064, 4ed58a0ba93a5d97a232a50c5876cda2
 0,  6,  6,1,   152064, a41f00034923e56ba51a0b598acc2e3a
 0,  7,  7,1,   152064, 63fa55ae9535ccdf06d44cce8065dda6
diff --git a/tests/ref/fate/vp9-16-intra-only b/tests/ref/fate/vp9-16-intra-only
index 5802a53..1e8d280 100644
--- a/tests/ref/fate/vp9-16-intra-only
+++ b/tests/ref/fate/vp9-16-intra-only
@@ -1,7 +1,7 @@
 #format: frame checksums
 #version: 1
 #hash: MD5
-#tb 0: 12/359
+#tb 0: 1001/3
 #stream#, dts,pts, duration, size, hash
 0,  0,  0,1,   152064, d57529601178948afa4818c3c8938884
 0,  1,  1,1,   152064, d47e00250c45733d64af067a417bcd06
-- 
2.1.2

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:44 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Oct 29, 2015 at 8:32 AM, Ganesh Ajjanagadde 
> wrote:
>
>> On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer
>> > >> wrote:
>> >
>> >> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
>> >> > This is more concise and conveys the intent better.
>> >> > Furthermore, it is likely more precise as well due to lack of floating
>> >> > point division.
>> >> >
>> >> > Signed-off-by: Ganesh Ajjanagadde 
>> >>
>> >> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
>> >> qemu-arm
>> >> fate passes on all, i could test on actual arm hw if people think
>> >> that is needed
>> >
>> >
>> > I don't think that's needed.
>> >
>> > Is there some way we can confirm that each of these files that is changed
>> > includes libavutil/libm.h for the compatibility macros in case they're
>> > lacking on the target system?
>>
>> Don't know, but in case any were wondering: I assumed the availability
>> simply because existing code already used log2, log10. This is also
>> why even without the accuracy change, I personally consider the
>> patchset an improvement due to it making FFmpeg more consistent in its
>> choice of log function.
>
>
> The risk is that some file in the dusty corners of our codebase forgets to
> include libavutil/libm.h, the system does not have log2 or whatnot (which
> is exactly why we have the compat wrappers in libavutil/libm.h - they are
> not there because somebody felt like writing it just in case it happens -
> they were written because it _did_ happen), and as a result it fails to
> compile when a user on aix or msvc2010 or whatever affected system tries to
> compile the next release.

Thanks for clarifying.

>
> That would suck, and users would think ffmpeg sucks as a result. So that's
> why I'm wondering if we can confirm that all affected files do include
> libavutil/libm.h, so that we can be sure there's nothing to worry about.

Indeed.

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

Re: [FFmpeg-devel] [PATCH 7/8] tools/crypto_bench: add support for multiple lavu versions by cpuflag

[Nicolas George]

Le septidi 7 brumaire, an CCXXIV, Rodger Combs a écrit :
> ---
>  tools/crypto_bench.c | 50 ++
>  1 file changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/crypto_bench.c b/tools/crypto_bench.c
> index 8a468ba..b513c55 100644
> --- a/tools/crypto_bench.c
> +++ b/tools/crypto_bench.c
> @@ -32,6 +32,7 @@
>  #include "libavutil/crc.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/timer.h"
> +#include "libavutil/cpu.h"
>  
>  #ifndef AV_READ_TIME
>  #define AV_READ_TIME(x) 0
> @@ -65,6 +66,7 @@ struct hash_impl {
>  const char *name;
>  void (*run)(uint8_t *output, const uint8_t *input, unsigned size);
>  const char *output;
> +int cpu_versions;
>  };
>  
>  /***
> @@ -667,6 +669,16 @@ static unsigned crc32(const uint8_t *data, unsigned size)
>  return av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, data, size);
>  }
>  
> +static const struct {
> +int flag;
> +const char *name;
> +} cpu_flag_tab[] = {
> +#if ARCH_X86
> +{ AV_CPU_FLAG_AESNI, "aesni"  },
> +#endif
> +{ 0 }
> +};
> +
>  static void run_implementation(const uint8_t *input, uint8_t *output,
> struct hash_impl *impl, unsigned size)
>  {
> @@ -677,9 +689,31 @@ static void run_implementation(const uint8_t *input, 
> uint8_t *output,
>  double mtime, ttime = 0, ttime2 = 0, stime;
>  uint8_t outref[MAX_OUTPUT_SIZE];
>  

> +if (!strcmp(impl->lib, "lavu")) {
> +char lib_name[32];
> +struct hash_impl impl2 = *impl;
> +int real_flags = av_get_cpu_flags();
> +int current_flags = real_flags;
> +impl2.cpu_versions = 0;
> +impl2.lib = lib_name;
> +for (i = 0; cpu_flag_tab[i].flag; i++) {
> +if (cpu_flag_tab[i].flag & impl->cpu_versions & real_flags) {
> +snprintf(lib_name, sizeof(lib_name), "lavu_%s", 
> cpu_flag_tab[i].name);
> +run_implementation(input, output, , size);
> +current_flags &= ~cpu_flag_tab[i].flag;
> +av_force_cpu_flags(current_flags);
> +}
> +}
> +impl2.lib = "lavu_c";
> +run_implementation(input, output, , size);
> +av_force_cpu_flags(real_flags);
> +return;
> +}
> +

I am still not very happy with the complex logic of the code, but I have no
good suggestion to make it simpler.

>  if (enabled_libs  && !av_stristr(enabled_libs,  impl->lib) ||
>  enabled_algos && !av_stristr(enabled_algos, impl->name))
>  return;

> +

Stray.

>  if (!sscanf(impl->output, "crc:%x", )) {
>  outlen = strlen(impl->output) / 2;
>  for (i = 0; i < outlen; i++) {
> @@ -718,8 +752,8 @@ static void run_implementation(const uint8_t *input, 
> uint8_t *output,
>  fflush(stdout);
>  }
>  
> -#define IMPL_USE(lib, name, symbol, output) \
> -{ #lib, name, run_ ## lib ## _ ## symbol, output },
> +#define IMPL_USE(lib, name, symbol, output, ...) \
> +{ #lib, name, run_ ## lib ## _ ## symbol, output, __VA_ARGS__ },
>  #define IMPL(lib, ...) IMPL_USE_ ## lib(lib, __VA_ARGS__)
>  #define IMPL_ALL(...) \
>  IMPL(lavu,   __VA_ARGS__) \
> @@ -736,12 +770,12 @@ struct hash_impl implementations[] = {
>  IMPL(lavu, "RIPEMD-128", ripemd128, 
> "9ab8bfba2ddccc5d99c9d4cdfb844a5f")
>  IMPL(tomcrypt, "RIPEMD-128", ripemd128, 
> "9ab8bfba2ddccc5d99c9d4cdfb844a5f")
>  IMPL_ALL("RIPEMD-160", ripemd160, 
> "62a5321e4fc8784903bb43ab7752c75f8b25af00")
> -IMPL_ALL("AES-128-ECB",aes128,"crc:ff6bc888")
> -IMPL_ALL("AES-192-ECB",aes192,"crc:1022815b")
> -IMPL_ALL("AES-256-ECB",aes256,"crc:792e4e8a")
> -IMPL_ALL("AES-128-CBC",aes128cbc, "crc:0efebabe")
> -IMPL_ALL("AES-192-CBC",aes192cbc, "crc:ee2e34e8")
> -IMPL_ALL("AES-256-CBC",aes256cbc, "crc:0c9b875c")
> +IMPL_ALL("AES-128-ECB",aes128,"crc:ff6bc888", AV_CPU_FLAG_AESNI)
> +IMPL_ALL("AES-192-ECB",aes192,"crc:1022815b", AV_CPU_FLAG_AESNI)
> +IMPL_ALL("AES-256-ECB",aes256,"crc:792e4e8a", AV_CPU_FLAG_AESNI)
> +IMPL_ALL("AES-128-CBC",aes128cbc, "crc:0efebabe", AV_CPU_FLAG_AESNI)
> +IMPL_ALL("AES-192-CBC",aes192cbc, "crc:ee2e34e8", AV_CPU_FLAG_AESNI)
> +IMPL_ALL("AES-256-CBC",aes256cbc, "crc:0c9b875c", AV_CPU_FLAG_AESNI)
>  IMPL_ALL("CAMELLIA",   camellia,  "crc:7abb59a7")
>  IMPL_ALL("CAST-128",   cast128,   "crc:456aa584")
>  IMPL_ALL("BLOWFISH",   blowfish,  "crc:33e8aa74")

Should be ok.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] vp9: update timestamps in ref files using multiple invisible frames.

[Hendrik Leppkes]

On Thu, Oct 29, 2015 at 12:40 PM, Ronald S. Bultje  wrote:
> For the 10-show-existing-frame, the source file indeed has a timestamp
> of 3 (or 100/33) for the second visible frame, so the fix appears to
> work correctly. For the other, only the timebase is fixed, but again
> appears to be correct now.
> ---
>  tests/ref/fate/vp9-10-show-existing-frame | 2 +-
>  tests/ref/fate/vp9-16-intra-only  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ref/fate/vp9-10-show-existing-frame 
> b/tests/ref/fate/vp9-10-show-existing-frame
> index 2885b00..6a2c904 100644
> --- a/tests/ref/fate/vp9-10-show-existing-frame
> +++ b/tests/ref/fate/vp9-10-show-existing-frame
> @@ -4,7 +4,7 @@
>  #tb 0: 1/30
>  #stream#, dts,pts, duration, size, hash
>  0,  0,  0,1,   152064, 
> 18981342ec178e082519451062c3a67f
> -0,  1,  1,1,   152064, 
> 04ab9dbeac49ec31be58f6e671698e05
> +0,  3,  3,1,   152064, 
> 04ab9dbeac49ec31be58f6e671698e05
>  0,  4,  4,1,   152064, 
> 4ed58a0ba93a5d97a232a50c5876cda2
>  0,  6,  6,1,   152064, 
> a41f00034923e56ba51a0b598acc2e3a
>  0,  7,  7,1,   152064, 
> 63fa55ae9535ccdf06d44cce8065dda6
> diff --git a/tests/ref/fate/vp9-16-intra-only 
> b/tests/ref/fate/vp9-16-intra-only
> index 5802a53..1e8d280 100644
> --- a/tests/ref/fate/vp9-16-intra-only
> +++ b/tests/ref/fate/vp9-16-intra-only
> @@ -1,7 +1,7 @@
>  #format: frame checksums
>  #version: 1
>  #hash: MD5
> -#tb 0: 12/359
> +#tb 0: 1001/3
>  #stream#, dts,pts, duration, size, hash
>  0,  0,  0,1,   152064, 
> d57529601178948afa4818c3c8938884
>  0,  1,  1,1,   152064, 
> d47e00250c45733d64af067a417bcd06
> --
> 2.1.2
>

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

Re: [FFmpeg-devel] [PATCH 03/11] avcodec/snowenc: use log2 instead of log() / log(2...)

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:01AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> The expression has also been accordingly simplified.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/snowenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
> index 7e8269c..fb0cd3f 100644
> --- a/libavcodec/snowenc.c
> +++ b/libavcodec/snowenc.c
> @@ -1547,7 +1547,7 @@ static void calculate_visual_weight(SnowContext *s, 
> Plane *p){
>  }
>  }
>  
> -b->qlog= (int)(log(352256.0/sqrt(error)) / log(pow(2.0, 
> 1.0/QROOT))+0.5);
> +b->qlog= (int)(QROOT * log2(352256.0/sqrt(error)) + 0.5);

LGTM

thanks

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH] avutil/opt: add av_warn_unused_result

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 5:23 AM, wm4  wrote:
> On Thu, 29 Oct 2015 09:17:16 +0100
> Hendrik Leppkes  wrote:
>
>> On Thu, Oct 29, 2015 at 3:51 AM, Ganesh Ajjanagadde
>>  wrote:
>> > On Thu, Oct 15, 2015 at 6:12 PM, Ganesh Ajjanagadde
>> >  wrote:
>> >> Some warnings do get triggered that will need to be fixed.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> ---
>> >>  libavutil/opt.h | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/libavutil/opt.h b/libavutil/opt.h
>> >> index 753434d..5abe963 100644
>> >> --- a/libavutil/opt.h
>> >> +++ b/libavutil/opt.h
>> >> @@ -414,6 +414,7 @@ void av_opt_set_defaults2(void *s, int mask, int 
>> >> flags);
>> >>   * the error code issued by av_opt_set() if a key/value pair
>> >>   * cannot be set
>> >>   */
>> >> +av_warn_unused_result
>> >>  int av_set_options_string(void *ctx, const char *opts,
>> >>const char *key_val_sep, const char 
>> >> *pairs_sep);
>> >>
>> >> @@ -444,6 +445,7 @@ int av_set_options_string(void *ctx, const char *opts,
>> >>   * Separators must use characters distinct from option names and from 
>> >> each
>> >>   * other.
>> >>   */
>> >> +av_warn_unused_result
>> >>  int av_opt_set_from_string(void *ctx, const char *opts,
>> >> const char *const *shorthand,
>> >> const char *key_val_sep, const char 
>> >> *pairs_sep);
>> >> --
>> >> 2.6.1
>> >>
>> >
>> > ping
>>
>> I would argue that failure from these functions is hardly critical,
>> ie. it can fail if an option is not found, and enforcing checks here
>> seems a bit too aggressive to me.
>
> +1
>
> You might often set options on e.g. a codec generically, without caring
> much whether it fails or not. This is because different codecs have
> different options, and those which lack a certain option will work
> without setting it. (Just as an example.)

Thanks for examining, patch dropped.

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

Re: [FFmpeg-devel] [PATCH 5/8] tools/crypto_bench: add AES-CBC modes

[Rodger Combs]

Take a look at some results; CBC is significantly slower than CTR in both 
libcrypto's and our AES code. There's some hardware limitation it hits.

> On Oct 29, 2015, at 06:13, Nicolas George  wrote:
> 
> Le septidi 7 brumaire, an CCXXIV, Rodger Combs a écrit :
>> ---
>> tools/crypto_bench.c | 140 
>> +--
>> 1 file changed, 137 insertions(+), 3 deletions(-)
> 
> Should be ok. Not sure all the modes are really needed since the operation
> mode is quite orthogonal to the block cipher itself, but it does not matter
> much.
> 
> Regards,
> 
> -- 
>  Nicolas George
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 7:36 AM, wm4  wrote:
> On Thu, 29 Oct 2015 07:27:20 -0400
> Ganesh Ajjanagadde  wrote:
>
>> On Thu, Oct 29, 2015 at 5:43 AM, wm4  wrote:
>> > On Thu, 29 Oct 2015 00:19:59 -0400
>> > Ganesh Ajjanagadde  wrote:
>> >
>> >> This is more concise and conveys the intent better.
>> >> Furthermore, it is likely more precise as well due to lack of floating
>> >> point division.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> ---
>> >
>> > These patches are all pretty similar. And likely tedious to check for
>> > correctness. So low gain, while some potential for regressions.
>>
>> In the time it took you to write that comment, you could have easily
>> reviewed a couple. log10 is already being used in the codebase, why
>> not make it consistent and also make it more precise?
>>
>> There is a good reason why libc has a log10 function.
>>
>> Maybe you don't care about such things, but I do: it is (roughly)
>> analogous to using double instead of float for filters/resampling etc
>> - noise floor should not be increased unless there is a clear benefit.
>> Here there is none from using log as opposed to log10.
>
> Not going to play this superficially-review-mass-patches-with-little-to-
> none-benefit-just-so-a-single-developer-doesn't-get-annoyed-and-then-deal-
> with-regression-fallout game.
>
> You're giving the ML more patches to review than it can deal with it, so
> my reaction is to flat out reject such patch-spam for things which seem to
> have little benefit and are not 100% trivial.

How? I send fewer patches than many other developers here.

>
> (This reminds me of mass cosmetics from Libav...)

Comparing this with cosmetics is not accurate - these are NOT
cosmetics. They affect the bit accuracy of the results. To prove to
the likes of wm4 that this is not cosmetic, here are some simple
results:
computing 10*log10(x) vs 10*log10(x)/log(10.0), top is first, bottom
is second expr:
x = 1e-12:
-120.000
-119.986

x = DBL_MIN:
-3076.526555685887615
-3076.526555685887161

x = 5*1e32:
326.989700043360187
326.989700043360131

These are differences obtained within the first 15 digits. I am pretty
confident that by finding the worst case, one can get even starker
differences.

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer > wrote:
>
>> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
>> > This is more concise and conveys the intent better.
>> > Furthermore, it is likely more precise as well due to lack of floating
>> > point division.
>> >
>> > Signed-off-by: Ganesh Ajjanagadde 
>>
>> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
>> qemu-arm
>> fate passes on all, i could test on actual arm hw if people think
>> that is needed
>
>
> I don't think that's needed.
>
> Is there some way we can confirm that each of these files that is changed
> includes libavutil/libm.h for the compatibility macros in case they're
> lacking on the target system?

Don't know, but in case any were wondering: I assumed the availability
simply because existing code already used log2, log10. This is also
why even without the accuracy change, I personally consider the
patchset an improvement due to it making FFmpeg more consistent in its
choice of log function.

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[wm4]

On Thu, 29 Oct 2015 07:27:20 -0400
Ganesh Ajjanagadde  wrote:

> On Thu, Oct 29, 2015 at 5:43 AM, wm4  wrote:
> > On Thu, 29 Oct 2015 00:19:59 -0400
> > Ganesh Ajjanagadde  wrote:
> >  
> >> This is more concise and conveys the intent better.
> >> Furthermore, it is likely more precise as well due to lack of floating
> >> point division.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---  
> >
> > These patches are all pretty similar. And likely tedious to check for
> > correctness. So low gain, while some potential for regressions.  
> 
> In the time it took you to write that comment, you could have easily
> reviewed a couple. log10 is already being used in the codebase, why
> not make it consistent and also make it more precise?
> 
> There is a good reason why libc has a log10 function.
> 
> Maybe you don't care about such things, but I do: it is (roughly)
> analogous to using double instead of float for filters/resampling etc
> - noise floor should not be increased unless there is a clear benefit.
> Here there is none from using log as opposed to log10.

Not going to play this superficially-review-mass-patches-with-little-to-
none-benefit-just-so-a-single-developer-doesn't-get-annoyed-and-then-deal-
with-regression-fallout game.

You're giving the ML more patches to review than it can deal with it, so
my reaction is to flat out reject such patch-spam for things which seem to
have little benefit and are not 100% trivial.

(This reminds me of mass cosmetics from Libav...)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/tremolo: fix wavetable buffer size

[Paul B Mahol]

On 10/29/15, Kyle Swanson  wrote:
> Signed-off-by: Kyle Swanson 
> ---
>  libavfilter/af_tremolo.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/af_tremolo.c b/libavfilter/af_tremolo.c
> index 50df2e4..572e9e3 100644
> --- a/libavfilter/af_tremolo.c
> +++ b/libavfilter/af_tremolo.c
> @@ -72,7 +72,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  dst += channels;
>  src += channels;
>  s->index++;
> -if (s->index >= inlink->sample_rate)
> +if (s->index >= inlink->sample_rate / s->freq)
>  s->index = 0;
>  }
>
> @@ -125,11 +125,11 @@ static int config_input(AVFilterLink *inlink)
>  const double offset = 1. - s->depth / 2.;
>  int i;
>
> -s->table = av_malloc_array(inlink->sample_rate, sizeof(*s->table));
> +s->table = av_malloc_array(inlink->sample_rate / s->freq,
> sizeof(*s->table));
>  if (!s->table)
>  return AVERROR(ENOMEM);
>
> -for (i = 0; i < inlink->sample_rate; i++) {
> +for (i = 0; i < inlink->sample_rate / s->freq; i++) {
>  double env = s->freq * i / inlink->sample_rate;
>  env = sin(2 * M_PI * fmod(env + 0.25, 1.0));
>  s->table[i] = env * (1 - fabs(offset)) + offset;
> --
> 2.6.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] [PATCH 08/11] avfilter/avf_showvolume: use log10 instead of log()/M_LN10

[Paul B Mahol]

On 10/29/15, Ganesh Ajjanagadde  wrote:
> This is likely more precise and conveys the intent better.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/avf_showvolume.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavfilter/avf_showvolume.c b/libavfilter/avf_showvolume.c
> index 95b5388..395375a 100644
> --- a/libavfilter/avf_showvolume.c
> +++ b/libavfilter/avf_showvolume.c
> @@ -197,7 +197,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *insamples)
>  max = FFMAX(max, src[i]);
>
>  max = av_clipf(max, 0, 1);
> -values[VAR_VOLUME] = 20.0 * log(max) / M_LN10;
> +values[VAR_VOLUME] = 20.0 * log10(max);
>  values[VAR_CHANNEL] = c;
>  color = av_expr_eval(s->c_expr, values, NULL);
>
> --
> 2.6.2

Have you checked which one is faster?
I really have no opinion on this but gain is neglible.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] vp9: update timestamps in ref files using multiple invisible frames.

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 7:42 AM, Hendrik Leppkes 
wrote:

> On Thu, Oct 29, 2015 at 12:40 PM, Ronald S. Bultje 
> wrote:
> > For the 10-show-existing-frame, the source file indeed has a timestamp
> > of 3 (or 100/33) for the second visible frame, so the fix appears to
> > work correctly. For the other, only the timebase is fixed, but again
> > appears to be correct now.
> > ---
> >  tests/ref/fate/vp9-10-show-existing-frame | 2 +-
> >  tests/ref/fate/vp9-16-intra-only  | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/ref/fate/vp9-10-show-existing-frame
> b/tests/ref/fate/vp9-10-show-existing-frame
> > index 2885b00..6a2c904 100644
> > --- a/tests/ref/fate/vp9-10-show-existing-frame
> > +++ b/tests/ref/fate/vp9-10-show-existing-frame
> > @@ -4,7 +4,7 @@
> >  #tb 0: 1/30
> >  #stream#, dts,pts, duration, size, hash
> >  0,  0,  0,1,   152064,
> 18981342ec178e082519451062c3a67f
> > -0,  1,  1,1,   152064,
> 04ab9dbeac49ec31be58f6e671698e05
> > +0,  3,  3,1,   152064,
> 04ab9dbeac49ec31be58f6e671698e05
> >  0,  4,  4,1,   152064,
> 4ed58a0ba93a5d97a232a50c5876cda2
> >  0,  6,  6,1,   152064,
> a41f00034923e56ba51a0b598acc2e3a
> >  0,  7,  7,1,   152064,
> 63fa55ae9535ccdf06d44cce8065dda6
> > diff --git a/tests/ref/fate/vp9-16-intra-only
> b/tests/ref/fate/vp9-16-intra-only
> > index 5802a53..1e8d280 100644
> > --- a/tests/ref/fate/vp9-16-intra-only
> > +++ b/tests/ref/fate/vp9-16-intra-only
> > @@ -1,7 +1,7 @@
> >  #format: frame checksums
> >  #version: 1
> >  #hash: MD5
> > -#tb 0: 12/359
> > +#tb 0: 1001/3
> >  #stream#, dts,pts, duration, size, hash
> >  0,  0,  0,1,   152064,
> d57529601178948afa4818c3c8938884
> >  0,  1,  1,1,   152064,
> d47e00250c45733d64af067a417bcd06
> > --
> > 2.1.2
> >
>
> LGTM.


Pushed, and sorry for the breakage.

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

Re: [FFmpeg-devel] [PATCH 08/11] avfilter/avf_showvolume: use log10 instead of log()/M_LN10

[Ganesh Ajjanagadde]

On Oct 29, 2015 9:26 AM, "Ronald S. Bultje"  wrote:
>
> Hi,
>
> On Thu, Oct 29, 2015 at 8:47 AM, Ganesh Ajjanagadde 
> wrote:
>
> > On Thu, Oct 29, 2015 at 8:22 AM, Paul B Mahol  wrote:
> > > On 10/29/15, Ganesh Ajjanagadde  wrote:
> > >> This is likely more precise and conveys the intent better.
> > >>
> > >> Signed-off-by: Ganesh Ajjanagadde 
> > >> ---
> > >>  libavfilter/avf_showvolume.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/libavfilter/avf_showvolume.c
b/libavfilter/avf_showvolume.c
> > >> index 95b5388..395375a 100644
> > >> --- a/libavfilter/avf_showvolume.c
> > >> +++ b/libavfilter/avf_showvolume.c
> > >> @@ -197,7 +197,7 @@ static int filter_frame(AVFilterLink *inlink,
> > AVFrame
> > >> *insamples)
> > >>  max = FFMAX(max, src[i]);
> > >>
> > >>  max = av_clipf(max, 0, 1);
> > >> -values[VAR_VOLUME] = 20.0 * log(max) / M_LN10;
> > >> +values[VAR_VOLUME] = 20.0 * log10(max);
> > >>  values[VAR_CHANNEL] = c;
> > >>  color = av_expr_eval(s->c_expr, values, NULL);
> > >>
> > >> --
> > >> 2.6.2
> > >
> > > Have you checked which one is faster?
> > > I really have no opinion on this but gain is neglible.
> >
> > No, I have not. I suspect, but have not confirmed that log10 is
> > slightly slower:
> >
> >
https://stackoverflow.com/questions/10810105/explanation-wanted-log10-faster-than-log-and-log2-but-only-with-o2-and-greater
> > (janneb's comment). Keep in mind their benchmark did not include the
> > cost of a division.
>
>
> A good compiler (including yours) will merge the division into the
multiply:
> a * func(x) / c
> is the same as:
> (a / c) * func(x)
>
> and the compiler can pre-calculate the output of a/c into a single
constant.

Not necessarily, floating point is not associative.

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

Re: [FFmpeg-devel] [PATCH] vp9_parser: don't overwrite cached timestamps with nopts.

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 6:39 AM, Ronald S. Bultje 
wrote:

> Hi,
>
> On Thu, Oct 29, 2015 at 12:45 AM, James Almer  wrote:
>
>> On 10/28/2015 11:16 PM, Ronald S. Bultje wrote:
>> > Hi,
>> >
>> > On Wed, Oct 28, 2015 at 5:51 PM, wm4  wrote:
>> >
>> >> On Wed, 28 Oct 2015 16:05:49 -0400
>> >> "Ronald S. Bultje"  wrote:
>> >>
>> >>> Hi,
>> >>>
>> >>> On Wed, Oct 28, 2015 at 3:44 PM, wm4  wrote:
>> >>>
>>  On Wed, 28 Oct 2015 12:21:05 -0400
>>  "Ronald S. Bultje"  wrote:
>> 
>> > ---
>> >  libavcodec/vp9_parser.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c
>> > index 0437097..6713850 100644
>> > --- a/libavcodec/vp9_parser.c
>> > +++ b/libavcodec/vp9_parser.c
>> > @@ -64,7 +64,7 @@ static int parse_frame(AVCodecParserContext *ctx,
>>  const uint8_t *buf, int size)
>> >  if (ctx->pts == AV_NOPTS_VALUE)
>> >  ctx->pts = s->pts;
>> >  s->pts = AV_NOPTS_VALUE;
>> > -} else {
>> > +} else if (ctx->pts != AV_NOPTS_VALUE) {
>> >  s->pts = ctx->pts;
>> >  ctx->pts = AV_NOPTS_VALUE;
>> >  }
>> 
>>  I find this a bit questionable. What is it needed for? Wouldn't it
>>  repeat the previous timestamp if new packets have none?
>> >>>
>> >>>
>> >>> I don't think so. Let's first go through the problem that I'm seeing,
>> >> maybe
>> >>> you see alternate solutions. Then I'll explain (very end) why I don't
>> >> think
>> >>> this happens.
>> >>>
>> >>> So, we're assuming here (this is generally true) that all input to the
>> >>> parser has timestamps (coming from ivf/webm), and that some frames are
>> >>> "superframes" which have one invisible (alt-ref) frame (only a
>> reference,
>> >>> not an actual frame that's ever displayed) packed with the following
>> >>> visible frame. So each packet in ivf/webm leads to one visible output
>> >>> frame, but in some cases this requires decoding of multiple
>> (invisible)
>> >>> references. For frame threading purposes, we split before we send it
>> to
>> >> the
>> >>> decoder.
>> >>>
>> >>> So what you get is:
>> >>> - ivf/webm file has packet of size a with timestamp b, calls parser
>> >>> - parser notices that packet is superframe with 2 frames in it
>> >>> - we output the first (invisible) frame with no timestamp, and cache
>> the
>> >>> timestamp of the input packet
>> >>> - utils.c code calls parser again with the same input data (we told
>> it we
>> >>> didn't consume any), but the (input) timestamp is now set to nopts
>> >>> - we output the second (visible) frame with the cached timestamp on
>> the
>> >>> packet
>> >>>
>> >>> This generally makes sense, the webm/ivf indeed assume that the
>> timestamp
>> >>> only is valid for the visible frame. Invisible frames have no
>> timestamp
>> >>> associated with them since they're never displayed.
>> >>>
>> >>> So, the above code has the issue: what if there's 2 invisible frames
>> >> packed
>> >>> in a superframe (followed by the visible frame)? Right now, this
>> happens:
>> >>> - ivf/webm file has packet of size a with timestamp b, calls parser
>> >>> - parser notices that packet is superframe with 3 frames in it
>> >>> - we output the first (invisible) frame with no timestamp, and cache
>> the
>> >>> timestamp of the input packet
>> >>> - utils.c code calls parser again with the same input data (we told
>> it we
>> >>> didn't consume any), but the (input) timestamp is now set to nopts
>> >>> - we output the second (invisible) frame with no timestamp, and cache
>> the
>> >>> timestamp of the input packet (which is now set to nopts)
>> >>> - utils.c code calls parser again with the same input data (we told
>> it we
>> >>> didn't consume any), but the (input) timestamp is now set to nopts
>> >>> - we output the third (visible) frame with the cached timestamp on the
>> >>> packet, which was nopts
>> >>>
>> >>> The last 3 are wrong; we should only cache timestamps if there is any
>> to
>> >> be
>> >>> cached, we should not override the cached timestamp with a new nopts
>> >> value,
>> >>> at least not in this particular case.
>> >>>
>> >>> --
>> >>> very end
>> >>> --
>> >>>
>> >>> Ok, so what about your point: can we output the same timestamp twice?
>> I
>> >>> don't think so, because as soon as we output the cached timestamp, we
>> >> reset
>> >>> the value of the cached timestamp to nopts, so if we were to reuse the
>> >>> cached timestamp, it would be nopts and the output data would have no
>> >>> timestamp.
>> >>>
>> >>> (Hope that wasn't too detailed.)
>> >>
>> >> Thanks for the explanations. I didn't know there could be more than 1
>> >> super frame, and I see how the new logic works and doesn't duplicate
>> >> timestamps.
>> >>
>> >> Patch looks good with me then.

Re: [FFmpeg-devel] [PATCH 02/11] avcodec/nellymoserenc: use log2 instead of log()/M_LN2

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:00AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/nellymoserenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thanks

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates


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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 5:43 AM, wm4  wrote:
> On Thu, 29 Oct 2015 00:19:59 -0400
> Ganesh Ajjanagadde  wrote:
>
>> This is more concise and conveys the intent better.
>> Furthermore, it is likely more precise as well due to lack of floating
>> point division.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>
> These patches are all pretty similar. And likely tedious to check for
> correctness. So low gain, while some potential for regressions.

In the time it took you to write that comment, you could have easily
reviewed a couple. log10 is already being used in the codebase, why
not make it consistent and also make it more precise?

There is a good reason why libc has a log10 function.

Maybe you don't care about such things, but I do: it is (roughly)
analogous to using double instead of float for filters/resampling etc
- noise floor should not be increased unless there is a clear benefit.
Here there is none from using log as opposed to log10.

>
> NACK from me.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/8] tools/crypto_bench: add AES-CBC modes

[Nicolas George]

L'octidi 8 brumaire, an CCXXIV, Rodger Combs a écrit :
> Take a look at some results; CBC is significantly slower than CTR in both
> libcrypto's and our AES code. There's some hardware limitation it hits.

Yes, that is to be expected. But I would expect something like this:

  time(AES-X-CBC) - time(AES-X-ECB) = time(AES-Y-CBC) - time(AES-Y-ECB)

for X and Y in { 128, 192, 256 }. In other words, I expect the extra time
added by CBC to be independent from the key size.

But of course, having a benchmark to check that it is true is good.

So patch still ok, thanks.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCHv2] all: fix enum definition for large values

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 12:29 AM, Michael Niedermayer
 wrote:
> On Wed, Oct 28, 2015 at 07:02:42PM -0400, Ganesh Ajjanagadde wrote:
>> On Wed, Oct 28, 2015 at 3:05 PM, Ronald S. Bultje  wrote:
>> > Hi,
>> >
>> > On Wed, Oct 28, 2015 at 2:46 PM, Ganesh Ajjanagadde 
>> > 
>> > wrote:
>> >>
>> >> On Wed, Oct 28, 2015 at 2:39 PM, Ronald S. Bultje 
>> >> wrote:
>> >> > Hi,
>> >> >
>> >> > On Wed, Oct 28, 2015 at 2:31 PM, Ganesh Ajjanagadde
>> >> > 
>> >> > wrote:
>> >> >>
>> >> >> On Wed, Oct 28, 2015 at 11:38 AM, Ronald S. Bultje 
>> >> >> wrote:
>> >> >> > Hi,
>> >> >> >
>> >> >> > On Wed, Oct 28, 2015 at 11:00 AM, Ganesh Ajjanagadde
>> >> >> >  wrote:
>> >> >> >>
>> >> >> >> On Wed, Oct 28, 2015 at 10:53 AM, Ronald S. Bultje
>> >> >> >> 
>> >> >> >> wrote:
>> >> >> >> > Hi,
>> >> >> >> >
>> >> >> >> > On Wed, Oct 28, 2015 at 10:48 AM, Ganesh Ajjanagadde
>> >> >> >> >  wrote:
>> >> >> >> >>
>> >> >> >> >> On Wed, Oct 28, 2015 at 10:34 AM, Ronald S. Bultje
>> >> >> >> >> 
>> >> >> >> >> wrote:
>> >> >> >> >> > Hi,
>> >> >> >> >> >
>> >> >> >> >> > On Wed, Oct 28, 2015 at 8:20 AM, Ganesh Ajjanagadde
>> >> >> >> >> > 
>> >> >> >> >> > wrote:
>> >> >> >> >> >>
>> >> >> >> >> >> On Wed, Oct 28, 2015 at 7:00 AM, Ronald S. Bultje
>> >> >> >> >> >> 
>> >> >> >> >> >> wrote:
>> >> >> >> >> >> > Hi,
>> >> >> >> >> >> >
>> >> >> >> >> >> > On Tue, Oct 27, 2015 at 10:58 PM, Ganesh Ajjanagadde
>> >> >> >> >> >> >  wrote:
>> >> >> >> >> >> >>
>> >> >> >> >> >> >> On Tue, Oct 27, 2015 at 10:41 PM, Ronald S. Bultje
>> >> >> >> >> >> >> 
>> >> >> >> >> >> >> wrote:
>> >> >> >> >> >> >> > Hi,
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> > On Tue, Oct 27, 2015 at 8:53 PM, Ganesh Ajjanagadde
>> >> >> >> >> >> >> > 
>> >> >> >> >> >> >> > wrote:
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> ISO C restricts enumerator values to the range of int.
>> >> >> >> >> >> >> >> Thus
>> >> >> >> >> >> >> >> (for
>> >> >> >> >> >> >> >> instance)
>> >> >> >> >> >> >> >> 0x8000
>> >> >> >> >> >> >> >> unfortunately does not work, and throws a warning with
>> >> >> >> >> >> >> >> -Wpedantic
>> >> >> >> >> >> >> >> on
>> >> >> >> >> >> >> >> clang 3.7.
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> This fixes it by using alternative expressions that
>> >> >> >> >> >> >> >> result
>> >> >> >> >> >> >> >> in
>> >> >> >> >> >> >> >> identical
>> >> >> >> >> >> >> >> values but do not have this issue.
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> Tested with FATE.
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> Signed-off-by: Ganesh Ajjanagadde
>> >> >> >> >> >> >> >> 
>> >> >> >> >> >> >> >> ---
>> >> >> >> >> >> >> >>  libavcodec/dca_syncwords.h | 26
>> >> >> >> >> >> >> >> --
>> >> >> >> >> >> >> >>  libavformat/cinedec.c  |  2 +-
>> >> >> >> >> >> >> >>  libavformat/mov_chan.c |  2 +-
>> >> >> >> >> >> >> >>  3 files changed, 14 insertions(+), 16 deletions(-)
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> diff --git a/libavcodec/dca_syncwords.h
>> >> >> >> >> >> >> >> b/libavcodec/dca_syncwords.h
>> >> >> >> >> >> >> >> index 3466b6b..6981cb8 100644
>> >> >> >> >> >> >> >> --- a/libavcodec/dca_syncwords.h
>> >> >> >> >> >> >> >> +++ b/libavcodec/dca_syncwords.h
>> >> >> >> >> >> >> >> @@ -19,19 +19,17 @@
>> >> >> >> >> >> >> >>  #ifndef AVCODEC_DCA_SYNCWORDS_H
>> >> >> >> >> >> >> >>  #define AVCODEC_DCA_SYNCWORDS_H
>> >> >> >> >> >> >> >>
>> >> >> >> >> >> >> >> -enum DCASyncwords {
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_BE= 0x7FFE8001U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_LE= 0xFE7F0180U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_14B_BE= 0x1FFFE800U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_CORE_14B_LE= 0xFF1F00E8U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_XCH= 0x5A5A5A5AU,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_XXCH   = 0x47004A03U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_X96= 0x1D95F262U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_XBR= 0x655E315EU,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_LBR= 0x0A801921U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_XLL= 0x41A29547U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_SUBSTREAM  = 0x64582025U,
>> >> >> >> >> >> >> >> -DCA_SYNCWORD_SUBSTREAM_CORE = 0x02B09261U,
>> >> >> >> >> >> >> >> -};
>> >> >> >> >> >> >> >> +#defineDCA_SYNCWORD_CORE_BE
>> >> >> >> >> >> >> >> 0x7FFE8001U
>> >> >> >> >> >> >> >> +#defineDCA_SYNCWORD_CORE_LE
>> >> >> >> >> >> >> >> 0xFE7F0180U
>> >> >> >> >> >> >> >> +#defineDCA_SYNCWORD_CORE_14B_BE
>> >> >> >> >> >> >> >> 0x1FFFE800U
>> >> >> >> >> >> 

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 8:32 AM, Ganesh Ajjanagadde 
wrote:

> On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer
>  >> wrote:
> >
> >> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
> >> > This is more concise and conveys the intent better.
> >> > Furthermore, it is likely more precise as well due to lack of floating
> >> > point division.
> >> >
> >> > Signed-off-by: Ganesh Ajjanagadde 
> >>
> >> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
> >> qemu-arm
> >> fate passes on all, i could test on actual arm hw if people think
> >> that is needed
> >
> >
> > I don't think that's needed.
> >
> > Is there some way we can confirm that each of these files that is changed
> > includes libavutil/libm.h for the compatibility macros in case they're
> > lacking on the target system?
>
> Don't know, but in case any were wondering: I assumed the availability
> simply because existing code already used log2, log10. This is also
> why even without the accuracy change, I personally consider the
> patchset an improvement due to it making FFmpeg more consistent in its
> choice of log function.


The risk is that some file in the dusty corners of our codebase forgets to
include libavutil/libm.h, the system does not have log2 or whatnot (which
is exactly why we have the compat wrappers in libavutil/libm.h - they are
not there because somebody felt like writing it just in case it happens -
they were written because it _did_ happen), and as a result it fails to
compile when a user on aix or msvc2010 or whatever affected system tries to
compile the next release.

That would suck, and users would think ffmpeg sucks as a result. So that's
why I'm wondering if we can confirm that all affected files do include
libavutil/libm.h, so that we can be sure there's nothing to worry about.

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

Re: [FFmpeg-devel] [PATCH] avfilter/asrc_sine: fix options typos

[Kyle Swanson]

On Wed, Oct 28, 2015 at 3:02 PM, Michael Niedermayer
 wrote:
> On Wed, Oct 28, 2015 at 07:10:24PM +0100, Nicolas George wrote:
>> Le septidi 7 brumaire, an CCXXIV, Kyle Swanson a écrit :
>> > Signed-off-by: Kyle Swanson 
>> > ---
>> >  libavfilter/asrc_sine.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Of course ok, thanks.
>
> applied
>
> thanks
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
>
> ___

This never got pushed. Can someone push it?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] Added QSV based VPP filter - second try

[Sven Dueking]

Hi all,

Please find attached my second attempt to add Intel´s VPP to FFMpeg. 
As requested, the patch includes a documentation file
(patch checker shows some warnings here, but I am not sure if they needs to
be fixed).

Again, looking forward to get feedback … and I am nervous ;).

Thanks,
Sven


0001-added-QSV-VPP-filter.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/tremolo: fix wavetable buffer size

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:48:05PM +0100, Paul B Mahol wrote:
> On 10/29/15, Kyle Swanson  wrote:
> > Signed-off-by: Kyle Swanson 
> > ---
> >  libavfilter/af_tremolo.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavfilter/af_tremolo.c b/libavfilter/af_tremolo.c
> > index 50df2e4..572e9e3 100644
> > --- a/libavfilter/af_tremolo.c
> > +++ b/libavfilter/af_tremolo.c
> > @@ -72,7 +72,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> >  dst += channels;
> >  src += channels;
> >  s->index++;
> > -if (s->index >= inlink->sample_rate)
> > +if (s->index >= inlink->sample_rate / s->freq)
> >  s->index = 0;
> >  }
> >
> > @@ -125,11 +125,11 @@ static int config_input(AVFilterLink *inlink)
> >  const double offset = 1. - s->depth / 2.;
> >  int i;
> >
> > -s->table = av_malloc_array(inlink->sample_rate, sizeof(*s->table));
> > +s->table = av_malloc_array(inlink->sample_rate / s->freq,
> > sizeof(*s->table));
> >  if (!s->table)
> >  return AVERROR(ENOMEM);
> >
> > -for (i = 0; i < inlink->sample_rate; i++) {
> > +for (i = 0; i < inlink->sample_rate / s->freq; i++) {
> >  double env = s->freq * i / inlink->sample_rate;
> >  env = sin(2 * M_PI * fmod(env + 0.25, 1.0));
> >  s->table[i] = env * (1 - fabs(offset)) + offset;
> > --
> > 2.6.2
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> probably ok

applied

thanks

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


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

Re: [FFmpeg-devel] [PATCH] Update Cookies on Setcookie playlist response

[Lucas Andrade]

Em ter, 27 de out de 2015 às 09:28, Lucas Andrade 
escreveu:

> Here is the patch to backport it to release/2.8. To be fixed on 2.8.2.
> Anything else I should do?
>
> ps. Sorry, that's my first time on opensource project, I'm a little
> confused that I needed to add it to the release branch.
>

Do I need to do anything else? It hasn't been added to release/2.8 branch
yet.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 09/11] avfilter/vf_psnr: use log10 instead of log()/log(10)

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:07AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/vf_psnr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

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

Democracy is the form of government in which you can choose your dictator


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

Re: [FFmpeg-devel] [PATCH 10/11] avfilter/vf_ssim: use log10 instead of log()/log(10)

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:08AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/vf_ssim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
> index ce1e3db..6b2a8d7 100644
> --- a/libavfilter/vf_ssim.c
> +++ b/libavfilter/vf_ssim.c
> @@ -176,7 +176,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
>  
>  static double ssim_db(double ssim, double weight)
>  {
> -return 10 * (log(weight) / log(10) - log(weight - ssim) / log(10));
> +return 10 * (log10(weight) - log10(weight - ssim));

LGTM

note, this can be simplified further but thats maybe off topic in
relation to switching to log10

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

You can kill me, but you cannot change the truth.


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

Re: [FFmpeg-devel] [PATCH] Added QSV based VPP filter - second try

[Carl Eugen Hoyos]

Sven Dueking  nablet.com> writes:

> Please find attached my second attempt to add Intel´s 
> VPP to FFMpeg. As requested, the patch includes a 
> documentation file

I don't know much about how the documentation works 
but why isn't this part of the filter documentation 
and how are users supposed to find it?

> +AV_PIX_FMT_RGB32,

The Intel documentation for RGB4 reads:
"ARGB in that order, A channel is 8 MSBs"
Making this AV_PIX_FMT_ARGB imo
But I'm probably wrong again...

> +if (inlink->format == AV_PIX_FMT_YUV420P)
> +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YV12;
> +else if (inlink->format == AV_PIX_FMT_YUV422P)
> +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YUY2;
> +else if (inlink->format == AV_PIX_FMT_NV12)
> +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_NV12;
> +else
> +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_RGB4;

Did you consider to add BGR4 and ARGB16 in the future?

> +unsigned int bits_per_pixel = get_bpp(vpp->pVppParam->vpp.In.FourCC);

See below.

> +width = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Width, 32);
> +height = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Height, 32);

Are the casts necessary or useful?

> +vpp->in_surface = av_mallocz(sizeof(mfxFrameSurface1) * 
> vpp->num_surfaces_in);

This looks wrong to me, I believe you want to allocate 
num_surfaces_in pointers, not structs. (Sorry if I miss 
somthing.)

> +return -1;

ENOMEM.

> +for (i = 0; i < vpp->num_surfaces_in; i++)
> +vpp->in_surface[i] = NULL;

This is unnecessary if you use mallocz() as you do.

> +for (i = 0; i < vpp->num_surfaces_in; i++) {
> +vpp->in_surface[i] = av_mallocz(sizeof(mfxFrameSurface1));
> +
> +if (!vpp->in_surface[i])
> +return -1;

ENOMEM but this leaks memory, you have to free 
everything else on failure.
(same below for out_surface)

> +bits_per_pixel = 12;

Imo, remove the variable and use get_bpp() once 
and "12" on the second usage.

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

Re: [FFmpeg-devel] [PATCH] avfilter/asrc_sine: fix options typos

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 10:29:39AM -0500, Kyle Swanson wrote:
> On Wed, Oct 28, 2015 at 3:02 PM, Michael Niedermayer
>  wrote:
> > On Wed, Oct 28, 2015 at 07:10:24PM +0100, Nicolas George wrote:
> >> Le septidi 7 brumaire, an CCXXIV, Kyle Swanson a écrit :
> >> > Signed-off-by: Kyle Swanson 
> >> > ---
> >> >  libavfilter/asrc_sine.c | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> Of course ok, thanks.
> >
> > applied
> >
> > thanks
> >
> > [...]
> >
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Complexity theory is the science of finding the exact solution to an
> > approximation. Benchmarking OTOH is finding an approximation of the exact
> >
> > ___
> 
> This never got pushed. Can someone push it?

done, sorry

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


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

Re: [FFmpeg-devel] [PATCH 10/11] avfilter/vf_ssim: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 1:37 PM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:08AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/vf_ssim.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
>> index ce1e3db..6b2a8d7 100644
>> --- a/libavfilter/vf_ssim.c
>> +++ b/libavfilter/vf_ssim.c
>> @@ -176,7 +176,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
>>
>>  static double ssim_db(double ssim, double weight)
>>  {
>> -return 10 * (log(weight) / log(10) - log(weight - ssim) / log(10));
>> +return 10 * (log10(weight) - log10(weight - ssim));
>
> LGTM
>
> note, this can be simplified further but thats maybe off topic in
> relation to switching to log10

I did note that you can rewrite as log10(weight/(weight-ssim)), but
avoided it deliberately as I did not know what people want with
respect to it. Since you brought it up and think it may be good, I
will change it.

I personally don't consider it too off topic, and prefer this over
sending a separate patch for the simplification and dealing with
another wm4 rant about it.
Will push all later, once everything is reviewed and ok'ed.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> You can kill me, but you cannot change the truth.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avformat: add DC STR demuxer

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 10:12:03PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/Makefile |  1 +
>  libavformat/allformats.c |  1 +
>  libavformat/dcstr.c  | 79 
> 
>  3 files changed, 81 insertions(+)
>  create mode 100644 libavformat/dcstr.c

LGTM

thanks

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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

Re: [FFmpeg-devel] [PATCH][RFC] avcodec: disallow hwaccel with frame threads

[Andy Furniss]

Hendrik Leppkes wrote:

On Thu, Oct 22, 2015 at 11:27 AM, Wang Bin 
wrote:

VLC is using frame threading with hwaccel


Then they should fix their usage, its broken by design, as explained
in detail my post earlier in this thread.


Seems ffmpeg cli does the same by default - OK so users can specify
-threads 1 if they use -hwaccel vdpau.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 10/11] avfilter/vf_ssim: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 2:33 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Oct 29, 2015 at 2:07 PM, Ganesh Ajjanagadde 
> wrote:
>
>> On Thu, Oct 29, 2015 at 1:37 PM, Michael Niedermayer
>>  wrote:
>> > On Thu, Oct 29, 2015 at 12:20:08AM -0400, Ganesh Ajjanagadde wrote:
>> >> This is likely more precise and conveys the intent better.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> ---
>> >>  libavfilter/vf_ssim.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
>> >> index ce1e3db..6b2a8d7 100644
>> >> --- a/libavfilter/vf_ssim.c
>> >> +++ b/libavfilter/vf_ssim.c
>> >> @@ -176,7 +176,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
>> >>
>> >>  static double ssim_db(double ssim, double weight)
>> >>  {
>> >> -return 10 * (log(weight) / log(10) - log(weight - ssim) / log(10));
>> >> +return 10 * (log10(weight) - log10(weight - ssim));
>> >
>> > LGTM
>> >
>> > note, this can be simplified further but thats maybe off topic in
>> > relation to switching to log10
>>
>> I did note that you can rewrite as log10(weight/(weight-ssim)), but
>> avoided it deliberately as I did not know what people want with
>> respect to it. Since you brought it up and think it may be good, I
>> will change it.
>>
>> I personally don't consider it too off topic, and prefer this over
>> sending a separate patch for the simplification and dealing with
>> another wm4 rant about it.
>> Will push all later, once everything is reviewed and ok'ed.
>
>
> Now, now, let's stay friendly and professional...

I referred to it as a "rant" because that is what it was, and I want
to make it sufficiently clear that the reason patches I send often
result in a ton of noise is often not technical, but simply a
knee-jerk reaction and associated bickering over it.

I do not enjoy dealing with such things, but I can almost surely
guarantee that is what would happen if I send a separate patch for it.
I (and most people here) want work to be done at minimal noise cost,
and hence I gave my rationale for changing without sending a separate
patch.

Your point is taken though, and I will attempt to refrain from such things.

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

Re: [FFmpeg-devel] [PATCH 1/2] avcodec: add ADPCM AICA decoder

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 10:12:02PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/Makefile |  1 +
>  libavcodec/adpcm.c  | 13 +
>  libavcodec/allcodecs.c  |  1 +
>  libavcodec/avcodec.h|  1 +
>  libavcodec/codec_desc.c |  7 +++
>  5 files changed, 23 insertions(+)

LGTM

thanks

[...]
-- 
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


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

Re: [FFmpeg-devel] [PATCH 10/11] avfilter/vf_ssim: use log10 instead of log()/log(10)

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 2:07 PM, Ganesh Ajjanagadde 
wrote:

> On Thu, Oct 29, 2015 at 1:37 PM, Michael Niedermayer
>  wrote:
> > On Thu, Oct 29, 2015 at 12:20:08AM -0400, Ganesh Ajjanagadde wrote:
> >> This is likely more precise and conveys the intent better.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavfilter/vf_ssim.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
> >> index ce1e3db..6b2a8d7 100644
> >> --- a/libavfilter/vf_ssim.c
> >> +++ b/libavfilter/vf_ssim.c
> >> @@ -176,7 +176,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
> >>
> >>  static double ssim_db(double ssim, double weight)
> >>  {
> >> -return 10 * (log(weight) / log(10) - log(weight - ssim) / log(10));
> >> +return 10 * (log10(weight) - log10(weight - ssim));
> >
> > LGTM
> >
> > note, this can be simplified further but thats maybe off topic in
> > relation to switching to log10
>
> I did note that you can rewrite as log10(weight/(weight-ssim)), but
> avoided it deliberately as I did not know what people want with
> respect to it. Since you brought it up and think it may be good, I
> will change it.
>
> I personally don't consider it too off topic, and prefer this over
> sending a separate patch for the simplification and dealing with
> another wm4 rant about it.
> Will push all later, once everything is reviewed and ok'ed.


Now, now, let's stay friendly and professional...

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

Re: [FFmpeg-devel] [PATCH] avutil/mathematics: make av_gcd more robust

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 10:48:14PM -0400, Ganesh Ajjanagadde wrote:
> This ensures that no undefined behavior is invoked, while retaining
> identical return values in all cases and at no loss of performance
> (identical asm on clang and gcc).
> Essentially, this patch exchanges undefined behavior with implementation
> defined behavior, a strict improvement.
> 
> Rationale:
> 1. The ideal solution is to have the return type a uint64_t. This
> unfortunately requires an API change.
> 2. The only pathological behavior happens if both arguments are
> INT64_MIN, to the best of my knowledge. In such a case, the
> implementation defined behavior is invoked in the sense that UINT64_MAX
> is interpreted as INT64_MIN, which any reasonable implementation will
> do. In any case, any usage where both arguments are INT64_MIN is a
> fuzzer anyway.
> 3. Alternatives of checking, etc require branching and lose performance
> for no concrete gain - no client cares about av_gcd's actual value when
> both args are INT64_MIN. Even if it did, on sane platforms (e.g all the
> ones FFmpeg cares about), it produces a correct gcd, namely INT64_MIN.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavutil/mathematics.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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

Re: [FFmpeg-devel] [PATCH] avutil/opencl_internal: add av_warn_unused_result

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 10:53:59PM -0400, Ganesh Ajjanagadde wrote:
> On Thu, Oct 15, 2015 at 6:24 PM, Ganesh Ajjanagadde
>  wrote:
> > Signed-off-by: Ganesh Ajjanagadde 
> > ---
> >  libavutil/opencl_internal.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/libavutil/opencl_internal.h b/libavutil/opencl_internal.h
> > index dacd930..a4f5885 100644
> > --- a/libavutil/opencl_internal.h
> > +++ b/libavutil/opencl_internal.h
> > @@ -30,4 +30,5 @@ typedef struct {
> >  void *ctx;
> >  } FFOpenclParam;
> >
> > +av_warn_unused_result
> >  int avpriv_opencl_set_parameter(FFOpenclParam *opencl_param, ...);
> > --
> > 2.6.1
> >
> 
> ping

probably ok

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

Re: [FFmpeg-devel] [PATCH] vf_lut: Add support for RGB48 and RGBA64

[Paul B Mahol]

On 10/25/15, Steven Robertson  wrote:
> Done, thanks.
>

probably ok

>
> On Tue, Oct 13, 2015 at 12:51 AM, Paul B Mahol  wrote:
>
>> On 10/11/15, Steven Robertson  wrote:
>> > Thanks for taking a look!
>> >
>> > Steve
>> >
>>
>> lgtm, do you need to update fate?
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/mathematics: correct documentation for av_gcd

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 10:49:08PM -0400, Ganesh Ajjanagadde wrote:
> On Wed, Oct 28, 2015 at 10:09 AM, Ganesh Ajjanagadde
>  wrote:
> > On Tue, Oct 27, 2015 at 8:18 PM, Ganesh Ajjanagadde
> >  wrote:
> >> av_gcd is now always defined regardless of input. This documents this
> >> change in the "documented API". Two benefits (closely related):
> >> 1. The function is robust, and there is no need to worry about INT64_MIN, 
> >> etc.
> >>
> >> 2. Clients of av_gcd, like av_reduce, can now be made fully correct. 
> >> Currently,
> >> av_reduce can trigger undefined behavior if e.g num is INT64_MIN due to
> >> integer overflow in the FFABS. Furthermore, this undefined behavior is
> >> completely undocumented, and could be a fuzzer's paradise. The FFABS was 
> >> needed in the past as
> >> av_gcd was undefined for negative inputs. In order to make av_reduce
> >> robust, it is essential to guarantee that av_gcd works for all int64_t.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavutil/mathematics.h | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavutil/mathematics.h b/libavutil/mathematics.h
> >> index ac94488..6fc2577 100644
> >> --- a/libavutil/mathematics.h
> >> +++ b/libavutil/mathematics.h
> >> @@ -77,9 +77,9 @@ enum AVRounding {
> >>  };
> >>
> >>  /**
> >> - * Return the greatest common divisor of a and b.
> >> - * If both a and b are 0 or either or both are <0 then behavior is
> >> - * undefined.
> >> + * Compute the greatest common divisor of a and b.
> >> + *
> >> + * @return gcd of a and b up to sign; if a and b are both zero returns 0
> >>   */
> >>  int64_t av_const av_gcd(int64_t a, int64_t b);
> >>
> >> --
> >> 2.6.2
> >>
> >
> > Patch dropped for now, undefined behavior is still possible with
> > av_gcd: take a and b to be both INT64_MIN. Need to examine this a
> > little more closely to make it robust without losing performance.
> 
> Request to reconsider; patch making av_gcd more robust sent.

I guess if the stricter API doesnt prevent any possibly optimizations
then the change is a good idea

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH] avutil/parseutils: add av_warn_unused_result

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 11:04:24PM -0400, Ganesh Ajjanagadde wrote:
> On Thu, Oct 15, 2015 at 10:24 PM, Ganesh Ajjanagadde  wrote:
> > On Thu, Oct 15, 2015 at 10:22 PM, Ganesh Ajjanagadde  
> > wrote:
> >> On Thu, Oct 15, 2015 at 10:16 PM, James Almer  wrote:
> >>> On 10/15/2015 10:23 PM, Ganesh Ajjanagadde wrote:
>  On Thu, Oct 15, 2015 at 8:06 PM, James Almer  wrote:
> > On 10/15/2015 8:52 PM, Ganesh Ajjanagadde wrote:
> >> This triggers a few warnings that will need to be fixed - not that bad,
> >> the current code is mostly fine.
> >>
> >
> > I think i asked you before, but in any case i'll do it again. Please,
> > send patchsets contained in a single thread. You're making things
> > hard to track and organize on people's inboxes.
> > See https://kernel.org/pub/software/scm/git/docs/git-send-email.html
> > --thread, --chain-reply-to and --in-reply-to options.
> 
>  Would greatly appreciate a command/command sequence: I am on master
>  (which is ahead of origin/master) and contains only the avutil warning
>  stuff (many commits). How do I get a proper chain of emails that you
>  would like?
> 
> >
> > Thanks.
> >>>
> >>> When you send several mails with a single git send-email command, it
> >>> should by default send every email after the first as a reply to the
> >>> first, effectively making a single "thread".
> >>
> >> Sorry, but gmail smtp/send-email does not let me do this - I have
> >> tried multiple patches in a single git send-email, and it closes the
> >> connection.
> >
> > The engineers at Google must have fixed something - I tried it just
> > now in the manner you described for the avdevice patches.
> 
> ping

what patch is this ping about ?
i assume its not for the paragraph directly above

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


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

Re: [FFmpeg-devel] [PATCH] avutil/pixdesc: add av_warn_unused_result to av_pix_fmt_get_chroma_sub_sample

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 6:13 PM, Michael Niedermayer
 wrote:
> On Wed, Oct 28, 2015 at 11:03:33PM -0400, Ganesh Ajjanagadde wrote:
>> On Fri, Oct 16, 2015 at 5:17 PM, Ganesh Ajjanagadde  wrote:
>> > On Fri, Oct 16, 2015 at 11:39 AM, Michael Niedermayer
>> >  wrote:
>> >> On Thu, Oct 15, 2015 at 07:20:30PM -0400, Ganesh Ajjanagadde wrote:
>> >>> This will trigger a bunch of warnings (rightfully so). This API has been
>> >>> abused, see the Doxygen comment above for what to do if the error code
>> >>> is not meant to be checked.
>> >>>
>> >>> Signed-off-by: Ganesh Ajjanagadde 
>> >>> ---
>> >>>  libavutil/pixdesc.h | 1 +
>> >>>  1 file changed, 1 insertion(+)
>> >>>
>> >>> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
>> >>> index a6056fe..975fbf2 100644
>> >>> --- a/libavutil/pixdesc.h
>> >>> +++ b/libavutil/pixdesc.h
>> >>> @@ -297,6 +297,7 @@ enum AVPixelFormat av_pix_fmt_desc_get_id(const 
>> >>> AVPixFmtDescriptor *desc);
>> >>>   *
>> >>>   * @return 0 on success, AVERROR(ENOSYS) on invalid or unknown pixel 
>> >>> format
>> >>>   */
>> >>> +av_warn_unused_result
>> >>>  int av_pix_fmt_get_chroma_sub_sample(enum AVPixelFormat pix_fmt,
>> >>>   int *h_shift, int *v_shift);
>> >>
>> >> if the caller knows the pixel format is valid (and non hw accel) then
>> >> there is no need to check the return code
>> >> in many cases this is true, like a decoder that set the pix_fmt itself
>> >> it can be sure all possible values are valid
>> >
>> > See the Doxygen right above (this is why I pointed to it in the commit
>> > message) - you are right, but for such "guaranteed safety" usage there
>> > is a different API in avcodec. It seems to have been created precisely
>> > for this purpose.
>>
>> ping; you don't find the above reasoning satisfying?
>
> avcodec_get_chroma_sub_sample() is part of libavcodec,
> av_pix_fmt_get_chroma_sub_sample is part of libavutil
>
> any code not depending on libavcodec cannot use
> avcodec_get_chroma_sub_sample(), an example might be filters in
> libavfilter which dont otherwise depend on libavcodec

Thanks for the explanation, patch dropped.

>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Freedom in capitalist society always remains about the same as it was in
> ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 05/11] avfilter/af_volume: use log10 instead of log()/M_LN10

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 5:45 PM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:03AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/af_volume.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> LGTM
>
> thx

pushed, thanks.
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: stop exporting ffurl_read_complete, ffurl_seek and ffurl_size

[Andreas Cadhalpun]

On 27.10.2015 22:52, Andreas Cadhalpun wrote:
> On 27.10.2015 22:04, Hendrik Leppkes wrote:
>> On Tue, Oct 27, 2015 at 9:58 PM, Andreas Cadhalpun
>>  wrote:
>>> They are not in public headers and not used outside of libavformat.
>>>
>>> Signed-off-by: Andreas Cadhalpun 
>>> ---
>>>
>>> It's been a year since the last attempt to remove these [1].
>>> Bad enough that ffserver still uses private functions, but keeping even
>>> more in the ABI for some unnamed third party applications is just wrong.
>>>
>>> 1: https://ffmpeg.org/pipermail/ffmpeg-devel/2014-August/160951.html
>>>
>>> ---
>>>  libavformat/libavformat.v | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/libavformat/libavformat.v b/libavformat/libavformat.v
>>> index e90aef7..a00a309 100644
>>> --- a/libavformat/libavformat.v
>>> +++ b/libavformat/libavformat.v
>>> @@ -10,9 +10,6 @@ LIBAVFORMAT_$MAJOR {
>>>  ffio_set_buf_size;
>>>  ffurl_close;
>>>  ffurl_open;
>>> -ffurl_read_complete;
>>> -ffurl_seek;
>>> -ffurl_size;
>>>  ffurl_write;
>>>  #those are deprecated, remove on next bump
>>>  url_feof;
>>> --
>>> 2.6.1
>>>
>>
>> Fine with me.
> 
> OK. I'll wait a bit to see if someone else wants to comment on this patch.

Seems not the case, so pushed now.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 2/3] swscale: rename sws_context_class to ff_sws_context_class

[Andreas Cadhalpun]

On 28.10.2015 00:21, Michael Niedermayer wrote:
> On Tue, Oct 27, 2015 at 11:10:58PM +0100, Andreas Cadhalpun wrote:
>> It is an internal swscale symbol and thus should not be exported.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libswscale/options.c  | 4 ++--
>>  libswscale/swscale_internal.h | 2 +-
>>  libswscale/utils.c| 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 3/3] swscale: rename sws_rgb2rgb_init to ff_sws_rgb2rgb_init

[Andreas Cadhalpun]

On 28.10.2015 00:22, Michael Niedermayer wrote:
> On Tue, Oct 27, 2015 at 11:11:09PM +0100, Andreas Cadhalpun wrote:
>> It is an internal swscale function and thus should not be exported.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libswscale/colorspace-test.c | 2 +-
>>  libswscale/rgb2rgb.c | 2 +-
>>  libswscale/rgb2rgb.h | 2 +-
>>  libswscale/utils.c   | 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> probably ok

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 09/11] avfilter/vf_psnr: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 1:34 PM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:07AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/vf_psnr.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> LGTM
>
> thx

pushed, thanks.
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Democracy is the form of government in which you can choose your dictator
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/pixdesc: add av_warn_unused_result to av_pix_fmt_get_chroma_sub_sample

[Michael Niedermayer]

On Wed, Oct 28, 2015 at 11:03:33PM -0400, Ganesh Ajjanagadde wrote:
> On Fri, Oct 16, 2015 at 5:17 PM, Ganesh Ajjanagadde  wrote:
> > On Fri, Oct 16, 2015 at 11:39 AM, Michael Niedermayer
> >  wrote:
> >> On Thu, Oct 15, 2015 at 07:20:30PM -0400, Ganesh Ajjanagadde wrote:
> >>> This will trigger a bunch of warnings (rightfully so). This API has been
> >>> abused, see the Doxygen comment above for what to do if the error code
> >>> is not meant to be checked.
> >>>
> >>> Signed-off-by: Ganesh Ajjanagadde 
> >>> ---
> >>>  libavutil/pixdesc.h | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
> >>> index a6056fe..975fbf2 100644
> >>> --- a/libavutil/pixdesc.h
> >>> +++ b/libavutil/pixdesc.h
> >>> @@ -297,6 +297,7 @@ enum AVPixelFormat av_pix_fmt_desc_get_id(const 
> >>> AVPixFmtDescriptor *desc);
> >>>   *
> >>>   * @return 0 on success, AVERROR(ENOSYS) on invalid or unknown pixel 
> >>> format
> >>>   */
> >>> +av_warn_unused_result
> >>>  int av_pix_fmt_get_chroma_sub_sample(enum AVPixelFormat pix_fmt,
> >>>   int *h_shift, int *v_shift);
> >>
> >> if the caller knows the pixel format is valid (and non hw accel) then
> >> there is no need to check the return code
> >> in many cases this is true, like a decoder that set the pix_fmt itself
> >> it can be sure all possible values are valid
> >
> > See the Doxygen right above (this is why I pointed to it in the commit
> > message) - you are right, but for such "guaranteed safety" usage there
> > is a different API in avcodec. It seems to have been created precisely
> > for this purpose.
> 
> ping; you don't find the above reasoning satisfying?

avcodec_get_chroma_sub_sample() is part of libavcodec,
av_pix_fmt_get_chroma_sub_sample is part of libavutil

any code not depending on libavcodec cannot use
avcodec_get_chroma_sub_sample(), an example might be filters in
libavfilter which dont otherwise depend on libavcodec

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH 03/11] avcodec/snowenc: use log2 instead of log() / log(2...)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:20 AM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:01AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>> The expression has also been accordingly simplified.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/snowenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
>> index 7e8269c..fb0cd3f 100644
>> --- a/libavcodec/snowenc.c
>> +++ b/libavcodec/snowenc.c
>> @@ -1547,7 +1547,7 @@ static void calculate_visual_weight(SnowContext *s, 
>> Plane *p){
>>  }
>>  }
>>
>> -b->qlog= (int)(log(352256.0/sqrt(error)) / log(pow(2.0, 
>> 1.0/QROOT))+0.5);
>> +b->qlog= (int)(QROOT * log2(352256.0/sqrt(error)) + 0.5);
>
> LGTM
>
> thanks

pushed, thanks.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 10/11] avfilter/vf_ssim: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 2:07 PM, Ganesh Ajjanagadde  wrote:
> On Thu, Oct 29, 2015 at 1:37 PM, Michael Niedermayer
>  wrote:
>> On Thu, Oct 29, 2015 at 12:20:08AM -0400, Ganesh Ajjanagadde wrote:
>>> This is likely more precise and conveys the intent better.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde 
>>> ---
>>>  libavfilter/vf_ssim.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
>>> index ce1e3db..6b2a8d7 100644
>>> --- a/libavfilter/vf_ssim.c
>>> +++ b/libavfilter/vf_ssim.c
>>> @@ -176,7 +176,7 @@ static float ssim_plane(SSIMDSPContext *dsp,
>>>
>>>  static double ssim_db(double ssim, double weight)
>>>  {
>>> -return 10 * (log(weight) / log(10) - log(weight - ssim) / log(10));
>>> +return 10 * (log10(weight) - log10(weight - ssim));
>>
>> LGTM
>>
>> note, this can be simplified further but thats maybe off topic in
>> relation to switching to log10
>
> I did note that you can rewrite as log10(weight/(weight-ssim)), but
> avoided it deliberately as I did not know what people want with
> respect to it. Since you brought it up and think it may be good, I
> will change it.
>
> I personally don't consider it too off topic, and prefer this over
> sending a separate patch for the simplification and dealing with
> another wm4 rant about it.
> Will push all later, once everything is reviewed and ok'ed.

pushed with the further simplification, thanks.

>
>>
>> [...]
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> You can kill me, but you cannot change the truth.
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 05/11] avfilter/af_volume: use log10 instead of log()/M_LN10

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:03AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/af_volume.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH 07/11] avfilter/avf_showspectrum: use log10 instead of log()/...

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 12:20:05AM -0400, Ganesh Ajjanagadde wrote:
> This is likely more precise and conveys the intent better.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/avf_showspectrum.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/avf_showspectrum.c b/libavfilter/avf_showspectrum.c
> index 936db60..7d5c438 100644
> --- a/libavfilter/avf_showspectrum.c
> +++ b/libavfilter/avf_showspectrum.c
> @@ -386,7 +386,7 @@ static int plot_spectrum_column(AVFilterLink *inlink, 
> AVFrame *insamples)
>  a = cbrt(a);
>  break;
>  case LOG:
> -a = 1 - log(FFMAX(FFMIN(1, a), 1e-6)) / log(1e-6); // zero = 
> -120dBFS
> +a = 1 + log10(FFMAX(FFMIN(1, a), 1e-6)) / 6; // zero = 
> -120dBFS

LGTM

thx

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH 1/2] swscale/swscale_internal: add av_warn_unused_result

[Michael Niedermayer]

On Thu, Oct 15, 2015 at 09:46:08PM -0400, Ganesh Ajjanagadde wrote:
> This triggers a few warnings that need to be fixed.

do you have a list of warnings that adds ?
Iam asking as it would make it easier to judge if they are all issues
that need fixing or if any are valid cases that do not need a return
value check

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

What does censorship reveal? It reveals fear. -- Julian Assange


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

Re: [FFmpeg-devel] [PATCH 03/11] avdevice/sndio: add av_warn_unused_result

[Michael Niedermayer]

Hi Brad

do you have a oppinion on the change below ?
you are the author of the code


On Thu, Oct 15, 2015 at 10:22:17PM -0400, Ganesh Ajjanagadde wrote:
> This does not trigger any warnings, but adds robustness.
> Untested, as my configure does not compile this file.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavdevice/sndio.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavdevice/sndio.h b/libavdevice/sndio.h
> index 54a5ec3..6571429 100644
> --- a/libavdevice/sndio.h
> +++ b/libavdevice/sndio.h
> @@ -42,6 +42,7 @@ typedef struct SndioData {
>  int sample_rate;
>  } SndioData;
>  
> +av_warn_unused_result
>  int ff_sndio_open(AVFormatContext *s1, int is_output, const char 
> *audio_device);
>  int ff_sndio_close(SndioData *s);
>  
> -- 
> 2.6.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange


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

Re: [FFmpeg-devel] [PATCH] avutil/parseutils: add av_warn_unused_result

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 7:52 PM, Ganesh Ajjanagadde
 wrote:
> This triggers a few warnings that will need to be fixed - not that bad,
> the current code is mostly fine.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavutil/parseutils.h | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/libavutil/parseutils.h b/libavutil/parseutils.h
> index e66d24b..46de518 100644
> --- a/libavutil/parseutils.h
> +++ b/libavutil/parseutils.h
> @@ -46,6 +46,7 @@
>   * @param[in] log_ctx parent logging context
>   * @return >= 0 on success, a negative error code otherwise
>   */
> +av_warn_unused_result
>  int av_parse_ratio(AVRational *q, const char *str, int max,
> int log_offset, void *log_ctx);
>
> @@ -63,6 +64,7 @@ int av_parse_ratio(AVRational *q, const char *str, int max,
>   * width x height or a valid video size abbreviation.
>   * @return >= 0 on success, a negative error code otherwise
>   */
> +av_warn_unused_result
>  int av_parse_video_size(int *width_ptr, int *height_ptr, const char *str);
>
>  /**
> @@ -74,6 +76,7 @@ int av_parse_video_size(int *width_ptr, int *height_ptr, 
> const char *str);
>   * rate_num / rate_den, a float number or a valid video rate abbreviation
>   * @return >= 0 on success, a negative error code otherwise
>   */
> +av_warn_unused_result
>  int av_parse_video_rate(AVRational *rate, const char *str);
>
>  /**
> @@ -95,6 +98,7 @@ int av_parse_video_rate(AVRational *rate, const char *str);
>   * @return >= 0 in case of success, a negative value in case of
>   * failure (for example if color_string cannot be parsed).
>   */
> +av_warn_unused_result
>  int av_parse_color(uint8_t *rgba_color, const char *color_string, int slen,
> void *log_ctx);
>
> @@ -143,6 +147,7 @@ const char *av_get_known_color_name(int color_idx, const 
> uint8_t **rgb);
>   * @return >= 0 in case of success, a negative value corresponding to an
>   * AVERROR code otherwise
>   */
> +av_warn_unused_result
>  int av_parse_time(int64_t *timeval, const char *timestr, int duration);
>
>  /**
> --
> 2.6.1
>

Pinged the wrong mail, here is the patch. Sorry.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 04/11] avcodec/zmbvenc: use log2 instead of log()/M_LN2

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 9:25 AM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:02AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/zmbvenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c
>> index df06e37..e832bed 100644
>> --- a/libavcodec/zmbvenc.c
>> +++ b/libavcodec/zmbvenc.c
>> @@ -277,7 +277,7 @@ static av_cold int encode_init(AVCodecContext *avctx)
>>  int lvl = 9;
>>
>>  for(i=1; i<256; i++)
>> -score_tab[i]= -i * log(i/(double)(ZMBV_BLOCK*ZMBV_BLOCK)) * 
>> (256/M_LN2);
>> +score_tab[i]= -i * log2(i/(double)(ZMBV_BLOCK*ZMBV_BLOCK)) * 256;
>
> Interrestingly this changes the output from
> ffmpeg -i matrixbench_mpeg2.mpg -vcodec zmbv -frames 30  -an -f framecrc -
> for a few frames

not too surprising, there are bit changes for many inputs when one
moves from log/ to log2 etc. See e.g my reply to wm4's original
comments.

>
> the code change looks good though

pushed, thanks.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Rewriting code that is poorly written but fully understood is good.
> Rewriting code that one doesnt understand is a sign that one is less smart
> then the original author, trying to rewrite it will not make it better.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 07/11] avfilter/avf_showspectrum: use log10 instead of log()/...

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 5:47 PM, Michael Niedermayer
 wrote:
> On Thu, Oct 29, 2015 at 12:20:05AM -0400, Ganesh Ajjanagadde wrote:
>> This is likely more precise and conveys the intent better.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/avf_showspectrum.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/avf_showspectrum.c b/libavfilter/avf_showspectrum.c
>> index 936db60..7d5c438 100644
>> --- a/libavfilter/avf_showspectrum.c
>> +++ b/libavfilter/avf_showspectrum.c
>> @@ -386,7 +386,7 @@ static int plot_spectrum_column(AVFilterLink *inlink, 
>> AVFrame *insamples)
>>  a = cbrt(a);
>>  break;
>>  case LOG:
>> -a = 1 - log(FFMAX(FFMIN(1, a), 1e-6)) / log(1e-6); // zero 
>> = -120dBFS
>> +a = 1 + log10(FFMAX(FFMIN(1, a), 1e-6)) / 6; // zero = 
>> -120dBFS
>
> LGTM
>
> thx

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/cdg: Add transparency

[Carl Eugen Hoyos]

Michael Niedermayer  niedermayer.cc> writes:

> >  libavcodec/cdgraphics.c   |8 
> >  tests/fate/video.mak  |2 
> >  tests/ref/fate/cdgraphics |  424 

> i can confirm that the outputted alpha looks correct after the patch

Patch applied.

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[wm4]

On Thu, 29 Oct 2015 20:05:28 -0400
Ganesh Ajjanagadde  wrote:

> More generally, how is this problem "easy to verify"? It may be
> included indirectly, etc. Since you seem to think it is easy, go ahead

Indirect inclusion is IMHO not fine for such compat headers which
define standard functions on systems where they're missing. And direct
inclusion is easy to verify.

Nobody expects that you think of everything, but here you ignored a
direct request from a reviewer.

> 
> If FFmpeg waited until verification on every single config was done,
> we would be nowhere. You may think it is not cool, I could say the
> same about many things you have posted on this mailing list as well.

Strange that you're so awfully pedantic about C standard conformance
(so that we need dozens of patches to fix what doesn't need to be
fixed), but when it gets annoying for you, suddenly pushing and waiting
until it breaks is fine. How does this even make sense?

> If Michael thought this was not cool, I will immediately take action.

Hendrik's voice counts as much as Michael's.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/proresdec2: Fix slice_count for very high resolutions

[Carl Eugen Hoyos]

Michael Niedermayer  niedermayer.cc> writes:

> > +// QT ignores the written value
> > +// slice_count = AV_RB16(buf + 5);
> > +slice_count = ctx->mb_height * 
> > ((ctx->mb_width >> log2_slice_mb_width) +
> > +
> > av_popcount(ctx->mb_width & (1 << log2_slice_mb_width) - 1));
> 
> if thats what QT does then sure LGTM

Patch applied.

Thank you, Carl Eugen

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:13 PM, Hendrik Leppkes  wrote:
> On Fri, Oct 30, 2015 at 1:05 AM, Ganesh Ajjanagadde  wrote:
>> On Thu, Oct 29, 2015 at 7:50 PM, Hendrik Leppkes  wrote:
>>> On Fri, Oct 30, 2015 at 12:08 AM, Ganesh Ajjanagadde  
>>> wrote:
 On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  
 wrote:
> Hi,
>
> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer 
> > wrote:
>
>> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
>> > This is more concise and conveys the intent better.
>> > Furthermore, it is likely more precise as well due to lack of floating
>> > point division.
>> >
>> > Signed-off-by: Ganesh Ajjanagadde 
>>
>> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
>> qemu-arm
>> fate passes on all, i could test on actual arm hw if people think
>> that is needed
>
>
> I don't think that's needed.
>
> Is there some way we can confirm that each of these files that is changed
> includes libavutil/libm.h for the compatibility macros in case they're
> lacking on the target system?

 pushed as is; we have until the next release or a FATE failure to
 check the build on such obscure configurations. I will try to keep an
 eye out, but please report if you find failures.

>>>
>>> This is really not the right way to go. You have been notified of a
>>> real-world portability concern with an easy fix (verify that all files
>>> you use log2/etc in include libavutil/libm.h), and you decided to wait
>>> until it blows up instead?
>>> Not cool.
>>
>> Ok. Do a grep on ffmpeg.c: line 56 answers your point.
>>
>> Other patches have been acked by Michael, and were pushed.
>>
>> More generally, how is this problem "easy to verify"? It may be
>> included indirectly, etc. Since you seem to think it is easy, go ahead
>> and do whatever you want. Or enlighten mortals like me who fail to see
>> it is easy with your wisdom. Or if it is "easy", why don't you do it
>> for the commits that I just pushed.
>>
>> If FFmpeg waited until verification on every single config was done,
>> we would be nowhere. You may think it is not cool, I could say the
>> same about many things you have posted on this mailing list as well.
>>
>> If Michael thought this was not cool, I will immediately take action.
>>
>
> Its not about this patch especially, but all in the series, to which
> Ronald's concerns apply equally.
> If you get a review which voices concerns, these concerns need to be
> addressed. Either by fixing the patch, or by convincing the reviewer
> that no action is needed.

Sometimes it is not possible to convince a reviewer, even when the
evidence has been laid out and is right in front of them. I still
recall your concerns with fabs/FFABS, where you simply did not look at
my random number benchmark, or Clement's asm, or the libc link. Even
if you did, your email did not reflect that. Furthermore, you did not
post saying that your objection had been withdrawn. I can quote
relevant links here, but doubt it is necessary. How can I guess your
mind? What do you propose me to do then?

Please note that this is not with respect to the patch series above,
but is a general comment.

> And one positive review does not overrule a
> negative one - not everyone knows all possible concerns equally well.
>
> Pushing without addressing concerns is not OK at all.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:31 PM, wm4  wrote:
> On Thu, 29 Oct 2015 20:05:28 -0400
> Ganesh Ajjanagadde  wrote:
>
>> More generally, how is this problem "easy to verify"? It may be
>> included indirectly, etc. Since you seem to think it is easy, go ahead
>
> Indirect inclusion is IMHO not fine for such compat headers which
> define standard functions on systems where they're missing. And direct
> inclusion is easy to verify.
>
> Nobody expects that you think of everything, but here you ignored a
> direct request from a reviewer.
>
>>
>> If FFmpeg waited until verification on every single config was done,
>> we would be nowhere. You may think it is not cool, I could say the
>> same about many things you have posted on this mailing list as well.
>
> Strange that you're so awfully pedantic about C standard conformance
> (so that we need dozens of patches to fix what doesn't need to be
> fixed), but when it gets annoying for you, suddenly pushing and waiting
> until it breaks is fine. How does this even make sense?

I assume an LGTM implied that the reviewer (in this case Michael)
checked this issue. Seems like a misunderstanding.

I did not push because it got "annoying" for me. I pushed because I
got an ack from Michael. You may think whatever you want about it, e.g
if you don't believe me, I am not going to try to convince you.

>
>> If Michael thought this was not cool, I will immediately take action.
>
> Hendrik's voice counts as much as Michael's.

I referred to Michael as he was the reviewer for the patches.

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

Re: [FFmpeg-devel] [PATCH] avutil/mathematics: make av_gcd more robust

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 4:46 PM, Michael Niedermayer
 wrote:
> On Wed, Oct 28, 2015 at 10:48:14PM -0400, Ganesh Ajjanagadde wrote:
>> This ensures that no undefined behavior is invoked, while retaining
>> identical return values in all cases and at no loss of performance
>> (identical asm on clang and gcc).
>> Essentially, this patch exchanges undefined behavior with implementation
>> defined behavior, a strict improvement.
>>
>> Rationale:
>> 1. The ideal solution is to have the return type a uint64_t. This
>> unfortunately requires an API change.
>> 2. The only pathological behavior happens if both arguments are
>> INT64_MIN, to the best of my knowledge. In such a case, the
>> implementation defined behavior is invoked in the sense that UINT64_MAX
>> is interpreted as INT64_MIN, which any reasonable implementation will
>> do. In any case, any usage where both arguments are INT64_MIN is a
>> fuzzer anyway.
>> 3. Alternatives of checking, etc require branching and lose performance
>> for no concrete gain - no client cares about av_gcd's actual value when
>> both args are INT64_MIN. Even if it did, on sane platforms (e.g all the
>> ones FFmpeg cares about), it produces a correct gcd, namely INT64_MIN.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavutil/mathematics.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> LGTM

pushed, thanks.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Hendrik Leppkes]

On Fri, Oct 30, 2015 at 1:05 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Oct 29, 2015 at 7:50 PM, Hendrik Leppkes  wrote:
>> On Fri, Oct 30, 2015 at 12:08 AM, Ganesh Ajjanagadde  
>> wrote:
>>> On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  
>>> wrote:
 Hi,

 On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer 
  wrote:

> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
> > This is more concise and conveys the intent better.
> > Furthermore, it is likely more precise as well due to lack of floating
> > point division.
> >
> > Signed-off-by: Ganesh Ajjanagadde 
>
> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
> qemu-arm
> fate passes on all, i could test on actual arm hw if people think
> that is needed


 I don't think that's needed.

 Is there some way we can confirm that each of these files that is changed
 includes libavutil/libm.h for the compatibility macros in case they're
 lacking on the target system?
>>>
>>> pushed as is; we have until the next release or a FATE failure to
>>> check the build on such obscure configurations. I will try to keep an
>>> eye out, but please report if you find failures.
>>>
>>
>> This is really not the right way to go. You have been notified of a
>> real-world portability concern with an easy fix (verify that all files
>> you use log2/etc in include libavutil/libm.h), and you decided to wait
>> until it blows up instead?
>> Not cool.
>
> Ok. Do a grep on ffmpeg.c: line 56 answers your point.
>
> Other patches have been acked by Michael, and were pushed.
>
> More generally, how is this problem "easy to verify"? It may be
> included indirectly, etc. Since you seem to think it is easy, go ahead
> and do whatever you want. Or enlighten mortals like me who fail to see
> it is easy with your wisdom. Or if it is "easy", why don't you do it
> for the commits that I just pushed.
>
> If FFmpeg waited until verification on every single config was done,
> we would be nowhere. You may think it is not cool, I could say the
> same about many things you have posted on this mailing list as well.
>
> If Michael thought this was not cool, I will immediately take action.
>

Its not about this patch especially, but all in the series, to which
Ronald's concerns apply equally.
If you get a review which voices concerns, these concerns need to be
addressed. Either by fixing the patch, or by convincing the reviewer
that no action is needed. And one positive review does not overrule a
negative one - not everyone knows all possible concerns equally well.

Pushing without addressing concerns is not OK at all.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer > wrote:
>
>> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
>> > This is more concise and conveys the intent better.
>> > Furthermore, it is likely more precise as well due to lack of floating
>> > point division.
>> >
>> > Signed-off-by: Ganesh Ajjanagadde 
>>
>> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
>> qemu-arm
>> fate passes on all, i could test on actual arm hw if people think
>> that is needed
>
>
> I don't think that's needed.
>
> Is there some way we can confirm that each of these files that is changed
> includes libavutil/libm.h for the compatibility macros in case they're
> lacking on the target system?

pushed as is; we have until the next release or a FATE failure to
check the build on such obscure configurations. I will try to keep an
eye out, but please report if you find failures.

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

Re: [FFmpeg-devel] [PATCH 08/11] avfilter/avf_showvolume: use log10 instead of log()/M_LN10

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 9:35 AM, Ganesh Ajjanagadde
 wrote:
>
> On Oct 29, 2015 9:26 AM, "Ronald S. Bultje"  wrote:
>>
>> Hi,
>>
>> On Thu, Oct 29, 2015 at 8:47 AM, Ganesh Ajjanagadde 
>> wrote:
>>
>> > On Thu, Oct 29, 2015 at 8:22 AM, Paul B Mahol  wrote:
>> > > On 10/29/15, Ganesh Ajjanagadde  wrote:
>> > >> This is likely more precise and conveys the intent better.
>> > >>
>> > >> Signed-off-by: Ganesh Ajjanagadde 
>> > >> ---
>> > >>  libavfilter/avf_showvolume.c | 2 +-
>> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >>
>> > >> diff --git a/libavfilter/avf_showvolume.c
>> > >> b/libavfilter/avf_showvolume.c
>> > >> index 95b5388..395375a 100644
>> > >> --- a/libavfilter/avf_showvolume.c
>> > >> +++ b/libavfilter/avf_showvolume.c
>> > >> @@ -197,7 +197,7 @@ static int filter_frame(AVFilterLink *inlink,
>> > AVFrame
>> > >> *insamples)
>> > >>  max = FFMAX(max, src[i]);
>> > >>
>> > >>  max = av_clipf(max, 0, 1);
>> > >> -values[VAR_VOLUME] = 20.0 * log(max) / M_LN10;
>> > >> +values[VAR_VOLUME] = 20.0 * log10(max);
>> > >>  values[VAR_CHANNEL] = c;
>> > >>  color = av_expr_eval(s->c_expr, values, NULL);
>> > >>
>> > >> --
>> > >> 2.6.2
>> > >
>> > > Have you checked which one is faster?
>> > > I really have no opinion on this but gain is neglible.
>> >
>> > No, I have not. I suspect, but have not confirmed that log10 is
>> > slightly slower:
>> >
>> >
>> > https://stackoverflow.com/questions/10810105/explanation-wanted-log10-faster-than-log-and-log2-but-only-with-o2-and-greater
>> > (janneb's comment). Keep in mind their benchmark did not include the
>> > cost of a division.
>>
>>
>> A good compiler (including yours) will merge the division into the
>> multiply:
>> a * func(x) / c
>> is the same as:
>> (a / c) * func(x)
>>
>> and the compiler can pre-calculate the output of a/c into a single
>> constant.
>
> Not necessarily, floating point is not associative.

pushed, thanks.

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Hendrik Leppkes]

On Fri, Oct 30, 2015 at 12:08 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  wrote:
>> Hi,
>>
>> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer >> wrote:
>>
>>> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
>>> > This is more concise and conveys the intent better.
>>> > Furthermore, it is likely more precise as well due to lack of floating
>>> > point division.
>>> >
>>> > Signed-off-by: Ganesh Ajjanagadde 
>>>
>>> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
>>> qemu-arm
>>> fate passes on all, i could test on actual arm hw if people think
>>> that is needed
>>
>>
>> I don't think that's needed.
>>
>> Is there some way we can confirm that each of these files that is changed
>> includes libavutil/libm.h for the compatibility macros in case they're
>> lacking on the target system?
>
> pushed as is; we have until the next release or a FATE failure to
> check the build on such obscure configurations. I will try to keep an
> eye out, but please report if you find failures.
>

This is really not the right way to go. You have been notified of a
real-world portability concern with an easy fix (verify that all files
you use log2/etc in include libavutil/libm.h), and you decided to wait
until it blows up instead?
Not cool.

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 7:50 PM, Hendrik Leppkes  wrote:
> On Fri, Oct 30, 2015 at 12:08 AM, Ganesh Ajjanagadde  wrote:
>> On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  wrote:
>>> Hi,
>>>
>>> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer >>
 On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
 > This is more concise and conveys the intent better.
 > Furthermore, it is likely more precise as well due to lack of floating
 > point division.
 >
 > Signed-off-by: Ganesh Ajjanagadde 

 patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
 qemu-arm
 fate passes on all, i could test on actual arm hw if people think
 that is needed
>>>
>>>
>>> I don't think that's needed.
>>>
>>> Is there some way we can confirm that each of these files that is changed
>>> includes libavutil/libm.h for the compatibility macros in case they're
>>> lacking on the target system?
>>
>> pushed as is; we have until the next release or a FATE failure to
>> check the build on such obscure configurations. I will try to keep an
>> eye out, but please report if you find failures.
>>
>
> This is really not the right way to go. You have been notified of a
> real-world portability concern with an easy fix (verify that all files
> you use log2/etc in include libavutil/libm.h), and you decided to wait
> until it blows up instead?
> Not cool.

Ok. Do a grep on ffmpeg.c: line 56 answers your point.

Other patches have been acked by Michael, and were pushed.

More generally, how is this problem "easy to verify"? It may be
included indirectly, etc. Since you seem to think it is easy, go ahead
and do whatever you want. Or enlighten mortals like me who fail to see
it is easy with your wisdom. Or if it is "easy", why don't you do it
for the commits that I just pushed.

If FFmpeg waited until verification on every single config was done,
we would be nowhere. You may think it is not cool, I could say the
same about many things you have posted on this mailing list as well.

If Michael thought this was not cool, I will immediately take action.

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Michael Niedermayer]

On Fri, Oct 30, 2015 at 12:50:46AM +0100, Hendrik Leppkes wrote:
> On Fri, Oct 30, 2015 at 12:08 AM, Ganesh Ajjanagadde  wrote:
> > On Thu, Oct 29, 2015 at 8:27 AM, Ronald S. Bultje  
> > wrote:
> >> Hi,
> >>
> >> On Thu, Oct 29, 2015 at 8:04 AM, Michael Niedermayer 
> >>  >>> wrote:
> >>
> >>> On Thu, Oct 29, 2015 at 12:19:59AM -0400, Ganesh Ajjanagadde wrote:
> >>> > This is more concise and conveys the intent better.
> >>> > Furthermore, it is likely more precise as well due to lack of floating
> >>> > point division.
> >>> >
> >>> > Signed-off-by: Ganesh Ajjanagadde 
> >>>
> >>> patchset tested on linux32, inux64, mingw32, ming64, qemu-mips and
> >>> qemu-arm
> >>> fate passes on all, i could test on actual arm hw if people think
> >>> that is needed
> >>
> >>
> >> I don't think that's needed.
> >>
> >> Is there some way we can confirm that each of these files that is changed
> >> includes libavutil/libm.h for the compatibility macros in case they're
> >> lacking on the target system?
> >
> > pushed as is; we have until the next release or a FATE failure to
> > check the build on such obscure configurations. I will try to keep an
> > eye out, but please report if you find failures.
> >
> 
> This is really not the right way to go. You have been notified of a
> real-world portability concern with an easy fix (verify that all files
> you use log2/etc in include libavutil/libm.h), and you decided to wait
> until it blows up instead?
> Not cool.

I think this is partly my fault as i approved several of the patches
after ronalds comment.
I did check most of them for including libm.h
some do through avfilter/interal.h ->avutil/internal.h->libm.h
some do though avutil/common.h->avutil/internal.h->libm.h
IIRC

of course it is possible i forgot something and i totally didnt
communicate that i did check that

sorry

[...]
-- 
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


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

Re: [FFmpeg-devel] [PATCH] avutil/opt: add av_warn_unused_result

[Hendrik Leppkes]

On Thu, Oct 29, 2015 at 3:51 AM, Ganesh Ajjanagadde
 wrote:
> On Thu, Oct 15, 2015 at 6:12 PM, Ganesh Ajjanagadde
>  wrote:
>> Some warnings do get triggered that will need to be fixed.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavutil/opt.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavutil/opt.h b/libavutil/opt.h
>> index 753434d..5abe963 100644
>> --- a/libavutil/opt.h
>> +++ b/libavutil/opt.h
>> @@ -414,6 +414,7 @@ void av_opt_set_defaults2(void *s, int mask, int flags);
>>   * the error code issued by av_opt_set() if a key/value pair
>>   * cannot be set
>>   */
>> +av_warn_unused_result
>>  int av_set_options_string(void *ctx, const char *opts,
>>const char *key_val_sep, const char *pairs_sep);
>>
>> @@ -444,6 +445,7 @@ int av_set_options_string(void *ctx, const char *opts,
>>   * Separators must use characters distinct from option names and from each
>>   * other.
>>   */
>> +av_warn_unused_result
>>  int av_opt_set_from_string(void *ctx, const char *opts,
>> const char *const *shorthand,
>> const char *key_val_sep, const char *pairs_sep);
>> --
>> 2.6.1
>>
>
> ping

I would argue that failure from these functions is hardly critical,
ie. it can fail if an option is not found, and enforcing checks here
seems a bit too aggressive to me.

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

Re: [FFmpeg-devel] [PATCH 3/3] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters

[Hendrik Leppkes]

On Thu, Oct 29, 2015 at 8:34 AM, Clément Bœsch  wrote:
> On Wed, Oct 28, 2015 at 06:34:06PM +0100, Hendrik Leppkes wrote:
>> On Sat, Oct 17, 2015 at 10:34 PM, Matthieu Bouron
>>  wrote:
>> > From: Matthieu Bouron 
>> >
>> > Avoid decoding twice images such as jpeg and png, once in the
>> > avformat_find_stream_info and once when the actual decode is made.
>> >
>> > The decoder must honor the skip_frame option in order to skip
>> > decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and
>> > png decoders.
>> > ---
>> >  libavformat/utils.c | 15 +++
>> >  1 file changed, 15 insertions(+)
>> >
>> > diff --git a/libavformat/utils.c b/libavformat/utils.c
>> > index 689473e..67dfffc 100644
>> > --- a/libavformat/utils.c
>> > +++ b/libavformat/utils.c
>> > @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, 
>> > const char **errmsg_ptr)
>> >  static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket 
>> > *avpkt,
>> >  AVDictionary **options)
>> >  {
>> > +int i;
>> >  const AVCodec *codec;
>> >  int got_picture = 1, ret = 0;
>> >  AVFrame *frame = av_frame_alloc();
>> >  AVSubtitle subtitle;
>> >  AVPacket pkt = *avpkt;
>> > +int skip_frame;
>> > +static const enum AVCodecID no_decode_codecs[] = {
>> > +AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG,
>> > +};
>>
>> Hardcoded lists of codecs in random places are never a good thing to have.
>> If this is a feature we agree to have, its codecs should just get an
>> internal capability that tells this code if it can parse all params
>> without decoding.
>>
>
> This list is supposed to be temporary (yes I know) until all other
> decoders that currently support AVDISCARD_ALL set the information field
> required for probing.
>

We all know what happens to temporary solutions, don't we. :)
Rather have a temporary internal capability, its not part of the
API/ABI or anything, but avoids such an ugly list inside generic code.

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

Re: [FFmpeg-devel] [PATCH 3/3] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters

[Clément Bœsch]

On Wed, Oct 28, 2015 at 06:34:06PM +0100, Hendrik Leppkes wrote:
> On Sat, Oct 17, 2015 at 10:34 PM, Matthieu Bouron
>  wrote:
> > From: Matthieu Bouron 
> >
> > Avoid decoding twice images such as jpeg and png, once in the
> > avformat_find_stream_info and once when the actual decode is made.
> >
> > The decoder must honor the skip_frame option in order to skip
> > decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and
> > png decoders.
> > ---
> >  libavformat/utils.c | 15 +++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 689473e..67dfffc 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, const 
> > char **errmsg_ptr)
> >  static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket 
> > *avpkt,
> >  AVDictionary **options)
> >  {
> > +int i;
> >  const AVCodec *codec;
> >  int got_picture = 1, ret = 0;
> >  AVFrame *frame = av_frame_alloc();
> >  AVSubtitle subtitle;
> >  AVPacket pkt = *avpkt;
> > +int skip_frame;
> > +static const enum AVCodecID no_decode_codecs[] = {
> > +AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG,
> > +};
> 
> Hardcoded lists of codecs in random places are never a good thing to have.
> If this is a feature we agree to have, its codecs should just get an
> internal capability that tells this code if it can parse all params
> without decoding.
> 

This list is supposed to be temporary (yes I know) until all other
decoders that currently support AVDISCARD_ALL set the information field
required for probing.

Not full frames decoding at probing but just filling the information will
also be useful for these codecs (I think there are about 20?).
Unfortunately, the changes are sensible so it's probably better to do it
progressively one by one, updating this white list progressively. A
blacklist system was also suggested earlier in the thread.

Maybe a FIXME/XXX should be added above? (And maybe even with a compiler
warning to not forget this?)

[...]

-- 
Clément B.


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] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters

[Clément Bœsch]

On Thu, Oct 29, 2015 at 08:34:42AM +0100, Clément Bœsch wrote:
> On Wed, Oct 28, 2015 at 06:34:06PM +0100, Hendrik Leppkes wrote:
> > On Sat, Oct 17, 2015 at 10:34 PM, Matthieu Bouron
> >  wrote:
> > > From: Matthieu Bouron 
> > >
> > > Avoid decoding twice images such as jpeg and png, once in the
> > > avformat_find_stream_info and once when the actual decode is made.
> > >
> > > The decoder must honor the skip_frame option in order to skip
> > > decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and
> > > png decoders.
> > > ---
> > >  libavformat/utils.c | 15 +++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index 689473e..67dfffc 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, 
> > > const char **errmsg_ptr)
> > >  static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket 
> > > *avpkt,
> > >  AVDictionary **options)
> > >  {
> > > +int i;
> > >  const AVCodec *codec;
> > >  int got_picture = 1, ret = 0;
> > >  AVFrame *frame = av_frame_alloc();
> > >  AVSubtitle subtitle;
> > >  AVPacket pkt = *avpkt;
> > > +int skip_frame;
> > > +static const enum AVCodecID no_decode_codecs[] = {
> > > +AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG,
> > > +};
> > 
> > Hardcoded lists of codecs in random places are never a good thing to have.
> > If this is a feature we agree to have, its codecs should just get an
> > internal capability that tells this code if it can parse all params
> > without decoding.
> > 
> 
> This list is supposed to be temporary (yes I know) until all other
> decoders that currently support AVDISCARD_ALL set the information field
> required for probing.
> 
> Not full frames decoding at probing but just filling the information will
> also be useful for these codecs (I think there are about 20?).
> Unfortunately, the changes are sensible so it's probably better to do it
> progressively one by one, updating this white list progressively. A
> blacklist system was also suggested earlier in the thread.
> 
> Maybe a FIXME/XXX should be added above? (And maybe even with a compiler
> warning to not forget this?)
> 

And forgot to say: if you choose to add a capability, it will have to be
dropped when the other decoders are updated to match the behaviour.

-- 
Clément B.


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

[FFmpeg-devel] [PATCH] avutil/file_open: avoid file handle inheritance on Windows

[Tobias Rapp]

Attached patch fixes file lock issues in my Windows application when a 
child process is started with handle inheritance enabled (standard 
input/output redirection) while a FFmpeg transcoding is running in the 
parent process.


BTW: It would be great if the patch could also be applied to the 2.7/2.8 
release branches.


Regards,
Tobias
>From b54a2177aaf0aef290c0741a63327ca678b4e7d6 Mon Sep 17 00:00:00 2001
From: Tobias Rapp 
Date: Thu, 29 Oct 2015 09:11:37 +0100
Subject: [PATCH] avutil/file_open: avoid file handle inheritance on Windows

Avoids inheritance of file handles on Windows systems similar to the
O_CLOEXEC/FD_CLOEXEC flag on Linux.

Fixes file lock issues in my Windows application when a child process
is started with handle inheritance enabled (standard input/output
redirection) while a FFmpeg transcoding is running in the parent
process.

Signed-off-by: Tobias Rapp 
---
 libavutil/file_open.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavutil/file_open.c b/libavutil/file_open.c
index 3f9a67c..9e76127 100644
--- a/libavutil/file_open.c
+++ b/libavutil/file_open.c
@@ -77,6 +77,9 @@ int avpriv_open(const char *filename, int flags, ...)
 #ifdef O_CLOEXEC
 flags |= O_CLOEXEC;
 #endif
+#ifdef O_NOINHERIT
+flags |= O_NOINHERIT;
+#endif
 
 fd = open(filename, flags, mode);
 #if HAVE_FCNTL
-- 
1.9.1

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

Re: [FFmpeg-devel] Idea for a better filter that reduces noise

[Paul B Mahol]

On 10/29/15, P. van Gaans  wrote:
> You all know the CSI episodes where they read a license plate by
> "enhancing" some super-grainy security footage. Nonsense, right? Well,
> maybe not.. If the car was parked. And it seems what I found doesn't
> exist yet. (but perhaps I overlooked it)
>
> If you quickly want to know what I'm on about, take a look at these images:
>
> http://members.ziggo.nl/sinaasappel/images/1_original.jpg (original)
> http://members.ziggo.nl/sinaasappel/images/2_40framewind.jpg (40f WIND)
> http://members.ziggo.nl/sinaasappel/images/3_wind_hqdn3d.jpg (comparison
> with hqdn3d and pp=tn)
>
> All have limited jpeg compression, but I can deliver PNG files and an
> XCF to experiment for yourself if desired.
>
> So what is WIND? It's what you see if you forget/fail to do motion
> detection. (like I did in the images) Also, Wind Is Not a Denoiser. ;-)
> It's a way to increase the exposure time of the camera used to shoot a
> movie after it's been shot. For the images, I took a 2-second somewhat
> grainy clip of a building with nearly no motion. I output the frames to
> PNG and load them as layers in The GIMP. I set the opacity for the
> bottom layer to 100. The layer above that 50. (100/2) Above that 33.3.
> (100/3) 25, 20, 16.7 and so on. Every image with noise is "wrong", some
> too dark and some too light. On average, they are spot-on. The
> advantage: improved quality and better compression.
>
> To make this into a proper useable filter, it would need to do this:
>
> 1. Deshake/stabilize the image.
> 2. Divide image in blocks. (e.g. 8x8 pixels)
> 3. Figure out it the average color of an 8x8 block is changing during
> the next X frames. If not, it's probably not moving and you can average
> the values. If it is or is about to, it should be averaged over fewer
> frames or not at all. Any area that is about to move will gradually pick
> up noise so it doesn't look too unnatural.
> 4. Reshake the image.
>
> Will I do this myself? Maybe, but don't hold your breath. I'm just
> sharing this in case somebody finds it interesting and to make sure
> nobody can patent it. (assuming it hasn't been done already)

See atadenoise.

>
> Best regards,
>
> P. van Gaans
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil/opt: add av_warn_unused_result

[wm4]

On Thu, 29 Oct 2015 09:17:16 +0100
Hendrik Leppkes  wrote:

> On Thu, Oct 29, 2015 at 3:51 AM, Ganesh Ajjanagadde
>  wrote:
> > On Thu, Oct 15, 2015 at 6:12 PM, Ganesh Ajjanagadde
> >  wrote:  
> >> Some warnings do get triggered that will need to be fixed.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavutil/opt.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/libavutil/opt.h b/libavutil/opt.h
> >> index 753434d..5abe963 100644
> >> --- a/libavutil/opt.h
> >> +++ b/libavutil/opt.h
> >> @@ -414,6 +414,7 @@ void av_opt_set_defaults2(void *s, int mask, int 
> >> flags);
> >>   * the error code issued by av_opt_set() if a key/value pair
> >>   * cannot be set
> >>   */
> >> +av_warn_unused_result
> >>  int av_set_options_string(void *ctx, const char *opts,
> >>const char *key_val_sep, const char *pairs_sep);
> >>
> >> @@ -444,6 +445,7 @@ int av_set_options_string(void *ctx, const char *opts,
> >>   * Separators must use characters distinct from option names and from each
> >>   * other.
> >>   */
> >> +av_warn_unused_result
> >>  int av_opt_set_from_string(void *ctx, const char *opts,
> >> const char *const *shorthand,
> >> const char *key_val_sep, const char 
> >> *pairs_sep);
> >> --
> >> 2.6.1
> >>  
> >
> > ping  
> 
> I would argue that failure from these functions is hardly critical,
> ie. it can fail if an option is not found, and enforcing checks here
> seems a bit too aggressive to me.

+1

You might often set options on e.g. a codec generically, without caring
much whether it fails or not. This is because different codecs have
different options, and those which lack a certain option will work
without setting it. (Just as an example.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters

[wm4]

On Thu, 29 Oct 2015 08:34:42 +0100
Clément Bœsch  wrote:

> On Wed, Oct 28, 2015 at 06:34:06PM +0100, Hendrik Leppkes wrote:
> > On Sat, Oct 17, 2015 at 10:34 PM, Matthieu Bouron
> >  wrote:  
> > > From: Matthieu Bouron 
> > >
> > > Avoid decoding twice images such as jpeg and png, once in the
> > > avformat_find_stream_info and once when the actual decode is made.
> > >
> > > The decoder must honor the skip_frame option in order to skip
> > > decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and
> > > png decoders.
> > > ---
> > >  libavformat/utils.c | 15 +++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index 689473e..67dfffc 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, 
> > > const char **errmsg_ptr)
> > >  static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket 
> > > *avpkt,
> > >  AVDictionary **options)
> > >  {
> > > +int i;
> > >  const AVCodec *codec;
> > >  int got_picture = 1, ret = 0;
> > >  AVFrame *frame = av_frame_alloc();
> > >  AVSubtitle subtitle;
> > >  AVPacket pkt = *avpkt;
> > > +int skip_frame;
> > > +static const enum AVCodecID no_decode_codecs[] = {
> > > +AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG,
> > > +};  
> > 
> > Hardcoded lists of codecs in random places are never a good thing to have.
> > If this is a feature we agree to have, its codecs should just get an
> > internal capability that tells this code if it can parse all params
> > without decoding.
> >   
> 
> This list is supposed to be temporary (yes I know) until all other
> decoders that currently support AVDISCARD_ALL set the information field
> required for probing.

I do not think this is the right direction. We should go away from
mutating AVCodecContext fields, towards returning all per-frame info in
AVFrame. We should also go away from decoding in libavformat.

Before this becomes final, maybe you could check whether this could be
done in a codec parser instead? Some parsers do set the pixfmt, e.g.
h264. Yes, this suggestion is a bit late. I don't actually mind this
hack being applied in this form; at least it makes it explicit that
it's a hack for the image formats, and can be easily fixed at a later
point.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] vp9_parser: don't overwrite cached timestamps with nopts.

[Ronald S. Bultje]

Hi,

On Thu, Oct 29, 2015 at 12:45 AM, James Almer  wrote:

> On 10/28/2015 11:16 PM, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Wed, Oct 28, 2015 at 5:51 PM, wm4  wrote:
> >
> >> On Wed, 28 Oct 2015 16:05:49 -0400
> >> "Ronald S. Bultje"  wrote:
> >>
> >>> Hi,
> >>>
> >>> On Wed, Oct 28, 2015 at 3:44 PM, wm4  wrote:
> >>>
>  On Wed, 28 Oct 2015 12:21:05 -0400
>  "Ronald S. Bultje"  wrote:
> 
> > ---
> >  libavcodec/vp9_parser.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c
> > index 0437097..6713850 100644
> > --- a/libavcodec/vp9_parser.c
> > +++ b/libavcodec/vp9_parser.c
> > @@ -64,7 +64,7 @@ static int parse_frame(AVCodecParserContext *ctx,
>  const uint8_t *buf, int size)
> >  if (ctx->pts == AV_NOPTS_VALUE)
> >  ctx->pts = s->pts;
> >  s->pts = AV_NOPTS_VALUE;
> > -} else {
> > +} else if (ctx->pts != AV_NOPTS_VALUE) {
> >  s->pts = ctx->pts;
> >  ctx->pts = AV_NOPTS_VALUE;
> >  }
> 
>  I find this a bit questionable. What is it needed for? Wouldn't it
>  repeat the previous timestamp if new packets have none?
> >>>
> >>>
> >>> I don't think so. Let's first go through the problem that I'm seeing,
> >> maybe
> >>> you see alternate solutions. Then I'll explain (very end) why I don't
> >> think
> >>> this happens.
> >>>
> >>> So, we're assuming here (this is generally true) that all input to the
> >>> parser has timestamps (coming from ivf/webm), and that some frames are
> >>> "superframes" which have one invisible (alt-ref) frame (only a
> reference,
> >>> not an actual frame that's ever displayed) packed with the following
> >>> visible frame. So each packet in ivf/webm leads to one visible output
> >>> frame, but in some cases this requires decoding of multiple (invisible)
> >>> references. For frame threading purposes, we split before we send it to
> >> the
> >>> decoder.
> >>>
> >>> So what you get is:
> >>> - ivf/webm file has packet of size a with timestamp b, calls parser
> >>> - parser notices that packet is superframe with 2 frames in it
> >>> - we output the first (invisible) frame with no timestamp, and cache
> the
> >>> timestamp of the input packet
> >>> - utils.c code calls parser again with the same input data (we told it
> we
> >>> didn't consume any), but the (input) timestamp is now set to nopts
> >>> - we output the second (visible) frame with the cached timestamp on the
> >>> packet
> >>>
> >>> This generally makes sense, the webm/ivf indeed assume that the
> timestamp
> >>> only is valid for the visible frame. Invisible frames have no timestamp
> >>> associated with them since they're never displayed.
> >>>
> >>> So, the above code has the issue: what if there's 2 invisible frames
> >> packed
> >>> in a superframe (followed by the visible frame)? Right now, this
> happens:
> >>> - ivf/webm file has packet of size a with timestamp b, calls parser
> >>> - parser notices that packet is superframe with 3 frames in it
> >>> - we output the first (invisible) frame with no timestamp, and cache
> the
> >>> timestamp of the input packet
> >>> - utils.c code calls parser again with the same input data (we told it
> we
> >>> didn't consume any), but the (input) timestamp is now set to nopts
> >>> - we output the second (invisible) frame with no timestamp, and cache
> the
> >>> timestamp of the input packet (which is now set to nopts)
> >>> - utils.c code calls parser again with the same input data (we told it
> we
> >>> didn't consume any), but the (input) timestamp is now set to nopts
> >>> - we output the third (visible) frame with the cached timestamp on the
> >>> packet, which was nopts
> >>>
> >>> The last 3 are wrong; we should only cache timestamps if there is any
> to
> >> be
> >>> cached, we should not override the cached timestamp with a new nopts
> >> value,
> >>> at least not in this particular case.
> >>>
> >>> --
> >>> very end
> >>> --
> >>>
> >>> Ok, so what about your point: can we output the same timestamp twice? I
> >>> don't think so, because as soon as we output the cached timestamp, we
> >> reset
> >>> the value of the cached timestamp to nopts, so if we were to reuse the
> >>> cached timestamp, it would be nopts and the output data would have no
> >>> timestamp.
> >>>
> >>> (Hope that wasn't too detailed.)
> >>
> >> Thanks for the explanations. I didn't know there could be more than 1
> >> super frame, and I see how the new logic works and doesn't duplicate
> >> timestamps.
> >>
> >> Patch looks good with me then.
> >
> >
> > TY, pushed.
> >
> > Ronald
>
> Did you forget to update the ref files for fate-vp9-10-show-existing-frame
> and
> fate-vp9-16-intra-only? Because they are failing in a lot of fate clients.


Oops, I will work on 

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[wm4]

On Thu, 29 Oct 2015 00:19:59 -0400
Ganesh Ajjanagadde  wrote:

> This is more concise and conveys the intent better.
> Furthermore, it is likely more precise as well due to lack of floating
> point division.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---

These patches are all pretty similar. And likely tedious to check for
correctness. So low gain, while some potential for regressions.

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

Re: [FFmpeg-devel] [PATCH 2/3] lavc/mjpegdec: honor skip_frame option

[Matthieu Bouron]

On Wed, Oct 28, 2015 at 5:36 PM, Michael Niedermayer  wrote:

> On Wed, Oct 28, 2015 at 03:10:00PM +0100, Matthieu Bouron wrote:
> > On Wed, Oct 28, 2015 at 12:58:53PM +0100, Michael Niedermayer wrote:
> > > On Wed, Oct 28, 2015 at 02:57:14AM +0100, Michael Niedermayer wrote:
> > > > On Tue, Oct 27, 2015 at 11:15:29PM +0100, Matthieu Bouron wrote:
> > > > > On Sun, Oct 18, 2015 at 11:06:50AM +0200, Matthieu Bouron wrote:
> > > > > [...]
> > > > > >
> > > > > > Patch updated, the markers are now properly skipped (which also
> fixes a
> > > > > > crash).
> > > > >
> > > > > Patch updated. It fixes an issue with mjpeg streams (and in
> particular with
> > > > > the fate sample ffmpeg-issue-897.avi) due to the EOI marker not
> handled when
> > > > > skip_frame is set to AVDISCARD_ALL.
> > > > >
> > > > > Matthieu
> > > >
> > > > >  mjpegdec.c |   26 ++
> > > > >  1 file changed, 26 insertions(+)
> > > > > 36d41f1bded2f864394843c6a49d8cc24933688c
> 0002-lavc-mjpegdec-honor-skip_frame-option.patch
> > > > > From 7325810d812c4182cd42946687a1f4abc04999d1 Mon Sep 17 00:00:00
> 2001
> > > > > From: Matthieu Bouron 
> > > > > Date: Fri, 9 Oct 2015 15:15:15 +0200
> > > > > Subject: [PATCH 2/3] lavc/mjpegdec: honor skip_frame option
> > > > >
> > > > > ---
> > > > >  libavcodec/mjpegdec.c | 26 ++
> > > > >  1 file changed, 26 insertions(+)
> > > > >
> > > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > > > > index 1a86b7b..b4ff83c 100644
> > > > > --- a/libavcodec/mjpegdec.c
> > > > > +++ b/libavcodec/mjpegdec.c
> > > > > @@ -2038,6 +2038,22 @@ int ff_mjpeg_decode_frame(AVCodecContext
> *avctx, void *data, int *got_frame,
> > > > >  return AVERROR(ENOSYS);
> > > > >  }
> > > > >
> > > > > +if (avctx->skip_frame == AVDISCARD_ALL) {
> > > > > +int i, found = 0;
> > > > > +static const int start_codes[] = { SOF0,
> > > > > +SOF1, SOF2, SOF3, SOF48, SOI, EOI };
> > > > > +
> > > > > +for (i = 0; i < FF_ARRAY_ELEMS(start_codes); i++) {
> > > > > +if (start_code == start_codes[i]) {
> > > > > +found = 1;
> > > > > +break;
> > > > > +}
> > > > > +}
> > > > > +if (!found) {
> > > > > +goto skip;
> > > > > +}
> > >
> > > i think this would be simpler and shorter if implemented as a
> > > switch(start_code)/case
> > >
> >
> > Patch updated using a switch/case to implement the skip logic.
> >
> > [...]
>
> >  mjpegdec.c |   25 +
> >  1 file changed, 25 insertions(+)
> > 2db19e21ec69adc30ec5a3fdf03bd0c60257cb65
> 0002-lavc-mjpegdec-honor-skip_frame-option.patch
> > From cb09375d6cf5051ce51e43a2787f0242bd6450bc Mon Sep 17 00:00:00 2001
> > From: Matthieu Bouron 
> > Date: Fri, 9 Oct 2015 15:15:15 +0200
> > Subject: [PATCH 2/3] lavc/mjpegdec: honor skip_frame option
>
> LGTM
>

Pushed.
Thanks

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

Re: [FFmpeg-devel] [PATCH 1/3] lavc/pngdec: honor skip_frame option

[Matthieu Bouron]

On Wed, Oct 28, 2015 at 5:35 PM, Michael Niedermayer  wrote:

> On Wed, Oct 28, 2015 at 03:09:02PM +0100, Matthieu Bouron wrote:
> > On Wed, Oct 28, 2015 at 12:59:54PM +0100, Michael Niedermayer wrote:
> > > On Mon, Oct 19, 2015 at 10:28:18AM +0200, Matthieu Bouron wrote:
> > > > On Sat, Oct 17, 2015 at 10:34:22PM +0200, Matthieu Bouron wrote:
> > > > > From: Matthieu Bouron 
> > > > >
> > > > > ---
> > > > >  libavcodec/pngdec.c | 34 ++
> > > > >  1 file changed, 34 insertions(+)
> > > > >
> > > > > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > > > > index 4cfdc58..5b2befe 100644
> > > > > --- a/libavcodec/pngdec.c
> > > > > +++ b/libavcodec/pngdec.c
> > > > > @@ -1087,6 +1087,13 @@ static int
> decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> > > > >  for (;;) {
> > > > >  length = bytestream2_get_bytes_left(>gb);
> > > > >  if (length <= 0) {
> > > > > +
> > > > > +if (avctx->codec_id == AV_CODEC_ID_PNG &&
> > > > > +avctx->skip_frame == AVDISCARD_ALL) {
> > > > > +av_frame_set_metadata(p, metadata);
> > > > > +return 0;
> > > > > +}
> > > > > +
> > > > >  if (CONFIG_APNG_DECODER && avctx->codec_id ==
> AV_CODEC_ID_APNG && length == 0) {
> > > > >  if (!(s->state & PNG_IDAT))
> > > > >  return 0;
> > > > > @@ -1114,6 +1121,22 @@ static int
> decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> > > > >  ((tag >> 8) & 0xff),
> > > > >  ((tag >> 16) & 0xff),
> > > > >  ((tag >> 24) & 0xff), length);
> > > > > +
> > > > > +if (avctx->codec_id == AV_CODEC_ID_PNG &&
> > > > > +avctx->skip_frame == AVDISCARD_ALL) {
> > > > > +int i, found = 0;
> > > > > +static const int tags[] = { MKTAG('I', 'H', 'D',
> 'R'), MKTAG('t', 'E', 'X', 't'), MKTAG('I', 'D', 'A', 'T') };
> > > > > +for (i = 0; i < FF_ARRAY_ELEMS(tags); i++) {
> > > > > +if (tag == tags[i]) {
> > > > > +found = 1;
> > > > > +break;
> > > > > +}
> > > > > +}
> > > > > +if (!found) {
> > > > > +goto skip_tag;
> > > > > +}
> > > > > +}
> > > > > +
> > > > >  switch (tag) {
> > > > >  case MKTAG('I', 'H', 'D', 'R'):
> > > > >  if ((ret = decode_ihdr_chunk(avctx, s, length)) < 0)
> > > > > @@ -1181,6 +1204,11 @@ skip_tag:
> > > > >  }
> > > > >  }
> > > > >  exit_loop:
> > > > > +if (avctx->codec_id == AV_CODEC_ID_PNG &&
> > > > > +avctx->skip_frame == AVDISCARD_ALL) {
> > > > > +av_frame_set_metadata(p, metadata);
> > > > > +return 0;
> > > > > +}
> > > > >
> > > > >  if (s->bits_per_pixel <= 4)
> > > > >  handle_small_bpp(s, p);
> > > > > @@ -1278,6 +1306,12 @@ static int decode_frame_png(AVCodecContext
> *avctx,
> > > > >  if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
> > > > >  goto the_end;
> > > > >
> > > > > +if (avctx->skip_frame == AVDISCARD_ALL) {
> > > > > +*got_frame = 0;
> > > > > +ret = bytestream2_tell(>gb);
> > > > > +goto the_end;
> > > > > +}
> > > > > +
> > > > >  if ((ret = av_frame_ref(data, s->picture.f)) < 0)
> > > > >  return ret;
> > > > >
> > > > > --
> > > > > 2.6.1
> > > > >
> > > >
> > > > Patch updated, the SAR is not discarded anymore.
> > > >
> > > > Matthieu
> > >
> > > >  pngdec.c |   39 +++
> > > >  1 file changed, 39 insertions(+)
> > > > eddfb042e2653025b37f9deacc0357ac3e5e5148
> 0001-lavc-pngdec-honor-skip_frame-option.patch
> > > > From ca6362457eb27e623eadcadb0e33a84ea461180f Mon Sep 17 00:00:00
> 2001
> > > > From: Matthieu Bouron 
> > > > Date: Fri, 9 Oct 2015 15:14:11 +0200
> > > > Subject: [PATCH 1/3] lavc/pngdec: honor skip_frame option
> > > >
> > > > ---
> > > >  libavcodec/pngdec.c | 39 +++
> > > >  1 file changed, 39 insertions(+)
> > > >
> > > > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > > > index 4cfdc58..cb99dbe 100644
> > > > --- a/libavcodec/pngdec.c
> > > > +++ b/libavcodec/pngdec.c
> > > > @@ -1087,6 +1087,13 @@ static int decode_frame_common(AVCodecContext
> *avctx, PNGDecContext *s,
> > > >  for (;;) {
> > > >  length = bytestream2_get_bytes_left(>gb);
> > > >  if (length <= 0) {
> > > > +
> > > > +if (avctx->codec_id == AV_CODEC_ID_PNG &&
> > > > +avctx->skip_frame == AVDISCARD_ALL) {
> > > > +av_frame_set_metadata(p, metadata);
> > > > +return 0;
> > > > +}
> > > > +
> > > >  if (CONFIG_APNG_DECODER && avctx->codec_id ==
> 

Re: [FFmpeg-devel] [PATCH] vf_lut: Add support for RGB48 and RGBA64

[Michael Niedermayer]

On Thu, Oct 29, 2015 at 09:41:21PM +0100, Paul B Mahol wrote:
> On 10/25/15, Steven Robertson  wrote:
> > Done, thanks.
> >
> 
> probably ok

applied

thanks

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

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


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

Re: [FFmpeg-devel] Idea for a better filter that reduces noise

[P. van Gaans]

On 10/29/2015 09:42 AM, Paul B Mahol wrote:

On 10/29/15, P. van Gaans  wrote:

You all know the CSI episodes where they read a license plate by
"enhancing" some super-grainy security footage. Nonsense, right? Well,
maybe not.. If the car was parked. And it seems what I found doesn't
exist yet. (but perhaps I overlooked it)

If you quickly want to know what I'm on about, take a look at these images:

http://members.ziggo.nl/sinaasappel/images/1_original.jpg (original)
http://members.ziggo.nl/sinaasappel/images/2_40framewind.jpg (40f WIND)
http://members.ziggo.nl/sinaasappel/images/3_wind_hqdn3d.jpg (comparison
with hqdn3d and pp=tn)

All have limited jpeg compression, but I can deliver PNG files and an
XCF to experiment for yourself if desired.

So what is WIND? It's what you see if you forget/fail to do motion
detection. (like I did in the images) Also, Wind Is Not a Denoiser. ;-)
It's a way to increase the exposure time of the camera used to shoot a
movie after it's been shot. For the images, I took a 2-second somewhat
grainy clip of a building with nearly no motion. I output the frames to
PNG and load them as layers in The GIMP. I set the opacity for the
bottom layer to 100. The layer above that 50. (100/2) Above that 33.3.
(100/3) 25, 20, 16.7 and so on. Every image with noise is "wrong", some
too dark and some too light. On average, they are spot-on. The
advantage: improved quality and better compression.

To make this into a proper useable filter, it would need to do this:

1. Deshake/stabilize the image.
2. Divide image in blocks. (e.g. 8x8 pixels)
3. Figure out it the average color of an 8x8 block is changing during
the next X frames. If not, it's probably not moving and you can average
the values. If it is or is about to, it should be averaged over fewer
frames or not at all. Any area that is about to move will gradually pick
up noise so it doesn't look too unnatural.
4. Reshake the image.

Will I do this myself? Maybe, but don't hold your breath. I'm just
sharing this in case somebody finds it interesting and to make sure
nobody can patent it. (assuming it hasn't been done already)


See atadenoise.



Best regards,

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


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



Thanks Paul! I think I scrolled past atadenoise in the docs a while ago 
but had otherwise never heard of it. While searching for it there is 
little to find besides the docs and source code. (i.e. no forum threads 
or mentions in guides or anything) My ffmpeg doesn't even have it so I 
guess it's quite a new thing. I'll first compile a fresh one and get 
testing. I hope atadenoise also performs the deshaking internally as I 
described since I noticed the camera is usually not fixed, without 
deshaking it won't be able to match much.


I still think WIND would have been a funnier name than atadenoise, but I 
don't complain. This will be great if it works as well as my pictures. :-)

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

Re: [FFmpeg-devel] [PATCH 01/11] ffmpeg: use log10 instead of log()/log(10)

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 8:31 PM, wm4  wrote:
> On Thu, 29 Oct 2015 20:05:28 -0400
> Ganesh Ajjanagadde  wrote:
>
>> More generally, how is this problem "easy to verify"? It may be
>> included indirectly, etc. Since you seem to think it is easy, go ahead
>
> Indirect inclusion is IMHO not fine for such compat headers which
> define standard functions on systems where they're missing. And direct
> inclusion is easy to verify.
>
> Nobody expects that you think of everything, but here you ignored a
> direct request from a reviewer.
>
>>
>> If FFmpeg waited until verification on every single config was done,
>> we would be nowhere. You may think it is not cool, I could say the
>> same about many things you have posted on this mailing list as well.
>
> Strange that you're so awfully pedantic about C standard conformance
> (so that we need dozens of patches to fix what doesn't need to be
> fixed),

I do not need to address your beliefs, but I do need to address
outright falsehood. I normally do not address such things, but things
have gone out of hand. I am tired of this, and want it on the public
record the actual state of affairs. I did not even submit "dozens" of
patches for C standard conformance.  There was a 5-patch series for
this, all available on ffmpeg-devel for those interested. Even with
the 5 patches, I was clear that I am ready to drop immediately since
after all they are "pedantic":
https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/181972.html

I knew they would likely not get much traction, and would likely be
ridiculed for fixing problems that don't need fixing. I never imagined
it being used in sarcasm to attack another action I took regarding
FFmpeg.

> but when it gets annoying for you, suddenly pushing and waiting
> until it breaks is fine. How does this even make sense?
>
>> If Michael thought this was not cool, I will immediately take action.
>
> Hendrik's voice counts as much as Michael's.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/5] ffprobe: add support for printing packet strings metadata as packet tags

[Stefano Sabatini]

On date Saturday 2015-10-24 22:42:12 +0200, Marton Balint encoded:
> ffprobe.xsd already contains the tag element.
> 
> Signed-off-by: Marton Balint 
> ---
>  ffprobe.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/ffprobe.c b/ffprobe.c
> index ac03689..f5930ae 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -77,6 +77,7 @@ static int do_show_format_tags = 0;
>  static int do_show_frame_tags = 0;
>  static int do_show_program_tags = 0;
>  static int do_show_stream_tags = 0;
> +static int do_show_packet_tags = 0;
>  
>  static int show_value_unit  = 0;
>  static int use_value_prefix = 0;
> @@ -135,6 +136,7 @@ typedef enum {
>  SECTION_ID_LIBRARY_VERSION,
>  SECTION_ID_LIBRARY_VERSIONS,
>  SECTION_ID_PACKET,
> +SECTION_ID_PACKET_TAGS,
>  SECTION_ID_PACKETS,
>  SECTION_ID_PACKETS_AND_FRAMES,
>  SECTION_ID_PACKET_SIDE_DATA_LIST,
> @@ -178,7 +180,8 @@ static struct section sections[] = {
>  [SECTION_ID_LIBRARY_VERSION] ={ SECTION_ID_LIBRARY_VERSION, 
> "library_version", 0, { -1 } },
>  [SECTION_ID_PACKETS] ={ SECTION_ID_PACKETS, "packets", 
> SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET, -1} },
>  [SECTION_ID_PACKETS_AND_FRAMES] = { SECTION_ID_PACKETS_AND_FRAMES, 
> "packets_and_frames", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET, -1} },
> -[SECTION_ID_PACKET] = { SECTION_ID_PACKET, "packet", 0, { 
> SECTION_ID_PACKET_SIDE_DATA_LIST, -1 } },
> +[SECTION_ID_PACKET] = { SECTION_ID_PACKET, "packet", 0, { 
> SECTION_ID_PACKET_TAGS, SECTION_ID_PACKET_SIDE_DATA_LIST, -1 } },
> +[SECTION_ID_PACKET_TAGS] ={ SECTION_ID_PACKET_TAGS, "tags", 
> SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name 
> = "packet_tags" },
>  [SECTION_ID_PACKET_SIDE_DATA_LIST] ={ SECTION_ID_PACKET_SIDE_DATA_LIST, 
> "side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET_SIDE_DATA, -1 } 
> },
>  [SECTION_ID_PACKET_SIDE_DATA] = { SECTION_ID_PACKET_SIDE_DATA, 
> "side_data", 0, { -1 } },
>  [SECTION_ID_PIXEL_FORMATS] =  { SECTION_ID_PIXEL_FORMATS, 
> "pixel_formats", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PIXEL_FORMAT, -1 } },
> @@ -1762,6 +1765,16 @@ static void show_packet(WriterContext *w, 
> AVFormatContext *fmt_ctx, AVPacket *pk
>  
>  if (pkt->side_data_elems) {
>  int i;
> +int size;
> +const uint8_t *side_metadata;
> +
> +side_metadata = av_packet_get_side_data(pkt, 
> AV_PKT_DATA_STRINGS_METADATA, );
> +if (side_metadata && size && do_show_packet_tags) {
> +AVDictionary *dict = NULL;
> +if (av_packet_unpack_dictionary(side_metadata, size, ) >= 0)
> +show_tags(w, dict, SECTION_ID_PACKET_TAGS);
> +av_dict_free();
> +}
>  writer_print_section_header(w, SECTION_ID_PACKET_SIDE_DATA_LIST);
>  for (i = 0; i < pkt->side_data_elems; i++) {
>  AVPacketSideData *sd = >side_data[i];
> @@ -3178,6 +3191,7 @@ int main(int argc, char **argv)
>  SET_DO_SHOW(FRAME_TAGS, frame_tags);
>  SET_DO_SHOW(PROGRAM_TAGS, program_tags);
>  SET_DO_SHOW(STREAM_TAGS, stream_tags);
> +SET_DO_SHOW(PACKET_TAGS, packet_tags);

LGTM, thanks.
-- 
FFmpeg = Freak and Fundamental Merciful Ponderous Eretic Guru
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] ffmpeg.c can not process invalid PTS

[朱世奇]

diff --git a/ffmpeg.c b/ffmpeg.c
index f91fb7b..22766b3 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -3864,6 +3864,9 @@ static int process_input(int file_index)
 if (pkt.dts != AV_NOPTS_VALUE)
 pkt.dts *= ist->ts_scale;
 
+if(pkt.pts == AV_NOPTS_VALUE)
+pkt.pts = ist->next_pts;
+
 if ((ist->dec_ctx->codec_type == AVMEDIA_TYPE_VIDEO ||
  ist->dec_ctx->codec_type == AVMEDIA_TYPE_AUDIO) &&
 pkt.dts != AV_NOPTS_VALUE && ist->next_dts == AV_NOPTS_VALUE && 
!copy_ts


Explanation:
In function "process_input" ffmpeg just handle "pts != AV_NOPTS_VALUE", but not 
handle "pts == AV_NOPTS_VALUE".Now I fixed it. 
if ( current_pts == AV_NOPTS_VALUE) {
current_pts  = predicted_pts;
}


Reproduce:
1. Download test file http://pan.baidu.com/s/1eQsb9hK;
2. Run command below:
ffmpeg -i ./manga.01.ts -vcodec copy -bsf:v h264_mp4toannexb -hls_list_size 0 
-hls_segment_filename "test_%04d.ts" playlist.m3u8
3. If the bug not fixed, the HLS slice just one; Then fix the bug, HLS' slice 
is normally.






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