[FFmpeg-devel] [PATCH 5/5] ffmpeg: add abort_on option to allow aborting on empty output
Signed-off-by: Marton Balint--- doc/ffmpeg.texi | 8 ffmpeg.c| 7 +++ ffmpeg.h| 3 +++ ffmpeg_opt.c| 21 + 4 files changed, 39 insertions(+) diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index de49618..022304d 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1236,6 +1236,14 @@ Discard all frames excepts keyframes. Discard all frames. @end table +@item -abort_on @var{flags} (@emph{global}) +Stop and abort on various conditions. The following flags are available: + +@table @option +@item empty_output +No packets were passed to the muxer, the output is empty. +@end table + @item -xerror (@emph{global}) Stop and exit on error diff --git a/ffmpeg.c b/ffmpeg.c index f6947f8..18b8c65 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -4049,6 +4049,7 @@ static int transcode(void) OutputStream *ost; InputStream *ist; int64_t timer_start; +int64_t total_packets_written = 0; ret = transcode_init(); if (ret < 0) @@ -4129,6 +4130,12 @@ static int transcode(void) if (ost->encoding_needed) { av_freep(>enc_ctx->stats_in); } +total_packets_written += ost->packets_written; +} + +if (!total_packets_written && (abort_on_flags & ABORT_ON_FLAG_EMPTY_OUTPUT)) { +av_log(NULL, AV_LOG_FATAL, "Empty output\n"); +exit_program(1); } /* close each decoder */ diff --git a/ffmpeg.h b/ffmpeg.h index 5722816..7844b9d 100644 --- a/ffmpeg.h +++ b/ffmpeg.h @@ -382,6 +382,8 @@ enum forced_keyframes_const { FKF_NB }; +#define ABORT_ON_FLAG_EMPTY_OUTPUT (1 << 0) + extern const char *const forced_keyframes_const_names[]; typedef enum { @@ -524,6 +526,7 @@ extern int start_at_zero; extern int copy_tb; extern int debug_ts; extern int exit_on_error; +extern int abort_on_flags; extern int print_stats; extern int qp_hist; extern int stdin_interaction; diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c index 7139114..4dd3a18 100644 --- a/ffmpeg_opt.c +++ b/ffmpeg_opt.c @@ -103,6 +103,7 @@ int start_at_zero = 0; int copy_tb = -1; int debug_ts = 0; int exit_on_error = 0; +int abort_on_flags= 0; int print_stats = -1; int qp_hist = 0; int stdin_interaction = 1; @@ -196,6 +197,24 @@ static AVDictionary *strip_specifiers(AVDictionary *dict) return ret; } +static int opt_abort_on(void *optctx, const char *opt, const char *arg) +{ +static const AVOption opts[] = { +{ "abort_on", NULL, 0, AV_OPT_TYPE_FLAGS, { .i64 = 0 }, INT64_MIN, INT64_MAX, .unit = "flags" }, +{ "empty_output", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = ABORT_ON_FLAG_EMPTY_OUTPUT },.unit = "flags" }, +{ NULL }, +}; +static const AVClass class = { +.class_name = "", +.item_name = av_default_item_name, +.option = opts, +.version= LIBAVUTIL_VERSION_INT, +}; +const AVClass *pclass = + +return av_opt_eval_flags(, [0], arg, _on_flags); +} + static int opt_sameq(void *optctx, const char *opt, const char *arg) { av_log(NULL, AV_LOG_ERROR, "Option '%s' was removed. " @@ -3120,6 +3139,8 @@ const OptionDef options[] = { "timestamp error delta threshold", "threshold" }, { "xerror", OPT_BOOL | OPT_EXPERT, { _on_error }, "exit on error", "error" }, +{ "abort_on", HAS_ARG | OPT_EXPERT,{ .func_arg = opt_abort_on }, +"abort on the specified condition flags", "flags" }, { "copyinkf", OPT_BOOL | OPT_EXPERT | OPT_SPEC | OPT_OUTPUT, { .off = OFFSET(copy_initial_nonkeyframes) }, "copy initial non-keyframes" }, -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/5] ffmpeg: exit on av_write_trailer failure if exit_on_error is set
Signed-off-by: Marton Balint--- ffmpeg.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ffmpeg.c b/ffmpeg.c index 95f7e2f..94dfb04 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -4106,6 +4106,8 @@ static int transcode(void) os = output_files[i]->ctx; if ((ret = av_write_trailer(os)) < 0) { av_log(NULL, AV_LOG_ERROR, "Error writing trailer: %s", av_err2str(ret)); +if (exit_on_error) +exit_program(1); } } -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avformat/vag: Remove unused variable pos
On 10/17/15, Michael Niedermayerwrote: > From: Michael Niedermayer > > Signed-off-by: Michael Niedermayer > --- > libavformat/vag.c |2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavformat/vag.c b/libavformat/vag.c > index 2209711..d3cd5ba 100644 > --- a/libavformat/vag.c > +++ b/libavformat/vag.c > @@ -34,7 +34,6 @@ static int vag_probe(AVProbeData *p) > static int vag_read_header(AVFormatContext *s) > { > AVStream *st; > -int64_t pos; > > st = avformat_new_stream(s, NULL); > if (!st) > @@ -53,7 +52,6 @@ static int vag_read_header(AVFormatContext *s) > st->codec->sample_rate = avio_rb32(s->pb); > if (st->codec->sample_rate <= 0) > return AVERROR_INVALIDDATA; > -pos = avio_tell(s->pb); > avio_seek(s->pb, 0x1000, SEEK_SET); > if (avio_rl32(s->pb) == MKTAG('V','A','G','p')) { > st->codec->block_align = 0x1000 * st->codec->channels; > -- > 1.7.9.5 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ok ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avformat/genh: Remove unused array coef_splitted
On 10/17/15, Michael Niedermayerwrote: > From: Michael Niedermayer > > Signed-off-by: Michael Niedermayer > --- > libavformat/genh.c |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/libavformat/genh.c b/libavformat/genh.c > index 3a4faf9..5d34491 100644 > --- a/libavformat/genh.c > +++ b/libavformat/genh.c > @@ -40,7 +40,6 @@ static int genh_read_header(AVFormatContext *s) > { > unsigned start_offset, header_size, codec, coef_type, coef[2]; > GENHDemuxContext *c = s->priv_data; > -unsigned coef_splitted[2]; > int align, ch; > AVStream *st; > > @@ -105,8 +104,8 @@ static int genh_read_header(AVFormatContext *s) > coef[1] = avio_rl32(s->pb); > c->dsp_int_type = avio_rl32(s->pb); > coef_type= avio_rl32(s->pb); > -coef_splitted[0] = avio_rl32(s->pb); > -coef_splitted[1] = avio_rl32(s->pb); > +/*coef_splitted[0] =*/ avio_rl32(s->pb); > +/*coef_splitted[1] =*/ avio_rl32(s->pb); > > if (st->codec->codec_id == AV_CODEC_ID_ADPCM_THP) { > if (st->codec->channels > 2) { > -- > 1.7.9.5 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > I don't see point in doing this, when I find files that make use of those variables I will change it agaian. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/5] ffmpeg: exit on av_write_trailer failure if exit_on_error is set
On Sun, Oct 18, 2015 at 12:38:51AM +0200, Michael Niedermayer wrote: > On Sun, Oct 18, 2015 at 12:24:04AM +0200, Marton Balint wrote: > > Signed-off-by: Marton Balint> > --- > > ffmpeg.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/ffmpeg.c b/ffmpeg.c > > index 95f7e2f..94dfb04 100644 > > --- a/ffmpeg.c > > +++ b/ffmpeg.c > > @@ -4106,6 +4106,8 @@ static int transcode(void) > > os = output_files[i]->ctx; > > if ((ret = av_write_trailer(os)) < 0) { > > av_log(NULL, AV_LOG_ERROR, "Error writing trailer: %s", > > av_err2str(ret)); > > +if (exit_on_error) > > +exit_program(1); > > i wonder if ffmpeg should attempt to write all files trailers before > exiting either way this LGTM [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avfilter/internal: Doxygen for ff_fmt_is_in
On Sat, Oct 17, 2015 at 6:44 PM, Ganesh Ajjanagaddewrote: > On Sat, Oct 17, 2015 at 6:34 PM, Timothy Gu wrote: >> On Sat, Oct 17, 2015 at 7:59 AM Ganesh Ajjanagadde >> wrote: >>> >>> On Thu, Oct 15, 2015 at 6:26 AM, Ganesh Ajjanagadde >>> wrote: >>> > On Thu, Oct 15, 2015 at 1:45 AM, Timothy Gu >>> > wrote: >>> >> On Wed, Oct 14, 2015 at 8:05 PM Ganesh Ajjanagadde >>> >> >>> >> wrote: >>> >>> This clarifies and Doxygen's the comment for ff_fmt_is_in. >>> >> >>> >> ^ >>> >> gajjanag.c:1:23: error: unexpected token ‘adjective’ >>> > >>> > sorry, missing something - does "This clarifies and adds Doxygen for >>> > ff_fmt_is_in" fix it? >> >> >> Yes. >> >>> >>> Alternatively, can you tell me what tool >>> you used so that I can check locally? >> >> >> I'm pretty sure you already have the tool I used. It's something *very* >> local to you, especially to the part of you that is reading this sentence ;) > > Ah, just a English language check - I thought of something else due to > the caret underneath the word, etc. I will update and push later > sometime. Thanks. pushed > >> >> Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 6/6] avutil/des: use EINVAL instead of -1 for the return code of av_des_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/des.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/des.c b/libavutil/des.c index c97158a..3ccbf89 100644 --- a/libavutil/des.c +++ b/libavutil/des.c @@ -292,7 +292,7 @@ AVDES *av_des_alloc(void) int av_des_init(AVDES *d, const uint8_t *key, int key_bits, av_unused int decrypt) { if (key_bits != 64 && key_bits != 192) -return -1; +return AVERROR(EINVAL); d->triple_des = key_bits > 64; gen_roundkeys(d->round_keys[0], AV_RB64(key)); if (d->triple_des) { -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/6] avutil/twofish: use EINVAL instead of -1 for the return code of av_twofish_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/twofish.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/twofish.c b/libavutil/twofish.c index f735a1f..162069b 100644 --- a/libavutil/twofish.c +++ b/libavutil/twofish.c @@ -273,7 +273,7 @@ av_cold int av_twofish_init(AVTWOFISH *cs, const uint8_t *key, int key_bits) uint32_t Key[8], Me[4], Mo[4], A, B; const uint32_t rho = 0x01010101; if (key_bits < 0) -return -1; +return AVERROR(EINVAL); if (key_bits <= 128) { cs->ksize = 2; } else if (key_bits <= 192) { -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/6] avutil/des: use EINVAL instead of -1 for the return code of av_des_init()
On Sat, Oct 17, 2015 at 7:39 PM, Ganesh Ajjanagaddewrote: > Signed-off-by: Ganesh Ajjanagadde > --- > libavutil/des.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/des.c b/libavutil/des.c > index c97158a..3ccbf89 100644 > --- a/libavutil/des.c > +++ b/libavutil/des.c > @@ -292,7 +292,7 @@ AVDES *av_des_alloc(void) > > int av_des_init(AVDES *d, const uint8_t *key, int key_bits, av_unused int > decrypt) { > if (key_bits != 64 && key_bits != 192) > -return -1; > +return AVERROR(EINVAL); > d->triple_des = key_bits > 64; > gen_roundkeys(d->round_keys[0], AV_RB64(key)); > if (d->triple_des) { > -- > 2.6.1 > I can squash these if people prefer, or leave as is. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 5/6] avutil/rc4: use EINVAL instead of -1 for the return code of av_rc4_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/rc4.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/rc4.c b/libavutil/rc4.c index 6bd702c..ffcb112 100644 --- a/libavutil/rc4.c +++ b/libavutil/rc4.c @@ -36,7 +36,7 @@ int av_rc4_init(AVRC4 *r, const uint8_t *key, int key_bits, int decrypt) { uint8_t *state = r->state; int keylen = key_bits >> 3; if (key_bits & 7) -return -1; +return AVERROR(EINVAL); for (i = 0; i < 256; i++) state[i] = i; y = 0; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] vc1dsp: Port ff_vc1_put_ver_16b_shift2_mmx to yasm
This function is only used within other inline asm functions, hence the HAVE_MMX_INLINE guard. Per recent discussions, we should not worry about the performance of inline asm-only builds. --- The conversion process has to start _somewhere_... Difference from previous version: use mova/h, correct typo, and fix %if guard position. --- libavcodec/x86/vc1dsp.asm | 89 - libavcodec/x86/vc1dsp_mmx.c | 59 -- 2 files changed, 95 insertions(+), 53 deletions(-) diff --git a/libavcodec/x86/vc1dsp.asm b/libavcodec/x86/vc1dsp.asm index 546688c..729bce7 100644 --- a/libavcodec/x86/vc1dsp.asm +++ b/libavcodec/x86/vc1dsp.asm @@ -1,5 +1,6 @@ ;** -;* VC1 deblocking optimizations +;* VC1 DSP optimizations +;* Copyright (c) 2007 Christophe GISQUET;* Copyright (c) 2009 David Conrad ;* ;* This file is part of FFmpeg. @@ -23,6 +24,7 @@ cextern pw_4 cextern pw_5 +cextern pw_9 section .text @@ -315,3 +317,88 @@ cglobal vc1_h_loop_filter8, 3,5,8 START_H_FILTER 8 VC1_H_LOOP_FILTER 8 RET + +%if HAVE_MMX_INLINE +%macro NORMALIZE_MMX 1 ; shift +paddw m3, m7 ; +bias-r +paddw m4, m7 ; +bias-r +psraw m3, %1 +psraw m4, %1 +%endmacro + +; Compute the rounder 32-r or 8-r and unpacks it to m7 +%macro LOAD_ROUNDER_MMX 1 ; round +movd m7, %1 +punpcklwd m7, m7 +punpckldq m7, m7 +%endmacro + +%macro SHIFT2_LINE 5 ; off, r0, r1, r2, r3 +paddw m%3, m%4 +movh m%2, [srcq + stride_neg2] +pmullw m%3, m6 +punpcklbw m%2, m0 +movh m%5, [srcq + strideq] +psubw m%3, m%2 +punpcklbw m%5, m0 +paddw m%3, m7 +psubw m%3, m%5 +psraw m%3, shift +movu [dstq + %1], m%3 +add srcq, strideq +%endmacro + +INIT_MMX mmx +; void ff_vc1_put_ver_16b_shift2_mmx(int16_t *dst, const uint8_t *src, +;x86_reg stride, int rnd, int64_t shift) +; Sacrificing m6 makes it possible to pipeline loads from src +%if ARCH_X86_32 +cglobal vc1_put_ver_16b_shift2, 3,6,0, dst, src, stride +DECLARE_REG_TMP 3, 4, 5 +%define rnd r3mp +%define shift qword r4m +%else ; X86_64 +cglobal vc1_put_ver_16b_shift2, 4,7,0, dst, src, stride +DECLARE_REG_TMP 4, 5, 6 +%define rnd r3d +; We need shift either in memory or in a mm reg as it's used in psraw +; On Windows, the arg is already on the stack +; On SysV, m5 doesn't seem to be used +%if WIN64 +%define shift r4mp +%else ; UNIX64 +%define shift m5 +mova shift, r4q +%endif ; WIN64 +%endif ; X86_32 +%define stride_neg2 t0q +%define stride_9minus4 t1q +%define i t2q +mov stride_neg2, strideq +neg stride_neg2 +add stride_neg2, stride_neg2 +leastride_9minus4, [strideq * 9 - 4] +mov i, 3 +LOAD_ROUNDER_MMX rnd +mova m6, [pw_9] +pxor m0, m0 +.loop: +movh m2, [srcq] +add srcq, strideq +movh m3, [srcq] +punpcklbw m2, m0 +punpcklbw m3, m0 +SHIFT2_LINE 0, 1, 2, 3, 4 +SHIFT2_LINE24, 2, 3, 4, 1 +SHIFT2_LINE48, 3, 4, 1, 2 +SHIFT2_LINE72, 4, 1, 2, 3 +SHIFT2_LINE96, 1, 2, 3, 4 +SHIFT2_LINE 120, 2, 3, 4, 1 +SHIFT2_LINE 144, 3, 4, 1, 2 +SHIFT2_LINE 168, 4, 1, 2, 3 +sub srcq, stride_9minus4 +add dstq, 8 +dec i +jnz .loop +REP_RET +%endif ; HAVE_MMX_INLINE diff --git a/libavcodec/x86/vc1dsp_mmx.c b/libavcodec/x86/vc1dsp_mmx.c index e42099b..c268cc6 100644 --- a/libavcodec/x86/vc1dsp_mmx.c +++ b/libavcodec/x86/vc1dsp_mmx.c @@ -33,7 +33,11 @@ #include "fpel.h" #include "vc1dsp.h" -#if HAVE_6REGS && HAVE_INLINE_ASM +#if HAVE_6REGS && HAVE_INLINE_ASM && HAVE_MMX_EXTERNAL + +void ff_vc1_put_ver_16b_shift2_mmx(int16_t *dst, + const uint8_t *src, x86_reg stride, + int rnd, int64_t shift); #define OP_PUT(S,D) #define OP_AVG(S,D) "pavgb " #S ", " #D " \n\t" @@ -66,55 +70,6 @@ "punpcklwd %%mm7, %%mm7 \n\t"\ "punpckldq %%mm7, %%mm7 \n\t" -#define SHIFT2_LINE(OFF, R0,R1,R2,R3) \ -"paddw %%mm"#R2", %%mm"#R1"\n\t"\ -"movd (%0,%3), %%mm"#R0" \n\t"\ -"pmullw%%mm6, %%mm"#R1"\n\t"\ -"punpcklbw %%mm0, %%mm"#R0"\n\t"\ -"movd (%0,%2), %%mm"#R3" \n\t"\ -"psubw %%mm"#R0", %%mm"#R1"\n\t"\ -"punpcklbw %%mm0, %%mm"#R3"\n\t"\ -"paddw %%mm7, %%mm"#R1"\n\t"\ -"psubw %%mm"#R3", %%mm"#R1"\n\t"\ -"psraw %4, %%mm"#R1"
Re: [FFmpeg-devel] [PATCH 2/4] x86: fpel: Remove erroneous ff_put_pixels8_mmxext prototype
On Sat, Oct 17, 2015 at 04:48:59PM -0700, Timothy Gu wrote: > This function does not exist. > --- > libavcodec/x86/fpel.h | 2 -- > 1 file changed, 2 deletions(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/5] ffmpeg: exit on av_write_trailer failure if exit_on_error is set
On Sun, Oct 18, 2015 at 12:24:04AM +0200, Marton Balint wrote: > Signed-off-by: Marton Balint> --- > ffmpeg.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/ffmpeg.c b/ffmpeg.c > index 95f7e2f..94dfb04 100644 > --- a/ffmpeg.c > +++ b/ffmpeg.c > @@ -4106,6 +4106,8 @@ static int transcode(void) > os = output_files[i]->ctx; > if ((ret = av_write_trailer(os)) < 0) { > av_log(NULL, AV_LOG_ERROR, "Error writing trailer: %s", > av_err2str(ret)); > +if (exit_on_error) > +exit_program(1); i wonder if ffmpeg should attempt to write all files trailers before exiting [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] compat/solaris/make_sunver.pl: Use /usr/bin/env perl instead of /usr/bin/perl
From: Michael NiedermayerThis is how the other perl scripts in git call perl Signed-off-by: Michael Niedermayer --- compat/solaris/make_sunver.pl |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/solaris/make_sunver.pl b/compat/solaris/make_sunver.pl index 929bdda..0e9ed1d 100755 --- a/compat/solaris/make_sunver.pl +++ b/compat/solaris/make_sunver.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl # make_sunver.pl # -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] dnxhdenc: Optimize get_pixels_8x4_sym for 10-bit
This reverts commit 628e6d0164febc8e69b0f10dfa487e8a2dd1a28a and uses a better fix. Before: 4483 decicycles in get_pixels_8x4_sym, 131032 runs, 40 skips After: 2569 decicycles in get_pixels_8x4_sym, 131054 runs, 18 skips --- libavcodec/dnxhdenc.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c index 36154ac..04eb5d5 100644 --- a/libavcodec/dnxhdenc.c +++ b/libavcodec/dnxhdenc.c @@ -87,22 +87,14 @@ void dnxhd_10bit_get_pixels_8x4_sym(int16_t *av_restrict block, const uint8_t *pixels, ptrdiff_t line_size) { -int i; -const uint16_t* pixels16 = (const uint16_t*)pixels; -line_size >>= 1; - -for (i = 0; i < 4; i++) { -block[0] = pixels16[0]; block[1] = pixels16[1]; -block[2] = pixels16[2]; block[3] = pixels16[3]; -block[4] = pixels16[4]; block[5] = pixels16[5]; -block[6] = pixels16[6]; block[7] = pixels16[7]; -pixels16 += line_size; -block += 8; -} -memcpy(block, block - 8, sizeof(*block) * 8); -memcpy(block + 8, block - 16, sizeof(*block) * 8); -memcpy(block + 16, block - 24, sizeof(*block) * 8); -memcpy(block + 24, block - 32, sizeof(*block) * 8); +memcpy(block + 0 * 8, pixels, 8 * sizeof(*block)); +memcpy(block + 7 * 8, pixels, 8 * sizeof(*block)); +memcpy(block + 1 * 8, pixels + 1 * line_size, 8 * sizeof(*block)); +memcpy(block + 6 * 8, pixels + 1 * line_size, 8 * sizeof(*block)); +memcpy(block + 2 * 8, pixels + 2 * line_size, 8 * sizeof(*block)); +memcpy(block + 5 * 8, pixels + 2 * line_size, 8 * sizeof(*block)); +memcpy(block + 3 * 8, pixels + 3 * line_size, 8 * sizeof(*block)); +memcpy(block + 4 * 8, pixels + 3 * line_size, 8 * sizeof(*block)); } static int dnxhd_10bit_dct_quantize(MpegEncContext *ctx, int16_t *block, -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/bitstream: replace qsort with AV_QSORT
Commit 3a0a2f33a6c955823fa4fb12c0b49cd29a496659 claims large performance advantages for AV_QSORT over libc's qsort. The reason is that I suspect that libc's qsort (at least on non LTO builds, like the typical FFmpeg config) can't inline the comparison callback: https://stackoverflow.com/questions/5290695/is-there-any-way-a-c-c-compiler-can-inline-a-c-callback-function. AV_QSORT has two things going for it: 1. The guaranteed inlining of qsort itself. This yields a negligible boost that may be ignored. 2. The more serious possibility of potentially allowing the comparison function to be inlined - this is likely responsible for the large boosts reported. There is a comment explaining that this is a place that could use some performance improvement. Thus AV_QSORT is used to achieve that. Benchmarks deemed unnecessary due to existing claims about AV_QSORT. Tested with FATE. Signed-off-by: Ganesh Ajjanagadde--- libavcodec/bitstream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c index 924cc51..1acb7a3 100644 --- a/libavcodec/bitstream.c +++ b/libavcodec/bitstream.c @@ -30,6 +30,7 @@ #include "libavutil/atomic.h" #include "libavutil/avassert.h" +#include "libavutil/qsort.h" #include "avcodec.h" #include "internal.h" #include "mathops.h" @@ -333,7 +334,7 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int nb_codes, } COPY(buf[j].bits > nb_bits); // qsort is the slowest part of init_vlc, and could probably be improved or avoided -qsort(buf, j, sizeof(VLCcode), compare_vlcspec); +AV_QSORT(buf, j, struct VLCcode, compare_vlcspec); COPY(buf[j].bits && buf[j].bits <= nb_bits); nb_codes = j; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/5] ffmpeg: factorize checking decoder result
Signed-off-by: Marton Balint--- ffmpeg.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/ffmpeg.c b/ffmpeg.c index 94dfb04..3a36af7 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -1925,6 +1925,15 @@ int guess_input_channel_layout(InputStream *ist) return 1; } +static void check_decode_result(int *got_output, int ret) +{ +if (*got_output || ret<0) +decode_error_stat[ret<0] ++; + +if (ret < 0 && exit_on_error) +exit_program(1); +} + static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output) { AVFrame *decoded_frame, *f; @@ -1947,11 +1956,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output) ret = AVERROR_INVALIDDATA; } -if (*got_output || ret<0) -decode_error_stat[ret<0] ++; - -if (ret < 0 && exit_on_error) -exit_program(1); +check_decode_result(got_output, ret); if (!*got_output || ret < 0) return ret; @@ -2088,11 +2093,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output) ist->st->codec->has_b_frames); } -if (*got_output || ret<0) -decode_error_stat[ret<0] ++; - -if (ret < 0 && exit_on_error) -exit_program(1); +check_decode_result(got_output, ret); if (*got_output && ret >= 0) { if (ist->dec_ctx->width != decoded_frame->width || @@ -2200,11 +2201,7 @@ static int transcode_subtitles(InputStream *ist, AVPacket *pkt, int *got_output) int i, ret = avcodec_decode_subtitle2(ist->dec_ctx, , got_output, pkt); -if (*got_output || ret<0) -decode_error_stat[ret<0] ++; - -if (ret < 0 && exit_on_error) -exit_program(1); +check_decode_result(got_output, ret); if (ret < 0 || !*got_output) { if (!pkt->size) -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avfilter/internal: Doxygen for ff_fmt_is_in
On Sat, Oct 17, 2015 at 6:34 PM, Timothy Guwrote: > On Sat, Oct 17, 2015 at 7:59 AM Ganesh Ajjanagadde > wrote: >> >> On Thu, Oct 15, 2015 at 6:26 AM, Ganesh Ajjanagadde >> wrote: >> > On Thu, Oct 15, 2015 at 1:45 AM, Timothy Gu >> > wrote: >> >> On Wed, Oct 14, 2015 at 8:05 PM Ganesh Ajjanagadde >> >> >> >> wrote: >> >>> This clarifies and Doxygen's the comment for ff_fmt_is_in. >> >> >> >> ^ >> >> gajjanag.c:1:23: error: unexpected token ‘adjective’ >> > >> > sorry, missing something - does "This clarifies and adds Doxygen for >> > ff_fmt_is_in" fix it? > > > Yes. > >> >> Alternatively, can you tell me what tool >> you used so that I can check locally? > > > I'm pretty sure you already have the tool I used. It's something *very* > local to you, especially to the part of you that is reading this sentence ;) Ah, just a English language check - I thought of something else due to the caret underneath the word, etc. I will update and push later sometime. Thanks. > > Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/6] avutil/des: use EINVAL instead of -1 for the return code of av_des_init()
On Sat, Oct 17, 2015 at 7:49 PM, Ronald S. Bultjewrote: > Hi, > > On Sat, Oct 17, 2015 at 7:40 PM, Ganesh Ajjanagadde > wrote: >> >> On Sat, Oct 17, 2015 at 7:39 PM, Ganesh Ajjanagadde >> wrote: >> > Signed-off-by: Ganesh Ajjanagadde >> > --- >> > libavutil/des.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/libavutil/des.c b/libavutil/des.c >> > index c97158a..3ccbf89 100644 >> > --- a/libavutil/des.c >> > +++ b/libavutil/des.c >> > @@ -292,7 +292,7 @@ AVDES *av_des_alloc(void) >> > >> > int av_des_init(AVDES *d, const uint8_t *key, int key_bits, av_unused >> > int decrypt) { >> > if (key_bits != 64 && key_bits != 192) >> > -return -1; >> > +return AVERROR(EINVAL); >> > d->triple_des = key_bits > 64; >> > gen_roundkeys(d->round_keys[0], AV_RB64(key)); >> > if (d->triple_des) { >> > -- >> > 2.6.1 >> > >> >> I can squash these if people prefer, or leave as is. > > > Whichever you prefer. All patches in this set LGTM. I think I have missed a few crypto things. Will add them, send out the patches (to get at least another eye on them), and once ok'ed, squash and push. > > Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/4] vc1dsp: Port ff_vc1_put_ver_16b_shift2_mmx to yasm
This function is only used within other inline asm functions, hence the HAVE_MMX_INLINE guard. Per recent discussions, we should not worry about the performance of inline asm-only builds. --- libavcodec/x86/vc1dsp.asm | 89 - libavcodec/x86/vc1dsp_mmx.c | 59 -- 2 files changed, 95 insertions(+), 53 deletions(-) diff --git a/libavcodec/x86/vc1dsp.asm b/libavcodec/x86/vc1dsp.asm index 546688c..87c9191 100644 --- a/libavcodec/x86/vc1dsp.asm +++ b/libavcodec/x86/vc1dsp.asm @@ -1,5 +1,6 @@ ;** -;* VC1 deblocking optimizations +;* VC1 DSP optimizations +;* Copyright (c) 2007 Christophe GISQUET;* Copyright (c) 2009 David Conrad ;* ;* This file is part of FFmpeg. @@ -23,6 +24,7 @@ cextern pw_4 cextern pw_5 +cextern pw_9 section .text @@ -315,3 +317,88 @@ cglobal vc1_h_loop_filter8, 3,5,8 START_H_FILTER 8 VC1_H_LOOP_FILTER 8 RET + +%macro NORMALIZE_MMX 1 ; shift +paddw m3, m7 ; +bias-r +paddw m4, m7 ; +bias-r +psraw m3, %1 +psraw m4, %1 +%endmacro + +; Compute the rounder 32-r or 8-r and unpacks it to m7 +%macro LOAD_ROUNDER_MMX 1 ; round +movd m7, %1 +punpcklwd m7, m7 +punpckldq m7, m7 +%endmacro + +%macro SHIFT2_LINE 5 ; off, r0, r1, r2, r3 +paddw m%3, m%4 +movd m%2, [srcq + stride_neg2] +pmullw m%3, m6 +punpcklbw m%2, m0 +movd m%5, [srcq + strideq] +psubw m%3, m%2 +punpcklbw m%5, m0 +paddw m%3, m7 +psubw m%3, m%5 +psraw m%3, shift +movq [dstq + %1], m%3 +add srcq, strideq +%endmacro + +%if HAVE_MMX_INLINE +INIT_MMX mmx +; void ff_vc1_put_ver_16b_shift2_mmx(int16_t *dst, const uint8_t *src, +;x86_reg stride, int rnd, int64_t shift) +; Sacrificing m6 makes it possible to pipeline loads from src +%if ARCH_X86_32 +cglobal vc1_put_ver_16b_shift2, 3,6,0, dst, src, stride +DECLARE_REG_TMP 3, 4, 5 +%define rnd r3mp +%define shift qword r4m +%else ; X86_64 +cglobal vc1_put_ver_16b_shift2, 4,7,0, dst, src, stride +DECLARE_REG_TMP 4, 5, 6 +%define rnd r3d +; We need shift either in memory or in a mm reg as it's used in psraw +; On Windows, the arg is already on the stack +; On SysV, m5 doesn't seem to be used +%if WIN64 +%define shift r4mp +%else ; UNIX64 +%define shift m5 +mova shift, r4q +%endif ; WIN64 +%endif ; X86_32 +%define stride_neg2 t0q +%define stride_9minus4 t1q +%define i t2q +mov stride_neg2, strideq +neg stride_neg2 +add stride_neg2, stride_neg2 +leastride_9minus4, [strideq * 9 - 4] +mov i, 3 +LOAD_ROUNDER_MMX rnd +mova m6, [pw_9] +pxor m0, m0 +.loop: +movh m2, [srcq] +add srcq, strideq +movh m3, [srcq] +punpcklbw m2, m0 +punpcklbw m3, m0 +SHIFT2_LINE 0, 1, 2, 3, 4 +SHIFT2_LINE24, 2, 3, 4, 1 +SHIFT2_LINE48, 3, 4, 1, 2 +SHIFT2_LINE72, 4, 1, 2, 3 +SHIFT2_LINE96, 1, 2, 3, 4 +SHIFT2_LINE 120, 2, 3, 4, 1 +SHIFT2_LINE 144, 3, 4, 1, 2 +SHIFT2_LINE 168, 4, 1, 2, 3 +sub srcq, stride_9minus4 +add dstq, 8 +dec i +jnz .loop +REP_RET +%endif ; HAVE_MMX_INLINE diff --git a/libavcodec/x86/vc1dsp_mmx.c b/libavcodec/x86/vc1dsp_mmx.c index e42099b..f647e33 100644 --- a/libavcodec/x86/vc1dsp_mmx.c +++ b/libavcodec/x86/vc1dsp_mmx.c @@ -33,7 +33,11 @@ #include "fpel.h" #include "vc1dsp.h" -#if HAVE_6REGS && HAVE_INLINE_ASM +#if HAVE_6REGS && HAVE_INLINE_ASM && HAVE_MMX_EXTERNAL + +void ff_vc1_put_ver_16b_shift2_mmx(int16_t *dst, + const uint8_t *src, x86_reg stride, + int rnd, int64_t shift); #define OP_PUT(S,D) #define OP_AVG(S,D) "pavgb " #S ", " #D " \n\t" @@ -66,55 +70,6 @@ "punpcklwd %%mm7, %%mm7 \n\t"\ "punpckldq %%mm7, %%mm7 \n\t" -#define SHIFT2_LINE(OFF, R0,R1,R2,R3) \ -"paddw %%mm"#R2", %%mm"#R1"\n\t"\ -"movd (%0,%3), %%mm"#R0" \n\t"\ -"pmullw%%mm6, %%mm"#R1"\n\t"\ -"punpcklbw %%mm0, %%mm"#R0"\n\t"\ -"movd (%0,%2), %%mm"#R3" \n\t"\ -"psubw %%mm"#R0", %%mm"#R1"\n\t"\ -"punpcklbw %%mm0, %%mm"#R3"\n\t"\ -"paddw %%mm7, %%mm"#R1"\n\t"\ -"psubw %%mm"#R3", %%mm"#R1"\n\t"\ -"psraw %4, %%mm"#R1" \n\t"\ -"movq %%mm"#R1", "#OFF"(%1) \n\t"\ -"add %2, %0 \n\t" - -/** Sacrificing mm6 makes
[FFmpeg-devel] [PATCH 2/3] avutil/sha: use EINVAL instead of -1 for the return code of av_sha_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/sha.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/sha.c b/libavutil/sha.c index 9963043..748bb9c 100644 --- a/libavutil/sha.c +++ b/libavutil/sha.c @@ -305,7 +305,7 @@ av_cold int av_sha_init(AVSHA *ctx, int bits) ctx->transform = sha256_transform; break; default: -return -1; +return AVERROR(EINVAL); } ctx->count = 0; return 0; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] avutil/camellia: use EINVAL instead of -1 for the return code of av_camellia_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/camellia.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/camellia.c b/libavutil/camellia.c index 483eed2..f21ca12 100644 --- a/libavutil/camellia.c +++ b/libavutil/camellia.c @@ -354,7 +354,7 @@ av_cold int av_camellia_init(AVCAMELLIA *cs, const uint8_t *key, int key_bits) uint64_t Kl[2], Kr[2], Ka[2], Kb[2]; uint64_t D1, D2; if (key_bits != 128 && key_bits != 192 && key_bits != 256) -return -1; +return AVERROR(EINVAL); memset(Kb, 0, sizeof(Kb)); memset(Kr, 0, sizeof(Kr)); cs->key_bits = key_bits; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] pixblockdsp: Use memcpy for get_pixels_16_c
Before: 15543 decicycles in get_pixels, 4193214 runs, 1090 skips After: 5713 decicycles in get_pixels, 8387564 runs, 1044 skips --- libavcodec/pixblockdsp.c | 38 - libavcodec/pixblockdsp_template.c | 40 --- 2 files changed, 33 insertions(+), 45 deletions(-) delete mode 100644 libavcodec/pixblockdsp_template.c diff --git a/libavcodec/pixblockdsp.c b/libavcodec/pixblockdsp.c index 322e1dd..fc21ea4 100644 --- a/libavcodec/pixblockdsp.c +++ b/libavcodec/pixblockdsp.c @@ -23,12 +23,40 @@ #include "avcodec.h" #include "pixblockdsp.h" -#define BIT_DEPTH 16 -#include "pixblockdsp_template.c" -#undef BIT_DEPTH +static void get_pixels_16_c(int16_t *av_restrict block, const uint8_t *pixels, +ptrdiff_t line_size) +{ +line_size /= 2; + +memcpy(block, pixels, 2 * 8); +memcpy(block + 1 * 8, pixels + 1 * line_size, 2 * 8); +memcpy(block + 2 * 8, pixels + 2 * line_size, 2 * 8); +memcpy(block + 3 * 8, pixels + 3 * line_size, 2 * 8); +memcpy(block + 4 * 8, pixels + 4 * line_size, 2 * 8); +memcpy(block + 5 * 8, pixels + 5 * line_size, 2 * 8); +memcpy(block + 6 * 8, pixels + 6 * line_size, 2 * 8); +memcpy(block + 7 * 8, pixels + 7 * line_size, 2 * 8); +} + +static void get_pixels_8_c(int16_t *av_restrict block, const uint8_t *pixels, + ptrdiff_t line_size) +{ +int i; -#define BIT_DEPTH 8 -#include "pixblockdsp_template.c" +/* read the pixels */ +for (i = 0; i < 8; i++) { +block[0] = pixels[0]; +block[1] = pixels[1]; +block[2] = pixels[2]; +block[3] = pixels[3]; +block[4] = pixels[4]; +block[5] = pixels[5]; +block[6] = pixels[6]; +block[7] = pixels[7]; +pixels += line_size; +block += 8; +} +} static void diff_pixels_c(int16_t *av_restrict block, const uint8_t *s1, const uint8_t *s2, int stride) diff --git a/libavcodec/pixblockdsp_template.c b/libavcodec/pixblockdsp_template.c deleted file mode 100644 index d1e9102..000 --- a/libavcodec/pixblockdsp_template.c +++ /dev/null @@ -1,40 +0,0 @@ -/* - * This file is part of FFmpeg. - * - * FFmpeg is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * FFmpeg is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with FFmpeg; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - */ - -#include "bit_depth_template.c" - -static void FUNCC(get_pixels)(int16_t *av_restrict block, const uint8_t *_pixels, - ptrdiff_t line_size) -{ -const pixel *pixels = (const pixel *) _pixels; -int i; - -/* read the pixels */ -for (i = 0; i < 8; i++) { -block[0] = pixels[0]; -block[1] = pixels[1]; -block[2] = pixels[2]; -block[3] = pixels[3]; -block[4] = pixels[4]; -block[5] = pixels[5]; -block[6] = pixels[6]; -block[7] = pixels[7]; -pixels += line_size / sizeof(pixel); -block += 8; -} -} -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/5] ffmpeg: log failed av_write_trailer
Signed-off-by: Marton Balint--- ffmpeg.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ffmpeg.c b/ffmpeg.c index 36a68fb..95f7e2f 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -4104,7 +4104,9 @@ static int transcode(void) /* write the trailer if needed and close file */ for (i = 0; i < nb_output_files; i++) { os = output_files[i]->ctx; -av_write_trailer(os); +if ((ret = av_write_trailer(os)) < 0) { +av_log(NULL, AV_LOG_ERROR, "Error writing trailer: %s", av_err2str(ret)); +} } /* dump report by using the first video and audio streams */ -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avfilter/internal: Doxygen for ff_fmt_is_in
On Sat, Oct 17, 2015 at 7:59 AM Ganesh Ajjanagaddewrote: > On Thu, Oct 15, 2015 at 6:26 AM, Ganesh Ajjanagadde > wrote: > > On Thu, Oct 15, 2015 at 1:45 AM, Timothy Gu > wrote: > >> On Wed, Oct 14, 2015 at 8:05 PM Ganesh Ajjanagadde < > gajjanaga...@gmail.com> > >> wrote: > >>> This clarifies and Doxygen's the comment for ff_fmt_is_in. > >> > >> ^ > >> gajjanag.c:1:23: error: unexpected token ‘adjective’ > > > > sorry, missing something - does "This clarifies and adds Doxygen for > > ff_fmt_is_in" fix it? > Yes. > Alternatively, can you tell me what tool > you used so that I can check locally? > I'm pretty sure you already have the tool I used. It's something *very* local to you, especially to the part of you that is reading this sentence ;) Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/5] ffmpeg: factorize checking decoder result
On Sun, Oct 18, 2015 at 12:24:05AM +0200, Marton Balint wrote: > Signed-off-by: Marton Balint> --- > ffmpeg.c | 27 --- > 1 file changed, 12 insertions(+), 15 deletions(-) LGTM thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Stream specifier enhancement
On Thu, 15 Oct 2015, Bodecs Bela wrote: [...] My enhancement does not alter the current behaviour in any way. Are you sure? What if somebody matched a semicolon, or a string which contained a semicolon in a metadata value? E.g.: m:timecode:00:00:00:00 This question still stands. Will the m:timecode:00:00:00:00 specifier work the same way as it did after your patch? I think it will not. [...] I do not understand this. My patch only gives an opportunity to use multiple specifiers in the same expression, it is not mandatory to use it. The patch does not affect any existing command line in any way. I don't agree, I think your patch changes existing behaviour and the proposed syntax limits future extensibility. I also accept your concern about the future, but double semicilon always will works for optional parameters. But may I ask: would it be better to introduce a "special character" for separating specifiers in the same expression? IMHO yes. You also have to know from the start that you are dealing with a complex specifier, in order not to break existing simple specifiers. I accept it if you suggest one. I only need the functionality to be able to give more criteria to select a stream as opposed to current oppurtunities. I am not stuck to my suggestion. Anyway, You may see my enhancement as you get many optional parameters for the existing type, metadata and program_id specifiers. :) It can be anything if it does not change existing behaviour, a complex specifier can be split to basic specifiers without worrying about the syntax of the basic specifier and if there is a well defined rule for escaping special characters. Also if it is readable to the user, that is a plus. The exact solution can be a bit about personal taste as well, but maybe something like (specifier)(specifier) or +specifier+specifier can work and is readable. Knowing that you are dealing with a complex expression also means that the special characters separating the basic specifiers needs escaping, I guess av_get_token can be used to get the proper unescaped basic specifiers when parsing the complex one. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/6] avutil/aes: use EINVAL instead of -1 for the return code of av_aes_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/aes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/aes.c b/libavutil/aes.c index 8d4..b59e7de 100644 --- a/libavutil/aes.c +++ b/libavutil/aes.c @@ -223,7 +223,7 @@ int av_aes_init(AVAES *a, const uint8_t *key, int key_bits, int decrypt) } if (key_bits != 128 && key_bits != 192 && key_bits != 256) -return -1; +return AVERROR(EINVAL); a->rounds = rounds; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/6] avutil/ripemd: use EINVAL instead of -1 for the return code of av_ripemd_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/ripemd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/ripemd.c b/libavutil/ripemd.c index 0084860..d247fb4 100644 --- a/libavutil/ripemd.c +++ b/libavutil/ripemd.c @@ -504,7 +504,7 @@ av_cold int av_ripemd_init(AVRIPEMD *ctx, int bits) ctx->transform = ripemd320_transform; break; default: -return -1; +return AVERROR(EINVAL); } ctx->count = 0; return 0; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/6] avutil/sha512: use EINVAL instead of -1 for the return code of av_sha512_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/sha512.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/sha512.c b/libavutil/sha512.c index 66a864f..e2fc58a 100644 --- a/libavutil/sha512.c +++ b/libavutil/sha512.c @@ -233,7 +233,7 @@ av_cold int av_sha512_init(AVSHA512 *ctx, int bits) ctx->state[7] = UINT64_C(0x5BE0CD19137E2179); break; default: -return -1; +return AVERROR(EINVAL); } ctx->count = 0; return 0; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] compat/solaris/make_sunver.pl: Use /usr/bin/env perl instead of /usr/bin/perl
On Sat, Oct 17, 2015 at 8:41 PM, Michael Niedermayerwrote: > From: Michael Niedermayer > > This is how the other perl scripts in git call perl > > Signed-off-by: Michael Niedermayer > --- > compat/solaris/make_sunver.pl |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/compat/solaris/make_sunver.pl b/compat/solaris/make_sunver.pl > index 929bdda..0e9ed1d 100755 > --- a/compat/solaris/make_sunver.pl > +++ b/compat/solaris/make_sunver.pl > @@ -1,4 +1,4 @@ > -#!/usr/bin/perl -w > +#!/usr/bin/env perl Should be ok. I don't know if the -w (which turns on warnings) is useful, but for the moment LGTM. > > # make_sunver.pl > # > -- > 1.7.9.5 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/bitstream: replace qsort with AV_QSORT
On Sat, Oct 17, 2015 at 09:30:01PM -0400, Ganesh Ajjanagadde wrote: > Commit 3a0a2f33a6c955823fa4fb12c0b49cd29a496659 claims large performance > advantages for AV_QSORT over libc's qsort. The reason is that I suspect > that libc's qsort (at least on non LTO builds, like the typical FFmpeg config) > can't inline the comparison callback: > https://stackoverflow.com/questions/5290695/is-there-any-way-a-c-c-compiler-can-inline-a-c-callback-function. > AV_QSORT has two things going for it: > 1. The guaranteed inlining of qsort itself. This yields a negligible > boost that may be ignored. > 2. The more serious possibility of potentially allowing the comparison > function to be inlined - this is likely responsible for the large boosts > reported. > > There is a comment explaining that this is a place that could use some > performance improvement. Thus AV_QSORT is used to achieve that. > > Benchmarks deemed unnecessary due to existing claims about AV_QSORT. > Tested with FATE. > > Signed-off-by: Ganesh Ajjanagadde> --- > libavcodec/bitstream.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/5] ffmpeg: exit on corrupt packets or decoded frames if exit_on_error flag is present
Signed-off-by: Marton Balint--- ffmpeg.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ffmpeg.c b/ffmpeg.c index 3a36af7..f6947f8 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -1925,13 +1925,20 @@ int guess_input_channel_layout(InputStream *ist) return 1; } -static void check_decode_result(int *got_output, int ret) +static void check_decode_result(int *got_output, int ret, AVFrame *frame) { if (*got_output || ret<0) decode_error_stat[ret<0] ++; if (ret < 0 && exit_on_error) exit_program(1); + +if (exit_on_error && *got_output && frame) { +if (av_frame_get_decode_error_flags(frame) || (frame->flags & AV_FRAME_FLAG_CORRUPT)) { +av_log(NULL, AV_LOG_FATAL, "Corrupt decoded frame\n"); +exit_program(1); +} +} } static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output) @@ -1956,7 +1963,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output) ret = AVERROR_INVALIDDATA; } -check_decode_result(got_output, ret); +check_decode_result(got_output, ret, decoded_frame); if (!*got_output || ret < 0) return ret; @@ -2093,7 +2100,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output) ist->st->codec->has_b_frames); } -check_decode_result(got_output, ret); +check_decode_result(got_output, ret, decoded_frame); if (*got_output && ret >= 0) { if (ist->dec_ctx->width != decoded_frame->width || @@ -2201,7 +2208,7 @@ static int transcode_subtitles(InputStream *ist, AVPacket *pkt, int *got_output) int i, ret = avcodec_decode_subtitle2(ist->dec_ctx, , got_output, pkt); -check_decode_result(got_output, ret); +check_decode_result(got_output, ret, NULL); if (ret < 0 || !*got_output) { if (!pkt->size) @@ -3770,6 +3777,11 @@ static int process_input(int file_index) if (ist->discard) goto discard_packet; +if (exit_on_error && (pkt.flags & AV_PKT_FLAG_CORRUPT)) { +av_log(NULL, AV_LOG_FATAL, "Corrupt input packet\n"); +exit_program(1); +} + if (debug_ts) { av_log(NULL, AV_LOG_INFO, "demuxer -> ist_index:%d type:%s " "next_dts:%s next_dts_time:%s next_pts:%s next_pts_time:%s pkt_pts:%s pkt_pts_time:%s pkt_dts:%s pkt_dts_time:%s off:%s off_time:%s\n", -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/6] avutil/des: use EINVAL instead of -1 for the return code of av_des_init()
Hi, On Sat, Oct 17, 2015 at 7:40 PM, Ganesh Ajjanagaddewrote: > On Sat, Oct 17, 2015 at 7:39 PM, Ganesh Ajjanagadde > wrote: > > Signed-off-by: Ganesh Ajjanagadde > > --- > > libavutil/des.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavutil/des.c b/libavutil/des.c > > index c97158a..3ccbf89 100644 > > --- a/libavutil/des.c > > +++ b/libavutil/des.c > > @@ -292,7 +292,7 @@ AVDES *av_des_alloc(void) > > > > int av_des_init(AVDES *d, const uint8_t *key, int key_bits, av_unused > int decrypt) { > > if (key_bits != 64 && key_bits != 192) > > -return -1; > > +return AVERROR(EINVAL); > > d->triple_des = key_bits > 64; > > gen_roundkeys(d->round_keys[0], AV_RB64(key)); > > if (d->triple_des) { > > -- > > 2.6.1 > > > > I can squash these if people prefer, or leave as is. Whichever you prefer. All patches in this set LGTM. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] x86: vc1dsp_mmx: Move yasm initiation steps to vc1dsp_init
That's where all yasm initiation steps are. Also removes the overlap between the two files. --- libavcodec/x86/vc1dsp_init.c | 36 +--- libavcodec/x86/vc1dsp_mmx.c | 27 --- 2 files changed, 25 insertions(+), 38 deletions(-) diff --git a/libavcodec/x86/vc1dsp_init.c b/libavcodec/x86/vc1dsp_init.c index 2bef5f5..8c5fc20 100644 --- a/libavcodec/x86/vc1dsp_init.c +++ b/libavcodec/x86/vc1dsp_init.c @@ -63,16 +63,22 @@ static void vc1_h_loop_filter16_sse4(uint8_t *src, int stride, int pq) ff_vc1_h_loop_filter8_sse4(src, stride, pq); ff_vc1_h_loop_filter8_sse4(src+8*stride, stride, pq); } -static void avg_vc1_mspel_mc00_mmxext(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_avg_pixels8_mmxext(dst, src, stride, 8); -} -static void avg_vc1_mspel_mc00_16_sse2(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_avg_pixels16_sse2(dst, src, stride, 16); -} + +#define DECLARE_FUNCTION(OP, DEPTH, INSN) \ +static void OP##vc1_mspel_mc00_##DEPTH##INSN(uint8_t *dst, \ + const uint8_t *src, ptrdiff_t stride, int rnd) \ +{ \ +ff_ ## OP ## pixels ## DEPTH ## INSN(dst, src, stride, DEPTH); \ +} + +DECLARE_FUNCTION(put_, 8, _mmx) +DECLARE_FUNCTION(put_, 16, _mmx) +DECLARE_FUNCTION(avg_, 8, _mmx) +DECLARE_FUNCTION(avg_, 16, _mmx) +DECLARE_FUNCTION(avg_, 8, _mmxext) +DECLARE_FUNCTION(avg_, 16, _mmxext) +DECLARE_FUNCTION(put_, 16, _sse2) +DECLARE_FUNCTION(avg_, 16, _sse2) #endif /* HAVE_YASM */ @@ -109,6 +115,11 @@ av_cold void ff_vc1dsp_init_x86(VC1DSPContext *dsp) #if HAVE_YASM if (EXTERNAL_MMX(cpu_flags)) { dsp->put_no_rnd_vc1_chroma_pixels_tab[0] = ff_put_vc1_chroma_mc8_nornd_mmx; + +dsp->put_vc1_mspel_pixels_tab[1][0] = put_vc1_mspel_mc00_8_mmx; +dsp->put_vc1_mspel_pixels_tab[0][0] = put_vc1_mspel_mc00_16_mmx; +dsp->avg_vc1_mspel_pixels_tab[1][0] = avg_vc1_mspel_mc00_8_mmx; +dsp->avg_vc1_mspel_pixels_tab[0][0] = avg_vc1_mspel_mc00_16_mmx; } if (EXTERNAL_AMD3DNOW(cpu_flags)) { dsp->avg_no_rnd_vc1_chroma_pixels_tab[0] = ff_avg_vc1_chroma_mc8_nornd_3dnow; @@ -117,13 +128,16 @@ av_cold void ff_vc1dsp_init_x86(VC1DSPContext *dsp) ASSIGN_LF(mmxext); dsp->avg_no_rnd_vc1_chroma_pixels_tab[0] = ff_avg_vc1_chroma_mc8_nornd_mmxext; -dsp->avg_vc1_mspel_pixels_tab[1][0] = avg_vc1_mspel_mc00_mmxext; +dsp->avg_vc1_mspel_pixels_tab[1][0] = avg_vc1_mspel_mc00_8_mmxext; +dsp->avg_vc1_mspel_pixels_tab[0][0] = avg_vc1_mspel_mc00_16_mmxext; } if (EXTERNAL_SSE2(cpu_flags)) { dsp->vc1_v_loop_filter8 = ff_vc1_v_loop_filter8_sse2; dsp->vc1_h_loop_filter8 = ff_vc1_h_loop_filter8_sse2; dsp->vc1_v_loop_filter16 = vc1_v_loop_filter16_sse2; dsp->vc1_h_loop_filter16 = vc1_h_loop_filter16_sse2; + +dsp->put_vc1_mspel_pixels_tab[0][0] = put_vc1_mspel_mc00_16_sse2; dsp->avg_vc1_mspel_pixels_tab[0][0] = avg_vc1_mspel_mc00_16_sse2; } if (EXTERNAL_SSSE3(cpu_flags)) { diff --git a/libavcodec/x86/vc1dsp_mmx.c b/libavcodec/x86/vc1dsp_mmx.c index a7eb59d..e42099b 100644 --- a/libavcodec/x86/vc1dsp_mmx.c +++ b/libavcodec/x86/vc1dsp_mmx.c @@ -728,39 +728,12 @@ static void vc1_inv_trans_8x8_dc_mmxext(uint8_t *dest, int linesize, ); } -#if HAVE_MMX_EXTERNAL -static void put_vc1_mspel_mc00_mmx(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_put_pixels8_mmx(dst, src, stride, 8); -} -static void put_vc1_mspel_mc00_16_mmx(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_put_pixels16_mmx(dst, src, stride, 16); -} -static void avg_vc1_mspel_mc00_mmx(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_avg_pixels8_mmx(dst, src, stride, 8); -} -static void avg_vc1_mspel_mc00_16_mmx(uint8_t *dst, const uint8_t *src, - ptrdiff_t stride, int rnd) -{ -ff_avg_pixels16_mmx(dst, src, stride, 16); -} -#endif - #define FN_ASSIGN(OP, X, Y, INSN) \ dsp->OP##vc1_mspel_pixels_tab[1][X+4*Y] = OP##vc1_mspel_mc##X##Y##INSN; \ dsp->OP##vc1_mspel_pixels_tab[0][X+4*Y] = OP##vc1_mspel_mc##X##Y##_16##INSN av_cold void ff_vc1dsp_init_mmx(VC1DSPContext *dsp) { -#if HAVE_MMX_EXTERNAL -FN_ASSIGN(put_, 0, 0, _mmx); -FN_ASSIGN(avg_, 0, 0, _mmx); -#endif FN_ASSIGN(put_, 0, 1, _mmx); FN_ASSIGN(put_, 0, 2, _mmx); FN_ASSIGN(put_, 0, 3, _mmx); -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org
[FFmpeg-devel] [PATCH 1/4] x86: fpel: Move prototypes for 4-px block functions
--- libavcodec/x86/fpel.h | 6 ++ libavcodec/x86/h264_qpel.c | 4 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/libavcodec/x86/fpel.h b/libavcodec/x86/fpel.h index 4d93959..625c47d 100644 --- a/libavcodec/x86/fpel.h +++ b/libavcodec/x86/fpel.h @@ -22,6 +22,10 @@ #include #include +void ff_avg_pixels4_mmx(uint8_t *block, const uint8_t *pixels, +ptrdiff_t line_size, int h); +void ff_avg_pixels4_mmxext(uint8_t *block, const uint8_t *pixels, + ptrdiff_t line_size, int h); void ff_avg_pixels8_mmx(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); void ff_avg_pixels8_mmxext(uint8_t *block, const uint8_t *pixels, @@ -32,6 +36,8 @@ void ff_avg_pixels16_mmxext(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); void ff_avg_pixels16_sse2(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); +void ff_put_pixels4_mmx(uint8_t *block, const uint8_t *pixels, +ptrdiff_t line_size, int h); void ff_put_pixels8_mmx(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); void ff_put_pixels8_mmxext(uint8_t *block, const uint8_t *pixels, diff --git a/libavcodec/x86/h264_qpel.c b/libavcodec/x86/h264_qpel.c index d9cb5f2..d759e88 100644 --- a/libavcodec/x86/h264_qpel.c +++ b/libavcodec/x86/h264_qpel.c @@ -29,10 +29,6 @@ #include "fpel.h" #if HAVE_YASM -void ff_put_pixels4_mmx(uint8_t *block, const uint8_t *pixels, -ptrdiff_t line_size, int h); -void ff_avg_pixels4_mmxext(uint8_t *block, const uint8_t *pixels, - ptrdiff_t line_size, int h); void ff_put_pixels4_l2_mmxext(uint8_t *dst, const uint8_t *src1, const uint8_t *src2, int dstStride, int src1Stride, int h); void ff_avg_pixels4_l2_mmxext(uint8_t *dst, const uint8_t *src1, const uint8_t *src2, -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] x86: fpel: Remove erroneous ff_put_pixels8_mmxext prototype
This function does not exist. --- libavcodec/x86/fpel.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/libavcodec/x86/fpel.h b/libavcodec/x86/fpel.h index 625c47d..4e83cf7 100644 --- a/libavcodec/x86/fpel.h +++ b/libavcodec/x86/fpel.h @@ -40,8 +40,6 @@ void ff_put_pixels4_mmx(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); void ff_put_pixels8_mmx(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); -void ff_put_pixels8_mmxext(uint8_t *block, const uint8_t *pixels, - ptrdiff_t line_size, int h); void ff_put_pixels16_mmx(uint8_t *block, const uint8_t *pixels, ptrdiff_t line_size, int h); void ff_put_pixels16_sse2(uint8_t *block, const uint8_t *pixels, -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] avutil/cast5: use EINVAL instead of -1 for the return code of av_cast5_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/cast5.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/cast5.c b/libavutil/cast5.c index 98aa19d..a47697b 100644 --- a/libavutil/cast5.c +++ b/libavutil/cast5.c @@ -459,7 +459,7 @@ av_cold int av_cast5_init(AVCAST5* cs, const uint8_t *key, int key_bits) int i; uint32_t p[4], q[4]; if (key_bits % 8 || key_bits < 40 || key_bits > 128) -return -1; +return AVERROR(EINVAL); memset(newKey, 0, sizeof(newKey)); memcpy(newKey, key, key_bits >> 3); -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] avutil/camellia: use EINVAL instead of -1 for the return code of av_camellia_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/camellia.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/camellia.c b/libavutil/camellia.c index 483eed2..f21ca12 100644 --- a/libavutil/camellia.c +++ b/libavutil/camellia.c @@ -354,7 +354,7 @@ av_cold int av_camellia_init(AVCAMELLIA *cs, const uint8_t *key, int key_bits) uint64_t Kl[2], Kr[2], Ka[2], Kb[2]; uint64_t D1, D2; if (key_bits != 128 && key_bits != 192 && key_bits != 256) -return -1; +return AVERROR(EINVAL); memset(Kb, 0, sizeof(Kb)); memset(Kr, 0, sizeof(Kr)); cs->key_bits = key_bits; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] avutil/sha: use EINVAL instead of -1 for the return code of av_sha_init()
Signed-off-by: Ganesh Ajjanagadde--- libavutil/sha.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/sha.c b/libavutil/sha.c index 9963043..748bb9c 100644 --- a/libavutil/sha.c +++ b/libavutil/sha.c @@ -305,7 +305,7 @@ av_cold int av_sha_init(AVSHA *ctx, int bits) ctx->transform = sha256_transform; break; default: -return -1; +return AVERROR(EINVAL); } ctx->count = 0; return 0; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/bitstream: replace qsort with AV_QSORT
On Sat, Oct 17, 2015 at 9:30 PM, Ganesh Ajjanagaddewrote: > Commit 3a0a2f33a6c955823fa4fb12c0b49cd29a496659 claims large performance > advantages for AV_QSORT over libc's qsort. The reason is that I suspect > that libc's qsort (at least on non LTO builds, like the typical FFmpeg config) > can't inline the comparison callback: > https://stackoverflow.com/questions/5290695/is-there-any-way-a-c-c-compiler-can-inline-a-c-callback-function. > AV_QSORT has two things going for it: > 1. The guaranteed inlining of qsort itself. This yields a negligible > boost that may be ignored. > 2. The more serious possibility of potentially allowing the comparison > function to be inlined - this is likely responsible for the large boosts > reported. > > There is a comment explaining that this is a place that could use some > performance improvement. Thus AV_QSORT is used to achieve that. > > Benchmarks deemed unnecessary due to existing claims about AV_QSORT. > Tested with FATE. > > Signed-off-by: Ganesh Ajjanagadde > --- > libavcodec/bitstream.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > index 924cc51..1acb7a3 100644 > --- a/libavcodec/bitstream.c > +++ b/libavcodec/bitstream.c > @@ -30,6 +30,7 @@ > > #include "libavutil/atomic.h" > #include "libavutil/avassert.h" > +#include "libavutil/qsort.h" > #include "avcodec.h" > #include "internal.h" > #include "mathops.h" > @@ -333,7 +334,7 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int > nb_codes, > } > COPY(buf[j].bits > nb_bits); > // qsort is the slowest part of init_vlc, and could probably be improved > or avoided > -qsort(buf, j, sizeof(VLCcode), compare_vlcspec); > +AV_QSORT(buf, j, struct VLCcode, compare_vlcspec); > COPY(buf[j].bits && buf[j].bits <= nb_bits); > nb_codes = j; > > -- > 2.6.1 > Indeed confirmed the difference in inlining: the asm for compare_vlcspec is gone. Thus performance gains should be obvious. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] doc/muxers: Document range for mpegts periods
On Sat, Oct 17, 2015 at 12:33 PM, Michael Niedermayerwrote: > On Fri, Oct 16, 2015 at 03:09:21PM -0400, Derek Buitenhuis wrote: >> Signed-off-by: Derek Buitenhuis >> --- >> doc/muxers.texi | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/doc/muxers.texi b/doc/muxers.texi >> index 06483fa..cef04e1 100644 >> --- a/doc/muxers.texi >> +++ b/doc/muxers.texi >> @@ -802,9 +802,9 @@ Set a constant muxrate (default VBR). >> Override the default PCR retransmission time (default 20ms), ignored >> if variable muxrate is selected. >> @item pat_period @var{number} >> -Maximal time in seconds between PAT/PMT tables. >> +Maximal time in seconds between PAT/PMT tables. Max value is INT_MAX / 2 - >> 1. >> @item sdt_period @var{number} >> -Maximal time in seconds between SDT tables. >> +Maximal time in seconds between SDT tables. Max value is INT_MAX / 2 - 1. > > iam unsure about this, INT_MAX is theoretically platform specific and > most non developers probably dont know its value > that said, iam not against it at all, just feels like this cant be > the best solution Certainly not the best, but definitely better than the current - range restrictions should be documented. Furthermore, INT_MAX is pretty well known even for a first exposure to C, and in all likelihood someone who reads the muxer docs is sharp/knows enough to do a quick google on what INT_MAX is (after all, he was savvy enough to look at the docs). Maybe one could in principle use configure to get the relevant actual value and use that in the docs. That is pretty complex and should be done IMHO only if there are more such type-width specific limits. Furthermore, that has problems, if docs were obtained off a website (website's config might be different from user's, etc). Given this, I still favor the INT_MAX solution. Also, on that note, it has already been noted that on all platforms we have (currently) int is 32 bits. A lot more serious things will break than the docs if the width of ints changes. That is another thing in INT_MAX's favor: it won't break on such a change :). > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Those who are best at talking, realize last or never when they are wrong. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] concatdec: fix file_start_time calculation regression
On Sat, 17 Oct 2015, Nicolas George wrote: Le duodi 22 vendémiaire, an CCXXIV, Marton Balint a écrit : Fixes ticket #4924. Found-by: Jaroslav ŠnajdrSigned-off-by: Marton Balint --- libavformat/concatdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Sorry I missed it for a few days. It looks good to me according to your analysis, thanks for taking care of it. Thanks, applied. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/11] avdevice/pulse_audio_common: add av_warn_unused_result
On Sat, Oct 17, 2015 at 12:36 PM, Michael Niedermayerwrote: > On Thu, Oct 15, 2015 at 10:22:16PM -0400, Ganesh Ajjanagadde wrote: >> This does not trigger any warnings, but adds robustness. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- >> libavdevice/pulse_audio_common.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavdevice/pulse_audio_common.h >> b/libavdevice/pulse_audio_common.h >> index 02534f7..902795e 100644 >> --- a/libavdevice/pulse_audio_common.h >> +++ b/libavdevice/pulse_audio_common.h >> @@ -28,8 +28,10 @@ >> >> pa_sample_format_t ff_codec_id_to_pulse_format(enum AVCodecID codec_id); >> >> +av_warn_unused_result >> int ff_pulse_audio_get_devices(AVDeviceInfoList *devices, const char >> *server, int output); >> >> +av_warn_unused_result >> int ff_pulse_audio_connect_context(pa_mainloop **pa_ml, pa_context **pa_ctx, >> const char *server, const char >> *description); > > iam not the maintainer of this but the changes look reasonable will wait for 48 hours, then push if no one responds. > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Democracy is the form of government in which you can choose your dictator > > ___ > 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 3/3] doc/muxers: Document range for mpegts periods
On Sat, Oct 17, 2015 at 12:43:32PM -0400, Ganesh Ajjanagadde wrote: > On Sat, Oct 17, 2015 at 12:33 PM, Michael Niedermayer >wrote: > > On Fri, Oct 16, 2015 at 03:09:21PM -0400, Derek Buitenhuis wrote: > >> Signed-off-by: Derek Buitenhuis > >> --- > >> doc/muxers.texi | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/doc/muxers.texi b/doc/muxers.texi > >> index 06483fa..cef04e1 100644 > >> --- a/doc/muxers.texi > >> +++ b/doc/muxers.texi > >> @@ -802,9 +802,9 @@ Set a constant muxrate (default VBR). > >> Override the default PCR retransmission time (default 20ms), ignored > >> if variable muxrate is selected. > >> @item pat_period @var{number} > >> -Maximal time in seconds between PAT/PMT tables. > >> +Maximal time in seconds between PAT/PMT tables. Max value is INT_MAX / 2 > >> - 1. > >> @item sdt_period @var{number} > >> -Maximal time in seconds between SDT tables. > >> +Maximal time in seconds between SDT tables. Max value is INT_MAX / 2 - 1. > > > > iam unsure about this, INT_MAX is theoretically platform specific and > > most non developers probably dont know its value > > that said, iam not against it at all, just feels like this cant be > > the best solution > > Certainly not the best, but definitely better than the current - range > restrictions should be documented. Furthermore, INT_MAX is pretty well > known even for a first exposure to C, and in all likelihood someone > who reads the muxer docs is sharp/knows enough to do a quick google on > what INT_MAX is (after all, he was savvy enough to look at the docs). > > Maybe one could in principle use configure to get the relevant actual > value and use that in the docs. That is pretty complex and should be > done IMHO only if there are more such type-width specific limits. > Furthermore, that has problems, if docs were obtained off a website > (website's config might be different from user's, etc). Given this, I > still favor the INT_MAX solution. theres mayybe no need to document the exact maximum, stating that 2^30 is the max could be fine (its 34 years) or the code could be changed as well to not have "INT_MAX / 2 - 1" as maximum theres also the aspect that if int is 64bit, INT_MAX / 2 - 1 cannot exactly be represented as IEEE 64bit double, i doubt that matters but this patchset seemed to have the goal to get this precissely correct > > Also, on that note, it has already been noted that on all platforms we > have (currently) int is 32 bits. A lot more serious things will break > than the docs if the width of ints changes. That is another thing in > INT_MAX's favor: it won't break on such a change :). what breaks with 64bit int ? (note if you say x86 asm then the question is which x86 ABI allows 64 bit int, if thats none then non x86 is implied by 64bit int) also is there a way to (easily) test a build with 64bit int ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you think the mosad wants you dead since a long time then you are either wrong or dead since a long time. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] concatdec: fix file_start_time calculation regression
Le duodi 22 vendémiaire, an CCXXIV, Marton Balint a écrit : > Fixes ticket #4924. > > Found-by: Jaroslav Šnajdr> Signed-off-by: Marton Balint > --- > libavformat/concatdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Sorry I missed it for a few days. It looks good to me according to your analysis, thanks for taking care of it. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avutil/opt: display a better default value for int/int64 options
Example: % ./ffmpeg -h encoder=aac -aac_coder E...A... Coding algorithm (from -1 to 3) (default twoloop) faac E...A... FAAC-inspired method anmr E...A... ANMR method twoloop E...A... Two loop searching method fast E...A... Constant quantizer [...] --- libavutil/opt.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/libavutil/opt.c b/libavutil/opt.c index 03160c7..36eeeb0 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -928,6 +928,19 @@ static void log_value(void *av_log_obj, int level, double d) } } +static const char *get_opt_const_name(void *obj, const char *unit, int64_t value) +{ +const AVOption *opt = NULL; + +if (!unit) +return NULL; +while ((opt = av_opt_next(obj, opt))) +if (opt->type == AV_OPT_TYPE_CONST && !strcmp(opt->unit, unit) && +opt->default_val.i64 == value) +return opt->name; +return NULL; +} + static void opt_list(void *obj, void *av_log_obj, const char *unit, int req_flags, int rej_flags) { @@ -1057,10 +1070,17 @@ static void opt_list(void *obj, void *av_log_obj, const char *unit, av_log(av_log_obj, AV_LOG_INFO, "%"PRIX64, opt->default_val.i64); break; case AV_OPT_TYPE_DURATION: -case AV_OPT_TYPE_INT: -case AV_OPT_TYPE_INT64: log_value(av_log_obj, AV_LOG_INFO, opt->default_val.i64); break; +case AV_OPT_TYPE_INT: +case AV_OPT_TYPE_INT64: { +const char *def_const = get_opt_const_name(obj, opt->unit, opt->default_val.i64); +if (def_const) +av_log(av_log_obj, AV_LOG_INFO, "%s", def_const); +else +log_value(av_log_obj, AV_LOG_INFO, opt->default_val.i64); +break; +} case AV_OPT_TYPE_DOUBLE: case AV_OPT_TYPE_FLOAT: log_value(av_log_obj, AV_LOG_INFO, opt->default_val.dbl); -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/Makefile: remove unnecessary object file from wtv demuxer
On Fri, Oct 16, 2015 at 12:25 AM, Hendrik Leppkeswrote: > The wtv demuxer doesn't reference any functionality from asfdec or asfcrypt > --- > Found another one.. > > libavformat/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index c58cfb4..7bcdadf 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -471,7 +471,7 @@ OBJS-$(CONFIG_WEBVTT_DEMUXER)+= webvttdec.o > subtitles.o > OBJS-$(CONFIG_WEBVTT_MUXER) += webvttenc.o > OBJS-$(CONFIG_WSAUD_DEMUXER) += westwood_aud.o > OBJS-$(CONFIG_WSVQA_DEMUXER) += westwood_vqa.o > -OBJS-$(CONFIG_WTV_DEMUXER) += wtvdec.o wtv_common.o asfdec_f.o > asf.o asfcrypt.o \ > +OBJS-$(CONFIG_WTV_DEMUXER) += wtvdec.o wtv_common.o asf.o \ > avlanguage.o mpegts.o isom.o > OBJS-$(CONFIG_WTV_MUXER) += wtvenc.o wtv_common.o \ > mpegtsenc.o asf.o > -- > 2.5.3.windows.1 > Since its a trivial change, applied. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] mpegts: Make the pat_period a double
On Fri, Oct 16, 2015 at 03:09:19PM -0400, Derek Buitenhuis wrote: > Having it as a float didn't even allow enough precision to check > for INT_MAX/2. > > Signed-off-by: Derek Buitenhuis> --- > libavformat/mpegtsenc.c | 4 ++-- > libavformat/version.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] mpegts: Make the sdt_period a double
On Fri, Oct 16, 2015 at 03:09:20PM -0400, Derek Buitenhuis wrote: > Having it as a float didn't even allow enough precision to check > for INT_MAX/2. > > Signed-off-by: Derek Buitenhuis> --- > libavformat/mpegtsenc.c | 4 ++-- > libavformat/version.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: improve junk skipping heuristic
On Sat, 17 Oct 2015 14:02:09 +0200 Michael Niedermayerwrote: > On Fri, Oct 16, 2015 at 08:04:08PM +0200, wm4 wrote: > > Commit 2b3e9bbfb529e6bde238aeb511b55ebe461664c8 caused problems for a > > certain API user: > > > > https://code.google.com/p/chromium/issues/detail?id=537725 > > https://code.google.com/p/chromium/issues/detail?id=542032 > > > > The problem seems rather arbitrary, because if there's junk, anything > > can happen. In this case, the imperfect junk skipping just caused it to > > read different junk, from what I can see. > > > > We can improve the accuracy of junk detection by a lot by checking if 2 > > consecutive frames use the same configuration. While in theory it might > > be completely fine for the 1st frame to have a different format than the > > 2nd frame, it's exceedingly unlikely, and I can't think of a legitimate > > use-case. > > > > This is approximately the same mpg123 does for junk skipping, and the > > set of compared header bits is exactly the same. > > --- > > The reason Chromium had so much problems with this is that they don't > > allow mid-stream format changes at all, and the junk triggered format > > changes. > > > > (Would be nice if Google paid me for this.) > > --- > > libavformat/mp3dec.c | 25 ++--- > > 1 file changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > > index d3080d7..e7b6234 100644 > > --- a/libavformat/mp3dec.c > > +++ b/libavformat/mp3dec.c > > @@ -54,7 +54,7 @@ typedef struct { > > int is_cbr; > > } MP3DecContext; > > > > -static int check(AVIOContext *pb, int64_t pos); > > +static int check(AVIOContext *pb, int64_t pos, uint32_t *header); > > > > /* mp3 read */ > > > > @@ -374,12 +374,21 @@ static int mp3_read_header(AVFormatContext *s) > > > > off = avio_tell(s->pb); > > for (i = 0; i < 64 * 1024; i++) { > > +uint32_t header, header2; > > +int frame_size; > > if (!(i&1023)) > > ffio_ensure_seekback(s->pb, i + 1024 + 4); > > -if (check(s->pb, off + i) >= 0) { > > -av_log(s, AV_LOG_INFO, "Skipping %d bytes of junk at > > %"PRId64".\n", i, off); > > -avio_seek(s->pb, off + i, SEEK_SET); > > -break; > > +frame_size = check(s->pb, off + i, ); > > +if (frame_size > 0) { > > +avio_seek(s->pb, off, SEEK_SET); > > +ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4); > > +if (check(s->pb, off + i + frame_size, ) >= 0 && > > +(header & 0xFFFEFCF0) == (header2 & 0xFFFEFCF0)) > > This also requires the bitrate and stereo type to match > teh bitrate can change in VBR, the stereo coding type can also change > i think > > [...] What do you suggest? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/opt: display a better default value for int/int64 options
On Sat, Oct 17, 2015 at 02:54:31PM +0200, Clément Bœsch wrote: > Example: > > % ./ffmpeg -h encoder=aac > -aac_coder E...A... Coding algorithm (from -1 to 3) > (default twoloop) > faac E...A... FAAC-inspired method > anmr E...A... ANMR method > twoloop E...A... Two loop searching method > fast E...A... Constant quantizer > [...] > --- > libavutil/opt.c | 24 ++-- > 1 file changed, 22 insertions(+), 2 deletions(-) should be ok [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: improve junk skipping heuristic
On Fri, Oct 16, 2015 at 08:04:08PM +0200, wm4 wrote: > Commit 2b3e9bbfb529e6bde238aeb511b55ebe461664c8 caused problems for a > certain API user: > > https://code.google.com/p/chromium/issues/detail?id=537725 > https://code.google.com/p/chromium/issues/detail?id=542032 > > The problem seems rather arbitrary, because if there's junk, anything > can happen. In this case, the imperfect junk skipping just caused it to > read different junk, from what I can see. > > We can improve the accuracy of junk detection by a lot by checking if 2 > consecutive frames use the same configuration. While in theory it might > be completely fine for the 1st frame to have a different format than the > 2nd frame, it's exceedingly unlikely, and I can't think of a legitimate > use-case. > > This is approximately the same mpg123 does for junk skipping, and the > set of compared header bits is exactly the same. > --- > The reason Chromium had so much problems with this is that they don't > allow mid-stream format changes at all, and the junk triggered format > changes. > > (Would be nice if Google paid me for this.) > --- > libavformat/mp3dec.c | 25 ++--- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > index d3080d7..e7b6234 100644 > --- a/libavformat/mp3dec.c > +++ b/libavformat/mp3dec.c > @@ -54,7 +54,7 @@ typedef struct { > int is_cbr; > } MP3DecContext; > > -static int check(AVIOContext *pb, int64_t pos); > +static int check(AVIOContext *pb, int64_t pos, uint32_t *header); > > /* mp3 read */ > > @@ -374,12 +374,21 @@ static int mp3_read_header(AVFormatContext *s) > > off = avio_tell(s->pb); > for (i = 0; i < 64 * 1024; i++) { > +uint32_t header, header2; > +int frame_size; > if (!(i&1023)) > ffio_ensure_seekback(s->pb, i + 1024 + 4); > -if (check(s->pb, off + i) >= 0) { > -av_log(s, AV_LOG_INFO, "Skipping %d bytes of junk at > %"PRId64".\n", i, off); > -avio_seek(s->pb, off + i, SEEK_SET); > -break; > +frame_size = check(s->pb, off + i, ); > +if (frame_size > 0) { > +avio_seek(s->pb, off, SEEK_SET); > +ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4); > +if (check(s->pb, off + i + frame_size, ) >= 0 && > +(header & 0xFFFEFCF0) == (header2 & 0xFFFEFCF0)) This also requires the bitrate and stereo type to match teh bitrate can change in VBR, the stereo coding type can also change i think [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: improve junk skipping heuristic
On Sat, Oct 17, 2015 at 04:18:14PM +0200, wm4 wrote: > On Sat, 17 Oct 2015 14:02:09 +0200 > Michael Niedermayerwrote: > > > On Fri, Oct 16, 2015 at 08:04:08PM +0200, wm4 wrote: > > > Commit 2b3e9bbfb529e6bde238aeb511b55ebe461664c8 caused problems for a > > > certain API user: > > > > > > https://code.google.com/p/chromium/issues/detail?id=537725 > > > https://code.google.com/p/chromium/issues/detail?id=542032 > > > > > > The problem seems rather arbitrary, because if there's junk, anything > > > can happen. In this case, the imperfect junk skipping just caused it to > > > read different junk, from what I can see. > > > > > > We can improve the accuracy of junk detection by a lot by checking if 2 > > > consecutive frames use the same configuration. While in theory it might > > > be completely fine for the 1st frame to have a different format than the > > > 2nd frame, it's exceedingly unlikely, and I can't think of a legitimate > > > use-case. > > > > > > This is approximately the same mpg123 does for junk skipping, and the > > > set of compared header bits is exactly the same. > > > --- > > > The reason Chromium had so much problems with this is that they don't > > > allow mid-stream format changes at all, and the junk triggered format > > > changes. > > > > > > (Would be nice if Google paid me for this.) > > > --- > > > libavformat/mp3dec.c | 25 ++--- > > > 1 file changed, 18 insertions(+), 7 deletions(-) > > > > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > > > index d3080d7..e7b6234 100644 > > > --- a/libavformat/mp3dec.c > > > +++ b/libavformat/mp3dec.c > > > @@ -54,7 +54,7 @@ typedef struct { > > > int is_cbr; > > > } MP3DecContext; > > > > > > -static int check(AVIOContext *pb, int64_t pos); > > > +static int check(AVIOContext *pb, int64_t pos, uint32_t *header); > > > > > > /* mp3 read */ > > > > > > @@ -374,12 +374,21 @@ static int mp3_read_header(AVFormatContext *s) > > > > > > off = avio_tell(s->pb); > > > for (i = 0; i < 64 * 1024; i++) { > > > +uint32_t header, header2; > > > +int frame_size; > > > if (!(i&1023)) > > > ffio_ensure_seekback(s->pb, i + 1024 + 4); > > > -if (check(s->pb, off + i) >= 0) { > > > -av_log(s, AV_LOG_INFO, "Skipping %d bytes of junk at > > > %"PRId64".\n", i, off); > > > -avio_seek(s->pb, off + i, SEEK_SET); > > > -break; > > > +frame_size = check(s->pb, off + i, ); > > > +if (frame_size > 0) { > > > +avio_seek(s->pb, off, SEEK_SET); > > > +ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4); > > > +if (check(s->pb, off + i + frame_size, ) >= 0 && > > > +(header & 0xFFFEFCF0) == (header2 & 0xFFFEFCF0)) > > > > This also requires the bitrate and stereo type to match > > teh bitrate can change in VBR, the stereo coding type can also change > > i think > > > > [...] > > What do you suggest? does checking only the remaining parts/bits work ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] doc/muxers: Document range for mpegts periods
On Sat, Oct 17, 2015 at 1:49 PM, Michael Niedermayerwrote: > On Sat, Oct 17, 2015 at 12:43:32PM -0400, Ganesh Ajjanagadde wrote: >> On Sat, Oct 17, 2015 at 12:33 PM, Michael Niedermayer >> wrote: >> > On Fri, Oct 16, 2015 at 03:09:21PM -0400, Derek Buitenhuis wrote: >> >> Signed-off-by: Derek Buitenhuis >> >> --- >> >> doc/muxers.texi | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/doc/muxers.texi b/doc/muxers.texi >> >> index 06483fa..cef04e1 100644 >> >> --- a/doc/muxers.texi >> >> +++ b/doc/muxers.texi >> >> @@ -802,9 +802,9 @@ Set a constant muxrate (default VBR). >> >> Override the default PCR retransmission time (default 20ms), ignored >> >> if variable muxrate is selected. >> >> @item pat_period @var{number} >> >> -Maximal time in seconds between PAT/PMT tables. >> >> +Maximal time in seconds between PAT/PMT tables. Max value is INT_MAX / 2 >> >> - 1. >> >> @item sdt_period @var{number} >> >> -Maximal time in seconds between SDT tables. >> >> +Maximal time in seconds between SDT tables. Max value is INT_MAX / 2 - 1. >> > >> > iam unsure about this, INT_MAX is theoretically platform specific and >> > most non developers probably dont know its value >> > that said, iam not against it at all, just feels like this cant be >> > the best solution >> >> Certainly not the best, but definitely better than the current - range >> restrictions should be documented. Furthermore, INT_MAX is pretty well >> known even for a first exposure to C, and in all likelihood someone >> who reads the muxer docs is sharp/knows enough to do a quick google on >> what INT_MAX is (after all, he was savvy enough to look at the docs). >> >> Maybe one could in principle use configure to get the relevant actual >> value and use that in the docs. That is pretty complex and should be >> done IMHO only if there are more such type-width specific limits. >> Furthermore, that has problems, if docs were obtained off a website >> (website's config might be different from user's, etc). Given this, I >> still favor the INT_MAX solution. > > theres mayybe no need to document the exact maximum, stating that 2^30 > is the max could be fine (its 34 years) or the code could be changed as > well to not have "INT_MAX / 2 - 1" as maximum > theres also the aspect that if int is 64bit, INT_MAX / 2 - 1 cannot > exactly be represented as IEEE 64bit double, i doubt that matters but > this patchset seemed to have the goal to get this precissely correct There are a number of such things, but I think they belong to a separate patch. > > >> >> Also, on that note, it has already been noted that on all platforms we >> have (currently) int is 32 bits. A lot more serious things will break >> than the docs if the width of ints changes. That is another thing in >> INT_MAX's favor: it won't break on such a change :). > > what breaks with 64bit int ? Look at ff_ctz_c (before or after the De-Bruijn commit) - it does not compute the right thing if int is 64 bits. Whether this causes "breakage" I can't say, but the code is definitely incorrect. I am sure there are plenty of other such instances, and out of those it is likely one of them actually causes breakage to some CLI usage, API usage, etc. > (note if you say x86 asm then the question is which x86 ABI allows 64 > bit int, if thats none then non x86 is implied by 64bit int) > > also is there a way to (easily) test a build with 64bit int ? I don't know, but I am eager to know this as well. In particular, I would be curious to test my hypothesis above. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you think the mosad wants you dead since a long time then you are either > wrong or dead since a long time. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2] avformat/movenc: suppress -Wstrict-overflow warnings
On Sat, Oct 17, 2015 at 1:51 PM, Ronald S. Bultjewrote: > Hi, > > On Fri, Oct 16, 2015 at 8:45 PM, Ganesh Ajjanagadde > wrote: > >> On Fri, Oct 16, 2015 at 8:34 PM, Ronald S. Bultje >> wrote: >> > Hi, >> > >> > On Fri, Oct 16, 2015 at 8:18 PM, Ganesh Ajjanagadde >> > wrote: >> > >> >> On Fri, Oct 16, 2015 at 8:05 PM, Ronald S. Bultje >> >> wrote: >> >> > Hi, >> >> > >> >> > On Fri, Oct 16, 2015 at 5:48 PM, Ganesh Ajjanagadde > > >> >> > wrote: >> >> > >> >> >> On Fri, Oct 16, 2015 at 5:45 PM, Hendrik Leppkes < >> h.lepp...@gmail.com> >> >> >> wrote: >> >> >> > On Fri, Oct 16, 2015 at 11:39 PM, Ganesh Ajjanagadde >> >> >> > wrote: >> >> >> >> On Wed, Oct 14, 2015 at 10:05 PM, Ganesh Ajjanagadde >> >> >> >> wrote: >> >> >> >>> This patch results in identical behavior of movenc, and >> suppresses >> >> >> -Wstrict-overflow >> >> >> >>> warnings observed in GCC 5.2: >> >> >> >>> >> >> >> >> >> >> http://fate.ffmpeg.org/log.cgi?time=20150926231053=compile=x86_64-archlinux-gcc-threads-misc >> >> >> , >> >> >> >>> "warning: assuming signed overflow does not occur when assuming >> that >> >> >> (X - c) > X is always false [-Wstrict-overflow]" >> >> >> >>> I have manually checked that all usages are safe, and overflow >> >> >> possibility does >> >> >> >>> not exist with this expression rewrite. >> >> >> >>> >> >> >> >>> Some expressed concern over readability loss, hence a comment is >> >> added. >> >> >> >>> This is the simplest way to suppress this warning. >> >> >> >>> >> >> >> >>> Signed-off-by: Ganesh Ajjanagadde >> >> >> >>> --- >> >> >> >>> libavformat/movenc.c | 4 +++- >> >> >> >>> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> >> >>> >> >> >> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> >> >> >>> index 5115585..ff997f2 100644 >> >> >> >>> --- a/libavformat/movenc.c >> >> >> >>> +++ b/libavformat/movenc.c >> >> >> >>> @@ -854,7 +854,9 @@ static int get_cluster_duration(MOVTrack >> *track, >> >> >> int cluster_idx) >> >> >> >>> { >> >> >> >>> int64_t next_dts; >> >> >> >>> >> >> >> >>> -if (cluster_idx >= track->entry) >> >> >> >>> +/* GCC 5.2 wants to "optimize" cluster_idx >= track->entry >> to >> >> the >> >> >> below >> >> >> >>> + * expression. We actually mean cluster_idx >= >> track->entry. */ >> >> >> >>> +if (cluster_idx - track->entry >= 0) >> >> >> >>> return 0; >> >> >> >>> >> >> >> >>> if (cluster_idx + 1 == track->entry) >> >> >> >>> -- >> >> >> >>> 2.6.1 >> >> >> >>> >> >> >> >> >> >> >> >> ping, is this solution acceptable? Note that I will get rid of the >> >> >> >> extra * on the second line of the comment. >> >> >> > >> >> >> > Not a fan myself of any such hacks to get compilers to shut up. Is >> GCC >> >> >> > doing something clearly wrong here, and the false warning should be >> >> >> > reported? >> >> >> >> >> >> I gave an (extremely detailed) explanation of what was going on >> >> >> earlier in the thread: >> >> >> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180976.html. >> >> >> The punch line is that it is not clearly wrong, and alternatives for >> >> >> warning suppression are less clean. >> >> > >> >> > >> >> > And I responded in the same thread: >> >> > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180977.html >> >> > >> >> > I'm still not a fan of this either. I don't think clang should be >> doing >> >> > what it's doing here. >> >> >> >> Neither am I a fan of this whole business. I try to make do with what >> >> is feasible, clean, and easy to revert at a future stage. >> >> >> >> My report was with gcc, have not checked clang. Funny that both of >> >> them try to do this. >> >> >> >> What alternative do you propose? Just keep the warning as it is, and >> >> forget suppressing it? >> >> >> >> Also, I would like to point out the following. Just see the following >> >> lines in avcodec/aacdec_template.c: >> >> /* This assignment is to silence a GCC warning about the variable >> >> being used >> >> * uninitialized when in fact it always is. >> >> */ >> >> pulse.num_pulse = 0; >> >> >> >> I have tested, and confirmed that only compilers strictly prior to GCC >> >> 4.6 actually complain. I showed this to Michael, and he did not feel >> >> like removing this ridiculous bit of code. It was in light of this >> >> that I crafted a patch with a comment, and thought people would be >> >> fine with it. >> > >> > >> > Sometimes these things are just based on personal opinions. :( I >> personally >> > wouldn't mind removing the assignment if recent gcc versions don't >> complain >> > anymore. >> >> I guess the reason why I am keen on the movenc suppression is that I >> highly value compiler warnings, and am trying to move FFmpeg towards a >> clean build (I have outlined my rationale before). >> >>
Re: [FFmpeg-devel] [PATCH 1/2] lavc/samidec: support multiple paragraphs in a packet
On Sat, Oct 10, 2015 at 11:05:49PM -0500, Rodger Combs wrote: > --- > libavcodec/samidec.c | 20 ++-- > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/samidec.c b/libavcodec/samidec.c > index 95f35ab..8dd2749 100644 > --- a/libavcodec/samidec.c > +++ b/libavcodec/samidec.c > @@ -46,6 +46,7 @@ static int sami_paragraph_to_ass(AVCodecContext *avctx, > const char *src) > char *p = dupsrc; > AVBPrint *dst_content = >encoded_content; > AVBPrint *dst_source = >encoded_source; > +int got_content = 0; > > av_bprint_clear(>encoded_content); > av_bprint_clear(>content); > @@ -76,17 +77,18 @@ static int sami_paragraph_to_ass(AVCodecContext *avctx, > const char *src) > av_bprint_clear(dst); > } > > -/* if empty event -> skip subtitle */ > -while (av_isspace(*p)) > +while (av_isspace(*p) || (!strncmp(p, "", 6) && (p += 5))) > p++; > -if (!strncmp(p, "", 6)) { > -ret = -1; > -goto end; > -} this looks independant of the rest of te patch > > /* extract the text, stripping most of the tags */ > while (*p) { > if (*p == '<') { > +if (!av_strncasecmp(p, "", 4)) { > +p += 4; > +while (av_isspace(*p)) > +p++; here av_isspace is used without checking for nbsp;, is that intended ? otherwise this looks ok to someone (me) not knowing samidec deeper [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2] avformat/movenc: suppress -Wstrict-overflow warnings
Hi, On Fri, Oct 16, 2015 at 8:45 PM, Ganesh Ajjanagaddewrote: > On Fri, Oct 16, 2015 at 8:34 PM, Ronald S. Bultje > wrote: > > Hi, > > > > On Fri, Oct 16, 2015 at 8:18 PM, Ganesh Ajjanagadde > > wrote: > > > >> On Fri, Oct 16, 2015 at 8:05 PM, Ronald S. Bultje > >> wrote: > >> > Hi, > >> > > >> > On Fri, Oct 16, 2015 at 5:48 PM, Ganesh Ajjanagadde > > >> > wrote: > >> > > >> >> On Fri, Oct 16, 2015 at 5:45 PM, Hendrik Leppkes < > h.lepp...@gmail.com> > >> >> wrote: > >> >> > On Fri, Oct 16, 2015 at 11:39 PM, Ganesh Ajjanagadde > >> >> > wrote: > >> >> >> On Wed, Oct 14, 2015 at 10:05 PM, Ganesh Ajjanagadde > >> >> >> wrote: > >> >> >>> This patch results in identical behavior of movenc, and > suppresses > >> >> -Wstrict-overflow > >> >> >>> warnings observed in GCC 5.2: > >> >> >>> > >> >> > >> > http://fate.ffmpeg.org/log.cgi?time=20150926231053=compile=x86_64-archlinux-gcc-threads-misc > >> >> , > >> >> >>> "warning: assuming signed overflow does not occur when assuming > that > >> >> (X - c) > X is always false [-Wstrict-overflow]" > >> >> >>> I have manually checked that all usages are safe, and overflow > >> >> possibility does > >> >> >>> not exist with this expression rewrite. > >> >> >>> > >> >> >>> Some expressed concern over readability loss, hence a comment is > >> added. > >> >> >>> This is the simplest way to suppress this warning. > >> >> >>> > >> >> >>> Signed-off-by: Ganesh Ajjanagadde > >> >> >>> --- > >> >> >>> libavformat/movenc.c | 4 +++- > >> >> >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >> >> >>> > >> >> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c > >> >> >>> index 5115585..ff997f2 100644 > >> >> >>> --- a/libavformat/movenc.c > >> >> >>> +++ b/libavformat/movenc.c > >> >> >>> @@ -854,7 +854,9 @@ static int get_cluster_duration(MOVTrack > *track, > >> >> int cluster_idx) > >> >> >>> { > >> >> >>> int64_t next_dts; > >> >> >>> > >> >> >>> -if (cluster_idx >= track->entry) > >> >> >>> +/* GCC 5.2 wants to "optimize" cluster_idx >= track->entry > to > >> the > >> >> below > >> >> >>> + * expression. We actually mean cluster_idx >= > track->entry. */ > >> >> >>> +if (cluster_idx - track->entry >= 0) > >> >> >>> return 0; > >> >> >>> > >> >> >>> if (cluster_idx + 1 == track->entry) > >> >> >>> -- > >> >> >>> 2.6.1 > >> >> >>> > >> >> >> > >> >> >> ping, is this solution acceptable? Note that I will get rid of the > >> >> >> extra * on the second line of the comment. > >> >> > > >> >> > Not a fan myself of any such hacks to get compilers to shut up. Is > GCC > >> >> > doing something clearly wrong here, and the false warning should be > >> >> > reported? > >> >> > >> >> I gave an (extremely detailed) explanation of what was going on > >> >> earlier in the thread: > >> >> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180976.html. > >> >> The punch line is that it is not clearly wrong, and alternatives for > >> >> warning suppression are less clean. > >> > > >> > > >> > And I responded in the same thread: > >> > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180977.html > >> > > >> > I'm still not a fan of this either. I don't think clang should be > doing > >> > what it's doing here. > >> > >> Neither am I a fan of this whole business. I try to make do with what > >> is feasible, clean, and easy to revert at a future stage. > >> > >> My report was with gcc, have not checked clang. Funny that both of > >> them try to do this. > >> > >> What alternative do you propose? Just keep the warning as it is, and > >> forget suppressing it? > >> > >> Also, I would like to point out the following. Just see the following > >> lines in avcodec/aacdec_template.c: > >> /* This assignment is to silence a GCC warning about the variable > >> being used > >> * uninitialized when in fact it always is. > >> */ > >> pulse.num_pulse = 0; > >> > >> I have tested, and confirmed that only compilers strictly prior to GCC > >> 4.6 actually complain. I showed this to Michael, and he did not feel > >> like removing this ridiculous bit of code. It was in light of this > >> that I crafted a patch with a comment, and thought people would be > >> fine with it. > > > > > > Sometimes these things are just based on personal opinions. :( I > personally > > wouldn't mind removing the assignment if recent gcc versions don't > complain > > anymore. > > I guess the reason why I am keen on the movenc suppression is that I > highly value compiler warnings, and am trying to move FFmpeg towards a > clean build (I have outlined my rationale before). > > I admit, initially I was very naive: in personal projects I always > have -Werror, -pedantic, -Wall, -Wextra, -Wshadow and god knows what > :). I wondered why FFmpeg is not as liberal and why warnings are not >
Re: [FFmpeg-devel] [PATCH] avcodec/xface: suppress -Wstrict-overflow warnings
On Sat, Sep 26, 2015 at 11:05 AM, Ganesh Ajjanagaddewrote: > On Fri, Sep 18, 2015 at 5:31 PM, Ganesh Ajjanagadde > wrote: >> This patch results in identical behavior of xface, and suppresses >> -Wstrict-overflow >> warnings observed in GCC 5.2. >> I have manually checked that this usage is safe, and overflow possibility >> does >> not exist with this expression rewrite. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- >> libavcodec/xface.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/xface.c b/libavcodec/xface.c >> index 8c0cbfd..78b6def 100644 >> --- a/libavcodec/xface.c >> +++ b/libavcodec/xface.c >> @@ -315,7 +315,7 @@ void ff_xface_generate_face(uint8_t *dst, uint8_t * >> const src) >> >> for (l = i - 2; l <= i + 2; l++) { >> for (m = j - 2; m <= j; m++) { >> -if (l >= i && m == j) >> +if ((l - i >= 0) && m == j) >> continue; >> if (l > 0 && l <= XFACE_WIDTH && m > 0) >> k = 2*k + src[l + m * XFACE_WIDTH]; >> -- >> 2.5.2 >> > > ping patch dropped, see the movenc one for reasons why. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] lavc/samidec: support Hulu subtitle encryption
On Sat, Oct 10, 2015 at 11:05:50PM -0500, Rodger Combs wrote: > hex_to_data should probably move to lavu before this is merged. > > This is probably a good case for sub_charenc to run _after_ the decoder. > > I could see an argument that this should go in the demuxer instead. Thoughts? > --- > libavcodec/samidec.c | 78 > > 1 file changed, 78 insertions(+) [...] > @@ -158,6 +207,18 @@ static av_cold int sami_init(AVCodecContext *avctx) > av_bprint_init(>encoded_source, 0, 2048); > av_bprint_init(>encoded_content, 0, 2048); > av_bprint_init(>full,0, 2048); > + > +if (sami->key && sami->iv && *sami->iv && !sami->iv_size) > +sami->iv_size = strlen(sami->iv); > + > +if (sami->key_size && sami->iv_size) { > +sami->aes = av_aes_alloc(); > +if (!sami->aes) > +return ENOMEM; AVERROR(ENOMEM) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH][RFC] avcodec: disallow hwaccel with frame threads
--- libavcodec/utils.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 0e4f3c0..52b480a 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -1006,6 +1006,12 @@ static int setup_hwaccel(AVCodecContext *avctx, AVHWAccel *hwa = find_hwaccel(avctx->codec_id, fmt); int ret= 0; +if (avctx->active_thread_type & FF_THREAD_FRAME) { +av_log(avctx, AV_LOG_ERROR, + "Hardware accelerated decoding with frame threading is not supported.\n"); +return AVERROR(EINVAL); +} + if (!hwa) { av_log(avctx, AV_LOG_ERROR, "Could not find an AVHWAccel for the pixel format: %s", -- 2.5.3.windows.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH][RFC] avcodec: disallow hwaccel with frame threads
On Sat, Oct 17, 2015 at 9:27 PM, Hendrik Leppkeswrote: > --- > libavcodec/utils.c | 6 ++ > 1 file changed, 6 insertions(+) > Above patch is submitted as an RFC, however I strongly believe its the only way to keep hwaccels sane in the future. There are several known problems when a hwaccel is used with frame threading, at least with DXVA2, ranging from simple image corruption to crashes in the GPU drivers. All the problems essentially come down to two factors: (1) while avcodec tries to prevent simultaneous access from different threads, it cannot control user code. As a API user you have no chance to avoid simultaneous access to the hardware surfaces with frame threading, because as soon as avcodec hands you a decoded surface, it'll already have started working on the next one. This is a common source for image corruption, as the decoder may fail to get a reference frame if its currently locked by the user code. This issue is not really fixable, other than introducing a mutex around every call that the user code would have to lock as well. API break and making hwaccel even more complex to use. (2) decoders often have had (or still have) trouble initializing the hwaccel properly with multiple threads, which can result in multiple threads re-creating the hwaccel and disrupting decoding, or even crashing in the driver. While this is in theory fixable inside the decoders in question, it can lead to quite a bit of complexity, which should be avoided. We should not be offering a mode that is known to be broken and even crash (even if the crash is not in our code). On top of that, frame threading with hwaccel does not actually improve the speed at all, if anything it might be slightly slower, and will definitely use more GPU memory. The only reason this combination even exists is because API users insisted on it, and it was then added without properly taking the consequences into account. There is an easy solution to keep using frame threads with software decoding while using a hwaccel otherwise - its used by myself, by mpv and by Kodi - simply re-open the decoder with the new threading flags. Its really not a lot of work, and keeps the code sane and the decoding reliable. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH][RFC] avcodec: disallow hwaccel with frame threads
On Sat, 17 Oct 2015 21:28:50 +0200 Hendrik Leppkeswrote: > On Sat, Oct 17, 2015 at 9:27 PM, Hendrik Leppkes wrote: > > --- > > libavcodec/utils.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > Above patch is submitted as an RFC, however I strongly believe its the > only way to keep hwaccels sane in the future. > There are several known problems when a hwaccel is used with frame > threading, at least with DXVA2, ranging from simple image corruption > to crashes in the GPU drivers. > > All the problems essentially come down to two factors: > > (1) while avcodec tries to prevent simultaneous access from different > threads, it cannot control user code. > As a API user you have no chance to avoid simultaneous access to the > hardware surfaces with frame threading, because as soon as avcodec > hands you a decoded surface, > it'll already have started working on the next one. This is a common > source for image corruption, as the decoder may fail to get a > reference frame if its currently > locked by the user code. > > This issue is not really fixable, other than introducing a mutex > around every call that the user code would have to lock as well. > API break and making hwaccel even more complex to use. > > (2) decoders often have had (or still have) trouble initializing the > hwaccel properly with multiple threads, which can result in multiple > threads re-creating the hwaccel > and disrupting decoding, or even crashing in the driver. > > While this is in theory fixable inside the decoders in question, it > can lead to quite a bit of complexity, which should be avoided. > > We should not be offering a mode that is known to be broken and even > crash (even if the crash is not in our code). > On top of that, frame threading with hwaccel does not actually improve > the speed at all, if anything it might be slightly slower, and will > definitely use more GPU memory. > > The only reason this combination even exists is because API users > insisted on it, and it was then added without properly taking the > consequences into account. > > There is an easy solution to keep using frame threads with software > decoding while using a hwaccel otherwise - its used by myself, by mpv > and by Kodi - simply re-open the decoder with the new threading flags. > Its really not a lot of work, and keeps the code sane and the decoding > reliable. I agree with all points. Even if you'd consider this worth fixing (and only so that the software fallback actually works?), there are many practical problems with making it work. Let's just disallow it. An alternative might be transparently disabling threading while a hwaccel is active (and using threading normally when doing sw decoding). Maybe someone comes up with a simply patch to achieve this. But until then, this patch should be applied, since multithreaded hw accel simply doesn't work and can crash. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2] avformat/movenc: suppress -Wstrict-overflow warnings
> -if (cluster_idx >= track->entry) > +/* GCC 5.2 wants to "optimize" cluster_idx >= track->entry to the below > + * expression. We actually mean cluster_idx >= track->entry. */ > +if (cluster_idx - track->entry >= 0) > return 0; On Sat, Oct 17, 2015 at 11:04 AM, Ganesh Ajjanagaddewrote: > Yes, it is really a questions of where on the ROC curve you want to > land, and as such has no technical solution. Since there seems to be > almost universal consensus against my proposal in this case, consider > the patch dropped. I may return to it in a few years :). Just a suggestion but I think the key thing to avoid in future patches is changing an obviously correct check to something else that has the potential to introduce new undefined behavior or subtle bugs. For example the original check here is the obvious check to ensure that access to the cluster array cannot occur at or beyond index track->entry. The proposed replacement cannot ensure that because the arithmetic has the potential to overflow and produce the wrong result, even without any compiler optimizations that eliminate the check. So this is simply replacing a potential for undefined behavior that is impossible (provable by looking only at this source file) but gcc (and not clang) falsely warn about, with a potential for new undefined behavior that gcc doesn't warn about even though it cannot be ruled out, at least not without analyzing other source files. As an example, suppose it was possible for some other source file to set track->entry to INT_MIN. Previously the comparison would be true and it would safely return 0, but with the proposed change it would result in undefined behavior and may attempt to access entries in the cluster array that do not exist. This is different than quieting a warning by unnecessarily initializing a local variable to 0, because changing a local variable from uninitialized to initialized does not introduce any new undefined behavior. - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat: add msf demuxer
Signed-off-by: Paul B Mahol--- libavformat/Makefile | 1 + libavformat/allformats.c | 1 + libavformat/msf.c| 88 3 files changed, 90 insertions(+) create mode 100644 libavformat/msf.c diff --git a/libavformat/Makefile b/libavformat/Makefile index ded2d54..7886b05 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -280,6 +280,7 @@ OBJS-$(CONFIG_MPEGVIDEO_DEMUXER) += mpegvideodec.o rawdec.o OBJS-$(CONFIG_MPJPEG_DEMUXER)+= mpjpegdec.o OBJS-$(CONFIG_MPJPEG_MUXER) += mpjpeg.o OBJS-$(CONFIG_MPL2_DEMUXER) += mpl2dec.o subtitles.o +OBJS-$(CONFIG_MSF_DEMUXER) += msf.o OBJS-$(CONFIG_MPSUB_DEMUXER) += mpsubdec.o subtitles.o OBJS-$(CONFIG_MSNWC_TCP_DEMUXER) += msnwc_tcp.o OBJS-$(CONFIG_MTV_DEMUXER) += mtv.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 40fea8e..f238118 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -206,6 +206,7 @@ void av_register_all(void) REGISTER_MUXDEMUX(MPJPEG, mpjpeg); REGISTER_DEMUXER (MPL2, mpl2); REGISTER_DEMUXER (MPSUB,mpsub); +REGISTER_DEMUXER (MSF, msf); REGISTER_DEMUXER (MSNWC_TCP,msnwc_tcp); REGISTER_DEMUXER (MTV, mtv); REGISTER_DEMUXER (MV, mv); diff --git a/libavformat/msf.c b/libavformat/msf.c new file mode 100644 index 000..292ae3c --- /dev/null +++ b/libavformat/msf.c @@ -0,0 +1,88 @@ +/* + * MSF demuxer + * Copyright (c) 2015 Paul B Mahol + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "avformat.h" +#include "internal.h" + +static int msf_probe(AVProbeData *p) +{ +if (memcmp(p->buf, "MSF", 3)) +return 0; + +return AVPROBE_SCORE_MAX; +} + +static int msf_read_header(AVFormatContext *s) +{ +unsigned codec, align, size; +AVStream *st; + +avio_skip(s->pb, 4); + +st = avformat_new_stream(s, NULL); +if (!st) +return AVERROR(ENOMEM); + +st->codec->codec_type = AVMEDIA_TYPE_AUDIO; +codec = avio_rb32(s->pb); +st->codec->channels= avio_rb32(s->pb); +if (st->codec->channels <= 0) +return AVERROR_INVALIDDATA; +size = avio_rb32(s->pb); +st->codec->sample_rate = avio_rb32(s->pb); +if (st->codec->sample_rate <= 0) +return AVERROR_INVALIDDATA; +align = avio_rb32(s->pb) ; +if (align > INT_MAX / st->codec->channels) +return AVERROR_INVALIDDATA; +st->codec->block_align = align; +st->duration = av_get_audio_frame_duration(st->codec, size); +switch (codec) { +case 0: st->codec->codec_id = AV_CODEC_ID_PCM_S16BE; break; +case 3: st->codec->block_align = 16 * st->codec->channels; +st->codec->codec_id = AV_CODEC_ID_ADPCM_PSX; break; +case 7: st->need_parsing = AVSTREAM_PARSE_FULL_RAW; +st->codec->codec_id = AV_CODEC_ID_MP3; break; +default: +avpriv_request_sample(s, "Codec %d", codec); +return AVERROR_PATCHWELCOME; +} +avio_skip(s->pb, 0x40 - avio_tell(s->pb)); +avpriv_set_pts_info(st, 64, 1, st->codec->sample_rate); + +return 0; +} + +static int msf_read_packet(AVFormatContext *s, AVPacket *pkt) +{ +AVCodecContext *codec = s->streams[0]->codec; + +return av_get_packet(s->pb, pkt, codec->block_align ? codec->block_align : 1024 * codec->channels); +} + +AVInputFormat ff_msf_demuxer = { +.name = "msf", +.long_name = NULL_IF_CONFIG_SMALL("MSF"), +.read_probe = msf_probe, +.read_header= msf_read_header, +.read_packet= msf_read_packet, +.extensions = "msf", +}; -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] WIP/RFC: Try to avoid decoding a frame to get the codec parameters
Hello, As of now, avformat_find_info_stream decodes a frame in order to get the codec parameters (width, height, pix_fmt (mandatory) and more depending on the codec) which leads to having images decoded twice. The following patchset addresses this issue: * by using the AVCodecContext skip_frame field and setting its value to AVDISCARD_ALL in the try_decode_frame function in lavformat/utils.c * by having the decoders interpretting this field a bit more differently: While they are honoring the skip_frame option, they *must* still extract and set the relevant codec parameters to the AVCodecContext. Regarding the last patch which set the avctx->skip_frame to AVDISCARD_ALL, i've choosen to whitelist the codecs that support this behaviour. A blacklist system (that would reflect a todolist) can also be used but it's a bit more risky as we can't miss any codec ids to not break the current probing behaviour. Matthieu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] WIP: lavf/utils: try to avoid decoding a frame to get the codec parameters
From: Matthieu BouronAvoid decoding twice images such as jpeg and png, once in the avformat_find_stream_info and once when the actual decode is made. The decoder must honor the skip_frame option in order to skip decoding. For now the AVDISCARD_ALL flag is only set for the mjpeg and png decoders. --- libavformat/utils.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/libavformat/utils.c b/libavformat/utils.c index 689473e..67dfffc 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -2676,11 +2676,16 @@ static int has_codec_parameters(AVStream *st, const char **errmsg_ptr) static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket *avpkt, AVDictionary **options) { +int i; const AVCodec *codec; int got_picture = 1, ret = 0; AVFrame *frame = av_frame_alloc(); AVSubtitle subtitle; AVPacket pkt = *avpkt; +int skip_frame; +static const enum AVCodecID no_decode_codecs[] = { +AV_CODEC_ID_MJPEG, AV_CODEC_ID_PNG, +}; if (!frame) return AVERROR(ENOMEM); @@ -2719,6 +2724,14 @@ static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket *avpkt, goto fail; } +skip_frame = st->codec->skip_frame; +for (i = 0; i < FF_ARRAY_ELEMS(no_decode_codecs); i++) { +if (st->codec->codec_id == no_decode_codecs[i]) { +st->codec->skip_frame = AVDISCARD_ALL; +break; +} +} + while ((pkt.size > 0 || (!pkt.data && got_picture)) && ret >= 0 && (!has_codec_parameters(st, NULL) || !has_decode_delay_been_guessed(st) || @@ -2753,6 +2766,8 @@ static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket *avpkt, if (!pkt.data && !got_picture) ret = -1; +st->codec->skip_frame = skip_frame; + fail: av_frame_free(); return ret; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] lavc/pngdec: honor skip_frame option
From: Matthieu Bouron--- libavcodec/pngdec.c | 34 ++ 1 file changed, 34 insertions(+) diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c index 4cfdc58..5b2befe 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -1087,6 +1087,13 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, for (;;) { length = bytestream2_get_bytes_left(>gb); if (length <= 0) { + +if (avctx->codec_id == AV_CODEC_ID_PNG && +avctx->skip_frame == AVDISCARD_ALL) { +av_frame_set_metadata(p, metadata); +return 0; +} + if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && length == 0) { if (!(s->state & PNG_IDAT)) return 0; @@ -1114,6 +1121,22 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s, ((tag >> 8) & 0xff), ((tag >> 16) & 0xff), ((tag >> 24) & 0xff), length); + +if (avctx->codec_id == AV_CODEC_ID_PNG && +avctx->skip_frame == AVDISCARD_ALL) { +int i, found = 0; +static const int tags[] = { MKTAG('I', 'H', 'D', 'R'), MKTAG('t', 'E', 'X', 't'), MKTAG('I', 'D', 'A', 'T') }; +for (i = 0; i < FF_ARRAY_ELEMS(tags); i++) { +if (tag == tags[i]) { +found = 1; +break; +} +} +if (!found) { +goto skip_tag; +} +} + switch (tag) { case MKTAG('I', 'H', 'D', 'R'): if ((ret = decode_ihdr_chunk(avctx, s, length)) < 0) @@ -1181,6 +1204,11 @@ skip_tag: } } exit_loop: +if (avctx->codec_id == AV_CODEC_ID_PNG && +avctx->skip_frame == AVDISCARD_ALL) { +av_frame_set_metadata(p, metadata); +return 0; +} if (s->bits_per_pixel <= 4) handle_small_bpp(s, p); @@ -1278,6 +1306,12 @@ static int decode_frame_png(AVCodecContext *avctx, if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0) goto the_end; +if (avctx->skip_frame == AVDISCARD_ALL) { +*got_frame = 0; +ret = bytestream2_tell(>gb); +goto the_end; +} + if ((ret = av_frame_ref(data, s->picture.f)) < 0) return ret; -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] lavc/mjpegdec: honor skip_frame option
From: Matthieu Bouron--- libavcodec/mjpegdec.c | 20 1 file changed, 20 insertions(+) diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 1a86b7b..8a90e94 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -2038,6 +2038,21 @@ int ff_mjpeg_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, return AVERROR(ENOSYS); } +if (avctx->skip_frame == AVDISCARD_ALL) { +int i, found = 0; +static const int sofs[] = { SOF0, SOF1, SOF2, SOF3, SOF48 }; + +for (i = 0; i < FF_ARRAY_ELEMS(sofs); i++) { +if (start_code == sofs[i]) { +found = 1; +break; +} +} +if (!found) { +continue; +} +} + switch (start_code) { case SOI: s->restart_interval = 0; @@ -2158,6 +2173,11 @@ eoi_parser: av_log(avctx, AV_LOG_WARNING, "EOI missing, emulating\n"); goto eoi_parser; } + +if (avctx->skip_frame == AVDISCARD_ALL) { +return buf_ptr - buf; +} + av_log(avctx, AV_LOG_FATAL, "No JPEG data found in image\n"); return AVERROR_INVALIDDATA; fail: -- 2.6.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] chromakey: Use the pixel descriptor API for chroma subsampling info
On Tue, Oct 13, 2015 at 9:33 PM Timothy Guwrote: > --- > libavfilter/vf_chromakey.c | 25 + > 1 file changed, 17 insertions(+), 8 deletions(-) > Reviewed by Timo on IRC and pushed. Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avfilter/internal: add av_warn_unused_result
On Wed, Oct 14, 2015 at 11:02:45PM -0400, Ganesh Ajjanagadde wrote: > av_warn_unused_result is added to functions whose return status should > be checked. Currently does not trigger any warnings, but should be > useful for future robustness. > > Signed-off-by: Ganesh Ajjanagadde> --- > libavfilter/internal.h | 5 + > 1 file changed, 5 insertions(+) LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes 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/2] avfilter/internal: Doxygen for ff_fmt_is_in
On Thu, Oct 15, 2015 at 6:26 AM, Ganesh Ajjanagaddewrote: > On Thu, Oct 15, 2015 at 1:45 AM, Timothy Gu wrote: >> On Wed, Oct 14, 2015 at 8:05 PM Ganesh Ajjanagadde >> wrote: >>> This clarifies and Doxygen's the comment for ff_fmt_is_in. >> >> ^ >> gajjanag.c:1:23: error: unexpected token ‘adjective’ > > sorry, missing something - does "This clarifies and adds Doxygen for > ff_fmt_is_in" fix it? Timothy: can you check this? Alternatively, can you tell me what tool you used so that I can check locally? > >> >>> >>> >>> Signed-off-by: Ganesh Ajjanagadde >>> --- >>> libavfilter/internal.h | 8 +++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavfilter/internal.h b/libavfilter/internal.h >>> index bb94707..c07d306 100644 >>> --- a/libavfilter/internal.h >>> +++ b/libavfilter/internal.h >>> @@ -152,7 +152,13 @@ struct AVFilterInternal { >>> avfilter_execute_func *execute; >>> }; >>> >>> -/** Tell is a format is contained in the provided list terminated by -1. >>> */ >>> +/** >> >> >>> >>> + * Tell if a format is contained in the provided -1 terminated list of >>> formats. >> >> >> -1-terminated >> >> Also what's the format supposed to be? If it can be used for all integers >> (as the source seems to indicate) then say "tell if an integer is contained >> … list of integers. This is useful for determining if BLAH format is in an >> array of supported formats." > > Yes, the source is weird. If it is a pixel format, shouldn't the type > signature have an enum instead of an int? At the moment though, you > are completely right - will update and send new patch. Thanks. > >> >>> >>> + * >>> + * @param fmt provided format >>> + * @param fmts -1 terminated list of formats >>> + * @return 1 if present, 0 if absent >>> + */ >>> int ff_fmt_is_in(int fmt, const int *fmts); >> >> >> Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/opt: display a better default value for int/int64 options
On Sat, Oct 17, 2015 at 14:54:31 +0200, Clément Bœsch wrote: > % ./ffmpeg -h encoder=aac > -aac_coder E...A... Coding algorithm (from -1 to 3) > (default twoloop) > faac E...A... FAAC-inspired method > anmr E...A... ANMR method > twoloop E...A... Two loop searching method > fast E...A... Constant quantizer If it could also map the numbers ("from -1 to 3") to the enumerated values, it would make it a lot easier to understand: > -aac_coder E...A... Coding algorithm (from -1 to 3) > (default twoloop (2)) > faac (0) E...A... FAAC-inspired method > anmr (1) E...A... ANMR method > twoloop (2) E...A... Two loop searching method > fast (3) E...A... Constant quantizer Or something like that - I'm not sure in what format I would actually want to see it. No patch provided either, sorry. ;) Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avfilter/internal: add av_warn_unused_result
On Sat, Oct 17, 2015 at 12:05 PM, Michael Niedermayerwrote: > On Wed, Oct 14, 2015 at 11:02:45PM -0400, Ganesh Ajjanagadde wrote: >> av_warn_unused_result is added to functions whose return status should >> be checked. Currently does not trigger any warnings, but should be >> useful for future robustness. >> >> Signed-off-by: Ganesh Ajjanagadde >> --- >> libavfilter/internal.h | 5 + >> 1 file changed, 5 insertions(+) > > LGTM > > thx pushed, thanks. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > No human being will ever know the Truth, for even if they happen to say it > by chance, they would not even known they had done so. -- Xenophanes > > ___ > 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 3/3] doc/muxers: Document range for mpegts periods
On Fri, Oct 16, 2015 at 03:09:21PM -0400, Derek Buitenhuis wrote: > Signed-off-by: Derek Buitenhuis> --- > doc/muxers.texi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/doc/muxers.texi b/doc/muxers.texi > index 06483fa..cef04e1 100644 > --- a/doc/muxers.texi > +++ b/doc/muxers.texi > @@ -802,9 +802,9 @@ Set a constant muxrate (default VBR). > Override the default PCR retransmission time (default 20ms), ignored > if variable muxrate is selected. > @item pat_period @var{number} > -Maximal time in seconds between PAT/PMT tables. > +Maximal time in seconds between PAT/PMT tables. Max value is INT_MAX / 2 - 1. > @item sdt_period @var{number} > -Maximal time in seconds between SDT tables. > +Maximal time in seconds between SDT tables. Max value is INT_MAX / 2 - 1. iam unsure about this, INT_MAX is theoretically platform specific and most non developers probably dont know its value that said, iam not against it at all, just feels like this cant be the best solution [...] -- 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 02/11] avdevice/pulse_audio_common: add av_warn_unused_result
On Thu, Oct 15, 2015 at 10:22:16PM -0400, Ganesh Ajjanagadde wrote: > This does not trigger any warnings, but adds robustness. > > Signed-off-by: Ganesh Ajjanagadde> --- > libavdevice/pulse_audio_common.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavdevice/pulse_audio_common.h > b/libavdevice/pulse_audio_common.h > index 02534f7..902795e 100644 > --- a/libavdevice/pulse_audio_common.h > +++ b/libavdevice/pulse_audio_common.h > @@ -28,8 +28,10 @@ > > pa_sample_format_t ff_codec_id_to_pulse_format(enum AVCodecID codec_id); > > +av_warn_unused_result > int ff_pulse_audio_get_devices(AVDeviceInfoList *devices, const char > *server, int output); > > +av_warn_unused_result > int ff_pulse_audio_connect_context(pa_mainloop **pa_ml, pa_context **pa_ctx, > const char *server, const char > *description); iam not the maintainer of this but the changes look reasonable [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2] avformat/movenc: suppress -Wstrict-overflow warnings
On Sat, Oct 17, 2015 at 3:57 PM, Mark Harris wrote: >> -if (cluster_idx >= track->entry) >> +/* GCC 5.2 wants to "optimize" cluster_idx >= track->entry to the below >> + * expression. We actually mean cluster_idx >= track->entry. */ >> +if (cluster_idx - track->entry >= 0) >> return 0; > > On Sat, Oct 17, 2015 at 11:04 AM, Ganesh Ajjanagaddewrote: >> Yes, it is really a questions of where on the ROC curve you want to >> land, and as such has no technical solution. Since there seems to be >> almost universal consensus against my proposal in this case, consider >> the patch dropped. I may return to it in a few years :). > > Just a suggestion but I think the key thing to avoid in future patches > is changing an obviously correct check to something else that has the > potential to introduce new undefined behavior or subtle bugs. > > For example the original check here is the obvious check to ensure > that access to the cluster array cannot occur at or beyond index > track->entry. The proposed replacement cannot ensure that because the > arithmetic has the potential to overflow and produce the wrong result, > even without any compiler optimizations that eliminate the check. So > this is simply replacing a potential for undefined behavior that is > impossible (provable by looking only at this source file) but gcc (and > not clang) falsely warn about, with a potential for new undefined > behavior that gcc doesn't warn about even though it cannot be ruled > out, at least not without analyzing other source files. > > As an example, suppose it was possible for some other source file to > set track->entry to INT_MIN. Previously the comparison would be true > and it would safely return 0, but with the proposed change it would > result in undefined behavior and may attempt to access entries in the > cluster array that do not exist. No it won't: please read the thread, GCC is already doing some expression massaging as an "optimization", and then warning us afterwards. > > This is different than quieting a warning by unnecessarily > initializing a local variable to 0, because changing a local variable > from uninitialized to initialized does not introduce any new undefined > behavior. The commit message said that I have manually audited it. By the way, that analysis was useful, simply because GCC is already doing it and we need to know that it is safe :). > > - Mark > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] avformat/vag: Remove unused variable pos
From: Michael NiedermayerSigned-off-by: Michael Niedermayer --- libavformat/vag.c |2 -- 1 file changed, 2 deletions(-) diff --git a/libavformat/vag.c b/libavformat/vag.c index 2209711..d3cd5ba 100644 --- a/libavformat/vag.c +++ b/libavformat/vag.c @@ -34,7 +34,6 @@ static int vag_probe(AVProbeData *p) static int vag_read_header(AVFormatContext *s) { AVStream *st; -int64_t pos; st = avformat_new_stream(s, NULL); if (!st) @@ -53,7 +52,6 @@ static int vag_read_header(AVFormatContext *s) st->codec->sample_rate = avio_rb32(s->pb); if (st->codec->sample_rate <= 0) return AVERROR_INVALIDDATA; -pos = avio_tell(s->pb); avio_seek(s->pb, 0x1000, SEEK_SET); if (avio_rl32(s->pb) == MKTAG('V','A','G','p')) { st->codec->block_align = 0x1000 * st->codec->channels; -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] avformat/genh: Remove unused array coef_splitted
From: Michael NiedermayerSigned-off-by: Michael Niedermayer --- libavformat/genh.c |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libavformat/genh.c b/libavformat/genh.c index 3a4faf9..5d34491 100644 --- a/libavformat/genh.c +++ b/libavformat/genh.c @@ -40,7 +40,6 @@ static int genh_read_header(AVFormatContext *s) { unsigned start_offset, header_size, codec, coef_type, coef[2]; GENHDemuxContext *c = s->priv_data; -unsigned coef_splitted[2]; int align, ch; AVStream *st; @@ -105,8 +104,8 @@ static int genh_read_header(AVFormatContext *s) coef[1] = avio_rl32(s->pb); c->dsp_int_type = avio_rl32(s->pb); coef_type= avio_rl32(s->pb); -coef_splitted[0] = avio_rl32(s->pb); -coef_splitted[1] = avio_rl32(s->pb); +/*coef_splitted[0] =*/ avio_rl32(s->pb); +/*coef_splitted[1] =*/ avio_rl32(s->pb); if (st->codec->codec_id == AV_CODEC_ID_ADPCM_THP) { if (st->codec->channels > 2) { -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel