Re: [FFmpeg-devel] [PATCH 1/2] avformat: always unref the packet after parsing

[Hendrik Leppkes]

On Sun, Nov 1, 2015 at 1:18 PM, wm4  wrote:
> On Sun, 1 Nov 2015 13:03:50 +0100
> Hendrik Leppkes  wrote:
>
>> On Sun, Nov 1, 2015 at 12:57 PM, wm4  wrote:
>> > On Sun,  1 Nov 2015 11:21:26 +0100
>> > Hendrik Leppkes  wrote:
>> >
>> >> This fixes a memory leak when side-data is present.
>> >> ---
>> >>  libavformat/utils.c | 9 -
>> >>  1 file changed, 4 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> >> index 7e4f54f..3f82659 100644
>> >> --- a/libavformat/utils.c
>> >> +++ b/libavformat/utils.c
>> >> @@ -1285,12 +1285,11 @@ static int parse_packet(AVFormatContext *s, 
>> >> AVPacket *pkt, int stream_index)
>> >>
>> >>  compute_pkt_fields(s, st, st->parser, &out_pkt, next_dts, 
>> >> next_pts);
>> >>
>> >> -if ((ret = add_to_pktbuf(&s->internal->parse_queue, &out_pkt,
>> >> - &s->internal->parse_queue_end,
>> >> - 1))) {
>> >> -av_packet_unref(&out_pkt);
>> >> +ret = add_to_pktbuf(&s->internal->parse_queue, &out_pkt,
>> >> +&s->internal->parse_queue_end, 1);
>> >> +av_packet_unref(&out_pkt);
>> >> +if (ret < 0)
>> >>  goto fail;
>> >> -}
>> >>  }
>> >>
>> >>  /* end of the stream => close and free the parser */
>> >
>> > I thought he semantics of add_to_pktbuf was to transfer packet
>> > ownership if the last parameter is 1?
>>
>> Actually 0 does that, 1 creates a new reference, including copying
>> data into a refcounted buffer if needed, which is wanted here since
>> the output buffer from the parser is not persistent otherwise.
>
> Both patches LGTM then.

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

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

[Sven Dueking]

> -Ursprüngliche Nachricht-
> Von: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] Im Auftrag
> von Carl Eugen Hoyos
> Gesendet: Donnerstag, 29. Oktober 2015 18:39
> An: ffmpeg-devel@ffmpeg.org
> Betreff: Re: [FFmpeg-devel] [PATCH] Added QSV based VPP filter - second
> try
> 
> Sven Dueking  nablet.com> writes:
> 
> > Please find attached my second attempt to add Intel´s VPP to FFMpeg.
> > As requested, the patch includes a documentation file
> 
> I don't know much about how the documentation works but why isn't this
> part of the filter documentation and how are users supposed to find it?

Hi Carl,

I was unsure how to add the documentation, that´s why I asked this before.
Will merge my changes into the filter documentation.

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

https://software.intel.com/sites/default/files/mediasdk-man.pdf

RGB4
Thirty-two-bit RGB color format. Also known as RGB32

MFX_FOURCC_RGB4 : RGB4 (RGB32) color planes

This matches the define in the latest SDK

MFX_FOURCC_RGB4 = MFX_MAKEFOURCC('R','G','B','4'),/* RGB32 */

As far as I know older versions of mfxstructures.h have such comment as you 
mentioned.

Anyway, according to the sample

   case MFX_FOURCC_RGB4:
ptr->G = ptr->B + 1;
ptr->R = ptr->B + 2;
ptr->A = ptr->B + 3;

it´s BGRA (and the output is fine using BRGA and "bad" using ARGB).

> 
> > +if (inlink->format == AV_PIX_FMT_YUV420P)
> > +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YV12;
> > +else if (inlink->format == AV_PIX_FMT_YUV422P)
> > +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YUY2;
> > +else if (inlink->format == AV_PIX_FMT_NV12)
> > +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_NV12;
> > +else
> > +vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_RGB4;
> 
> Did you consider to add BGR4 and ARGB16 in the future?

No, VPP doesn´t support ARGB16.

> 
> > +unsigned int bits_per_pixel =
> > + get_bpp(vpp->pVppParam->vpp.In.FourCC);
> 
> See below.
> 
> > +width = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Width,
> 32);
> > +height = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Height,
> > + 32);
> 
> Are the casts necessary or useful?

Nope ;)

> 
> > +vpp->in_surface = av_mallocz(sizeof(mfxFrameSurface1) *
> > vpp->num_surfaces_in);
> 
> This looks wrong to me, I believe you want to allocate
> num_surfaces_in pointers, not structs. (Sorry if I miss
> somthing.)

You are correct, thanks !

> 
> > +return -1;
> 
> ENOMEM.
> 
> > +for (i = 0; i < vpp->num_surfaces_in; i++)
> > +vpp->in_surface[i] = NULL;
> 
> This is unnecessary if you use mallocz() as you do.
> 
> > +for (i = 0; i < vpp->num_surfaces_in; i++) {
> > +vpp->in_surface[i] = av_mallocz(sizeof(mfxFrameSurface1));
> > +
> > +if (!vpp->in_surface[i])
> > +return -1;
> 
> ENOMEM but this leaks memory, you have to free
> everything else on failure.
> (same below for out_surface)
> 
> > +bits_per_pixel = 12;
> 
> Imo, remove the variable and use get_bpp() once
> and "12" on the second usage.
> 

Also ok.

Many thanks for your feedback !

> Carl Eugen
> ___
> 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] [PATCH 1/4] lavc/internal: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM

[Matthieu Bouron]

From: Matthieu Bouron 

Codec supporting FF_CODEC_SKIP_FRAME_FILL must still extract and fill
their parameters into AVCodecContext while honoring the skip_frame
flag.
---
 libavcodec/internal.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 0abe17f..a023d1a 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -53,6 +53,11 @@
  * from the input AVPacket.
  */
 #define FF_CODEC_CAP_SETS_PKT_DTS   (1 << 2)
+/**
+ * Codec still extract and fill its parameters even if the skip_frame
+ * parameter is honored.
+ */
+#define FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM  (1 << 3)
 
 #ifdef TRACE
 #   define ff_tlog(ctx, ...) av_log(ctx, AV_LOG_TRACE, __VA_ARGS__)
-- 
2.6.2

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

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

[Matthieu Bouron]

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(&frame);
 return ret;
 }
-- 
2.6.2

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

[FFmpeg-devel] [PATCH 3/4] lavc/mjpegdec: set FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM capability

[Matthieu Bouron]

From: Matthieu Bouron 

---
 libavcodec/mjpegdec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index e17b213..801eb29 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2442,7 +2442,8 @@ AVCodec ff_mjpeg_decoder = {
 .capabilities   = AV_CODEC_CAP_DR1,
 .max_lowres = 3,
 .priv_class = &mjpegdec_class,
-.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
+.caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
+  FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
 };
 #endif
 #if CONFIG_THP_DECODER
-- 
2.6.2

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

[FFmpeg-devel] [PATCH 4/4] lavc/pngdec: set FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM capability

[Matthieu Bouron]

From: Matthieu Bouron 

---
 libavcodec/pngdec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 0bdd04e..7bb3c7d 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1519,5 +1519,6 @@ AVCodec ff_png_decoder = {
 .init_thread_copy = ONLY_IF_THREADS_ENABLED(png_dec_init),
 .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
 .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS /*| 
AV_CODEC_CAP_DRAW_HORIZ_BAND*/,
+.caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
 };
 #endif
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH 2/3] avfilter/vf_frei0r: use av_strtod instead of strtod for added flexibility

[Michael Niedermayer]

On Sun, Nov 01, 2015 at 08:37:01PM -0500, Ganesh Ajjanagadde wrote:
> On Sun, Nov 1, 2015 at 8:23 PM, Michael Niedermayer
>  wrote:
> > On Sun, Nov 01, 2015 at 11:57:54AM -0500, Ganesh Ajjanagadde wrote:
> >> This converts the usage of strtod to av_strtod in order to be more free
> >> in accepting double parameters.
> >
> > "more free" is a bit unclear and confusing
> > the change itself should be ok
> 
> "more flexible" maybe? If no one replies, will push with "more flexible".

i was thinking of 
something like, unifies / makes number parsing more consistent
and add support for SI postfixes

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


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/3] avfilter/vf_frei0r: use av_strtod instead of strtod for added flexibility

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 6:40 AM, Michael Niedermayer
 wrote:
> On Sun, Nov 01, 2015 at 08:37:01PM -0500, Ganesh Ajjanagadde wrote:
>> On Sun, Nov 1, 2015 at 8:23 PM, Michael Niedermayer
>>  wrote:
>> > On Sun, Nov 01, 2015 at 11:57:54AM -0500, Ganesh Ajjanagadde wrote:
>> >> This converts the usage of strtod to av_strtod in order to be more free
>> >> in accepting double parameters.
>> >
>> > "more free" is a bit unclear and confusing
>> > the change itself should be ok
>>
>> "more flexible" maybe? If no one replies, will push with "more flexible".
>
> i was thinking of
> something like, unifies / makes number parsing more consistent
> and add support for SI postfixes

accordingly reworded and pushed, thanks.

>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Ronald S. Bultje]

Hi,

On Mon, Nov 2, 2015 at 5:45 AM, Matthieu Bouron 
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(&frame);
>  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.

(These features break easily and subtly, and ideally we'd detect such
breakage in fate, not 6 months after the next release.)

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

Re: [FFmpeg-devel] [PATCH 1/4] fate: add concat demuxer tests

[Michael Niedermayer]

On Mon, Nov 02, 2015 at 12:15:55AM +0100, Marton Balint wrote:
> Signed-off-by: Marton Balint 
> ---
>  tests/Makefile |1 +
>  tests/extended.ffconcat|  114 +
>  tests/fate-run.sh  |   20 +
>  tests/fate/concatdec.mak   |   21 +
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf|1 +
>  .../ref/fate/concat-demuxer-extended-lavf-mxf_d10  |1 +
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 1713 ++
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 1193 ++
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts  | 2326 
> 
>  tests/simple1.ffconcat |   12 +
>  tests/simple2.ffconcat |   19 +
>  11 files changed, 5421 insertions(+)
>  create mode 100644 tests/extended.ffconcat
>  create mode 100644 tests/fate/concatdec.mak
>  create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf
>  create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
>  create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf
>  create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
>  create mode 100644 tests/ref/fate/concat-demuxer-simple2-lavf-ts
>  create mode 100644 tests/simple1.ffconcat
>  create mode 100644 tests/simple2.ffconcat

seems to break fate here:

--- ./tests/ref/fate/concat-demuxer-extended-lavf-mxf   2015-11-02 
13:44:01.131937697 +0100
+++ tests/data/fate/concat-demuxer-extended-lavf-mxf2015-11-02 
13:52:51.555948872 +0100
@@ -1 +1,8758 @@
-55cca059678d6335ab36e8fad700319d 
*tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
+[PACKET]
+codec_type=video
+stream_index=0
+pts=0
+pts_time=0.00
+dts=-1
+dts_time=-0.04
+duration=1
+duration_time=0.04
+convergence_duration=N/A
...
+DISPOSITION:karaoke=0
+DISPOSITION:forced=0
+DISPOSITION:hearing_impaired=0
+DISPOSITION:visual_impaired=0
+DISPOSITION:clean_effects=0
+DISPOSITION:attached_pic=0
+TAG:file_package_umid=0x060A2B340101010501010D001301
+[/STREAM]
Test concat-demuxer-extended-lavf-mxf failed. Look at 
tests/data/fate/concat-demuxer-extended-lavf-mxf.err for details.
make: *** [fate-concat-demuxer-extended-lavf-mxf] Error 1

--- ./tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10   2015-11-02 
13:44:01.131937697 +0100
+++ tests/data/fate/concat-demuxer-extended-lavf-mxf_d102015-11-02 
13:52:51.683948875 +0100
@@ -1 +1,3343 @@
-651eca7722187ff6836f55826bb1d110 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
+[PACKET]
+codec_type=video
+stream_index=0
+pts=0
+pts_time=0.00
+dts=0
...


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

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


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

[FFmpeg-devel] [PATCH 0/2] Fix/clarify the sdp_file option.

[Simon Thelen]

Fixes a segfault when trying to write sdp information without an rtp
output stream, also clarifies that the sdp_file option requires an rtp
output format.

Example of segfaulting command:
ffmpeg -re -f lavfi -i testsrc -re -f lavfi -i aevalsrc=0 -sdp_file test -c:v 
libx264 -strict -2 -f rtp_mpegts rtp://localhost:

Simon Thelen (2):
  ffmpeg: Don't try and write sdp info if none of the outputs had an rtp
format.
  doc/ffmpeg: Clarify that the sdp_file option requires an rtp output.

 doc/ffmpeg.texi | 4 ++--
 ffmpeg.c| 4 
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.6.2

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

[FFmpeg-devel] [PATCH 1/2] ffmpeg: Don't try and write sdp info if none of the outputs had an rtp format.

[Simon Thelen]

Fixes a segfault when trying to write nonexistent rtp information.

Signed-off-by: Simon Thelen 
---
 ffmpeg.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ffmpeg.c b/ffmpeg.c
index f8b071a..a6b3c1c 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2447,6 +2447,9 @@ static void print_sdp(void)
 }
 }
 
+   if (j == 0)
+   goto fail;
+
 av_sdp_create(avc, j, sdp, sizeof(sdp));
 
 if (!sdp_filename) {
@@ -2462,6 +2465,7 @@ static void print_sdp(void)
 }
 }
 
+fail:
 av_freep(&avc);
 }
 
-- 
2.6.2

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

[FFmpeg-devel] [PATCH 2/2] doc/ffmpeg: Clarify that the sdp_file option requires an rtp output.

[Simon Thelen]

Signed-off-by: Simon Thelen 
---
 doc/ffmpeg.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 08b1ed2..cf74734 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1233,9 +1233,9 @@ The option is intended for cases where features are 
needed that cannot be
 specified to @command{ffserver} but can be to @command{ffmpeg}.
 
 @item -sdp_file @var{file} (@emph{global})
-Print sdp information to @var{file}.
+Print sdp information for an output stream to @var{file}.
 This allows dumping sdp information when at least one output isn't an
-rtp stream.
+rtp stream. (Requires at least one of the output formats to be rtp).
 
 @item -discard (@emph{input})
 Allows discarding specific streams or frames of streams at the demuxer.
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH 1/2] ffmpeg: Don't try and write sdp info if none of the outputs had an rtp format.

[Carl Eugen Hoyos]

Simon Thelen  c-14.de> writes:

> + if (j == 0)
> + goto fail;

Tabs cannot be committed to our repository, please 
remove them (this has to be fixed).
Most FFmpeg code uses "if (!j)", feel free to ignore.

Carl Eugen

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

Re: [FFmpeg-devel] [PATCHv3 2/2] mpegtsenc: Implement writing of Opus trim_start/trim_end control values

[Carl Eugen Hoyos]

Sebastian Dröge  coaxion.net> writes:

> +if (ts_st->opus_pending_trim_start)
> +  ctrl_header_size += 2;
> +if (trim_end)
> +  ctrl_header_size += 2;

You could move the additions inside the larger if 
blocks below, that would make the code slightly 
cleaner imo, feel free to ignore.

Could you clarify that the specification was fixed?
I believe it wasn't some time ago or am I wrong?

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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/segafilm: implement seeking

[Michael Niedermayer]

On Fri, Oct 30, 2015 at 12:11:23PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/segafilm.c | 21 +
>  1 file changed, 21 insertions(+)

LGTM

thanks

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"


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] avformat/segafilm: set video stream duration

[Michael Niedermayer]

On Fri, Oct 30, 2015 at 12:11:25PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/segafilm.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

LGTM

thanks

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

Republics decline into democracies and democracies degenerate into
despotisms. -- 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 2/3] avformat/segafilm: set audio stream duration

[Michael Niedermayer]

On Fri, Oct 30, 2015 at 12:11:24PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/segafilm.c | 3 +++
>  1 file changed, 3 insertions(+)

LGTM

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- 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] avformat/cache: Separate read and seek return-values within cache_read

[Michael Niedermayer]

On Sun, Nov 01, 2015 at 10:56:45PM -0800, Bryan Huh wrote:
> Fixes an issue where an int64_t ffurl_seek return-value was being stored
> in a generic int "r" variable, leading to integer overflow when seeking
> into a large file (>2GB), and ultimately a "Failed to perform internal
> seek" error mesage. The "r" variable was used both for storing the
> result of any seeks and for the result of any reads. Instead, I separate
> the variable into an int64_t seek-variable and int read-variable.
> 
> To test, try running `ffprobe 'cache:http://'` on a file that
> is ~3GB large, whose moov atom is at the end of the file
> ---
>  libavformat/cache.c |   49 +
>  1 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/libavformat/cache.c b/libavformat/cache.c
> index a762aa9..b654f45 100644
> --- a/libavformat/cache.c
> +++ b/libavformat/cache.c
> @@ -156,7 +156,8 @@ static int cache_read(URLContext *h, unsigned char *buf, 
> int size)
>  {
>  Context *c= h->priv_data;
>  CacheEntry *entry, *next[2] = {NULL, NULL};

> -int r;
> +int64_t off;
> +int bytes_read = 0;

renaming the variable is unrelated (its not neccessary)
but if you want to rename/split it, i dont mind but it should be in
a seperate patch


>
>  entry = av_tree_find(c->root, &c->logical_pos, cmp, (void**)next);
>  
> @@ -169,21 +170,21 @@ static int cache_read(URLContext *h, unsigned char 
> *buf, int size)
>  if (in_block_pos < entry->size) {
>  int64_t physical_target = entry->physical_pos + in_block_pos;
>  

> -if (c->cache_pos != physical_target) {
> +if (c->cache_pos != physical_target)

removing {} is not needed, without it the change is smaller and
simpeler to read (for example when someone reads git log changes)


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

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


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

Re: [FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Ronald S. Bultje]

Hi,

On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde  wrote:

> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje 
> wrote:
>
> - I think the name of this function is confusing. The av_clip* namespace
> > seems reserved for int to int clips, so using it for a float to int
> > conversion with clip is kind of weird. I would use a different namespace
> > for it.
>
> Mathematically, it is the same as clipping, albeit "lossy", but then
> again all clipping is by nature "lossy" on inputs outside the clipping
> range, hence the choice of name. Any ideas for a namespace for this?
>

Clip is the namespace for _just_ a clip. This function converts _and_
clips. How about av_rint64_clip.

This is modeled after llrint, except that we "rint" a int64_t instead of a
long long (hence rint64 instead of llrint). The clip is an additional thing
this function does, so rint64_clip.

> So, finally, maybe I'm being pedantic now, but it seems one of these
> usages
> > overflows if we go over 2^64 = 16000 petabytes. Is that right? Do we
> really
> > care? I mean, we're talking ffserver here, ffserver has much bigger
> issues
> > than this.
>
> You are right, gave it as an illustration of API usage and an issue,
> albeit not a terribly important one :) - user's config is an untrusted
> file.


It affects where it lives. I guess it's ok since it seems common.h is
installed, so nevermind this.

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

[FFmpeg-devel] [PATCH 1/2 v2] ffmpeg: Don't try and write sdp info if none of the outputs had an rtp format.

[Simon Thelen]

Fixes a segfault when trying to write nonexistent rtp information.

Signed-off-by: Simon Thelen 
---
 ffmpeg.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ffmpeg.c b/ffmpeg.c
index f8b071a..6b41bb3 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2447,6 +2447,9 @@ static void print_sdp(void)
 }
 }
 
+if (!j)
+goto fail;
+
 av_sdp_create(avc, j, sdp, sizeof(sdp));
 
 if (!sdp_filename) {
@@ -2462,6 +2465,7 @@ static void print_sdp(void)
 }
 }
 
+fail:
 av_freep(&avc);
 }
 
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCHv3 2/2] mpegtsenc: Implement writing of Opus trim_start/trim_end control values

[Sebastian Dröge]

On Mo, 2015-11-02 at 15:12 +, Carl Eugen Hoyos wrote:
> Sebastian Dröge  coaxion.net> writes:
> 
> > +if (ts_st->opus_pending_trim_start)
> > +  ctrl_header_size += 2;
> > +if (trim_end)
> > +  ctrl_header_size += 2;
> 
> You could move the additions inside the larger if 
> blocks below, that would make the code slightly 
> cleaner imo, feel free to ignore.

Thanks for the review!

That's not possible as this size is used for av_malloc(). I guess I
could always allocate the maximum size and make it smaller in the end,
if you prefer that.

> Could you clarify that the specification was fixed?
> I believe it wasn't some time ago or am I wrong?

What was wrong with the specification? The au_size of the control
header is wrong and going to be (or already is?) changed to be how it's
used in ffmpeg in the demuxer and in obe.tv's muxer. Apart from that
I'm not aware of any problems.

signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 10:28 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde  wrote:
>
>> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje 
>> wrote:
>>
>> - I think the name of this function is confusing. The av_clip* namespace
>> > seems reserved for int to int clips, so using it for a float to int
>> > conversion with clip is kind of weird. I would use a different namespace
>> > for it.
>>
>> Mathematically, it is the same as clipping, albeit "lossy", but then
>> again all clipping is by nature "lossy" on inputs outside the clipping
>> range, hence the choice of name. Any ideas for a namespace for this?
>>
>
> Clip is the namespace for _just_ a clip. This function converts _and_
> clips. How about av_rint64_clip.

I like this name, thanks. Also, please tell me what to do about
version bumps etc, so that I can get this done for v2.

>
> This is modeled after llrint, except that we "rint" a int64_t instead of a
> long long (hence rint64 instead of llrint). The clip is an additional thing
> this function does, so rint64_clip.
>
>> So, finally, maybe I'm being pedantic now, but it seems one of these
>> usages
>> > overflows if we go over 2^64 = 16000 petabytes. Is that right? Do we
>> really
>> > care? I mean, we're talking ffserver here, ffserver has much bigger
>> issues
>> > than this.
>>
>> You are right, gave it as an illustration of API usage and an issue,
>> albeit not a terribly important one :) - user's config is an untrusted
>> file.
>
>
> It affects where it lives. I guess it's ok since it seems common.h is
> installed, so nevermind this.
>
> Ronald
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Michael Niedermayer]

On Fri, Oct 30, 2015 at 04:15:30PM +0100, Tobias Rapp wrote:
> On 30.10.2015 15:06, wm4 wrote:
> >On Fri, 30 Oct 2015 11:30:40 +0100
> >Tobias Rapp  wrote:
> >
> >>On 29.10.2015 09:38, Tobias Rapp wrote:
> >>>Attached patch fixes file lock issues in my Windows application when a
> >>>child process is started with handle inheritance enabled (standard
> >>>input/output redirection) while a FFmpeg transcoding is running in the
> >>>parent process.
> >>>
> >>>BTW: It would be great if the patch could also be applied to the 2.7/2.8
> >>>release branches.
> >>
> >>Forgot to add links relevant to the subject.
> >>
> >>[1] https://msdn.microsoft.com/en-us/library/w7sa2b22.aspx
> >>
> >>Describes the _wsopen() function and the O_NOINHERIT flag. File handles
> >>opened by _wsopen() are inheritable by default.
> >>
> >>[2]
> >>https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858%28v=vs.85%29.aspx
> >>
> >>Describes the CreateFile() function. Not directly relevant here, often
> >>used in cases outside of C/libc. Opens file without inheritance by
> >>default (lpSecurityAttributes is NULL).
> >>
> >>[3]
> >>https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx
> >>
> >>Describes handle inheritance when creating new processes. Handle
> >>inheritance must be enabled (bInheritHandles = TRUE) e.g. when you want
> >>to pass handles for stdin/stdout via lpStartupInfo.
> >
> >Would be great if you could put all this stuff into the commit message.
> 
> Have attached a more verbose version of the patch.
> 
> >Would it make sense to define O_CLOEXEC to O_NOINHERIT on Windows?
> 
> Although to my knowledge O_CLOEXEC and O_NOINHERIT cause the same
> behavior it would be confusing to trace when O_CLOEXEC maps to the
> original Linux definition and when it maps to the Windows override
> when reading source code. If someone finds it useful a definition
> like FF_O_CLOEXEC could be added (to which file?) that maps to the
> correct flag depending on the platform.
> 
> Regards,
> Tobias

>  file_open.c |3 +++
>  1 file changed, 3 insertions(+)
> 6f492d310d48ec875ec57d09d285e2f682603524  
> 0001-avutil-file_open-avoid-file-handle-inheritance-on-Wi.patch
> From c2b599c18a173ce3a2f053701bc4ef1f14ef7aea Mon Sep 17 00:00:00 2001
> From: Tobias Rapp 
> Date: Thu, 29 Oct 2015 09:11:37 +0100
> Subject: [PATCH] avutil/file_open: avoid file handle inheritance on Windows
> 
> Avoids inheritance of file handles on Windows systems similar to the
> O_CLOEXEC/FD_CLOEXEC flag on Linux.
> 
> Fixes file lock issues in Windows applications when a child process
> is started with handle inheritance enabled (standard input/output
> redirection) while a FFmpeg transcoding is running in the parent
> process.
> 
> Links relevant to the subject:
> 
> https://msdn.microsoft.com/en-us/library/w7sa2b22.aspx
> 
> Describes the _wsopen() function and the O_NOINHERIT flag. File handles
> opened by _wsopen() are inheritable by default.
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx
> 
> Describes handle inheritance when creating new processes. Handle
> inheritance must be enabled (bInheritHandles = TRUE) e.g. when you want
> to pass handles for stdin/stdout via lpStartupInfo.
> 
> Signed-off-by: Tobias Rapp 
> ---
>  libavutil/file_open.c | 3 +++
>  1 file changed, 3 insertions(+)

applied

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Ronald S. Bultje]

Hi,

On Mon, Nov 2, 2015 at 11:07 AM, Ganesh Ajjanagadde 
wrote:

> On Mon, Nov 2, 2015 at 10:28 AM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde 
> wrote:
> >
> >> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje 
> >> wrote:
> >>
> >> - I think the name of this function is confusing. The av_clip* namespace
> >> > seems reserved for int to int clips, so using it for a float to int
> >> > conversion with clip is kind of weird. I would use a different
> namespace
> >> > for it.
> >>
> >> Mathematically, it is the same as clipping, albeit "lossy", but then
> >> again all clipping is by nature "lossy" on inputs outside the clipping
> >> range, hence the choice of name. Any ideas for a namespace for this?
> >>
> >
> > Clip is the namespace for _just_ a clip. This function converts _and_
> > clips. How about av_rint64_clip.
>
> I like this name, thanks. Also, please tell me what to do about
> version bumps etc, so that I can get this done for v2.


I'm not sure actually.

I believe for new API you increment minor (see libavutil/version.h) by 1
and reset micro to 0. You bump major on API changes, and you bump micro for
behavioural changes (?).

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

Re: [FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 11:58 AM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Nov 2, 2015 at 11:07 AM, Ganesh Ajjanagadde 
> wrote:
>
>> On Mon, Nov 2, 2015 at 10:28 AM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde 
>> wrote:
>> >
>> >> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje 
>> >> wrote:
>> >>
>> >> - I think the name of this function is confusing. The av_clip* namespace
>> >> > seems reserved for int to int clips, so using it for a float to int
>> >> > conversion with clip is kind of weird. I would use a different
>> namespace
>> >> > for it.
>> >>
>> >> Mathematically, it is the same as clipping, albeit "lossy", but then
>> >> again all clipping is by nature "lossy" on inputs outside the clipping
>> >> range, hence the choice of name. Any ideas for a namespace for this?
>> >>
>> >
>> > Clip is the namespace for _just_ a clip. This function converts _and_
>> > clips. How about av_rint64_clip.
>>
>> I like this name, thanks. Also, please tell me what to do about
>> version bumps etc, so that I can get this done for v2.
>
>
> I'm not sure actually.
>
> I believe for new API you increment minor (see libavutil/version.h) by 1
> and reset micro to 0. You bump major on API changes, and you bump micro for
> behavioural changes (?).

Hmm, then there is a micro bump that I missed: av_gcd works
differently for negative inputs than before: previously it was
undefined in terms of its documentation, not actuality I believe, but
now it has well defined semantics for all int64_t, and this has been
documented.

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

[FFmpeg-devel] Monochrome 12bit compression help

[Ken Milnes]

Our company is working on a project that requires some video compression.
In our application we are capturing 12 bit monochrome video using IP
cameras.  The video is taken from fixed cameras on a golf course putting
green.   The camera video is approximately 1900 X 1200 pixels, 12 bits, and
20 Hz frame rate.   The scene is largely static as the cameras are fixed.

 

Our desire is to compress this video for archival.  It is important that the
video is either lossless compressed or that the compression does little
damage because we want to run the machine vision tracking algorithms after
decompression.

 

We did some very preliminary tests using h.264 compression from the ffmpeg
library and was quite impressed with the results.  It looked like 100:1
compression was achievable without damaging the video.   The test was not
really valid because we use RGB compression and it was only 8 bit video.   I
could not find 12 bit, monochrome options in the library.

 

We are looking for a consultant to help select or modify a codec that is
optimal for this application.   I wish  to scope a project that would give
us an API that we could include in our software product that would compress
and uncompress our 12 bit monochrome video.

 

Our project schedule would require a finished API by the end of December.

 

Regards,

 

Ken

 

Ken Milnes

Project  Manager / System Engineer

Sportvision

www.sportvision.com

kenmil...@sportvision.com

 

 

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

[FFmpeg-devel] [PATCH] avformat/cache: Use int64_t to avoid int overflow in cache_read

[Bryan Huh]

Fixes an issue where an int64_t ffurl_seek return-value was being stored
in an int (32-bit) "r" variable, leading to integer overflow when seeking
into a large file (>2GB), and ultimately a "Failed to perform internal
seek" error mesage.

To test, try running `ffprobe 'cache:http://'` on a file that
is ~3GB large, whose moov atom is at the end of the file
---
 libavformat/cache.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libavformat/cache.c b/libavformat/cache.c
index a762aa9..31f63e6 100644
--- a/libavformat/cache.c
+++ b/libavformat/cache.c
@@ -156,7 +156,7 @@ static int cache_read(URLContext *h, unsigned char *buf, 
int size)
 {
 Context *c= h->priv_data;
 CacheEntry *entry, *next[2] = {NULL, NULL};
-int r;
+int64_t r;
 
 entry = av_tree_find(c->root, &c->logical_pos, cmp, (void**)next);
 
-- 
1.7.1

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

Re: [FFmpeg-devel] [PATCH] avformat/cache: Separate read and seek return-values within cache_read

[Bryan Huh]

Ok. Will push a separate patch with a one-line type-change and abandon this.

I wanted to avoid coercing "r" into a 32-bit int when add_entry is called.
I thought the compiler would give me warnings about it, but it doesn't seem
to be after all.

On Mon, Nov 2, 2015 at 7:04 AM, Michael Niedermayer 
wrote:

> On Sun, Nov 01, 2015 at 10:56:45PM -0800, Bryan Huh wrote:
> > Fixes an issue where an int64_t ffurl_seek return-value was being stored
> > in a generic int "r" variable, leading to integer overflow when seeking
> > into a large file (>2GB), and ultimately a "Failed to perform internal
> > seek" error mesage. The "r" variable was used both for storing the
> > result of any seeks and for the result of any reads. Instead, I separate
> > the variable into an int64_t seek-variable and int read-variable.
> >
> > To test, try running `ffprobe 'cache:http://'` on a file that
> > is ~3GB large, whose moov atom is at the end of the file
> > ---
> >  libavformat/cache.c |   49
> +
> >  1 files changed, 25 insertions(+), 24 deletions(-)
> >
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index a762aa9..b654f45 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -156,7 +156,8 @@ static int cache_read(URLContext *h, unsigned char
> *buf, int size)
> >  {
> >  Context *c= h->priv_data;
> >  CacheEntry *entry, *next[2] = {NULL, NULL};
>
> > -int r;
> > +int64_t off;
> > +int bytes_read = 0;
>
> renaming the variable is unrelated (its not neccessary)
> but if you want to rename/split it, i dont mind but it should be in
> a seperate patch
>
>
> >
> >  entry = av_tree_find(c->root, &c->logical_pos, cmp, (void**)next);
> >
> > @@ -169,21 +170,21 @@ static int cache_read(URLContext *h, unsigned char
> *buf, int size)
> >  if (in_block_pos < entry->size) {
> >  int64_t physical_target = entry->physical_pos +
> in_block_pos;
> >
>
> > -if (c->cache_pos != physical_target) {
> > +if (c->cache_pos != physical_target)
>
> removing {} is not needed, without it the change is smaller and
> simpeler to read (for example when someone reads git log changes)
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Avoid a single point of failure, be that a person or equipment.
>
> ___
> 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/2] first steps to resolving float to int undefined behavior

[Ronald S. Bultje]

Hi,

On Mon, Nov 2, 2015 at 12:32 PM, Ganesh Ajjanagadde 
wrote:

> On Mon, Nov 2, 2015 at 11:58 AM, Ronald S. Bultje 
> wrote:
> > Hi,
> >
> > On Mon, Nov 2, 2015 at 11:07 AM, Ganesh Ajjanagadde 
> > wrote:
> >
> >> On Mon, Nov 2, 2015 at 10:28 AM, Ronald S. Bultje 
> >> wrote:
> >> > Hi,
> >> >
> >> > On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde 
> >> wrote:
> >> >
> >> >> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje  >
> >> >> wrote:
> >> >>
> >> >> - I think the name of this function is confusing. The av_clip*
> namespace
> >> >> > seems reserved for int to int clips, so using it for a float to int
> >> >> > conversion with clip is kind of weird. I would use a different
> >> namespace
> >> >> > for it.
> >> >>
> >> >> Mathematically, it is the same as clipping, albeit "lossy", but then
> >> >> again all clipping is by nature "lossy" on inputs outside the
> clipping
> >> >> range, hence the choice of name. Any ideas for a namespace for this?
> >> >>
> >> >
> >> > Clip is the namespace for _just_ a clip. This function converts _and_
> >> > clips. How about av_rint64_clip.
> >>
> >> I like this name, thanks. Also, please tell me what to do about
> >> version bumps etc, so that I can get this done for v2.
> >
> >
> > I'm not sure actually.
> >
> > I believe for new API you increment minor (see libavutil/version.h) by 1
> > and reset micro to 0. You bump major on API changes, and you bump micro
> for
> > behavioural changes (?).
>
> Hmm, then there is a micro bump that I missed: av_gcd works
> differently for negative inputs than before: previously it was
> undefined in terms of its documentation, not actuality I believe, but
> now it has well defined semantics for all int64_t, and this has been
> documented.


A micro does not have to be bumped on the very commit. We can merge that
micro-bump together for various features, or we can forget it altogether
and it's simply part of the next (totally unrelated) microbump. You can
even bump it today if there has not been a bump since already.

Regardless of when the micro was bumped after the fact, the application
that relies on the new/fixed/changed behaviour can then use that micro as
minimum required runtime version. That this was committed a month later is
merely a detail that most users will never know or care for.

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

Re: [FFmpeg-devel] [PATCH 1/2] avutil/common: add av_clipd64

[Daniel Serpell]

Hi,

On Sun, Nov 01, 2015 at 01:03:35PM -0500, Ganesh Ajjanagadde wrote:
> The rationale for this function is reflected in the documentation for
> it, and is copied here:
> 
> Clip a double value into the long long amin-amax range.
> This function is needed because conversion of floating point to integers when
> it does not fit in the integer's representation does not necessarily saturate
> correctly (usually converted to a cvttsd2si on x86) which saturates numbers
> > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
> behavior, allowing this sort of mathematically bogus conversions. This 
> provides
> a safe alternative that is slower obviously but assures safety and better
> mathematical behavior.
> @param a value to clip
> @param amin minimum value of the clip range
> @param amax maximum value of the clip range
> @return clipped value
> 
> Note that a priori if one can guarantee from the calling side that the
> double is in range, it is safe to simply do an explicit/implicit cast,
> and that will be far faster. However, otherwise, this function should be
> used.
> 
> --
> As a general remark, Clang/GCC has a -Wfloat-conversion so that at least
> implicit conversions can be found. This helped me in auditing the
> codebase. In order to reduce noise while testing, an explicit cast on the 
> return
> was placed. I can remove it if people prefer, though I like the cast as
> it makes the intent of possible narrowing explicit.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavutil/common.h | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 6f0f582..e778dd2 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -298,6 +298,34 @@ static av_always_inline av_const double 
> av_clipd_c(double a, double amin, double
>  else   return a;
>  }
>  
> +/**
> + * Clip a double value into the long long amin-amax range.
> + * This function is needed because conversion of floating point to integers 
> when
> + * it does not fit in the integer's representation does not necessarily 
> saturate
> + * correctly (usually converted to a cvttsd2si on x86) which saturates 
> numbers
> + * > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
> + * behavior, allowing this sort of mathematically bogus conversions. This 
> provides
> + * a safe alternative that is slower obviously but assures safety and better
> + * mathematical behavior.
> + * @param a value to clip
> + * @param amin minimum value of the clip range
> + * @param amax maximum value of the clip range
> + * @return clipped value
> + */
> +static av_always_inline av_const int64_t av_clipd64_c(double a, int64_t 
> amin, int64_t amax)
> +{
> +#if defined(HAVE_AV_CONFIG_H) && defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
> +if (amin > amax) abort();
> +#endif
> +// INT64_MAX+1,INT64_MIN are exactly representable as IEEE doubles
> +if (a >= 9223372036854775808.0)
> +return INT64_MAX;
> +if (a <= INT64_MIN)
> +return INT64_MIN;
> +// Finally safe to call av_clipd_c
> +return (int64_t)av_clipd_c(a, amin, amax);
> +}
> +

This wont work, because double precision has less than 64bit of mantisa,
as the number 0x7C00 is the highest double precision number
that can be converted to a 64bit integer, values near INT64_MAX will be
rounded up to INT64_MAX and give undefined results.

So, I propose you do (a >= 9223372036854775296.0) instead, (this is
0x7E00, half the way to INT64_MAX+1).

Daniel.

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

Re: [FFmpeg-devel] [PATCH 1/2] avutil/common: add av_clipd64

[Daniel Serpell]

Hi,

On Sun, Nov 01, 2015 at 01:03:35PM -0500, Ganesh Ajjanagadde wrote:
> The rationale for this function is reflected in the documentation for
> it, and is copied here:
> 
> Clip a double value into the long long amin-amax range.
> This function is needed because conversion of floating point to integers when
> it does not fit in the integer's representation does not necessarily saturate
> correctly (usually converted to a cvttsd2si on x86) which saturates numbers
> > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
> behavior, allowing this sort of mathematically bogus conversions. This 
> provides
> a safe alternative that is slower obviously but assures safety and better
> mathematical behavior.
> @param a value to clip
> @param amin minimum value of the clip range
> @param amax maximum value of the clip range
> @return clipped value
> 
> Note that a priori if one can guarantee from the calling side that the
> double is in range, it is safe to simply do an explicit/implicit cast,
> and that will be far faster. However, otherwise, this function should be
> used.
> 
> --
> As a general remark, Clang/GCC has a -Wfloat-conversion so that at least
> implicit conversions can be found. This helped me in auditing the
> codebase. In order to reduce noise while testing, an explicit cast on the 
> return
> was placed. I can remove it if people prefer, though I like the cast as
> it makes the intent of possible narrowing explicit.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavutil/common.h | 31 +++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 6f0f582..e778dd2 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -298,6 +298,34 @@ static av_always_inline av_const double 
> av_clipd_c(double a, double amin, double
>  else   return a;
>  }
>  
> +/**
> + * Clip a double value into the long long amin-amax range.
> + * This function is needed because conversion of floating point to integers 
> when
> + * it does not fit in the integer's representation does not necessarily 
> saturate
> + * correctly (usually converted to a cvttsd2si on x86) which saturates 
> numbers
> + * > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
> + * behavior, allowing this sort of mathematically bogus conversions. This 
> provides
> + * a safe alternative that is slower obviously but assures safety and better
> + * mathematical behavior.
> + * @param a value to clip
> + * @param amin minimum value of the clip range
> + * @param amax maximum value of the clip range
> + * @return clipped value
> + */
> +static av_always_inline av_const int64_t av_clipd64_c(double a, int64_t 
> amin, int64_t amax)
> +{
> +#if defined(HAVE_AV_CONFIG_H) && defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
> +if (amin > amax) abort();
> +#endif
> +// INT64_MAX+1,INT64_MIN are exactly representable as IEEE doubles
> +if (a >= 9223372036854775808.0)
> +return INT64_MAX;
> +if (a <= INT64_MIN)
> +return INT64_MIN;
> +// Finally safe to call av_clipd_c
> +return (int64_t)av_clipd_c(a, amin, amax);
> +}
> +

Sorry, disregard my previous e-mail, this *will* work because "a" is
already a double, so there is no possibility for further rounding after
the comparison.

Daniel.

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

Re: [FFmpeg-devel] [PATCH 1/2] avutil/common: add av_clipd64

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 1:59 PM, Daniel Serpell  wrote:
> Hi,
>
> On Sun, Nov 01, 2015 at 01:03:35PM -0500, Ganesh Ajjanagadde wrote:
>> The rationale for this function is reflected in the documentation for
>> it, and is copied here:
>>
>> Clip a double value into the long long amin-amax range.
>> This function is needed because conversion of floating point to integers when
>> it does not fit in the integer's representation does not necessarily saturate
>> correctly (usually converted to a cvttsd2si on x86) which saturates numbers
>> > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
>> behavior, allowing this sort of mathematically bogus conversions. This 
>> provides
>> a safe alternative that is slower obviously but assures safety and better
>> mathematical behavior.
>> @param a value to clip
>> @param amin minimum value of the clip range
>> @param amax maximum value of the clip range
>> @return clipped value
>>
>> Note that a priori if one can guarantee from the calling side that the
>> double is in range, it is safe to simply do an explicit/implicit cast,
>> and that will be far faster. However, otherwise, this function should be
>> used.
>>
>> --
>> As a general remark, Clang/GCC has a -Wfloat-conversion so that at least
>> implicit conversions can be found. This helped me in auditing the
>> codebase. In order to reduce noise while testing, an explicit cast on the 
>> return
>> was placed. I can remove it if people prefer, though I like the cast as
>> it makes the intent of possible narrowing explicit.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavutil/common.h | 31 +++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/libavutil/common.h b/libavutil/common.h
>> index 6f0f582..e778dd2 100644
>> --- a/libavutil/common.h
>> +++ b/libavutil/common.h
>> @@ -298,6 +298,34 @@ static av_always_inline av_const double 
>> av_clipd_c(double a, double amin, double
>>  else   return a;
>>  }
>>
>> +/**
>> + * Clip a double value into the long long amin-amax range.
>> + * This function is needed because conversion of floating point to integers 
>> when
>> + * it does not fit in the integer's representation does not necessarily 
>> saturate
>> + * correctly (usually converted to a cvttsd2si on x86) which saturates 
>> numbers
>> + * > INT64_MAX to INT64_MIN. The standard marks such conversions as 
>> undefined
>> + * behavior, allowing this sort of mathematically bogus conversions. This 
>> provides
>> + * a safe alternative that is slower obviously but assures safety and better
>> + * mathematical behavior.
>> + * @param a value to clip
>> + * @param amin minimum value of the clip range
>> + * @param amax maximum value of the clip range
>> + * @return clipped value
>> + */
>> +static av_always_inline av_const int64_t av_clipd64_c(double a, int64_t 
>> amin, int64_t amax)
>> +{
>> +#if defined(HAVE_AV_CONFIG_H) && defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
>> +if (amin > amax) abort();
>> +#endif
>> +// INT64_MAX+1,INT64_MIN are exactly representable as IEEE doubles
>> +if (a >= 9223372036854775808.0)
>> +return INT64_MAX;
>> +if (a <= INT64_MIN)
>> +return INT64_MIN;
>> +// Finally safe to call av_clipd_c
>> +return (int64_t)av_clipd_c(a, amin, amax);
>> +}
>> +
>
> This wont work, because double precision has less than 64bit of mantisa,
> as the number 0x7C00 is the highest double precision number
> that can be converted to a 64bit integer, values near INT64_MAX will be
> rounded up to INT64_MAX and give undefined results.
>
> So, I propose you do (a >= 9223372036854775296.0) instead, (this is
> 0x7E00, half the way to INT64_MAX+1).

I see what you are saying, but think your reasoning is wrong. I have
tested with a ubsan build on clang and gcc.
The reason why the code is fine is as follows:
Values near INT64_MAX will be rounded to INT64_MAX + 1 (closest
representable point), and the first condition will hold true (yes, it
is weird, but that is what happens). Technically, they don't need to
round that way, C does not specify "nearest". All C specifies is that
it must be one of the two neighboring representation points. However,
this does not matter either, since if it rounds down, it will round
down subsequently in av_clipd_c, so it is ok. This is because the
floating point environment is unchanged during the call.

I think your confusion comes from thinking that INT64_MAX is
representable: it is not, hence the comment and choice of INT64_MAX +
1.

I suggest you try out a ubsan build to test it out - I did look at
values near INT64_MAX (the trouble spot), but it is entirely possible
my reasoning above is faulty. If you could reproduce evidence
supporting your claim, it will be very helpful.

>
> Daniel.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg

[FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Ganesh Ajjanagadde]

This improves accuracy for the bessel function, and this in turn should
improve the quality of the Kaiser window.
The algorithm is taken from the Boost project, who have done a detailed
investigation of the accuracy of their method, as compared with e.g the
GNU Scientific Library (GSL):
http://www.boost.org/doc/libs/1_52_0/libs/math/doc/sf_and_dist/html/math_toolkit/special/bessel/mbessel.html.
Boost source code (also cited in the code):
https://searchcode.com/codesearch/view/14918379/.

For easy reference, here are some sample accuracy values obtained as
follows. i0 denotes the old bessel code, i0_boost the approach here, and
i0_real an arbitrary precision result (truncated) from e.g Wolfram Alpha:
type e.g "bessel i0(6.0)" to reproduce. These are evaluation points that
occur for the default kaiser_beta = 9.

bessel(8.0)
i0  (8.00) = 427.564115721804739678191254
i0_boost(8.00) = 427.564115721804796521610115
i0_real (8.00) = 427.564115721804785177396791

bessel(6.0)
i0  (6.00) = 67.234406976477956163762428
i0_boost(6.00) = 67.234406976477970374617144
i0_real (6.00) = 67.234406976477975326188025

Main accuracy benefits come at larger bessel arguments, where the
Taylor-Maclaurin method is not that good: 23+ iterations
(at large arguments, since the series is about 0) can cause
significant floating point error accumulation.

Benchmarks are sort of besides the point, since this is in an init
function, and if one wants a fast window, Kaiser is likely not the best
idea. For that matter, if clients want fast reinit's, it may make more
sense to cache the results than evaluate again.

Also, I was unable to get multiple runs to do a proper
bench in FATE easily using simple methods such as stream_loop, since the init
is only called while changing the sample rate parameters
---
Note that there is another usage of bessel in libavcodec, so it may or
may not be useful to place this in libavutil someplace.
I wonder if there is a way to check easily in FATE the accuracy
benefits as translated to the actual resampling, as that is the main selling
point of this patch.

Signed-off-by: Ganesh Ajjanagadde 
---
 libswresample/resample.c | 99 +++-
 1 file changed, 72 insertions(+), 27 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 036eff3..695b23a 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -28,38 +28,83 @@
 #include "libavutil/avassert.h"
 #include "resample.h"
 
+static inline double eval_poly(const double *coeff, int size, double x) {
+double sum = coeff[size-1];
+int i;
+for (i = size-2; i >= 0; --i) {
+sum *= x;
+sum += coeff[i];
+}
+return sum;
+}
+
 /**
  * 0th order modified bessel function of the first kind.
+ * Algorithm taken from the Boost project, source:
+ * https://searchcode.com/codesearch/view/14918379/
  */
-static double bessel(double x){
-double lastv=0;
-double t, v;
-int i;
-static const double inv[100]={
- 1.0/( 1* 1), 1.0/( 2* 2), 1.0/( 3* 3), 1.0/( 4* 4), 1.0/( 5* 5), 1.0/( 6* 6), 
1.0/( 7* 7), 1.0/( 8* 8), 1.0/( 9* 9), 1.0/(10*10),
- 1.0/(11*11), 1.0/(12*12), 1.0/(13*13), 1.0/(14*14), 1.0/(15*15), 1.0/(16*16), 
1.0/(17*17), 1.0/(18*18), 1.0/(19*19), 1.0/(20*20),
- 1.0/(21*21), 1.0/(22*22), 1.0/(23*23), 1.0/(24*24), 1.0/(25*25), 1.0/(26*26), 
1.0/(27*27), 1.0/(28*28), 1.0/(29*29), 1.0/(30*30),
- 1.0/(31*31), 1.0/(32*32), 1.0/(33*33), 1.0/(34*34), 1.0/(35*35), 1.0/(36*36), 
1.0/(37*37), 1.0/(38*38), 1.0/(39*39), 1.0/(40*40),
- 1.0/(41*41), 1.0/(42*42), 1.0/(43*43), 1.0/(44*44), 1.0/(45*45), 1.0/(46*46), 
1.0/(47*47), 1.0/(48*48), 1.0/(49*49), 1.0/(50*50),
- 1.0/(51*51), 1.0/(52*52), 1.0/(53*53), 1.0/(54*54), 1.0/(55*55), 1.0/(56*56), 
1.0/(57*57), 1.0/(58*58), 1.0/(59*59), 1.0/(60*60),
- 1.0/(61*61), 1.0/(62*62), 1.0/(63*63), 1.0/(64*64), 1.0/(65*65), 1.0/(66*66), 
1.0/(67*67), 1.0/(68*68), 1.0/(69*69), 1.0/(70*70),
- 1.0/(71*71), 1.0/(72*72), 1.0/(73*73), 1.0/(74*74), 1.0/(75*75), 1.0/(76*76), 
1.0/(77*77), 1.0/(78*78), 1.0/(79*79), 1.0/(80*80),
- 1.0/(81*81), 1.0/(82*82), 1.0/(83*83), 1.0/(84*84), 1.0/(85*85), 1.0/(86*86), 
1.0/(87*87), 1.0/(88*88), 1.0/(89*89), 1.0/(90*90),
- 1.0/(91*91), 1.0/(92*92), 1.0/(93*93), 1.0/(94*94), 1.0/(95*95), 1.0/(96*96), 
1.0/(97*97), 1.0/(98*98), 1.0/(99*99), 1.0/(1)
+static double bessel(double x) {
+// Modified Bessel function of the first kind of order zero
+// minimax rational approximations on intervals, see
+// Blair and Edwards, Chalk River Report AECL-4928, 1974
+static const double p1[] = {
+-2.2335582639474375249e+15,
+-5.5050369673018427753e+14,
+-3.2940087627407749166e+13,
+-8.4925101247114157499e+11,
+-1.1912746104985237192e+10,
+-1.0313066708737980747e+08,
+-5.9545626019847898221e+05,
+-2.4125195876041896775e+03,
+   

Re: [FFmpeg-devel] [PATCH 1/4] fate: add concat demuxer tests

[Marton Balint]

On Mon, 2 Nov 2015, Michael Niedermayer wrote:


On Mon, Nov 02, 2015 at 12:15:55AM +0100, Marton Balint wrote:

Signed-off-by: Marton Balint 
---
 tests/Makefile |1 +
 tests/extended.ffconcat|  114 +
 tests/fate-run.sh  |   20 +
 tests/fate/concatdec.mak   |   21 +
 tests/ref/fate/concat-demuxer-extended-lavf-mxf|1 +
 .../ref/fate/concat-demuxer-extended-lavf-mxf_d10  |1 +
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 1713 ++
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 1193 ++
 tests/ref/fate/concat-demuxer-simple2-lavf-ts  | 2326 
 tests/simple1.ffconcat |   12 +
 tests/simple2.ffconcat |   19 +
 11 files changed, 5421 insertions(+)
 create mode 100644 tests/extended.ffconcat
 create mode 100644 tests/fate/concatdec.mak
 create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf
 create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
 create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf
 create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
 create mode 100644 tests/ref/fate/concat-demuxer-simple2-lavf-ts
 create mode 100644 tests/simple1.ffconcat
 create mode 100644 tests/simple2.ffconcat


seems to break fate here:



Probably because of my mistake of using double = in fate-run.sh... Will 
send a new patch.


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

Re: [FFmpeg-devel] Monochrome 12bit compression help

[Paul B Mahol]

Dana 2. 11. 2015. 19:08 osoba "Ken Milnes" 
napisala je:
>
> Our company is working on a project that requires some video compression.
> In our application we are capturing 12 bit monochrome video using IP
> cameras.  The video is taken from fixed cameras on a golf course putting
> green.   The camera video is approximately 1900 X 1200 pixels, 12 bits,
and
> 20 Hz frame rate.   The scene is largely static as the cameras are fixed.
>
>
>
> Our desire is to compress this video for archival.  It is important that
the
> video is either lossless compressed or that the compression does little
> damage because we want to run the machine vision tracking algorithms after
> decompression.
>
>
>
> We did some very preliminary tests using h.264 compression from the ffmpeg
> library and was quite impressed with the results.  It looked like 100:1
> compression was achievable without damaging the video.   The test was not
> really valid because we use RGB compression and it was only 8 bit video.
 I
> could not find 12 bit, monochrome options in the library.
>

You need to rebuild x264 and ffmpeg with highbit support. You can't use
options.

>
>
> We are looking for a consultant to help select or modify a codec that is
> optimal for this application.   I wish  to scope a project that would give
> us an API that we could include in our software product that would
compress
> and uncompress our 12 bit monochrome video.
>
>
>
> Our project schedule would require a finished API by the end of December.
>
>
>
> Regards,
>
>
>
> Ken
>
>
>
> Ken Milnes
>
> Project  Manager / System Engineer
>
> Sportvision
>
> www.sportvision.com
>
> kenmil...@sportvision.com
>
>
>
>
>
> ___
> 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] [PATCHv2 1/4] fate: add concat demuxer tests

[Marton Balint]

Signed-off-by: Marton Balint 
---
 tests/Makefile |1 +
 tests/extended.ffconcat|  114 +
 tests/fate-run.sh  |   20 +
 tests/fate/concatdec.mak   |   21 +
 tests/ref/fate/concat-demuxer-extended-lavf-mxf|1 +
 .../ref/fate/concat-demuxer-extended-lavf-mxf_d10  |1 +
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 1713 ++
 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 1193 ++
 tests/ref/fate/concat-demuxer-simple2-lavf-ts  | 2326 
 tests/simple1.ffconcat |   12 +
 tests/simple2.ffconcat |   19 +
 11 files changed, 5421 insertions(+)
 create mode 100644 tests/extended.ffconcat
 create mode 100644 tests/fate/concatdec.mak
 create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf
 create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
 create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf
 create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
 create mode 100644 tests/ref/fate/concat-demuxer-simple2-lavf-ts
 create mode 100644 tests/simple1.ffconcat
 create mode 100644 tests/simple2.ffconcat

diff --git a/tests/Makefile b/tests/Makefile
index 7ee4a46..62544d0 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -113,6 +113,7 @@ include $(SRC_PATH)/tests/fate/audio.mak
 include $(SRC_PATH)/tests/fate/bmp.mak
 include $(SRC_PATH)/tests/fate/cdxl.mak
 include $(SRC_PATH)/tests/fate/checkasm.mak
+include $(SRC_PATH)/tests/fate/concatdec.mak
 include $(SRC_PATH)/tests/fate/cover-art.mak
 include $(SRC_PATH)/tests/fate/demux.mak
 include $(SRC_PATH)/tests/fate/dfa.mak
diff --git a/tests/extended.ffconcat b/tests/extended.ffconcat
new file mode 100644
index 000..7359113
--- /dev/null
+++ b/tests/extended.ffconcat
@@ -0,0 +1,114 @@
+ffconcat version 1.0
+
+file  %SRCFILE%
+
+file  %SRCFILE%
+duration  1
+file_packet_metadata dummy=1
+
+file  %SRCFILE%
+inpoint   00:00.00
+outpoint  00:00.04
+
+file  %SRCFILE%
+inpoint   00:00.04
+outpoint  00:00.08
+
+file  %SRCFILE%
+inpoint   00:00.08
+outpoint  00:00.12
+
+file  %SRCFILE%
+inpoint   00:00.12
+outpoint  00:00.16
+
+file  %SRCFILE%
+inpoint   00:00.16
+outpoint  00:00.20
+
+file  %SRCFILE%
+inpoint   00:00.20
+outpoint  00:00.24
+
+file  %SRCFILE%
+inpoint   00:00.24
+outpoint  00:00.28
+
+file  %SRCFILE%
+inpoint   00:00.28
+outpoint  00:00.32
+
+file  %SRCFILE%
+inpoint   00:00.32
+outpoint  00:00.36
+
+file  %SRCFILE%
+inpoint   00:00.36
+outpoint  00:00.40
+
+file  %SRCFILE%
+inpoint   00:00.40
+outpoint  00:00.44
+
+file  %SRCFILE%
+inpoint   00:00.44
+outpoint  00:00.48
+
+file  %SRCFILE%
+inpoint   00:00.48
+outpoint  00:00.52
+
+file  %SRCFILE%
+inpoint   00:00.52
+outpoint  00:00.56
+
+file  %SRCFILE%
+inpoint   00:00.56
+outpoint  00:00.60
+
+file  %SRCFILE%
+inpoint   00:00.60
+outpoint  00:00.64
+
+file  %SRCFILE%
+inpoint   00:00.64
+outpoint  00:00.68
+
+file  %SRCFILE%
+inpoint   00:00.68
+outpoint  00:00.72
+
+file  %SRCFILE%
+inpoint   00:00.72
+outpoint  00:00.76
+
+file  %SRCFILE%
+inpoint   00:00.76
+outpoint  00:00.80
+
+file  %SRCFILE%
+inpoint   00:00.80
+outpoint  00:00.84
+
+file  %SRCFILE%
+inpoint   00:00.84
+outpoint  00:00.88
+
+file  %SRCFILE%
+inpoint   00:00.88
+outpoint  00:00.92
+
+file  %SRCFILE%
+inpoint   00:00.92
+outpoint  00:00.96
+
+file  %SRCFILE%
+inpoint   00:00.96
+outpoint  00:01.00
+
+file  %SRCFILE%
+outpoint  00:00.40
+
+file  %SRCFILE%
+inpoint   00:00.40
+
diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index a3938dc..612d9a2 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -249,6 +249,26 @@ gapless(){
 do_md5sum $decfile3
 }
 
+concat(){
+template=tests/$1
+sample=$(target_path $2)
+mode=$3
+extra_args=$4
+
+concatfile="${outdir}/${test}.ffconcat"
+packetfile="${outdir}/${test}.ffprobe"
+cleanfiles="$concatfile $packetfile"
+
+awk "{gsub(/%SRCFILE%/, \"$sample\"); print}" $template > $concatfile
+
+if [ "$mode" = "md5" ]; then
+  run ffprobe${PROGSUF} -show_streams -show_packets -v 0 -fflags keepside 
-safe 0 $extra_args $concatfile > $packetfile
+  do_md5sum $packetfile
+else
+  run ffprobe${PROGSUF} -show_streams -show_packets -v 0 -fflags keepside 
-safe 0 $extra_args $concatfile
+fi
+}
+
 mkdir -p "$outdir"
 
 # Disable globbing: command arguments may contain globbing characters and
diff --git a/tests/fate/concatdec.mak b/tests/fate/concatdec.mak
new file mode 100644
index 000..344cbc2
--- /dev/null
+++ b/tests/fate/concatdec.mak
@@ -0,0 +1,21 @@
+FATE_CONCAT_DEMUXER_SIMPLE1_LAVF-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF)   
+= mxf
+FATE_CONCAT_DEMUXER_SIMPLE1_LAVF-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF)   
+= mxf_d10
+
+

Re: [FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Hendrik Leppkes]

On Mon, Nov 2, 2015 at 7:57 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Mon, Nov 2, 2015 at 12:32 PM, Ganesh Ajjanagadde 
> wrote:
>
>> On Mon, Nov 2, 2015 at 11:58 AM, Ronald S. Bultje 
>> wrote:
>> > Hi,
>> >
>> > On Mon, Nov 2, 2015 at 11:07 AM, Ganesh Ajjanagadde 
>> > wrote:
>> >
>> >> On Mon, Nov 2, 2015 at 10:28 AM, Ronald S. Bultje 
>> >> wrote:
>> >> > Hi,
>> >> >
>> >> > On Sun, Nov 1, 2015 at 8:20 PM, Ganesh Ajjanagadde 
>> >> wrote:
>> >> >
>> >> >> On Sun, Nov 1, 2015 at 7:59 PM, Ronald S. Bultje > >
>> >> >> wrote:
>> >> >>
>> >> >> - I think the name of this function is confusing. The av_clip*
>> namespace
>> >> >> > seems reserved for int to int clips, so using it for a float to int
>> >> >> > conversion with clip is kind of weird. I would use a different
>> >> namespace
>> >> >> > for it.
>> >> >>
>> >> >> Mathematically, it is the same as clipping, albeit "lossy", but then
>> >> >> again all clipping is by nature "lossy" on inputs outside the
>> clipping
>> >> >> range, hence the choice of name. Any ideas for a namespace for this?
>> >> >>
>> >> >
>> >> > Clip is the namespace for _just_ a clip. This function converts _and_
>> >> > clips. How about av_rint64_clip.
>> >>
>> >> I like this name, thanks. Also, please tell me what to do about
>> >> version bumps etc, so that I can get this done for v2.
>> >
>> >
>> > I'm not sure actually.
>> >
>> > I believe for new API you increment minor (see libavutil/version.h) by 1
>> > and reset micro to 0. You bump major on API changes, and you bump micro
>> for
>> > behavioural changes (?).
>>
>> Hmm, then there is a micro bump that I missed: av_gcd works
>> differently for negative inputs than before: previously it was
>> undefined in terms of its documentation, not actuality I believe, but
>> now it has well defined semantics for all int64_t, and this has been
>> documented.
>
>
> A micro does not have to be bumped on the very commit. We can merge that
> micro-bump together for various features, or we can forget it altogether
> and it's simply part of the next (totally unrelated) microbump. You can
> even bump it today if there has not been a bump since already.
>
> Regardless of when the micro was bumped after the fact, the application
> that relies on the new/fixed/changed behaviour can then use that micro as
> minimum required runtime version. That this was committed a month later is
> merely a detail that most users will never know or care for.
>

We don't really bump micro in the majority of cases anyway, not every
bugfix or change needs a bump.
Its more important to remember to bump minor when adding new API or
similar, so people can use that version to know if its present.

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

Re: [FFmpeg-devel] [PATCH 1/2] ffmpeg: Don't try and write sdp info if none of the outputs had an rtp format.

[Simon Thelen]

On 15-11-02 at 15:09, Carl Eugen Hoyos wrote:
> Simon Thelen  c-14.de> writes:
> 
> > +   if (j == 0)
> > +   goto fail;
> 
> Tabs cannot be committed to our repository, please 
> remove them (this has to be fixed).
> Most FFmpeg code uses "if (!j)", feel free to ignore.
Looks like my gitconfig for ffmpeg lost the warn for tab-in-indent.
fixed in v2.

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

Re: [FFmpeg-devel] libavformat/segment : add option to increment timecode

[Martin Vignali]

ping

2015-10-03 15:49 GMT+02:00 Martin Vignali :

> Hello,
>
> in attach a patch, who add an option to increment timecode using segment.
>
> To test :
>
> ffmpeg -i src.mov -timecode 10:00:00:00 -vcodec copy -f segment
> -segment_time 2 -reset_timestamps 1 -increment_tc 1 target_%03d.mov
>
> the second file have timecode 10:00:02:00 (after the patch)
> instead of 10:00:00:00 (before the patch)
>
>
> This patch is useful for two kind of uses :
> - splitting a file, but keeping timecode of each part
> - have a continuous timecode when recording a stream using segment.
>
>
> Comments welcome.
>
> Martin
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[wm4]

On Mon,  2 Nov 2015 14:49:54 -0500
Ganesh Ajjanagadde  wrote:

> This improves accuracy for the bessel function, and this in turn should
> improve the quality of the Kaiser window.


"Should"? Does it or does it not? If you don't know, why the patch?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
> On Mon,  2 Nov 2015 14:49:54 -0500
> Ganesh Ajjanagadde  wrote:
>
>> This improves accuracy for the bessel function, and this in turn should
>> improve the quality of the Kaiser window.
>
>
> "Should"? Does it or does it not? If you don't know, why the patch?

It improves the window in the sense of mathematically matching the
Kaiser window expression due to the improved bessel function accuracy.
That claim I have verified and placed in the commit message with
evidence.

What that translates into in terms of resampling accuracy, I don't
know. Normally, such things do help reduce the noise floor, but I
don't know an easy way to test via FATE or associated tools. I put
that caveat in the bottom lines of the patch.

> ___
> 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] swresample/resample: improve bessel function accuracy

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 6:49 PM, Ganesh Ajjanagadde  wrote:
> On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
>> On Mon,  2 Nov 2015 14:49:54 -0500
>> Ganesh Ajjanagadde  wrote:
>>
>>> This improves accuracy for the bessel function, and this in turn should
>>> improve the quality of the Kaiser window.
>>
>>
>> "Should"? Does it or does it not? If you don't know, why the patch?
>
> It improves the window in the sense of mathematically matching the
> Kaiser window expression due to the improved bessel function accuracy.
> That claim I have verified and placed in the commit message with
> evidence.
>
> What that translates into in terms of resampling accuracy, I don't
> know. Normally, such things do help reduce the noise floor, but I
> don't know an easy way to test via FATE or associated tools. I put
> that caveat in the bottom lines of the patch.

Turns out the init speed is definitely improved as well (~20% boost
with default settings).
I did a cheap trick of calling build filter 1000 times to get a large
number of runs.
Results (x86-64, Haswell, GNU/Linux):

test: fate-swr-resample-dblp-44100-2626

new:
995894468 decicycles in build_filter(loop 1000), 256 runs,  0 skips
1029719302 decicycles in build_filter(loop 1000), 512 runs,  0 skips
984101131 decicycles in build_filter(loop 1000),1024 runs,  0 skips

old:
1250020763 decicycles in build_filter(loop 1000), 256 runs,  0 skips
1246353282 decicycles in build_filter(loop 1000), 512 runs,  0 skips
1220017565 decicycles in build_filter(loop 1000),1024 runs,  0 skips

Thus, this patch has both things going for it luckily. Will leave to
the maintainer (Michael I believe) to give details of accuracy
benefits as translated to the actual resampling if easily testable,
and I will add the perf numbers to the message.

>
>> ___
>> 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] [PATCHv2 1/4] fate: add concat demuxer tests

[Michael Niedermayer]

On Mon, Nov 02, 2015 at 09:12:27PM +0100, Marton Balint wrote:
> Signed-off-by: Marton Balint 
> ---
>  tests/Makefile |1 +
>  tests/extended.ffconcat|  114 +
>  tests/fate-run.sh  |   20 +
>  tests/fate/concatdec.mak   |   21 +
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf|1 +
>  .../ref/fate/concat-demuxer-extended-lavf-mxf_d10  |1 +
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf | 1713 ++
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 | 1193 ++
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts  | 2326 
> 
>  tests/simple1.ffconcat |   12 +
>  tests/simple2.ffconcat |   19 +
>  11 files changed, 5421 insertions(+)
>  create mode 100644 tests/extended.ffconcat
>  create mode 100644 tests/fate/concatdec.mak
>  create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf
>  create mode 100644 tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
>  create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf
>  create mode 100644 tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
>  create mode 100644 tests/ref/fate/concat-demuxer-simple2-lavf-ts
>  create mode 100644 tests/simple1.ffconcat
>  create mode 100644 tests/simple2.ffconcat

this works better but still fails when build from a subdirectory
mkdir foobar ; cd foobar
make distclean ; ../configure && make -j12 
fate-concat-demuxer-extended-lavf-mxf fate-concat-demuxer-extended-lavf-mxf_d10 
fate-concat-demuxer-simple1-lavf-mxf fate-concat-demuxer-simple1-lavf-mxf_d10 
fate-concat-demuxer-simple2-lavf-ts

cat tests/data/fate/concat-demuxer-simple2-lavf-ts.err
awk: cmd. line:1: fatal: cannot open file `tests/simple2.ffconcat' for reading 
(No such file or directory)

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 


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

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Ganesh Ajjanagadde]

On Mon, Nov 2, 2015 at 9:32 PM, Ganesh Ajjanagadde  wrote:
> On Mon, Nov 2, 2015 at 6:49 PM, Ganesh Ajjanagadde  wrote:
>> On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
>>> On Mon,  2 Nov 2015 14:49:54 -0500
>>> Ganesh Ajjanagadde  wrote:
>>>
 This improves accuracy for the bessel function, and this in turn should
 improve the quality of the Kaiser window.
>>>
>>>
>>> "Should"? Does it or does it not? If you don't know, why the patch?
>>
>> It improves the window in the sense of mathematically matching the
>> Kaiser window expression due to the improved bessel function accuracy.
>> That claim I have verified and placed in the commit message with
>> evidence.
>>
>> What that translates into in terms of resampling accuracy, I don't
>> know. Normally, such things do help reduce the noise floor, but I
>> don't know an easy way to test via FATE or associated tools. I put
>> that caveat in the bottom lines of the patch.
>
> Turns out the init speed is definitely improved as well (~20% boost
> with default settings).
> I did a cheap trick of calling build filter 1000 times to get a large
> number of runs.
> Results (x86-64, Haswell, GNU/Linux):
>
> test: fate-swr-resample-dblp-44100-2626
>
> new:
> 995894468 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 1029719302 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 984101131 decicycles in build_filter(loop 1000),1024 runs,  0 skips
>
> old:
> 1250020763 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 1246353282 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 1220017565 decicycles in build_filter(loop 1000),1024 runs,  0 skips
>
> Thus, this patch has both things going for it luckily. Will leave to
> the maintainer (Michael I believe) to give details of accuracy
> benefits as translated to the actual resampling if easily testable,
> and I will add the perf numbers to the message.

One last comment on performance (this is something I have discussed
with Timothy privately), if one removes the crippling
-fno-tree-vectorize, FATE still passes on my GCC 5.2 setup, and one
gets further ~5 % perf improvement here:
949318408 decicycles in build_filter(loop 1000), 256 runs,  0 skips
948795082 decicycles in build_filter(loop 1000), 512 runs,  0 skips
928925076 decicycles in build_filter(loop 1000),1024 runs,  0 skips

I am sure a lot of other places may benefit. Yes, I know it was buggy
on older GCC versions, etc, but I see no reason to cripple users with
modern toolchains.
The commit 973859f5230e77beea7bb59dc081870689d6d191 is ancient and has
a "sloppy" commit message.

"It provides no speed benefit" is not true, and even back then was
never backed up by hard numbers, though the commit would have been ok
on non x86 at the time, possibly even now. As can be seen clearly from
https://ffmpeg.org/pipermail/ffmpeg-devel/2009-July/069586.html, much
of Mans's evidence came from non x86 architectures which are Mans's
expertise. In the absence of any complaints, the change was committed
and x86 people have suffered from it. Yes, particular versions may be
buggy, but such things should be checked more carefully especially
since this is "free performance".

Mans's argument was that adventurous people could add it back in.
Needless to say, it was not documented anywhere, and casual users like
me (and I suspect some others here) were completely unaware of this.
It is quite obscure and easy to miss unless one digs through configure
(or config.mak), or finds out from someone else.

I also recall (and this prompted the private conversation with
Timothy) that Timothy's memcpy change from the loop actually yield no
difference with a half decent auto-vectoriser. It is a low hanging
fruit in my view, and I have posted a perf number above.

At the very least, we should inform users of this low hanging thing
e.g by printing out during configure an "info" message - they can try
out an "adventurous" build with this, test out FATE, and if it passes,
go ahead with a new build.
Or we could have a "dev option" for --enable-vectorize.
Or at a more ambitious scale, we could collect configs where this will
work, test for them, and only disable otherwise.
There are a number of possibilities, for which a separate thread is better.

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

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Michael Niedermayer]

On Mon, Nov 02, 2015 at 09:32:13PM -0500, Ganesh Ajjanagadde wrote:
> On Mon, Nov 2, 2015 at 6:49 PM, Ganesh Ajjanagadde  wrote:
> > On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
> >> On Mon,  2 Nov 2015 14:49:54 -0500
> >> Ganesh Ajjanagadde  wrote:
> >>
> >>> This improves accuracy for the bessel function, and this in turn should
> >>> improve the quality of the Kaiser window.
> >>
> >>
> >> "Should"? Does it or does it not? If you don't know, why the patch?
> >
> > It improves the window in the sense of mathematically matching the
> > Kaiser window expression due to the improved bessel function accuracy.
> > That claim I have verified and placed in the commit message with
> > evidence.
> >
> > What that translates into in terms of resampling accuracy, I don't
> > know. Normally, such things do help reduce the noise floor, but I
> > don't know an easy way to test via FATE or associated tools. I put
> > that caveat in the bottom lines of the patch.
> 
> Turns out the init speed is definitely improved as well (~20% boost
> with default settings).
> I did a cheap trick of calling build filter 1000 times to get a large
> number of runs.
> Results (x86-64, Haswell, GNU/Linux):
> 
> test: fate-swr-resample-dblp-44100-2626
> 
> new:
> 995894468 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 1029719302 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 984101131 decicycles in build_filter(loop 1000),1024 runs,  0 skips
> 
> old:
> 1250020763 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 1246353282 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 1220017565 decicycles in build_filter(loop 1000),1024 runs,  0 skips
> 
> Thus, this patch has both things going for it luckily. Will leave to
> the maintainer (Michael I believe) to give details of accuracy
> benefits as translated to the actual resampling if easily testable,
> and I will add the perf numbers to the message.

the patch should be ok
its faster, its more accurate

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein


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

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Rostislav Pehlivanov]

>if one removes the crippling
>-fno-tree-vectorize
Yes, I think a config option to turn this flag on (like the unsafe
bitstream reader) would be good. Defaulting to off by default if it doesn't
break anything for at least a few people (and compilers) who test it. It's
not a big performance impact but every little bit counts nowadays.

On 3 November 2015 at 03:10, Ganesh Ajjanagadde  wrote:

> On Mon, Nov 2, 2015 at 9:32 PM, Ganesh Ajjanagadde 
> wrote:
> > On Mon, Nov 2, 2015 at 6:49 PM, Ganesh Ajjanagadde 
> wrote:
> >> On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
> >>> On Mon,  2 Nov 2015 14:49:54 -0500
> >>> Ganesh Ajjanagadde  wrote:
> >>>
>  This improves accuracy for the bessel function, and this in turn
> should
>  improve the quality of the Kaiser window.
> >>>
> >>>
> >>> "Should"? Does it or does it not? If you don't know, why the patch?
> >>
> >> It improves the window in the sense of mathematically matching the
> >> Kaiser window expression due to the improved bessel function accuracy.
> >> That claim I have verified and placed in the commit message with
> >> evidence.
> >>
> >> What that translates into in terms of resampling accuracy, I don't
> >> know. Normally, such things do help reduce the noise floor, but I
> >> don't know an easy way to test via FATE or associated tools. I put
> >> that caveat in the bottom lines of the patch.
> >
> > Turns out the init speed is definitely improved as well (~20% boost
> > with default settings).
> > I did a cheap trick of calling build filter 1000 times to get a large
> > number of runs.
> > Results (x86-64, Haswell, GNU/Linux):
> >
> > test: fate-swr-resample-dblp-44100-2626
> >
> > new:
> > 995894468 decicycles in build_filter(loop 1000), 256 runs,  0
> skips
> > 1029719302 decicycles in build_filter(loop 1000), 512 runs,  0
> skips
> > 984101131 decicycles in build_filter(loop 1000),1024 runs,  0
> skips
> >
> > old:
> > 1250020763 decicycles in build_filter(loop 1000), 256 runs,  0
> skips
> > 1246353282 decicycles in build_filter(loop 1000), 512 runs,  0
> skips
> > 1220017565 decicycles in build_filter(loop 1000),1024 runs,  0
> skips
> >
> > Thus, this patch has both things going for it luckily. Will leave to
> > the maintainer (Michael I believe) to give details of accuracy
> > benefits as translated to the actual resampling if easily testable,
> > and I will add the perf numbers to the message.
>
> One last comment on performance (this is something I have discussed
> with Timothy privately), if one removes the crippling
> -fno-tree-vectorize, FATE still passes on my GCC 5.2 setup, and one
> gets further ~5 % perf improvement here:
> 949318408 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 948795082 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 928925076 decicycles in build_filter(loop 1000),1024 runs,  0 skips
>
> I am sure a lot of other places may benefit. Yes, I know it was buggy
> on older GCC versions, etc, but I see no reason to cripple users with
> modern toolchains.
> The commit 973859f5230e77beea7bb59dc081870689d6d191 is ancient and has
> a "sloppy" commit message.
>
> "It provides no speed benefit" is not true, and even back then was
> never backed up by hard numbers, though the commit would have been ok
> on non x86 at the time, possibly even now. As can be seen clearly from
> https://ffmpeg.org/pipermail/ffmpeg-devel/2009-July/069586.html, much
> of Mans's evidence came from non x86 architectures which are Mans's
> expertise. In the absence of any complaints, the change was committed
> and x86 people have suffered from it. Yes, particular versions may be
> buggy, but such things should be checked more carefully especially
> since this is "free performance".
>
> Mans's argument was that adventurous people could add it back in.
> Needless to say, it was not documented anywhere, and casual users like
> me (and I suspect some others here) were completely unaware of this.
> It is quite obscure and easy to miss unless one digs through configure
> (or config.mak), or finds out from someone else.
>
> I also recall (and this prompted the private conversation with
> Timothy) that Timothy's memcpy change from the loop actually yield no
> difference with a half decent auto-vectoriser. It is a low hanging
> fruit in my view, and I have posted a perf number above.
>
> At the very least, we should inform users of this low hanging thing
> e.g by printing out during configure an "info" message - they can try
> out an "adventurous" build with this, test out FATE, and if it passes,
> go ahead with a new build.
> Or we could have a "dev option" for --enable-vectorize.
> Or at a more ambitious scale, we could collect configs where this will
> work, test for them, and only disable otherwise.
> There are a number of possibilities, for which a separate thread is better.
>
> >
> >>
> >>> ___
> >>> ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Timothy Gu]

On Mon, Nov 2, 2015 at 8:23 PM Rostislav Pehlivanov 
wrote:

> >if one removes the crippling
> >-fno-tree-vectorize
> Yes, I think a config option to turn this flag on (like the unsafe
> bitstream reader) would be good. Defaulting to off by default if it doesn't
> break anything for at least a few people (and compilers) who test it. It's
> not a big performance impact but every little bit counts nowadays.
>

FWIW, I recently (i.e. 2 days ago) did some tests with auto-vectorization
and a few compilers. Fortunately, none of the compilers I tested caused any
miscompilation, when purely measured by FATE:

compiling:
clang3.7 4m3.034s
gcc5vectorize 5m50.637s (1.14x gcc5)
gcc5 5m7.262s
gcc4.9vectorize 5m29.669s (1.11x gcc4.9)
gcc4.9 4m54.602s
gcc4.8vectorize 5m18.848s (1.09x gcc4.8)
gcc4.8 4m53.940s

FATE:
clang3.7 3m13.923s
gcc5vectorize 3m5.988s (0.980x gcc5)
gcc5 3m9.618s
gcc4.9vectorize 3m12.880s (0.983x gcc4.9)
gcc4.9 3m16.563s
gcc4.8vectorize 3m10.321s (0.993x gcc4.8)
gcc4.8 3m11.608s

Tested with:
- Debian jessie/stable/8.2
- Dual-core Haswell i7 ultra low voltage
- clang-3.7 3.7.0-svn251177-1~exp1 (from the offical clang apt repo)
- gcc-5 (Debian 5.2.1-22) 5.2.1 20151010 (Debian testing stock)
- gcc-4.9 (Debian 4.9.2-10) 4.9.2 (Debian stable stock)
- gcc-4.8 (Debian 4.8.4-1) 4.8.4 (Debian stable stock)

Note that FATE is probably the worst benchmark one can find, but it does
show something.

Some observations:

- GCC vectorization slows down compilation A LOT in all versions. The newer
the worse.
- If you are developing, use clang, and DON'T use GCC 5 with vectorization.
- For release builds, an option to turn it on (or rather to not turn it
off) would be helpful; but if you really care about performance _that_ much
then you should probably use some other compilers instead.

FYI, as I have told Ganesh so in our private exchanges, I did also test
vectorization on GCC 4.6 on a Ubuntu 12.04/Precise box, which miscompiled
the code hilariously, _and_ made the code slower, just as illustrated in
Mans's commit message.

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

[FFmpeg-devel] [PATCH] fate: replace custom md5 tests with those from RFC 1321

[James Almer]

Signed-off-by: James Almer 
---
 libavutil/md5.c| 23 ++-
 tests/ref/fate/md5 | 12 +++-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/libavutil/md5.c b/libavutil/md5.c
index 876bd55..2a77304 100644
--- a/libavutil/md5.c
+++ b/libavutil/md5.c
@@ -217,19 +217,16 @@ static void print_md5(uint8_t *md5)
 
 int main(void){
 uint8_t md5val[16];
-int i;
-volatile uint8_t in[1000]; // volatile to workaround 
http://llvm.org/bugs/show_bug.cgi?id=20849
-// FIXME remove volatile once it has been fixed and all fate clients are 
updated
-
-for (i = 0; i < 1000; i++)
-in[i] = i * i;
-av_md5_sum(md5val, in, 1000); print_md5(md5val);
-av_md5_sum(md5val, in,   63); print_md5(md5val);
-av_md5_sum(md5val, in,   64); print_md5(md5val);
-av_md5_sum(md5val, in,   65); print_md5(md5val);
-for (i = 0; i < 1000; i++)
-in[i] = i % 127;
-av_md5_sum(md5val, in,  999); print_md5(md5val);
+
+av_md5_sum(md5val, "",0); print_md5(md5val);
+av_md5_sum(md5val, "a",   1); print_md5(md5val);
+av_md5_sum(md5val, "abc", 3); print_md5(md5val);
+av_md5_sum(md5val, "message digest", 14); print_md5(md5val);
+av_md5_sum(md5val, "abcdefghijklmnopqrstuvwxyz", 26); print_md5(md5val);
+av_md5_sum(md5val, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcde"
+   "fghijklmnopqrstuvwxyz0123456789",  62); 
print_md5(md5val);
+av_md5_sum(md5val, "1234567890123456789012345678901234567890"
+   "1234567890123456789012345678901234567890", 80); 
print_md5(md5val);
 
 return 0;
 }
diff --git a/tests/ref/fate/md5 b/tests/ref/fate/md5
index af08a84..bb86bb6 100644
--- a/tests/ref/fate/md5
+++ b/tests/ref/fate/md5
@@ -1,5 +1,7 @@
-0bf1bcc8a1d72e2cf58d42182b637e56
-993a3eb298e52aca83ecfbb6a766b4d0
-07c01ca7c733475fad38c84c56f305c1
-9fc8404827cac26385f48f4f58fd32ce
-a22bfef14302c5ca46e0ae91092bc0e0
+d41d8cd98f00b204e9800998ecf8427e
+0cc175b9c0f1b6a831c399e269772661
+900150983cd24fb0d6963f7d28e17f72
+f96b697d7cb7938d525a2f31aaf161d0
+c3fcd3d76192e4007dfb496cca67e13b
+d174ab98d277d9f5a5611c2c9f419d9f
+57edf4a22be3c955ac49da2e2107b67a
-- 
2.6.2

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

Re: [FFmpeg-devel] [PATCH] swresample/resample: improve bessel function accuracy

[Clément Bœsch]

On Mon, Nov 02, 2015 at 10:10:33PM -0500, Ganesh Ajjanagadde wrote:
> On Mon, Nov 2, 2015 at 9:32 PM, Ganesh Ajjanagadde  wrote:
> > On Mon, Nov 2, 2015 at 6:49 PM, Ganesh Ajjanagadde  wrote:
> >> On Mon, Nov 2, 2015 at 6:37 PM, wm4  wrote:
> >>> On Mon,  2 Nov 2015 14:49:54 -0500
> >>> Ganesh Ajjanagadde  wrote:
> >>>
>  This improves accuracy for the bessel function, and this in turn should
>  improve the quality of the Kaiser window.
> >>>
> >>>
> >>> "Should"? Does it or does it not? If you don't know, why the patch?
> >>
> >> It improves the window in the sense of mathematically matching the
> >> Kaiser window expression due to the improved bessel function accuracy.
> >> That claim I have verified and placed in the commit message with
> >> evidence.
> >>
> >> What that translates into in terms of resampling accuracy, I don't
> >> know. Normally, such things do help reduce the noise floor, but I
> >> don't know an easy way to test via FATE or associated tools. I put
> >> that caveat in the bottom lines of the patch.
> >
> > Turns out the init speed is definitely improved as well (~20% boost
> > with default settings).
> > I did a cheap trick of calling build filter 1000 times to get a large
> > number of runs.
> > Results (x86-64, Haswell, GNU/Linux):
> >
> > test: fate-swr-resample-dblp-44100-2626
> >
> > new:
> > 995894468 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> > 1029719302 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> > 984101131 decicycles in build_filter(loop 1000),1024 runs,  0 skips
> >
> > old:
> > 1250020763 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> > 1246353282 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> > 1220017565 decicycles in build_filter(loop 1000),1024 runs,  0 skips
> >
> > Thus, this patch has both things going for it luckily. Will leave to
> > the maintainer (Michael I believe) to give details of accuracy
> > benefits as translated to the actual resampling if easily testable,
> > and I will add the perf numbers to the message.
> 
> One last comment on performance (this is something I have discussed
> with Timothy privately), if one removes the crippling
> -fno-tree-vectorize, FATE still passes on my GCC 5.2 setup, and one
> gets further ~5 % perf improvement here:
> 949318408 decicycles in build_filter(loop 1000), 256 runs,  0 skips
> 948795082 decicycles in build_filter(loop 1000), 512 runs,  0 skips
> 928925076 decicycles in build_filter(loop 1000),1024 runs,  0 skips
> 
> I am sure a lot of other places may benefit.

It's been a while I want to remove -fno-tree-vectorize. We've already
observed relatively small performance improvements in various places. If
it's still buggy, compilers need to be fixed anyway.

--extra-cflags=-fno-tree-vectorize is a good enough workaround if some
people explicitly want to disable it.

About the bessel patch itself, I'm happy that you benched it, because its
performance are actually relevant: last time I tried to replace the
current unrolled loop with a real loop (trying to match libavresample
code), I observed a noticeable overall slow down while resampling (the
init is very slow). Yes, no number, sorry :)

[...]

-- 
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] fate: replace custom md5 tests with those from RFC 1321

[Clément Bœsch]

On Tue, Nov 03, 2015 at 04:09:43AM -0300, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>  libavutil/md5.c| 23 ++-
>  tests/ref/fate/md5 | 12 +++-
>  2 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/libavutil/md5.c b/libavutil/md5.c
> index 876bd55..2a77304 100644
> --- a/libavutil/md5.c
> +++ b/libavutil/md5.c
> @@ -217,19 +217,16 @@ static void print_md5(uint8_t *md5)
>  
>  int main(void){
>  uint8_t md5val[16];
> -int i;
> -volatile uint8_t in[1000]; // volatile to workaround 
> http://llvm.org/bugs/show_bug.cgi?id=20849
> -// FIXME remove volatile once it has been fixed and all fate clients are 
> updated
> -
> -for (i = 0; i < 1000; i++)
> -in[i] = i * i;
> -av_md5_sum(md5val, in, 1000); print_md5(md5val);
> -av_md5_sum(md5val, in,   63); print_md5(md5val);
> -av_md5_sum(md5val, in,   64); print_md5(md5val);
> -av_md5_sum(md5val, in,   65); print_md5(md5val);
> -for (i = 0; i < 1000; i++)
> -in[i] = i % 127;
> -av_md5_sum(md5val, in,  999); print_md5(md5val);
> +
> +av_md5_sum(md5val, "",0); print_md5(md5val);
> +av_md5_sum(md5val, "a",   1); print_md5(md5val);
> +av_md5_sum(md5val, "abc", 3); print_md5(md5val);
> +av_md5_sum(md5val, "message digest", 14); print_md5(md5val);
> +av_md5_sum(md5val, "abcdefghijklmnopqrstuvwxyz", 26); print_md5(md5val);
> +av_md5_sum(md5val, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcde"
> +   "fghijklmnopqrstuvwxyz0123456789",  62); 
> print_md5(md5val);
> +av_md5_sum(md5val, "1234567890123456789012345678901234567890"
> +   "1234567890123456789012345678901234567890", 80); 
> print_md5(md5val);

Is it really OK to only test ASCII?

[...]

-- 
Clément B.


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