Re: [FFmpeg-devel] VS 2015 patch

[compn]

On Wed, 25 Nov 2015 11:21:55 -0800
Bruce Dawson  wrote:
> I'm mentioning this just in case some other lost soul shows up trying
> to contribute a patch to a Chromium only file. I'll get my patch
> landed in the right place.
> 
> Thanks for the help.

hope we did not scare you off.

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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[Ronald S. Bultje]

Hi,

On Thu, Nov 26, 2015 at 3:08 PM, wm4  wrote:

> On Thu, 26 Nov 2015 20:46:25 +0100
> Nicolas George  wrote:
>
> > Le sextidi 6 frimaire, an CCXXIV, wm4 a écrit :
> > > Better do the looping internal in flvdec.c (if there's no huge number
> > > of other demuxers which need this), instead of adding a new error code
> > > that is also part of the public API.
> >
> > There are a few, but not many. This was my first intent, but looping in
> the
> > framework is way more elegant and much cleaner design.
>
> I fail to see how letting such a workaround (required for flv) leak to
> common code is more elegant.


I +1 this design comment, I have some (...) reservations about adding
EAGAIN2 (which is really what this is) to our public API (which is really
what this is).

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

[FFmpeg-devel] [PATCH] avcodec/aac_tablegen: speed up table initialization

[Ganesh Ajjanagadde]

This speeds up aac_tablegen to a ludicruous degree (~97%), i.e to the point
where it can be argued that runtime initialization can always be done instead of
hard-coded tables. The only cost is essentially a trivial increase in
the stack size.

Even if one does not care about this, the patch also improves accuracy
as detailed below.

Performance:
Benchmark obtained by looping 10^4 times over ff_aac_tableinit.

Sample benchmark (x86-64, Haswell, GNU/Linux):
old:
1295292 decicycles in ff_aac_tableinit, 512 runs,  0 skips
1275981 decicycles in ff_aac_tableinit,1024 runs,  0 skips
1272932 decicycles in ff_aac_tableinit,2048 runs,  0 skips
1262164 decicycles in ff_aac_tableinit,4096 runs,  0 skips
1256720 decicycles in ff_aac_tableinit,8192 runs,  0 skips

new:
25691 decicycles in ff_aac_tableinit, 505 runs,  7 skips
25130 decicycles in ff_aac_tableinit,1016 runs,  8 skips
25973 decicycles in ff_aac_tableinit,2036 runs, 12 skips
25911 decicycles in ff_aac_tableinit,4078 runs, 18 skips
25816 decicycles in ff_aac_tableinit,8154 runs, 38 skips

Accuracy:
The previous code was resulting in needless loss of
accuracy due to the pow being called in succession. As an illustration
of this:
ff_aac_pow34sf_tab[3]
old : 0.0007598092294225
new : 0.0007598091426864
real: 0.0007598091778545

truncated to float
old : 0.0007598092294225
new : 0.0007598091426864
real: 0.0007598091426864

showing that the old value was not correctly rounded. This affects a
large number of elements of the array.

Patch tested with FATE.

Signed-off-by: Ganesh Ajjanagadde 
---
 libavcodec/aac_tablegen.h | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/libavcodec/aac_tablegen.h b/libavcodec/aac_tablegen.h
index 8b223f9..255723b 100644
--- a/libavcodec/aac_tablegen.h
+++ b/libavcodec/aac_tablegen.h
@@ -35,9 +35,43 @@ float ff_aac_pow34sf_tab[428];
 av_cold void ff_aac_tableinit(void)
 {
 int i;
+
+/* 2^(i/16) for 0 <= i <= 15 */
+const double exp2_lut[] = {
+1.,
+1.04427378242741384032,
+1.09050773266525765921,
+1.13878863475669165370,
+1.18920711500272106672,
+1.24185781207348404859,
+1.29683955465100966593,
+1.35425554693689272830,
+1.41421356237309504880,
+1.47682614593949931139,
+1.54221082540794082361,
+1.61049033194925430818,
+1.68179283050742908606,
+1.75625216037329948311,
+1.83400808640934246349,
+1.91520656139714729387,
+};
+double t1 = 8.8817841970012523233890533447265625e-16; // 2^(-50)
+double t2 = 3.63797880709171295166015625e-12; // 2^(-38)
+int t1_inc_cur, t2_inc_cur;
+int t1_inc_prev = 0;
+int t2_inc_prev = 8;
+
 for (i = 0; i < 428; i++) {
-ff_aac_pow2sf_tab[i] = pow(2, (i - POW_SF2_ZERO) / 4.0);
-ff_aac_pow34sf_tab[i] = pow(ff_aac_pow2sf_tab[i], 3.0/4.0);
+t1_inc_cur = 4 * (i % 4);
+t2_inc_cur = (8 + 3*i) % 16;
+if (t1_inc_cur < t1_inc_prev)
+t1 *= 2;
+if (t2_inc_cur < t2_inc_prev)
+t2 *= 2;
+ff_aac_pow2sf_tab[i] = t1 * exp2_lut[t1_inc_cur];
+ff_aac_pow34sf_tab[i] = t2 * exp2_lut[t2_inc_cur];
+t1_inc_prev = t1_inc_cur;
+t2_inc_prev = t2_inc_cur;
 }
 }
 #endif /* CONFIG_HARDCODED_TABLES */
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH] avcodec/aac_tablegen: speed up table initialization

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 4:31 PM, Ganesh Ajjanagadde
 wrote:
> This speeds up aac_tablegen to a ludicruous degree (~97%), i.e to the point
> where it can be argued that runtime initialization can always be done instead 
> of
> hard-coded tables. The only cost is essentially a trivial increase in
> the stack size.
>
> Even if one does not care about this, the patch also improves accuracy
> as detailed below.
>
> Performance:
> Benchmark obtained by looping 10^4 times over ff_aac_tableinit.
>
> Sample benchmark (x86-64, Haswell, GNU/Linux):
> old:
> 1295292 decicycles in ff_aac_tableinit, 512 runs,  0 skips
> 1275981 decicycles in ff_aac_tableinit,1024 runs,  0 skips
> 1272932 decicycles in ff_aac_tableinit,2048 runs,  0 skips
> 1262164 decicycles in ff_aac_tableinit,4096 runs,  0 skips
> 1256720 decicycles in ff_aac_tableinit,8192 runs,  0 skips
>
> new:
> 25691 decicycles in ff_aac_tableinit, 505 runs,  7 skips
> 25130 decicycles in ff_aac_tableinit,1016 runs,  8 skips
> 25973 decicycles in ff_aac_tableinit,2036 runs, 12 skips
> 25911 decicycles in ff_aac_tableinit,4078 runs, 18 skips
> 25816 decicycles in ff_aac_tableinit,8154 runs, 38 skips
>
> Accuracy:
> The previous code was resulting in needless loss of
> accuracy due to the pow being called in succession. As an illustration
> of this:
> ff_aac_pow34sf_tab[3]
> old : 0.0007598092294225
> new : 0.0007598091426864
> real: 0.0007598091778545
>
> truncated to float
> old : 0.0007598092294225
> new : 0.0007598091426864
> real: 0.0007598091426864
>
> showing that the old value was not correctly rounded. This affects a
> large number of elements of the array.
>
> Patch tested with FATE.
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/aac_tablegen.h | 38 --
>  1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/aac_tablegen.h b/libavcodec/aac_tablegen.h
> index 8b223f9..255723b 100644
> --- a/libavcodec/aac_tablegen.h
> +++ b/libavcodec/aac_tablegen.h
> @@ -35,9 +35,43 @@ float ff_aac_pow34sf_tab[428];
>  av_cold void ff_aac_tableinit(void)
>  {
>  int i;
> +
> +/* 2^(i/16) for 0 <= i <= 15 */
> +const double exp2_lut[] = {
> +1.,
> +1.04427378242741384032,
> +1.09050773266525765921,
> +1.13878863475669165370,
> +1.18920711500272106672,
> +1.24185781207348404859,
> +1.29683955465100966593,
> +1.35425554693689272830,
> +1.41421356237309504880,
> +1.47682614593949931139,
> +1.54221082540794082361,
> +1.61049033194925430818,
> +1.68179283050742908606,
> +1.75625216037329948311,
> +1.83400808640934246349,
> +1.91520656139714729387,
> +};
> +double t1 = 8.8817841970012523233890533447265625e-16; // 2^(-50)
> +double t2 = 3.63797880709171295166015625e-12; // 2^(-38)
> +int t1_inc_cur, t2_inc_cur;
> +int t1_inc_prev = 0;
> +int t2_inc_prev = 8;
> +
>  for (i = 0; i < 428; i++) {
> -ff_aac_pow2sf_tab[i] = pow(2, (i - POW_SF2_ZERO) / 4.0);
> -ff_aac_pow34sf_tab[i] = pow(ff_aac_pow2sf_tab[i], 3.0/4.0);
> +t1_inc_cur = 4 * (i % 4);
> +t2_inc_cur = (8 + 3*i) % 16;
> +if (t1_inc_cur < t1_inc_prev)
> +t1 *= 2;
> +if (t2_inc_cur < t2_inc_prev)
> +t2 *= 2;
> +ff_aac_pow2sf_tab[i] = t1 * exp2_lut[t1_inc_cur];
> +ff_aac_pow34sf_tab[i] = t2 * exp2_lut[t2_inc_cur];
> +t1_inc_prev = t1_inc_cur;
> +t2_inc_prev = t2_inc_cur;
>  }
>  }
>  #endif /* CONFIG_HARDCODED_TABLES */
> --
> 2.6.2
>

BTW, further speedup (from ~25000 to ~2 decicycles) that turns out
to not change any of the table values (from the more accurate new
ones) may be obtained by changing t1, t2, and the lut to float. This
also reduces the stack size.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Matthieu Bouron]

On Thu, Nov 26, 2015 at 08:08:25PM +0100, Michael Niedermayer wrote:
> On Thu, Nov 26, 2015 at 05:16:32PM +0100, Matthieu Bouron wrote:
> > On Thu, Nov 19, 2015 at 12:10:20PM +0100, Matthieu Bouron wrote:
> > > On Mon, Nov 16, 2015 at 11:16:42AM -0500, Ronald S. Bultje wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Nov 16, 2015 at 11:06 AM, Matthieu Bouron 
> > > >  > > > > wrote:
> > > > 
> > > > > On Sun, Nov 15, 2015 at 08:12:57AM -0500, Ronald S. Bultje wrote:
> > > > > > Hi,
> > > > >
> > > > > Hi,
> > > > >
> > > > > >
> > > > > > On Sun, Nov 15, 2015 at 4:49 AM, Matthieu Bouron <
> > > > > matthieu.bou...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > On Mon, Nov 02, 2015 at 07:56:50AM -0500, Ronald S. Bultje wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron <
> > > > > > > matthieu.bou...@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > From: Matthieu Bouron 
> > > > > > > > >
> > > > > > > > > Avoid decoding a frame to get the codec parameters while the 
> > > > > > > > > codec
> > > > > > > > > supports FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM. This is 
> > > > > > > > > particulary
> > > > > useful
> > > > > > > > > to avoid decoding twice images (once in 
> > > > > > > > > avformat_find_stream_info
> > > > > and
> > > > > > > > > once when the actual decode is made).
> > > > > > > > > ---
> > > > > > > > >  libavformat/utils.c | 12 
> > > > > > > > >  1 file changed, 12 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > > > > > > index 5c4d452..ba62f2b 100644
> > > > > > > > > --- a/libavformat/utils.c
> > > > > > > > > +++ b/libavformat/utils.c
> > > > > > > > > @@ -2695,6 +2695,8 @@ static int 
> > > > > > > > > try_decode_frame(AVFormatContext
> > > > > *s,
> > > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > > >  AVFrame *frame = av_frame_alloc();
> > > > > > > > >  AVSubtitle subtitle;
> > > > > > > > >  AVPacket pkt = *avpkt;
> > > > > > > > > +int do_skip_frame = 0;
> > > > > > > > > +enum AVDiscard skip_frame;
> > > > > > > > >
> > > > > > > > >  if (!frame)
> > > > > > > > >  return AVERROR(ENOMEM);
> > > > > > > > > @@ -2733,6 +2735,12 @@ static int 
> > > > > > > > > try_decode_frame(AVFormatContext
> > > > > *s,
> > > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > > >  goto fail;
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > +if (st->codec->codec->caps_internal &
> > > > > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > > > > +do_skip_frame = 1;
> > > > > > > > > +skip_frame = st->codec->skip_frame;
> > > > > > > > > +st->codec->skip_frame = AVDISCARD_ALL;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  while ((pkt.size > 0 || (!pkt.data && got_picture)) &&
> > > > > > > > > ret >= 0 &&
> > > > > > > > > (!has_codec_parameters(st, NULL) ||
> > > > > > > > > !has_decode_delay_been_guessed(st) ||
> > > > > > > > > @@ -2768,6 +2776,10 @@ static int 
> > > > > > > > > try_decode_frame(AVFormatContext
> > > > > *s,
> > > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > > >  ret = -1;
> > > > > > > > >
> > > > > > > > >  fail:
> > > > > > > > > +if (do_skip_frame) {
> > > > > > > > > +st->codec->skip_frame = skip_frame;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  av_frame_free();
> > > > > > > > >  return ret;
> > > > > > > > >  }
> > > > > > > > > --
> > > > > > > > > 2.6.2
> > > > > > > >
> > > > > > > >
> > > > > > > > I think we need an assert in the try_decode loop to ensure that 
> > > > > > > > it
> > > > > indeed
> > > > > > > > did fill all the params. This is to prevent the case where 
> > > > > > > > someone
> > > > > adds a
> > > > > > > > new "thing" to the list of things required for find_stream_info 
> > > > > > > > to
> > > > > > > succeed,
> > > > > > > > and forgets to update the codecs.
> > > > > > >
> > > > > > > While the codec_has_parameters function fits that role, it does 
> > > > > > > not
> > > > > check
> > > > > > > all codec parameters as they can be codec dependant.
> > > > > > >
> > > > > > > I'm not sure if we can create a generic rule here for the same 
> > > > > > > reason
> > > > > as
> > > > > > > above, as the parameters set can be codec specific and maybe 
> > > > > > > stream
> > > > > > > specific.
> > > > > > >
> > > > > > > Having some fate test to cover this area seems to be another 
> > > > > > > option but
> > > > > > > would
> > > > > > > require to inspect all the relevant parameters of AVCodecContext
> > > > > depending
> > > > > > > on the codec and a lot of different streams.
> > > > > >
> > > > > >
> > > > > > I don't care about _which_ parameters were filled. The goal of this
> > > > > option
> > > > > > is only directly 

Re: [FFmpeg-devel] [PATCH] qsvenc: write a53 caption data to SEI

[Michael Niedermayer]

On Thu, Nov 12, 2015 at 02:13:08PM +0300, Ivan Uskov wrote:
> Hello Will,
> 
> Thursday, November 12, 2015, 12:53:46 AM, you wrote:
> 
> WK> On 11/07, Ivan Uskov wrote:
> >> Although   the  code  looks  ok  by itself, I believe it is bad idea to 
> >> place
> >> H.264-specific   codeto   the   function   which  is  commonfor   
> >> all
> >> encoders.  I believe H.264-specific user data insertion should  locates  
> >> into
> >> the qsvenc_h264.c 
> >> I.e. there is necessary some kind of 'SetEncodeCtrl' callback which points 
> >> to
> >> function into the encoder-specific module.
> >> I believe if you will define a callback pointer 
> >> QSVEncContext::SetEncodeCtrlCB,
> >> setup  it  into  qsv_enc_init()  of the qsvenc_h264.c and call if it 
> >> non-zero
> >> from   ff_qsv_encode()  we  will  have  good base to extend add user data 
> >> for
> >> MPEG2 later. Else we will get very bulky and ugly ff_qsv_encode().
> >> I can release something but I hope you will able to catch my idea.
> >> Please let me know if something is unclear.
> 
> WK> Hi Ivan,
> 
> WK> Here is an updated patch.  Let me know if this is what you were thinking.
> I'm like and agree this implementation, no questions from my side. Thank!

I tried to apply this but it seems to doesnt apply cleanly anymore
id fix the conflicts and apply but i cant test this so i think its
better if you would fix it

Thanks

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

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


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

Re: [FFmpeg-devel] [PATCH] qsvenc: Add adaptive_i and adaptive_b toggles

[Michael Niedermayer]

On Thu, Nov 12, 2015 at 05:10:33PM -0600, Will Kelleher wrote:
> > Scene change detection ?
> > and
> > Content dependant B frame insertion
> > 
> > And if people agree then please someone submit a patch with it
> > ill apply it
> > 
> 
> New patch.
> 

>  qsvenc.c  |2 ++
>  qsvenc.h  |2 ++
>  qsvenc_h264.c |2 ++
>  3 files changed, 6 insertions(+)
> 9d1c2a0da13b358f77fbb84a3c5ac723d563d46a  
> 0001-qsvenc-Add-adaptive_i-and-adaptive_b-toggles.patch
> From 983fb04e74c133de350da41bd5961f8c840ff327 Mon Sep 17 00:00:00 2001
> From: Will Kelleher 
> Date: Tue, 10 Nov 2015 14:30:21 -0600
> Subject: [PATCH] qsvenc: Add adaptive_i and adaptive_b toggles

as with the other patch, this too doesnt apply cleanly anymore
can you update it ?

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[wm4]

On Thu, 26 Nov 2015 20:46:25 +0100
Nicolas George  wrote:

> Le sextidi 6 frimaire, an CCXXIV, wm4 a écrit :
> > Better do the looping internal in flvdec.c (if there's no huge number
> > of other demuxers which need this), instead of adding a new error code
> > that is also part of the public API.  
> 
> There are a few, but not many. This was my first intent, but looping in the
> framework is way more elegant and much cleaner design.

I fail to see how letting such a workaround (required for flv) leak to
common code is more elegant. It's better to keep such exceptions to the
rule inside the demuxers which violate the normal API. Or you will
eventually end up with dozens of special cases in utils.c.

> Nothing would prevent us from defining the error code in
> libavformat/internal.h if we make sure it does not leaks to the application,
> but as Michael mentioned earlier and I did in the comments for patch 2/3 in

> this series, having a public code allows to give the application
> fine-grained control over the looping, for example to maintain basic user
> interaction while a demuxer tries to find a sync word in a big corrupted
> file on a slow local medium.

Such an application will have to run the demuxer in a thread. Not
returning from the demuxer for a while because data is read is
perfectly ok because the API is inherently blocking. What are you going
to do if reading even a single packet takes milliseconds because the
data is e.g. shuffled over a slow HTTP link?

I don't foresee that more demuxers will make use of AVERROR_REDO when
no new packet data is immediately available, only to appease to
applications with broken or less than ideal IO code.

> Therefore, I maintain the proposed patch as is.

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

[FFmpeg-devel] QSV

[Michael Niedermayer]

Hi

are there any QSV patches which have been reviewed and have no
objections raised against them ?
that is patches i should apply/push for qsv ...

ivan, also it might make sense if you would be on IRC, so you and
others could discuss any QSV issues or questions or patches or ...
iam asking as it was asked on IRC a few days ago
"Nov 22 18:51:46 *  Daemon404 wonders if "Ivan Uskov" is on irc"

Thanks

-- 
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 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, wm4 a écrit :
> I fail to see how letting such a workaround (required for flv) leak to

... and a few other demuxers...

> common code is more elegant.

You fail to see, but I do, and I am not alone:
http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180681.html

> eventually end up with dozens of special cases in utils.c.

"Slippery slope" fallacy.

> Such an application will have to run the demuxer in a thread.

Can we use threads to get peace in the Middle East too? Threads seem to be
your solution to almost everything, but you fail to realize they bring a
whole lot of problems of their own, ranging from portability to trickiness
and memory consumption.

If you want to use threads everywhere, fine, by all means do so. Bu please
have the decency not to bikeshed proposals that make it easier NOT to use
them when they do not make your life easier.

> returning from the demuxer for a while because data is read is
> perfectly ok because the API is inherently blocking. What are you going
> to do if reading even a single packet takes milliseconds because the
> data is e.g. shuffled over a slow HTTP link?

Unrelated.

> I don't foresee that more demuxers will make use of AVERROR_REDO when
> no new packet data is immediately available, only to appease to
> applications with broken or less than ideal IO code.

This block is fairly unclear, but the way I understand it, it is another
"slippery slope" fallacy.

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] VS 2015 patch

[Bruce Dawson]

I don't scare that easily. :-)


On Thu, Nov 26, 2015 at 12:39 PM, compn  wrote:

> On Wed, 25 Nov 2015 11:21:55 -0800
> Bruce Dawson  wrote:
> > I'm mentioning this just in case some other lost soul shows up trying
> > to contribute a patch to a Chromium only file. I'll get my patch
> > landed in the right place.
> >
> > Thanks for the help.
>
> hope we did not scare you off.
>
> -compn
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>



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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: handle AVERROR_REDO.

[Clément Bœsch]

On Thu, Nov 26, 2015 at 07:47:53PM +0100, Nicolas George wrote:
> Signed-off-by: Nicolas George 
> ---
>  libavformat/utils.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 
> An option can be added later to grant applications fine-grained control on
> the looping, but it can not be the default as it would be an API change, and
> it probably should not be the default anyway.
> 
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index f33f2f5..d9165f1 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -680,6 +680,8 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>  av_init_packet(pkt);
>  ret = s->iformat->read_packet(s, pkt);
>  if (ret < 0) {
> +if (ret == AVERROR_REDO)
> +continue;

is the final user ever going to expect it somehow?

-- 
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] avfilter: add acompressor filter

[Moritz Barsnick]

Paul, you do have some excellent filter additions!

On Wed, Nov 25, 2015 at 16:32:13 +0100, Paul B Mahol wrote:
> +@section acompressor

Very good description, few remarks only:

> +A compressor is mainly used to reduce the dynamic of a signal.
> +Especially modern music is mostly compressed at a high ratio to
> +improve the overall loudness. It's done to get the highest attention
> +of a listener, "fatten" the sound and bring more "power" to the track.
> +If a signal is compressed too much it may sound dull or "dead"
> +afterwards or it may start to "pump" (which could be a powerful effect
> +but can also destroy a track completely).
> +The right compression is the key to reach a professional sound and is
> +the high art of mixing and mastering. Because of it's complex settings

it's -> its

> +Compression is done by detecting the volume above a chosen level
> +@code{threshold} and divide it by the factor set with @code{ratio}.

divide -> dividing (stick to the infinitive, as for "detecting")

> +So if you set the threshold to -12dB and your signal reaches -6dB a ratio
> +of 2:1 will result in a signal at -9dB. Because an exact manipulation of
> +the signal would cause distrotion of the waveform the reduction can be

distrotion -> distortion

> +levelled over the time. This is done by setting "Attack" and "Release".
> +@code{attack} determines how long the signal has to rise above the threshold
> +before any reduction will occur and @code{release} sets the time the signal
> +has to fall below the threshold to reduce the reduction again. Shorter 
> signals
> +than the chosen attack time will be left untouched.
> +The overall reduction of the signal can be made up afterwards with the
> +@code{makeup} setting. So compressing the peaks of a signal about 6dB and
> +rising the makeup to this level results in a signal two times louder than the

rising -> raising

"two times louder" (which could be read as "three times as loud") or "twice as 
loud"?

> +source. To gain a softer entry in the compression the @code{knee} flattens 
> the
> +hard edge at the threshold in the range of the chosen decibels.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item threshold
> +If a signal of second stream raises above this level it will affect the gain
> +reduction of first stream.

raises -> rises ("to rise" is active, "to be raised" is passive)

of ... stream -> of the ... stream

> +By default is 0.125. Range is between 0.00097563 and 1.

"By default is" -> "By default it is" or "The default is" or "Default
is"

> +@item ratio
> +Set a ratio about which the signal is reduced. 1:2 means that if the level

"about which" -> "by which"

> +raised 4dB above the threshold, it will be only 2dB above after the 
> reduction.
> +Default is 2. Range is between 1 and 20.

raised -> rose (active, again ;-))

> +@item attack
> +Amount of milliseconds the signal has to rise above the threshold before gain
> +reduction starts. Default is 20. Range is between 0.01 and 2000.
> +
> +@item release
> +Amount of milliseconds the signal has to fall below the threshold before
> +reduction is decreased again. Default is 250. Range is between 0.01 and 9000.
> +
> +@item makeup
> +Set the amount by how much signal will be amplified after processing.
> +Default is 2. Range is from 1 and 64.

"Amount" -> "Number" (not sure about this one, "number" seems more
correct to me personally)

> +of @code{rms}. Default is @code{rms} which is mainly smoother.

"mainly"? (i.e. its main effect) -> "mostly"? (i.e. more often?)

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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: handle AVERROR_REDO.

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, Clement Boesch a écrit :
> > An option can be added later to grant applications fine-grained control on
> > the looping, but it can not be the default as it would be an API change, and
> > it probably should not be the default anyway.

> > +if (ret == AVERROR_REDO)
> > +continue;
> is the final user ever going to expect it somehow?

The final user does not see it at all, that is the whole point.

(Unless, of course, the option I mentioned above is enabled, but if you
enable an option, hopefully you expect its consequences.)

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 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, Ronald S. Bultje a écrit :
> I +1 this design comment, I have some (...) reservations about adding
> EAGAIN2 (which is really what this is) to our public API (which is really
> what this is).

Would you care to address my arguments in your own words?

Note that the FLV demuxer (and a few others) is currently broken. To observe
it yourself is very easy: just take any big FLV file and remux or transcode
it to "-f null" with or without -an or -vn and compare the speed.

If this patch series gets bikeshedded or cold-shouldered to death, then I
wash my hands from this bug: convince me or fix it yourselves.

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 1/3] lavu/error: add AVERROR_REDO.

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, Marton Balint a écrit :
> Maybe I am missing something, but the existing error AVERROR(EINTR) cannot
> be used for this?

It would be less broken than EAGAIN, since it is almost always treated like
that by Unix code.

But without the loop in utils.c (patch 2/3 in this series), the result is as
bad or worse than EAGAIN, since the application sees it as an error code.
And this is this loop that wm4 and Ronald are bikeshedding.

But in principle, I am quite against reusing codes like that. The error
message for EINTR is "Interrupted function" or more frequently "Interrupted
system call", and nothing was interrupted, especially not a system call. The
bug in the FLV demuxer at the origin of this threads was precisely caused
because EAGAIN was reused for a slightly different semantic.

(As a side note, it seems that EINTR is already abused in a completely
different way by the MPEG-TS demuxer; so much for the fabled review
process.)

On the other hand, I can see a point for making this error code MORE
specific, for example AVERROR_DATA_DISCARDED "Data was discarded".

Also, a specific error code is the only way of providing applications the
fine-grained control that Michael suggested.

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 1/3] lavu/error: add AVERROR_REDO.

[Marton Balint]

On Fri, 27 Nov 2015, Nicolas George wrote:


Le sextidi 6 frimaire, an CCXXIV, Marton Balint a écrit :

Maybe I am missing something, but the existing error AVERROR(EINTR) cannot
be used for this?


It would be less broken than EAGAIN, since it is almost always treated like
that by Unix code.

But without the loop in utils.c (patch 2/3 in this series), the result is as
bad or worse than EAGAIN, since the application sees it as an error code.
And this is this loop that wm4 and Ronald are bikeshedding.


I agree that a loop is necessary if a demuxer can return an error with 
such semantics to maintain compatibility.



But in principle, I am quite against reusing codes like that. The error
message for EINTR is "Interrupted function" or more frequently "Interrupted
system call", and nothing was interrupted, especially not a system call.


The only reaseon for considering this if we return AVERROR(errno) directly 
from some syscalls to factorize code... probably not likely to happen, so 
ok.



The
bug in the FLV demuxer at the origin of this threads was precisely caused
because EAGAIN was reused for a slightly different semantic.

(As a side note, it seems that EINTR is already abused in a completely
different way by the MPEG-TS demuxer; so much for the fabled review
process.)


Yes, it should be fixed...


On the other hand, I can see a point for making this error code MORE
specific, for example AVERROR_DATA_DISCARDED "Data was discarded".


Okay, there are two different kind of semantics here:

1) Packets were discarded becuase the user did not want them, or they were 
empty packets (stuffing). This "error" does not indicate any problem with 
the stream.


2) Data was discarded becuase it was garbage. (sync byte lost case)
This should simply be an AVERROR_INVALIDDATA? Or we should add a new error 
code and pass it simply to the user?


Anyway, IMHO the important thing is to define/document that the error code 
you introduce is only used for the first case.


Because EAGAIN is used for both at the moment I guess and neither of these 
is EAGAIN...


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

Re: [FFmpeg-devel] [PATCH 1/3] lavu/error: add AVERROR_REDO.

[Marton Balint]

On Thu, 26 Nov 2015, Nicolas George wrote:


It is meant for demuxers to signal that they consumed data
but did not return a packet; the framework is then supposed
to loop.


Maybe I am missing something, but the existing error AVERROR(EINTR) cannot 
be used for this?


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

Re: [FFmpeg-devel] [PATCH] avfilter: add acompressor filter

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 5:17 PM, Moritz Barsnick  wrote:
> Paul, you do have some excellent filter additions!
>
> On Wed, Nov 25, 2015 at 16:32:13 +0100, Paul B Mahol wrote:
>> +@section acompressor
>
> Very good description, few remarks only:
>
>> +A compressor is mainly used to reduce the dynamic of a signal.

dynamic -> dynamic range?

Rest seems to be very thoroughly examined by Moritz.
[...]
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] lavu/error: add AVERROR_REDO.

[Nicolas George]

Le septidi 7 frimaire, an CCXXIV, Marton Balint a écrit :
> I agree that a loop is necessary if a demuxer can return an error with such
> semantics to maintain compatibility.

So basically, the question boils down to what people consider the better
design: adding a loop in every demuxer that needs it, or have the loop in
the framework.

> Okay, there are two different kind of semantics here:
> 
> 1) Packets were discarded becuase the user did not want them, or they were
> empty packets (stuffing). This "error" does not indicate any problem with
> the stream.
> 
> 2) Data was discarded becuase it was garbage. (sync byte lost case)
> This should simply be an AVERROR_INVALIDDATA? Or we should add a new error
> code and pass it simply to the user?

INVALIDDATA would be bad, because it is already used for fatal errors. We
have a big problem: error codes are just integers, and we have no API to
check "this error code is not really an error" or "this error code is fatal
/ non-fatal". Right now, applications check a few known error codes (EOF,
EAGAIN in non-blocking mode) and consider everything else fatal.

This is exactly the same pattern than classic Unix system programming: there
are a few known error codes (EINTR, EAGAIN in non-blocking mode, EINPROGRESS
for non-blocking connect(); EOF is not returned as an error), and everything
else is considered fatal.

Therefore adding new error codes makes the application developer's life
harder.

> Anyway, IMHO the important thing is to define/document that the error code
> you introduce is only used for the first case.

Right now, the application never sees the error code, since the framework
loops on it. If we introduce an option to allow the error to reach the
application, then this option is the place to document it.

> Because EAGAIN is used for both at the moment I guess and neither of these
> is EAGAIN...

I am not sure distinguishing the different cases (packet in a disabled
stream, utility data, corrupted data until a sync word) is very important.

What is bad right now is that EAGAIN means, has always meant, something else
entirely: in non-blocking mode only, data is not available right now but may
be available later. The normal reaction to EAGAIN is to sleep for a little
(or even better: to watch with poll() or similar, but we lack the API). But
if we do that for ignored streams, we end up sleeping when unnecessary, and
that causes reading FLV at ~70 packets per second instead of thousands.

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] Intel QuickSync Video

[Sevan Gelici]

Thanks to Sven Dueking I have finally solved it.

Centos 7.1 works like charm :)

2015-11-09 15:28 GMT+01:00 Sven Dueking :

>
>
> > -Ursprüngliche Nachricht-
> > Von: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] Im Auftrag
> > von Sevan Gelici
> > Gesendet: Freitag, 6. November 2015 20:35
> > An: FFmpeg development discussions and patches
> > Betreff: Re: [FFmpeg-devel] Intel QuickSync Video
> >
> > @Will Kelleher
> > CPU:
> > http://ark.intel.com/products/75122/Intel-Core-i7-4770-Processor-8M-
> > Cache-up-to-3_90-GHz
> > MediaSDK: Intel-linux-media-ocl_generic_16.4.2.1-39163_64bit.tar.gz
> > (inside package of mediaserverstudioessentials2015r6.tar.gz)
> > Ubuntu 14.04.3 LTS, Codename:   trusty
> >
> >
> >
> > @Sven Dueking
> > file to file also not working, I made a debug log
> >
> >
> > Splitting the commandline.
> > Reading option '-y' ... matched as option 'y' (overwrite output files)
> > with argument '1'.
> > Reading option '-fflags' ... matched as AVOption 'fflags' with argument
> > '+genpts'.
> > Reading option '-loglevel' ... matched as option 'loglevel' (set
> > logging
> > level) with argument 'debug'.
> > Reading option '-probesize' ... matched as AVOption 'probesize' with
> > argument '1000'.
> > Reading option '-analyzeduration' ... matched as AVOption
> > 'analyzeduration'
> > with argument '1500'.
> > Reading option '-i' ... matched as input file with argument
> > 'm84_2.mpg'.
> > Reading option '-strict' ...Routing option strict to both codec and
> > muxer layer  matched as AVOption 'strict' with argument '-2'.
> > Reading option '-dn' ... matched as option 'dn' (disable data) with
> > argument '1'.
> > Reading option '-vcodec' ... matched as option 'vcodec' (force video
> > codec ('copy' to copy stream)) with argument 'h264_qsv'.
> > Reading option '-preset' ... matched as AVOption 'preset' with argument
> > 'veryfast'.
> > Reading option '-profile:v' ... matched as option 'profile' (set
> > profile) with argument 'baseline'.
> > Reading option '-level' ... matched as AVOption 'level' with argument
> > '3.0'.
> > Reading option '-acodec' ... matched as option 'acodec' (force audio
> > codec ('copy' to copy stream)) with argument 'aac'.
> > Reading option '-b:v' ... matched as option 'b' (video bitrate (please
> > use
> > -b:v)) with argument '1000k'.
> > Reading option '-r' ... matched as option 'r' (set frame rate (Hz
> > value, fraction or abbreviation)) with argument '25'.
> > Reading option '-b:a' ... matched as option 'b' (video bitrate (please
> > use
> > -b:v)) with argument '128k'.
> > Reading option '-minrate' ... matched as AVOption 'minrate' with
> > argument '200k'.
> > Reading option '-maxrate' ... matched as AVOption 'maxrate' with
> > argument '1200k'.
> > Reading option '-bufsize' ... matched as AVOption 'bufsize' with
> > argument '1200k'.
> > Reading option '-vf' ... matched as option 'vf' (set video filters)
> > with argument 'scale=720:576'.
> > Reading option '-aspect' ... matched as option 'aspect' (set aspect
> > ratio (4:3, 16:9 or 1., 1.)) with argument '16:9'.
> > Reading option '-ar' ... matched as option 'ar' (set audio sampling
> > rate (in Hz)) with argument '48000'.
> > Reading option '-ac' ... matched as option 'ac' (set number of audio
> > channels) with argument '2'.
> > Reading option '-f' ... matched as option 'f' (force format) with
> > argument 'mpegts'.
> > Reading option 'bla.ts' ... matched as output file.
> > Finished splitting the commandline.
> > Parsing a group of options: global .
> > Applying option y (overwrite output files) with argument 1.
> > Applying option loglevel (set logging level) with argument debug.
> > Successfully parsed a group of options.
> > Parsing a group of options: input file m84_2.mpg.
> > Successfully parsed a group of options.
> > Opening an input file: m84_2.mpg.
> > [mpeg @ 0x311aa40] Format mpeg probed with size=2048 and score=26 [mpeg
> > @ 0x311aa40] Before avformat_find_stream_info() pos: 0 bytes
> > read:32768 seeks:0
> > [mpeg @ 0x311aa40] probing stream 0 pp:2500 [mpeg @ 0x311aa40] Probe
> > with size=2012, packets=1 detected mpegvideo with
> > score=25
> > [mpeg @ 0x311aa40] probed stream 0
> > [mpeg @ 0x311aa40] Probe buffer size limit of 1000 bytes reached
> > [mpeg @ 0x311aa40] rfps: 30.00 0.013228 [mpeg @ 0x311aa40] rfps:
> > 29.970030 0.00
> > Last message repeated 1 times
> > [mpeg @ 0x311aa40] rfps: 59.940060 0.00
> > Last message repeated 1 times
> > [mpeg @ 0x311aa40] After avformat_find_stream_info() pos: 0 bytes
> > read:10473616 seeks:2 frames:399
> > Input #0, mpeg, from 'm84_2.mpg':
> >   Duration: 00:00:21.99, start: 0.387500, bitrate: 6108 kb/s
> > Stream #0:0[0x1e0], 399, 1/9: Video: mpeg2video (Main), 1
> > reference frame, yuv420p(tv, bt470bg, left), 720x480 [SAR 8:9 DAR 4:3],
> > 1001/6, 6000 kb/s, 29.97 fps, 29.97 tbr, 90k tbn, 59.94 tbc
> > Successfully opened the file.
> > Parsing a group of 

Re: [FFmpeg-devel] [FFmpeg-cvslog] avfilter/vf_stack: make it possible to stop with shortest stream

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, Paul B Mahol a écrit :
> avfilter/vf_stack: make it possible to stop with shortest stream

> +in[i].after  = s->shortest ? EXT_STOP : EXT_INFINITY;

> +{ "shortest", "force termination when the shortest input terminates", 
> OFFSET(shortest), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, .flags = FLAGS },

In the long run, it would certainly be much better that all filters using
framsync expose all its modes and options in a consistent way. This is
probably possible by declaring framesync a child object of the filters and
filling its option array.

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] [FFmpeg-cvslog] avfilter/vf_stack: make it possible to stop with shortest stream

[Paul B Mahol]

On 11/26/15, Nicolas George  wrote:
> Le sextidi 6 frimaire, an CCXXIV, Paul B Mahol a ecrit :
>> avfilter/vf_stack: make it possible to stop with shortest stream
>
>> +in[i].after  = s->shortest ? EXT_STOP : EXT_INFINITY;
>
>> +{ "shortest", "force termination when the shortest input terminates",
>> OFFSET(shortest), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, .flags = FLAGS },
>
> In the long run, it would certainly be much better that all filters using
> framsync expose all its modes and options in a consistent way. This is
> probably possible by declaring framesync a child object of the filters and
> filling its option array.

Yes, child object will do. But what options to give?
The number of inputs can differ.

>
> Regards,
>
> --
>   Nicolas George
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/4] more accurate constants

[John Warburton]

On Fri, Nov 13, 2015 at 4:42 PM, Ganesh Ajjanagadde
 wrote:
>
> 4/4 is a "best effort" patch that maximizes accuracy on all platforms for
> avcodec/faandct. It results in concrete accuracy benefits on the "default" 
> x86-64
> GNU/Linux platform.
>
> Patches tested with FATE. ppc untested.

Sorry to say the libavcodec/faandct.c patch prevents compilation on a
ppc using GCC 5.2.0. In all cases (the first of which are given
below), the GCC "note" states:

"in expansion of macro 'Bn'"

...where n is the digit following B

libavcodec/faandct.c:39:12: error: initializer element is not constant
 #define B1 0.720959822006947913789091890943021267L // (cos(pi*1/16)sqrt(2))^-1
...
libavcodec/faandct.c:40:12: error: initializer element is not constant
 #define B2 0.765366864730179543456919968060797734L // (cos(pi*2/16)sqrt(2))^-1
...
libavcodec/faandct.c:41:12: error: initializer element is not constant
 #define B3 0.850430094767256448766702844371412325L // (cos(pi*3/16)sqrt(2))^-1
...
libavcodec/faandct.c:43:12: error: initializer element is not constant
 #define B5 1.272758580572833938461007018281767032L // (cos(pi*5/16)sqrt(2))^-1
...
libavcodec/faandct.c:44:12: error: initializer element is not constant
 #define B6 1.847759065022573512256366378793576574L // (cos(pi*6/16)sqrt(2))^-1
...
libavcodec/faandct.c:45:12: error: initializer element is not constant
 #define B7 3.624509785411551372409941227504289587L // (cos(pi*7/16)sqrt(2))^-1
...
kind regards,
John
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > From: Matthieu Bouron 
> [...]
> > >
> > > > +
> > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket 
> > > > *pkt, int decode)
> > > > +{
> > > > +int ret = 0;
> > > > +int got_frame = 0;
> > > > +AVFrame *frame = NULL;
> > > > +int skip_frame = codec_ctx->skip_frame;
> > > > +
> > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > +const AVCodec *codec = 
> > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > +
> > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > +if (ret < 0) {
> > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
> > > > +goto end;
> > > > +}
> > > > +}
> > > > +
> > > > +frame = av_frame_alloc();
> > > > +if (!frame) {
> > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > +}
> > > > +
> > > > +do {
> > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
> > > > +av_assert0(decode || (!decode && !got_frame));
> > > > +if (ret < 0)
> > > > +break;
> > > > +pkt->data += ret;
> > > > +pkt->size -= ret;
> > > > +
> > > > +if (got_frame) {
> > > > +break;
> > > > +}
> > > > +} while (pkt->size > 0);
> > > > +
> > > > +end:
> > > > +codec_ctx->skip_frame = skip_frame;
> > > > +
> > > > +av_frame_free();
> > > > +return ret;
> > > > +}
> > > > +
> > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
> > > > +{
> > > > +int ret = 0;
> > > > +int i, done = 0;
> > > > +AVPacket pkt;
> > > > +
> > > > +av_init_packet();
> > > > +
> > > > +while (!done) {
> > > > +AVCodecContext *codec_ctx = NULL;
> > > > +AVStream *st;
> > > > +
> > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > +codec_ctx = st->codec;
> > > > +
> > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > 
> > > > +st->codec_info_nb_frames++ > 0) {
> > > 
> > > writing into this is not ok, iam not sure it should be touched at
> > > all from API users and tests/api/* could be used as example for
> > > correct API use ...
> > 
> > This function intends to mimic avformat_find_stream_info which increments
> > this field, if it's a blocker i'll use something else.
> 
> presonally iam fine if you document that this should not be used by
> user applications and why it is used in this "regression test"
> 
> 
> > 
> > > 
> > > 
> > > > +av_packet_unref();
> > > > +continue;
> > > > +}
> > > > +
> > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > +if (ret < 0) {
> > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +av_packet_unref();
> > > > +
> > > > +/* check if all video streams have demuxed a packet */
> > > > +done = 1;
> > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > +st = fmt_ctx->streams[i];
> > > > +codec_ctx = st->codec;
> > > > +
> > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > +continue;
> > > > +
> > > > +done &= st->codec_info_nb_frames > 0;
> > > > +}
> > > > +}
> > > > +
> > > > +end:
> > > > +av_packet_unref();
> > > > +
> > > > +return ret < 0;
> > > > +}
> > > > +
> > > > +static void dump_video_streams(const AVFormatContext *fmt_ctx, int 
> > > > decode)
> > > > +{
> > > > +int i;
> > > > +
> > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > +const AVStream *st = fmt_ctx->streams[i];
> > > > +const AVCodecContext *codec_ctx = st->codec;
> > > > +const AVRational sar = codec_ctx->sample_aspect_ratio;
> > > > +
> > > > +printf("stream=%d, decode=%d\n", i, decode);
> > > > +printf("width=%d\n", codec_ctx->width);
> > > > +printf("height=%d\n", codec_ctx->height);
> > > > +printf("pix_fmt=%s\n", 
> > > > av_get_pix_fmt_name(codec_ctx->pix_fmt));
> > > > +printf("sar=%d/%d\n", sar.num, sar.den);
> > > > +  

Re: [FFmpeg-devel] [FFmpeg-cvslog] avfilter/vf_stack: make it possible to stop with shortest stream

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, Paul B Mahol a écrit :
> Yes, child object will do. But what options to give?
> The number of inputs can differ.

True, this is tricky. I had not thought of that.

I consider the options system to be too limited for many cases: escaping
hell, clumsy syntax. This is another instance. I have detailed ideas on how
it can be replaced in a compatible and incremental way, but I am so swamped
in work-related stuff these days, not mentioning long internet outages from
my ISP and giant XKCD strips, that I barely have time to make progress on
request_frame(). OTOH, I can write mails at times where I can not hack, and
I will do just that.

In the meantime, of course, users of framesync can stay that way, unless
someone has a better idea.

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] configure: add host libm capability detection

[Derek Buitenhuis]

On 11/26/2015 7:49 AM, Hendrik Leppkes wrote:
> I'm really not a fan of this change. configure is slow and complex
> enough as it is, and host tools don't need to be micro-optimized,
> since they are never run by a user.

I agree. It's opening an endless can complexity for essentially no gain.

We went down this path 2+ years ago (in Libav), and in the end it was not done.

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

Re: [FFmpeg-devel] [PATCH 0/4] more accurate constants

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 5:23 AM, John Warburton  wrote:
> On Fri, Nov 13, 2015 at 4:42 PM, Ganesh Ajjanagadde
>  wrote:
>>
>> 4/4 is a "best effort" patch that maximizes accuracy on all platforms for
>> avcodec/faandct. It results in concrete accuracy benefits on the "default" 
>> x86-64
>> GNU/Linux platform.
>>
>> Patches tested with FATE. ppc untested.
>
> Sorry to say the libavcodec/faandct.c patch prevents compilation on a
> ppc using GCC 5.2.0. In all cases (the first of which are given
> below), the GCC "note" states:
>
> "in expansion of macro 'Bn'"
>
> ...where n is the digit following B

Does removing the L fix it?

>
> libavcodec/faandct.c:39:12: error: initializer element is not constant
>  #define B1 0.720959822006947913789091890943021267L // 
> (cos(pi*1/16)sqrt(2))^-1
> ...
> libavcodec/faandct.c:40:12: error: initializer element is not constant
>  #define B2 0.765366864730179543456919968060797734L // 
> (cos(pi*2/16)sqrt(2))^-1
> ...
> libavcodec/faandct.c:41:12: error: initializer element is not constant
>  #define B3 0.850430094767256448766702844371412325L // 
> (cos(pi*3/16)sqrt(2))^-1
> ...
> libavcodec/faandct.c:43:12: error: initializer element is not constant
>  #define B5 1.272758580572833938461007018281767032L // 
> (cos(pi*5/16)sqrt(2))^-1
> ...
> libavcodec/faandct.c:44:12: error: initializer element is not constant
>  #define B6 1.847759065022573512256366378793576574L // 
> (cos(pi*6/16)sqrt(2))^-1
> ...
> libavcodec/faandct.c:45:12: error: initializer element is not constant
>  #define B7 3.624509785411551372409941227504289587L // 
> (cos(pi*7/16)sqrt(2))^-1
> ...
> kind regards,
> John
> ___
> 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] configure: add host libm capability detection

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 2:49 AM, Hendrik Leppkes  wrote:
> On Thu, Nov 26, 2015 at 6:04 AM, Ganesh Ajjanagadde
>  wrote:
>> This is needed in order to obtain what is available for hardcoded
>> table generation.
>>
>> A minimal avutil/host_libm.h is also created.
>>
>
> I'm really not a fan of this change. configure is slow and complex
> enough as it is, and host tools don't need to be micro-optimized,
> since they are never run by a user.

2 comments:
1. I deliberately kept the host_math_func list very short because I
recognized this concern regarding configure's slowness.
2. They are run by a lot of users, since tablegen is done at runtime
by default. "Never run by a user" is thus incorrect. It is not a
"micro-optimization", pow is often 4x slower than relevant function.

>
> - 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] configure: add host libm capability detection

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 6:52 AM, Derek Buitenhuis
 wrote:
> On 11/26/2015 7:49 AM, Hendrik Leppkes wrote:
>> I'm really not a fan of this change. configure is slow and complex
>> enough as it is, and host tools don't need to be micro-optimized,
>> since they are never run by a user.
>
> I agree. It's opening an endless can complexity for essentially no gain.

How does it open an "endless can of complexity"? There are not many
host funcs needed. Gain of a couple of 0.1 ms startup across various
codecs is not minimal. In that case, why even bother keeping the
"hardcoded tables" option? The complexity of that and associated
ifdefry (note: it is being done at every tablegen file, not just
localized to a avutil/host_libm) is definitely at least as great as
that of doing this change.

>
> We went down this path 2+ years ago (in Libav), and in the end it was not 
> done.
>
> - Derek
> ___
> 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 0/4] more accurate constants

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 8:07 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Nov 26, 2015 at 5:23 AM, John Warburton  
> wrote:
>> On Fri, Nov 13, 2015 at 4:42 PM, Ganesh Ajjanagadde
>>  wrote:
>>>
>>> 4/4 is a "best effort" patch that maximizes accuracy on all platforms for
>>> avcodec/faandct. It results in concrete accuracy benefits on the "default" 
>>> x86-64
>>> GNU/Linux platform.
>>>
>>> Patches tested with FATE. ppc untested.
>>
>> Sorry to say the libavcodec/faandct.c patch prevents compilation on a
>> ppc using GCC 5.2.0. In all cases (the first of which are given
>> below), the GCC "note" states:
>>
>> "in expansion of macro 'Bn'"
>>
>> ...where n is the digit following B
>
> Does removing the L fix it?

Read up a bit and suspect it should, and more generally long double is
a can of worms on ppc with bugs on some compilers:
https://gcc.gnu.org/wiki/Ieee128PowerPC, ABI in transition,
https://llvm.org/bugs/show_bug.cgi?id=11933 etc etc. Well known
projects have gone through pain:
https://github.com/numpy/numpy/issues/2001.

So basically, a good lesson for FFmpeg is not to touch long double for
at least some time to come, even though ironically it is C89.
I am pretty sure with appropriate configure flags something can be
done, but I have negative interest in hacking around upstream for such
nonsense.

I will push the removal soon unless there are further comments.

>
>>
>> libavcodec/faandct.c:39:12: error: initializer element is not constant
>>  #define B1 0.720959822006947913789091890943021267L // 
>> (cos(pi*1/16)sqrt(2))^-1
>> ...
>> libavcodec/faandct.c:40:12: error: initializer element is not constant
>>  #define B2 0.765366864730179543456919968060797734L // 
>> (cos(pi*2/16)sqrt(2))^-1
>> ...
>> libavcodec/faandct.c:41:12: error: initializer element is not constant
>>  #define B3 0.850430094767256448766702844371412325L // 
>> (cos(pi*3/16)sqrt(2))^-1
>> ...
>> libavcodec/faandct.c:43:12: error: initializer element is not constant
>>  #define B5 1.272758580572833938461007018281767032L // 
>> (cos(pi*5/16)sqrt(2))^-1
>> ...
>> libavcodec/faandct.c:44:12: error: initializer element is not constant
>>  #define B6 1.847759065022573512256366378793576574L // 
>> (cos(pi*6/16)sqrt(2))^-1
>> ...
>> libavcodec/faandct.c:45:12: error: initializer element is not constant
>>  #define B7 3.624509785411551372409941227504289587L // 
>> (cos(pi*7/16)sqrt(2))^-1
>> ...
>> kind regards,
>> John
>> ___
>> 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] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ronald S. Bultje]

Hi,

On Wed, Nov 25, 2015 at 10:46 PM, Ganesh Ajjanagadde 
wrote:

> On Wed, Nov 25, 2015 at 10:13 PM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Wed, Nov 25, 2015 at 8:48 PM, Ganesh Ajjanagadde 
> > wrote:
> >>
> >> On Wed, Nov 25, 2015 at 8:29 PM, Ganesh Ajjanagadde 
> >> wrote:
> >> > On Wed, Nov 25, 2015 at 8:19 PM, Ronald S. Bultje  >
> >> > wrote:
> >> >> Hi,
> >> >>
> >> >> On Wed, Nov 25, 2015 at 7:36 PM, Ganesh Ajjanagadde <
> gajja...@mit.edu>
> >> >> wrote:
> >> >>
> >> >>> On Wed, Nov 25, 2015 at 6:49 PM, James Almer 
> >> >>> wrote:
> >> >>> > On 11/25/2015 8:32 PM, Ganesh Ajjanagadde wrote:
> >> >>> >> On Wed, Nov 25, 2015 at 6:19 PM, Ronald S. Bultje
> >> >>> >> 
> >> >>> wrote:
> >> >>> >>> Hi,
> >> >>> >>>
> >> >>> >>> On Wed, Nov 25, 2015 at 5:17 PM, Ganesh Ajjanagadde <
> >> >>> gajjanaga...@gmail.com>
> >> >>> >>> wrote:
> >> >>> 
> >> >>>  On systems having cbrt, there is no reason to use the slow pow
> >> >>> function.
> >> >>> 
> >> >>>  Sample benchmark (x86-64, Haswell, GNU/Linux):
> >> >>>  new:
> >> >>>  5124920 decicycles in cbrt_tableinit,   1 runs,  0
> skips
> >> >>> 
> >> >>>  old:
> >> >>>  12321680 decicycles in cbrt_tableinit,   1 runs,  0
> skips
> >> >>> 
> >> >>>  Signed-off-by: Ganesh Ajjanagadde 
> >> >>> 
> >> >>> 
> >> >>>
> >> >>>
> ---
> >> >>>  What I wonder about is why --enable-hardcoded-tables is not the
> >> >>> default
> >> >>>  for
> >> >>>  FFmpeg. Unless I am missing something, static storage is anyway
> >> >>> allocated
> >> >>>  even
> >> >>>  if hardcoded tables are not used, and the cost is deferred to
> >> >>>  runtime
> >> >>>  instead of
> >> >>>  build time. Thus binary size should not be affected, but users
> >> >>>  burn
> >> >>> cycles
> >> >>>  unnecessarily for every codec having these kinds of tables. I
> >> >>>  have
> >> >>> these
> >> >>>  patches,
> >> >>>  simply because at the moment users are paying a price for the
> >> >>>  typical
> >> >>>  default.
> >> >>>  ---
> >> >>>   libavcodec/cbrt_tablegen.h | 6 +++---
> >> >>>   1 file changed, 3 insertions(+), 3 deletions(-)
> >> >>> >>>
> >> >>> >>>
> >> >>> >>> This has been discussed extensively in the past...
> >> >>> >>
> >> >>> >> Can you please give a link and/or timeframe to search for?
> >> >>> >>
> >> >>> >>>
> >> >>> >>> As for the patch, don't forget that tablegen runs on the host
> >> >>> >>> (build),
> >> >>> not
> >> >>> >>> target (runtime), whereas libm.h is for target (runtime) and may
> >> >>> >>> not be
> >> >>> >>> compatible. I believe that's why we don't use libm.h in tablegen
> >> >>> >>> files.
> >> >>> >>
> >> >>> >> I don't understand this, it seems to me like any other code (at
> >> >>> >> least
> >> >>> >> in the default configure), it gets called, and like all other
> such
> >> >>> >> things, we use libavutil/libm for hackery. This host/target
> >> >>> >> business
> >> >>> >> affects other things as well. What is the issue?
> >> >>> >
> >> >>> > libavutil/libm.h uses defines from config.h, which are based on
> the
> >> >>> tests run
> >> >>> > by configure for the target, and not the host where compilation
> >> >>> > takes
> >> >>> place.
> >> >>> > The tablegen applications all run at compile time. What is
> available
> >> >>> > on
> >> >>> the
> >> >>> > target may not be on the host.
> >> >>>
> >> >>> Ok. So I would like an answer to two simple questions that are
> outside
> >> >>> my knowledge or interest.
> >> >>>
> >> >>> Is it possible with some hackery to get this change through, or not?
> >> >>> If so, what is it?
> >> >>
> >> >>
> >> >> You need to understand the issue before you can evaluate hacks.
> >> >>
> >> >> The issue is:
> >> >> - I'm using a linux x86-64 machine using gcc as a compiler, with
> >> >> libc=glibc
> >> >> 2.18 (A);
> >> >> - to build a binary that will run on a Windows Mobile ARMv7 machine,
> >> >> with
> >> >> libC=something-from-Microsoft (B).
> >> >>
> >> >> tablegen runs on A, but ffmpeg.exe runs on B. libavutil/libm.h only
> >> >> works
> >> >> for B. If you want a version of libm.h on A, you need to generate a
> >> >> version
> >> >> of libm.h that works on A. There is no relationship between A and B,
> >> >> and
> >> >> thus there can not possibly ever be any relationship between A's
> libm.h
> >> >> and
> >> >> B's libavutil/libm.h.
> >> >>
> >> >> It's probably possible to generate a version of libm.h for A, but
> >> >> that's
> >> >> not so much a coding issue, as it is an issue of understanding the
> >> >> build
> >> >> system and including detection for stuff on machine A, as opposed to
> >> >> machine B (which is what most of configure does).

Re: [FFmpeg-devel] [PATCH] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ronald S. Bultje]

Hi,

On Wed, Nov 25, 2015 at 8:29 PM, Ganesh Ajjanagadde 
wrote:

> On Wed, Nov 25, 2015 at 8:19 PM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Wed, Nov 25, 2015 at 7:36 PM, Ganesh Ajjanagadde 
> > wrote:
> >
> >> On Wed, Nov 25, 2015 at 6:49 PM, James Almer  wrote:
> >> > On 11/25/2015 8:32 PM, Ganesh Ajjanagadde wrote:
> >> >> On Wed, Nov 25, 2015 at 6:19 PM, Ronald S. Bultje <
> rsbul...@gmail.com>
> >> wrote:
> >> >>> Hi,
> >> >>>
> >> >>> On Wed, Nov 25, 2015 at 5:17 PM, Ganesh Ajjanagadde <
> >> gajjanaga...@gmail.com>
> >> >>> wrote:
> >> 
> >>  On systems having cbrt, there is no reason to use the slow pow
> >> function.
> >> 
> >>  Sample benchmark (x86-64, Haswell, GNU/Linux):
> >>  new:
> >>  5124920 decicycles in cbrt_tableinit,   1 runs,  0 skips
> >> 
> >>  old:
> >>  12321680 decicycles in cbrt_tableinit,   1 runs,  0 skips
> >> 
> >>  Signed-off-by: Ganesh Ajjanagadde 
> >> 
> >> 
> >>
> ---
> >>  What I wonder about is why --enable-hardcoded-tables is not the
> >> default
> >>  for
> >>  FFmpeg. Unless I am missing something, static storage is anyway
> >> allocated
> >>  even
> >>  if hardcoded tables are not used, and the cost is deferred to
> runtime
> >>  instead of
> >>  build time. Thus binary size should not be affected, but users burn
> >> cycles
> >>  unnecessarily for every codec having these kinds of tables. I have
> >> these
> >>  patches,
> >>  simply because at the moment users are paying a price for the
> typical
> >>  default.
> >>  ---
> >>   libavcodec/cbrt_tablegen.h | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >> >>>
> >> >>>
> >> >>> This has been discussed extensively in the past...
> >> >>
> >> >> Can you please give a link and/or timeframe to search for?
> >> >>
> >> >>>
> >> >>> As for the patch, don't forget that tablegen runs on the host
> (build),
> >> not
> >> >>> target (runtime), whereas libm.h is for target (runtime) and may
> not be
> >> >>> compatible. I believe that's why we don't use libm.h in tablegen
> files.
> >> >>
> >> >> I don't understand this, it seems to me like any other code (at least
> >> >> in the default configure), it gets called, and like all other such
> >> >> things, we use libavutil/libm for hackery. This host/target business
> >> >> affects other things as well. What is the issue?
> >> >
> >> > libavutil/libm.h uses defines from config.h, which are based on the
> >> tests run
> >> > by configure for the target, and not the host where compilation takes
> >> place.
> >> > The tablegen applications all run at compile time. What is available
> on
> >> the
> >> > target may not be on the host.
> >>
> >> Ok. So I would like an answer to two simple questions that are outside
> >> my knowledge or interest.
> >>
> >> Is it possible with some hackery to get this change through, or not?
> >> If so, what is it?
> >
> >
> > You need to understand the issue before you can evaluate hacks.
> >
> > The issue is:
> > - I'm using a linux x86-64 machine using gcc as a compiler, with
> libc=glibc
> > 2.18 (A);
> > - to build a binary that will run on a Windows Mobile ARMv7 machine, with
> > libC=something-from-Microsoft (B).
> >
> > tablegen runs on A, but ffmpeg.exe runs on B. libavutil/libm.h only works
> > for B. If you want a version of libm.h on A, you need to generate a
> version
> > of libm.h that works on A. There is no relationship between A and B, and
> > thus there can not possibly ever be any relationship between A's libm.h
> and
> > B's libavutil/libm.h.
> >
> > It's probably possible to generate a version of libm.h for A, but that's
> > not so much a coding issue, as it is an issue of understanding the build
> > system and including detection for stuff on machine A, as opposed to
> > machine B (which is what most of configure does).
>
> Thanks a lot for the detail. So how about using a local
> #ifndef cbrt
> #define cbrt(x) pow(x, 1 / 3.0)
> code...
> #undef cbrt // at the very end of the file
> #endif


If you really want to pursue this, I would propose something like this
(this is open for discussion):
- create a tablegen common header file (if it doesn't exist already), e.g.
libavutil/tablegen.h or compat/tablegen.h;
- add some code like this to this header file:

#include "config.h"
#if CONFIG_HARDCODED_TABLES
// we don't have cbrt on all host platforms, and we don't care about
performance since it's performed during build-time
#define cbrt(x) pow(x, 1.0 / 3)
// other defines as currently existing in various files
#else
#include "libavutil/libm.h"
#endif

Include this in the relevant file. If building hardcoded tables, everything
works as it does now. If building runtime-generated tables, it'll use the
target tools 

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: improve accuracy and speed of gain computation

[Paul B Mahol]

You gonna apply this? or?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: improve accuracy and speed of gain computation

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 9:05 AM, Paul B Mahol  wrote:
> You gonna apply this? or?

I am, later today (pushing with some other stuff).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 8:47 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Wed, Nov 25, 2015 at 8:29 PM, Ganesh Ajjanagadde 
> wrote:
>
>> On Wed, Nov 25, 2015 at 8:19 PM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Wed, Nov 25, 2015 at 7:36 PM, Ganesh Ajjanagadde 
>> > wrote:
>> >
>> >> On Wed, Nov 25, 2015 at 6:49 PM, James Almer  wrote:
>> >> > On 11/25/2015 8:32 PM, Ganesh Ajjanagadde wrote:
>> >> >> On Wed, Nov 25, 2015 at 6:19 PM, Ronald S. Bultje <
>> rsbul...@gmail.com>
>> >> wrote:
>> >> >>> Hi,
>> >> >>>
>> >> >>> On Wed, Nov 25, 2015 at 5:17 PM, Ganesh Ajjanagadde <
>> >> gajjanaga...@gmail.com>
>> >> >>> wrote:
>> >> 
>> >>  On systems having cbrt, there is no reason to use the slow pow
>> >> function.
>> >> 
>> >>  Sample benchmark (x86-64, Haswell, GNU/Linux):
>> >>  new:
>> >>  5124920 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> >> 
>> >>  old:
>> >>  12321680 decicycles in cbrt_tableinit,   1 runs,  0 skips
>> >> 
>> >>  Signed-off-by: Ganesh Ajjanagadde 
>> >> 
>> >> 
>> >>
>> ---
>> >>  What I wonder about is why --enable-hardcoded-tables is not the
>> >> default
>> >>  for
>> >>  FFmpeg. Unless I am missing something, static storage is anyway
>> >> allocated
>> >>  even
>> >>  if hardcoded tables are not used, and the cost is deferred to
>> runtime
>> >>  instead of
>> >>  build time. Thus binary size should not be affected, but users burn
>> >> cycles
>> >>  unnecessarily for every codec having these kinds of tables. I have
>> >> these
>> >>  patches,
>> >>  simply because at the moment users are paying a price for the
>> typical
>> >>  default.
>> >>  ---
>> >>   libavcodec/cbrt_tablegen.h | 6 +++---
>> >>   1 file changed, 3 insertions(+), 3 deletions(-)
>> >> >>>
>> >> >>>
>> >> >>> This has been discussed extensively in the past...
>> >> >>
>> >> >> Can you please give a link and/or timeframe to search for?
>> >> >>
>> >> >>>
>> >> >>> As for the patch, don't forget that tablegen runs on the host
>> (build),
>> >> not
>> >> >>> target (runtime), whereas libm.h is for target (runtime) and may
>> not be
>> >> >>> compatible. I believe that's why we don't use libm.h in tablegen
>> files.
>> >> >>
>> >> >> I don't understand this, it seems to me like any other code (at least
>> >> >> in the default configure), it gets called, and like all other such
>> >> >> things, we use libavutil/libm for hackery. This host/target business
>> >> >> affects other things as well. What is the issue?
>> >> >
>> >> > libavutil/libm.h uses defines from config.h, which are based on the
>> >> tests run
>> >> > by configure for the target, and not the host where compilation takes
>> >> place.
>> >> > The tablegen applications all run at compile time. What is available
>> on
>> >> the
>> >> > target may not be on the host.
>> >>
>> >> Ok. So I would like an answer to two simple questions that are outside
>> >> my knowledge or interest.
>> >>
>> >> Is it possible with some hackery to get this change through, or not?
>> >> If so, what is it?
>> >
>> >
>> > You need to understand the issue before you can evaluate hacks.
>> >
>> > The issue is:
>> > - I'm using a linux x86-64 machine using gcc as a compiler, with
>> libc=glibc
>> > 2.18 (A);
>> > - to build a binary that will run on a Windows Mobile ARMv7 machine, with
>> > libC=something-from-Microsoft (B).
>> >
>> > tablegen runs on A, but ffmpeg.exe runs on B. libavutil/libm.h only works
>> > for B. If you want a version of libm.h on A, you need to generate a
>> version
>> > of libm.h that works on A. There is no relationship between A and B, and
>> > thus there can not possibly ever be any relationship between A's libm.h
>> and
>> > B's libavutil/libm.h.
>> >
>> > It's probably possible to generate a version of libm.h for A, but that's
>> > not so much a coding issue, as it is an issue of understanding the build
>> > system and including detection for stuff on machine A, as opposed to
>> > machine B (which is what most of configure does).
>>
>> Thanks a lot for the detail. So how about using a local
>> #ifndef cbrt
>> #define cbrt(x) pow(x, 1 / 3.0)
>> code...
>> #undef cbrt // at the very end of the file
>> #endif
>
>
> If you really want to pursue this, I would propose something like this
> (this is open for discussion):
> - create a tablegen common header file (if it doesn't exist already), e.g.
> libavutil/tablegen.h or compat/tablegen.h;
> - add some code like this to this header file:
>
> #include "config.h"
> #if CONFIG_HARDCODED_TABLES
> // we don't have cbrt on all host platforms, and we don't care about
> performance since it's performed during build-time
> #define cbrt(x) pow(x, 1.0 / 3)
> // other defines as currently 

Re: [FFmpeg-devel] [PATCH 2/2] fate/concatdec: Use -bitexact

[Michael Niedermayer]

On Wed, Nov 25, 2015 at 09:35:07PM -0800, Timothy Gu wrote:
> Fixes FATE failures on --enable-small builds.
> ---
>  tests/fate-run.sh   | 4 ++--
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf | 2 +-
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +-
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf  | 4 ++--
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10  | 4 ++--
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts   | 4 ++--
>  6 files changed, 10 insertions(+), 10 deletions(-)

this or a patch disabling the test on config-small is ok

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


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

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: improve accuracy and speed of gain computation

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 9:10 AM, Ganesh Ajjanagadde
 wrote:
> On Thu, Nov 26, 2015 at 9:05 AM, Paul B Mahol  wrote:
>> You gonna apply this? or?
>
> I am, later today (pushing with some other stuff).

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

Re: [FFmpeg-devel] [PATCHv2] avutil/lls: speed up performance of solve_lls

[Ganesh Ajjanagadde]

On Wed, Nov 25, 2015 at 6:29 AM, Michael Niedermayer  wrote:
> On Tue, Nov 24, 2015 at 10:13:22PM -0500, Ganesh Ajjanagadde wrote:
>> This is a trivial rewrite of the loops that results in better
>> prefetching and associated cache efficiency. Essentially, the problem is
>> that modern prefetching logic is based on finite state Markov memory, a 
>> reasonable
>> assumption that is used elsewhere in CPU's in for instance branch
>> predictors.
>>
>> Surrounding loops all iterate forward through the array, making the
>> predictor think of prefetching in the forward direction, but the
>> intermediate loop is unnecessarily in the backward direction.
>>
>> Speedup is nontrivial. Benchmarks obtained by 10^6 iterations within
>> solve_lls, with START/STOP_TIMER. File is 
>> tests/data/fate/flac-16-lpc-cholesky.err.
>> Hardware: x86-64, Haswell, GNU/Linux.
>>
>> new:
>>   17291 decicycles in solve_lls, 2096706 runs,446 skips
>>   17255 decicycles in solve_lls, 4193657 runs,647 skips
>>   17231 decicycles in solve_lls, 8384997 runs,   3611 skips
>>   17189 decicycles in solve_lls,16771010 runs,   6206 skips
>>   17132 decicycles in solve_lls,33544757 runs,   9675 skips
>>   17092 decicycles in solve_lls,67092404 runs,  16460 skips
>>   17058 decicycles in solve_lls,134188213 runs,  29515 skips
>>
>> old:
>>   18009 decicycles in solve_lls, 2096665 runs,487 skips
>>   17805 decicycles in solve_lls, 4193320 runs,984 skips
>>   17779 decicycles in solve_lls, 8386855 runs,   1753 skips
>>   18289 decicycles in solve_lls,16774280 runs,   2936 skips
>>   18158 decicycles in solve_lls,33548104 runs,   6328 skips
>>   18420 decicycles in solve_lls,67091793 runs,  17071 skips
>>   18310 decicycles in solve_lls,134187219 runs,  30509 skips
>>
>> Reviewed-by: Michael Niedermayer 
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavutil/lls.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> LGTM
>
> thx

pushed, thanks

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
>
> ___
> 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/10] avfilter/vsrc_mptestsrc: use hypot()

[Ganesh Ajjanagadde]

On Wed, Nov 25, 2015 at 12:54 PM, Michael Niedermayer  wrote:
> On Sun, Nov 22, 2015 at 12:05:50PM -0500, Ganesh Ajjanagadde wrote:
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/vsrc_mptestsrc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> should be ok
>
> thx

pushed, thanks.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> In a rich man's house there is no place to spit but his face.
> -- Diogenes of Sinope
>
> ___
> 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] avfilter/af_dynaudnorm: remove wasteful pow

[Ganesh Ajjanagadde]

On Wed, Nov 25, 2015 at 4:16 AM, Paul B Mahol  wrote:
> On 11/25/15, Ganesh Ajjanagadde  wrote:
>> This removes wasteful pow(x, 2.0) that although not terribly important
>> for speed, is still useless.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> --
>> Observed while auditing the codebase for wasteful pow calls.
>>
>> ---
>>  libavfilter/af_dynaudnorm.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavfilter/af_dynaudnorm.c b/libavfilter/af_dynaudnorm.c
>> index 62a2653..5f412f5 100644
>> --- a/libavfilter/af_dynaudnorm.c
>> +++ b/libavfilter/af_dynaudnorm.c
>> @@ -237,13 +237,13 @@ static void
>> init_gaussian_filter(DynamicAudioNormalizerContext *s)
>>  // Pre-compute constants
>>  const int offset = s->filter_size / 2;
>>  const double c1 = 1.0 / (sigma * sqrt(2.0 * M_PI));
>> -const double c2 = 2.0 * pow(sigma, 2.0);
>> +const double c2 = 2.0 * sigma * sigma;
>>
>>  // Compute weights
>>  for (i = 0; i < s->filter_size; i++) {
>>  const int x = i - offset;
>>
>> -s->weights[i] = c1 * exp(-(pow(x, 2.0) / c2));
>> +s->weights[i] = c1 * exp(-x * x / c2);
>>  total_weight += s->weights[i];
>>  }
>>
>> --
>> 2.6.2
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> probably ok

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

Re: [FFmpeg-devel] [PATCH] configure: add host libm capability detection

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 8:50 AM, Hendrik Leppkes  wrote:
> On Thu, Nov 26, 2015 at 2:10 PM, Ganesh Ajjanagadde  wrote:
>> On Thu, Nov 26, 2015 at 2:49 AM, Hendrik Leppkes  wrote:
>>> On Thu, Nov 26, 2015 at 6:04 AM, Ganesh Ajjanagadde
>>>  wrote:
 This is needed in order to obtain what is available for hardcoded
 table generation.

 A minimal avutil/host_libm.h is also created.

>>>
>>> I'm really not a fan of this change. configure is slow and complex
>>> enough as it is, and host tools don't need to be micro-optimized,
>>> since they are never run by a user.
>>
>> 2 comments:
>> 1. I deliberately kept the host_math_func list very short because I
>> recognized this concern regarding configure's slowness.
>> 2. They are run by a lot of users, since tablegen is done at runtime
>> by default. "Never run by a user" is thus incorrect. It is not a
>> "micro-optimization", pow is often 4x slower than relevant function.
>>
>
> These things are for host code, that is not run by users.

I see what you mean now, alternative solution by Ronald is far
superior. Patch dropped, thanks all.

>
> - 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] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Michael Niedermayer]

On Thu, Nov 26, 2015 at 11:57:46AM +0100, Matthieu Bouron wrote:
> On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> > On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > > From: Matthieu Bouron 
> > [...]
> > > >
> > > > > +
> > > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, 
> > > > > AVPacket *pkt, int decode)
> > > > > +{
> > > > > +int ret = 0;
> > > > > +int got_frame = 0;
> > > > > +AVFrame *frame = NULL;
> > > > > +int skip_frame = codec_ctx->skip_frame;
> > > > > +
> > > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > > +const AVCodec *codec = 
> > > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > > +
> > > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > > +if (ret < 0) {
> > > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open 
> > > > > codec\n");
> > > > > +goto end;
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +frame = av_frame_alloc();
> > > > > +if (!frame) {
> > > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > > +}
> > > > > +
> > > > > +do {
> > > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, 
> > > > > pkt);
> > > > > +av_assert0(decode || (!decode && !got_frame));
> > > > > +if (ret < 0)
> > > > > +break;
> > > > > +pkt->data += ret;
> > > > > +pkt->size -= ret;
> > > > > +
> > > > > +if (got_frame) {
> > > > > +break;
> > > > > +}
> > > > > +} while (pkt->size > 0);
> > > > > +
> > > > > +end:
> > > > > +codec_ctx->skip_frame = skip_frame;
> > > > > +
> > > > > +av_frame_free();
> > > > > +return ret;
> > > > > +}
> > > > > +
> > > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int 
> > > > > decode)
> > > > > +{
> > > > > +int ret = 0;
> > > > > +int i, done = 0;
> > > > > +AVPacket pkt;
> > > > > +
> > > > > +av_init_packet();
> > > > > +
> > > > > +while (!done) {
> > > > > +AVCodecContext *codec_ctx = NULL;
> > > > > +AVStream *st;
> > > > > +
> > > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > > +codec_ctx = st->codec;
> > > > > +
> > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > > 
> > > > > +st->codec_info_nb_frames++ > 0) {
> > > > 
> > > > writing into this is not ok, iam not sure it should be touched at
> > > > all from API users and tests/api/* could be used as example for
> > > > correct API use ...
> > > 
> > > This function intends to mimic avformat_find_stream_info which increments
> > > this field, if it's a blocker i'll use something else.
> > 
> > presonally iam fine if you document that this should not be used by
> > user applications and why it is used in this "regression test"
> > 
> > 
> > > 
> > > > 
> > > > 
> > > > > +av_packet_unref();
> > > > > +continue;
> > > > > +}
> > > > > +
> > > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > > +if (ret < 0) {
> > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > > frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +av_packet_unref();
> > > > > +
> > > > > +/* check if all video streams have demuxed a packet */
> > > > > +done = 1;
> > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > +st = fmt_ctx->streams[i];
> > > > > +codec_ctx = st->codec;
> > > > > +
> > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > > +continue;
> > > > > +
> > > > > +done &= st->codec_info_nb_frames > 0;
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +end:
> > > > > +av_packet_unref();
> > > > > +
> > > > > +return ret < 0;
> > > > > +}
> > > > > +
> > > > > +static void dump_video_streams(const AVFormatContext *fmt_ctx, int 
> > > > > decode)
> > > > > +{
> > > > > +int i;
> > > > > +
> > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > +const AVStream *st = fmt_ctx->streams[i];
> > > > > +const AVCodecContext *codec_ctx = st->codec;
> > > > > +const AVRational sar = 

[FFmpeg-devel] [PATCHv2 1/3] avutil/tablegen: add tablegen compatibility hacks

[Ganesh Ajjanagadde]

Based on a suggestion by Ronald S. Bultje.

Signed-off-by: Ganesh Ajjanagadde 
---
 libavutil/tablegen.h | 32 
 1 file changed, 32 insertions(+)
 create mode 100644 libavutil/tablegen.h

diff --git a/libavutil/tablegen.h b/libavutil/tablegen.h
new file mode 100644
index 000..057db2a
--- /dev/null
+++ b/libavutil/tablegen.h
@@ -0,0 +1,32 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Compatibility libm for table generation files
+ */
+
+#ifndef AVUTIL_TABLEGEN_H
+#define AVUTIL_TABLEGEN_H
+
+// we lack some functions on all host platforms, and we don't care about
+// performance as it's performed at build time
+#define cbrt(x) pow(x, 1.0 / 3)
+#define llrint(x) floor((x) + 0.5)
+
+#endif /* AVUTIL_TABLEGEN_H */
-- 
2.6.2

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

[FFmpeg-devel] [PATCHv2 2/3] avcodec/mpegaudio_tablegen: speed up table generation

[Ganesh Ajjanagadde]

This does some miscellaneous stuff mainly avoiding the usage of pow to
achieve significant speedups. This is not speed critical, but is
unnecessary latency and cycles wasted for a user.

All tables tested and are identical to the old ones
(bit-exact even in floating point case).

Sample benchmark (x86-64, Haswell, GNU/Linux):
old:
102329530 decicycles in mpegaudio_tableinit,   1 runs,  0 skips

new:
34111900 decicycles in mpegaudio_tableinit,   1 runs,  0 skips

Signed-off-by: Ganesh Ajjanagadde 
---
 libavcodec/mpegaudio_tablegen.h | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/libavcodec/mpegaudio_tablegen.h b/libavcodec/mpegaudio_tablegen.h
index 86b2cd3..6cd5ad6 100644
--- a/libavcodec/mpegaudio_tablegen.h
+++ b/libavcodec/mpegaudio_tablegen.h
@@ -29,9 +29,11 @@
 
 #define TABLE_4_3_SIZE (8191 + 16)*4
 #if CONFIG_HARDCODED_TABLES
+#include "libavutil/tablegen.h"
 #define mpegaudio_tableinit()
 #include "libavcodec/mpegaudio_tables.h"
 #else
+#include "libavutil/libm.h"
 static int8_t   table_4_3_exp[TABLE_4_3_SIZE];
 static uint32_t table_4_3_value[TABLE_4_3_SIZE];
 static uint32_t exp_table_fixed[512];
@@ -45,12 +47,21 @@ static float expval_table_float[512][16];
 static av_cold void mpegaudio_tableinit(void)
 {
 int i, value, exponent;
+double pow2_lut[4] = {
+1., /* 2 ^ (0 * 0.25) */
+1.18920711500272106672, /* 2 ^ (1 * 0.25) */
+M_SQRT2   , /* 2 ^ (2 * 0.25) */
+1.68179283050742908606, /* 2 ^ (3 * 0.25) */
+};
+double cbrt_lut[16];
+for (i = 0; i < 16; ++i)
+cbrt_lut[i] = cbrt(i);
+
 for (i = 1; i < TABLE_4_3_SIZE; i++) {
 double value = i / 4;
 double f, fm;
 int e, m;
-/* cbrtf() isn't available on all systems, so we use powf(). */
-f  = value / IMDCT_SCALAR * pow(value, 1.0 / 3.0) * pow(2, (i & 3) * 
0.25);
+f  = value / IMDCT_SCALAR * cbrt(value) * pow2_lut[i & 3];
 fm = frexp(f, );
 m  = (uint32_t)(fm * (1LL << 31) + 0.5);
 e += FRAC_BITS - 31 + 5 - 100;
@@ -61,10 +72,8 @@ static av_cold void mpegaudio_tableinit(void)
 }
 for (exponent = 0; exponent < 512; exponent++) {
 for (value = 0; value < 16; value++) {
-/* cbrtf() isn't available on all systems, so we use powf(). */
-double f = (double)value * pow(value, 1.0 / 3.0) * pow(2, 
(exponent - 400) * 0.25 + FRAC_BITS + 5) / IMDCT_SCALAR;
-/* llrint() isn't always available, so round and cast manually. */
-expval_table_fixed[exponent][value] = (long long int) (f < 
0x ? floor(f + 0.5) : 0x);
+double f = value * cbrt_lut[value] * pow(2, (exponent - 400) * 
0.25 + FRAC_BITS + 5) / IMDCT_SCALAR;
+expval_table_fixed[exponent][value] = (f < 0x ? llrint(f) 
: 0x);
 expval_table_float[exponent][value] = f;
 }
 exp_table_fixed[exponent] = expval_table_fixed[exponent][1];
-- 
2.6.2

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

[FFmpeg-devel] [PATCHv3 3/3] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ganesh Ajjanagadde]

On systems having cbrt, there is no reason to use the slow pow function.

Sample benchmark (x86-64, Haswell, GNU/Linux):
new:
5124920 decicycles in cbrt_tableinit,   1 runs,  0 skips

old:
12321680 decicycles in cbrt_tableinit,   1 runs,  0 skips

Signed-off-by: Ganesh Ajjanagadde 
---
 libavcodec/cbrt_tablegen.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/cbrt_tablegen.h b/libavcodec/cbrt_tablegen.h
index 27a3e3a..4e90ac1 100644
--- a/libavcodec/cbrt_tablegen.h
+++ b/libavcodec/cbrt_tablegen.h
@@ -26,15 +26,17 @@
 #include 
 #include 
 #include "libavutil/attributes.h"
+#include "libavutil/tablegen.h"
 #include "libavcodec/aac_defines.h"
 
 #if USE_FIXED
-#define CBRT(x) (int)floor((x).f * 8192 + 0.5)
+#define CBRT(x) lrint((x).f * 8192)
 #else
 #define CBRT(x) x.i
 #endif
 
 #if CONFIG_HARDCODED_TABLES
+#include "libavutil/tablegen.h"
 #if USE_FIXED
 #define cbrt_tableinit_fixed()
 #include "libavcodec/cbrt_fixed_tables.h"
@@ -43,19 +45,19 @@
 #include "libavcodec/cbrt_tables.h"
 #endif
 #else
+#include "libavutil/libm.h"
 static uint32_t cbrt_tab[1 << 13];
 
 static av_cold void AAC_RENAME(cbrt_tableinit)(void)
 {
 if (!cbrt_tab[(1<<13) - 1]) {
 int i;
-/* cbrtf() isn't available on all systems, so we use powf(). */
 for (i = 0; i < 1<<13; i++) {
 union {
 float f;
 uint32_t i;
 } f;
-f.f = pow(i, 1.0 / 3.0) * i;
+f.f = cbrt(i) * i;
 cbrt_tab[i] = CBRT(f);
 }
 }
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCHv2 2/3] avcodec/mpegaudio_tablegen: speed up table generation

[Ronald S. Bultje]

Hi,

On Thu, Nov 26, 2015 at 10:23 AM, Ganesh Ajjanagadde  wrote:

> This does some miscellaneous stuff mainly avoiding the usage of pow to
> achieve significant speedups. This is not speed critical, but is
> unnecessary latency and cycles wasted for a user.
>
> All tables tested and are identical to the old ones
> (bit-exact even in floating point case).
>
> Sample benchmark (x86-64, Haswell, GNU/Linux):
> old:
> 102329530 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>
> new:
> 34111900 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavcodec/mpegaudio_tablegen.h | 21 +++--
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/mpegaudio_tablegen.h
> b/libavcodec/mpegaudio_tablegen.h
> index 86b2cd3..6cd5ad6 100644
> --- a/libavcodec/mpegaudio_tablegen.h
> +++ b/libavcodec/mpegaudio_tablegen.h
> @@ -29,9 +29,11 @@
>
>  #define TABLE_4_3_SIZE (8191 + 16)*4
>  #if CONFIG_HARDCODED_TABLES
> +#include "libavutil/tablegen.h"
>  #define mpegaudio_tableinit()
>  #include "libavcodec/mpegaudio_tables.h"
>  #else
> +#include "libavutil/libm.h"
>

I don't think this works yet. Check mpegaudio_tablegen.c (it should be
obvious why it does this):

#include 
#define CONFIG_HARDCODED_TABLES 0
#include "mpegaudio_tablegen.h"
#include "tableprint.h"

Note that this is the table generation which runs on the host platform. So,
the location where you add this, is important. I would recommend including
tablegen.h inside mpegaudio_tablegen.c, and then including libavutil/libm.h
outside the scope of this header, e.g. in users of this header, if
tablegen.h wasn't included. Make sure make checkheaders passes after your
changes, and the build works with as well as without hardcoded_tables.

 static int8_t   table_4_3_exp[TABLE_4_3_SIZE];
>  static uint32_t table_4_3_value[TABLE_4_3_SIZE];
>  static uint32_t exp_table_fixed[512];
> @@ -45,12 +47,21 @@ static float expval_table_float[512][16];
>  static av_cold void mpegaudio_tableinit(void)
>  {
>  int i, value, exponent;
> +double pow2_lut[4] = {
>

I think somebody else already mentioned exp2. I'd call this table exp2,
since table[idx] corresponds to exp2(idx * constant).


> +double f = value * cbrt_lut[value] * pow(2, (exponent - 400)
> * 0.25 + FRAC_BITS + 5) / IMDCT_SCALAR;


exp2 here also. For compatibility, you can #define exp2(x) to pow(2, x) in
libavutil/tablegen.h.

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

Re: [FFmpeg-devel] [PATCHv3 3/3] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ronald S. Bultje]

Hi,

On Thu, Nov 26, 2015 at 10:23 AM, Ganesh Ajjanagadde  wrote:

>  #if CONFIG_HARDCODED_TABLES
> +#include "libavutil/tablegen.h"
>  #if USE_FIXED
>  #define cbrt_tableinit_fixed()
>  #include "libavcodec/cbrt_fixed_tables.h"
> @@ -43,19 +45,19 @@
>  #include "libavcodec/cbrt_tables.h"
>  #endif
>  #else
> +#include "libavutil/libm.h"
>  static uint32_t cbrt_tab[1 << 13];


In cbrt_tablegen_template.c:

#include 
#define CONFIG_HARDCODED_TABLES 0
#include "cbrt_tablegen.h"
#include "tableprint.h"

So same comment as for the other patch. (Rest looks good, I think.)

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

Re: [FFmpeg-devel] [PATCHv2 2/3] avcodec/mpegaudio_tablegen: speed up table generation

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 10:37 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Nov 26, 2015 at 10:23 AM, Ganesh Ajjanagadde
>  wrote:
>>
>> This does some miscellaneous stuff mainly avoiding the usage of pow to
>> achieve significant speedups. This is not speed critical, but is
>> unnecessary latency and cycles wasted for a user.
>>
>> All tables tested and are identical to the old ones
>> (bit-exact even in floating point case).
>>
>> Sample benchmark (x86-64, Haswell, GNU/Linux):
>> old:
>> 102329530 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>>
>> new:
>> 34111900 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavcodec/mpegaudio_tablegen.h | 21 +++--
>>  1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/mpegaudio_tablegen.h
>> b/libavcodec/mpegaudio_tablegen.h
>> index 86b2cd3..6cd5ad6 100644
>> --- a/libavcodec/mpegaudio_tablegen.h
>> +++ b/libavcodec/mpegaudio_tablegen.h
>> @@ -29,9 +29,11 @@
>>
>>  #define TABLE_4_3_SIZE (8191 + 16)*4
>>  #if CONFIG_HARDCODED_TABLES
>> +#include "libavutil/tablegen.h"
>>  #define mpegaudio_tableinit()
>>  #include "libavcodec/mpegaudio_tables.h"
>>  #else
>> +#include "libavutil/libm.h"
>
>
> I don't think this works yet. Check mpegaudio_tablegen.c (it should be
> obvious why it does this):
>
> #include 
> #define CONFIG_HARDCODED_TABLES 0
> #include "mpegaudio_tablegen.h"
> #include "tableprint.h"
>
> Note that this is the table generation which runs on the host platform. So,
> the location where you add this, is important. I would recommend including
> tablegen.h inside mpegaudio_tablegen.c, and then including libavutil/libm.h
> outside the scope of this header, e.g. in users of this header, if
> tablegen.h wasn't included. Make sure make checkheaders passes after your
> changes, and the build works with as well as without hardcoded_tables.

I did a build test with and without hardcoded , did not think of
checkheaders, will recheck and correct depending on the result.

>
>>  static int8_t   table_4_3_exp[TABLE_4_3_SIZE];
>>  static uint32_t table_4_3_value[TABLE_4_3_SIZE];
>>  static uint32_t exp_table_fixed[512];
>> @@ -45,12 +47,21 @@ static float expval_table_float[512][16];
>>  static av_cold void mpegaudio_tableinit(void)
>>  {
>>  int i, value, exponent;
>> +double pow2_lut[4] = {
>
>
> I think somebody else already mentioned exp2. I'd call this table exp2,
> since table[idx] corresponds to exp2(idx * constant).

Fine, will change.

>
>>
>> +double f = value * cbrt_lut[value] * pow(2, (exponent - 400)
>> * 0.25 + FRAC_BITS + 5) / IMDCT_SCALAR;
>
>
> exp2 here also. For compatibility, you can #define exp2(x) to pow(2, x) in
> libavutil/tablegen.h.

This won't be done, I already discussed with Timothy - if it is being
improved, it needs to improved well, and belongs in a separate patch.
Bascially, the exp2 can be removed entirely. For me, it is low
priority as it won't improve > 10% overall perf.

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

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

On Thu, Nov 26, 2015 at 04:07:00PM +0100, Michael Niedermayer wrote:
> On Thu, Nov 26, 2015 at 11:57:46AM +0100, Matthieu Bouron wrote:
> > On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> > > On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > > > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > > > From: Matthieu Bouron 
> > > [...]
> > > > >
> > > > > > +
> > > > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, 
> > > > > > AVPacket *pkt, int decode)
> > > > > > +{
> > > > > > +int ret = 0;
> > > > > > +int got_frame = 0;
> > > > > > +AVFrame *frame = NULL;
> > > > > > +int skip_frame = codec_ctx->skip_frame;
> > > > > > +
> > > > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > > > +const AVCodec *codec = 
> > > > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > > > +
> > > > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > > > +if (ret < 0) {
> > > > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open 
> > > > > > codec\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +}
> > > > > > +
> > > > > > +frame = av_frame_alloc();
> > > > > > +if (!frame) {
> > > > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > > > +}
> > > > > > +
> > > > > > +do {
> > > > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, 
> > > > > > pkt);
> > > > > > +av_assert0(decode || (!decode && !got_frame));
> > > > > > +if (ret < 0)
> > > > > > +break;
> > > > > > +pkt->data += ret;
> > > > > > +pkt->size -= ret;
> > > > > > +
> > > > > > +if (got_frame) {
> > > > > > +break;
> > > > > > +}
> > > > > > +} while (pkt->size > 0);
> > > > > > +
> > > > > > +end:
> > > > > > +codec_ctx->skip_frame = skip_frame;
> > > > > > +
> > > > > > +av_frame_free();
> > > > > > +return ret;
> > > > > > +}
> > > > > > +
> > > > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int 
> > > > > > decode)
> > > > > > +{
> > > > > > +int ret = 0;
> > > > > > +int i, done = 0;
> > > > > > +AVPacket pkt;
> > > > > > +
> > > > > > +av_init_packet();
> > > > > > +
> > > > > > +while (!done) {
> > > > > > +AVCodecContext *codec_ctx = NULL;
> > > > > > +AVStream *st;
> > > > > > +
> > > > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read 
> > > > > > frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > > > +codec_ctx = st->codec;
> > > > > > +
> > > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > > > 
> > > > > > +st->codec_info_nb_frames++ > 0) {
> > > > > 
> > > > > writing into this is not ok, iam not sure it should be touched at
> > > > > all from API users and tests/api/* could be used as example for
> > > > > correct API use ...
> > > > 
> > > > This function intends to mimic avformat_find_stream_info which 
> > > > increments
> > > > this field, if it's a blocker i'll use something else.
> > > 
> > > presonally iam fine if you document that this should not be used by
> > > user applications and why it is used in this "regression test"
> > > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > +av_packet_unref();
> > > > > > +continue;
> > > > > > +}
> > > > > > +
> > > > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > > > +if (ret < 0) {
> > > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > > > frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +av_packet_unref();
> > > > > > +
> > > > > > +/* check if all video streams have demuxed a packet */
> > > > > > +done = 1;
> > > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > > +st = fmt_ctx->streams[i];
> > > > > > +codec_ctx = st->codec;
> > > > > > +
> > > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > > > +continue;
> > > > > > +
> > > > > > +done &= st->codec_info_nb_frames > 0;
> > > > > > +}
> > > > > > +}
> > > > > > +
> > > > > > +end:
> > > > > > +av_packet_unref();
> > > > > > +
> > > > > > +return ret < 0;
> > > > > > +}
> > > > > > +
> > > > > > +static void 

Re: [FFmpeg-devel] [PATCH 0/4] more accurate constants

[John Warburton]

On Thu, Nov 26, 2015 at 1:26 PM, Ganesh Ajjanagadde  wrote:

>> Does removing the L fix it?

> Read up a bit and suspect it should, and more generally long double is
> a can of worms on ppc with bugs on some compilers:
> https://gcc.gnu.org/wiki/Ieee128PowerPC, ABI in transition,
> https://llvm.org/bugs/show_bug.cgi?id=11933 etc etc. Well known
> projects have gone through pain:
> https://github.com/numpy/numpy/issues/2001.

Thank you for looking into this. I removed the 'L' at the end of each
written-out constant, and the code compiles on PPC.

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

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

[Matthieu Bouron]

On Thu, Nov 19, 2015 at 12:10:20PM +0100, Matthieu Bouron wrote:
> On Mon, Nov 16, 2015 at 11:16:42AM -0500, Ronald S. Bultje wrote:
> > Hi,
> > 
> > On Mon, Nov 16, 2015 at 11:06 AM, Matthieu Bouron  > > wrote:
> > 
> > > On Sun, Nov 15, 2015 at 08:12:57AM -0500, Ronald S. Bultje wrote:
> > > > Hi,
> > >
> > > Hi,
> > >
> > > >
> > > > On Sun, Nov 15, 2015 at 4:49 AM, Matthieu Bouron <
> > > matthieu.bou...@gmail.com>
> > > > wrote:
> > > >
> > > > > On Mon, Nov 02, 2015 at 07:56:50AM -0500, Ronald S. Bultje wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron <
> > > > > matthieu.bou...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > From: Matthieu Bouron 
> > > > > > >
> > > > > > > Avoid decoding a frame to get the codec parameters while the codec
> > > > > > > supports FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM. This is particulary
> > > useful
> > > > > > > to avoid decoding twice images (once in avformat_find_stream_info
> > > and
> > > > > > > once when the actual decode is made).
> > > > > > > ---
> > > > > > >  libavformat/utils.c | 12 
> > > > > > >  1 file changed, 12 insertions(+)
> > > > > > >
> > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > > > > index 5c4d452..ba62f2b 100644
> > > > > > > --- a/libavformat/utils.c
> > > > > > > +++ b/libavformat/utils.c
> > > > > > > @@ -2695,6 +2695,8 @@ static int try_decode_frame(AVFormatContext
> > > *s,
> > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > >  AVFrame *frame = av_frame_alloc();
> > > > > > >  AVSubtitle subtitle;
> > > > > > >  AVPacket pkt = *avpkt;
> > > > > > > +int do_skip_frame = 0;
> > > > > > > +enum AVDiscard skip_frame;
> > > > > > >
> > > > > > >  if (!frame)
> > > > > > >  return AVERROR(ENOMEM);
> > > > > > > @@ -2733,6 +2735,12 @@ static int try_decode_frame(AVFormatContext
> > > *s,
> > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > >  goto fail;
> > > > > > >  }
> > > > > > >
> > > > > > > +if (st->codec->codec->caps_internal &
> > > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > > +do_skip_frame = 1;
> > > > > > > +skip_frame = st->codec->skip_frame;
> > > > > > > +st->codec->skip_frame = AVDISCARD_ALL;
> > > > > > > +}
> > > > > > > +
> > > > > > >  while ((pkt.size > 0 || (!pkt.data && got_picture)) &&
> > > > > > > ret >= 0 &&
> > > > > > > (!has_codec_parameters(st, NULL) ||
> > > > > > > !has_decode_delay_been_guessed(st) ||
> > > > > > > @@ -2768,6 +2776,10 @@ static int try_decode_frame(AVFormatContext
> > > *s,
> > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > >  ret = -1;
> > > > > > >
> > > > > > >  fail:
> > > > > > > +if (do_skip_frame) {
> > > > > > > +st->codec->skip_frame = skip_frame;
> > > > > > > +}
> > > > > > > +
> > > > > > >  av_frame_free();
> > > > > > >  return ret;
> > > > > > >  }
> > > > > > > --
> > > > > > > 2.6.2
> > > > > >
> > > > > >
> > > > > > I think we need an assert in the try_decode loop to ensure that it
> > > indeed
> > > > > > did fill all the params. This is to prevent the case where someone
> > > adds a
> > > > > > new "thing" to the list of things required for find_stream_info to
> > > > > succeed,
> > > > > > and forgets to update the codecs.
> > > > >
> > > > > While the codec_has_parameters function fits that role, it does not
> > > check
> > > > > all codec parameters as they can be codec dependant.
> > > > >
> > > > > I'm not sure if we can create a generic rule here for the same reason
> > > as
> > > > > above, as the parameters set can be codec specific and maybe stream
> > > > > specific.
> > > > >
> > > > > Having some fate test to cover this area seems to be another option 
> > > > > but
> > > > > would
> > > > > require to inspect all the relevant parameters of AVCodecContext
> > > depending
> > > > > on the codec and a lot of different streams.
> > > >
> > > >
> > > > I don't care about _which_ parameters were filled. The goal of this
> > > option
> > > > is only directly to fill parameters, but the purpose goes far beyond
> > > that.
> > > > The indirect goal of this change is to _not_ call try_decode_frame() for
> > > > certain image codecs. Whether they do that by dancing on the table or by
> > > > filling AVCodecContext parameters when a special flag is set, is merely
> > > an
> > > > implementation detail.
> > > >
> > > > I want the test to confirm that we did not call try_decode_frame() when
> > > the
> > > > skip-flag was set. It seems easiest to me that we check that by adding a
> > > > one-line assert, for example inside try_decode_frame, that checks that
> > > the
> > > > flag does not apply to this codec, right? If the assert triggers, 
> > > > clearly
> > > > something broke, and either the flag should be removed, or the check
> > > before
> > > > 

Re: [FFmpeg-devel] [PATCH 0/4] more accurate constants

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 11:15 AM, John Warburton  wrote:
> On Thu, Nov 26, 2015 at 1:26 PM, Ganesh Ajjanagadde  wrote:
>
>>> Does removing the L fix it?
>
>> Read up a bit and suspect it should, and more generally long double is
>> a can of worms on ppc with bugs on some compilers:
>> https://gcc.gnu.org/wiki/Ieee128PowerPC, ABI in transition,
>> https://llvm.org/bugs/show_bug.cgi?id=11933 etc etc. Well known
>> projects have gone through pain:
>> https://github.com/numpy/numpy/issues/2001.
>
> Thank you for looking into this. I removed the 'L' at the end of each
> written-out constant, and the code compiles on PPC.

Thanks for reporting, pushed into master.

>
> J
> ___
> 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

[FFmpeg-devel] [PATCHv3 1/3] avutil/tablegen: add tablegen compatibility hacks

[Ganesh Ajjanagadde]

Reviewed-by: Ronald S. Bultje 
Signed-off-by: Ganesh Ajjanagadde 
---
 libavutil/tablegen.h | 33 +
 1 file changed, 33 insertions(+)
 create mode 100644 libavutil/tablegen.h

diff --git a/libavutil/tablegen.h b/libavutil/tablegen.h
new file mode 100644
index 000..01c8eea
--- /dev/null
+++ b/libavutil/tablegen.h
@@ -0,0 +1,33 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Compatibility libm for table generation files
+ */
+
+#ifndef AVUTIL_TABLEGEN_H
+#define AVUTIL_TABLEGEN_H
+
+// we lack some functions on all host platforms, and we don't care about
+// performance as it's performed at build time
+#define cbrt(x)   pow(x, 1.0 / 3)
+#define llrint(x) floor((x) + 0.5)
+#define lrint(x)  floor((x) + 0.5)
+
+#endif /* AVUTIL_TABLEGEN_H */
-- 
2.6.2

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

[FFmpeg-devel] [PATCHv3 3/3] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ganesh Ajjanagadde]

On systems having cbrt, there is no reason to use the slow pow function.

Sample benchmark (x86-64, Haswell, GNU/Linux):
new:
5124920 decicycles in cbrt_tableinit,   1 runs,  0 skips

old:
12321680 decicycles in cbrt_tableinit,   1 runs,  0 skips

Reviewed-by: Ronald S. Bultje 
Signed-off-by: Ganesh Ajjanagadde 
---
 libavcodec/cbrt_tablegen.h  | 5 ++---
 libavcodec/cbrt_tablegen_template.c | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/cbrt_tablegen.h b/libavcodec/cbrt_tablegen.h
index 27a3e3a..59b5a1d 100644
--- a/libavcodec/cbrt_tablegen.h
+++ b/libavcodec/cbrt_tablegen.h
@@ -29,7 +29,7 @@
 #include "libavcodec/aac_defines.h"
 
 #if USE_FIXED
-#define CBRT(x) (int)floor((x).f * 8192 + 0.5)
+#define CBRT(x) lrint((x).f * 8192)
 #else
 #define CBRT(x) x.i
 #endif
@@ -49,13 +49,12 @@ static av_cold void AAC_RENAME(cbrt_tableinit)(void)
 {
 if (!cbrt_tab[(1<<13) - 1]) {
 int i;
-/* cbrtf() isn't available on all systems, so we use powf(). */
 for (i = 0; i < 1<<13; i++) {
 union {
 float f;
 uint32_t i;
 } f;
-f.f = pow(i, 1.0 / 3.0) * i;
+f.f = cbrt(i) * i;
 cbrt_tab[i] = CBRT(f);
 }
 }
diff --git a/libavcodec/cbrt_tablegen_template.c 
b/libavcodec/cbrt_tablegen_template.c
index 9dd2cf5..1d71d34 100644
--- a/libavcodec/cbrt_tablegen_template.c
+++ b/libavcodec/cbrt_tablegen_template.c
@@ -23,6 +23,7 @@
 #include 
 #define CONFIG_HARDCODED_TABLES 0
 #include "cbrt_tablegen.h"
+#include "libavutil/tablegen.h"
 #include "tableprint.h"
 
 int main(void)
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCHv2 2/3] avcodec/mpegaudio_tablegen: speed up table generation

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 10:44 AM, Ganesh Ajjanagadde
 wrote:
> On Thu, Nov 26, 2015 at 10:37 AM, Ronald S. Bultje  wrote:
>> Hi,
>>
>> On Thu, Nov 26, 2015 at 10:23 AM, Ganesh Ajjanagadde
>>  wrote:
>>>
>>> This does some miscellaneous stuff mainly avoiding the usage of pow to
>>> achieve significant speedups. This is not speed critical, but is
>>> unnecessary latency and cycles wasted for a user.
>>>
>>> All tables tested and are identical to the old ones
>>> (bit-exact even in floating point case).
>>>
>>> Sample benchmark (x86-64, Haswell, GNU/Linux):
>>> old:
>>> 102329530 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>>>
>>> new:
>>> 34111900 decicycles in mpegaudio_tableinit,   1 runs,  0 skips
>>>
>>> Signed-off-by: Ganesh Ajjanagadde 
>>> ---
>>>  libavcodec/mpegaudio_tablegen.h | 21 +++--
>>>  1 file changed, 15 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/libavcodec/mpegaudio_tablegen.h
>>> b/libavcodec/mpegaudio_tablegen.h
>>> index 86b2cd3..6cd5ad6 100644
>>> --- a/libavcodec/mpegaudio_tablegen.h
>>> +++ b/libavcodec/mpegaudio_tablegen.h
>>> @@ -29,9 +29,11 @@
>>>
>>>  #define TABLE_4_3_SIZE (8191 + 16)*4
>>>  #if CONFIG_HARDCODED_TABLES
>>> +#include "libavutil/tablegen.h"
>>>  #define mpegaudio_tableinit()
>>>  #include "libavcodec/mpegaudio_tables.h"
>>>  #else
>>> +#include "libavutil/libm.h"
>>
>>
>> I don't think this works yet. Check mpegaudio_tablegen.c (it should be
>> obvious why it does this):
>>
>> #include 
>> #define CONFIG_HARDCODED_TABLES 0
>> #include "mpegaudio_tablegen.h"
>> #include "tableprint.h"
>>
>> Note that this is the table generation which runs on the host platform. So,
>> the location where you add this, is important. I would recommend including
>> tablegen.h inside mpegaudio_tablegen.c, and then including libavutil/libm.h
>> outside the scope of this header, e.g. in users of this header, if
>> tablegen.h wasn't included. Make sure make checkheaders passes after your
>> changes, and the build works with as well as without hardcoded_tables.
>
> I did a build test with and without hardcoded , did not think of
> checkheaders, will recheck and correct depending on the result.

checkheaders also passes with just this patch (both with or without
hardcoded). Nevertheless, I agree with you and have now posted v3,
tested with/without checkheaders and with/without
--enable-hardcoded-tables.

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

Re: [FFmpeg-devel] [PATCHv3 3/3] avcodec/cbrt_tablegen: avoid pow and speed up cbrt_tableinit

[Ganesh Ajjanagadde]

On Thu, Nov 26, 2015 at 10:39 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Thu, Nov 26, 2015 at 10:23 AM, Ganesh Ajjanagadde
>  wrote:
>>
>>  #if CONFIG_HARDCODED_TABLES
>> +#include "libavutil/tablegen.h"
>>  #if USE_FIXED
>>  #define cbrt_tableinit_fixed()
>>  #include "libavcodec/cbrt_fixed_tables.h"
>> @@ -43,19 +45,19 @@
>>  #include "libavcodec/cbrt_tables.h"
>>  #endif
>>  #else
>> +#include "libavutil/libm.h"
>>  static uint32_t cbrt_tab[1 << 13];
>
>
> In cbrt_tablegen_template.c:
>
> #include 
> #define CONFIG_HARDCODED_TABLES 0
> #include "cbrt_tablegen.h"
> #include "tableprint.h"
>
> So same comment as for the other patch. (Rest looks good, I think.)

Also noted that lrint needs to go into the avutil/tablegen.h.

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

[FFmpeg-devel] [PATCH 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[Nicolas George]

Signed-off-by: Nicolas George 
---
 libavformat/flvdec.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index ca73969..96fff68 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -882,7 +882,7 @@ retry:
 }
 
 if (size == 0) {
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 
@@ -927,13 +927,13 @@ retry:
type, size, flags);
 skip:
 avio_seek(s->pb, next, SEEK_SET);
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 
 /* skip empty data packets */
 if (!size) {
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 
@@ -973,7 +973,7 @@ skip:
 || st->discard >= AVDISCARD_ALL
 ) {
 avio_seek(s->pb, next, SEEK_SET);
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 
@@ -1068,7 +1068,7 @@ retry_duration:
 if (st->codec->extradata) {
 if ((ret = flv_queue_extradata(flv, s->pb, stream_type, size)) 
< 0)
 return ret;
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 if ((ret = flv_get_extradata(s, st, size)) < 0)
@@ -1095,14 +1095,14 @@ retry_duration:
 }
 }
 
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 }
 
 /* skip empty data packets */
 if (!size) {
-ret = AVERROR(EAGAIN);
+ret = AVERROR_REDO;
 goto leave;
 }
 
-- 
2.6.2

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

[FFmpeg-devel] [PATCH 1/3] lavu/error: add AVERROR_REDO.

[Nicolas George]

It is meant for demuxers to signal that they consumed data
but did not return a packet; the framework is then supposed
to loop.

Signed-off-by: Nicolas George 
---
 libavutil/error.c   | 1 +
 libavutil/error.h   | 2 +-
 libavutil/version.h | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)


There was a stray empty line just at the same place.


diff --git a/libavutil/error.c b/libavutil/error.c
index 4425968..a253982 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -50,6 +50,7 @@ static const struct error_entry error_entries[] = {
 { ERROR_TAG(OUTPUT_CHANGED), "Output changed"  
   },
 { ERROR_TAG(PATCHWELCOME),   "Not yet implemented in FFmpeg, patches 
welcome" },
 { ERROR_TAG(PROTOCOL_NOT_FOUND), "Protocol not found"  
   },
+{ ERROR_TAG(REDO),   "Operation must be repeated"  
   },
 { ERROR_TAG(STREAM_NOT_FOUND),   "Stream not found"
   },
 { ERROR_TAG(UNKNOWN),"Unknown error occurred"  
   },
 { ERROR_TAG(EXPERIMENTAL),   "Experimental feature"
   },
diff --git a/libavutil/error.h b/libavutil/error.h
index 71df4da..09b47c2 100644
--- a/libavutil/error.h
+++ b/libavutil/error.h
@@ -61,7 +61,7 @@
 #define AVERROR_OPTION_NOT_FOUND   FFERRTAG(0xF8,'O','P','T') ///< Option not 
found
 #define AVERROR_PATCHWELCOME   FFERRTAG( 'P','A','W','E') ///< Not yet 
implemented in FFmpeg, patches welcome
 #define AVERROR_PROTOCOL_NOT_FOUND FFERRTAG(0xF8,'P','R','O') ///< Protocol 
not found
-
+#define AVERROR_REDO   FFERRTAG( 'R','E','D','O') ///< Operation 
must be repeated
 #define AVERROR_STREAM_NOT_FOUND   FFERRTAG(0xF8,'S','T','R') ///< Stream not 
found
 /**
  * This is semantically identical to AVERROR_BUG
diff --git a/libavutil/version.h b/libavutil/version.h
index e0ddfd2..6b0d60b 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -56,7 +56,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  55
-#define LIBAVUTIL_VERSION_MINOR   9
+#define LIBAVUTIL_VERSION_MINOR  10
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.6.2

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

[FFmpeg-devel] [PATCH 2/3] lavf/utils: handle AVERROR_REDO.

[Nicolas George]

Signed-off-by: Nicolas George 
---
 libavformat/utils.c | 2 ++
 1 file changed, 2 insertions(+)


An option can be added later to grant applications fine-grained control on
the looping, but it can not be the default as it would be an API change, and
it probably should not be the default anyway.


diff --git a/libavformat/utils.c b/libavformat/utils.c
index f33f2f5..d9165f1 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -680,6 +680,8 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 av_init_packet(pkt);
 ret = s->iformat->read_packet(s, pkt);
 if (ret < 0) {
+if (ret == AVERROR_REDO)
+continue;
 if (!pktl || ret == AVERROR(EAGAIN))
 return ret;
 for (i = 0; i < s->nb_streams; i++) {
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[wm4]

On Thu, 26 Nov 2015 19:47:54 +0100
Nicolas George  wrote:

> Signed-off-by: Nicolas George 
> ---
>  libavformat/flvdec.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index ca73969..96fff68 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -882,7 +882,7 @@ retry:
>  }
>  
>  if (size == 0) {
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  
> @@ -927,13 +927,13 @@ retry:
> type, size, flags);
>  skip:
>  avio_seek(s->pb, next, SEEK_SET);
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  
>  /* skip empty data packets */
>  if (!size) {
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  
> @@ -973,7 +973,7 @@ skip:
>  || st->discard >= AVDISCARD_ALL
>  ) {
>  avio_seek(s->pb, next, SEEK_SET);
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  
> @@ -1068,7 +1068,7 @@ retry_duration:
>  if (st->codec->extradata) {
>  if ((ret = flv_queue_extradata(flv, s->pb, stream_type, 
> size)) < 0)
>  return ret;
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  if ((ret = flv_get_extradata(s, st, size)) < 0)
> @@ -1095,14 +1095,14 @@ retry_duration:
>  }
>  }
>  
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  }
>  
>  /* skip empty data packets */
>  if (!size) {
> -ret = AVERROR(EAGAIN);
> +ret = AVERROR_REDO;
>  goto leave;
>  }
>  

Better do the looping internal in flvdec.c (if there's no huge number
of other demuxers which need this), instead of adding a new error code
that is also part of the public API.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Thu, Nov 26, 2015 at 05:16:32PM +0100, Matthieu Bouron wrote:
> On Thu, Nov 19, 2015 at 12:10:20PM +0100, Matthieu Bouron wrote:
> > On Mon, Nov 16, 2015 at 11:16:42AM -0500, Ronald S. Bultje wrote:
> > > Hi,
> > > 
> > > On Mon, Nov 16, 2015 at 11:06 AM, Matthieu Bouron 
> > >  > > > wrote:
> > > 
> > > > On Sun, Nov 15, 2015 at 08:12:57AM -0500, Ronald S. Bultje wrote:
> > > > > Hi,
> > > >
> > > > Hi,
> > > >
> > > > >
> > > > > On Sun, Nov 15, 2015 at 4:49 AM, Matthieu Bouron <
> > > > matthieu.bou...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > On Mon, Nov 02, 2015 at 07:56:50AM -0500, Ronald S. Bultje wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron <
> > > > > > matthieu.bou...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > From: Matthieu Bouron 
> > > > > > > >
> > > > > > > > Avoid decoding a frame to get the codec parameters while the 
> > > > > > > > codec
> > > > > > > > supports FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM. This is particulary
> > > > useful
> > > > > > > > to avoid decoding twice images (once in 
> > > > > > > > avformat_find_stream_info
> > > > and
> > > > > > > > once when the actual decode is made).
> > > > > > > > ---
> > > > > > > >  libavformat/utils.c | 12 
> > > > > > > >  1 file changed, 12 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > > > > > index 5c4d452..ba62f2b 100644
> > > > > > > > --- a/libavformat/utils.c
> > > > > > > > +++ b/libavformat/utils.c
> > > > > > > > @@ -2695,6 +2695,8 @@ static int 
> > > > > > > > try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  AVFrame *frame = av_frame_alloc();
> > > > > > > >  AVSubtitle subtitle;
> > > > > > > >  AVPacket pkt = *avpkt;
> > > > > > > > +int do_skip_frame = 0;
> > > > > > > > +enum AVDiscard skip_frame;
> > > > > > > >
> > > > > > > >  if (!frame)
> > > > > > > >  return AVERROR(ENOMEM);
> > > > > > > > @@ -2733,6 +2735,12 @@ static int 
> > > > > > > > try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  goto fail;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +if (st->codec->codec->caps_internal &
> > > > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > > > +do_skip_frame = 1;
> > > > > > > > +skip_frame = st->codec->skip_frame;
> > > > > > > > +st->codec->skip_frame = AVDISCARD_ALL;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  while ((pkt.size > 0 || (!pkt.data && got_picture)) &&
> > > > > > > > ret >= 0 &&
> > > > > > > > (!has_codec_parameters(st, NULL) ||
> > > > > > > > !has_decode_delay_been_guessed(st) ||
> > > > > > > > @@ -2768,6 +2776,10 @@ static int 
> > > > > > > > try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  ret = -1;
> > > > > > > >
> > > > > > > >  fail:
> > > > > > > > +if (do_skip_frame) {
> > > > > > > > +st->codec->skip_frame = skip_frame;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  av_frame_free();
> > > > > > > >  return ret;
> > > > > > > >  }
> > > > > > > > --
> > > > > > > > 2.6.2
> > > > > > >
> > > > > > >
> > > > > > > I think we need an assert in the try_decode loop to ensure that it
> > > > indeed
> > > > > > > did fill all the params. This is to prevent the case where someone
> > > > adds a
> > > > > > > new "thing" to the list of things required for find_stream_info to
> > > > > > succeed,
> > > > > > > and forgets to update the codecs.
> > > > > >
> > > > > > While the codec_has_parameters function fits that role, it does not
> > > > check
> > > > > > all codec parameters as they can be codec dependant.
> > > > > >
> > > > > > I'm not sure if we can create a generic rule here for the same 
> > > > > > reason
> > > > as
> > > > > > above, as the parameters set can be codec specific and maybe stream
> > > > > > specific.
> > > > > >
> > > > > > Having some fate test to cover this area seems to be another option 
> > > > > > but
> > > > > > would
> > > > > > require to inspect all the relevant parameters of AVCodecContext
> > > > depending
> > > > > > on the codec and a lot of different streams.
> > > > >
> > > > >
> > > > > I don't care about _which_ parameters were filled. The goal of this
> > > > option
> > > > > is only directly to fill parameters, but the purpose goes far beyond
> > > > that.
> > > > > The indirect goal of this change is to _not_ call try_decode_frame() 
> > > > > for
> > > > > certain image codecs. Whether they do that by dancing on the table or 
> > > > > by
> > > > > filling AVCodecContext parameters when a special flag is set, is 
> > > > > merely
> > > > an
> > > > > implementation detail.
> > > > >
> > > > > I 

Re: [FFmpeg-devel] [PATCH 2/3] lavf/utils: handle AVERROR_REDO.

[Michael Niedermayer]

On Thu, Nov 26, 2015 at 07:47:53PM +0100, Nicolas George wrote:
> Signed-off-by: Nicolas George 
> ---
>  libavformat/utils.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> 
> An option can be added later to grant applications fine-grained control on
> the looping, but it can not be the default as it would be an API change, and
> it probably should not be the default anyway.

assuming wm4s objection is resolved, this LGTM

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

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


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/3] lavu/error: add AVERROR_REDO.

[Michael Niedermayer]

On Thu, Nov 26, 2015 at 07:47:52PM +0100, Nicolas George wrote:
> It is meant for demuxers to signal that they consumed data
> but did not return a packet; the framework is then supposed
> to loop.
> 
> Signed-off-by: Nicolas George 
> ---
>  libavutil/error.c   | 1 +
>  libavutil/error.h   | 2 +-
>  libavutil/version.h | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
> There was a stray empty line just at the same place.

assuming wm4s objection is resolved, this LGTM

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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

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

[Ronald S. Bultje]

Hi,

On Thu, Nov 26, 2015 at 11:16 AM, Matthieu Bouron  wrote:

> On Thu, Nov 19, 2015 at 12:10:20PM +0100, Matthieu Bouron wrote:
> > On Mon, Nov 16, 2015 at 11:16:42AM -0500, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Mon, Nov 16, 2015 at 11:06 AM, Matthieu Bouron <
> matthieu.bou...@gmail.com
> > > > wrote:
> > >
> > > > On Sun, Nov 15, 2015 at 08:12:57AM -0500, Ronald S. Bultje wrote:
> > > > > Hi,
> > > >
> > > > Hi,
> > > >
> > > > >
> > > > > On Sun, Nov 15, 2015 at 4:49 AM, Matthieu Bouron <
> > > > matthieu.bou...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > On Mon, Nov 02, 2015 at 07:56:50AM -0500, Ronald S. Bultje wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron <
> > > > > > matthieu.bou...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > From: Matthieu Bouron 
> > > > > > > >
> > > > > > > > Avoid decoding a frame to get the codec parameters while the
> codec
> > > > > > > > supports FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM. This is
> particulary
> > > > useful
> > > > > > > > to avoid decoding twice images (once in
> avformat_find_stream_info
> > > > and
> > > > > > > > once when the actual decode is made).
> > > > > > > > ---
> > > > > > > >  libavformat/utils.c | 12 
> > > > > > > >  1 file changed, 12 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > > > > > index 5c4d452..ba62f2b 100644
> > > > > > > > --- a/libavformat/utils.c
> > > > > > > > +++ b/libavformat/utils.c
> > > > > > > > @@ -2695,6 +2695,8 @@ static int
> try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  AVFrame *frame = av_frame_alloc();
> > > > > > > >  AVSubtitle subtitle;
> > > > > > > >  AVPacket pkt = *avpkt;
> > > > > > > > +int do_skip_frame = 0;
> > > > > > > > +enum AVDiscard skip_frame;
> > > > > > > >
> > > > > > > >  if (!frame)
> > > > > > > >  return AVERROR(ENOMEM);
> > > > > > > > @@ -2733,6 +2735,12 @@ static int
> try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  goto fail;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +if (st->codec->codec->caps_internal &
> > > > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > > > +do_skip_frame = 1;
> > > > > > > > +skip_frame = st->codec->skip_frame;
> > > > > > > > +st->codec->skip_frame = AVDISCARD_ALL;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  while ((pkt.size > 0 || (!pkt.data && got_picture)) &&
> > > > > > > > ret >= 0 &&
> > > > > > > > (!has_codec_parameters(st, NULL) ||
> > > > > > > > !has_decode_delay_been_guessed(st) ||
> > > > > > > > @@ -2768,6 +2776,10 @@ static int
> try_decode_frame(AVFormatContext
> > > > *s,
> > > > > > > > AVStream *st, AVPacket *avpkt,
> > > > > > > >  ret = -1;
> > > > > > > >
> > > > > > > >  fail:
> > > > > > > > +if (do_skip_frame) {
> > > > > > > > +st->codec->skip_frame = skip_frame;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  av_frame_free();
> > > > > > > >  return ret;
> > > > > > > >  }
> > > > > > > > --
> > > > > > > > 2.6.2
> > > > > > >
> > > > > > >
> > > > > > > I think we need an assert in the try_decode loop to ensure
> that it
> > > > indeed
> > > > > > > did fill all the params. This is to prevent the case where
> someone
> > > > adds a
> > > > > > > new "thing" to the list of things required for
> find_stream_info to
> > > > > > succeed,
> > > > > > > and forgets to update the codecs.
> > > > > >
> > > > > > While the codec_has_parameters function fits that role, it does
> not
> > > > check
> > > > > > all codec parameters as they can be codec dependant.
> > > > > >
> > > > > > I'm not sure if we can create a generic rule here for the same
> reason
> > > > as
> > > > > > above, as the parameters set can be codec specific and maybe
> stream
> > > > > > specific.
> > > > > >
> > > > > > Having some fate test to cover this area seems to be another
> option but
> > > > > > would
> > > > > > require to inspect all the relevant parameters of AVCodecContext
> > > > depending
> > > > > > on the codec and a lot of different streams.
> > > > >
> > > > >
> > > > > I don't care about _which_ parameters were filled. The goal of this
> > > > option
> > > > > is only directly to fill parameters, but the purpose goes far
> beyond
> > > > that.
> > > > > The indirect goal of this change is to _not_ call
> try_decode_frame() for
> > > > > certain image codecs. Whether they do that by dancing on the table
> or by
> > > > > filling AVCodecContext parameters when a special flag is set, is
> merely
> > > > an
> > > > > implementation detail.
> > > > >
> > > > > I want the test to confirm that we did not call try_decode_frame()
> when
> > > > the
> > 

Re: [FFmpeg-devel] [PATCH 3/3] lavf/flvdec: use AVERROR_REDO instead of AVERROR(EAGAIN).

[Nicolas George]

Le sextidi 6 frimaire, an CCXXIV, wm4 a écrit :
> Better do the looping internal in flvdec.c (if there's no huge number
> of other demuxers which need this), instead of adding a new error code
> that is also part of the public API.

There are a few, but not many. This was my first intent, but looping in the
framework is way more elegant and much cleaner design.

Nothing would prevent us from defining the error code in
libavformat/internal.h if we make sure it does not leaks to the application,
but as Michael mentioned earlier and I did in the comments for patch 2/3 in
this series, having a public code allows to give the application
fine-grained control over the looping, for example to maintain basic user
interaction while a demuxer tries to find a sync word in a big corrupted
file on a slow local medium.

Therefore, I maintain the proposed patch as is.

Regards,

-- 
  Nicolas George


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