[FFmpeg-devel] [PATCH 5/5] ffmpeg: add abort_on option to allow aborting on empty output

2015-10-17 Thread Marton Balint
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

2015-10-17 Thread Marton Balint
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

2015-10-17 Thread Paul B Mahol
On 10/17/15, Michael Niedermayer  wrote:
> 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

2015-10-17 Thread Paul B Mahol
On 10/17/15, Michael Niedermayer  wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 6:44 PM, Ganesh Ajjanagadde
 wrote:
> 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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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.
___
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()

2015-10-17 Thread Ganesh Ajjanagadde
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

2015-10-17 Thread Timothy Gu
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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
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
 
 # 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

2015-10-17 Thread Timothy Gu
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

2015-10-17 Thread Ganesh Ajjanagadde
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

2015-10-17 Thread Marton Balint
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

2015-10-17 Thread Ganesh Ajjanagadde
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.

>
> 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()

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 7:49 PM, Ronald S. Bultje  wrote:
> 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

2015-10-17 Thread Timothy Gu
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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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

2015-10-17 Thread Timothy Gu
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

2015-10-17 Thread Marton Balint
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

2015-10-17 Thread Timothy Gu
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 <
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Marton Balint


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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 8:41 PM, Michael Niedermayer  wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Marton Balint
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()

2015-10-17 Thread Ronald S. Bultje
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.

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

2015-10-17 Thread Timothy Gu
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

2015-10-17 Thread Timothy Gu
---
 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

2015-10-17 Thread Timothy Gu
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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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()

2015-10-17 Thread Ganesh Ajjanagadde
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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 9:30 PM, 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(-)
>
> 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

2015-10-17 Thread Ganesh Ajjanagadde
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.

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

2015-10-17 Thread Marton Balint


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 Š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.



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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 12:36 PM, Michael Niedermayer
 wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Nicolas George
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

2015-10-17 Thread Clément Bœsch
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

2015-10-17 Thread Hendrik Leppkes
On Fri, Oct 16, 2015 at 12:25 AM, Hendrik Leppkes  wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread wm4
On Sat, 17 Oct 2015 14:02:09 +0200
Michael Niedermayer  wrote:

> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
On Sat, Oct 17, 2015 at 04:18:14PM +0200, wm4 wrote:
> On Sat, 17 Oct 2015 14:02:09 +0200
> Michael Niedermayer  wrote:
> 
> > 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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 1:49 PM, Michael Niedermayer
 wrote:
> 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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 1:51 PM, Ronald S. Bultje  wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Ronald S. Bultje
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).
>
> 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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Sep 26, 2015 at 11:05 AM, Ganesh Ajjanagadde
 wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Hendrik Leppkes
---
 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

2015-10-17 Thread Hendrik Leppkes
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.

- 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

2015-10-17 Thread wm4
On Sat, 17 Oct 2015 21:28:50 +0200
Hendrik Leppkes  wrote:

> 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

2015-10-17 Thread Mark Harris
> -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 Ajjanagadde  wrote:
> 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

2015-10-17 Thread Paul B Mahol
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

2015-10-17 Thread Matthieu Bouron
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

2015-10-17 Thread Matthieu Bouron
From: Matthieu Bouron 

Avoid 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

2015-10-17 Thread Matthieu Bouron
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

2015-10-17 Thread Matthieu Bouron
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

2015-10-17 Thread Timothy Gu
On Tue, Oct 13, 2015 at 9:33 PM Timothy Gu  wrote:

> ---
>  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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Ganesh Ajjanagadde
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?

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

2015-10-17 Thread Moritz Barsnick
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

2015-10-17 Thread Ganesh Ajjanagadde
On Sat, Oct 17, 2015 at 12:05 PM, Michael Niedermayer
 wrote:
> 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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Michael Niedermayer
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

2015-10-17 Thread Ganesh Ajjanagadde
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 Ajjanagadde  wrote:
>> 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

2015-10-17 Thread Michael Niedermayer
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


[FFmpeg-devel] [PATCH 2/2] avformat/genh: Remove unused array coef_splitted

2015-10-17 Thread Michael Niedermayer
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