Re: [FFmpeg-devel] [PATCH 2/2] configure: libbsd support for arc4random()
On Mon, Dec 7, 2015 at 4:56 AM, Ganesh Ajjanagadde
wrote:
> On non-BSD machines, there exists a package libbsd for providing BSD
> functionality. This can be used to get support for arc4random.
>
> Thus, an opt-in --enable-libbsd is added to configure for this
> functionality.
>
> Tested on GNU/Linux.
>
> Signed-off-by: Ganesh Ajjanagadde
> ---
> configure | 23 +++
> libavutil/random_seed.c | 7 ++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index e676269..bf18198 100755
> --- a/configure
> +++ b/configure
> @@ -211,6 +211,7 @@ External library support:
>--enable-libass enable libass subtitles rendering,
> needed for subtitles and ass filter [no]
>--enable-libbluray enable BluRay reading using libbluray [no]
> + --enable-libbsd enable random seeding via arc4random [no]
>--enable-libbs2b enable bs2b DSP library [no]
>--enable-libcaca enable textual display using libcaca [no]
>--enable-libcelt enable CELT decoding via libcelt [no]
> @@ -1295,6 +1296,26 @@ require_pkg_config(){
> use_pkg_config "$@" || die "ERROR: $pkg not found using
> pkg-config$pkg_config_fail_message"
> }
>
> +require_libbsd(){
> +log require_libbsd "$@"
> +pkg="libbsd"
> +check_cmd $pkg_config --exists --print-errors $pkg \
> + || die "ERROR: $pkg not found"
> +pkg_cflags=$($pkg_config --cflags $pkg_config_flags $pkg)
> +pkg_libs=$($pkg_config --libs $pkg_config_flags $pkg)
> +{
> +echo "#define _DEFAULT_SOURCE"
> +echo "#include "
> +echo "long check_func(void) { return (long) arc4random; }"
> +echo "int main(void) { return 0; }"
> +} | check_ld "cc" $pkg_cflags $pkg_libs \
> + && set_safe "${pkg}_cflags" $pkg_cflags \
> + && set_safe "${pkg}_libs" $pkg_libs \
> + || die "ERROR: $pkg not found"
> +add_cflags$(get_safe "${pkg}_cflags")
> +add_extralibs $(get_safe "${pkg}_libs")
> +}
> +
We don't usually define such functions for a dep, it would be better
if you can implement the check using the normal "require" function, or
even better if libbsd provides a pkgconfig file, use that.
Or is there any particular reason why this would need a much more
complex handling than default pkg config could offer?
> require_libfreetype(){
> log require_libfreetype "$@"
> pkg="freetype2"
> @@ -1441,6 +1462,7 @@ EXTERNAL_LIBRARY_LIST="
> libaacplus
> libass
> libbluray
> +libbsd
> libbs2b
> libcaca
> libcdio
> @@ -5386,6 +5408,7 @@ enabled libiec61883 && require libiec61883
> libiec61883/iec61883.h iec61883
> enabled libaacplus&& require "libaacplus >= 2.0.0" aacplus.h
> aacplusEncOpen -laacplus
> enabled libass&& require_pkg_config libass ass/ass.h
> ass_library_init
> enabled libbluray && require_pkg_config libbluray libbluray/bluray.h
> bd_open
> +enabled libbsd&& require_libbsd
> enabled libbs2b && require_pkg_config libbs2b bs2b.h bs2b_open
> enabled libcelt && require libcelt celt/celt.h celt_decode -lcelt0
> &&
> { check_lib celt/celt.h
> celt_decoder_create_custom -lcelt0 ||
> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
> index 205a636..464b406 100644
> --- a/libavutil/random_seed.c
> +++ b/libavutil/random_seed.c
> @@ -20,6 +20,8 @@
>
> #include "config.h"
>
> +#define _DEFAULT_SOURCE
> +#define _BSD_SOURCE
> #if HAVE_UNISTD_H
> #include
> #endif
> @@ -30,6 +32,9 @@
> #include
> #include
> #endif
> +#if CONFIG_LIBBSD
> +#include
> +#endif
> #include
> #include
> #include
> @@ -121,7 +126,7 @@ uint32_t av_get_random_seed(void)
> }
> #endif
>
> -#if HAVE_ARC4RANDOM
> +#if HAVE_ARC4RANDOM || CONFIG_LIBBSD
> return arc4random();
> #endif
>
> --
> 2.6.3
>
> ___
> 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] nvenc set slice number to 1 to improve encoding quality
Le septidi 17 frimaire, an CCXXIV, Agatha Hu a écrit :
> +switch (avctx->codec->id) {
> +case AV_CODEC_ID_H264:
> +ctx->encode_config.encodeCodecConfig.h264Config.sliceMode = 3;
> +ctx->encode_config.encodeCodecConfig.h264Config.sliceModeData = 1;
Should this be an option? Slice encode may be preferred by users for some
reason (faster decoding on some devices, possibly faster encoding), or not?
Also: is there a symbolic constant for mode 3?
Regards,
--
Nicolas George
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] nvenc set slice number to 1 to improve encoding quality
---
libavcodec/nvenc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 43b8e78..f0e5a19 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -762,6 +762,17 @@ static av_cold int nvenc_encode_init(AVCodecContext
*avctx)
}
}
+switch (avctx->codec->id) {
+case AV_CODEC_ID_H264:
+ctx->encode_config.encodeCodecConfig.h264Config.sliceMode = 3;
+ctx->encode_config.encodeCodecConfig.h264Config.sliceModeData = 1;
+break;
+case AV_CODEC_ID_H265:
+ctx->encode_config.encodeCodecConfig.hevcConfig.sliceMode = 3;
+ctx->encode_config.encodeCodecConfig.hevcConfig.sliceModeData = 1;
+break;
+}
+
/* when there're b frames, set dts offset */
if (ctx->encode_config.frameIntervalP >= 2)
ctx->last_dts = -2;
--
1.9.5.github.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] nvenc set slice number to 1 to improve encoding quality
---
libavcodec/nvenc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 43b8e78..f0e5a19 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -762,6 +762,17 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx)
}
}
+switch (avctx->codec->id) {
+case AV_CODEC_ID_H264:
+ctx->encode_config.encodeCodecConfig.h264Config.sliceMode = 3;
+ctx->encode_config.encodeCodecConfig.h264Config.sliceModeData = 1;
+break;
+case AV_CODEC_ID_H265:
+ctx->encode_config.encodeCodecConfig.hevcConfig.sliceMode = 3;
+ctx->encode_config.encodeCodecConfig.hevcConfig.sliceModeData = 1;
+break;
+}
+
/* when there're b frames, set dts offset */
if (ctx->encode_config.frameIntervalP >= 2)
ctx->last_dts = -2;
--
1.9.5.github.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] nvenc set slice number to 1 to improve encoding quality
在 2015/12/5 3:26, Timo Rothenpieler 写道: --- libavcodec/nvenc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index 43b8e78..b8f7f91 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -762,6 +762,9 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx) } } +ctx->encode_config.encodeCodecConfig.h264Config.sliceMode = 3; + ctx->encode_config.encodeCodecConfig.h264Config.sliceModeData = 1; It's missing a check if we are encoding h264 or hevc. Does it have any negative sideeffects, or does it just increase the image quality? /* when there're b frames, set dts offset */ if (ctx->encode_config.frameIntervalP >= 2) ctx->last_dts = -2; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Yes, need add a check Just aimed to increase image quality. Nvidia are doing some interal tests to tune the quality Agatha Hu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] nvenc set slice number to 1 to improve encoding quality
在 2015/12/5 3:26, Timo Rothenpieler 写道: --- libavcodec/nvenc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index 43b8e78..b8f7f91 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -762,6 +762,9 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx) } } +ctx->encode_config.encodeCodecConfig.h264Config.sliceMode = 3; + ctx->encode_config.encodeCodecConfig.h264Config.sliceModeData = 1; It's missing a check if we are encoding h264 or hevc. Does it have any negative sideeffects, or does it just increase the image quality? /* when there're b frames, set dts offset */ if (ctx->encode_config.frameIntervalP >= 2) ctx->last_dts = -2; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Yes, need add a check Just aimed to increase image quality. Nvidia are doing some interal tests to tune the quality --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] configure: libbsd support for arc4random()
On non-BSD machines, there exists a package libbsd for providing BSD
functionality. This can be used to get support for arc4random.
Thus, an opt-in --enable-libbsd is added to configure for this
functionality.
Tested on GNU/Linux.
Signed-off-by: Ganesh Ajjanagadde
---
configure | 23 +++
libavutil/random_seed.c | 7 ++-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index e676269..bf18198 100755
--- a/configure
+++ b/configure
@@ -211,6 +211,7 @@ External library support:
--enable-libass enable libass subtitles rendering,
needed for subtitles and ass filter [no]
--enable-libbluray enable BluRay reading using libbluray [no]
+ --enable-libbsd enable random seeding via arc4random [no]
--enable-libbs2b enable bs2b DSP library [no]
--enable-libcaca enable textual display using libcaca [no]
--enable-libcelt enable CELT decoding via libcelt [no]
@@ -1295,6 +1296,26 @@ require_pkg_config(){
use_pkg_config "$@" || die "ERROR: $pkg not found using
pkg-config$pkg_config_fail_message"
}
+require_libbsd(){
+log require_libbsd "$@"
+pkg="libbsd"
+check_cmd $pkg_config --exists --print-errors $pkg \
+ || die "ERROR: $pkg not found"
+pkg_cflags=$($pkg_config --cflags $pkg_config_flags $pkg)
+pkg_libs=$($pkg_config --libs $pkg_config_flags $pkg)
+{
+echo "#define _DEFAULT_SOURCE"
+echo "#include "
+echo "long check_func(void) { return (long) arc4random; }"
+echo "int main(void) { return 0; }"
+} | check_ld "cc" $pkg_cflags $pkg_libs \
+ && set_safe "${pkg}_cflags" $pkg_cflags \
+ && set_safe "${pkg}_libs" $pkg_libs \
+ || die "ERROR: $pkg not found"
+add_cflags$(get_safe "${pkg}_cflags")
+add_extralibs $(get_safe "${pkg}_libs")
+}
+
require_libfreetype(){
log require_libfreetype "$@"
pkg="freetype2"
@@ -1441,6 +1462,7 @@ EXTERNAL_LIBRARY_LIST="
libaacplus
libass
libbluray
+libbsd
libbs2b
libcaca
libcdio
@@ -5386,6 +5408,7 @@ enabled libiec61883 && require libiec61883
libiec61883/iec61883.h iec61883
enabled libaacplus&& require "libaacplus >= 2.0.0" aacplus.h
aacplusEncOpen -laacplus
enabled libass&& require_pkg_config libass ass/ass.h
ass_library_init
enabled libbluray && require_pkg_config libbluray libbluray/bluray.h
bd_open
+enabled libbsd&& require_libbsd
enabled libbs2b && require_pkg_config libbs2b bs2b.h bs2b_open
enabled libcelt && require libcelt celt/celt.h celt_decode -lcelt0 &&
{ check_lib celt/celt.h
celt_decoder_create_custom -lcelt0 ||
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 205a636..464b406 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -20,6 +20,8 @@
#include "config.h"
+#define _DEFAULT_SOURCE
+#define _BSD_SOURCE
#if HAVE_UNISTD_H
#include
#endif
@@ -30,6 +32,9 @@
#include
#include
#endif
+#if CONFIG_LIBBSD
+#include
+#endif
#include
#include
#include
@@ -121,7 +126,7 @@ uint32_t av_get_random_seed(void)
}
#endif
-#if HAVE_ARC4RANDOM
+#if HAVE_ARC4RANDOM || CONFIG_LIBBSD
return arc4random();
#endif
--
2.6.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/2] arc4random support
arc4random is a great interface on the BSD's, see e.g https://www.youtube.com/watch?v=aWmLWx8ut20 for details on it, and is thus preferred over /dev/random, /dev/urandom. 1/2 (the less controversial of the 2 patches) adds support for it whenever available naturally on the system. 2/2 adds support via libbsd, readily available on GNU/Linux systems. It is opt-in via an --enable-libbsd flag. Note that this may become obsolete once posix_random gets finalized and widespread. Patches untested on BSD since I lack it; only tested on GNU/Linux. Ganesh Ajjanagadde (2): avutil/random_seed: use arc4random() when available configure: libbsd support for arc4random() configure | 25 + libavutil/random_seed.c | 9 + 2 files changed, 34 insertions(+) -- 2.6.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] avutil/random_seed: use arc4random() when available
arc4random() was designed as a superior interface for system random
number generation, designed for OpenBSD. It is thus an improvement to
use it whenever available.
As a side note, this may or may not get included in glibc, and there is
a proposal to create a posix_random family based on these ideas:
http://austingroupbugs.net/view.php?id=859.
Untested as I lack OpenBSD.
Signed-off-by: Ganesh Ajjanagadde
---
configure | 2 ++
libavutil/random_seed.c | 4
2 files changed, 6 insertions(+)
diff --git a/configure b/configure
index 7530c88..e676269 100755
--- a/configure
+++ b/configure
@@ -1840,6 +1840,7 @@ MATH_FUNCS="
SYSTEM_FUNCS="
access
aligned_malloc
+arc4random
clock_gettime
closesocket
CommandLineToArgvW
@@ -5218,6 +5219,7 @@ check_func ${malloc_prefix}memalign&& enable
memalign
check_func ${malloc_prefix}posix_memalign && enable posix_memalign
check_func access
+check_func arc4random
check_func_headers time.h clock_gettime || { check_func_headers time.h
clock_gettime -lrt && add_extralibs -lrt && LIBRT="-lrt"; }
check_func fcntl
check_func fork
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 8aa8c38..205a636 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -121,6 +121,10 @@ uint32_t av_get_random_seed(void)
}
#endif
+#if HAVE_ARC4RANDOM
+return arc4random();
+#endif
+
if (read_random(&seed, "/dev/urandom") == sizeof(seed))
return seed;
if (read_random(&seed, "/dev/random") == sizeof(seed))
--
2.6.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Delay with mails
On Sun, Dec 06, 2015 at 08:28:44PM +0100, Nicolas George wrote: > Hi. Not sure this is the best place to mention it: since a few days, I > noticed that mail does not arrive as swiftly as it used to. This especially > visible for trac, where I used to hear the bell for the mail sometimes > before the web browser finished loading the updated page, but it also > happens with the mailing list. > > Here is a sample of the headers showing the delay: > > Received: from [178.63.43.86] (localhost.localdomain [127.0.0.1]) > by ffbox0.mplayerhq.hu (Postfix) with ESMTP id 902574100266; > Sun, 6 Dec 2015 20:25:40 +0100 (CET) > X-Original-To: ffmpeg-t...@avcodec.org > Delivered-To: ffmpeg-t...@avcodec.org > Received: from ffmail.obe.tv (ffmail.obe.tv [217.138.67.134]) > by ffbox0.mplayerhq.hu (Postfix) with ESMTP id 40FF3410034F > for ; Sun, 6 Dec 2015 20:14:20 +0100 (CET) > > Does anyone know what is happening? for the archive, this should be fixed, hopefully [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus 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] decklink: Header cleanup
On Tue, Dec 01, 2015 at 09:46:05PM -0800, Timothy Gu wrote: > --- > libavdevice/decklink_common.cpp | 4 > libavdevice/decklink_common.h | 8 +++- > libavdevice/decklink_common_c.h | 2 ++ > libavdevice/decklink_dec.cpp| 4 > libavdevice/decklink_dec.h | 2 ++ > libavdevice/decklink_enc.cpp| 4 > libavdevice/decklink_enc.h | 2 ++ > 7 files changed, 13 insertions(+), 13 deletions(-) Please explain more verbose what this changes does and why in the commit message ive also added ramiro and deti to the CC, maybe they can do a real review [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Ideas to replace the options system
On Fri, 4 Dec 2015 15:33:39 +0100 Nicolas George wrote: > Hi. > > This is a rather long explanation on ideas I have to replace the > options system with something better. I will not work on it before I great, i'd suggest asking for comments also on libav-user and ffmpeg-user list. there maybe some specific option feature, be it powershell or lisp or xml or whatever standard. -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/showcqt: BASEFREQ and ENDFREQ cast to double
On Tue, Dec 01, 2015 at 11:02:07AM -0500, Ganesh Ajjanagadde wrote: > On Tue, Dec 1, 2015 at 10:38 AM, Nicolas George wrote: > > Le primidi 11 frimaire, an CCXXIV, Michael Niedermayer a écrit : > >> nicolas are you ok with this being applied ? (IIRC you objected to it) > > > > I am not really satisfied but I do not object. Hopefully it will not break > > again. > > same here - it seems like the heart of the matter is really not well > understood, and someone down the road may see the code and wonder why > the cast is being done. Hopefully the git blame still points to this > change. i hope it will solve the issue long term, if not its still a step forward and we eliminated one option in that case patch applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF: Multiple Entry with same desc
On Sun, Dec 06, 2015 at 11:55:25PM +0530, Anshul wrote: > Hello, > > There were multiple entry of same descriptor, I have kept entry with > /* MPEG2VideoDescriptor */ comment > and removed other one. > > Attached patch for same. > > Thanks > Anshul > mxfdec.c |1 - > 1 file changed, 1 deletion(-) > b0e663ebce94e3806fe3c61f747239bdcb9e03e2 > 0001-Remove-Redundant-Entry-of-MPEG2-Video-Desc.patch > From 88836433c9d4bc66aaeb1143f7af6e4fa93656d8 Mon Sep 17 00:00:00 2001 > From: Anshul Maheshwari > Date: Sun, 6 Dec 2015 23:52:15 +0530 > Subject: [PATCH] Remove Redundant Entry of MPEG2 Video Desc > > Signed-off-by: Anshul Maheshwari applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
On Sun, Dec 06, 2015 at 03:42:34PM +, Eran Kornblau wrote: > Hi, > > Sorry for spamming, ran some more tests and found a bug in my patch, updated > patch file attached. > The bug was that in case subsample encryption was enabled (the default for > AVC) the subsample size > reported in the 'saiz' atom was wrong - it did not include the size of the IV. > I originally tested my change by decrypting the file using MP4Box, and > playing it, I guess MP4Box > doesn't care about this discrepancy... > > In case anyone started reviewing the patch, the fix is in > mov_cenc_end_packet, the lines: > ctx->auxiliary_info_sizes[ctx->auxiliary_info_entries] = > ctx->auxiliary_info_size - ctx->auxiliary_info_subsample_start; > > were changed to: > ctx->auxiliary_info_sizes[ctx->auxiliary_info_entries] = > AES_CTR_IV_SIZE + ctx->auxiliary_info_size - > ctx->auxiliary_info_subsample_start; > > Thanks > > Eran > libavformat/Makefile |2 > libavformat/movenc.c | 92 +- > libavformat/movenc.h | 16 + > libavformat/movenccenc.c | 410 > +++ > libavformat/movenccenc.h | 86 + > libavutil/Makefile |2 > libavutil/aes_ctr.c | 132 +++ > libavutil/aes_ctr.h | 79 + changes to libavutil and libavformat should likely be in seperate patches/commits, more generally any independant changes should be in seperate patches (i dont know if there are any other changes that should be split off) > 8 files changed, 811 insertions(+), 8 deletions(-) > 4d4e64b516df4d1b2cb518600550f836c94849bb > 0001-movenc-support-cenc-common-encryption.patch > From 7ccfd1f4fcce2b3c301e14bd3173244241b822bd Mon Sep 17 00:00:00 2001 > From: erankor > Date: Thu, 3 Dec 2015 11:42:40 +0200 > Subject: [PATCH] movenc: support cenc (common encryption) fails to build ffmpeg/libavformat/movenccenc.c: In function ‘auxiliary_info_add_subsample’: ffmpeg/libavformat/movenccenc.c:61:5: error: unknown type name ‘u_char’ ffmpeg/libavformat/movenccenc.c:73:7: warning: assignment from incompatible pointer type [enabled by default] also a Changelog entry should probably be added and if libavformat uses a new feature from libavutil than libavutils minor version needs to be bumped when that feature is added [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][WIP][PATCH] avfilter: add ITU-R 468-4 noise meter
On Sun, Dec 6, 2015 at 6:17 PM, Paul B Mahol wrote:
> On 12/6/15, Ganesh Ajjanagadde wrote:
>> On Sun, Dec 6, 2015 at 3:58 PM, Paul B Mahol wrote:
[...]
>>
>> Please add a comment with a link: where do these "magic" numbers come from?
>>
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>>> +{
>>> +AVFilterContext *ctx = inlink->dst;
>>> +ITUR468Context *s = ctx->priv;
>>> +AVDictionary **metadata;
>>> +metadata = avpriv_frame_get_metadatap(in);
>>> +int n, c;
>>> +
>>> +for (c = 0; c < inlink->channels; c++) {
>>> +ITUR468Filter *f = &s->filter[c];
>>> +double *src = (double *)in->extended_data[c];
>>> +double out, x, zhp, z11, z12, z21, z22, z31, z32, z1, z2;
>>> +
>>> +zhp = f->zhp;
>>> +z11 = f->z11;
>>> +z12 = f->z12;
>>> +z21 = f->z21;
>>> +z22 = f->z22;
>>> +z31 = f->z31;
>>> +z32 = f->z32;
>>> +z1 = f->z1;
>>> +z2 = f->z2;
>>> +
>>> +for (n = 0; n < in->nb_samples; n++) {
>>> +x = src[n];
>>> +zhp += f->whp * (x - zhp) + 1e-20;
>>> +x -= zhp;
>>> +x -= f->a11 * z11 + f->a12 * z12;
>>> +z12 = z11;
>>> +z11 = x;
>>> +x -= f->a21 * z21 + f->a22 * z22;
>>> +z22 = z21;
>>> +z21 = x;
>>> +x -= f->a31 * z31 + f->a32 * z32;
>>> +out = f->b30 * x + f->b31 * z31 + f->b32 * z32;
>>> +z32 = z31;
>>> +z31 = x;
>>> +x = out;
>>> +x = fabs(x) + 1e-30;
>>
>> All this 1e-20, 1e-30: is is needed for well-conditioning the output
>> or what? A lot of the questions I ask can simply be clarified by
>> adding a one-liner link to the actual source of this filter, which for
>> some obscure reason you seem to not do.
>> How can you expect the reviewer to know the significance of all these
>> magic numbers?
>
> It is from obscure jnoisemeter utility, there is even ladspa plugin
> for it (bobdsp)
>
> Search for ITU-R 468 for more info.
>
Thanks for the reference. Can you please add 1 line saying e.g
"inspired/taken from jnoisemeter:
http://wiki.linuxaudio.org/apps/all/jnoisemeter";? It will help a lot,
not just for me, but for anyone else interested in contributing to it.
I also highly encourage such things for other filters as well.
As for further review, I most likely can't do it for reasons of time.
[...]
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On Sun, Dec 6, 2015 at 6:12 PM, Andreas Cadhalpun
wrote:
> On 06.12.2015 22:48, Michael Niedermayer wrote:
>> On Sun, Dec 06, 2015 at 08:26:41PM +0100, Andreas Cadhalpun wrote:
>>> On 05.12.2015 03:12, Michael Niedermayer wrote:
On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
> On 03.12.2015 23:09, Michael Niedermayer wrote:
>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>> index d30bb6b..323665d 100644
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>> -buf >>= log;
>> +buf >>= log & 31;
>> buf--;
>>
>> return buf;
>>
>
> While that certainly fixes the undefined behavior, I'm wondering what's
> the relation
> to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV
> from
> the error check above?
the & 31 is a hack really (and choosen because at least clang
optimizes it out)
the "correct" way would be to test, print a warning and return an
error code. That way if a valid (non fuzzed) file triggers it we know
that the range of get_ue_golomb() is potentially too small.
With the & 31 no information is shown, before this patch here
ubsan would show it as well without the normal case being slower
so its all perfect already ... except ... that its wrong because its
undefined behavior
>>>
>>> So you're only worried that the check makes this slower?
>>> This check is only needed in the less-likely/less-optimized branch
>>> of get_ue_golomb, so it shouldn't matter that much.
>>
>> well, this is a inlined function in a header
>> adding this check might cause the compiler to stop inlining it
>> or the larger code size might lead to more cache misses
>>
>>
>>>
maybe someone has a better idea ...
>>>
>>> Actually, I think the correct error check would be 'log < 7', because
>>> UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
>>> --- a/libavcodec/golomb.h
>>> +++ b/libavcodec/golomb.h
>>> @@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
>>> int log = 2 * av_log2(buf) - 31;
>>> LAST_SKIP_BITS(re, gb, 32 - log);
>>> CLOSE_READER(re, gb);
>>> -if (CONFIG_FTRAPV && log < 0) {
>>> +if (log < 7) {
>>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>>> return AVERROR_INVALIDDATA;
>>> }
>>>
>>> I've benchmarked this with the fate-cavs sample (which triggers the error)
>>> cat'ed 100 times together and it isn't any slower than without this change.
>>
>> my concern is more on h264 (CAVLC) and hevc speed
>
> I tested with 444_8bit_cavlc.h264 added 100 together with the concat demuxer,
> and couldn't see a measurable speed difference caused by this error check.
Ok, so here is my understanding of the situation.
I think both of you are right, but have different perspectives on this.
So in practice a log < 7 may be usually predicted correctly, and the
compiler in all likelihood will continue to inline the function. Thus,
excepting the first run, there should not be an issue, and maybe the
compiler even feeds in the "likely" information for the first run
also.
Nevertheless, I also understand Michael's perspective: h264 is
arguably one of the most important codecs as supplied by FFmpeg. Even
a 0.01% speedloss in some place should be done with extreme caution,
since over time these may accumulate to something more sizable, say
0.5%. There should be a very good justification for it, and thus
Michael spends effort trying to ensure that the security issue is
fixed at identical asm.
I personally agree with Michael here due to h264's importance (note:
this is modulo Michael's fix being correct, which I can't comment
upon). I would have thought differently 6 months back, but with more
work with FFmpeg, I have shifted my views.
To understand this better, see e.g commit how b7fb7c45 by me: I got
rid of undefined behavior so as to not trigger ubsan stuff, and
substituted with implementation defined behavior at zero cost that
should be consistent across platforms in practice. Yes, one could
insert a branch, but there is minimal utility: anyone feeding such
extreme values is fuzzing or placing an irregular file in any case.
Also, to understand Michael's views better, see e.g
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2015-March/170611.html.
Even superficially "cosmetic" effects can have a cost. See also commit
by me: 68e79b27a5ed7. Even small things can matter.
Really these things should be done with START/STOP timers since the
change is in a tight inner construct.
edit: seems like Michael beat me to it.
> Also ff_h264_decode_mb_cavlc uses mainly get_ue_golomb_31, which isn't
> affect
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On Mon, Dec 07, 2015 at 12:12:54AM +0100, Andreas Cadhalpun wrote:
> On 06.12.2015 22:48, Michael Niedermayer wrote:
> > On Sun, Dec 06, 2015 at 08:26:41PM +0100, Andreas Cadhalpun wrote:
> >> On 05.12.2015 03:12, Michael Niedermayer wrote:
> >>> On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
> On 03.12.2015 23:09, Michael Niedermayer wrote:
> > diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> > index d30bb6b..323665d 100644
> > --- a/libavcodec/golomb.h
> > +++ b/libavcodec/golomb.h
> > @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> > av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> > return AVERROR_INVALIDDATA;
> > }
> > -buf >>= log;
> > +buf >>= log & 31;
> > buf--;
> >
> > return buf;
> >
>
> While that certainly fixes the undefined behavior, I'm wondering what's
> the relation
> to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV
> from
> the error check above?
> >>>
> >>> the & 31 is a hack really (and choosen because at least clang
> >>> optimizes it out)
> >>> the "correct" way would be to test, print a warning and return an
> >>> error code. That way if a valid (non fuzzed) file triggers it we know
> >>> that the range of get_ue_golomb() is potentially too small.
> >>> With the & 31 no information is shown, before this patch here
> >>> ubsan would show it as well without the normal case being slower
> >>> so its all perfect already ... except ... that its wrong because its
> >>> undefined behavior
> >>
> >> So you're only worried that the check makes this slower?
> >> This check is only needed in the less-likely/less-optimized branch
> >> of get_ue_golomb, so it shouldn't matter that much.
> >
> > well, this is a inlined function in a header
> > adding this check might cause the compiler to stop inlining it
> > or the larger code size might lead to more cache misses
> >
> >
> >>
> >>> maybe someone has a better idea ...
> >>
> >> Actually, I think the correct error check would be 'log < 7', because
> >> UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
> >> --- a/libavcodec/golomb.h
> >> +++ b/libavcodec/golomb.h
> >> @@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >> int log = 2 * av_log2(buf) - 31;
> >> LAST_SKIP_BITS(re, gb, 32 - log);
> >> CLOSE_READER(re, gb);
> >> -if (CONFIG_FTRAPV && log < 0) {
> >> +if (log < 7) {
> >> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> >> return AVERROR_INVALIDDATA;
> >> }
> >>
> >> I've benchmarked this with the fate-cavs sample (which triggers the error)
> >> cat'ed 100 times together and it isn't any slower than without this change.
> >
> > my concern is more on h264 (CAVLC) and hevc speed
>
> I tested with 444_8bit_cavlc.h264 added 100 together with the concat demuxer,
> and couldn't see a measurable speed difference caused by this error check.
> Also ff_h264_decode_mb_cavlc uses mainly get_ue_golomb_31, which isn't
> affected by the change.
> And the hevc decoder uses get_ue_golomb_long, so it is also not affected
> by this change.
hmm, well, if its not slower then that change is fine
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][WIP][PATCH] avfilter: add ITU-R 468-4 noise meter
On 12/6/15, Ganesh Ajjanagadde wrote:
> On Sun, Dec 6, 2015 at 3:58 PM, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol
>> ---
>> libavfilter/Makefile | 1 +
>> libavfilter/af_itur468.c | 223
>> +++
>> libavfilter/allfilters.c | 1 +
>> 3 files changed, 225 insertions(+)
>> create mode 100644 libavfilter/af_itur468.c
>>
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 8884d1d..2a053d0 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -76,6 +76,7 @@ OBJS-$(CONFIG_EQUALIZER_FILTER) +=
>> af_biquads.o
>> OBJS-$(CONFIG_EXTRASTEREO_FILTER)+= af_extrastereo.o
>> OBJS-$(CONFIG_FLANGER_FILTER)+= af_flanger.o
>> generate_wave_table.o
>> OBJS-$(CONFIG_HIGHPASS_FILTER) += af_biquads.o
>> +OBJS-$(CONFIG_ITUR468_FILTER)+= af_itur468.o
>> OBJS-$(CONFIG_JOIN_FILTER) += af_join.o
>> OBJS-$(CONFIG_LADSPA_FILTER) += af_ladspa.o
>> OBJS-$(CONFIG_LOWPASS_FILTER)+= af_biquads.o
>> diff --git a/libavfilter/af_itur468.c b/libavfilter/af_itur468.c
>> new file mode 100644
>> index 000..9b0eddc
>> --- /dev/null
>> +++ b/libavfilter/af_itur468.c
>> @@ -0,0 +1,223 @@
>> +/*
>> + * Copyright (c) 2010 Fons Adriaensen
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
>> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#include "libavutil/avassert.h"
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/channel_layout.h"
>> +#include "libavutil/dict.h"
>> +#include "libavutil/xga_font_data.h"
>> +#include "libavutil/opt.h"
>> +#include "audio.h"
>> +#include "avfilter.h"
>> +#include "formats.h"
>> +#include "internal.h"
>> +
>> +typedef struct ITUR468Filter {
>> +double whp;
>> +double a11, a12;
>> +double a21, a22;
>> +double a31, a32;
>> +double b30, b31, b32;
>> +double zhp;
>> +double z11, z12;
>> +double z21, z22;
>> +double z31, z32;
>> +
>> +double a1, b1;
>> +double a2, b2;
>> +double z1, z2;
>> +} ITUR468Filter;
>> +
>> +typedef struct {
>> +const AVClass *class;
>> +
>> +ITUR468Filter *filter;
>> +} ITUR468Context;
>> +
>> +static int config_output(AVFilterLink *outlink)
>> +{
>> +AVFilterContext *ctx = outlink->src;
>> +AVFilterLink *inlink = ctx->inputs[0];
>> +ITUR468Context *s = ctx->priv;
>> +int c;
>> +
>> +s->filter = av_calloc(inlink->channels, sizeof(*s->filter));
>> +if (!s->filter)
>> +return AVERROR(ENOMEM);
>> +
>> +for (c = 0; c < inlink->channels; c++) {
>> +ITUR468Filter *f = &s->filter[c];
>> +
>> +f->whp = 3.8715217e-01;
>> +f->a11 = -8.4163201e-01;
>> +f->a12 = 3.0498350e-01;
>> +f->a21 = -6.5680242e-01;
>> +f->a22 = 2.3733993e-01;
>> +f->a31 = -3.3843556e-01;
>> +f->a32 = 4.3756709e-01;
>> +f->b30 = 9.8607997e-01;
>> +f->b31 = 5.4846389e-01;
>> +f->b32 = -8.2465158e-02;
>> +f->a1 = 670. / inlink->sample_rate;
>> +f->b1 = 3.5 / inlink->sample_rate;
>> +f->a2 = 6.6 / inlink->sample_rate;
>> +f->b2 = 0.65 / inlink->sample_rate;
>
> Please add a comment with a link: where do these "magic" numbers come from?
>
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>> +{
>> +AVFilterContext *ctx = inlink->dst;
>> +ITUR468Context *s = ctx->priv;
>> +AVDictionary **metadata;
>> +metadata = avpriv_frame_get_metadatap(in);
>> +int n, c;
>> +
>> +for (c = 0; c < inlink->channels; c++) {
>> +ITUR468Filter *f = &s->filter[c];
>> +double *src = (double *)in->extended_data[c];
>> +double out, x, zhp, z11, z12, z21, z22, z31, z32, z1, z2;
>> +
>> +zhp = f->zhp;
>> +z11 = f->z11;
>> +z12 = f->z12;
>> +z21 = f->z21;
>> +z22 = f->z22;
>> +z31 = f->z31;
>> +z32 = f->z32;
>> +z1 = f->z1;
>> +z2 = f->z2;
>> +
>> +for (n = 0; n < in->nb_samples; n++) {
>> +x = src[n];
>> +zhp += f->whp * (x - zhp) + 1e-20;
>> +x -= zhp;
>
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On 06.12.2015 22:48, Michael Niedermayer wrote:
> On Sun, Dec 06, 2015 at 08:26:41PM +0100, Andreas Cadhalpun wrote:
>> On 05.12.2015 03:12, Michael Niedermayer wrote:
>>> On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
On 03.12.2015 23:09, Michael Niedermayer wrote:
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index d30bb6b..323665d 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
> -buf >>= log;
> +buf >>= log & 31;
> buf--;
>
> return buf;
>
While that certainly fixes the undefined behavior, I'm wondering what's
the relation
to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV
from
the error check above?
>>>
>>> the & 31 is a hack really (and choosen because at least clang
>>> optimizes it out)
>>> the "correct" way would be to test, print a warning and return an
>>> error code. That way if a valid (non fuzzed) file triggers it we know
>>> that the range of get_ue_golomb() is potentially too small.
>>> With the & 31 no information is shown, before this patch here
>>> ubsan would show it as well without the normal case being slower
>>> so its all perfect already ... except ... that its wrong because its
>>> undefined behavior
>>
>> So you're only worried that the check makes this slower?
>> This check is only needed in the less-likely/less-optimized branch
>> of get_ue_golomb, so it shouldn't matter that much.
>
> well, this is a inlined function in a header
> adding this check might cause the compiler to stop inlining it
> or the larger code size might lead to more cache misses
>
>
>>
>>> maybe someone has a better idea ...
>>
>> Actually, I think the correct error check would be 'log < 7', because
>> UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> int log = 2 * av_log2(buf) - 31;
>> LAST_SKIP_BITS(re, gb, 32 - log);
>> CLOSE_READER(re, gb);
>> -if (CONFIG_FTRAPV && log < 0) {
>> +if (log < 7) {
>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>>
>> I've benchmarked this with the fate-cavs sample (which triggers the error)
>> cat'ed 100 times together and it isn't any slower than without this change.
>
> my concern is more on h264 (CAVLC) and hevc speed
I tested with 444_8bit_cavlc.h264 added 100 together with the concat demuxer,
and couldn't see a measurable speed difference caused by this error check.
Also ff_h264_decode_mb_cavlc uses mainly get_ue_golomb_31, which isn't
affected by the change.
And the hevc decoder uses get_ue_golomb_long, so it is also not affected
by this change.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On Sun, Dec 06, 2015 at 08:26:41PM +0100, Andreas Cadhalpun wrote:
> On 05.12.2015 03:12, Michael Niedermayer wrote:
> > On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
> >> On 03.12.2015 23:09, Michael Niedermayer wrote:
> >>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> >>> index d30bb6b..323665d 100644
> >>> --- a/libavcodec/golomb.h
> >>> +++ b/libavcodec/golomb.h
> >>> @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> >>> return AVERROR_INVALIDDATA;
> >>> }
> >>> -buf >>= log;
> >>> +buf >>= log & 31;
> >>> buf--;
> >>>
> >>> return buf;
> >>>
> >>
> >> While that certainly fixes the undefined behavior, I'm wondering what's
> >> the relation
> >> to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV
> >> from
> >> the error check above?
> >
> > the & 31 is a hack really (and choosen because at least clang
> > optimizes it out)
> > the "correct" way would be to test, print a warning and return an
> > error code. That way if a valid (non fuzzed) file triggers it we know
> > that the range of get_ue_golomb() is potentially too small.
> > With the & 31 no information is shown, before this patch here
> > ubsan would show it as well without the normal case being slower
> > so its all perfect already ... except ... that its wrong because its
> > undefined behavior
>
> So you're only worried that the check makes this slower?
> This check is only needed in the less-likely/less-optimized branch
> of get_ue_golomb, so it shouldn't matter that much.
>
> > maybe someone has a better idea ...
>
> Actually, I think the correct error check would be 'log < 7', because
> UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> int log = 2 * av_log2(buf) - 31;
> LAST_SKIP_BITS(re, gb, 32 - log);
> CLOSE_READER(re, gb);
> -if (CONFIG_FTRAPV && log < 0) {
> +if (log < 7) {
s/0/7/ is ok
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Fix SChannel compilation on cygwin
On Sun, Dec 6, 2015 at 7:38 PM, Timo Rothenpieler wrote: > Patch from https://trac.ffmpeg.org/ticket/5036 is attached > Should be correct. On normal windows targets probably came from some other header above, which isn't good style, I suppose. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Fix SChannel compilation on cygwin
On Sun, Dec 06, 2015 at 07:38:21PM +0100, Timo Rothenpieler wrote: > Patch from https://trac.ffmpeg.org/ticket/5036 is attached > tls_schannel.c |1 + > 1 file changed, 1 insertion(+) > f2fb9e0792fa907f3d925a2be23c5a36f747f9ad > 0001-avformat-add-windows.h-to-SChannel-SSP-TLS-code.patch > From d5d18f91514b6c31f903b04a1c44b0e2de2ffd1a Mon Sep 17 00:00:00 2001 > From: Kevin Mitchell > Date: Tue, 24 Nov 2015 19:25:12 -0800 > Subject: [PATCH] avformat: add windows.h to SChannel SSP TLS code > > This fixes building on cygwin iam not a window devel but this sounds correct [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][WIP][PATCH] avfilter: add ITU-R 468-4 noise meter
On Sun, Dec 6, 2015 at 3:58 PM, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol
> ---
> libavfilter/Makefile | 1 +
> libavfilter/af_itur468.c | 223
> +++
> libavfilter/allfilters.c | 1 +
> 3 files changed, 225 insertions(+)
> create mode 100644 libavfilter/af_itur468.c
>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 8884d1d..2a053d0 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -76,6 +76,7 @@ OBJS-$(CONFIG_EQUALIZER_FILTER) += af_biquads.o
> OBJS-$(CONFIG_EXTRASTEREO_FILTER)+= af_extrastereo.o
> OBJS-$(CONFIG_FLANGER_FILTER)+= af_flanger.o
> generate_wave_table.o
> OBJS-$(CONFIG_HIGHPASS_FILTER) += af_biquads.o
> +OBJS-$(CONFIG_ITUR468_FILTER)+= af_itur468.o
> OBJS-$(CONFIG_JOIN_FILTER) += af_join.o
> OBJS-$(CONFIG_LADSPA_FILTER) += af_ladspa.o
> OBJS-$(CONFIG_LOWPASS_FILTER)+= af_biquads.o
> diff --git a/libavfilter/af_itur468.c b/libavfilter/af_itur468.c
> new file mode 100644
> index 000..9b0eddc
> --- /dev/null
> +++ b/libavfilter/af_itur468.c
> @@ -0,0 +1,223 @@
> +/*
> + * Copyright (c) 2010 Fons Adriaensen
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include
> +#include
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/channel_layout.h"
> +#include "libavutil/dict.h"
> +#include "libavutil/xga_font_data.h"
> +#include "libavutil/opt.h"
> +#include "audio.h"
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +
> +typedef struct ITUR468Filter {
> +double whp;
> +double a11, a12;
> +double a21, a22;
> +double a31, a32;
> +double b30, b31, b32;
> +double zhp;
> +double z11, z12;
> +double z21, z22;
> +double z31, z32;
> +
> +double a1, b1;
> +double a2, b2;
> +double z1, z2;
> +} ITUR468Filter;
> +
> +typedef struct {
> +const AVClass *class;
> +
> +ITUR468Filter *filter;
> +} ITUR468Context;
> +
> +static int config_output(AVFilterLink *outlink)
> +{
> +AVFilterContext *ctx = outlink->src;
> +AVFilterLink *inlink = ctx->inputs[0];
> +ITUR468Context *s = ctx->priv;
> +int c;
> +
> +s->filter = av_calloc(inlink->channels, sizeof(*s->filter));
> +if (!s->filter)
> +return AVERROR(ENOMEM);
> +
> +for (c = 0; c < inlink->channels; c++) {
> +ITUR468Filter *f = &s->filter[c];
> +
> +f->whp = 3.8715217e-01;
> +f->a11 = -8.4163201e-01;
> +f->a12 = 3.0498350e-01;
> +f->a21 = -6.5680242e-01;
> +f->a22 = 2.3733993e-01;
> +f->a31 = -3.3843556e-01;
> +f->a32 = 4.3756709e-01;
> +f->b30 = 9.8607997e-01;
> +f->b31 = 5.4846389e-01;
> +f->b32 = -8.2465158e-02;
> +f->a1 = 670. / inlink->sample_rate;
> +f->b1 = 3.5 / inlink->sample_rate;
> +f->a2 = 6.6 / inlink->sample_rate;
> +f->b2 = 0.65 / inlink->sample_rate;
Please add a comment with a link: where do these "magic" numbers come from?
> +}
> +
> +return 0;
> +}
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> +{
> +AVFilterContext *ctx = inlink->dst;
> +ITUR468Context *s = ctx->priv;
> +AVDictionary **metadata;
> +metadata = avpriv_frame_get_metadatap(in);
> +int n, c;
> +
> +for (c = 0; c < inlink->channels; c++) {
> +ITUR468Filter *f = &s->filter[c];
> +double *src = (double *)in->extended_data[c];
> +double out, x, zhp, z11, z12, z21, z22, z31, z32, z1, z2;
> +
> +zhp = f->zhp;
> +z11 = f->z11;
> +z12 = f->z12;
> +z21 = f->z21;
> +z22 = f->z22;
> +z31 = f->z31;
> +z32 = f->z32;
> +z1 = f->z1;
> +z2 = f->z2;
> +
> +for (n = 0; n < in->nb_samples; n++) {
> +x = src[n];
> +zhp += f->whp * (x - zhp) + 1e-20;
> +x -= zhp;
> +x -= f->a11 * z11 + f->a12 * z12;
> +z12 = z11;
> +z11 = x;
> +x -= f->a21 * z21 + f->a22 * z22;
> +z22 = z21;
> +z21 =
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On Sun, Dec 06, 2015 at 08:26:41PM +0100, Andreas Cadhalpun wrote:
> On 05.12.2015 03:12, Michael Niedermayer wrote:
> > On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
> >> On 03.12.2015 23:09, Michael Niedermayer wrote:
> >>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> >>> index d30bb6b..323665d 100644
> >>> --- a/libavcodec/golomb.h
> >>> +++ b/libavcodec/golomb.h
> >>> @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> >>> return AVERROR_INVALIDDATA;
> >>> }
> >>> -buf >>= log;
> >>> +buf >>= log & 31;
> >>> buf--;
> >>>
> >>> return buf;
> >>>
> >>
> >> While that certainly fixes the undefined behavior, I'm wondering what's
> >> the relation
> >> to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV
> >> from
> >> the error check above?
> >
> > the & 31 is a hack really (and choosen because at least clang
> > optimizes it out)
> > the "correct" way would be to test, print a warning and return an
> > error code. That way if a valid (non fuzzed) file triggers it we know
> > that the range of get_ue_golomb() is potentially too small.
> > With the & 31 no information is shown, before this patch here
> > ubsan would show it as well without the normal case being slower
> > so its all perfect already ... except ... that its wrong because its
> > undefined behavior
>
> So you're only worried that the check makes this slower?
> This check is only needed in the less-likely/less-optimized branch
> of get_ue_golomb, so it shouldn't matter that much.
well, this is a inlined function in a header
adding this check might cause the compiler to stop inlining it
or the larger code size might lead to more cache misses
>
> > maybe someone has a better idea ...
>
> Actually, I think the correct error check would be 'log < 7', because
> UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
> int log = 2 * av_log2(buf) - 31;
> LAST_SKIP_BITS(re, gb, 32 - log);
> CLOSE_READER(re, gb);
> -if (CONFIG_FTRAPV && log < 0) {
> +if (log < 7) {
> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
>
> I've benchmarked this with the fate-cavs sample (which triggers the error)
> cat'ed 100 times together and it isn't any slower than without this change.
my concern is more on h264 (CAVLC) and hevc speed
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mjpegdec: consider chroma subsampling in size check
On 06.12.2015 22:18, Michael Niedermayer wrote: > On Sun, Dec 06, 2015 at 06:56:35PM +0100, Andreas Cadhalpun wrote: >> mjpegdec.c | 11 --- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> a294ce9a780fdd710d3661bc201b0c72d30786d3 >> 0001-mjpegdec-consider-chroma-subsampling-in-size-check.patch >> From 7788195340e1d0e1206660f12f003f952da750a6 Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun >> Date: Wed, 2 Dec 2015 21:52:23 +0100 >> Subject: [PATCH] mjpegdec: consider chroma subsampling in size check >> >> If the chroma components are subsampled, smaller buffers are allocated >> for them. In that case the maximal block_offset for the chroma >> components is not as large as for the luma component. >> >> This fixes out of bounds writes causing segmentation faults or memory >> corruption. >> > > LGTM Pushed. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] aaccoder: prevent crash of anmr coder
On 05.12.2015 02:58, Claudio Freire wrote:
> Alright, I see what's going on. minscaler-maxscaler and/or q0-q1 are
> empty ranges, so that results in no possible solutions.
>
> I pushed a fix for that, leaving your "safety net" intact.
Thanks.
> You may wish to re-fuzz ;)
I did that and found more issues, this time also affecting the twoloop encoder.
One is an out-of-bounds read in avoid_clipping, which is caused by
ics->max_sfb being larger than ics->num_swb. I've sent a patch for that.
The other is a regression since 01ecb71, so I hope you know how to fix that.
In search_for_pns in libavcodec/aaccoder.c:
for (w = 0; w < sce->ics.num_windows; w += sce->ics.group_len[w]) {
[...]
for (g = 0; g < sce->ics.num_swb; g++) {
[...]
for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
[...]
}
if (g && sce->sf_idx[(w+w2)*16+g-1] == NOISE_BT) {
At this point w+w2 can be sce->ics.num_windows, which causes an
out-of-bounds read.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] aacenc: update max_sfb when num_swb changes
This fixes out-of-bounds reads in avoid_clipping. Signed-off-by: Andreas Cadhalpun --- libavcodec/aacenc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index 2156fc0..d48fb32 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -574,6 +574,7 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, ics->num_windows= wi[ch].num_windows; ics->swb_sizes = s->psy.bands[ics->num_windows == 8]; ics->num_swb= tag == TYPE_LFE ? ics->num_swb : s->psy.num_bands[ics->num_windows == 8]; +ics->max_sfb= FFMIN(ics->max_sfb, ics->num_swb); ics->swb_offset = wi[ch].window_type[0] == EIGHT_SHORT_SEQUENCE ? ff_swb_offset_128 [s->samplerate_index]: ff_swb_offset_1024[s->samplerate_index]; -- 2.6.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mjpegdec: consider chroma subsampling in size check
On Sun, Dec 06, 2015 at 06:56:35PM +0100, Andreas Cadhalpun wrote:
> On 05.12.2015 04:02, Michael Niedermayer wrote:
> > On Fri, Dec 04, 2015 at 03:14:21PM +0100, Andreas Cadhalpun wrote:
> >> On 03.12.2015 15:48, Michael Niedermayer wrote:
> >>> On Wed, Dec 02, 2015 at 10:00:13PM +0100, Andreas Cadhalpun wrote:
> @@ -1293,14 +1296,16 @@ static int mjpeg_decode_scan(MJpegDecodeContext
> *s, int nb_components, int Ah,
> v = s->v_scount[i];
> x = 0;
> y = 0;
> +h_shift = c ? chroma_h_shift: 0;
> +v_shift = c ? chroma_v_shift: 0;
> for (j = 0; j < n; j++) {
> block_offset = (((linesize[c] * (v * mb_y + y) * 8)
> +
> (h * mb_x + x) * 8 *
> bytes_per_pixel) >> s->avctx->lowres);
>
> if (s->interlaced && s->bottom_field)
> block_offset += linesize[c] >> 1;
> -if ( 8*(h * mb_x + x) < s->width
> -&& 8*(v * mb_y + y) < s->height) {
> +if ( 8*(h * mb_x + x) < (s->width + (1 <<
> h_shift) - 1) >> h_shift
> +&& 8*(v * mb_y + y) < (s->height + (1 <<
> v_shift) - 1) >> v_shift) {
> >>>
> >>> please move the w/h computation out of the block loop
> >>> it stays the same for a component and does not need to be
> >>> recalculated
> >>> theres a loop above that fills data[] that can probably be used to
> >>> fill w/h arrays
> >>
> >> OK, but since there are only two possible values, I don't think filling
> >> arrays is necessary. Attached is an updated patch.
> >
> > couldnt there be a alpha plane too ?
>
> Yes, fixed patch attached.
> I also tested filling w/h arrays, but that turned out to be slightly slower.
>
> Best regards,
> Andreas
>
> mjpegdec.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
> a294ce9a780fdd710d3661bc201b0c72d30786d3
> 0001-mjpegdec-consider-chroma-subsampling-in-size-check.patch
> From 7788195340e1d0e1206660f12f003f952da750a6 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun
> Date: Wed, 2 Dec 2015 21:52:23 +0100
> Subject: [PATCH] mjpegdec: consider chroma subsampling in size check
>
> If the chroma components are subsampled, smaller buffers are allocated
> for them. In that case the maximal block_offset for the chroma
> components is not as large as for the luma component.
>
> This fixes out of bounds writes causing segmentation faults or memory
> corruption.
>
LGTM
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [RFC][WIP][PATCH] avfilter: add ITU-R 468-4 noise meter
Signed-off-by: Paul B Mahol
---
libavfilter/Makefile | 1 +
libavfilter/af_itur468.c | 223 +++
libavfilter/allfilters.c | 1 +
3 files changed, 225 insertions(+)
create mode 100644 libavfilter/af_itur468.c
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 8884d1d..2a053d0 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -76,6 +76,7 @@ OBJS-$(CONFIG_EQUALIZER_FILTER) += af_biquads.o
OBJS-$(CONFIG_EXTRASTEREO_FILTER)+= af_extrastereo.o
OBJS-$(CONFIG_FLANGER_FILTER)+= af_flanger.o
generate_wave_table.o
OBJS-$(CONFIG_HIGHPASS_FILTER) += af_biquads.o
+OBJS-$(CONFIG_ITUR468_FILTER)+= af_itur468.o
OBJS-$(CONFIG_JOIN_FILTER) += af_join.o
OBJS-$(CONFIG_LADSPA_FILTER) += af_ladspa.o
OBJS-$(CONFIG_LOWPASS_FILTER)+= af_biquads.o
diff --git a/libavfilter/af_itur468.c b/libavfilter/af_itur468.c
new file mode 100644
index 000..9b0eddc
--- /dev/null
+++ b/libavfilter/af_itur468.c
@@ -0,0 +1,223 @@
+/*
+ * Copyright (c) 2010 Fons Adriaensen
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include
+#include
+
+#include "libavutil/avassert.h"
+#include "libavutil/avstring.h"
+#include "libavutil/channel_layout.h"
+#include "libavutil/dict.h"
+#include "libavutil/xga_font_data.h"
+#include "libavutil/opt.h"
+#include "audio.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "internal.h"
+
+typedef struct ITUR468Filter {
+double whp;
+double a11, a12;
+double a21, a22;
+double a31, a32;
+double b30, b31, b32;
+double zhp;
+double z11, z12;
+double z21, z22;
+double z31, z32;
+
+double a1, b1;
+double a2, b2;
+double z1, z2;
+} ITUR468Filter;
+
+typedef struct {
+const AVClass *class;
+
+ITUR468Filter *filter;
+} ITUR468Context;
+
+static int config_output(AVFilterLink *outlink)
+{
+AVFilterContext *ctx = outlink->src;
+AVFilterLink *inlink = ctx->inputs[0];
+ITUR468Context *s = ctx->priv;
+int c;
+
+s->filter = av_calloc(inlink->channels, sizeof(*s->filter));
+if (!s->filter)
+return AVERROR(ENOMEM);
+
+for (c = 0; c < inlink->channels; c++) {
+ITUR468Filter *f = &s->filter[c];
+
+f->whp = 3.8715217e-01;
+f->a11 = -8.4163201e-01;
+f->a12 = 3.0498350e-01;
+f->a21 = -6.5680242e-01;
+f->a22 = 2.3733993e-01;
+f->a31 = -3.3843556e-01;
+f->a32 = 4.3756709e-01;
+f->b30 = 9.8607997e-01;
+f->b31 = 5.4846389e-01;
+f->b32 = -8.2465158e-02;
+f->a1 = 670. / inlink->sample_rate;
+f->b1 = 3.5 / inlink->sample_rate;
+f->a2 = 6.6 / inlink->sample_rate;
+f->b2 = 0.65 / inlink->sample_rate;
+}
+
+return 0;
+}
+
+static int filter_frame(AVFilterLink *inlink, AVFrame *in)
+{
+AVFilterContext *ctx = inlink->dst;
+ITUR468Context *s = ctx->priv;
+AVDictionary **metadata;
+metadata = avpriv_frame_get_metadatap(in);
+int n, c;
+
+for (c = 0; c < inlink->channels; c++) {
+ITUR468Filter *f = &s->filter[c];
+double *src = (double *)in->extended_data[c];
+double out, x, zhp, z11, z12, z21, z22, z31, z32, z1, z2;
+
+zhp = f->zhp;
+z11 = f->z11;
+z12 = f->z12;
+z21 = f->z21;
+z22 = f->z22;
+z31 = f->z31;
+z32 = f->z32;
+z1 = f->z1;
+z2 = f->z2;
+
+for (n = 0; n < in->nb_samples; n++) {
+x = src[n];
+zhp += f->whp * (x - zhp) + 1e-20;
+x -= zhp;
+x -= f->a11 * z11 + f->a12 * z12;
+z12 = z11;
+z11 = x;
+x -= f->a21 * z21 + f->a22 * z22;
+z22 = z21;
+z21 = x;
+x -= f->a31 * z31 + f->a32 * z32;
+out = f->b30 * x + f->b31 * z31 + f->b32 * z32;
+z32 = z31;
+z31 = x;
+x = out;
+x = fabs(x) + 1e-30;
+z1 -= z1 * f->b1;
+if (x > z1)
+z1 += f->a1 * (x - z1);
+
+z2 -= z2 * f->b2;
+if (z1 > z2)
+z2 += f->a2 * (z1 - z2);
+}
+
+
[FFmpeg-devel] Delay with mails
Hi. Not sure this is the best place to mention it: since a few days, I noticed that mail does not arrive as swiftly as it used to. This especially visible for trac, where I used to hear the bell for the mail sometimes before the web browser finished loading the updated page, but it also happens with the mailing list. Here is a sample of the headers showing the delay: Received: from [178.63.43.86] (localhost.localdomain [127.0.0.1]) by ffbox0.mplayerhq.hu (Postfix) with ESMTP id 902574100266; Sun, 6 Dec 2015 20:25:40 +0100 (CET) X-Original-To: ffmpeg-t...@avcodec.org Delivered-To: ffmpeg-t...@avcodec.org Received: from ffmail.obe.tv (ffmail.obe.tv [217.138.67.134]) by ffbox0.mplayerhq.hu (Postfix) with ESMTP id 40FF3410034F for ; Sun, 6 Dec 2015 20:14:20 +0100 (CET) Does anyone know what is happening? Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Mask shift amount before use in get_ue_golomb()
On 05.12.2015 03:12, Michael Niedermayer wrote:
> On Fri, Dec 04, 2015 at 10:28:35PM +0100, Andreas Cadhalpun wrote:
>> On 03.12.2015 23:09, Michael Niedermayer wrote:
>>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>>> index d30bb6b..323665d 100644
>>> --- a/libavcodec/golomb.h
>>> +++ b/libavcodec/golomb.h
>>> @@ -72,7 +72,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
>>> av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>>> return AVERROR_INVALIDDATA;
>>> }
>>> -buf >>= log;
>>> +buf >>= log & 31;
>>> buf--;
>>>
>>> return buf;
>>>
>>
>> While that certainly fixes the undefined behavior, I'm wondering what's the
>> relation
>> to commit fd165ac. In other words, why not just remove the CONFIG_FTRAPV from
>> the error check above?
>
> the & 31 is a hack really (and choosen because at least clang
> optimizes it out)
> the "correct" way would be to test, print a warning and return an
> error code. That way if a valid (non fuzzed) file triggers it we know
> that the range of get_ue_golomb() is potentially too small.
> With the & 31 no information is shown, before this patch here
> ubsan would show it as well without the normal case being slower
> so its all perfect already ... except ... that its wrong because its
> undefined behavior
So you're only worried that the check makes this slower?
This check is only needed in the less-likely/less-optimized branch
of get_ue_golomb, so it shouldn't matter that much.
> maybe someone has a better idea ...
Actually, I think the correct error check would be 'log < 7', because
UPDATE_CACHE only guarantees 25 meaningful bits. Thus I propose:
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -68,7 +68,7 @@ static inline int get_ue_golomb(GetBitContext *gb)
int log = 2 * av_log2(buf) - 31;
LAST_SKIP_BITS(re, gb, 32 - log);
CLOSE_READER(re, gb);
-if (CONFIG_FTRAPV && log < 0) {
+if (log < 7) {
av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
return AVERROR_INVALIDDATA;
}
I've benchmarked this with the fate-cavs sample (which triggers the error)
cat'ed 100 times together and it isn't any slower than without this change.
>> Also, if you are interested in fixing such undefined behavior, I have lots of
>> fuzzed samples triggering ubsan all over the place...
>
> I think my todo list is too long to fix all. Maybe others are
> interrested in helping.
I see. I've also other things higher up in my TODO list...
> The really tricky part is to fix some of these issues without causing
> a slowdown in speed relevant code and without making maintaince
> harder
> for example decoding a file from a bug report with ubsan and looking
> for overflows can in rare instances directly point to the problem
> or close to it. One needs to keep this in mind when Fixing/Hiding
> ubsan issues, sometimes a log message and error out is better than
> extending precission or using unsigned sometimes its not ...
Indeed.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp9: fix pixel format changes with threading
Hi,
On Sun, Dec 6, 2015 at 10:45 AM, Hendrik Leppkes
wrote:
> ---
> libavcodec/vp9.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index dc0..5b90c13 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -4228,7 +4228,7 @@ static int
> vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
> // detect size changes in other threads
> if (s->intra_pred_data[0] &&
> (!ssrc->intra_pred_data[0] || s->cols != ssrc->cols ||
> - s->rows != ssrc->rows || s->bpp != ssrc->bpp)) {
> + s->rows != ssrc->rows || s->bpp != ssrc->bpp || s->pix_fmt !=
> ssrc->pix_fmt)) {
> free_buffers(s);
> }
>
> @@ -4260,6 +4260,7 @@ static int
> vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
> s->bytesperpixel = ssrc->bytesperpixel;
> s->bpp = ssrc->bpp;
> s->bpp_index = ssrc->bpp_index;
> +s->pix_fmt = ssrc->pix_fmt;
> memcpy(&s->prob_ctx, &ssrc->prob_ctx, sizeof(s->prob_ctx));
> memcpy(&s->s.h.lf_delta, &ssrc->s.h.lf_delta,
> sizeof(s->s.h.lf_delta));
> memcpy(&s->s.h.segmentation.feat, &ssrc->s.h.segmentation.feat,
> --
> 2.6.2.windows.1
Okay.
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] aacenc: move the TNS search and filtering before PNS
On Sun, Dec 06, 2015 at 01:57:48PM +, Rostislav Pehlivanov wrote: > The original plan was to have TNS use data from the PNS search to better > tune itself to noise but this was never used nor necessary. This should > slightly boost the PNS accuracy if TNS was used. > > Signed-off-by: Rostislav Pehlivanov this patchset fixes the 32bit PNS noasm fate failure it seems thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB You can kill me, but you cannot change the truth. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Fix SChannel compilation on cygwin
Patch from https://trac.ffmpeg.org/ticket/5036 is attached From d5d18f91514b6c31f903b04a1c44b0e2de2ffd1a Mon Sep 17 00:00:00 2001 From: Kevin Mitchell Date: Tue, 24 Nov 2015 19:25:12 -0800 Subject: [PATCH] avformat: add windows.h to SChannel SSP TLS code This fixes building on cygwin --- libavformat/tls_schannel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/tls_schannel.c b/libavformat/tls_schannel.c index c76f8a9..85c01a0 100644 --- a/libavformat/tls_schannel.c +++ b/libavformat/tls_schannel.c @@ -28,6 +28,7 @@ #include "tls.h" #define SECURITY_WIN32 +#include #include #include -- 2.6.2 signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] MXF: Multiple Entry with same desc
Hello,
There were multiple entry of same descriptor, I have kept entry with /*
MPEG2VideoDescriptor */ comment
and removed other one.
Attached patch for same.
Thanks
Anshul
>From 88836433c9d4bc66aaeb1143f7af6e4fa93656d8 Mon Sep 17 00:00:00 2001
From: Anshul Maheshwari
Date: Sun, 6 Dec 2015 23:52:15 +0530
Subject: [PATCH] Remove Redundant Entry of MPEG2 Video Desc
Signed-off-by: Anshul Maheshwari
---
libavformat/mxfdec.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 6b1c654..0e055fd 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2279,7 +2279,6 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x42,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* Generic Sound */
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x28,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* CDCI */
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x29,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* RGBA */
-{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x51,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* MPEG 2 Video */
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x48,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* Wave */
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x47,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* AES3 */
{ { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x51,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* MPEG2VideoDescriptor */
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mjpegdec: consider chroma subsampling in size check
On 05.12.2015 04:02, Michael Niedermayer wrote:
> On Fri, Dec 04, 2015 at 03:14:21PM +0100, Andreas Cadhalpun wrote:
>> On 03.12.2015 15:48, Michael Niedermayer wrote:
>>> On Wed, Dec 02, 2015 at 10:00:13PM +0100, Andreas Cadhalpun wrote:
@@ -1293,14 +1296,16 @@ static int mjpeg_decode_scan(MJpegDecodeContext
*s, int nb_components, int Ah,
v = s->v_scount[i];
x = 0;
y = 0;
+h_shift = c ? chroma_h_shift: 0;
+v_shift = c ? chroma_v_shift: 0;
for (j = 0; j < n; j++) {
block_offset = (((linesize[c] * (v * mb_y + y) * 8) +
(h * mb_x + x) * 8 *
bytes_per_pixel) >> s->avctx->lowres);
if (s->interlaced && s->bottom_field)
block_offset += linesize[c] >> 1;
-if ( 8*(h * mb_x + x) < s->width
-&& 8*(v * mb_y + y) < s->height) {
+if ( 8*(h * mb_x + x) < (s->width + (1 << h_shift)
- 1) >> h_shift
+&& 8*(v * mb_y + y) < (s->height + (1 << v_shift)
- 1) >> v_shift) {
>>>
>>> please move the w/h computation out of the block loop
>>> it stays the same for a component and does not need to be
>>> recalculated
>>> theres a loop above that fills data[] that can probably be used to
>>> fill w/h arrays
>>
>> OK, but since there are only two possible values, I don't think filling
>> arrays is necessary. Attached is an updated patch.
>
> couldnt there be a alpha plane too ?
Yes, fixed patch attached.
I also tested filling w/h arrays, but that turned out to be slightly slower.
Best regards,
Andreas
>From 7788195340e1d0e1206660f12f003f952da750a6 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Wed, 2 Dec 2015 21:52:23 +0100
Subject: [PATCH] mjpegdec: consider chroma subsampling in size check
If the chroma components are subsampled, smaller buffers are allocated
for them. In that case the maximal block_offset for the chroma
components is not as large as for the luma component.
This fixes out of bounds writes causing segmentation faults or memory
corruption.
Signed-off-by: Andreas Cadhalpun
---
libavcodec/mjpegdec.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 4c9c82d..c812b86 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1246,7 +1246,7 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
int mb_bitmask_size,
const AVFrame *reference)
{
-int i, mb_x, mb_y;
+int i, mb_x, mb_y, chroma_h_shift, chroma_v_shift, chroma_width, chroma_height;
uint8_t *data[MAX_COMPONENTS];
const uint8_t *reference_data[MAX_COMPONENTS];
int linesize[MAX_COMPONENTS];
@@ -1263,6 +1263,11 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
s->restart_count = 0;
+av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt, &chroma_h_shift,
+ &chroma_v_shift);
+chroma_width = FF_CEIL_RSHIFT(s->width, chroma_h_shift);
+chroma_height = FF_CEIL_RSHIFT(s->height, chroma_v_shift);
+
for (i = 0; i < nb_components; i++) {
int c = s->comp_index[i];
data[c] = s->picture_ptr->data[c];
@@ -1299,8 +1304,8 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
if (s->interlaced && s->bottom_field)
block_offset += linesize[c] >> 1;
-if ( 8*(h * mb_x + x) < s->width
-&& 8*(v * mb_y + y) < s->height) {
+if ( 8*(h * mb_x + x) < ((c == 1) || (c == 2) ? chroma_width : s->width)
+&& 8*(v * mb_y + y) < ((c == 1) || (c == 2) ? chroma_height : s->height)) {
ptr = data[c] + block_offset;
} else
ptr = NULL;
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] vp9: fix pixel format changes with threading
---
libavcodec/vp9.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index dc0..5b90c13 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4228,7 +4228,7 @@ static int
vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
// detect size changes in other threads
if (s->intra_pred_data[0] &&
(!ssrc->intra_pred_data[0] || s->cols != ssrc->cols ||
- s->rows != ssrc->rows || s->bpp != ssrc->bpp)) {
+ s->rows != ssrc->rows || s->bpp != ssrc->bpp || s->pix_fmt !=
ssrc->pix_fmt)) {
free_buffers(s);
}
@@ -4260,6 +4260,7 @@ static int
vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo
s->bytesperpixel = ssrc->bytesperpixel;
s->bpp = ssrc->bpp;
s->bpp_index = ssrc->bpp_index;
+s->pix_fmt = ssrc->pix_fmt;
memcpy(&s->prob_ctx, &ssrc->prob_ctx, sizeof(s->prob_ctx));
memcpy(&s->s.h.lf_delta, &ssrc->s.h.lf_delta, sizeof(s->s.h.lf_delta));
memcpy(&s->s.h.segmentation.feat, &ssrc->s.h.segmentation.feat,
--
2.6.2.windows.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
Hi, Sorry for spamming, ran some more tests and found a bug in my patch, updated patch file attached. The bug was that in case subsample encryption was enabled (the default for AVC) the subsample size reported in the 'saiz' atom was wrong - it did not include the size of the IV. I originally tested my change by decrypting the file using MP4Box, and playing it, I guess MP4Box doesn't care about this discrepancy... In case anyone started reviewing the patch, the fix is in mov_cenc_end_packet, the lines: ctx->auxiliary_info_sizes[ctx->auxiliary_info_entries] = ctx->auxiliary_info_size - ctx->auxiliary_info_subsample_start; were changed to: ctx->auxiliary_info_sizes[ctx->auxiliary_info_entries] = AES_CTR_IV_SIZE + ctx->auxiliary_info_size - ctx->auxiliary_info_subsample_start; Thanks Eran 0001-movenc-support-cenc-common-encryption.patch Description: 0001-movenc-support-cenc-common-encryption.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc/developer: Remove mentioning of sending pull requests
On Sun, Dec 6, 2015 at 8:46 AM, wm4 wrote:
> On Sat, 5 Dec 2015 21:49:55 -0800
> Timothy Gu wrote:
>
>> The _de facto_ policy on patch submission has always been "sending it to
>> the mailing list."
>> ---
>> doc/developer.texi | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/doc/developer.texi b/doc/developer.texi
>> index 9a901d8..9383b21 100644
>> --- a/doc/developer.texi
>> +++ b/doc/developer.texi
>> @@ -32,13 +32,11 @@ consult @url{https://ffmpeg.org/legal.html}.
>>
>> @section Contributing
>>
>> -There are 3 ways by which code gets into ffmpeg.
>> +There are two ways by which code gets into ffmpeg.
>> @itemize @bullet
>> @item Submitting Patches to the main developer mailing list
>>see @ref{Submitting patches} for details.
>> @item Directly committing changes to the main tree.
>> -@item Committing changes to a git clone, for example on github.com or
>> - gitorious.org. And asking us to merge these changes.
>
> Technically, doing this is ok. We just don't want to use github's pull
> request UI. Changes should be reviewed and discussed on the ML. But
> posting on the ML and linking to an existing git tree isn't too bad, I
> suppose.
But that is covered in point 1: patches get sent to the ML. This needs
to be done regardless. What a user does on top is really up to them,
e.g referencing a git tree in the patch notes.
>
>> @end itemize
>>
>> Whichever way, changes should be reviewed by the maintainer of the code
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] ffprobe: Do not print profile names in -bitexact
On Tue, Dec 01, 2015 at 05:37:54PM -0800, Timothy Gu wrote: > On Fri, Nov 27, 2015 at 02:33:02PM -0800, Timothy Gu wrote: > > Instead, print "unknown" if it's unknown, or their numerical values if > > they are known. > > --- > > > > Addresses Nicholas's comment. > > > > --- > > ffprobe.c | 12 +--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > Ping. applied both patches so this is fixed, sorry i forgot about them Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Old school: Use the lowest level language in which you can solve the problem conveniently. New school: Use the highest level language in which the latest supercomputer can solve the problem without the user falling asleep waiting. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] fate: change the CMP_TARGETs for the recent AAC encoder changes
The case of PNS was outdated and resulted in failures on some kdfreebds systems. Signed-off-by: Rostislav Pehlivanov --- tests/fate/aac.mak | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/fate/aac.mak b/tests/fate/aac.mak index 09777af..d658751 100644 --- a/tests/fate/aac.mak +++ b/tests/fate/aac.mak @@ -146,7 +146,7 @@ fate-aac-aref-encode: CMD = enc_dec_pcm adts wav s16le $(REF) -strict -2 -c:a aa fate-aac-aref-encode: CMP = stddev fate-aac-aref-encode: REF = ./tests/data/asynth-44100-2.wav fate-aac-aref-encode: CMP_SHIFT = -4096 -fate-aac-aref-encode: CMP_TARGET = 670 +fate-aac-aref-encode: CMP_TARGET = 586 fate-aac-aref-encode: SIZE_TOLERANCE = 2464 fate-aac-aref-encode: FUZZ = 89 @@ -155,7 +155,7 @@ fate-aac-ln-encode: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audio-ref fate-aac-ln-encode: CMP = stddev fate-aac-ln-encode: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-ln-encode: CMP_SHIFT = -4096 -fate-aac-ln-encode: CMP_TARGET = 50 +fate-aac-ln-encode: CMP_TARGET = 61 fate-aac-ln-encode: SIZE_TOLERANCE = 3560 fate-aac-ln-encode: FUZZ = 30 @@ -164,7 +164,7 @@ fate-aac-ln-encode-128k: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audi fate-aac-ln-encode-128k: CMP = stddev fate-aac-ln-encode-128k: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-ln-encode-128k: CMP_SHIFT = -4096 -fate-aac-ln-encode-128k: CMP_TARGET = 798 +fate-aac-ln-encode-128k: CMP_TARGET = 800 fate-aac-ln-encode-128k: SIZE_TOLERANCE = 3560 fate-aac-ln-encode-128k: FUZZ = 5 @@ -173,7 +173,7 @@ fate-aac-pns-encode: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audio-re fate-aac-pns-encode: CMP = stddev fate-aac-pns-encode: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-pns-encode: CMP_SHIFT = -4096 -fate-aac-pns-encode: CMP_TARGET = 663 +fate-aac-pns-encode: CMP_TARGET = 616 fate-aac-pns-encode: SIZE_TOLERANCE = 3560 fate-aac-pns-encode: FUZZ = 72 @@ -182,7 +182,7 @@ fate-aac-tns-encode: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audio-re fate-aac-tns-encode: CMP = stddev fate-aac-tns-encode: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-tns-encode: CMP_SHIFT = -4096 -fate-aac-tns-encode: CMP_TARGET = 857 +fate-aac-tns-encode: CMP_TARGET = 818 fate-aac-tns-encode: FUZZ = 6 fate-aac-tns-encode: SIZE_TOLERANCE = 3560 @@ -200,7 +200,7 @@ fate-aac-ms-encode: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audio-ref fate-aac-ms-encode: CMP = stddev fate-aac-ms-encode: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-ms-encode: CMP_SHIFT = -4096 -fate-aac-ms-encode: CMP_TARGET = 682 +fate-aac-ms-encode: CMP_TARGET = 675 fate-aac-ms-encode: SIZE_TOLERANCE = 3560 fate-aac-ms-encode: FUZZ = 15 @@ -218,7 +218,7 @@ fate-aac-pred-encode: CMD = enc_dec_pcm adts wav s16le $(TARGET_SAMPLES)/audio-r fate-aac-pred-encode: CMP = stddev fate-aac-pred-encode: REF = $(SAMPLES)/audio-reference/luckynight_2ch_44kHz_s16.wav fate-aac-pred-encode: CMP_SHIFT = -4096 -fate-aac-pred-encode: CMP_TARGET = 835 +fate-aac-pred-encode: CMP_TARGET = 841 fate-aac-pred-encode: FUZZ = 12 fate-aac-pred-encode: SIZE_TOLERANCE = 3560 -- 2.6.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] aacenc_tns: tune and reduce artifacts
There are a couple of major changes here:
1. Start using TNS coefficient compression.
2. Start using 3 bits per coefficient maximum for short windows.
The bits we save from these 2 changes seem to make a nice impact on the
rest of the file/windows.
3. Remove special case gain checking for short windows.
4. Modify the coefficient loop to support up to 3 windows.
The additional restrictions on TNS were something that was no in the
specifications and furthermore restricting TNS to only low energy short
windows was done to compensate for bugs elsewhere in the code.
Overall, the improvements here reduce crackling artifacts heard in very
noisy tracks.
Signed-off-by: Rostislav Pehlivanov
---
libavcodec/aacenc_tns.c | 66 +++--
1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/libavcodec/aacenc_tns.c b/libavcodec/aacenc_tns.c
index 85fcb0f..84f5d09 100644
--- a/libavcodec/aacenc_tns.c
+++ b/libavcodec/aacenc_tns.c
@@ -35,17 +35,14 @@
#define TNS_Q_BITS 4
/* Coefficient resolution in short windows */
-#define TNS_Q_BITS_IS8 4
+#define TNS_Q_BITS_IS8 3
-/* Define this to save a bit, be warned decoders can't deal with it
- * so it is not lossless despite what the specifications say */
-// #define TNS_ENABLE_COEF_COMPRESSION
+/* We really need the bits we save here elsewhere */
+#define TNS_ENABLE_COEF_COMPRESSION
/* TNS will only be used if the LPC gain is within these margins */
-#define TNS_GAIN_THRESHOLD_LOW 1.477f
-#define TNS_GAIN_THRESHOLD_HIGH 7.0f
-#define TNS_GAIN_THRESHOLD_LOW_IS8 0.16f*TNS_GAIN_THRESHOLD_LOW
-#define TNS_GAIN_THRESHOLD_HIGH_IS8 0.26f*TNS_GAIN_THRESHOLD_HIGH
+#define TNS_GAIN_THRESHOLD_LOW 1.4f
+#define TNS_GAIN_THRESHOLD_HIGH 1.16f*TNS_GAIN_THRESHOLD_LOW
static inline int compress_coeffs(int *coef, int order, int c_bits)
{
@@ -71,8 +68,8 @@ static inline int compress_coeffs(int *coef, int order, int
c_bits)
*/
void ff_aac_encode_tns_info(AACEncContext *s, SingleChannelElement *sce)
{
-int i, w, filt, coef_compress = 0, coef_len;
TemporalNoiseShaping *tns = &sce->tns;
+int i, w, filt, coef_compress = 0, coef_len;
const int is8 = sce->ics.window_sequence[0] == EIGHT_SHORT_SEQUENCE;
const int c_bits = is8 ? TNS_Q_BITS_IS8 == 4 : TNS_Q_BITS == 4;
@@ -152,7 +149,7 @@ static inline void quantize_coefs(double *coef, int *idx,
float *lpc, int order,
int i;
const float *quant_arr = tns_tmp2_map[c_bits];
for (i = 0; i < order; i++) {
-idx[i] = quant_array_idx((float)coef[i], quant_arr, c_bits ? 16 : 8);
+idx[i] = quant_array_idx(coef[i], quant_arr, c_bits ? 16 : 8);
lpc[i] = quant_arr[idx[i]];
}
}
@@ -163,36 +160,37 @@ static inline void quantize_coefs(double *coef, int *idx,
float *lpc, int order,
void ff_aac_search_for_tns(AACEncContext *s, SingleChannelElement *sce)
{
TemporalNoiseShaping *tns = &sce->tns;
-double gain, coefs[MAX_LPC_ORDER];
int w, w2, g, count = 0;
+double gain, coefs[MAX_LPC_ORDER];
const int mmm = FFMIN(sce->ics.tns_max_bands, sce->ics.max_sfb);
const int is8 = sce->ics.window_sequence[0] == EIGHT_SHORT_SEQUENCE;
const int c_bits = is8 ? TNS_Q_BITS_IS8 == 4 : TNS_Q_BITS == 4;
+const int sfb_start = av_clip(tns_min_sfb[is8][s->samplerate_index], 0,
mmm);
+const int sfb_end = av_clip(sce->ics.num_swb, 0, mmm);
+const int order = is8 ? 7 : s->profile == FF_PROFILE_AAC_LOW ? 12 :
TNS_MAX_ORDER;
const int slant = sce->ics.window_sequence[0] == LONG_STOP_SEQUENCE ? 1 :
sce->ics.window_sequence[0] == LONG_START_SEQUENCE ? 0 :
2;
-int sfb_start = av_clip(tns_min_sfb[is8][s->samplerate_index], 0, mmm);
-int sfb_end = av_clip(sce->ics.num_swb, 0, mmm);
-int order = is8 ? 5 : s->profile == FF_PROFILE_AAC_LOW ? 12 :
TNS_MAX_ORDER;
-
for (w = 0; w < sce->ics.num_windows; w++) {
float en[2] = {0.0f, 0.0f};
+int oc_start = 0, os_start = 0;
int coef_start = w*sce->ics.num_swb + sce->ics.swb_offset[sfb_start];
int coef_len = sce->ics.swb_offset[sfb_end] -
sce->ics.swb_offset[sfb_start];
+const int sfb_len = sfb_end - sfb_start;
for (g = 0; g < sce->ics.num_swb; g++) {
if (w*16+g < sfb_start || w*16+g > sfb_end)
continue;
for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
FFPsyBand *band =
&s->psy.ch[s->cur_channel].psy_bands[(w+w2)*16+g];
-if ((w+w2)*16+g > sfb_start + ((sfb_end - sfb_start)/2))
+if ((w+w2)*16+g > sfb_start + (sfb_len/2))
en[1] += band->energy;
else
en[0] += band->energy;
}
}
-if (coef_len <= 0 || (sfb_end - sfb_start) <= 0)
+if (coef_len <= 0 || sfb_len <= 0)
continue;
/* LPC */
@@ -201,30 +199,18 @@ void ff_aac_search_for_tns(AACEncContext *s,
S
[FFmpeg-devel] [PATCH 1/3] aacenc: move the TNS search and filtering before PNS
The original plan was to have TNS use data from the PNS search to better
tune itself to noise but this was never used nor necessary. This should
slightly boost the PNS accuracy if TNS was used.
Signed-off-by: Rostislav Pehlivanov
---
libavcodec/aacenc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index 7220ddc..2156fc0 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -678,14 +678,14 @@ static int aac_encode_frame(AVCodecContext *avctx,
AVPacket *avpkt,
for (ch = 0; ch < chans; ch++) { /* TNS and PNS */
sce = &cpe->ch[ch];
s->cur_channel = start_ch + ch;
-if (s->options.pns && s->coder->search_for_pns)
-s->coder->search_for_pns(s, avctx, sce);
if (s->options.tns && s->coder->search_for_tns)
s->coder->search_for_tns(s, sce);
if (s->options.tns && s->coder->apply_tns_filt)
s->coder->apply_tns_filt(s, sce);
if (sce->tns.present)
tns_mode = 1;
+if (s->options.pns && s->coder->search_for_pns)
+s->coder->search_for_pns(s, avctx, sce);
}
s->cur_channel = start_ch;
if (s->options.intensity_stereo) { /* Intensity Stereo */
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc/developer: Remove mentioning of sending pull requests
On Sat, 5 Dec 2015 21:49:55 -0800
Timothy Gu wrote:
> The _de facto_ policy on patch submission has always been "sending it to
> the mailing list."
> ---
> doc/developer.texi | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 9a901d8..9383b21 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -32,13 +32,11 @@ consult @url{https://ffmpeg.org/legal.html}.
>
> @section Contributing
>
> -There are 3 ways by which code gets into ffmpeg.
> +There are two ways by which code gets into ffmpeg.
> @itemize @bullet
> @item Submitting Patches to the main developer mailing list
>see @ref{Submitting patches} for details.
> @item Directly committing changes to the main tree.
> -@item Committing changes to a git clone, for example on github.com or
> - gitorious.org. And asking us to merge these changes.
Technically, doing this is ok. We just don't want to use github's pull
request UI. Changes should be reviewed and discussed on the ML. But
posting on the ML and linking to an existing git tree isn't too bad, I
suppose.
> @end itemize
>
> Whichever way, changes should be reviewed by the maintainer of the code
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/8] avfilter/show_palette: fix memory leak
On Sat, Dec 5, 2015 at 6:40 AM, Clément Bœsch wrote: > On Fri, Dec 04, 2015 at 05:56:12PM -0500, Ganesh Ajjanagadde wrote: >> On Fri, Dec 4, 2015 at 5:29 PM, Marton Balint wrote: >> if ((ret = ff_formats_ref(in , &ctx->inputs[0]->out_formats)) < 0 >> || >> (ret = ff_formats_ref(out, &ctx->outputs[0]->in_formats)) < 0) >> -return ret; >> +goto fail; >> return 0; >> +fail: >> >>> >> >>> >> +av_freep(&in->formats); >> >>> >> >>> >> >>> what if in==NULL? >> >>> >> +av_freep(&in); >> >>> >> >>> >> +av_freep(&out->formats); >> >>> >> >>> >> >>> ditto >> >>> >> +av_freep(&out); >> +return ret; >> } >> >> >> >> >> >> Fixed locally with an if(in) and similar checks. Also applies to other >> >> patches I sent. >> > >> > >> > Maybe it's just me, but don't we usually use two labels for such cases? >> > >> > E.g. >> > >> > fail1: >> >av_freep(&in->xxx); >> > fail2: >> >av_freep(&in); >> >return ret; >> >> I don't really mind, I personally prefer a single goto as I find it >> logically simpler to analyze. >> > > I also prefer a if branching here. Any further comments, or is it ok to push? > > -- > Clément B. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] avformat/rtsp: free opts dictionary on failure of getnameinfo
On Fri, Dec 4, 2015 at 9:24 PM, Michael Niedermayer wrote: > On Fri, Dec 04, 2015 at 09:39:40AM -0500, Ganesh Ajjanagadde wrote: >> Fixes: CID 1341579. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- >> libavformat/rtsp.c | 1 + >> 1 file changed, 1 insertion(+) > > LGTM > > thx pushed, thanks. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I do not agree with what you have to say, but I'll defend to the death your > right to say it. -- Voltaire > > ___ > 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 4/8] avformat/movenc-test: correct varargs usage
On Fri, Dec 4, 2015 at 9:27 PM, Michael Niedermayer wrote: > On Fri, Dec 04, 2015 at 09:39:39AM -0500, Ganesh Ajjanagadde wrote: >> It is required to call va_end for each invocation of va_start within the >> same function. >> >> Fixes: CID 1341583. >> >> Signed-off-by: Ganesh Ajjanagadde > > should be correct pushed, thanks. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The real ebay dictionary, page 3 > "Rare item" - "Common item with rare defect or maybe just a lie" > "Professional" - "'Toy' made in china, not functional except as doorstop" > "Experts will know" - "The seller hopes you are not an expert" > > ___ > 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 8/8] avcodec/dvdsubdec: fix typo in dlog message
On Fri, Dec 4, 2015 at 9:28 PM, Michael Niedermayer wrote: > On Fri, Dec 04, 2015 at 09:39:43AM -0500, Ganesh Ajjanagadde wrote: >> Likely accidental in 764900d6458a2f79166ff91df4f20ad39cd6acec. >> >> Fixes: CID 1341578. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- > > ok pushed, thanks. > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > > ___ > 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 02/10] ffplay: use hypot()
On Sun, Dec 6, 2015 at 7:07 AM, Marton Balint wrote:
>
> On Sat, 5 Dec 2015, Ganesh Ajjanagadde wrote:
>
>> On Sun, Nov 22, 2015 at 12:05 PM, Ganesh Ajjanagadde
>> wrote:
>>>
>>> Signed-off-by: Ganesh Ajjanagadde
>>> ---
>>> ffplay.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ffplay.c b/ffplay.c
>>> index 2c1817e..36da8a5 100644
>>> --- a/ffplay.c
>>> +++ b/ffplay.c
>>> @@ -1115,9 +1115,9 @@ static void video_audio_display(VideoState *s)
>>> * directly access it but it is more than fast enough. */
>>> for (y = 0; y < s->height; y++) {
>>> double w = 1 / sqrt(nb_freq);
>>> -int a = sqrt(w * sqrt(data[0][2 * y + 0] * data[0][2 * y
>>> + 0] + data[0][2 * y + 1] * data[0][2 * y + 1]));
>>> -int b = (nb_display_channels == 2 ) ? sqrt(w *
>>> sqrt(data[1][2 * y + 0] * data[1][2 * y + 0]
>>> - + data[1][2 * y + 1] * data[1][2 * y + 1])) : a;
>>> +int a = sqrt(w * hypot(data[0][2 * y + 0], data[0][2 * y
>>> + 1]));
>>> +int b = (nb_display_channels == 2 ) ? sqrt(w *
>>> hypot(data[1][2 * y + 0], data[1][2 * y + 1]))
>>> +: a;
>>> a = FFMIN(a, 255);
>>> b = FFMIN(b, 255);
>>> fgcolor = SDL_MapRGB(screen->format, a, b, (a + b) / 2);
>>> --
>>> 2.6.2
>>>
>>
>> @Marton: are you fine with this? I think it improves readability;
>> accuracy benefits are likely irrelevant here.
>
>
> Yes, sure, LGTM.
>
> Sorry for forgetting this.
No problem. Pushed, thanks.
>
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 7/9] avcodec/mdct_template: use lrint instead of floor hack
On Tue, Dec 1, 2015 at 7:27 PM, Ganesh Ajjanagadde wrote: > Signed-off-by: Ganesh Ajjanagadde > --- > libavcodec/mdct_template.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/mdct_template.c b/libavcodec/mdct_template.c > index e7e5f62..ecdeb54 100644 > --- a/libavcodec/mdct_template.c > +++ b/libavcodec/mdct_template.c > @@ -23,6 +23,7 @@ > #include > #include "libavutil/common.h" > #include "libavutil/mathematics.h" > +#include "libavutil/libm.h" > #include "fft.h" > #include "fft-internal.h" > > @@ -82,8 +83,8 @@ av_cold int ff_mdct_init(FFTContext *s, int nbits, int > inverse, double scale) > for(i=0;i alpha = 2 * M_PI * (i + theta) / n; > #if FFT_FIXED_32 > -s->tcos[i*tstep] = (FFTSample)floor(-cos(alpha) * 2147483648.0 + > 0.5); > -s->tsin[i*tstep] = (FFTSample)floor(-sin(alpha) * 2147483648.0 + > 0.5); > +s->tcos[i*tstep] = lrint(-cos(alpha) * 2147483648.0); > +s->tsin[i*tstep] = lrint(-sin(alpha) * 2147483648.0); > #else > s->tcos[i*tstep] = FIX15(-cos(alpha) * scale); > s->tsin[i*tstep] = FIX15(-sin(alpha) * scale); > -- > 2.6.2 > Ping for this one, it is relatively important among the lrint patches as this is frequently used and table may be large, making speed benefit somewhat useful. I have noted the header alphabetical include order mistake above. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc/developer: Remove mentioning of sending pull requests
On Sun, Dec 6, 2015 at 12:49 AM, Timothy Gu wrote:
> The _de facto_ policy on patch submission has always been "sending it to
> the mailing list."
> ---
> doc/developer.texi | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 9a901d8..9383b21 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -32,13 +32,11 @@ consult @url{https://ffmpeg.org/legal.html}.
>
> @section Contributing
>
> -There are 3 ways by which code gets into ffmpeg.
> +There are two ways by which code gets into ffmpeg.
While at it, change ffmpeg->FFmpeg, i.e the actual project name.
> @itemize @bullet
> @item Submitting Patches to the main developer mailing list
>see @ref{Submitting patches} for details.
> @item Directly committing changes to the main tree.
> -@item Committing changes to a git clone, for example on github.com or
> - gitorious.org. And asking us to merge these changes.
> @end itemize
>
> Whichever way, changes should be reviewed by the maintainer of the code
Else LGTM, thanks.
> --
> 2.1.4
>
> ___
> 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] diracdec: Move strides to bytes, and pointer types to uint8_t.
On Sun, Dec 06, 2015 at 12:36:24PM +, Kieran Kunhya wrote: > Start templating functions for move to support 10-bit > Parts of this patch were written by Rostislav Pehlivanov "Signed-off-by:"? [...] -- Clément B. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] diracdec: Move strides to bytes, and pointer types to uint8_t.
Start templating functions for move to support 10-bit
Parts of this patch were written by Rostislav Pehlivanov
---
libavcodec/dirac.c | 10 +-
libavcodec/dirac.h | 3 +-
libavcodec/diracdec.c | 227 ++--
libavformat/oggparsedirac.c | 4 +-
4 files changed, 146 insertions(+), 98 deletions(-)
diff --git a/libavcodec/dirac.c b/libavcodec/dirac.c
index 07db919..aa82dd9 100644
--- a/libavcodec/dirac.c
+++ b/libavcodec/dirac.c
@@ -118,7 +118,7 @@ static const enum AVPixelFormat dirac_pix_fmt[2][3] = {
/* [DIRAC_STD] 10.3 Parse Source Parameters.
* source_parameters(base_video_format) */
static int parse_source_parameters(AVCodecContext *avctx, GetBitContext *gb,
- dirac_source_params *source)
+ dirac_source_params *source, int *bit_depth)
{
AVRational frame_rate = { 0, 0 };
unsigned luma_depth = 8, luma_offset = 16;
@@ -239,6 +239,9 @@ static int parse_source_parameters(AVCodecContext *avctx,
GetBitContext *gb,
if (luma_depth > 8)
av_log(avctx, AV_LOG_WARNING, "Bitdepth greater than 8\n");
+
+*bit_depth = luma_depth;
+
avctx->pix_fmt = dirac_pix_fmt[!luma_offset][source->chroma_format];
avcodec_get_chroma_sub_sample(avctx->pix_fmt, &chroma_x_shift,
&chroma_y_shift);
if ((source->width % (1height);
diff --git a/libavcodec/dirac.h b/libavcodec/dirac.h
index b0f955b..14653f1 100644
--- a/libavcodec/dirac.h
+++ b/libavcodec/dirac.h
@@ -55,6 +55,7 @@ typedef struct dirac_source_params {
} dirac_source_params;
int avpriv_dirac_parse_sequence_header(AVCodecContext *avctx, GetBitContext
*gb,
- dirac_source_params *source);
+ dirac_source_params *source,
+ int *bit_depth);
#endif /* AVCODEC_DIRAC_H */
diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c
index ea16007..945ee3f 100644
--- a/libavcodec/diracdec.c
+++ b/libavcodec/diracdec.c
@@ -97,11 +97,12 @@ typedef struct {
typedef struct SubBand {
int level;
int orientation;
-int stride;
+int stride; /* in bytes */
int width;
int height;
+int pshift;
int quant;
-IDWTELEM *ibuf;
+uint8_t *ibuf;
struct SubBand *parent;
/* for low delay */
@@ -117,9 +118,9 @@ typedef struct Plane {
int idwt_width;
int idwt_height;
int idwt_stride;
-IDWTELEM *idwt_buf;
-IDWTELEM *idwt_buf_base;
-IDWTELEM *idwt_tmp;
+uint8_t *idwt_buf;
+uint8_t *idwt_buf_base;
+uint8_t *idwt_tmp;
/* block length */
uint8_t xblen;
@@ -147,6 +148,9 @@ typedef struct DiracContext {
int chroma_x_shift;
int chroma_y_shift;
+int bit_depth; /* bit depth */
+int pshift; /* pixel shift = bit_depth > 8 */
+
int zero_res; /* zero residue flag */
int is_arith; /* whether coeffs use arith or golomb coding */
int low_delay; /* use the low delay syntax */
@@ -339,9 +343,9 @@ static int alloc_sequence_buffers(DiracContext *s)
w = FFALIGN(CALC_PADDING(w, MAX_DWT_LEVELS), 8); /* FIXME: Should this
be 16 for SSE??? */
h = top_padding + CALC_PADDING(h, MAX_DWT_LEVELS) + max_yblen/2;
-s->plane[i].idwt_buf_base = av_mallocz_array((w+max_xblen), h *
sizeof(IDWTELEM));
-s->plane[i].idwt_tmp = av_malloc_array((w+16), sizeof(IDWTELEM));
-s->plane[i].idwt_buf = s->plane[i].idwt_buf_base + top_padding*w;
+s->plane[i].idwt_buf_base = av_mallocz_array((w+max_xblen), h * (2 <<
s->pshift));
+s->plane[i].idwt_tmp = av_malloc_array((w+16), 2 << s->pshift);
+s->plane[i].idwt_buf = s->plane[i].idwt_buf_base +
(top_padding*w)*(2 << s->pshift);
if (!s->plane[i].idwt_buf_base || !s->plane[i].idwt_tmp)
return AVERROR(ENOMEM);
}
@@ -462,38 +466,6 @@ static av_cold int dirac_decode_end(AVCodecContext *avctx)
#define SIGN_CTX(x) (CTX_SIGN_ZERO + ((x) > 0) - ((x) < 0))
-static inline void coeff_unpack_arith(DiracArith *c, int qfactor, int qoffset,
- SubBand *b, IDWTELEM *buf, int x, int y)
-{
-int coeff, sign;
-int sign_pred = 0;
-int pred_ctx = CTX_ZPZN_F1;
-
-/* Check if the parent subband has a 0 in the corresponding position */
-if (b->parent)
-pred_ctx += !!b->parent->ibuf[b->parent->stride * (y>>1) + (x>>1)] <<
1;
-
-if (b->orientation == subband_hl)
-sign_pred = buf[-b->stride];
-
-/* Determine if the pixel has only zeros in its neighbourhood */
-if (x) {
-pred_ctx += !(buf[-1] | buf[-b->stride] | buf[-1-b->stride]);
-if (b->orientation == subband_lh)
-sign_pred = buf[-1];
-
[FFmpeg-devel] [PATCH] avfilter/x86/vf_maskedmerge: move %define out of .nextrow
Signed-off-by: Paul B Mahol --- libavfilter/x86/vf_maskedmerge.asm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavfilter/x86/vf_maskedmerge.asm b/libavfilter/x86/vf_maskedmerge.asm index 3b128b4..e548d25 100644 --- a/libavfilter/x86/vf_maskedmerge.asm +++ b/libavfilter/x86/vf_maskedmerge.asm @@ -40,9 +40,9 @@ cglobal maskedmerge8, 10, 11, 7, 0, bsrc, osrc, msrc, dst, blinesize, olinesize, add msrcq, wq add dstq, wq neg wq -.nextrow: -mov r10q, wq %define x r10q +.nextrow: +mov x, wq .loop: movhm0, [bsrcq + x] -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/10] ffplay: use hypot()
On Sat, 5 Dec 2015, Ganesh Ajjanagadde wrote:
On Sun, Nov 22, 2015 at 12:05 PM, Ganesh Ajjanagadde
wrote:
Signed-off-by: Ganesh Ajjanagadde
---
ffplay.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ffplay.c b/ffplay.c
index 2c1817e..36da8a5 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -1115,9 +1115,9 @@ static void video_audio_display(VideoState *s)
* directly access it but it is more than fast enough. */
for (y = 0; y < s->height; y++) {
double w = 1 / sqrt(nb_freq);
-int a = sqrt(w * sqrt(data[0][2 * y + 0] * data[0][2 * y + 0]
+ data[0][2 * y + 1] * data[0][2 * y + 1]));
-int b = (nb_display_channels == 2 ) ? sqrt(w * sqrt(data[1][2
* y + 0] * data[1][2 * y + 0]
- + data[1][2 * y + 1] * data[1][2 * y + 1])) : a;
+int a = sqrt(w * hypot(data[0][2 * y + 0], data[0][2 * y +
1]));
+int b = (nb_display_channels == 2 ) ? sqrt(w * hypot(data[1][2
* y + 0], data[1][2 * y + 1]))
+: a;
a = FFMIN(a, 255);
b = FFMIN(b, 255);
fgcolor = SDL_MapRGB(screen->format, a, b, (a + b) / 2);
--
2.6.2
@Marton: are you fine with this? I think it improves readability;
accuracy benefits are likely irrelevant here.
Yes, sure, LGTM.
Sorry for forgetting this.
Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
Hi > One more question: Is FFmpeg able to decrypt the files (if the keys are > available)? If not, it would be nice if you could also add the decryption > function... > (I only realize now that the subject promises both.) > In any case, a fate test will be needed (but is not necessarily part of > your patch). > Yes, already wrote the code for that as well, I will send the patch once this one gets approved (both share the new aes_ctr module that I added, so I can't send them in parallel) > Carl Eugen Thanks Eran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] fate/api: test threadmessage
Le duodi 12 frimaire, an CCXXIV, Clement Boesch a écrit :
> From: Clément Bœsch
>
> ---
> tests/api/Makefile | 1 +
> tests/api/api-threadmessage-test.c | 263
> +
> tests/fate/api.mak | 6 +
> 3 files changed, 270 insertions(+)
> create mode 100644 tests/api/api-threadmessage-test.c
>
> diff --git a/tests/api/Makefile b/tests/api/Makefile
> index c48c34a..3556a9b 100644
> --- a/tests/api/Makefile
> +++ b/tests/api/Makefile
> @@ -3,6 +3,7 @@ APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264
> APITESTPROGS-yes += api-seek
> APITESTPROGS-yes += api-codec-param
> APITESTPROGS-$(call DEMDEC, H263, H263) += api-band
> +APITESTPROGS-yes += api-threadmessage
> APITESTPROGS += $(APITESTPROGS-yes)
>
> APITESTOBJS := $(APITESTOBJS:%=$(APITESTSDIR)%)
> $(APITESTPROGS:%=$(APITESTSDIR)/%-test.o)
> diff --git a/tests/api/api-threadmessage-test.c
> b/tests/api/api-threadmessage-test.c
> new file mode 100644
> index 000..5c75051
> --- /dev/null
> +++ b/tests/api/api-threadmessage-test.c
> @@ -0,0 +1,263 @@
> +/*
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +/**
> + * Thread message API test
> + */
> +
> +#include
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/frame.h"
> +#include "libavutil/threadmessage.h"
> +
> +struct worker_data {
> +int id;
> +pthread_t tid;
> +int workload;
> +AVThreadMessageQueue *queue;
> +};
> +
> +/* same as worker_data but shuffled for testing purpose */
Is it really useful? If you merge both, you can probably get rid of the
macros below with just a conditional for the function on pthread_create().
> +struct reader_data {
Nit: worker/reader sounds like a mixed metaphor.
> +pthread_t tid;
> +int workload;
> +int id;
> +AVThreadMessageQueue *queue;
> +};
> +
> +struct message {
> +AVFrame *frame;
> +// we add some junk in the message to make sure the message size is >
> +// sizeof(void*)
> +int magic;
> +};
> +
> +#define MAGIC 0xdeadc0de
> +
> +static void free_frame(void *arg)
> +{
> +struct message *msg = arg;
> +av_assert0(msg->magic == MAGIC);
> +av_frame_free(&msg->frame);
> +}
> +
> +static void *worker_thread(void *arg)
> +{
> +int i, ret = 0;
> +struct worker_data *wd = arg;
> +
> +av_log(NULL, AV_LOG_INFO, "worker #%d: workload=%d\n", wd->id,
> wd->workload);
> +for (i = 0; i < wd->workload; i++) {
> +if (rand() % wd->workload < wd->workload / 10) {
Nit: using module for PRNG is not recommended, it gives very bad results
with LCG.
> +av_log(NULL, AV_LOG_INFO, "worker #%d: flushing the queue\n",
> wd->id);
> +av_thread_message_flush(wd->queue);
> +} else {
Nit: inconsistent structure: if/else here, if+continue for readers.
> +char *val;
> +AVDictionary *meta = NULL;
> +struct message msg = {
> +.magic = MAGIC,
> +.frame = av_frame_alloc(),
> +};
> +
> +if (!msg.frame) {
> +ret = AVERROR(ENOMEM);
> +break;
> +}
> +
> +/* we add some metadata to identify the frames */
> +val = av_asprintf("frame from worker %d", wd->id);
> +if (!val) {
> +av_frame_free(&msg.frame);
> +ret = AVERROR(ENOMEM);
> +break;
> +}
> +ret = av_dict_set(&meta, "sig", val, AV_DICT_DONT_STRDUP_VAL);
> +if (ret < 0) {
> +av_frame_free(&msg.frame);
> +break;
> +}
> +av_frame_set_metadata(msg.frame, meta);
> +
> +/* allocate a real frame in order to simulate "real" work */
> +msg.frame->format = AV_PIX_FMT_RGBA;
> +msg.frame->width
Re: [FFmpeg-devel] [PATCH 1/3] avutil/threadmessage: add av_thread_message_flush()
Le quartidi 14 frimaire, an CCXXIV, Clement Boesch a écrit :
> From: Clément Bœsch
>
> ---
> libavutil/threadmessage.c | 31 +++
> libavutil/threadmessage.h | 16
> 2 files changed, 47 insertions(+)
>
> diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
> index b7fcbe2..a5f1507 100644
> --- a/libavutil/threadmessage.c
> +++ b/libavutil/threadmessage.c
> @@ -40,6 +40,7 @@ struct AVThreadMessageQueue {
> int err_send;
> int err_recv;
> unsigned elsize;
> +void (*free_func)(void *msg);
> #else
> int dummy;
> #endif
> @@ -81,10 +82,17 @@ int av_thread_message_queue_alloc(AVThreadMessageQueue
> **mq,
> #endif /* HAVE_THREADS */
> }
>
> +void av_thread_message_queue_set_free_func(AVThreadMessageQueue *mq,
> + void (*free_func)(void *msg))
> +{
> +mq->free_func = free_func;
> +}
> +
> void av_thread_message_queue_free(AVThreadMessageQueue **mq)
> {
> #if HAVE_THREADS
> if (*mq) {
> +av_thread_message_flush(*mq);
> av_fifo_freep(&(*mq)->fifo);
> pthread_cond_destroy(&(*mq)->cond);
> pthread_mutex_destroy(&(*mq)->lock);
> @@ -182,3 +190,26 @@ void
> av_thread_message_queue_set_err_recv(AVThreadMessageQueue *mq,
> pthread_mutex_unlock(&mq->lock);
> #endif /* HAVE_THREADS */
> }
> +
> +static void free_func_wrap(void *arg, void *msg, int size)
> +{
> +void (*free_func)(void *msg) = arg;
Technically, this is not legal: void* is a data pointer, it could be smaller
than a function pointer (remember the "medium" memory model for 16-bits
code). I do not object much, but it is easy to fix: just pass &free_func as
argument, or even squarely mq itself.
> +free_func(msg);
> +}
> +
> +void av_thread_message_flush(AVThreadMessageQueue *mq)
> +{
> +#if HAVE_THREADS
> +int used, off;
> +void *free_func = mq->free_func;
> +
> +pthread_mutex_lock(&mq->lock);
> +used = av_fifo_size(mq->fifo);
> +if (free_func)
> +for (off = 0; off < used; off += mq->elsize)
> +av_fifo_generic_peek_at(mq->fifo, free_func, off, mq->elsize,
> free_func_wrap);
> +av_fifo_drain(mq->fifo, used);
> +pthread_cond_broadcast(&mq->cond);
> +pthread_mutex_unlock(&mq->lock);
> +#endif /* HAVE_THREADS */
> +}
> diff --git a/libavutil/threadmessage.h b/libavutil/threadmessage.h
> index a8481d8..e256cae 100644
> --- a/libavutil/threadmessage.h
> +++ b/libavutil/threadmessage.h
> @@ -88,4 +88,20 @@ void
> av_thread_message_queue_set_err_send(AVThreadMessageQueue *mq,
> void av_thread_message_queue_set_err_recv(AVThreadMessageQueue *mq,
>int err);
>
> +/**
> + * Set the optional free message callback function which will be called if an
> + * operation is removing messages from the queue.
> + */
> +void av_thread_message_queue_set_free_func(AVThreadMessageQueue *mq,
> + void (*free_func)(void *msg));
> +
> +/**
> + * Flush the message queue
> + *
> + * This function is mostly equivalent to reading and free-ing every message
> + * except that it will be done in a single operation (no lock/unlock between
> + * reads).
> + */
> +void av_thread_message_flush(AVThreadMessageQueue *mq);
> +
> #endif /* AVUTIL_THREADMESSAGE_H */
Rest looks good to me, thanks for humouring me.
Regards,
--
Nicolas George
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avutil/threadmessage: fix condition broadcasting
Le quintidi 15 frimaire, an CCXXIV, Clement Boesch a écrit : > This was before providing the 2 cond version, but OK Sorry, missed that. Patch looks good to me, but you forgot to update the commit message. > I'd like to push the test as well, but it depends on the first patch > (flush func). I clarified a bit the use of the flush in the doxy. Are you > still uncomfortable about this one? I will look at it soon. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavtuil: add version component accessor macros
Pushed as: commit 21c34cb26154a5eadd6e10df86c20e2df3a7bd55 Author: Reynaldo H. Verdejo Pinochet Date: Fri Dec 4 14:07:23 2015 -0800 libavutil: add version component accessor macros [..] Thanks -- Reynaldo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavtuil: add version component accessor macros
On 12/05/2015 11:36 AM, Ganesh Ajjanagadde wrote: > [...] > > minor nit: commit message typo (libavtuil -> libavutil). > True. Corrected. Thanks for taking a look -- Reynaldo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
Hi! On Saturday 05 December 2015 09:16:22 pm Eran Kornblau wrote: > Fixed the else convention and squashed all commits into one, the updated > patch is attached. Thank you! One more question: Is FFmpeg able to decrypt the files (if the keys are available)? If not, it would be nice if you could also add the decryption function... (I only realize now that the subject promises both.) In any case, a fate test will be needed (but is not necessarily part of your patch). Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
