Re: [FFmpeg-devel] [PATCH] Use v4l2 input format automatically if filename starts with "/dev/video"
2014-10-25 2:28 GMT+04:00 Timothy Gu : > 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"
On Fri, 24 Oct 2014 19:50:25 +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; > +} > + > 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"
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
On Fri, Oct 24, 2014 at 11:31:14PM +0200, Lukasz Marek wrote: > Signed-off-by: Lukasz Marek > --- > 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"
On Sat, 25 Oct 2014 11:43:12 +0200 Nicolas George 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"
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"
On Sat, Oct 25, 2014 at 01:10:20PM +0200, wm4 wrote: > On Sat, 25 Oct 2014 11:43:12 +0200 > Nicolas George 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
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
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
--- 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
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
>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
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 --- a/tests/ref/v
[FFmpeg-devel] [PATCH 6/6] dv: move inverse weight tables to decoder
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,
Re: [FFmpeg-devel] [PATCH 0/6] dv: of inverse weight tables
2014-10-25 13:19 GMT+02:00 Christophe Gisquet : > 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 #include #include #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; i<64; 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; j<8; j++) { printf(" %1.8f", w[j]); } printf("\n"); for(order = 0, j=0; j<8; j++) { for(i=0; i<8; 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(v<4) 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 0/6] dv: of inverse weight tables
On Sat, Oct 25, 2014 at 01:21:47PM +0200, Christophe Gisquet wrote: > 2014-10-25 13:19 GMT+02:00 Christophe Gisquet : > > 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). Maybe as a patch that puts it into tools? I don't know if that's the right place, but I would kind of prefer to have these kind of things around. Probably overkill, but we have libavcodec/tableprint.h that has helpers to generate files with tables. ___ 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
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
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"
On Sat, 25 Oct 2014 13:14:26 +0200 Reimar Döffinger wrote: > On Sat, Oct 25, 2014 at 01:10:20PM +0200, wm4 wrote: > > On Sat, 25 Oct 2014 11:43:12 +0200 > > Nicolas George 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"
On Sat, Oct 25, 2014 at 01:40:11PM +0200, wm4 wrote: > On Sat, 25 Oct 2014 13:14:26 +0200 > Reimar Döffinger 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
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"
On Sat, 25 Oct 2014 13:50:58 +0200 Reimar Döffinger 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 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
Clément Bœsch 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 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"
Reimar Döffinger 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"
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"
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"
On Sat, Oct 25, 2014 at 12:35:11PM +, Carl Eugen Hoyos wrote: > Reimar Döffinger 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"
On Sat, 25 Oct 2014 14:43:39 +0200 Nicolas George 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"
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"
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"
On Sat, Oct 25, 2014 at 03:08:10PM +0200, wm4 wrote: > On Sat, 25 Oct 2014 14:43:39 +0200 > Nicolas George 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"
Reimar Döffinger 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"
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.
Updated patch fixing off-by-one in device listing. -Thilo >From d8dc49423dbcdadf739997c453204e137ed8c088 Mon Sep 17 00:00:00 2001 From: Thilo Borgmann 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 --- 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 coun
Re: [FFmpeg-devel] [PATCH 2/6] dv: use smaller type for weight tables
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
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 13:35 GMT+02:00 Reimar Döffinger : > 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 4/6] dv: fix weight table for 2x4x8 transform
On Sat, Oct 25, 2014 at 05:35:37PM +0200, Christophe Gisquet wrote: > 2014-10-25 13:35 GMT+02:00 Reimar Döffinger : > > 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? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter
On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer 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 > > 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 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 + * + * 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=10&t=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 +**/ +st
Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter
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 wrote: > > > On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer > 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 >> > 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
On Sat, Oct 25, 2014 at 10:14 PM, arwa arif 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 > wrote: > >> >> >> On Sat, Oct 25, 2014 at 1:01 AM, Michael Niedermayer >> 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 >>> > 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 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 + * + * 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=10&t=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->da
Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter
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
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
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 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 #include "libavutil/opt.h" #inclu
Re: [FFmpeg-devel] [PATCH] lavfi: add xbr filter
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 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 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=6&t=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 + * + * 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
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] doc/fftools-common-opts: document -devices option
On 25.10.2014 11:56, Michael Niedermayer wrote: On Fri, Oct 24, 2014 at 11:31:14PM +0200, Lukasz Marek wrote: Signed-off-by: Lukasz Marek --- doc/fftools-common-opts.texi | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) LGTM Pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] opts: add list device sources/sinks options
On 24.10.2014 23:13, Michael Niedermayer wrote: +The returned list cannot be assumed to be always completed. complete +The returned list cannot be assumed to be always completed. complete Both changed i wonder if some of the code can be factorized/simplifed but either way it LGTM I know the code looks similar, but these small differences make it hard to really simplify (making common function that is longer and more complex than 2 is not really an improvement imho) @@ -2136,7 +2136,7 @@ static int print_device_sinks(AVOutputFormat *fmt, AVDictionary *opts) fail: av_dict_free(&tmp_opts); avdevice_free_list_devices(&device_list); -avformat_close_input(&dev); +avformat_free_context(dev); return ret; } Pushed with this small local fix. ___ 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
Hi, 2014-10-25 18:25 GMT+02:00 Reimar Döffinger : > On Sat, Oct 25, 2014 at 05:35:37PM +0200, Christophe Gisquet wrote: >> 2014-10-25 13:35 GMT+02:00 Reimar Döffinger : >> > 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
On Fri, Oct 24, 2014 at 12:35:33AM +0200, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer > --- > 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()
This fixes the issue that a not set or not forwarded whitelist would allow to bypass it. Signed-off-by: Michael Niedermayer --- 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()
On Sat, 25 Oct 2014 21:51:25 +0200 Michael Niedermayer wrote: > This fixes the issue that a not set or not forwarded whitelist > would allow to bypass it. > > Signed-off-by: Michael Niedermayer > --- > 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; More global state... > > 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->nam
Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()
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] libavformat/mxfdec: read source timecode from pulldown component
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? 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 signature.asc Description: This is a digitally signed message part ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavd/avfoundation: Add support for screen capturing.
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 > 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 > --- > 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
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()
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()
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
On Sat, Oct 25, 2014 at 1:42 PM, 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 > 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] lavfi: add xbr filter
On Sat, Oct 25, 2014 at 11:32:43PM +0530, arwa arif wrote: > 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 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 > > > > > doc/filters.texi |7 + > libavfilter/Makefile |1 > libavfilter/allfilters.c |1 > libavfilter/vf_xbr.c | 303 > +++ > 4 files changed, 312 insertions(+) > a92794344ddf0d5d4ea3205091ee4a19699c5c06 > 0001-PATCH-lavfi-add-xbr-filter.patch > From cf3a7a6bcb9735c6f6157f80da208c1c191b3e02 Mon Sep 17 00:00:00 2001 > From: Arwa Arif > 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 I tried the filter but it segfaults ./ffplay matrixbench_mpeg2.mpg -vf xbr Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffcfff7700 (LWP 14880)] 0x004b40c5 in d (in=0x7fffc85eb000, x1=-1, y1=576, x2=0, y2=577) at libavfilter/vf_xbr.c:49 49 int r2 = *(in->data[0] + y2 * in->linesize[0] + x2*3); (gdb) bt #0 0x004b40c5 in d (in=0x7fffc85eb000, x1=-1, y1=576, x2=0, y2=577) at libavfilter/vf_xbr.c:49 #1 0x004b49db in apply_edge_detection_rules (in=0x7fffc85eb000, out=0x7fffc85ead20, x=0, y=575) at libavfilter/vf_xbr.c:159 #2 0x004b5173 in filter_frame (inlink=0x7fffc85a7800, in=0x7fffc85eb000) at libavfilter/vf_xbr.c:270 #3 0x00444270 in ff_filter_frame_framed (link=0x7fffc85a7800, frame=0x7fffc85eb000) at libavfilter/avfilter.c:1103 #4 0x004447c4 in ff_filter_frame (link=0x7fffc85a7800, frame=0x7fffc85eb000) at libavfilter/avfilter.c:1183 #5 0x004a4aff in filter_frame (link=0x7fffc85a70a0, in=0x0) at libavfilter/vf_scale.c:530 #6 0x00444270 in ff_filter_frame_framed (link=0x7fffc85a70a0, frame=0x7fffc85ead20) at libavfilter/avfilter.c:1103 #7 0x004447c4 in ff_filter_frame (link=0x7fffc85a70a0, frame=0x7fffc85ead20) at libavfilter/avfilter.c:1183 #8 0x0044b1e3 in request_frame (link=0x7fffc85a70a0) at libavfilter/buffersrc.c:499 #9 0x0044212b in ff_request_frame (link=0x7fffc85a70a0) at libavfilter/avfilter.c:351 #10 0x00442159 in ff_request_frame (link=0x7fffc85a7800) at libavfilter/avfilter.c:353 #11 0x00442159 in ff_request_frame (link=0x7fffc85a71e0) at libavfilter/avfilter.c:353 #12 0x00442159 in ff_request_frame (link=0x7fffc85a7680) at libavfilter/avfilter.c:353 #13 0x00442159 in ff_request_frame (link=0x7fffc85a6c00) at libavfilter/avfilter.c:353 #14 0x00448f1e in av_buffersink_get_frame_flags (ctx=0x7fffc85a6220, frame=0x7fffc80008c0, flags=0) at libavfilter/buffersink.c:137 #15 0x0041f526 in video_thread (arg=0x7fffeb1a7040) at ffplay.c:2168 #16 0x764a6fd5 in ?? () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0 #17 0x764ea999 in ?? () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0 #18 0x7006ae9a in start_thread (arg=0x7fffcfff7700) at pthread_create.c:308 #19 0x7fffefd9831d in clon
Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()
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()
On Sun, 26 Oct 2014 00:16:26 +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? That's exactly the same idea I suggested some hours ago on IRC - so it's not good, I guess? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Question about edit-list+fragment explanation from " [PATCH] mov.c: read fragment start dts from fragmented mp4"
Sorry I am not posting to the same thread directly, I joined the list after this discussion finished a few weeks ago. I was reading Yusuke's explanation (pasted below) regarding the relationship between PTS, CTS, and DTS in the context of fragments, and there are some parts that I don't understand or don't agree with (I am also a little new to these concepts). The part where I get confused is: The presentation starts with CTS=2002 because of media_time of the second edit, so the PTS of the sync sample corresponds to X - 1001. >*From this, X is equal to 49001, and the DTS of trun.sample[0] is equal to X *- trun.sample[0].sample_duration = 49001 - 1001 = 48000. It looks like X is supposed to represent the DTS of the sync-sample. I see that (1001 + X) - 2002 = (X - 1001) represents the amount of time, in mdhd.timescale, into the presentation of actual media. Earlier, we had computed that the 48000 'time' from tfra corresponded to 600 into the presentation of actual media, in mvhd.timescale. So the way I would compute X would be: 600*mdhd.timescale/mvhd.timescale = X-1001 So X = 600*24000/600 + 1001 = 25001 Which does not agree with the original calculation of X = 49001. Do I have a misunderstanding? Thanks, Bryan >>* Would you mind explaining how edit lists and fragment times are *>* supposed to work together? *>>The tfra box is designed as the player seeks and finds sync sample on presentation timeline i.e. by PTS in units of mdhd.timescale. PTS comes from CTS via edit list, and CTS comes from DTS. So, basically you can't get DTS directly from the 'time' field in the tfra box. Let's say mvhd.timescale=600, mdhd.timescale=24000 and the edit list contains two edits (edit[0] and edit[1]), edit[0] = {segment_duration=600, media_time=-1, media_rate=1}; // empty edit edit[1] = {segment_duration=1200, media_time=2002, media_rate=1}; and the track fragment run in a track fragment which you get from an entry in the tfra box, where 'time' in that entry is equal to 48000 is as follows. trun.sample[0].sample_is_non_sync_sample = 1 trun.sample[0].sample_duration=1001 trun.sample[0].sample_composition_time_offset=1001 trun.sample[1].sample_is_non_sync_sample = 0 trun.sample[1].sample_duration=1001 trun.sample[1].sample_composition_time_offset=1001 Then, time/mdhd.timescale*mvhd.timescale=1200, that is, the PTS of the sync sample is equal to 1200 in mvhd.timescale. And, the first edit is an empty edit, so the presentation of actual media starts with 1200 - 600 = 600 in mvhd.timescale. The CTS of the second sample in the trun.sample[1] is equal to 1001 + X, where the X is the sum of the duration of all preceding samples. The presentation starts with CTS=2002 because of media_time of the second edit, so the PTS of the sync sample corresponds to X - 1001. >*From this, X is equal to 49001, and the DTS of trun.sample[0] is equal to X *- trun.sample[0].sample_duration = 49001 - 1001 = 48000. |*<--edit[0]-->|<-edit[1]->| *|*-|-|-|>presentation timeline *0 D T' |-|-->composition timeline media_timeT |-|---|-|-->decode timeline 0 media_time X T |<--->| ct_offset D = edit[0].segment_duration = 600 T' = time/mdhd.timescale*mvhd.timescale = 1200 media_time = edit[1].media_time = 1001 ct_offset = trun.sample[1].sample_composition_time_offset = 1001 T=ct_offset + X T-media_time = (T'-D)*mdhd.timescale/mvhd.timescale ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] add av_enable_strict_whitelists()
On Sun, Oct 26, 2014 at 02:09:19AM +0200, wm4 wrote: > On Sun, 26 Oct 2014 00:16:26 +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? > > 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
__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 --- 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 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; return