Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Andrey Utkin
2014-10-25 2:28 GMT+04:00 Timothy Gu timothyg...@gmail.com:
 Those URLs are very explicit, while `/dev/` can mean anything,
 including `/dev/stdin`.

Could you elaborate what this practically means? BTW there's no /dev
in my patch, but /dev/video.

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread wm4
On Fri, 24 Oct 2014 19:50:25 +0400
Andrey Utkin andrey.krieger.ut...@gmail.com wrote:

 ---
  libavdevice/v4l2.c | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
 index cf7a92c..ebc50bb 100644
 --- a/libavdevice/v4l2.c
 +++ b/libavdevice/v4l2.c
 @@ -806,6 +806,13 @@ static int device_try_init(AVFormatContext *ctx,
  return ret;
  }
  
 +static int v4l2_read_probe(AVProbeData *p)
 +{
 +if (av_strstart(p-filename, /dev/video, NULL))
 +return AVPROBE_SCORE_MAX;
 +return 0;
 +}
 +
  static int v4l2_read_header(AVFormatContext *ctx)
  {
  struct video_data *s = ctx-priv_data;
 @@ -1033,6 +1040,7 @@ AVInputFormat ff_v4l2_demuxer = {
  .name   = video4linux2,v4l2,
  .long_name  = NULL_IF_CONFIG_SMALL(Video4Linux2 device grab),
  .priv_data_size = sizeof(struct video_data),
 +.read_probe = v4l2_read_probe,
  .read_header= v4l2_read_header,
  .read_packet= v4l2_read_packet,
  .read_close = v4l2_read_close,

IMHO it should check whether it's really a video device.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Nicolas George
Le tridi 3 brumaire, an CCXXIII, Timothy Gu a écrit :
 Those URLs are very explicit, while `/dev/` can mean anything,
 including `/dev/stdin`.

Andrey quoted these bits of code to show that checking filename for NULL is
not necessary. I believe his proof is conclusive for that point.

wm4 :
 IMHO it should check whether it's really a video device.

Yes, it should. Do you intend to propose the patch that invokes stat() on
all supported operating systems, check that can not devolve into a security
issue in the context of probing, find the relevant list of major/minor
pairs and a way of testing the dynamically allocated ones soon, or are you
just bikeshedding.

Because if you are just bikeshedding, I believe the patch with a lower
score, EXTENSION instead of MAX maybe, would be fine.

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] doc/fftools-common-opts: document -devices option

2014-10-25 Thread Michael Niedermayer
On Fri, Oct 24, 2014 at 11:31:14PM +0200, Lukasz Marek wrote:
 Signed-off-by: Lukasz Marek lukasz.m.lu...@gmail.com
 ---
  doc/fftools-common-opts.texi | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

LGTM

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

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


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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread wm4
On Sat, 25 Oct 2014 11:43:12 +0200
Nicolas George geo...@nsup.org wrote:

 Le tridi 3 brumaire, an CCXXIII, Timothy Gu a écrit :
  Those URLs are very explicit, while `/dev/` can mean anything,
  including `/dev/stdin`.
 
 Andrey quoted these bits of code to show that checking filename for NULL is
 not necessary. I believe his proof is conclusive for that point.
 
 wm4 :
  IMHO it should check whether it's really a video device.
 
 Yes, it should. Do you intend to propose the patch that invokes stat() on
 all supported operating systems, check that can not devolve into a security
 issue in the context of probing, find the relevant list of major/minor
 pairs and a way of testing the dynamically allocated ones soon, or are you
 just bikeshedding.

Don't present your extremely bad ideas as mine. Thanks.

 Because if you are just bikeshedding, I believe the patch with a lower
 score, EXTENSION instead of MAX maybe, would be fine.
 
 Regards,
 

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, wm4 a écrit :
 Don't present your extremely bad ideas as mine. Thanks.

Sorry, I had not realized you wanted him to check whether it's really a
video device by the application of careful handwaving and accurate magic.

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] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 01:10:20PM +0200, wm4 wrote:
 On Sat, 25 Oct 2014 11:43:12 +0200
 Nicolas George geo...@nsup.org wrote:
 
  Le tridi 3 brumaire, an CCXXIII, Timothy Gu a écrit :
   Those URLs are very explicit, while `/dev/` can mean anything,
   including `/dev/stdin`.
  
  Andrey quoted these bits of code to show that checking filename for NULL is
  not necessary. I believe his proof is conclusive for that point.
  
  wm4 :
   IMHO it should check whether it's really a video device.
  
  Yes, it should. Do you intend to propose the patch that invokes stat() on
  all supported operating systems, check that can not devolve into a security
  issue in the context of probing, find the relevant list of major/minor
  pairs and a way of testing the dynamically allocated ones soon, or are you
  just bikeshedding.
 
 Don't present your extremely bad ideas as mine. Thanks.

Could you both please either bury or hide your hatchet?
It is really annoying to all other when you write such obviously
aggressive and non-constructive emails.
wm4, if you have a simple way of doing this check, please explain it.
Otherwise I think EXTENSION score is good enough to signal well, we
guess that's what it is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/6] dv: better split weight tables assignment

2014-10-25 Thread Christophe Gisquet
This is a mostly cosmetical patch in preparation for the following.
---
 libavcodec/dv.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index 4b23f2a..e1e5dd9 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -186,7 +186,6 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 {
 int j, i, c, s, p;
 uint32_t *factor1, *factor2;
-const int *iweight1, *iweight2;
 
 p = i = 0;
 for (c = 0; c  d-n_difchan; c++) {
@@ -206,14 +205,15 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 
 factor1 = ctx-idct_factor[0];
 factor2 = ctx-idct_factor[DV_PROFILE_IS_HD(d) ? 4096 : 2816];
-if (d-height == 720) {
-iweight1 = ff_dv_iweight_720_y[0];
-iweight2 = ff_dv_iweight_720_c[0];
-} else {
-iweight1 = ff_dv_iweight_1080_y[0];
-iweight2 = ff_dv_iweight_1080_c[0];
-}
 if (DV_PROFILE_IS_HD(d)) {
+const int *iweight1, *iweight2;
+if (d-height == 720) {
+iweight1 = ff_dv_iweight_720_y[0];
+iweight2 = ff_dv_iweight_720_c[0];
+} else {
+iweight1 = ff_dv_iweight_1080_y[0];
+iweight2 = ff_dv_iweight_1080_c[0];
+}
 for (c = 0; c  4; c++) {
 for (s = 0; s  16; s++) {
 for (i = 0; i  64; i++) {
@@ -223,7 +223,7 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 }
 }
 } else {
-iweight1 = ff_dv_iweight_88[0];
+const int *iweight1 = ff_dv_iweight_88[0];
 for (j = 0; j  2; j++, iweight1 = ff_dv_iweight_248[0]) {
 for (s = 0; s  22; s++) {
 for (i = c = 0; c  4; c++) {
-- 
1.9.2.msysgit.0

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


[FFmpeg-devel] [PATCH 0/6] dv: of inverse weight tables

2014-10-25 Thread Christophe Gisquet
While investigating ticket #2970, I noticed several things about these
tables. Patch #1 is kind of preparatory for #2.

Patch #3 uses mathematically more correct values, which happens to match
another dv implementation (cedocida). It does not seem to affect that
much the decoded output, as there is PSNR variations on the 3rd decimal
at most. The best I got was, using a resized version of lena.ppm, a PSNR
going from 49.35dB to 49.36dB. It would be ok not to integrate it, as
one may argue they were devised with the simple_idct implementation in
mind (I haven't checked).

Patch #4 fixes the actual issue. As I don't want to bother determining
how the table was incorrectly mangled, I've preferred regenerating it.

Patch #5 is somewhat futile, but it's free lunch.

Patch #6 uses the fact that the encoder has its own weight tables, not
shareable, and thus all inverse weight table init code is moved to the
decoder.

Christophe Gisquet (6):
  dv: better split weight tables assignment
  dv: use smaller type for weight tables
  dv: more precise weight table for 8x8
  dv: fix weight table for 2x4x8 transform
  dv: increase reading bits to 10
  dv: move inverse weight tables to decoder

 libavcodec/dv.c |  34 
 libavcodec/dv.h |   3 +-
 libavcodec/dvdata.c |  65 ---
 libavcodec/dvdata.h |   7 ---
 libavcodec/dvdec.c  | 112 
 tests/ref/lavf/dv_fmt   |   6 +--
 tests/ref/vsynth/vsynth1-dv |   2 +-
 tests/ref/vsynth/vsynth1-dv-411 |   4 +-
 tests/ref/vsynth/vsynth1-dv-50  |   2 +-
 tests/ref/vsynth/vsynth2-dv |   4 +-
 tests/ref/vsynth/vsynth2-dv-411 |   2 +-
 tests/ref/vsynth/vsynth2-dv-50  |   2 +-
 12 files changed, 125 insertions(+), 118 deletions(-)

-- 
1.9.2.msysgit.0

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


[FFmpeg-devel] [PATCH 2/6] dv: use smaller type for weight tables

2014-10-25 Thread Christophe Gisquet
---
 libavcodec/dv.c |  4 ++--
 libavcodec/dvdata.c | 12 ++--
 libavcodec/dvdata.h | 12 ++--
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index e1e5dd9..1f04861 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -206,7 +206,7 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 factor1 = ctx-idct_factor[0];
 factor2 = ctx-idct_factor[DV_PROFILE_IS_HD(d) ? 4096 : 2816];
 if (DV_PROFILE_IS_HD(d)) {
-const int *iweight1, *iweight2;
+const uint16_t *iweight1, *iweight2;
 if (d-height == 720) {
 iweight1 = ff_dv_iweight_720_y[0];
 iweight2 = ff_dv_iweight_720_c[0];
@@ -223,7 +223,7 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 }
 }
 } else {
-const int *iweight1 = ff_dv_iweight_88[0];
+const uint16_t *iweight1 = ff_dv_iweight_88[0];
 for (j = 0; j  2; j++, iweight1 = ff_dv_iweight_248[0]) {
 for (s = 0; s  22; s++) {
 for (i = c = 0; c  4; c++) {
diff --git a/libavcodec/dvdata.c b/libavcodec/dvdata.c
index 85cba53..007976e 100644
--- a/libavcodec/dvdata.c
+++ b/libavcodec/dvdata.c
@@ -69,7 +69,7 @@ const uint8_t ff_dv_quant_shifts[22][4] = {
 
 const uint8_t ff_dv_quant_offset[4] = { 6, 3, 0, 1 };
 
-const int ff_dv_iweight_88[64] = {
+const uint16_t ff_dv_iweight_88[64] = {
 32768, 16710, 16710, 17735, 17015, 17735, 18197, 18079,
 18079, 18197, 18725, 18559, 19196, 18559, 18725, 19284,
 19108, 19692, 19692, 19108, 19284, 21400, 19645, 20262,
@@ -79,7 +79,7 @@ const int ff_dv_iweight_88[64] = {
 24600, 25267, 24457, 22672, 24457, 25267, 25971, 25191,
 25191, 25971, 26715, 27962, 26715, 29642, 29642, 31536,
 };
-const int ff_dv_iweight_248[64] = {
+const uint16_t ff_dv_iweight_248[64] = {
 32768, 17735, 16710, 18079, 18725, 21400, 17735, 19196,
 19108, 21845, 16384, 17735, 18725, 21400, 16710, 18079,
 20262, 23173, 18197, 19692, 18725, 20262, 20815, 23764,
@@ -93,7 +93,7 @@ const int ff_dv_iweight_248[64] = {
 /**
  * The inverse DV100 weights are actually just the spec weights (zig-zagged).
  */
-const int ff_dv_iweight_1080_y[64] = {
+const uint16_t ff_dv_iweight_1080_y[64] = {
 128,  16,  16,  17,  17,  17,  18,  18,
  18,  18,  18,  18,  19,  18,  18,  19,
  19,  19,  19,  19,  19,  42,  38,  40,
@@ -103,7 +103,7 @@ const int ff_dv_iweight_1080_y[64] = {
  48,  49,  48,  44,  48,  49, 101,  98,
  98, 101, 104, 109, 104, 116, 116, 123,
 };
-const int ff_dv_iweight_1080_c[64] = {
+const uint16_t ff_dv_iweight_1080_c[64] = {
 128,  16,  16,  17,  17,  17,  25,  25,
  25,  25,  26,  25,  26,  25,  26,  26,
  26,  27,  27,  26,  26,  42,  38,  40,
@@ -113,7 +113,7 @@ const int ff_dv_iweight_1080_c[64] = {
  96, 197, 191, 177, 191, 197, 203, 197,
 197, 203, 209, 219, 209, 232, 232, 246,
 };
-const int ff_dv_iweight_720_y[64] = {
+const uint16_t ff_dv_iweight_720_y[64] = {
 128,  16,  16,  17,  17,  17,  18,  18,
  18,  18,  18,  18,  19,  18,  18,  19,
  19,  19,  19,  19,  19,  42,  38,  40,
@@ -123,7 +123,7 @@ const int ff_dv_iweight_720_y[64] = {
  96,  98,  96,  88,  96,  98, 202, 196,
 196, 202, 208, 218, 208, 232, 232, 246,
 };
-const int ff_dv_iweight_720_c[64] = {
+const uint16_t ff_dv_iweight_720_c[64] = {
 128,  24,  24,  26,  26,  26,  36,  36,
  36,  36,  36,  36,  38,  36,  36,  38,
  38,  38,  38,  38,  38,  84,  76,  80,
diff --git a/libavcodec/dvdata.h b/libavcodec/dvdata.h
index 0932d3a..3c4da44 100644
--- a/libavcodec/dvdata.h
+++ b/libavcodec/dvdata.h
@@ -26,12 +26,12 @@ extern const uint8_t ff_dv_zigzag248_direct[64];
 extern const uint8_t ff_dv_quant_shifts[22][4];
 extern const uint8_t ff_dv_quant_offset[4];
 
-extern const int ff_dv_iweight_88[64];
-extern const int ff_dv_iweight_248[64];
-extern const int ff_dv_iweight_1080_y[64];
-extern const int ff_dv_iweight_1080_c[64];
-extern const int ff_dv_iweight_720_y[64];
-extern const int ff_dv_iweight_720_c[64];
+extern const uint16_t ff_dv_iweight_88[64];
+extern const uint16_t ff_dv_iweight_248[64];
+extern const uint16_t ff_dv_iweight_1080_y[64];
+extern const uint16_t ff_dv_iweight_1080_c[64];
+extern const uint16_t ff_dv_iweight_720_y[64];
+extern const uint16_t ff_dv_iweight_720_c[64];
 
 #define NB_DV_VLC 409
 
-- 
1.9.2.msysgit.0

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


[FFmpeg-devel] [PATCH 4/6] dv: fix weight table for 2x4x8 transform

2014-10-25 Thread Christophe Gisquet
The coefficients must be in the appropriate zigzag scan order.
Also fix their values at the same time, as they were pretty wrong.

Fixes ticket #2970.
---
 libavcodec/dvdata.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/dvdata.c b/libavcodec/dvdata.c
index d7bee71..5f0a239 100644
--- a/libavcodec/dvdata.c
+++ b/libavcodec/dvdata.c
@@ -80,14 +80,14 @@ const uint16_t ff_dv_iweight_88[64] = {
 25172, 25972, 26722, 27969, 26722, 29692, 29692, 31521,
 };
 const uint16_t ff_dv_iweight_248[64] = {
-32768, 17735, 16710, 18079, 18725, 21400, 17735, 19196,
-19108, 21845, 16384, 17735, 18725, 21400, 16710, 18079,
-20262, 23173, 18197, 19692, 18725, 20262, 20815, 23764,
-17735, 19196, 19108, 21845, 20262, 23173, 18197, 19692,
-21400, 24457, 19284, 20867, 21400, 23173, 22017, 25191,
-18725, 20262, 20815, 23764, 21400, 24457, 19284, 20867,
-24457, 27962, 22733, 24600, 25971, 29642, 21400, 23173,
-22017, 25191, 24457, 27962, 22733, 24600, 25971, 29642,
+32768, 16384, 16705, 16705, 17734, 17734, 17734, 17734,
+18081, 18081, 18725, 18725, 21407, 21407, 19091, 19091,
+19195, 19195, 18205, 18205, 18725, 18725, 19705, 19705,
+20267, 20267, 21826, 21826, 23170, 23170, 20806, 20806,
+20267, 20267, 19266, 19266, 21407, 21407, 20853, 20853,
+21400, 21400, 23786, 23786, 24465, 24465, 22018, 22018,
+23170, 23170, 22725, 22725, 24598, 24598, 24465, 24465,
+25172, 25172, 27969, 27969, 25972, 25972, 29692, 29692
 };
 
 /**
-- 
1.9.2.msysgit.0

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


[FFmpeg-devel] [PATCH 5/6] dv: increase reading bits to 10

2014-10-25 Thread Christophe Gisquet
From 356 to 348 cycles.
---
 libavcodec/dv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/dv.h b/libavcodec/dv.h
index 8a54cfe..3065806 100644
--- a/libavcodec/dv.h
+++ b/libavcodec/dv.h
@@ -90,7 +90,7 @@ enum dv_pack_type {
  */
 #define DV_MAX_BPM 8
 
-#define TEX_VLC_BITS 9
+#define TEX_VLC_BITS 10
 
 extern RL_VLC_ELEM ff_dv_rl_vlc[1184];
 
-- 
1.9.2.msysgit.0

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


[FFmpeg-devel] [PATCH 3/6] dv: more precise weight table for 8x8

2014-10-25 Thread Christophe Gisquet
It is derived from the actual equations of the specs. In
particular, it is closer to the inverse of what the encoder uses.

fate tests accordingly updated.
---
 libavcodec/dvdata.c | 16 
 tests/ref/lavf/dv_fmt   |  6 +++---
 tests/ref/vsynth/vsynth1-dv |  2 +-
 tests/ref/vsynth/vsynth1-dv-411 |  4 ++--
 tests/ref/vsynth/vsynth1-dv-50  |  2 +-
 tests/ref/vsynth/vsynth2-dv |  4 ++--
 tests/ref/vsynth/vsynth2-dv-411 |  2 +-
 tests/ref/vsynth/vsynth2-dv-50  |  2 +-
 8 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/libavcodec/dvdata.c b/libavcodec/dvdata.c
index 007976e..d7bee71 100644
--- a/libavcodec/dvdata.c
+++ b/libavcodec/dvdata.c
@@ -70,14 +70,14 @@ const uint8_t ff_dv_quant_shifts[22][4] = {
 const uint8_t ff_dv_quant_offset[4] = { 6, 3, 0, 1 };
 
 const uint16_t ff_dv_iweight_88[64] = {
-32768, 16710, 16710, 17735, 17015, 17735, 18197, 18079,
-18079, 18197, 18725, 18559, 19196, 18559, 18725, 19284,
-19108, 19692, 19692, 19108, 19284, 21400, 19645, 20262,
-20214, 20262, 19645, 21400, 22733, 21845, 20867, 20815,
-20815, 20867, 21845, 22733, 23173, 23173, 21400, 21400,
-21400, 23173, 23173, 24600, 23764, 22017, 22017, 23764,
-24600, 25267, 24457, 22672, 24457, 25267, 25971, 25191,
-25191, 25971, 26715, 27962, 26715, 29642, 29642, 31536,
+32768, 16705, 16705, 17734, 17032, 17734, 18205, 18081,
+18081, 18205, 18725, 18562, 19195, 18562, 18725, 19266,
+19091, 19705, 19705, 19091, 19266, 21407, 19643, 20267,
+20228, 20267, 19643, 21407, 22725, 21826, 20853, 20806,
+20806, 20853, 21826, 22725, 23170, 23170, 21407, 21400,
+21407, 23170, 23170, 24598, 23786, 22018, 22018, 23786,
+24598, 25251, 24465, 22654, 24465, 25251, 25972, 25172,
+25172, 25972, 26722, 27969, 26722, 29692, 29692, 31521,
 };
 const uint16_t ff_dv_iweight_248[64] = {
 32768, 17735, 16710, 18079, 18725, 21400, 17735, 19196,
diff --git a/tests/ref/lavf/dv_fmt b/tests/ref/lavf/dv_fmt
index 3c8e5b1..b152c84 100644
--- a/tests/ref/lavf/dv_fmt
+++ b/tests/ref/lavf/dv_fmt
@@ -1,9 +1,9 @@
 11be3e5caa2892236b3475c3f7807b76 *./tests/data/lavf/lavf.dv
 360 ./tests/data/lavf/lavf.dv
-./tests/data/lavf/lavf.dv CRC=0x25bdd732
+./tests/data/lavf/lavf.dv CRC=0x0b2cd3ec
 e9949bc767924e1e7d28856029fee024 *./tests/data/lavf/lavf.dv
 348 ./tests/data/lavf/lavf.dv
-./tests/data/lavf/lavf.dv CRC=0xc4f27ca7
+./tests/data/lavf/lavf.dv CRC=0xfab17c4a
 87d3b20f656235671383a7eaa2f66330 *./tests/data/lavf/lavf.dv
 360 ./tests/data/lavf/lavf.dv
-./tests/data/lavf/lavf.dv CRC=0x0e868a82
+./tests/data/lavf/lavf.dv CRC=0xf3e6873c
diff --git a/tests/ref/vsynth/vsynth1-dv b/tests/ref/vsynth/vsynth1-dv
index d051e8d..6237b07 100644
--- a/tests/ref/vsynth/vsynth1-dv
+++ b/tests/ref/vsynth/vsynth1-dv
@@ -1,4 +1,4 @@
 4d572f758b55a1756adf9f54132f3b9e *tests/data/fate/vsynth1-dv.dv
 720 tests/data/fate/vsynth1-dv.dv
-02ac7cdeab91d4d5621e7ce96dddc498 *tests/data/fate/vsynth1-dv.out.rawvideo
+1cda5a62c3a2f17cc7d5b4cddccf2524 *tests/data/fate/vsynth1-dv.out.rawvideo
 stddev:6.90 PSNR: 31.34 MAXDIFF:   76 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth1-dv-411 b/tests/ref/vsynth/vsynth1-dv-411
index bc4b802..48e01a1 100644
--- a/tests/ref/vsynth/vsynth1-dv-411
+++ b/tests/ref/vsynth/vsynth1-dv-411
@@ -1,4 +1,4 @@
 f179899efba432c6f01149c36c709092 *tests/data/fate/vsynth1-dv-411.dv
 720 tests/data/fate/vsynth1-dv-411.dv
-53946d51762b7826773e681fb02f377b *tests/data/fate/vsynth1-dv-411.out.rawvideo
-stddev:9.45 PSNR: 28.62 MAXDIFF:   84 bytes:  7603200/  7603200
+48904744fabbbc3421a762f615ef6456 *tests/data/fate/vsynth1-dv-411.out.rawvideo
+stddev:9.44 PSNR: 28.62 MAXDIFF:   84 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth1-dv-50 b/tests/ref/vsynth/vsynth1-dv-50
index e747075..d5da88d 100644
--- a/tests/ref/vsynth/vsynth1-dv-50
+++ b/tests/ref/vsynth/vsynth1-dv-50
@@ -1,4 +1,4 @@
 a193c5f92bf6e74c604e759d5f4f0f94 *tests/data/fate/vsynth1-dv-50.dv
 1440 tests/data/fate/vsynth1-dv-50.dv
-a2ff093e93ffed10f730fa21df02fc50 *tests/data/fate/vsynth1-dv-50.out.rawvideo
+41c4df5f2d876fcd5245643b9ded6711 *tests/data/fate/vsynth1-dv-50.out.rawvideo
 stddev:1.72 PSNR: 43.38 MAXDIFF:   29 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-dv b/tests/ref/vsynth/vsynth2-dv
index 0d1465c..7bcc5ad 100644
--- a/tests/ref/vsynth/vsynth2-dv
+++ b/tests/ref/vsynth/vsynth2-dv
@@ -1,4 +1,4 @@
 85b8d55b0b68bb3fc2e90babb580f9b7 *tests/data/fate/vsynth2-dv.dv
 720 tests/data/fate/vsynth2-dv.dv
-7ec62bd3350a6848364669e6e1e4b9cc *tests/data/fate/vsynth2-dv.out.rawvideo
-stddev:1.71 PSNR: 43.47 MAXDIFF:   33 bytes:  7603200/  7603200
+7dac420637360b031ccae7c5a69c5e0c *tests/data/fate/vsynth2-dv.out.rawvideo
+stddev:1.70 PSNR: 43.47 MAXDIFF:   33 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-dv-411 b/tests/ref/vsynth/vsynth2-dv-411
index d0e6d29..541673a 100644
--- 

[FFmpeg-devel] [PATCH 6/6] dv: move inverse weight tables to decoder

2014-10-25 Thread Christophe Gisquet
The encoder has its own tables and does not access the idct_factor
member of the DVVideoContext structure.
---
 libavcodec/dv.c |  34 
 libavcodec/dv.h |   1 +
 libavcodec/dvdata.c |  65 --
 libavcodec/dvdata.h |   7 
 libavcodec/dvdec.c  | 112 
 5 files changed, 113 insertions(+), 106 deletions(-)

diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index 1f04861..6cd8a89 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -185,7 +185,6 @@ static const uint8_t dv_quant_areas[4] = { 6, 21, 43, 64 };
 int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const AVDVProfile *d)
 {
 int j, i, c, s, p;
-uint32_t *factor1, *factor2;
 
 p = i = 0;
 for (c = 0; c  d-n_difchan; c++) {
@@ -203,39 +202,6 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const 
AVDVProfile *d)
 }
 }
 
-factor1 = ctx-idct_factor[0];
-factor2 = ctx-idct_factor[DV_PROFILE_IS_HD(d) ? 4096 : 2816];
-if (DV_PROFILE_IS_HD(d)) {
-const uint16_t *iweight1, *iweight2;
-if (d-height == 720) {
-iweight1 = ff_dv_iweight_720_y[0];
-iweight2 = ff_dv_iweight_720_c[0];
-} else {
-iweight1 = ff_dv_iweight_1080_y[0];
-iweight2 = ff_dv_iweight_1080_c[0];
-}
-for (c = 0; c  4; c++) {
-for (s = 0; s  16; s++) {
-for (i = 0; i  64; i++) {
-*factor1++ = (dv100_qstep[s]  (c + 9)) * iweight1[i];
-*factor2++ = (dv100_qstep[s]  (c + 9)) * iweight2[i];
-}
-}
-}
-} else {
-const uint16_t *iweight1 = ff_dv_iweight_88[0];
-for (j = 0; j  2; j++, iweight1 = ff_dv_iweight_248[0]) {
-for (s = 0; s  22; s++) {
-for (i = c = 0; c  4; c++) {
-for (; i  dv_quant_areas[c]; i++) {
-*factor1   = iweight1[i]  (ff_dv_quant_shifts[s][c] 
+ 1);
-*factor2++ = (*factor1++)  1;
-}
-}
-}
-}
-}
-
 return 0;
 }
 
diff --git a/libavcodec/dv.h b/libavcodec/dv.h
index 3065806..02950b7 100644
--- a/libavcodec/dv.h
+++ b/libavcodec/dv.h
@@ -95,6 +95,7 @@ enum dv_pack_type {
 extern RL_VLC_ELEM ff_dv_rl_vlc[1184];
 
 int ff_dv_init_dynamic_tables(DVVideoContext *s, const AVDVProfile *d);
+
 int ff_dvvideo_init(AVCodecContext *avctx);
 
 static inline int dv_work_pool_size(const AVDVProfile *d)
diff --git a/libavcodec/dvdata.c b/libavcodec/dvdata.c
index 5f0a239..231569a 100644
--- a/libavcodec/dvdata.c
+++ b/libavcodec/dvdata.c
@@ -69,71 +69,6 @@ const uint8_t ff_dv_quant_shifts[22][4] = {
 
 const uint8_t ff_dv_quant_offset[4] = { 6, 3, 0, 1 };
 
-const uint16_t ff_dv_iweight_88[64] = {
-32768, 16705, 16705, 17734, 17032, 17734, 18205, 18081,
-18081, 18205, 18725, 18562, 19195, 18562, 18725, 19266,
-19091, 19705, 19705, 19091, 19266, 21407, 19643, 20267,
-20228, 20267, 19643, 21407, 22725, 21826, 20853, 20806,
-20806, 20853, 21826, 22725, 23170, 23170, 21407, 21400,
-21407, 23170, 23170, 24598, 23786, 22018, 22018, 23786,
-24598, 25251, 24465, 22654, 24465, 25251, 25972, 25172,
-25172, 25972, 26722, 27969, 26722, 29692, 29692, 31521,
-};
-const uint16_t ff_dv_iweight_248[64] = {
-32768, 16384, 16705, 16705, 17734, 17734, 17734, 17734,
-18081, 18081, 18725, 18725, 21407, 21407, 19091, 19091,
-19195, 19195, 18205, 18205, 18725, 18725, 19705, 19705,
-20267, 20267, 21826, 21826, 23170, 23170, 20806, 20806,
-20267, 20267, 19266, 19266, 21407, 21407, 20853, 20853,
-21400, 21400, 23786, 23786, 24465, 24465, 22018, 22018,
-23170, 23170, 22725, 22725, 24598, 24598, 24465, 24465,
-25172, 25172, 27969, 27969, 25972, 25972, 29692, 29692
-};
-
-/**
- * The inverse DV100 weights are actually just the spec weights (zig-zagged).
- */
-const uint16_t ff_dv_iweight_1080_y[64] = {
-128,  16,  16,  17,  17,  17,  18,  18,
- 18,  18,  18,  18,  19,  18,  18,  19,
- 19,  19,  19,  19,  19,  42,  38,  40,
- 40,  40,  38,  42,  44,  43,  41,  41,
- 41,  41,  43,  44,  45,  45,  42,  42,
- 42,  45,  45,  48,  46,  43,  43,  46,
- 48,  49,  48,  44,  48,  49, 101,  98,
- 98, 101, 104, 109, 104, 116, 116, 123,
-};
-const uint16_t ff_dv_iweight_1080_c[64] = {
-128,  16,  16,  17,  17,  17,  25,  25,
- 25,  25,  26,  25,  26,  25,  26,  26,
- 26,  27,  27,  26,  26,  42,  38,  40,
- 40,  40,  38,  42,  44,  43,  41,  41,
- 41,  41,  43,  44,  91,  91,  84,  84,
- 84,  91,  91,  96,  93,  86,  86,  93,
- 96, 197, 191, 177, 191, 197, 203, 197,
-197, 203, 209, 219, 209, 232, 232, 246,
-};
-const uint16_t ff_dv_iweight_720_y[64] = {
-128,  16,  16,  17,  17,  17,  18,  18,
- 18,  18,  18,  18,  19,  18,  18,  19,
- 19,  19,  19,  19,  19,  42,  38,  40,
- 40,  40,  38,  

Re: [FFmpeg-devel] [PATCH 0/6] dv: of inverse weight tables

2014-10-25 Thread Christophe Gisquet
2014-10-25 13:19 GMT+02:00 Christophe Gisquet christophe.gisq...@gmail.com:
 Patch #3 uses mathematically more correct values, which happens to match
 another dv implementation (cedocida). It does not seem to affect that
 much the decoded output, as there is PSNR variations on the 3rd decimal
 at most. The best I got was, using a resized version of lena.ppm, a PSNR
 going from 49.35dB to 49.36dB. It would be ok not to integrate it, as
 one may argue they were devised with the simple_idct implementation in
 mind (I haven't checked).

 Patch #4 fixes the actual issue. As I don't want to bother determining
 how the table was incorrectly mangled, I've preferred regenerating it.

And here's a program to regenerate it (hopefully I haven't mangled too
much when testing other stuff).

-- 
Christophe
#include stdio.h
#include stdint.h
#include math.h

#define CS(n) cos(n*M_PI/16.0)

#define SCAN_8x8  1
#define USE_ZZ_SCAN   1

int main(void)
{
//double w[8] = { 1.0, CS(4)/(4*CS(7)*CS(2)), CS(4)/(2*CS(6)), 1.0/(2*CS(5)), 7.0/8, CS(4)/CS(3), CS(4)/CS(2), CS(4)/CS(1) };
double w[8] = { 1., 0.98078528, 0.92387953, 0.89997622, 0.8750, 0.85043009, 0.76536686, 0.72095982 };
int i, j, order;
# if USE_ZZ_SCAN
const uint8_t dv_zz[64] = {
# if SCAN_8x8
 0,  1,  8, 16,  9,  2,  3, 10,
17, 24, 32, 25, 18, 11,  4,  5,
12, 19, 26, 33, 40, 48, 41, 34,
27, 20, 13,  6,  7, 14, 21, 28,
35, 42, 49, 56, 57, 50, 43, 36,
29, 22, 15, 23, 30, 37, 44, 51,
58, 59, 52, 45, 38, 31, 39, 46,
53, 60, 61, 54, 47, 55, 62, 63
# else
 0,  8,  1,  9, 16, 24,  2, 10,
17, 25, 32, 40, 48, 56, 33, 41,
18, 26,  3, 11,  4, 12, 19, 27,
34, 42, 49, 57, 50, 58, 35, 43,
20, 28,  5, 13,  6, 14, 21, 29,
36, 44, 51, 59, 52, 60, 37, 45,
22, 30,  7, 15, 23, 31, 38, 46,
53, 61, 54, 62, 39, 47, 55, 63,
# endif
};
#endif

#if 0
const uint16_t ff_dv_iweight_88[64] = {
32768, 16705, 16705, 17734, 17032, 17734, 18205, 18081,
18081, 18205, 18725, 18562, 19195, 18562, 18725, 19266,
19091, 19705, 19705, 19091, 19266, 21407, 19643, 20267,
20228, 20267, 19643, 21407, 22725, 21826, 20853, 20806,
20806, 20853, 21826, 22725, 23170, 23170, 21407, 21400,
21407, 23170, 23170, 24598, 23786, 22018, 22018, 23786,
24598, 25251, 24465, 22654, 24465, 25251, 25972, 25172,
25172, 25972, 26722, 27969, 26722, 29692, 29692, 31521,
};
static const int dv_weight_88[64] = {
131072, 257107, 257107, 242189, 252167, 242189, 235923, 237536,
237536, 235923, 229376, 231390, 223754, 231390, 229376, 222935,
224969, 217965, 217965, 224969, 222935, 200636, 218652, 211916,
212325, 211916, 218652, 200636, 188995, 196781, 205965, 206433,
206433, 205965, 196781, 188995, 185364, 185364, 200636, 200704,
200636, 185364, 185364, 174609, 180568, 195068, 195068, 180568,
174609, 170091, 175557, 189591, 175557, 170091, 165371, 170627,
170627, 165371, 160727, 153560, 160727, 144651, 144651, 136258,
};
double sum = 0;
double sqsum = 0;
for(i=0; i64; i++)
{
double val = ff_dv_iweight_88[i]*dv_weight_88[i]/(double)(0xU);
sum += val;
sqsum += val*val;
}
printf(std dev: %e\n, sum, sqsum, sqrt(sqsum-sum*sum/64.0));
#else
//for(j=0; j8; j++) { printf( %1.8f, w[j]); } printf(\n);

for(order = 0, j=0; j8; j++)
{
for(i=0; i8; i++, order++)
{
# if USE_ZZ_SCAN
int zz = dv_zz[order];
int v = zz/8, h = zz%8;
# else
int v = j, h = i;
# endif
double coeff;

#if SCAN_8x8 // 8x8
if(!v  !h) coeff=1.0/4.0;
else
{
coeff = w[v];
#else // 2x4x8
if(!v  !h) coeff=1.0/2.0;
else
{
if(v4) coeff=w[2*v];
elsecoeff=w[2*v-8];
#endif
coeff *= w[h]/2.0;
}
printf( %5d,, (int)(8192/coeff+0.5));
}
printf(\n);
}
#endif

return 0;
}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/6] dv: use smaller type for weight tables

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 11:19:21AM +, Christophe Gisquet wrote:
 ---
  libavcodec/dv.c |  4 ++--
  libavcodec/dvdata.c | 12 ++--
  libavcodec/dvdata.h | 12 ++--
  3 files changed, 14 insertions(+), 14 deletions(-)

Looks good to me.
I think in all places the uint16_t will be promoted to int
automatically.
But it might be good for someone else to double-check that this
signed-unsigned change doesn't break anything, I consider C
behaviour a bit too tricky...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/6] dv: fix weight table for 2x4x8 transform

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 11:19:23AM +, Christophe Gisquet wrote:
 The coefficients must be in the appropriate zigzag scan order.
 Also fix their values at the same time, as they were pretty wrong.
 
 Fixes ticket #2970.

Could you maybe add e.g. a FATE test that clearly shows the before-after
improvements?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread wm4
On Sat, 25 Oct 2014 13:14:26 +0200
Reimar Döffinger reimar.doeffin...@gmx.de wrote:

 On Sat, Oct 25, 2014 at 01:10:20PM +0200, wm4 wrote:
  On Sat, 25 Oct 2014 11:43:12 +0200
  Nicolas George geo...@nsup.org wrote:
  
   Le tridi 3 brumaire, an CCXXIII, Timothy Gu a écrit :
Those URLs are very explicit, while `/dev/` can mean anything,
including `/dev/stdin`.
   
   Andrey quoted these bits of code to show that checking filename for NULL 
   is
   not necessary. I believe his proof is conclusive for that point.
   
   wm4 :
IMHO it should check whether it's really a video device.
   
   Yes, it should. Do you intend to propose the patch that invokes stat() on
   all supported operating systems, check that can not devolve into a 
   security
   issue in the context of probing, find the relevant list of major/minor
   pairs and a way of testing the dynamically allocated ones soon, or are you
   just bikeshedding.
  
  Don't present your extremely bad ideas as mine. Thanks.
 
 Could you both please either bury or hide your hatchet?
 It is really annoying to all other when you write such obviously
 aggressive and non-constructive emails.

I agree.

 wm4, if you have a simple way of doing this check, please explain it.
 Otherwise I think EXTENSION score is good enough to signal well, we
 guess that's what it is.

There are two solutions:
- actually open the video device, and try a v4l-only ioctl() to test
  whether it's really a device (the artificial separation between
  avio/probing/actual opening becomes rather annoying here)
- add a specific protocol prefix that forces the input format, and which
  lets the user invoke video capture (think MPlayer's tv://)

All in all, there's much you can do about raw devices. A video device
probably doesn't even need to be named /dev/video..., nor does that
filename need to be a reliable indicator that the device understand
V4L. Whatever you do, it can't be perfect, because you're working on a
too low level and system specific layer. tv:// could solve that
actually rather elegantly, including abstracting platform differences
from the user.


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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 01:40:11PM +0200, wm4 wrote:
 On Sat, 25 Oct 2014 13:14:26 +0200
 Reimar Döffinger reimar.doeffin...@gmx.de wrote:
  wm4, if you have a simple way of doing this check, please explain it.
  Otherwise I think EXTENSION score is good enough to signal well, we
  guess that's what it is.
 
 There are two solutions:
 - actually open the video device, and try a v4l-only ioctl() to test
   whether it's really a device (the artificial separation between
   avio/probing/actual opening becomes rather annoying here)

That would end up trying a v4l ioctl on each and every file that
FFmpeg tries to open (unless you mean only after the /dev/video check,
but that would not solve the issue you mention later).
That feels a bit heavy-weight.
I guess it's not unreasonable if done in addition, but on the other
hand, is this likely to ever increase user experience in reality?

 - add a specific protocol prefix that forces the input format, and which
   lets the user invoke video capture (think MPlayer's tv://)
 
 All in all, there's much you can do about raw devices. A video device
 probably doesn't even need to be named /dev/video..., nor does that
 filename need to be a reliable indicator that the device understand
 V4L. Whatever you do, it can't be perfect, because you're working on a
 too low level and system specific layer. tv:// could solve that
 actually rather elegantly, including abstracting platform differences
 from the user.

I admittedly just assumed that v4l2:///dev/video/... would work
currently. If not, that sounds like something that can and should
be fixed.
However as a convenience feature, I think it is good that /dev/video
just ends up picking the v4l2 input, because in almost all cases
that would be the desired behaviour.
It should be possible to override the other way via file:///dev/video/... I 
believe?
If I am wrong about any of that, that would probably change my opinion.

And I probably should leave this discussion to someone with a clue about
this part of the code.

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


[FFmpeg-devel] [RFC]Do not set the lame quality if the user didn't set it

2014-10-25 Thread Carl Eugen Hoyos
Hi!

Attached patch makes FFmpeg mp3 output more similar to lame's.
It appears to me that 5 is not the default if vbr is used.

Please comment, Carl Eugen
diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
index 661d1c0..d8a444d 100644
--- a/libavcodec/libmp3lame.c
+++ b/libavcodec/libmp3lame.c
@@ -106,9 +106,7 @@ static av_cold int mp3lame_encode_init(AVCodecContext 
*avctx)
 lame_set_out_samplerate(s-gfp, avctx-sample_rate);
 
 /* algorithmic quality */
-if (avctx-compression_level == FF_COMPRESSION_DEFAULT)
-lame_set_quality(s-gfp, 5);
-else
+if (avctx-compression_level != FF_COMPRESSION_DEFAULT)
 lame_set_quality(s-gfp, avctx-compression_level);
 
 /* rate control */
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread wm4
On Sat, 25 Oct 2014 13:50:58 +0200
Reimar Döffinger reimar.doeffin...@gmx.de wrote:

 On Sat, Oct 25, 2014 at 01:40:11PM +0200, wm4 wrote:
  On Sat, 25 Oct 2014 13:14:26 +0200
  Reimar Döffinger reimar.doeffin...@gmx.de wrote:
   wm4, if you have a simple way of doing this check, please explain it.
   Otherwise I think EXTENSION score is good enough to signal well, we
   guess that's what it is.
  
  There are two solutions:
  - actually open the video device, and try a v4l-only ioctl() to test
whether it's really a device (the artificial separation between
avio/probing/actual opening becomes rather annoying here)
 
 That would end up trying a v4l ioctl on each and every file that
 FFmpeg tries to open (unless you mean only after the /dev/video check,
 but that would not solve the issue you mention later).
 That feels a bit heavy-weight.
 I guess it's not unreasonable if done in addition, but on the other
 hand, is this likely to ever increase user experience in reality?

Well, yes, this is a weakness of the current probing architecture. Code
written from scratch could just always open() local filenames, and then
test whether it's actually a video device or a normal file. (One thing
on the side: there's this special utility lib, that requires you to
call v4l2_open() instead of open(), but you can also just use open()
and then v4l2_open_fd() in the v4l-specific code.)

  - add a specific protocol prefix that forces the input format, and which
lets the user invoke video capture (think MPlayer's tv://)
  
  All in all, there's much you can do about raw devices. A video device
  probably doesn't even need to be named /dev/video..., nor does that
  filename need to be a reliable indicator that the device understand
  V4L. Whatever you do, it can't be perfect, because you're working on a
  too low level and system specific layer. tv:// could solve that
  actually rather elegantly, including abstracting platform differences
  from the user.
 
 I admittedly just assumed that v4l2:///dev/video/... would work
 currently. If not, that sounds like something that can and should
 be fixed.
 However as a convenience feature, I think it is good that /dev/video
 just ends up picking the v4l2 input, because in almost all cases
 that would be the desired behaviour.

As a user, I wouldn't really expect that this works. Rather, I'd assume
I'd have to do something special to get the program to use video.
With current ffmpeg, you have to specify _both_ the special
thing (the demuxer) _and_ the video device, which is a bit of a WTF.
As I said, using a special protocol (like tv://) would be a nice
alternative, which can also include automatically selecting the video
device (for the right platform!), but maybe I'm spoiled by the MPlayer
way of doing things.

 It should be possible to override the other way via file:///dev/video/... I 
 believe?
 If I am wrong about any of that, that would probably change my opinion.
 
 And I probably should leave this discussion to someone with a clue about
 this part of the code.
 
 Regards,
 Reimar

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


Re: [FFmpeg-devel] [PATCH]Mention in the documentation that fieldmatch needs cfr input

2014-10-25 Thread Carl Eugen Hoyos
Clément Bœsch u at pkh.me writes:

  +The filter only works for constant frame rate input. If your input
  +has mixed telecined and progressive content with changing framerate,
  +try the  at ref{pullup} filter.
 
 Well I don't mind much but then... how is pullup making 
 any difference here actually?

You mean why does it work with pullup but not fieldmatch? 
I honestly cannot answer, sorry...

 fieldmatch isn't actually touching the pts nor using them.

I wonder if that (together with using decimate) isn't the 
problem.
But this is of course completely unrelated.

Just to sum it up:
pullup works fine and is fast for all samples I have seen, 
it definitely misses many merging opportunities if the 
horizontal motion is very low (meaning some frames with 
artefacts remain for every real-world input).
fieldmatch is very slow, it is apparently able to 
produce perfect output for badly cut input with constant 
(telecined) framerate but it fails completely for mixed 
content (as found on many DVD's).

Imo, fps=3/1001,fieldmatch,decimate should fix this 
but decimate unfortunately does not drop the frame that 
fps inserted but a random (?) frame.
(Or fieldmatch finds matches in progressive input?)

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Carl Eugen Hoyos
Reimar Döffinger Reimar.Doeffinger at gmx.de writes:

 It should be possible to override the other way via 
 file:///dev/video/... I believe?

Of course.

Could the original patch please be applied?
If really necessary with MAX - 1, we have 
32bit probe functions that return MAX and 
no user will ever want to read a file from 
/dev/videofile.avi

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, Reimar Döffinger a écrit :
 That would end up trying a v4l ioctl on each and every file that
 FFmpeg tries to open (unless you mean only after the /dev/video check,
 but that would not solve the issue you mention later).
 That feels a bit heavy-weight.

That would not be heavy-weight, that would be outright irresponsible:
ioctl() opcodes are not globally unique. An opcode that maps to a harmless
probe on v4l devices can trigger catastrophic consequences on other devices.

(Remember 2003, when installing Mandrake 9.2 would brick LG CD drives? That
was the same issue, but our case is worse because what Mandrake did was
actually supposed to work reliably if LG had not messed their firmware.)

(If your conclusion is that the ioctl() interface is braindead, I am right
there with you, but that is something we have to live with.)

 I admittedly just assumed that v4l2:///dev/video/... would work
 currently. If not, that sounds like something that can and should
 be fixed.

Well, foo: prefixes are normally reserved for protocols. We could add to
that or formats with the NOFILE flag, that would probably make things
better. Or if we are afraid to trigger untested corner cases, have a
dedicated new flag AVFMT_PROTO_PREFIX or something.

But if we start going in that direction, why stop there? Why should
v4l2:/dev/video0 work and not rawvideo:/dev/zero? And if that last one
works, how do I specify the frame dimensions?

That brings back the discussion on whether to accept options from the file
name. You objected based on security considerations, but I still think that
a dedicated function doing clean and reliable parsing would be useful for a
lot of cases.

At the very least, the limit on what options to accept from the file name
should be decided by a proper discussion based on security and usability
considerations, not by what patches are pushed before the usual bikeshedders
(I'm #1) got up in the morning.

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] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 02:15:47PM +0200, wm4 wrote:
  I admittedly just assumed that v4l2:///dev/video/... would work
  currently. If not, that sounds like something that can and should
  be fixed.
  However as a convenience feature, I think it is good that /dev/video
  just ends up picking the v4l2 input, because in almost all cases
  that would be the desired behaviour.
 
 As a user, I wouldn't really expect that this works. Rather, I'd assume
 I'd have to do something special to get the program to use video.
 With current ffmpeg, you have to specify _both_ the special
 thing (the demuxer) _and_ the video device, which is a bit of a WTF.

Hm. Haven't looked, but there should be mechanisms for the device to
specify a demuxer to use.
Unfortunately I don't have the right hardware to test any of this.

 As I said, using a special protocol (like tv://) would be a nice
 alternative, which can also include automatically selecting the video
 device (for the right platform!), but maybe I'm spoiled by the MPlayer
 way of doing things.

I misunderstood what you meant by that earlier.
Such a special protocol that doesn't need /dev/video magic would probably be
a good idea.
But it's kind of unrelated to this (if you want a special video device,
just being able to specify it is IMHO nice).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 12:35:11PM +, Carl Eugen Hoyos wrote:
 Reimar Döffinger Reimar.Doeffinger at gmx.de writes:
 
  It should be possible to override the other way via 
  file:///dev/video/... I believe?
 
 Of course.
 
 Could the original patch please be applied?

I think I agree, however...

 If really necessary with MAX - 1, we have 
 32bit probe functions that return MAX and 
 no user will ever want to read a file from 
 /dev/videofile.avi

I think this is a bit Linux/Unix/centric.
On Windows, BeOS etc. you can use / as path separator and
you can easily have c:/dev as path for development stuff.
ffmpeg /dev/video/test.avi probably has non-0 probability.
Though I'd guess nobody manages to compile the v4l2 thing
on anything but Linux anyway, so it should be an irrelevant point
(I hope).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread wm4
On Sat, 25 Oct 2014 14:43:39 +0200
Nicolas George geo...@nsup.org wrote:

 Le quartidi 4 brumaire, an CCXXIII, Reimar Döffinger a écrit :
  That would end up trying a v4l ioctl on each and every file that
  FFmpeg tries to open (unless you mean only after the /dev/video check,
  but that would not solve the issue you mention later).
  That feels a bit heavy-weight.
 
 That would not be heavy-weight, that would be outright irresponsible:
 ioctl() opcodes are not globally unique. An opcode that maps to a harmless
 probe on v4l devices can trigger catastrophic consequences on other devices.
 
 (Remember 2003, when installing Mandrake 9.2 would brick LG CD drives? That
 was the same issue, but our case is worse because what Mandrake did was
 actually supposed to work reliably if LG had not messed their firmware.)
 
 (If your conclusion is that the ioctl() interface is braindead, I am right
 there with you, but that is something we have to live with.)

Then there's no sane way to handle this. Basically, you need to know:
yes, this is a video device.

  I admittedly just assumed that v4l2:///dev/video/... would work
  currently. If not, that sounds like something that can and should
  be fixed.
 
 Well, foo: prefixes are normally reserved for protocols. We could add to
 that or formats with the NOFILE flag, that would probably make things
 better. Or if we are afraid to trigger untested corner cases, have a
 dedicated new flag AVFMT_PROTO_PREFIX or something.

Such a flag might probably work...

 But if we start going in that direction, why stop there? Why should
 v4l2:/dev/video0 work and not rawvideo:/dev/zero? And if that last one
 works, how do I specify the frame dimensions?
 
 That brings back the discussion on whether to accept options from the file
 name. You objected based on security considerations, but I still think that
 a dedicated function doing clean and reliable parsing would be useful for a
 lot of cases.

The lavfi libavdevice demuxer already accepts arbitrary options from
URLs. All in all, I'd say ffmpeg is not optimized for security issues.
That libavdevice things require specifying the format explicitly
actually helps a little but. But in general, a flag that indicates that
an URL is e.g. from a playlist downloaded over http might be more
helpful for security.

Why can't the frame size be detected automatically?

 At the very least, the limit on what options to accept from the file name
 should be decided by a proper discussion based on security and usability
 considerations, not by what patches are pushed before the usual bikeshedders
 (I'm #1) got up in the morning.
 
 Regards,
 

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, wm4 a écrit :
 Then there's no sane way to handle this. Basically, you need to know:
 yes, this is a video device.

And that does not exist.

 Such a flag might probably work...

Actually, probably not without more changes, because the devices do not
remove the protocol prefix from the file name.

 The lavfi libavdevice demuxer already accepts arbitrary options from
 URLs.

Yes, but the lavfi device can not be selected automatically from the URL in
the first place.

 All in all, I'd say ffmpeg is not optimized for security issues.

That is your opinion.

 Why can't the frame size be detected automatically?

... Thinking about what rawvideo means would probably have taken less time
than typing the question.

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] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, Reimar Döffinger a écrit :
 Such a special protocol that doesn't need /dev/video magic would probably be
 a good idea.

We could agree on a global convention that states something like this:

Whenever it makes sense, demuxers that use the file name directly should
treat an empty string or a single dash as equivalent to the default device
name.

(In other words: ALSA should treat  and - as default, v4l2 should
treat them as /dev/video0; x11grab already accepts  thanks to the magic in
Xlib, but not -.)

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] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Reimar Döffinger
On Sat, Oct 25, 2014 at 03:08:10PM +0200, wm4 wrote:
 On Sat, 25 Oct 2014 14:43:39 +0200
 Nicolas George geo...@nsup.org wrote:
  That would not be heavy-weight, that would be outright irresponsible:
  ioctl() opcodes are not globally unique. An opcode that maps to a harmless
  probe on v4l devices can trigger catastrophic consequences on other devices.
  
  (Remember 2003, when installing Mandrake 9.2 would brick LG CD drives? That
  was the same issue, but our case is worse because what Mandrake did was
  actually supposed to work reliably if LG had not messed their firmware.)
  
  (If your conclusion is that the ioctl() interface is braindead, I am right
  there with you, but that is something we have to live with.)
 
 Then there's no sane way to handle this. Basically, you need to know:
 yes, this is a video device.

I think this might be the whole reason for the disagreement.
Me and Nicolas don't think there is any reason at all for you to
_know_ it is a video device.
That's why we have probe scores.
We just make the best guess we find reasonable and then assign a score
corresponding to the confidence we have in that.
So I actually don't see much of an issue with the patch except the
question which confidence we should assign to a test that just
checks for /dev/video.
Anything I miss?

   I admittedly just assumed that v4l2:///dev/video/... would work
   currently. If not, that sounds like something that can and should
   be fixed.
  
  Well, foo: prefixes are normally reserved for protocols. We could add to
  that or formats with the NOFILE flag, that would probably make things
  better. Or if we are afraid to trigger untested corner cases, have a
  dedicated new flag AVFMT_PROTO_PREFIX or something.
 
 Such a flag might probably work...

I think this is too complex and not directly related to this.
Maybe we should move it to a different thread if someone wants to work
on it?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Carl Eugen Hoyos
Reimar Döffinger Reimar.Doeffinger at gmx.de writes:

 I think this is a bit Linux/Unix/centric.

Of course, as it is quite difficult to compile 
the demuxer on anything else...

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with /dev/video

2014-10-25 Thread Michael Niedermayer
On Fri, Oct 24, 2014 at 07:50:25PM +0400, Andrey Utkin wrote:
 ---
  libavdevice/v4l2.c | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
 index cf7a92c..ebc50bb 100644
 --- a/libavdevice/v4l2.c
 +++ b/libavdevice/v4l2.c
 @@ -806,6 +806,13 @@ static int device_try_init(AVFormatContext *ctx,
  return ret;
  }
  
 +static int v4l2_read_probe(AVProbeData *p)
 +{
 +if (av_strstart(p-filename, /dev/video, NULL))
 +return AVPROBE_SCORE_MAX;
 +return 0;

can  strcmp || sscanf(/dev/video%d
be used too ?
or would these miss some cases ?

also like others suggested the score maybe should be lower


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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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


Re: [FFmpeg-devel] [PATCH] lavd/avfoundation: Add support for screen capturing.

2014-10-25 Thread Thilo Borgmann
Updated patch fixing off-by-one in device listing.

-Thilo
From d8dc49423dbcdadf739997c453204e137ed8c088 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@mail.de
Date: Sat, 25 Oct 2014 17:02:28 +0200
Subject: [PATCH] lavd/avfoundation: Add support for screen capturing.

Patch based on pull-request by Joseph Benden j...@benden.us
---
 configure  |  2 +-
 libavdevice/avfoundation.m | 70 --
 2 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/configure b/configure
index 3e181aa..3eb1aa0 100755
--- a/configure
+++ b/configure
@@ -2452,7 +2452,7 @@ xwma_demuxer_select=riffdec
 # indevs / outdevs
 alsa_indev_deps=alsa_asoundlib_h snd_pcm_htimestamp
 alsa_outdev_deps=alsa_asoundlib_h
-avfoundation_indev_extralibs=-framework CoreVideo -framework Foundation 
-framework AVFoundation -framework CoreMedia
+avfoundation_indev_extralibs=-framework CoreVideo -framework Foundation 
-framework AVFoundation -framework CoreMedia -framework CoreGraphics
 avfoundation_indev_select=avfoundation
 bktr_indev_deps_any=dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h
 caca_outdev_deps=libcaca
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 8c00a0e..75c62ed 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -99,6 +99,8 @@ typedef struct
 char*video_filename;
 char*audio_filename;
 
+int num_video_devices;
+
 int audio_channels;
 int audio_bits_per_sample;
 int audio_float;
@@ -264,16 +266,22 @@ static int add_video_device(AVFormatContext *s, 
AVCaptureDevice *video_device)
 {
 AVFContext *ctx = (AVFContext*)s-priv_data;
 NSError *error  = nil;
-AVCaptureDeviceInput* capture_dev_input = [[[AVCaptureDeviceInput alloc] 
initWithDevice:video_device error:error] autorelease];
+AVCaptureInput* capture_input = nil;
+
+if (ctx-video_device_index  ctx-num_video_devices) {
+capture_input = (AVCaptureInput*) [[[AVCaptureDeviceInput alloc] 
initWithDevice:video_device error:error] autorelease];
+} else {
+capture_input = (AVCaptureInput*) video_device;
+}
 
-if (!capture_dev_input) {
+if (!capture_input) {
 av_log(s, AV_LOG_ERROR, Failed to create AV capture input device: 
%s\n,
[[error localizedDescription] UTF8String]);
 return 1;
 }
 
-if ([ctx-capture_session canAddInput:capture_dev_input]) {
-[ctx-capture_session addInput:capture_dev_input];
+if ([ctx-capture_session canAddInput:capture_input]) {
+[ctx-capture_session addInput:capture_input];
 } else {
 av_log(s, AV_LOG_ERROR, can't add video input to capture session\n);
 return 1;
@@ -522,19 +530,32 @@ static int avf_read_header(AVFormatContext *s)
 AVFContext *ctx = (AVFContext*)s-priv_data;
 ctx-first_pts  = av_gettime();
 ctx-first_audio_pts= av_gettime();
+uint32_t num_screens= 0;
 
 pthread_mutex_init(ctx-frame_lock, NULL);
 pthread_cond_init(ctx-frame_wait_cond, NULL);
 
+CGGetActiveDisplayList(0, NULL, num_screens);
+
 // List devices if requested
 if (ctx-list_devices) {
 av_log(ctx, AV_LOG_INFO, AVFoundation video devices:\n);
 NSArray *devices = [AVCaptureDevice 
devicesWithMediaType:AVMediaTypeVideo];
+int index = 0;
 for (AVCaptureDevice *device in devices) {
 const char *name = [[device localizedName] UTF8String];
-int index  = [devices indexOfObject:device];
+index= [devices indexOfObject:device];
 av_log(ctx, AV_LOG_INFO, [%d] %s\n, index, name);
+index++;
 }
+if (num_screens  0) {
+CGDirectDisplayID screens[num_screens];
+CGGetActiveDisplayList(num_screens, screens, num_screens);
+for (int i = 0; i  num_screens; i++) {
+av_log(ctx, AV_LOG_INFO, [%d] Capture screen %d\n, index + 
i, i);
+}
+}
+
 av_log(ctx, AV_LOG_INFO, AVFoundation audio devices:\n);
 devices = [AVCaptureDevice devicesWithMediaType:AVMediaTypeAudio];
 for (AVCaptureDevice *device in devices) {
@@ -549,6 +570,9 @@ static int avf_read_header(AVFormatContext *s)
 AVCaptureDevice *video_device = nil;
 AVCaptureDevice *audio_device = nil;
 
+NSArray *video_devices = [AVCaptureDevice 
devicesWithMediaType:AVMediaTypeVideo];
+ctx-num_video_devices = [video_devices count];
+
 // parse input filename for video and audio device
 parse_device_name(s);
 
@@ -561,25 +585,39 @@ static int avf_read_header(AVFormatContext *s)
 }
 
 if (ctx-video_device_index = 0) {
-NSArray *devices = [AVCaptureDevice 
devicesWithMediaType:AVMediaTypeVideo];
-
-if (ctx-video_device_index = [devices count]) {
+if 

Re: [FFmpeg-devel] [PATCH 2/6] dv: use smaller type for weight tables

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 01:34:22PM +0200, Reimar Döffinger wrote:
 On Sat, Oct 25, 2014 at 11:19:21AM +, Christophe Gisquet wrote:
  ---
   libavcodec/dv.c |  4 ++--
   libavcodec/dvdata.c | 12 ++--
   libavcodec/dvdata.h | 12 ++--
   3 files changed, 14 insertions(+), 14 deletions(-)
 
 Looks good to me.
 I think in all places the uint16_t will be promoted to int
 automatically.
 But it might be good for someone else to double-check that this
 signed-unsigned change doesn't break anything, I consider C
 behaviour a bit too tricky...

applied

thanks

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

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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


Re: [FFmpeg-devel] [PATCH 1/6] dv: better split weight tables assignment

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 11:19:20AM +, Christophe Gisquet wrote:
 This is a mostly cosmetical patch in preparation for the following.
 ---
  libavcodec/dv.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

applied

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


Re: [FFmpeg-devel] [PATCH 4/6] dv: fix weight table for 2x4x8 transform

2014-10-25 Thread Christophe Gisquet
2014-10-25 13:35 GMT+02:00 Reimar Döffinger reimar.doeffin...@gmx.de:
 Could you maybe add e.g. a FATE test that clearly shows the before-after
 improvements?

I've tried for a small while, by swapping fields on lena and converting to
yuv42[02]p and feeding it to ffmpeg with:
-pix_fmt yuv422p -s 720x576 -i lena.yuv -flags ildct -vf
setfield=1,fieldorder=bff -vcodec dvvideo out.dv
The PSNR results were weird (with 2 exes I thought were before/after), so I
didn't follow through. Maybe someone more versed in libavfi can offer a
command-line doing the job.

The only conclusive impact is on the #2970 sequence, but it has too few
blocks coded as interlaced (!) to matter for anything but visual. And
indeed the fate tests do not seem to exercise the affected code.

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


Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter

2014-10-25 Thread arwa arif
On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer michae...@gmx.at
wrote:

 On Fri, Oct 24, 2014 at 10:34:32PM +0530, arwa arif wrote:
  I have taken care of aal the things mentioned except the floating point.
 I
  will update the floating point part till tomorrow. For now, I have
 attached
  the patch updated till now.
 [...]

   vf_xbr.c |  418
 +++
   1 file changed, 181 insertions(+), 237 deletions(-)
  6b1a6fc1ae74e0881cf0f9d1fb8831bd6aa77fa8  0001-xBR-filter.patch
  From 941bef1dcebbcfca5e6a665bfb744ee89599cc0e Mon Sep 17 00:00:00 2001
  From: Arwa Arif arwaarif1...@gmail.com
  Date: Fri, 24 Oct 2014 22:31:34 +0530
  Subject: [PATCH] xBR-filter

 please post a new patch instead of a patch on top of a previous
 patch

 [...]

 --
 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

 Awnsering whenever a program halts or runs forever is
 On a turing machine, in general impossible (turings halting problem).
 On any real computer, always possible as a real computer has a finite
 number
 of states N, and will either halt in less than N cycles or never halt.

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


From 739717795dded26f6f3fd44de81a2eee2bd9c838 Mon Sep 17 00:00:00 2001
From: Arwa Arif arwaarif1...@gmail.com
Date: Sat, 25 Oct 2014 22:04:51 +0530
Subject: [PATCH] xBR-filter

---
 libavfilter/vf_xbr.c |  303 ++
 1 file changed, 303 insertions(+)
 create mode 100644 libavfilter/vf_xbr.c

diff --git a/libavfilter/vf_xbr.c b/libavfilter/vf_xbr.c
new file mode 100644
index 000..6ebeff4
--- /dev/null
+++ b/libavfilter/vf_xbr.c
@@ -0,0 +1,303 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * Copyright (c) 2014 Arwa Arif arwaarif1...@gmail.com
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * XBR Filter is used for depixelization of image.
+ * This is based on Hyllian's 2xBR shader.
+ * 2xBR Filter v0.2.5 
+ * Reference : http://board.byuu.org/viewtopic.php?f=10t=2248
+ */
+
+#include libavutil/opt.h
+#include libavutil/avassert.h
+#include libavutil/pixdesc.h
+#include internal.h
+
+const int THRESHHOLD_Y = 48;
+const int THRESHHOLD_U = 7;
+const int THRESHHOLD_V = 6;
+
+/**
+* Calculates the weight of difference of the pixels, by transforming these
+* pixels into their Y'UV parts. It then uses the threshold used by HQx filters:
+* 48*Y + 7*U + 6*V, to give it those smooth looking edges.
+**/
+static int d(AVFrame *in,int x1,int y1,int x2,int y2){
+
+int r1 = *(in-data[0] + y1 * in-linesize[0] + x1*3);
+int g1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 1);
+int b1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 2);
+
+int r2 = *(in-data[0] + y2 * in-linesize[0] + x2*3);
+int g2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 1);
+int b2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 2);
+
+int r = abs(r1 - r2);
+int g = abs(g1 - g2);
+int b = abs(b1 - b2);	
+
+/*Convert RGB to Y'UV*/
+int y = ( (  66 * r + 129 * g +  25 * b + 128)  8) +  16; 
+int u = ( ( -38 * r -  74 * g + 112 * b + 128)  8) + 128;
+int v = ( ( 112 * r -  94 * g -  18 * b + 128)  8) + 128;
+
+/*Add HQx filters threshold  return*/
+return (y * THRESHHOLD_Y) + (u* THRESHHOLD_U) + (v* THRESHHOLD_V);
+}
+
+/**
+* Mixes a pixel A, with pixel B, with B's transperancy set to 'a'
+* In other words, A is a solid color (bottom) and B is a transparent color (top)
+**/
+static int mix(AVFrame *in,int x1,int y1,int x2,int y2,int a,int mode){
+/*If red color*/
+int col1,col2;
+if(mode==0){
+col1 = *(in-data[0] + y1 * in-linesize[0] + x1*3);
+col2 = *(in-data[0] + y2 * in-linesize[0] + x2*3);
+}
+
+/*If green color*/
+else if(mode==1){
+col1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 1);
+col2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 1);
+}
+
+/*If blue color*/
+else{
+col1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 2);
+col2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 2);
+}
+
+return (a*col2 + (2-a)*col1)/2;
+};
+
+/**
+* Fills the output matrix
+**/
+static 

Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter

2014-10-25 Thread arwa arif
Please ignore the previous mail. I attached the wrong patch. New patch is
attached with this mail.

On Sat, Oct 25, 2014 at 10:06 PM, arwa arif arwaarif1...@gmail.com wrote:



 On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer michae...@gmx.at
 wrote:

 On Fri, Oct 24, 2014 at 10:34:32PM +0530, arwa arif wrote:
  I have taken care of aal the things mentioned except the floating
 point. I
  will update the floating point part till tomorrow. For now, I have
 attached
  the patch updated till now.
 [...]

   vf_xbr.c |  418
 +++
   1 file changed, 181 insertions(+), 237 deletions(-)
  6b1a6fc1ae74e0881cf0f9d1fb8831bd6aa77fa8  0001-xBR-filter.patch
  From 941bef1dcebbcfca5e6a665bfb744ee89599cc0e Mon Sep 17 00:00:00 2001
  From: Arwa Arif arwaarif1...@gmail.com
  Date: Fri, 24 Oct 2014 22:31:34 +0530
  Subject: [PATCH] xBR-filter

 please post a new patch instead of a patch on top of a previous
 patch

 [...]

 --
 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

 Awnsering whenever a program halts or runs forever is
 On a turing machine, in general impossible (turings halting problem).
 On any real computer, always possible as a real computer has a finite
 number
 of states N, and will either halt in less than N cycles or never halt.

 ___
 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] lavfi: add xbr filter

2014-10-25 Thread arwa arif
On Sat, Oct 25, 2014 at 10:14 PM, arwa arif arwaarif1...@gmail.com wrote:

 Please ignore the previous mail. I attached the wrong patch. New patch is
 attached with this mail.

 On Sat, Oct 25, 2014 at 10:06 PM, arwa arif arwaarif1...@gmail.com
 wrote:



 On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer michae...@gmx.at
 wrote:

 On Fri, Oct 24, 2014 at 10:34:32PM +0530, arwa arif wrote:
  I have taken care of aal the things mentioned except the floating
 point. I
  will update the floating point part till tomorrow. For now, I have
 attached
  the patch updated till now.
 [...]

   vf_xbr.c |  418
 +++
   1 file changed, 181 insertions(+), 237 deletions(-)
  6b1a6fc1ae74e0881cf0f9d1fb8831bd6aa77fa8  0001-xBR-filter.patch
  From 941bef1dcebbcfca5e6a665bfb744ee89599cc0e Mon Sep 17 00:00:00 2001
  From: Arwa Arif arwaarif1...@gmail.com
  Date: Fri, 24 Oct 2014 22:31:34 +0530
  Subject: [PATCH] xBR-filter

 please post a new patch instead of a patch on top of a previous
 patch

 [...]

 --
 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

 Awnsering whenever a program halts or runs forever is
 On a turing machine, in general impossible (turings halting problem).
 On any real computer, always possible as a real computer has a finite
 number
 of states N, and will either halt in less than N cycles or never halt.

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




From 739717795dded26f6f3fd44de81a2eee2bd9c838 Mon Sep 17 00:00:00 2001
From: Arwa Arif arwaarif1...@gmail.com
Date: Sat, 25 Oct 2014 22:04:51 +0530
Subject: [PATCH] xBR-filter

---
 libavfilter/vf_xbr.c |  303 ++
 1 file changed, 303 insertions(+)
 create mode 100644 libavfilter/vf_xbr.c

diff --git a/libavfilter/vf_xbr.c b/libavfilter/vf_xbr.c
new file mode 100644
index 000..6ebeff4
--- /dev/null
+++ b/libavfilter/vf_xbr.c
@@ -0,0 +1,303 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * Copyright (c) 2014 Arwa Arif arwaarif1...@gmail.com
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * XBR Filter is used for depixelization of image.
+ * This is based on Hyllian's 2xBR shader.
+ * 2xBR Filter v0.2.5 
+ * Reference : http://board.byuu.org/viewtopic.php?f=10t=2248
+ */
+
+#include libavutil/opt.h
+#include libavutil/avassert.h
+#include libavutil/pixdesc.h
+#include internal.h
+
+const int THRESHHOLD_Y = 48;
+const int THRESHHOLD_U = 7;
+const int THRESHHOLD_V = 6;
+
+/**
+* Calculates the weight of difference of the pixels, by transforming these
+* pixels into their Y'UV parts. It then uses the threshold used by HQx filters:
+* 48*Y + 7*U + 6*V, to give it those smooth looking edges.
+**/
+static int d(AVFrame *in,int x1,int y1,int x2,int y2){
+
+int r1 = *(in-data[0] + y1 * in-linesize[0] + x1*3);
+int g1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 1);
+int b1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 2);
+
+int r2 = *(in-data[0] + y2 * in-linesize[0] + x2*3);
+int g2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 1);
+int b2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 2);
+
+int r = abs(r1 - r2);
+int g = abs(g1 - g2);
+int b = abs(b1 - b2);	
+
+/*Convert RGB to Y'UV*/
+int y = ( (  66 * r + 129 * g +  25 * b + 128)  8) +  16; 
+int u = ( ( -38 * r -  74 * g + 112 * b + 128)  8) + 128;
+int v = ( ( 112 * r -  94 * g -  18 * b + 128)  8) + 128;
+
+/*Add HQx filters threshold  return*/
+return (y * THRESHHOLD_Y) + (u* THRESHHOLD_U) + (v* THRESHHOLD_V);
+}
+
+/**
+* Mixes a pixel A, with pixel B, with B's transperancy set to 'a'
+* In other words, A is a solid color (bottom) and B is a transparent color (top)
+**/
+static int mix(AVFrame *in,int x1,int y1,int x2,int y2,int a,int mode){
+/*If red color*/
+int col1,col2;
+if(mode==0){
+col1 = *(in-data[0] + y1 * in-linesize[0] + x1*3);
+col2 = *(in-data[0] + y2 * in-linesize[0] + x2*3);
+}
+
+/*If green color*/
+else if(mode==1){
+col1 = *(in-data[0] + y1 * in-linesize[0] + x1*3 + 1);
+col2 = *(in-data[0] + y2 * in-linesize[0] + x2*3 + 1);
+}

Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, arwa arif a écrit :
  please post a new patch instead of a patch on top of a previous
  patch
  libavfilter/vf_xbr.c |  303 
 ++
  1 file changed, 303 insertions(+)
  create mode 100644 libavfilter/vf_xbr.c

This patch does not contain the changes to Makefile and allfilters.c, so I
believe you still have a bit of tweaking to do with Git.

If you used a branch (that is widely advisable), you should be able to type
this:

git log --stat master..

Then you should see a single commit, yours, with changes to Makefile,
allfilters.c and all other common files you needed to changes, and of course
the new file(s).

(If you have stand-alone changes, such as moving code into a shared function
to use in your actual patch, then it should go in a separate commit. But it
does not seem to apply here.)

Also, I suspect you forgot to add the documentation for the filter in
doc/filters.texi. A few words are enough, but at the very least let people
know what it does, because xbr looks like just three random letters -- not
your fault of course.

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/6] dv: more precise weight table for 8x8

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 11:19:22AM +, Christophe Gisquet wrote:
 It is derived from the actual equations of the specs. In
 particular, it is closer to the inverse of what the encoder uses.
 
 fate tests accordingly updated.
 ---
  libavcodec/dvdata.c | 16 
  tests/ref/lavf/dv_fmt   |  6 +++---
  tests/ref/vsynth/vsynth1-dv |  2 +-
  tests/ref/vsynth/vsynth1-dv-411 |  4 ++--
  tests/ref/vsynth/vsynth1-dv-50  |  2 +-
  tests/ref/vsynth/vsynth2-dv |  4 ++--
  tests/ref/vsynth/vsynth2-dv-411 |  2 +-
  tests/ref/vsynth/vsynth2-dv-50  |  2 +-
  8 files changed, 19 insertions(+), 19 deletions(-)

applied

thanks

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

Those who are best at talking, realize last or never when they are wrong.


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


Re: [FFmpeg-devel] [PATCH 4/4] ffserver_config: postpone codec context creation

2014-10-25 Thread Lukasz Marek

On 24.10.2014 00:18, Reynaldo H. Verdejo Pinochet wrote:

Hi Lukasz

+static int ffserver_apply_stream_config(AVCodecContext *enc, const 
AVDictionary *conf, AVDictionary **opts)
+{
+static const char *error_message = Cannot parse '%s' as number for %s 
parameter.\n;
+AVDictionaryEntry *e;
+char *tailp;
+int ret = 0;
+
+#define SET_INT_PARAM(factor, param, key)   \
+if ((e = av_dict_get(conf, #key, NULL, 0))) {   \
+if (!e-value[0]) { \
+av_log(NULL, AV_LOG_ERROR, error_message, e-value, #key); \
+ret = AVERROR(EINVAL);  \
+}   \
+enc-param = strtol(e-value, tailp, 0);   \
+if (factor) enc-param *= (factor); \
+if (tailp[0] || errno) {\
+av_log(NULL, AV_LOG_ERROR, error_message, e-value, #key); \
+ret = AVERROR(errno);   \
+}   \
+}
+#define SET_DOUBLE_PARAM(factor, param, key)\
+if ((e = av_dict_get(conf, #key, NULL, 0))) {   \
+if (!e-value[0]) { \
+av_log(NULL, AV_LOG_ERROR, error_message, e-value, #key); \
+ret = AVERROR(EINVAL);  \
+}   \
+enc-param = strtod(e-value, tailp);  \
+if (factor) enc-param *= (factor); \
+if (tailp[0] || errno) {\
+av_log(NULL, AV_LOG_ERROR, error_message, e-value, #key); \
+ret = AVERROR(errno);   \
+}   \
+}


Can you turn those two macros into static inline funcs?. Also,
looks like it shouldn't be too hard to factor those two into
just 1 func. This is mostly to aid debugging. Nothing vital.


I changed macros to functions. I think inline is not required in such 
code. Small comment, there is plenty of atoi in parsing code.
It returns 0 in case parsed string is not a number and ignores random 
suffixed. Probably more problems can be pointed here. I prepared these 
functions to replace all atoi with them.


Since these functions allows to check ranges, I split back all options 
to separate if's, so every option can have its own range.



[..]
@@ -624,136 +712,104 @@ static int ffserver_parse_config_stream(FFServerConfig 
*config, const char *cmd,
  stream-max_time = atof(arg) * 1000;
  } else if (!av_strcasecmp(cmd, AudioBitRate)) {
  ffserver_get_arg(arg, sizeof(arg), p);
-config-audio_enc.bit_rate = lrintf(atof(arg) * 1000);
-} else if (!av_strcasecmp(cmd, AudioChannels)) {
+av_dict_set_int(config-audio_conf, cmd, lrintf(atof(arg) * 1000), 0);
+} else if (!av_strcasecmp(cmd, AudioChannels) ||
+   !av_strcasecmp(cmd, AudioSampleRate)) {
  ffserver_get_arg(arg, sizeof(arg), p);
-config-audio_enc.channels = atoi(arg);
-} else if (!av_strcasecmp(cmd, AudioSampleRate)) {
-ffserver_get_arg(arg, sizeof(arg), p);
-config-audio_enc.sample_rate = atoi(arg);
+av_dict_set(config-audio_conf, cmd, arg, 0);


Here and on every occurrence:

Any particular reason why not to check av_dict_set*()'s return
for  0? Only reason I ask is because the config code already
has failed allocation checks elsewhere. Also, should help avoiding
some coverity scan noise.


I forgot about that. Checks added.



[..]
-
+AVDictionary *video_opts; /* Contains AVOptions for video encoder */
+AVDictionary *video_conf; /* Contains values stored in video 
AVCodecContext.fields */
+AVDictionary *audio_opts; /* Contains AVOptions for audio encoder */
+AVDictionary *audio_conf; /* Contains values stored in audio 
AVCodecContext.fields */


Would drop the repeated Contains.


Dropped.


From ab5395a62b60cedd47d9e6894b685c29a8c87f3d Mon Sep 17 00:00:00 2001
From: Lukasz Marek lukasz.m.lu...@gmail.com
Date: Sun, 19 Oct 2014 21:29:40 +0200
Subject: [PATCH] ffserver_config: postpone codec context creation

So far AVCodecContext was created without codec specified.
This causes internal data to not be initialized to defaults.

This commit postpone context creation until all information is gathered.

Partially fixes #1275
---
 ffserver_config.c | 386 --
 ffserver_config.h |   9 +-
 2 files changed, 297 insertions(+), 98 deletions(-)

diff --git a/ffserver_config.c b/ffserver_config.c
index e44cdf7..4c8740d 100644
--- a/ffserver_config.c
+++ b/ffserver_config.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include float.h
 #include libavutil/opt.h
 #include 

Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter

2014-10-25 Thread arwa arif
Can you please specify what is meant by stand-alone changes? Do I need to
add the non-default functions in different commit? I am not very sure if I
understood it right. Apart from that, I have updated the patch.

On Sat, Oct 25, 2014 at 10:16 PM, Nicolas George geo...@nsup.org wrote:

 Le quartidi 4 brumaire, an CCXXIII, arwa arif a écrit :
   please post a new patch instead of a patch on top of a previous
   patch
   libavfilter/vf_xbr.c |  303
 ++
   1 file changed, 303 insertions(+)
   create mode 100644 libavfilter/vf_xbr.c

 This patch does not contain the changes to Makefile and allfilters.c, so I
 believe you still have a bit of tweaking to do with Git.

 If you used a branch (that is widely advisable), you should be able to type
 this:

 git log --stat master..

 Then you should see a single commit, yours, with changes to Makefile,
 allfilters.c and all other common files you needed to changes, and of
 course
 the new file(s).

 (If you have stand-alone changes, such as moving code into a shared
 function
 to use in your actual patch, then it should go in a separate commit. But it
 does not seem to apply here.)

 Also, I suspect you forgot to add the documentation for the filter in
 doc/filters.texi. A few words are enough, but at the very least let people
 know what it does, because xbr looks like just three random letters -- not
 your fault of course.

 Regards,

 --
   Nicolas George

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


From cf3a7a6bcb9735c6f6157f80da208c1c191b3e02 Mon Sep 17 00:00:00 2001
From: Arwa Arif arwaarif1...@gmail.com
Date: Sat, 25 Oct 2014 22:04:51 +0530
Subject: [PATCH] [PATCH]lavfi: add xbr filter

Makefile

allfilters.c

filters.texi
---
 doc/filters.texi |7 ++
 libavfilter/Makefile |1 +
 libavfilter/allfilters.c |1 +
 libavfilter/vf_xbr.c |  303 ++
 4 files changed, 312 insertions(+)
 create mode 100644 libavfilter/vf_xbr.c

diff --git a/doc/filters.texi b/doc/filters.texi
index c70ddf3..c05368d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -9159,6 +9159,13 @@ Only deinterlace frames marked as interlaced.
 Default value is @samp{all}.
 @end table
 
+@section xBR
+
+A high-quality magnification filter which is designed for pixel art. It follows a set
+of edge-detection rules @url{http://www.libretro.com/forums/viewtopic.php?f=6t=134}.
+This filter was originally created by Hyllian. The current implementation scales the
+image by scale factor 2.
+
 @anchor{yadif}
 @section yadif
 
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 6d868e7..2c56e38 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -198,6 +198,7 @@ OBJS-$(CONFIG_VIDSTABDETECT_FILTER)  += vidstabutils.o vf_vidstabdetect.
 OBJS-$(CONFIG_VIDSTABTRANSFORM_FILTER)   += vidstabutils.o vf_vidstabtransform.o
 OBJS-$(CONFIG_VIGNETTE_FILTER)   += vf_vignette.o
 OBJS-$(CONFIG_W3FDIF_FILTER) += vf_w3fdif.o
+OBJS-$(CONFIG_XBR_FILTER)+= vf_xbr.o
 OBJS-$(CONFIG_YADIF_FILTER)  += vf_yadif.o
 OBJS-$(CONFIG_ZMQ_FILTER)+= f_zmq.o
 OBJS-$(CONFIG_ZOOMPAN_FILTER)+= vf_zoompan.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index d88a9ad..2352d44 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -213,6 +213,7 @@ void avfilter_register_all(void)
 REGISTER_FILTER(VIDSTABTRANSFORM, vidstabtransform, vf);
 REGISTER_FILTER(VIGNETTE,   vignette,   vf);
 REGISTER_FILTER(W3FDIF, w3fdif, vf);
+REGISTER_FILTER(XBR,xbr,vf);
 REGISTER_FILTER(YADIF,  yadif,  vf);
 REGISTER_FILTER(ZMQ,zmq,vf);
 REGISTER_FILTER(ZOOMPAN,zoompan,vf);
diff --git a/libavfilter/vf_xbr.c b/libavfilter/vf_xbr.c
new file mode 100644
index 000..6ebeff4
--- /dev/null
+++ b/libavfilter/vf_xbr.c
@@ -0,0 +1,303 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * Copyright (c) 2014 Arwa Arif arwaarif1...@gmail.com
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, arwa arif a écrit :
 Can you please specify what is meant by stand-alone changes?

I believe it was already specified in my original message: such as moving
code into a shared function, and pore importantly: it does not seem to
apply here.

 Apart from that, I have updated the patch.

I have no further remarks about commit organization. The doc looks fine too,
except a small nitpick: capitalization should follow the name of the filter
in filter graphs, all lowercase.

I will leave others comment the actual code.

Thanks.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH 4/6] dv: fix weight table for 2x4x8 transform

2014-10-25 Thread Christophe Gisquet
Hi,

2014-10-25 18:25 GMT+02:00 Reimar Döffinger reimar.doeffin...@gmx.de:
 On Sat, Oct 25, 2014 at 05:35:37PM +0200, Christophe Gisquet wrote:
 2014-10-25 13:35 GMT+02:00 Reimar Döffinger reimar.doeffin...@gmx.de:
  Could you maybe add e.g. a FATE test that clearly shows the before-after
  improvements?

 I've tried for a small while, by swapping fields on lena and converting to
 yuv42[02]p and feeding it to ffmpeg with:
 -pix_fmt yuv422p -s 720x576 -i lena.yuv -flags ildct -vf
 setfield=1,fieldorder=bff -vcodec dvvideo out.dv
 The PSNR results were weird (with 2 exes I thought were before/after), so I
 didn't follow through. Maybe someone more versed in libavfi can offer a
 command-line doing the job.

 The only conclusive impact is on the #2970 sequence, but it has too few
 blocks coded as interlaced (!) to matter for anything but visual. And
 indeed the fate tests do not seem to exercise the affected code.

 Maybe I misunderstand the issue, but maybe a encoder option
 to force interlaced encoding would work to trigger this reliably?

What I meant is I would need someone to:
1) provide a command line to swap fields to produce an artificially
interlaced image
2) confirm that -flags ildct sets CODEC_FLAG_INTERLACED_DCT and does
what is needed

Regarding 1), it is all the more needed since the encoder decides on
the fly which of the interlaced or normal dcts are better. And I would
then expect to notice a difference only if it is used frequently.
That's not what I observed so I bet I got something wrong for 1)
and/or 2).

Best regards,
-- 
Christophe
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/rdt: Forward whitelists to rdt demuxer

2014-10-25 Thread Michael Niedermayer
On Fri, Oct 24, 2014 at 12:35:33AM +0200, Michael Niedermayer wrote:
 Signed-off-by: Michael Niedermayer michae...@gmx.at
 ---
  libavformat/rdt.c |   21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

reviewed by ronald

applied

thx

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato


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


[FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

2014-10-25 Thread Michael Niedermayer
This fixes the issue that a not set or not forwarded whitelist
would allow to bypass it.

Signed-off-by: Michael Niedermayer michae...@gmx.at
---
 libavcodec/avcodec.h   |   17 +
 libavcodec/utils.c |   14 +-
 libavformat/avformat.h |4 
 libavformat/utils.c|6 --
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index eac3fc7..1000c80 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3118,6 +3118,8 @@ typedef struct AVCodecContext {
  * If NULL then all are allowed
  * - encoding: unused
  * - decoding: set by user through AVOPtions (NO direct access)
+ *
+ * @see av_enable_strict_whitelists()
  */
 char *codec_whitelist;
 } AVCodecContext;
@@ -5240,6 +5242,21 @@ const AVCodecDescriptor *avcodec_descriptor_next(const 
AVCodecDescriptor *prev);
 const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
 
 /**
+ * Enables strict whitelists, so that if no whitelist is set nothing will be
+ * allowed.
+ * This improves security because when some code forgets to set or forward
+ * the whitelists it will fail instead of allowing an attacker to access a
+ * larger codebase than intended/needed.
+ */
+void av_enable_strict_whitelists(void);
+
+/**
+ * returns non zero if strict whitelists are enabled.
+ * @see av_enable_strict_whitelists()
+ */
+int av_are_strict_whitelists_enabled(void);
+
+/**
  * @}
  */
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index b6ae1c0..6eb455a 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -118,6 +118,7 @@ volatile int ff_avcodec_locked;
 static int volatile entangled_thread_counter = 0;
 static void *codec_mutex;
 static void *avformat_mutex;
+static int strict_whitelists;
 
 static inline int ff_fast_malloc(void *ptr, unsigned int *size, size_t 
min_size, int zero_realloc)
 {
@@ -157,6 +158,16 @@ void av_fast_padded_mallocz(void *ptr, unsigned int *size, 
size_t min_size)
 memset(*p, 0, min_size + FF_INPUT_BUFFER_PADDING_SIZE);
 }
 
+void av_enable_strict_whitelists(void)
+{
+strict_whitelists = 1;
+}
+
+int av_are_strict_whitelists_enabled(void)
+{
+return strict_whitelists;
+}
+
 /* encoder management */
 static AVCodec *first_avcodec = NULL;
 static AVCodec **last_avcodec = first_avcodec;
@@ -1385,7 +1396,8 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
 if ((ret = av_opt_set_dict(avctx, tmp))  0)
 goto free_and_end;
 
-if (avctx-codec_whitelist  av_match_list(codec-name, 
avctx-codec_whitelist, ',') = 0) {
+if (   (avctx-codec_whitelist || av_are_strict_whitelists_enabled())
+ av_match_list(codec-name, avctx-codec_whitelist, ',') = 0) {
 av_log(avctx, AV_LOG_ERROR, Codec (%s) not on whitelist\n, 
codec-name);
 ret = AVERROR(EINVAL);
 goto free_and_end;
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index f21a1d6..529b068 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1589,6 +1589,8 @@ typedef struct AVFormatContext {
  * If NULL then all are allowed
  * - encoding: unused
  * - decoding: set by user through AVOptions (NO direct access)
+ *
+ * @see av_enable_strict_whitelists()
  */
 char *codec_whitelist;
 
@@ -1597,6 +1599,8 @@ typedef struct AVFormatContext {
  * If NULL then all are allowed
  * - encoding: unused
  * - decoding: set by user through AVOptions (NO direct access)
+ *
+ * @see av_enable_strict_whitelists()
  */
 char *format_whitelist;
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 61421c0..f8d5c88 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -304,7 +304,8 @@ static int set_codec_from_probe_data(AVFormatContext *s, 
AVStream *st,
 int av_demuxer_open(AVFormatContext *ic) {
 int err;
 
-if (ic-format_whitelist  av_match_list(ic-iformat-name, 
ic-format_whitelist, ',') = 0) {
+if (   (ic-format_whitelist || av_are_strict_whitelists_enabled())
+ av_match_list(ic-iformat-name, ic-format_whitelist, ',') = 0) {
 av_log(ic, AV_LOG_ERROR, Format not on whitelist\n);
 return AVERROR(EINVAL);
 }
@@ -421,7 +422,8 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
 goto fail;
 s-probe_score = ret;
 
-if (s-format_whitelist  av_match_list(s-iformat-name, 
s-format_whitelist, ',') = 0) {
+if (   (s-format_whitelist || av_are_strict_whitelists_enabled())
+ av_match_list(s-iformat-name, s-format_whitelist, ',') = 0) {
 av_log(s, AV_LOG_ERROR, Format not on whitelist\n);
 ret = AVERROR(EINVAL);
 goto fail;
-- 
1.7.9.5

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


Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

2014-10-25 Thread James Almer
On 25/10/14 4:51 PM, Michael Niedermayer wrote:
 diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
 index eac3fc7..1000c80 100644
 --- a/libavcodec/avcodec.h
 +++ b/libavcodec/avcodec.h
 @@ -3118,6 +3118,8 @@ typedef struct AVCodecContext {
   * If NULL then all are allowed
   * - encoding: unused
   * - decoding: set by user through AVOPtions (NO direct access)
 + *
 + * @see av_enable_strict_whitelists()
   */
  char *codec_whitelist;
  } AVCodecContext;
 @@ -5240,6 +5242,21 @@ const AVCodecDescriptor *avcodec_descriptor_next(const 
 AVCodecDescriptor *prev);
  const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
  
  /**
 + * Enables strict whitelists, so that if no whitelist is set nothing will be
 + * allowed.
 + * This improves security because when some code forgets to set or forward
 + * the whitelists it will fail instead of allowing an attacker to access a
 + * larger codebase than intended/needed.
 + */
 +void av_enable_strict_whitelists(void);
 +
 +/**
 + * returns non zero if strict whitelists are enabled.
 + * @see av_enable_strict_whitelists()
 + */
 +int av_are_strict_whitelists_enabled(void);
 +
 +/**
   * @}
   */

How about

av_codec_whitelist_strict_enable() av_codec_whitelist_strict_enabled()
av_codec_whitelist_enable_strict() av_codec_whitelist_enabled_strict()
av_strict_whitelist_enable() av_strict_whitelist_enabled()

or similar, to make both names consistent?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavd/avfoundation: Add support for screen capturing.

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 05:07:02PM +0200, Thilo Borgmann wrote:
 Updated patch fixing off-by-one in device listing.
 
 -Thilo

  configure  |2 -
  libavdevice/avfoundation.m |   70 
 -
  2 files changed, 57 insertions(+), 15 deletions(-)
 142e96af6d48f77a2b4a4920043fc5166c9f0cc7  
 0001-lavd-avfoundation-Add-support-for-screen-capturing.patch
 From d8dc49423dbcdadf739997c453204e137ed8c088 Mon Sep 17 00:00:00 2001
 From: Thilo Borgmann thilo.borgm...@mail.de
 Date: Sat, 25 Oct 2014 17:02:28 +0200
 Subject: [PATCH] lavd/avfoundation: Add support for screen capturing.
 
 Patch based on pull-request by Joseph Benden j...@benden.us
 ---
  configure  |  2 +-
  libavdevice/avfoundation.m | 70 
 --
  2 files changed, 57 insertions(+), 15 deletions(-)

applied

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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


Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec: read source timecode from pulldown component

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 10:42:25PM +0200, Tomas Härdin wrote:
 On Fri, 2014-10-24 at 17:31 -0700, Mark Reid wrote:
  ---
   libavformat/mxf.h  |  1 +
   libavformat/mxfdec.c   | 31 +--
   tests/ref/lavf/mxf |  6 +++---
   tests/ref/lavf/mxf_d10 |  2 +-
   4 files changed, 34 insertions(+), 6 deletions(-)
  
  diff --git a/libavformat/mxf.h b/libavformat/mxf.h
  index 036c15e..5b95efa 100644
 
 Looks good.  Simple enough that it shouldn't cause any problems. Do you

applied

thx


 have a spec for it, or is it just an undocumented Avid extension?
 
 Taking the opportunity here to note that the strong ref resolving stuff
 is getting a bit hairy. Could use some refactoring in the future
 perhaps?
 
 /Tomas



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


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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


Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

2014-10-25 Thread Michael Niedermayer
On Sat, Oct 25, 2014 at 05:43:00PM -0300, James Almer wrote:
 On 25/10/14 4:51 PM, Michael Niedermayer wrote:
  diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
  index eac3fc7..1000c80 100644
  --- a/libavcodec/avcodec.h
  +++ b/libavcodec/avcodec.h
  @@ -3118,6 +3118,8 @@ typedef struct AVCodecContext {
* If NULL then all are allowed
* - encoding: unused
* - decoding: set by user through AVOPtions (NO direct access)
  + *
  + * @see av_enable_strict_whitelists()
*/
   char *codec_whitelist;
   } AVCodecContext;
  @@ -5240,6 +5242,21 @@ const AVCodecDescriptor 
  *avcodec_descriptor_next(const AVCodecDescriptor *prev);
   const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
   
   /**
  + * Enables strict whitelists, so that if no whitelist is set nothing will 
  be
  + * allowed.
  + * This improves security because when some code forgets to set or forward
  + * the whitelists it will fail instead of allowing an attacker to access a
  + * larger codebase than intended/needed.
  + */
  +void av_enable_strict_whitelists(void);
  +
  +/**
  + * returns non zero if strict whitelists are enabled.
  + * @see av_enable_strict_whitelists()
  + */
  +int av_are_strict_whitelists_enabled(void);
  +
  +/**
* @}
*/
 
 How about
 
 av_codec_whitelist_strict_enable() av_codec_whitelist_strict_enabled()
 av_codec_whitelist_enable_strict() av_codec_whitelist_enabled_strict()
 av_strict_whitelist_enable() av_strict_whitelist_enabled()

these are typo prone, i mean will you spot this:
(could easily happen with auto completion of words ...)

av_codec_whitelist_strict_enabled(); // enabling strict whitelists
avcodec_open();

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

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


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


Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()

2014-10-25 Thread Nicolas George
Le quartidi 4 brumaire, an CCXXIII, Michael Niedermayer a écrit :
 This fixes the issue that a not set or not forwarded whitelist
 would allow to bypass it.

I am a bit worried by the sheer amount of extra code and API this simple
feature requires. Maybe it is a sign that it was not the best approach to
begin with.

Also, I did not say at the time, but I believe using a string to hold a list
like that is quite ugly.

Unless I am mistaken, the core of the problem resides in the existence of
global variables in the library, and this is not the only instance of this
issue: remember people who want to free the global mutex manager, not
understanding that still reachable is not a memory leak.

There is an idea that I have been nursing for some time: moving all these
global variables to a structure, and start passing it around everywhere.
Most of the functions that access the global variables already have some
kind of context where the pointer can be stored, so it would only require
changing a few API entry points.

That may look somewhat like that:

AVCodecInstance *libavcodec = avcodec_library_create(OPTIONS);
AVFormatInstance *libavformat = avformat_library_create(libavformat, OPTIONS);

avformat_instance_open_input(libavformat, ctx, filename, NULL, NULL);

Then the whitelist feature comes for free: use the options to disable
registering all the codecs, then manually register the ones needed.

Of course, for compatibility, a global instance need to be inited if the
legacy entry points are used, but it becomes much easier to test extensively
that modern API usage will not touch them.

Of course, that requires more work, and the whitelist feature will not be
available immediately, but in the long run I believe it would be greatly
beneficial.

What do people think of that?

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] libavformat/mxfdec: read source timecode from pulldown component

2014-10-25 Thread Mark Reid
On Sat, Oct 25, 2014 at 1:42 PM, Tomas Härdin tomas.har...@codemill.se
wrote:

 On Fri, 2014-10-24 at 17:31 -0700, Mark Reid wrote:
  ---
   libavformat/mxf.h  |  1 +
   libavformat/mxfdec.c   | 31 +--
   tests/ref/lavf/mxf |  6 +++---
   tests/ref/lavf/mxf_d10 |  2 +-
   4 files changed, 34 insertions(+), 6 deletions(-)
 
  diff --git a/libavformat/mxf.h b/libavformat/mxf.h
  index 036c15e..5b95efa 100644

 Looks good. Simple enough that it shouldn't cause any problems. Do you
 have a spec for it, or is it just an undocumented Avid extension?


I found the uuids via the MXFDump util and pulldown objects are documented
in the aaf sdk

http://aaf.cvs.sourceforge.net/viewvc/aaf/AAF/doc/aafobjectspec-v1.1.pdf
(page 56)
http://sourceforge.net/p/bmxlib/libmxf/ci/master/tree/tools/MXFDump/AAFMetaDictionary.h#l1158


 Taking the opportunity here to note that the strong ref resolving stuff
 is getting a bit hairy. Could use some refactoring in the future
 perhaps?


I was thinking the same thing, I'll try take a look into doing that and
send a refactoring patch in the future.
thanks for taking the time to review.


 /Tomas

 ___
 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] add av_enable_strict_whitelists()

2014-10-25 Thread Michael Niedermayer
On Sun, Oct 26, 2014 at 12:16:26AM +0200, Nicolas George wrote:
 Le quartidi 4 brumaire, an CCXXIII, Michael Niedermayer a écrit :
  This fixes the issue that a not set or not forwarded whitelist
  would allow to bypass it.
 
 I am a bit worried by the sheer amount of extra code and API this simple
 feature requires. Maybe it is a sign that it was not the best approach to
 begin with.
 
 Also, I did not say at the time, but I believe using a string to hold a list
 like that is quite ugly.
 

 Unless I am mistaken, the core of the problem resides in the existence of
 global variables in the library, and this is not the only instance of this
 issue: remember people who want to free the global mutex manager, not
 understanding that still reachable is not a memory leak.
 
 There is an idea that I have been nursing for some time: moving all these
 global variables to a structure, and start passing it around everywhere.
 Most of the functions that access the global variables already have some
 kind of context where the pointer can be stored, so it would only require
 changing a few API entry points.
 
 That may look somewhat like that:
 
 AVCodecInstance *libavcodec = avcodec_library_create(OPTIONS);
 AVFormatInstance *libavformat = avformat_library_create(libavformat, OPTIONS);
 
 avformat_instance_open_input(libavformat, ctx, filename, NULL, NULL);
 
 Then the whitelist feature comes for free: use the options to disable
 registering all the codecs, then manually register the ones needed.
 
 Of course, for compatibility, a global instance need to be inited if the
 legacy entry points are used, but it becomes much easier to test extensively
 that modern API usage will not touch them.
 
 Of course, that requires more work, and the whitelist feature will not be
 available immediately, but in the long run I believe it would be greatly
 beneficial.
 
 What do people think of that?

about whitelists, there are 2 use cases / features

The first is to globally within a process limit the decoders and
demuxers which can be used. This is to limit the attack surface a
attacker has, for this to really work well other libs, plugins
using their own local contexts and so on should be limited as well.
This can currently be achived by disabling decoders and demuxers at
compile time only, but the register list patch i posted previously
would allow to do this at runtime

The second is to limit a specific set of contexts, for example ones
used to demux and decode a remote url that would be intended for
for example the flashplayer, the set of demuxers and decoders for this
could be very limited wihout any loss of functionality as the original
flash player only supports a small set of formats, thus nicely
reducing the attack surface for an attacker.
This can currently be done with the whitelists but has the issue that
forgetting to pass a whitelist somewhere in the depth of one of the
libs could be a security issue.
The patch here fixes this weakness.
The whitelists could be passed around using a different system
like what you suggest, still this patch here would none the
less be needed to protect against the case where the context is not
copied/forwarded correctly
Thus your system probably is nicer than the whitelists as they are now
but it has the same problem and need for a global switch to enable
security
something that in plain english globally says everyone received a
whitelist, if you didnt then theres a bug dont open anything for
saftey and of course this implies that the default must be nothing
for the whitelists not all because otherwise a context opened with
defaults would bypass this and such a thing is easily made by mistake
like in a demuxer opening another demuxer

Also i think its best if we move the whitelists in the structs you
suggest when they are finished and in git but not to delay/block
work on them till then.

[...]

-- 
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] [PATCH] add av_enable_strict_whitelists()

2014-10-25 Thread Michael Niedermayer
On Sun, Oct 26, 2014 at 02:09:19AM +0200, wm4 wrote:
 On Sun, 26 Oct 2014 00:16:26 +0200
 Nicolas George geo...@nsup.org wrote:
 
  Le quartidi 4 brumaire, an CCXXIII, Michael Niedermayer a écrit :
   This fixes the issue that a not set or not forwarded whitelist
   would allow to bypass it.
  
  I am a bit worried by the sheer amount of extra code and API this simple
  feature requires. Maybe it is a sign that it was not the best approach to
  begin with.
  
  Also, I did not say at the time, but I believe using a string to hold a list
  like that is quite ugly.
  
  Unless I am mistaken, the core of the problem resides in the existence of
  global variables in the library, and this is not the only instance of this
  issue: remember people who want to free the global mutex manager, not
  understanding that still reachable is not a memory leak.
  
  There is an idea that I have been nursing for some time: moving all these
  global variables to a structure, and start passing it around everywhere.
  Most of the functions that access the global variables already have some
  kind of context where the pointer can be stored, so it would only require
  changing a few API entry points.
  
  That may look somewhat like that:
  
  AVCodecInstance *libavcodec = avcodec_library_create(OPTIONS);
  AVFormatInstance *libavformat = avformat_library_create(libavformat, 
  OPTIONS);
  
  avformat_instance_open_input(libavformat, ctx, filename, NULL, NULL);
  
  Then the whitelist feature comes for free: use the options to disable
  registering all the codecs, then manually register the ones needed.
  
  Of course, for compatibility, a global instance need to be inited if the
  legacy entry points are used, but it becomes much easier to test extensively
  that modern API usage will not touch them.
  
  Of course, that requires more work, and the whitelist feature will not be
  available immediately, but in the long run I believe it would be greatly
  beneficial.
  
  What do people think of that?
 
 That's exactly the same idea I suggested some hours ago on IRC - so
 it's not good, I guess?

the idea is only good with a volunteer to implement it.
the funny thing is i had thought about using such context too before
i read about it today from you and nicolas.
I didnt further look into it when i thought about it as it seemed
mostly orthogonal to the whitelists.
Its a nicer place to put them in but it doesnt really change things.
Also its a moderate amount of work and looks like it would require
user apps to be updated to a new API again which isnt all that great


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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


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


[FFmpeg-devel] [PATCH] x86: use new gcc atomic built-ins if available

2014-10-25 Thread James Almer
__sync built-ins are considered legacy and will be deprecated.
These new memory model aware built-ins have been available since GCC 4.7.0

Signed-off-by: James Almer jamr...@gmail.com
---
https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/_005f_005fatomic-Builtins.html
This is an RFC for a couple reasons.

The first is the memory model parameter. The documentation mentions that the 
__sync functions match the behavoir of the new __atomic functions when the 
latter use the full barrier model (__ATOMIC_SEQ_CST), so i went with it for 
consistency's sake. It may however be a good idea to check if any of the more 
relaxed models available for these new functions can be used instead.
It's worth mentioning that when i tested, gcc-tsan liked the __atomic load and 
store functions a lot more than __sync_synchronize(), regardless of memory 
model.

The second reason is __atomic_compare_exchange_n(), and how it differs from
__sync_val_compare_and_swap().
While the latter returns *ptr as it was before the operation, the former
doesn't and instead copies *ptr to oldval if the result of the comparison is 
false. This means that returning oldval will match the old behavoir without 
having to change the wrapper.
A disassemble example from libavutil/buffer.o however hints that the __atomic
function may be slower because of it writting oldval.

__sync_val_compare_and_swap:
 8e3:   48 89 d8movrax,rbx
 8e6:   f0 48 0f b1 16  lock cmpxchg QWORD PTR [rsi],rdx
 8eb:   48 85 c0test   rax,rax

__atomic_compare_exchange_n:
 8f0:   48 8d 4c 24 20  learcx,[rsp+0x20]
 [...]
 90c:   48 89 d8movrax,rbx
 90f:   48 89 5c 24 20  movQWORD PTR [rsp+0x20],rbx
 914:   f0 48 0f b1 16  lock cmpxchg QWORD PTR [rsi],rdx
 919:   74 03   je 91e av_buffer_pool_get+0x3e
 91b:   48 89 01movQWORD PTR [rcx],rax
 91e:   48 8b 44 24 20  movrax,QWORD PTR [rsp+0x20]
 923:   48 85 c0test   rax,rax

So the question is, do we keep using __sync_val_compare_and_swap as long as 
gcc offers it (Which is probably a very long time), or immediately switch to 
__atomic_compare_exchange_n if available?

 configure  |  4 +++-
 libavutil/atomic_gcc.h | 17 -
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 3eb1aa0..7697ed8 100755
--- a/configure
+++ b/configure
@@ -1596,6 +1596,7 @@ ARCH_FEATURES=
 
 BUILTIN_LIST=
 atomic_cas_ptr
+atomic_compare_exchange
 machine_rw_barrier
 MemoryBarrier
 mm_empty
@@ -2021,7 +2022,7 @@ simd_align_16_if_any=altivec neon sse
 symver_if_any=symver_asm_label symver_gnu_asm
 
 # threading support
-atomics_gcc_if=sync_val_compare_and_swap
+atomics_gcc_if_any=sync_val_compare_and_swap atomic_compare_exchange
 atomics_suncc_if=atomic_cas_ptr machine_rw_barrier
 atomics_win32_if=MemoryBarrier
 atomics_native_if_any=$ATOMICS_LIST
@@ -4673,6 +4674,7 @@ if ! disabled network; then
 fi
 
 check_builtin atomic_cas_ptr atomic.h void **ptr; void *oldval, *newval; 
atomic_cas_ptr(ptr, oldval, newval)
+check_builtin atomic_compare_exchange  int *ptr, *oldval; int newval; 
__atomic_compare_exchange_n(ptr, oldval, newval, 0, __ATOMIC_SEQ_CST, 
__ATOMIC_SEQ_CST)
 check_builtin machine_rw_barrier mbarrier.h __machine_rw_barrier()
 check_builtin MemoryBarrier windows.h MemoryBarrier()
 check_builtin sarestart signal.h SA_RESTART
diff --git a/libavutil/atomic_gcc.h b/libavutil/atomic_gcc.h
index 2bb43c3..4b0e425 100644
--- a/libavutil/atomic_gcc.h
+++ b/libavutil/atomic_gcc.h
@@ -28,28 +28,43 @@
 #define avpriv_atomic_int_get atomic_int_get_gcc
 static inline int atomic_int_get_gcc(volatile int *ptr)
 {
+#if HAVE_ATOMIC_COMPARE_EXCHANGE
+return __atomic_load_n(ptr, __ATOMIC_SEQ_CST);
+#else
 __sync_synchronize();
 return *ptr;
+#endif
 }
 
 #define avpriv_atomic_int_set atomic_int_set_gcc
 static inline void atomic_int_set_gcc(volatile int *ptr, int val)
 {
+#if HAVE_ATOMIC_COMPARE_EXCHANGE
+__atomic_store_n(ptr, val, __ATOMIC_SEQ_CST);
+#else
 *ptr = val;
 __sync_synchronize();
+#endif
 }
 
 #define avpriv_atomic_int_add_and_fetch atomic_int_add_and_fetch_gcc
 static inline int atomic_int_add_and_fetch_gcc(volatile int *ptr, int inc)
 {
+#if HAVE_ATOMIC_COMPARE_EXCHANGE
+return __atomic_add_fetch(ptr, inc, __ATOMIC_SEQ_CST);
+#else
 return __sync_add_and_fetch(ptr, inc);
+#endif
 }
 
 #define avpriv_atomic_ptr_cas atomic_ptr_cas_gcc
 static inline void *atomic_ptr_cas_gcc(void * volatile *ptr,
void *oldval, void *newval)
 {
-#ifdef __ARMCC_VERSION
+#if HAVE_ATOMIC_COMPARE_EXCHANGE
+__atomic_compare_exchange_n(ptr, oldval, newval, 0, __ATOMIC_SEQ_CST, 
__ATOMIC_SEQ_CST);
+return oldval;
+#elif defined(__ARMCC_VERSION)
 // armcc will throw an error if ptr is not an integer type
 volatile uintptr_t *tmp = (volatile uintptr_t*)ptr;