Re: [FFmpeg-devel] [PATCH] avfilter: do not write to unwritable frame

[Nicolas George]

L'octidi 8 pluviôse, an CCXXV, Muhammad Faiz a écrit :
> affect filters that set partial_buf_size
> test-case
> ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp, asplit 
> [a][b];
> [a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
> [b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
> [a1][b1] vstack'
> 
> Signed-off-by: Muhammad Faiz 
> ---
>  libavfilter/avfilter.c | 17 +
>  1 file changed, 17 insertions(+)

I think there is a much cleaner solution. Please let me have a closer
look at it.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 9/9] boadec: prevent overflow during block alignment calculation

[Ronald S. Bultje]

Hi,

On Sat, Jan 28, 2017 at 7:25 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> On 27.01.2017 03:27, Michael Niedermayer wrote:
> > On Thu, Jan 26, 2017 at 02:14:09AM +0100, Andreas Cadhalpun wrote:
> >> Signed-off-by: Andreas Cadhalpun 
> >> ---
> >>  libavformat/boadec.c | 14 +-
> >>  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > LGTM
>
> Pushed.


Hm ... So I guess I wasn't clear about this, but the reason I didn't reply
to other patches with log messages was not because I'm OK with, but simply
to keep the discussion contained in a single thread and not spam the list.
I'd prefer if the log msg disappeared from all fuzz-only checks...

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

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Marton Balint]

On Sun, 29 Jan 2017, Andreas Cadhalpun wrote:


On 28.01.2017 12:44, Marton Balint wrote:

If we reduce the number of extra lines (not at any cost), I think that helps.
There is also a solution which keeps the traditional C syntax, and is easy to 
undestand even at first glance.

if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
return ff_elog(AVERROR(ENOSYS), s, "Too many channels %d > %d\n", 
st->codecpar->channels, FF_SANE_NB_CHANNELS);


How would you define ff_elog for this to work?



static inline int ff_elog(int error, void *log_ctx, const char *fmt, ...) 
{

if (!CONFIG_SMALL) {
va_list vl;
va_start(vl, fmt);
av_vlog(log_ctx, AV_LOG_ERROR, fmt, vl);
va_end(vl);
}
return error;
}

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

Re: [FFmpeg-devel] [PATCH] avformat: parse iTunes gapless information

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 04:09:20PM -0800, Christopher Snowhill wrote:
> Disregard those two, they were mistaken transcodes. These two are supposed to 
> be gapless, however. Same signal, just not transcoded and downsampled first.

where should they be uploaded to ?
also is there a corresponding patch with fate test to test ?

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH 8/9] xvag: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:13:48AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/xvag.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM assuming the author/maintainer does not object


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


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

Re: [FFmpeg-devel] [PATCH 7/9] epafdec: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:13:33AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/epafdec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM assuming the author/maintainer does not object

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

Re: [FFmpeg-devel] [PATCH 4/9] ircamdec: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:12:19AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/ircamdec.c | 6 ++
>  1 file changed, 6 insertions(+)

LGTM assuming the author/maintainer does not object, maybe he
prefers this without the log message

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri


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/9] genh: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Thu, Jan 26, 2017 at 02:11:54AM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/genh.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM assuming the author/maintainer does not object

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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/9] electronicarts: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 27.01.2017 01:44, Michael Niedermayer wrote:
> On Thu, Jan 26, 2017 at 02:11:31AM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/electronicarts.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 9/9] boadec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 27.01.2017 03:27, Michael Niedermayer wrote:
> On Thu, Jan 26, 2017 at 02:14:09AM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/boadec.c | 14 +-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> LGTM

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 6/9] pvfdec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 26.01.2017 09:37, Paul B Mahol wrote:
> On 1/26/17, Andreas Cadhalpun  wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/pvfdec.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
> 
> lgtm

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 1/9] 4xm: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 26.01.2017 03:21, Michael Niedermayer wrote:
> On Thu, Jan 26, 2017 at 02:11:02AM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/4xm.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> LGTM
> 
> thx

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 29.01.2017 00:26, Paul B Mahol wrote:
> On 1/29/17, Andreas Cadhalpun  wrote:
>> On 28.01.2017 12:44, Marton Balint wrote:
>>> If we reduce the number of extra lines (not at any cost), I think that
>>> helps.
>>> There is also a solution which keeps the traditional C syntax, and is easy
>>> to undestand even at first glance.
>>>
>>> if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
>>> return ff_elog(AVERROR(ENOSYS), s, "Too many channels %d > %d\n",
>>> st->codecpar->channels, FF_SANE_NB_CHANNELS);
>>
>> How would you define ff_elog for this to work?
> 
> I'm maintainer of this file, and I'm fed up with this nuisance conversation.
> I'm against log message.

Fair enough, attached is a patch without the log messages in this file.

However, this discussion is also about error logging in general and it
should come to some conclusion that prevents this nuisance from recurring
for future patches.

Best regards,
Andreas
>From 2386e24e38bbf9847870dfec22998e8fa252e359 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Thu, 15 Dec 2016 02:14:49 +0100
Subject: [PATCH] nistspheredec: prevent overflow during block alignment
 calculation

Signed-off-by: Andreas Cadhalpun 
---
 libavformat/nistspheredec.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
index 782d1dfbfb..588174482c 100644
--- a/libavformat/nistspheredec.c
+++ b/libavformat/nistspheredec.c
@@ -21,6 +21,7 @@
 
 #include "libavutil/avstring.h"
 #include "libavutil/intreadwrite.h"
+#include "libavcodec/internal.h"
 #include "avformat.h"
 #include "internal.h"
 #include "pcm.h"
@@ -90,6 +91,8 @@ static int nist_read_header(AVFormatContext *s)
 return 0;
 } else if (!memcmp(buffer, "channel_count", 13)) {
 sscanf(buffer, "%*s %*s %"SCNd32, >codecpar->channels);
+if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
+return AVERROR(ENOSYS);
 } else if (!memcmp(buffer, "sample_byte_format", 18)) {
 sscanf(buffer, "%*s %*s %31s", format);
 
@@ -109,6 +112,8 @@ static int nist_read_header(AVFormatContext *s)
 sscanf(buffer, "%*s %*s %"SCNd64, >duration);
 } else if (!memcmp(buffer, "sample_n_bytes", 14)) {
 sscanf(buffer, "%*s %*s %"SCNd32, );
+if (bps > (INT_MAX / FF_SANE_NB_CHANNELS) >> 3)
+return AVERROR_INVALIDDATA;
 } else if (!memcmp(buffer, "sample_rate", 11)) {
 sscanf(buffer, "%*s %*s %"SCNd32, >codecpar->sample_rate);
 } else if (!memcmp(buffer, "sample_sig_bits", 15)) {
-- 
2.11.0

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

[FFmpeg-devel] [PATCH] avfilter: add mkmap source filter

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 doc/filters.texi |  39 
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vsrc_mkmap.c | 163 +++
 4 files changed, 204 insertions(+)
 create mode 100644 libavfilter/vsrc_mkmap.c

diff --git a/doc/filters.texi b/doc/filters.texi
index cd1aaab..d1790ac 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -15248,6 +15248,45 @@ Set the initial y position. Must be a floating point 
value between
 -100 and 100. Default value is -0.131825904205311970493132056385139.
 @end table
 
+@section mkmap
+
+Generate map of pixels for one of axis, either X or Y. Mainly useful
+as providing input frames for remap filter.
+
+This source accepts the following options:
+
+@table @option
+
+@item size, s
+Set the size of the output video. For the syntax of this option, check the
+@ref{video size syntax,,"Video size" section in the ffmpeg-utils 
manual,ffmpeg-utils}.
+
+@item rate, r
+Set the video rate, that is the number of frames generated per second.
+Default is 25.
+
+@item expr, e
+Set map expression.
+
+The expressions can contain the following constants and functions:
+
+@table @option
+@item W
+@item H
+The output width and height.
+
+@item X
+@item Y
+The current column and row.
+
+@item N
+The current frame number, starting from 0.
+
+@item T
+Time of the current frame, expressed in seconds.
+@end table
+@end table
+
 @section mptestsrc
 
 Generate various test patterns, as generated by the MPlayer test filter.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 91621d9..68e1923 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -321,6 +321,7 @@ OBJS-$(CONFIG_FREI0R_SRC_FILTER) += vf_frei0r.o
 OBJS-$(CONFIG_HALDCLUTSRC_FILTER)+= vsrc_testsrc.o
 OBJS-$(CONFIG_LIFE_FILTER)   += vsrc_life.o
 OBJS-$(CONFIG_MANDELBROT_FILTER) += vsrc_mandelbrot.o
+OBJS-$(CONFIG_MKMAP_FILTER)  += vsrc_mkmap.o
 OBJS-$(CONFIG_MPTESTSRC_FILTER)  += vsrc_mptestsrc.o
 OBJS-$(CONFIG_NULLSRC_FILTER)+= vsrc_testsrc.o
 OBJS-$(CONFIG_RGBTESTSRC_FILTER) += vsrc_testsrc.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index d71d14a..1d8ca94 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -336,6 +336,7 @@ void avfilter_register_all(void)
 REGISTER_FILTER(HALDCLUTSRC,haldclutsrc,vsrc);
 REGISTER_FILTER(LIFE,   life,   vsrc);
 REGISTER_FILTER(MANDELBROT, mandelbrot, vsrc);
+REGISTER_FILTER(MKMAP,  mkmap,  vsrc);
 REGISTER_FILTER(MPTESTSRC,  mptestsrc,  vsrc);
 REGISTER_FILTER(NULLSRC,nullsrc,vsrc);
 REGISTER_FILTER(RGBTESTSRC, rgbtestsrc, vsrc);
diff --git a/libavfilter/vsrc_mkmap.c b/libavfilter/vsrc_mkmap.c
new file mode 100644
index 000..e699c5f
--- /dev/null
+++ b/libavfilter/vsrc_mkmap.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2017 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 "libavutil/eval.h"
+#include "libavutil/opt.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "internal.h"
+#include "video.h"
+
+typedef struct MkMapContext {
+const AVClass *class;
+
+int w, h;
+AVRational frame_rate;
+char *expr_str;
+
+AVExpr *expr;
+int64_t pts;
+} MkMapContext;
+
+static const char *const var_names[] = {
+"W", "H", "X", "Y", "N", "T",
+NULL
+};
+
+enum var_name {
+VAR_W, VAR_H, VAR_X, VAR_Y, VAR_N, VAR_T,
+VAR_VARS_NB
+};
+
+#define OFFSET(x) offsetof(MkMapContext, x)
+#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
+
+static const AVOption mkmap_options[] = {
+{ "size", "set video size", OFFSET(w),  
AV_OPT_TYPE_IMAGE_SIZE, {.str = "hd720"},  0, 0,   FLAGS },
+{ "s","set video size", OFFSET(w),  
AV_OPT_TYPE_IMAGE_SIZE, {.str = "hd720"},  0, 0,   FLAGS },
+{ "rate", "set video rate", OFFSET(frame_rate), 
AV_OPT_TYPE_VIDEO_RATE, {.str = "25"}, 0, INT_MAX, FLAGS },
+{ "r","set video rate", OFFSET(frame_rate), 
AV_OPT_TYPE_VIDEO_RATE, 

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Paul B Mahol]

On 1/29/17, Andreas Cadhalpun  wrote:
> On 28.01.2017 12:44, Marton Balint wrote:
>> If we reduce the number of extra lines (not at any cost), I think that
>> helps.
>> There is also a solution which keeps the traditional C syntax, and is easy
>> to undestand even at first glance.
>>
>> if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
>> return ff_elog(AVERROR(ENOSYS), s, "Too many channels %d > %d\n",
>> st->codecpar->channels, FF_SANE_NB_CHANNELS);
>
> How would you define ff_elog for this to work?

I'm maintainer of this file, and I'm fed up with this nuisance conversation.
I'm against log message.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 28.01.2017 12:44, Marton Balint wrote:
> If we reduce the number of extra lines (not at any cost), I think that helps.
> There is also a solution which keeps the traditional C syntax, and is easy to 
> undestand even at first glance.
> 
> if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
> return ff_elog(AVERROR(ENOSYS), s, "Too many channels %d > %d\n", 
> st->codecpar->channels, FF_SANE_NB_CHANNELS);

How would you define ff_elog for this to work?

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 28.01.2017 03:48, Ronald S. Bultje wrote:
> I agree a macro here doesn't help. My concern wasn't with the check itself,
> I agree a file with 100 channels should error out. My concern is that these
> files will universally be the result of fuzzing, so I don't want to spam
> stderr with messages related to it, nor do I want source/binary size to
> increase because of it.
> 
> If you make ff_elog similar to assert (only if NDEBUG is not set), that may
> work for the binary size concern, but the source code size is still a
> concern. Again, not because it's bad code, but because it's needless since
> it only happens for fuzzed samples.

You claim that, but it's impossible to prove and thus likely wrong.

Also it's quite arbitrary that you object to this log message, while e.g.
the following has been there for years:
 if (s->nb_streams == ASF_MAX_STREAMS) {
 av_log(s, AV_LOG_ERROR, "too many streams\n");
 return AVERROR(EINVAL);
 }

Unless you can come up with objective criteria, when to add log messages
and when not, this topic is going to be a pointless waste of time.

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

Re: [FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection by looking at the actual frame rather than the first detected frame.

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 09:33:35PM +0100, Marijn Meijles wrote:
> On Sat, Jan 28, 2017 at 07:56:51PM +0100, Michael Niedermayer wrote:
> > On Sat, Jan 28, 2017 at 01:52:30PM +0100, Marijn Meijles wrote:
> > > Signed-off-by: Marijn Meijles 
> > > ---
> > >  libavformat/ac3dec.c | 8 
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > the previous mail contains a description but the patch itself lacks
> > a commit message beyond the first line, the patch should contain a
> > commit message
> > 
> 
> strange, git must have eaten it. Anyway, here is an untouched patch file from 
> git.
> 
> Marijn

>  ac3dec.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> b0db85d47bad47ca779e1748c7f0d4e97bd32e6b  
> 0001-Fix-to-prevent-runaway-ac3-detection-by-looking-at-t.patch
> From 27e9209c1094b24bcc45ef3ff174b80cb17db775 Mon Sep 17 00:00:00 2001
> From: Marijn Meijles 
> Date: Fri, 27 Jan 2017 22:08:15 +0100
> Subject: [PATCH] Fix to prevent runaway ac3 detection by looking at the actual
>  frame rather than the first detected frame.
> 
> When detecting a swapped AC3 marker the data of the frame is swapped. 
> However, in subsequent frames the data swapped is taken from the first frame 
> rather than the current frame.
> 
> Signed-off-by: Marijn Meijles 
> ---
>  libavformat/ac3dec.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/ac3dec.c b/libavformat/ac3dec.c
> index 363a32e..e85b0ac 100644
> --- a/libavformat/ac3dec.c
> +++ b/libavformat/ac3dec.c
> @@ -49,8 +49,8 @@ static int ac3_eac3_probe(AVProbeData *p, enum AVCodecID 
> expected_codec_id)
>  buf2+=16;
>  if (buf[0] == 0x77 && buf[1] == 0x0B) {

these could be changed to buf2 too but i guess its kind of making
sense to keep them so as to count stable endianness

patch applied

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


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

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: hint next marker position in ff_mjpeg_find_marker

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 01:59:10PM +0100, Matthieu Bouron wrote:
> On Sat, Jan 28, 2017 at 12:59:39PM +0100, Matthieu Bouron wrote:
> > On Sat, Jan 28, 2017 at 02:25:34AM +0100, Michael Niedermayer wrote:
> > > On Fri, Jan 27, 2017 at 09:47:18AM +0100, Matthieu Bouron wrote:
> > > > On Fri, Jan 27, 2017 at 02:30:40AM +0100, Michael Niedermayer wrote:
> > > > > On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> > > > > > Speeds up next marker search when a SOS marker is found.
> > > > > > 
> > > > > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg 
> > > > > > -i
> > > > > > sample_2992x4000.jpg
> > > > > > ---
> > > > > >  libavcodec/mjpegdec.c | 31 +--
> > > > > >  libavcodec/mjpegdec.h |  2 +-
> > > > > >  libavcodec/mxpegdec.c |  5 +++--
> > > > > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > > > > 
> > > > > this breaks decoding multiple jpeg files
> > > > > for example tickets//856/nint.jpg
> > > > 
> > > > New patch attached fixing the issue.
> > > > 
> > > > [...]
> > > 
> > > >  mjpegdec.c |   31 +--
> > > >  mjpegdec.h |2 +-
> > > >  mxpegdec.c |5 +++--
> > > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > > > 6ba49d73c189b197bdc1a983e26ccd49713c617c  
> > > > 0001-lavc-mjpegdec-hint-next-marker-position-in-ff_mjpeg_.patch
> > > > From ebef63ebdc75872d52529d5b74b6d05254d4c0fd Mon Sep 17 00:00:00 2001
> > > > From: Matthieu Bouron 
> > > > Date: Thu, 26 Jan 2017 15:17:36 +0100
> > > > Subject: [PATCH] lavc/mjpegdec: hint next marker position in
> > > >  ff_mjpeg_find_marker
> > > > 
> > > > Speeds up next marker search when a SOS marker is found.
> > > > 
> > > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > > > sample_2992x4000.jpg
> > > 
> > > the patch seems to work but
> > > why is the "buf_ptr += (get_bits_count(>gb) + 7) / 8;" not
> > > already skiping all that what the buf_hint does ?
> > > is the difference the unescaping ?
> > > maybe the buf_hint stuff can be simplified by adjusting buf_ptr instead
> > > ?
> > 
> > I've just realized that the SOS marker handling does not consume any byte
> > from the GetBitContext *only* when the frame is being discard (which
> > happens in avformat_find_stream_info).
> > 
> > So I guess a better solution would be to consume all the bytes from the
> > GetBitContext while this marker is skipped and let the code already in
> > place handle that.
> > However, the buf_ptr won't directly be at the next marker position since
> > the sos data is unescaped. With the sample I use there is a difference of
> > 13774 bytes (thus iterations in find_marker) to find the next marker. It
> > reduces the function (according to perf) from 5,4% to 4.8% while the frame
> > is being decoded (ffmpeg -i sample_2999x4000.jpeg -f null -) but it seems
> > the performance gain is too little versus the code that the patch
> > introduces and the decoder will benefits more from having aarch64 simd
> > code for the various idct functions (something that i plan to do in the
> > upcoming days).

you could keep track of the unescaped bytes ad add them too


> > 
> > I'll send a new patch soon.
> 
> Alternative patch attached.

LGTM

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

You can kill me, but you cannot change the truth.


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

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 06:55:15PM +0100, wm4 wrote:
> On Sat, 28 Jan 2017 17:28:29 +0100
> Paul Arzelier  wrote:
> 
> > Hopefuly, everything is okay right now :)
> 
> Patch is fine with me.

applied

thx

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH] avcodec: add ATRAC Advanced Lossless decoders

[Paul B Mahol]

On 1/25/17, Paul B Mahol  wrote:
> Only lossy part is decoded for now.
>
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/Makefile|   3 +
>  libavcodec/allcodecs.c |   2 +
>  libavcodec/atrac3.c| 134 ++-
>  libavcodec/atrac3plusdec.c |  14 +++-
>  libavcodec/avcodec.h   |   2 +
>  libavcodec/codec_desc.c|  14 
>  libavformat/oma.c  |  10 +--
>  libavformat/oma.h  |   2 +
>  libavformat/omadec.c   | 173
> -
>  9 files changed, 313 insertions(+), 41 deletions(-)
>

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

Re: [FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection

[Marijn Meijles]

On Sat, Jan 28, 2017 at 06:04:02PM +, Kieran Kunhya wrote:
> On Sat, 28 Jan 2017 at 12:53 Marijn Meijles  wrote:
> 
> > This is a fix for https://trac.ffmpeg.org/ticket/6106 .
> >
> > The issue is that when detecting a swapped AC3 marker the data of the
> > frame is swapped. However, in subsequent frames the data swapped is
> > taken from the first frame rather than the current frame. So all frames
> > are considered to be correct even though they might not be. This
> > leads to a high probe score and eventual misidentification.
> >
> 
> Do these files not have SPDIF syncwords?

No, the ones I included in the sample are generated by the stock flac
decoder. All other programs I tried consider them pcm wave files.

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

Re: [FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection by looking at the actual frame rather than the first detected frame.

[Marijn Meijles]

On Sat, Jan 28, 2017 at 07:56:51PM +0100, Michael Niedermayer wrote:
> On Sat, Jan 28, 2017 at 01:52:30PM +0100, Marijn Meijles wrote:
> > Signed-off-by: Marijn Meijles 
> > ---
> >  libavformat/ac3dec.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> the previous mail contains a description but the patch itself lacks
> a commit message beyond the first line, the patch should contain a
> commit message
> 

strange, git must have eaten it. Anyway, here is an untouched patch file from 
git.

Marijn
>From 27e9209c1094b24bcc45ef3ff174b80cb17db775 Mon Sep 17 00:00:00 2001
From: Marijn Meijles 
Date: Fri, 27 Jan 2017 22:08:15 +0100
Subject: [PATCH] Fix to prevent runaway ac3 detection by looking at the actual
 frame rather than the first detected frame.

When detecting a swapped AC3 marker the data of the frame is swapped. However, in subsequent frames the data swapped is taken from the first frame rather than the current frame.

Signed-off-by: Marijn Meijles 
---
 libavformat/ac3dec.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavformat/ac3dec.c b/libavformat/ac3dec.c
index 363a32e..e85b0ac 100644
--- a/libavformat/ac3dec.c
+++ b/libavformat/ac3dec.c
@@ -49,8 +49,8 @@ static int ac3_eac3_probe(AVProbeData *p, enum AVCodecID expected_codec_id)
 buf2+=16;
 if (buf[0] == 0x77 && buf[1] == 0x0B) {
 for(i=0; i<8; i+=2) {
-buf3[i  ] = buf[i+1];
-buf3[i+1] = buf[i  ];
+buf3[i  ] = buf2[i+1];
+buf3[i+1] = buf2[i  ];
 }
 init_get_bits(, buf3, 54);
 }else
@@ -62,8 +62,8 @@ static int ac3_eac3_probe(AVProbeData *p, enum AVCodecID expected_codec_id)
 if (buf[0] == 0x77 && buf[1] == 0x0B) {
 av_assert0(phdr->frame_size <= sizeof(buf3));
 for(i=8; iframe_size; i+=2) {
-buf3[i  ] = buf[i+1];
-buf3[i+1] = buf[i  ];
+buf3[i  ] = buf2[i+1];
+buf3[i+1] = buf2[i  ];
 }
 }
 if(av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, gbc.buffer + 2, phdr->frame_size - 2))
-- 
2.10.1 (Apple Git-78)

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

Re: [FFmpeg-devel] patch 1: comments cleanup in libavcodec/cinepakenc.c

[u-9iep]

On Sat, Jan 28, 2017 at 07:53:06PM +0100, Michael Niedermayer wrote:
> On Sat, Jan 28, 2017 at 11:42:24AM +0100, u-9...@aetey.se wrote:
> > Attaching the patch.
> 
> please provide a git compatible patch
> git format-patch / send-email
> 
> git is quite flexible and often takes diffs in mails too
> but unfortunately my git does not accept this one

Sigh. Thanks for trying!

Will pass the patches through git and come back.

Regards,
Rune

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

Re: [FFmpeg-devel] patch 2: comments style cleanup in libavcodec/cinepakenc.c

[u-9iep]

On Sat, Jan 28, 2017 at 08:30:34PM +0100, Moritz Barsnick wrote:
> On Sat, Jan 28, 2017 at 11:50:27 +0100, u-9...@aetey.se wrote:
> > -// MAX_STRIPS limits the maximum quality you can reach
> > -//when you want high quality on high resolutions,
> > -// MIN_STRIPS limits the minimum efficiently encodable bit rate
> > -//on low resolutions
> > -// the numbers are only used for brute force optimization for the first 
> > frame,
> > -// for the following frames they are adaptively readjusted
> > -// NOTE the decoder in ffmpeg has its own arbitrary limitation on the 
> > number
> > -// of strips, currently 32
> > +/* MAX_STRIPS limits the maximum quality you can reach */
> > +/*when you want high quality on high resolutions, */
> > +/* MIN_STRIPS limits the minimum efficiently encodable bit rate */
> > +/*on low resolutions */
> > +/* the numbers are only used for brute force optimization for the first 
> > frame, */
> > +/* for the following frames they are adaptively readjusted */
> > +/* NOTE the decoder in ffmpeg has its own arbitrary limitation on the 
> > number */
> > +/* of strips, currently 32 */
> 
> If this is supposed to be cosmetic, it's extremely ugly. Does any other

You have a point. At this place the layout should be changed beyond
simply replacing the delimiters.

> ffmpeg code use this "style"?

:) No, this was not an intended style, just a reflection of the old one.

> I think you'd rather use:
> 
>   /*
>* Extremely long
>* multiline comment
>* using non-C++ style
>* comment delimiters.
>*/

Yes this is what should have been done, like it looks in other places
in the same file.

Regards,
Rune

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

Re: [FFmpeg-devel] patch 2: comments style cleanup in libavcodec/cinepakenc.c

[Moritz Barsnick]

On Sat, Jan 28, 2017 at 11:50:27 +0100, u-9...@aetey.se wrote:
> -// MAX_STRIPS limits the maximum quality you can reach
> -//when you want high quality on high resolutions,
> -// MIN_STRIPS limits the minimum efficiently encodable bit rate
> -//on low resolutions
> -// the numbers are only used for brute force optimization for the first 
> frame,
> -// for the following frames they are adaptively readjusted
> -// NOTE the decoder in ffmpeg has its own arbitrary limitation on the number
> -// of strips, currently 32
> +#define MAX_STRIPS  32  /* Note: having fewer choices regarding the 
> number of strips speeds up encoding (obviously) */
> +#define MIN_STRIPS  1   /* Note: having more strips speeds up encoding 
> the frame (this is less obvious) */
> +/* MAX_STRIPS limits the maximum quality you can reach */
> +/*when you want high quality on high resolutions, */
> +/* MIN_STRIPS limits the minimum efficiently encodable bit rate */
> +/*on low resolutions */
> +/* the numbers are only used for brute force optimization for the first 
> frame, */
> +/* for the following frames they are adaptively readjusted */
> +/* NOTE the decoder in ffmpeg has its own arbitrary limitation on the number 
> */
> +/* of strips, currently 32 */

If this is supposed to be cosmetic, it's extremely ugly. Does any other
ffmpeg code use this "style"?

I think you'd rather use:

  /*
   * Extremely long
   * multiline comment
   * using non-C++ style
   * comment delimiters.
   */

(I do agree though that mixing styles isn't nice.)

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

Re: [FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection by looking at the actual frame rather than the first detected frame.

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 01:52:30PM +0100, Marijn Meijles wrote:
> Signed-off-by: Marijn Meijles 
> ---
>  libavformat/ac3dec.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

the previous mail contains a description but the patch itself lacks
a commit message beyond the first line, the patch should contain a
commit message

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

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


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

Re: [FFmpeg-devel] patch 1: comments cleanup in libavcodec/cinepakenc.c

[Michael Niedermayer]

On Sat, Jan 28, 2017 at 11:42:24AM +0100, u-9...@aetey.se wrote:
> Comments cleanup:
>  - change the encoding of the original developer name from ISO-8859-1 to UTF-8
>  - remove the stale/completed TODO list
>  - fix a typo
> 
> No code changes, only improved consistency in the comments.
> 
> I kindly ask to apply this cleanup, which of course was due from the 
> beginning.
> 
> Attaching the patch.

please provide a git compatible patch
git format-patch / send-email

git is quite flexible and often takes diffs in mails too
but unfortunately my git does not accept this one

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Paul B Mahol]

On 1/28/17, Michael Niedermayer  wrote:
> On Fri, Jan 27, 2017 at 09:48:05PM -0500, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Fri, Jan 27, 2017 at 9:28 PM, Michael Niedermayer 
>> wrote:
>>
>> > On Sat, Jan 28, 2017 at 01:26:40AM +0100, Andreas Cadhalpun wrote:
>> > > On 27.01.2017 02:56, Marton Balint wrote:
>> > > > I see 3 problems (wm4 explicitly named them, but I also had them in
>> > mind)
>> > > > - Little benefit, yet
>> > > > - Makes the code less clean, more cluttered
>> > > > - Increases binary size
>> > > >
>> > > > The ideas I proposed (use macros, use common / factorized checks
>> > > > for
>> > common
>> > > > validatons and errors) might be a good compromise IMHO. Fuzzing
>> > thereforce
>> > > > can be done with "debug" builds.
>> > > >
>> > > > Anyway, I am not blocking the patch, just expressing what I would
>> > prefer in
>> > > > the long run.
>> > >
>> > > How about the following macro?
>> > > #define FF_RETURN_ERROR(condition, error, log_ctx, ...) {   \
>> > > if (condition) {\
>> > > if (!CONFIG_SMALL && log_ctx)   \
>> > > av_log(log_ctx, AV_LOG_ERROR, __VA_ARGS__); \
>> > > return error;   \
>> > > }   \
>> > > }
>> > >
>> > > That could be used with error message:
>> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,
>> > AVERROR(ENOSYS),
>> > > s, "Too many channels %d > %d\n",
>> > st->codecpar->channels, FF_SANE_NB_CHANNELS)
>> > >
>> > > Or without:
>> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,
>> > AVERROR(ENOSYS), NULL)
>> >
>> > I suggest
>> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
>> > ff_elog(s, "Too many channels %d > %d\n", st->codecpar->channels,
>> > FF_SANE_NB_CHANNELS);
>> > return AVERROR(ENOSYS);
>> > }
>> >
>> > or for the 2nd example:
>> >
>> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
>> > return AVERROR(ENOSYS);
>> >
>> >
>> > ff_elog() can then be defined to nothing based on CONFIG_SMALL
>> >
>> > the difference between my suggestion and yours is that mine has
>> > new-lines seperating the condition, message and return and that its
>> > self explanatory C code.
>> >
>> > What you do is removing newlines and standard C keywords with a custom
>> > macro that people not working on FFmpeg on a regular basis will have
>> > difficulty understanding
>> >
>> > The macro is similar to writing (minus the C keywords)
>> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) { ff_elog(
>> > s, "Too many channels %d > %d\n", st->codecpar->channels,
>> > FF_SANE_NB_CHANNELS); return AVERROR(ENOSYS); }
>> >
>> > we dont do that becuause its just bad code
>> > why does this suddenly become desirable when its in a macro?
>> >
>> > Or maybe the question should be, does code become less cluttered and
>> > cleaner when newlines and C keywords are removed ?
>> > if not why is that done here ?
>> > if yes why is it done just here ?
>>
>>
>> I agree a macro here doesn't help. My concern wasn't with the check
>> itself,
>> I agree a file with 100 channels should error out. My concern is that
>> these
>> files will universally be the result of fuzzing, so I don't want to spam
>> stderr with messages related to it, nor do I want source/binary size to
>> increase because of it.
>>
>
>> If you make ff_elog similar to assert (only if NDEBUG is not set), that
>> may
>> work for the binary size concern, but the source code size is still a
>> concern. Again, not because it's bad code, but because it's needless
>> since
>> it only happens for fuzzed samples.
>
> its needless to you, its not needless to me. When i work on fixing
> issues found by fuzzed samples, crashes, undefined behavior, security
> issues, having detailed error messages makes it sometimes simpler
>
> Having no error messages related to fuzzed samples or only a subset of
> them will make debuging issues related to fuzzing harder.
>
> an issue after an error path for example, its easier if you
> know which error it was than if you just see a crash and dont even
> know that an error occured as the crash happens before the app
> exists with an error of its own.
> Nothing of that makes it impossible to fix but it makes it cost more
> time if the specific error path prints no error message
>
> I dont think its desirable to make debuging issues related to fuzzing
> harder. Nor to make me spend more time per bugfix than otherwise needed,
> even if that is maybe not a large factor its not nothing

For debugging crashes you use crash file and valgrind. You do not spread code
with av_log messages.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Michael Niedermayer]

On Fri, Jan 27, 2017 at 09:48:05PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Jan 27, 2017 at 9:28 PM, Michael Niedermayer 
> wrote:
> 
> > On Sat, Jan 28, 2017 at 01:26:40AM +0100, Andreas Cadhalpun wrote:
> > > On 27.01.2017 02:56, Marton Balint wrote:
> > > > I see 3 problems (wm4 explicitly named them, but I also had them in
> > mind)
> > > > - Little benefit, yet
> > > > - Makes the code less clean, more cluttered
> > > > - Increases binary size
> > > >
> > > > The ideas I proposed (use macros, use common / factorized checks for
> > common
> > > > validatons and errors) might be a good compromise IMHO. Fuzzing
> > thereforce
> > > > can be done with "debug" builds.
> > > >
> > > > Anyway, I am not blocking the patch, just expressing what I would
> > prefer in
> > > > the long run.
> > >
> > > How about the following macro?
> > > #define FF_RETURN_ERROR(condition, error, log_ctx, ...) {   \
> > > if (condition) {\
> > > if (!CONFIG_SMALL && log_ctx)   \
> > > av_log(log_ctx, AV_LOG_ERROR, __VA_ARGS__); \
> > > return error;   \
> > > }   \
> > > }
> > >
> > > That could be used with error message:
> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,
> > AVERROR(ENOSYS),
> > > s, "Too many channels %d > %d\n",
> > st->codecpar->channels, FF_SANE_NB_CHANNELS)
> > >
> > > Or without:
> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,
> > AVERROR(ENOSYS), NULL)
> >
> > I suggest
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
> > ff_elog(s, "Too many channels %d > %d\n", st->codecpar->channels,
> > FF_SANE_NB_CHANNELS);
> > return AVERROR(ENOSYS);
> > }
> >
> > or for the 2nd example:
> >
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
> > return AVERROR(ENOSYS);
> >
> >
> > ff_elog() can then be defined to nothing based on CONFIG_SMALL
> >
> > the difference between my suggestion and yours is that mine has
> > new-lines seperating the condition, message and return and that its
> > self explanatory C code.
> >
> > What you do is removing newlines and standard C keywords with a custom
> > macro that people not working on FFmpeg on a regular basis will have
> > difficulty understanding
> >
> > The macro is similar to writing (minus the C keywords)
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) { ff_elog(
> > s, "Too many channels %d > %d\n", st->codecpar->channels,
> > FF_SANE_NB_CHANNELS); return AVERROR(ENOSYS); }
> >
> > we dont do that becuause its just bad code
> > why does this suddenly become desirable when its in a macro?
> >
> > Or maybe the question should be, does code become less cluttered and
> > cleaner when newlines and C keywords are removed ?
> > if not why is that done here ?
> > if yes why is it done just here ?
> 
> 
> I agree a macro here doesn't help. My concern wasn't with the check itself,
> I agree a file with 100 channels should error out. My concern is that these
> files will universally be the result of fuzzing, so I don't want to spam
> stderr with messages related to it, nor do I want source/binary size to
> increase because of it.
> 

> If you make ff_elog similar to assert (only if NDEBUG is not set), that may
> work for the binary size concern, but the source code size is still a
> concern. Again, not because it's bad code, but because it's needless since
> it only happens for fuzzed samples.

its needless to you, its not needless to me. When i work on fixing
issues found by fuzzed samples, crashes, undefined behavior, security
issues, having detailed error messages makes it sometimes simpler

Having no error messages related to fuzzed samples or only a subset of
them will make debuging issues related to fuzzing harder.

an issue after an error path for example, its easier if you
know which error it was than if you just see a crash and dont even
know that an error occured as the crash happens before the app
exists with an error of its own.
Nothing of that makes it impossible to fix but it makes it cost more
time if the specific error path prints no error message

I dont think its desirable to make debuging issues related to fuzzing
harder. Nor to make me spend more time per bugfix than otherwise needed,
even if that is maybe not a large factor its not nothing

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


signature.asc
Description: Digital signature

Re: [FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection

[Kieran Kunhya]

On Sat, 28 Jan 2017 at 12:53 Marijn Meijles  wrote:

> This is a fix for https://trac.ffmpeg.org/ticket/6106 .
>
> The issue is that when detecting a swapped AC3 marker the data of the
> frame is swapped. However, in subsequent frames the data swapped is
> taken from the first frame rather than the current frame. So all frames
> are considered to be correct even though they might not be. This
> leads to a high probe score and eventual misidentification.
>

Do these files not have SPDIF syncwords?

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

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[wm4]

On Sat, 28 Jan 2017 18:35:01 +0100
Nicolas George  wrote:

> Le nonidi 9 pluviôse, an CCXXV, Muhammad Faiz a écrit :
> > So, if ff_inlink_make_frame_writable() takes AVFrame**, it will be
> > incompatible with framequeue framework, because it stores AVFrame*, so
> > it will contain dangling AVFrame*.  
> 
> There is nothing "incompatible" about it, what would it even mean? It
> just can not be used as is on this specific call site; it was not meant
> to. It is a smaller, more versatile brick. You just need to add the
> little bit of mortar to have it fit here.
> 
> It is not possible to "make" a frame writable, the original name is a
> misnomer; the only thing possible is to create a new copy that is

Well, that's what the existing API calls it. In several places.

> uniquely referenced and therefore writable. If that is needed, an extra
> copy can be made to have the new frame take the place of the old one.
> But that is only rarely needed; making that copy always like you propose

It doesn't always copy.

> following wm4's bad advice

Please cease your childish attacks.

> will just make the code less efficient and
> more awkward.

Amazing.

We're talking about copying a few bytes additional in a case that
copies an entire frame of media data. And it's only "rarely needed", as
you said just above. (To remind other readers,
ff_inlink_make_frame_writable() exits immediately if the frame is
writable.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[wm4]

On Sat, 28 Jan 2017 17:28:29 +0100
Paul Arzelier  wrote:

> Hopefuly, everything is okay right now :)

Patch is fine with me.

Bonus points for adding a fate tests for conflicting tags (I suppose
that would be a tiny FLAC file), and maybe one for mp3+ID3 tags. Of
course that would be a separate patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[Nicolas George]

Le nonidi 9 pluviôse, an CCXXV, Muhammad Faiz a écrit :
> So, if ff_inlink_make_frame_writable() takes AVFrame**, it will be
> incompatible with framequeue framework, because it stores AVFrame*, so
> it will contain dangling AVFrame*.

There is nothing "incompatible" about it, what would it even mean? It
just can not be used as is on this specific call site; it was not meant
to. It is a smaller, more versatile brick. You just need to add the
little bit of mortar to have it fit here.

It is not possible to "make" a frame writable, the original name is a
misnomer; the only thing possible is to create a new copy that is
uniquely referenced and therefore writable. If that is needed, an extra
copy can be made to have the new frame take the place of the old one.
But that is only rarely needed; making that copy always like you propose
following wm4's bad advice will just make the code less efficient and
more awkward.

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] avformat: add SCC muxer

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   2 +-
 libavformat/sccenc.c | 122 +++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 libavformat/sccenc.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index e291925..478c06b 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -435,6 +435,7 @@ OBJS-$(CONFIG_SAP_DEMUXER)   += sapdec.o
 OBJS-$(CONFIG_SAP_MUXER) += sapenc.o
 OBJS-$(CONFIG_SBG_DEMUXER)   += sbgdec.o
 OBJS-$(CONFIG_SCC_DEMUXER)   += sccdec.o subtitles.o
+OBJS-$(CONFIG_SCC_MUXER) += sccenc.o subtitles.o
 OBJS-$(CONFIG_SDP_DEMUXER)   += rtsp.o
 OBJS-$(CONFIG_SDR2_DEMUXER)  += sdr2.o
 OBJS-$(CONFIG_SDS_DEMUXER)   += sdsdec.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 9c1f6dd..35869e3 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -273,7 +273,7 @@ void av_register_all(void)
 REGISTER_DEMUXER (SAMI, sami);
 REGISTER_MUXDEMUX(SAP,  sap);
 REGISTER_DEMUXER (SBG,  sbg);
-REGISTER_DEMUXER (SCC,  scc);
+REGISTER_MUXDEMUX(SCC,  scc);
 REGISTER_DEMUXER (SDP,  sdp);
 REGISTER_DEMUXER (SDR2, sdr2);
 REGISTER_DEMUXER (SDS,  sds);
diff --git a/libavformat/sccenc.c b/libavformat/sccenc.c
new file mode 100644
index 000..5205f31
--- /dev/null
+++ b/libavformat/sccenc.c
@@ -0,0 +1,122 @@
+/*
+ * SCC muxer
+ * Copyright (c) 2017 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"
+#include "libavutil/log.h"
+#include "libavutil/intreadwrite.h"
+
+typedef struct SCCContext {
+int prev_h, prev_m, prev_s, prev_f;
+int inside;
+int n;
+} SCCContext;
+
+static int scc_write_header(AVFormatContext *avf)
+{
+SCCContext *scc = avf->priv_data;
+
+if (avf->nb_streams != 1 ||
+avf->streams[0]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE) {
+av_log(avf, AV_LOG_ERROR,
+   "SCC supports only a single subtitles stream.\n");
+return AVERROR(EINVAL);
+}
+if (avf->streams[0]->codecpar->codec_id != AV_CODEC_ID_EIA_608) {
+av_log(avf, AV_LOG_ERROR,
+   "Unsupported subtitles codec: %s\n",
+   avcodec_get_name(avf->streams[0]->codecpar->codec_id));
+return AVERROR(EINVAL);
+}
+avpriv_set_pts_info(avf->streams[0], 64, 1, 1000);
+avio_printf(avf->pb, "Scenarist_SCC V1.0\n");
+
+scc->prev_h = scc->prev_m = scc->prev_s = scc->prev_f = -1;
+scc->inside = 0;
+
+return 0;
+}
+
+static int scc_write_packet(AVFormatContext *avf, AVPacket *pkt)
+{
+SCCContext *scc = avf->priv_data;
+int64_t pts = pkt->pts;
+int i, h, m, s, f;
+
+if (pts == AV_NOPTS_VALUE) {
+av_log(avf, AV_LOG_WARNING,
+   "Insufficient timestamps.\n");
+return 0;
+}
+
+h = (int)(pts / (360));
+m = (int)(pts / (6)) % 60;
+s = (int)(pts /  1000) % 60;
+f = (int)(pts %  1000) / 33;
+
+for (i = 0; i < pkt->size; i+=3) {
+if (pkt->data[i] == 0xfc && ((pkt->data[i + 1] != 0x80 || pkt->data[i 
+ 2] != 0x80)))
+break;
+}
+if (i >= pkt->size)
+return 0;
+
+if (!scc->inside && (scc->prev_h != h || scc->prev_m != m || scc->prev_s 
!= s || scc->prev_f != f)) {
+avio_printf(avf->pb, "\n%02d:%02d:%02d:%02d\t", h, m, s, f);
+scc->inside = 1;
+}
+for (i = 0; i < pkt->size; i+=3) {
+if (i + 3 > pkt->size)
+break;
+
+if (pkt->data[i] != 0xfc || (pkt->data[i + 1] == 0x80 && pkt->data[i + 
2] == 0x80))
+continue;
+if (!scc->inside) {
+avio_printf(avf->pb, "\n%02d:%02d:%02d:%02d\t", h, m, s, f);
+scc->inside = 1;
+}
+if (scc->n > 0)
+avio_printf(avf->pb, " ");
+avio_printf(avf->pb, "%02x%02x", pkt->data[i + 1], pkt->data[i + 2]);
+scc->n++;
+}
+if (scc->inside && (scc->prev_h != h || scc->prev_m != 

Re: [FFmpeg-devel] [PATCH] Ignore duplicate ID3 tags if vorbis tags exist

[Paul Arzelier]

Hopefuly, everything is okay right now :)


Le 27/01/2017 à 17:42, wm4 a écrit :

On Fri, 27 Jan 2017 17:32:08 +0100
Paul Arzelier  wrote:


 From 227d7d1f4b5665f824900cf2b14863621180dd1c Mon Sep 17 00:00:00 2001
From: Polochon-street 
Date: Fri, 27 Jan 2017 16:49:41 +0100
Subject: [PATCH] Ignore ID3 tags if other tags are present (e.g. vorbis)
Originally-by: Ben Boeckel 
---
  libavformat/internal.h |  5 +
  libavformat/mp3dec.c   |  5 +
  libavformat/options.c  |  1 +
  libavformat/utils.c| 16 +++-
  4 files changed, 26 insertions(+), 1 deletion(-)


Looks largely ok to me, just a few minor nits.


diff --git a/libavformat/internal.h b/libavformat/internal.h
index 9d614fb12c..63a1724cfa 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -140,6 +140,11 @@ struct AVFormatInternal {
   * Whether or not avformat_init_output fully initialized streams
   */
  int streams_initialized;
+
+/**
+ * ID3v2 tag useful for MP3 demuxing

The tag is used for many file formats (unfortunately), not just mp3. So
I'm not sure if the comment is correct.


+ */
+AVDictionary *id3v2_meta;
  };
  
  struct AVStreamInternal {

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 099ca57d24..8540e45e4f 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -349,6 +349,11 @@ static int mp3_read_header(AVFormatContext *s)
  int ret;
  int i;
  
+if (s->internal->id3v2_meta)

+s->metadata = s->internal->id3v2_meta;

Slightly odd that it checks for id3v2_meta being NULL. Seems redundant?


+
+s->internal->id3v2_meta = NULL;
+
  st = avformat_new_stream(s, NULL);
  if (!st)
  return AVERROR(ENOMEM);
diff --git a/libavformat/options.c b/libavformat/options.c
index 25a506eef8..d8aca472c5 100644
--- a/libavformat/options.c
+++ b/libavformat/options.c
@@ -144,6 +144,7 @@ AVFormatContext *avformat_alloc_context(void)
  ic->internal->offset = AV_NOPTS_VALUE;
  ic->internal->raw_packet_buffer_remaining_size = RAW_PACKET_BUFFER_SIZE;
  ic->internal->shortest_end = AV_NOPTS_VALUE;
+ic->internal->id3v2_meta = NULL;

Not necessary. All fields are initialized to 0 by default, because the
struct is allocated by av_mallocz().

  
  return ic;

  }
diff --git a/libavformat/utils.c b/libavformat/utils.c
index d5dfca7dec..b44d688213 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -588,12 +588,25 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  
  /* e.g. AVFMT_NOFILE formats will not have a AVIOContext */

  if (s->pb)
-ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, _extra_meta, 0);
+ff_id3v2_read_dict(s->pb, >internal->id3v2_meta, ID3v2_DEFAULT_MAGIC, 
_extra_meta);
  
  if (!(s->flags_FLAG_PRIV_OPT) && s->iformat->read_header)

  if ((ret = s->iformat->read_header(s)) < 0)
  goto fail;
  
+if (!s->metadata) {

+s->metadata = s->internal->id3v2_meta;
+s->internal->id3v2_meta = NULL;
+} else if (s->internal->id3v2_meta) {
+int level = AV_LOG_WARNING;
+if (s->error_recognition & AV_EF_COMPLIANT)
+level = AV_LOG_ERROR;
+av_log(s, level, "Discarding ID3 tags because more suitable tags were 
found.\n");
+av_dict_free(>internal->id3v2_meta);
+if (s->error_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+}
+
  if (id3v2_extra_meta) {
  if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, 
"aac") ||
  !strcmp(s->iformat->name, "tta")) {
@@ -627,6 +640,7 @@ int avformat_open_input(AVFormatContext **ps, const char 
*filename,
  fail:
  ff_id3v2_free_extra_meta(_extra_meta);
  av_dict_free();
+av_dict_free(>internal->id3v2_meta);
  if (s->pb && !(s->flags & AVFMT_FLAG_CUSTOM_IO))
  avio_closep(>pb);
  avformat_free_context(s);

You could free the tag in avformat_free_context(), which might be
slightly simpler (don't need to care about success vs. error path), but
it doesn't actually matter.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>From 08de8e059393f4f6b412d482aa3e045f6a6b06c7 Mon Sep 17 00:00:00 2001
From: Paul Arzelier 
Date: Sat, 28 Jan 2017 17:25:27 +0100
Subject: [PATCH] Ignore ID3v2 tags if other tags are present e.g. vorbis
Originally-by: Ben Boeckel 
---
 libavformat/internal.h |  5 +
 libavformat/mp3dec.c   |  3 +++
 libavformat/utils.c| 17 -
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index 9d614fb12c..63a1724cfa 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -140,6 +140,11 @@ struct AVFormatInternal {
  * Whether or not 

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[Muhammad Faiz]

On 1/28/17, Nicolas George  wrote:
> Le nonidi 9 pluviôse, an CCXXV, Muhammad Faiz a écrit :
>> so the behavior will be similar to
>> av_frame_make_writable().
>
> The point was to move away from that. Who copies a struct when you can
> move a pointer?

So, if ff_inlink_make_frame_writable() takes AVFrame**, it will be
incompatible with framequeue framework, because it stores AVFrame*, so
it will contain dangling AVFrame*.

So, which one you choose? This patch or the previous patch that makes
frame writable manually?

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

[FFmpeg-devel] patch 3 (of 3): comment FIX in libavcodec/cinepak.c

[u-9iep]

Fix a plainly wrong (inverted) misleading comment in the cinepak decoder.

No code changes.

I kindly ask to apply this fix, which of course was due from the beginning.

Attaching the patch.

Regards,
Rune
--- libavcodec/cinepak.c.orig   2017-01-28 17:00:37.0 +0100
+++ libavcodec/cinepak.c2017-01-28 17:04:20.513123515 +0100
@@ -155,8 +155,8 @@
 }
 }
 }
-/* to get the correct picture for not-multiple-of-4 cases let us fill
- * each block from the bottom up, thus possibly overwriting the top line
+/* to get the correct picture for not-multiple-of-4 cases let us fill each
+ * block from the bottom up, thus possibly overwriting the bottommost line
  * more than once but ending with the correct data in place
  * (instead of in-loop checking) */
 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[wm4]

On Sat, 28 Jan 2017 16:43:36 +0100
Nicolas George  wrote:

> Le nonidi 9 pluviôse, an CCXXV, Muhammad Faiz a écrit :
> > so the behavior will be similar to
> > av_frame_make_writable().  
> 
> The point was to move away from that. Who copies a struct when you can
> move a pointer?
> 
> Regards,
> 

Because it's trickier, harder to use, and creates the risk of dangling
pointers that will be spotted only if a frame was not writable.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[Nicolas George]

Le nonidi 9 pluviôse, an CCXXV, Muhammad Faiz a écrit :
> so the behavior will be similar to
> av_frame_make_writable().

The point was to move away from that. Who copies a struct when you can
move a pointer?

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[wm4]

On Sat, 28 Jan 2017 22:23:53 +0700
Muhammad Faiz  wrote:

> so the behavior will be similar to
> av_frame_make_writable().
> 
> Also use av_frame_copy() replacing
> av_image_copy()/av_samples_copy().
> 
> Needed for the next patch.
> 
> Suggested-by: wm4 
> Signed-off-by: Muhammad Faiz 
> ---
>  libavfilter/avfilter.c | 26 ++
>  libavfilter/filters.h  |  2 +-
>  2 files changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index c12d491..c8dafd2 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1104,7 +1104,7 @@ static int ff_filter_frame_framed(AVFilterLink *link, 
> AVFrame *frame)
>  filter_frame = default_filter_frame;
>  
>  if (dst->needs_writable) {
> -ret = ff_inlink_make_frame_writable(link, );
> +ret = ff_inlink_make_frame_writable(link, frame);
>  if (ret < 0)
>  goto fail;
>  }
> @@ -1556,9 +1556,8 @@ int ff_inlink_consume_samples(AVFilterLink *link, 
> unsigned min, unsigned max,
>  return 1;
>  }
>  
> -int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame **rframe)
> +int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame *frame)
>  {
> -AVFrame *frame = *rframe;
>  AVFrame *out;
>  int ret;
>  
> @@ -1585,23 +1584,10 @@ int ff_inlink_make_frame_writable(AVFilterLink *link, 
> AVFrame **rframe)
>  return ret;
>  }
>  
> -switch (link->type) {
> -case AVMEDIA_TYPE_VIDEO:
> -av_image_copy(out->data, out->linesize, (const uint8_t 
> **)frame->data, frame->linesize,
> -  frame->format, frame->width, frame->height);
> -break;
> -case AVMEDIA_TYPE_AUDIO:
> -av_samples_copy(out->extended_data, frame->extended_data,
> -0, 0, frame->nb_samples,
> -av_frame_get_channels(frame),
> -frame->format);
> -break;
> -default:
> -av_assert0(!"reached");
> -}
> -
> -av_frame_free();
> -*rframe = out;
> +av_frame_copy(out, frame);
> +av_frame_unref(frame);
> +av_frame_move_ref(frame, out);
> +av_frame_free();
>  return 0;
>  }
>  
> diff --git a/libavfilter/filters.h b/libavfilter/filters.h
> index 2c78d60..5d32403 100644
> --- a/libavfilter/filters.h
> +++ b/libavfilter/filters.h
> @@ -101,7 +101,7 @@ int ff_inlink_consume_samples(AVFilterLink *link, 
> unsigned min, unsigned max,
>   * This is similar to av_frame_make_writable() except it uses the link's
>   * buffer allocation callback, and therefore allows direct rendering.
>   */
> -int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame **rframe);
> +int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame *frame);
>  
>  /**
>   * Test and acknowledge the change of status on the link.

LGTM. Maybe you should wait for confirmation by Nicolas George or
someone else who knows this code well before you push this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: do not write to unwritable frame

[Muhammad Faiz]

On 1/28/17, wm4  wrote:
> On Sat, 28 Jan 2017 15:59:11 +0700
> Muhammad Faiz  wrote:
>
>> On 1/28/17, Nicolas George  wrote:
>> > L'octidi 8 pluviôse, an CCXXV, Muhammad Faiz a écrit :
>> >> affect filters that set partial_buf_size
>> >> test-case
>> >> ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp,
>> >> asplit
>> >> [a][b];
>> >> [a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
>> >> [b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
>> >> [a1][b1] vstack'
>> >>
>> >> Signed-off-by: Muhammad Faiz 
>> >> ---
>> >>  libavfilter/avfilter.c | 17 +
>> >>  1 file changed, 17 insertions(+)
>> >
>> > Maybe this can be of use:
>> >
>> > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/28c62df672865890cbb13e5f0e94bde29c8fbacd
>> >
>>
>> Unfortunately, this modify pointer to AVFrame, i think it is incompatible
>> with framequeue framework.
>>
>> Note that av_frame_make_writable() takes AVFrame*,
>> while ff_inlink_make_frame_writable() takes AVFrame**.
>
> There doesn't seem to be any reason that it takes AVFrame**. AVFrame
> refs can be moved, so even if a frame is newly allocated it can be put
> into the AVFrame passed to the function without replacing the pointer.
>
> Also the code for copying the data in the function could have be
> reduced to 1 line by using av_frame_copy().
>
> Maybe someone should have reviewed the commit that added this.
>
> Anyway, maybe you can fix it and use it. That would be the ideal
> outcome.

Good idea. I've posted it.

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

[FFmpeg-devel] [PATCH 1/2] avfilter: change ff_inlink_make_frame_writable() to take AVFrame* argument

[Muhammad Faiz]

so the behavior will be similar to
av_frame_make_writable().

Also use av_frame_copy() replacing
av_image_copy()/av_samples_copy().

Needed for the next patch.

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 libavfilter/avfilter.c | 26 ++
 libavfilter/filters.h  |  2 +-
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index c12d491..c8dafd2 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -1104,7 +1104,7 @@ static int ff_filter_frame_framed(AVFilterLink *link, 
AVFrame *frame)
 filter_frame = default_filter_frame;
 
 if (dst->needs_writable) {
-ret = ff_inlink_make_frame_writable(link, );
+ret = ff_inlink_make_frame_writable(link, frame);
 if (ret < 0)
 goto fail;
 }
@@ -1556,9 +1556,8 @@ int ff_inlink_consume_samples(AVFilterLink *link, 
unsigned min, unsigned max,
 return 1;
 }
 
-int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame **rframe)
+int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame *frame)
 {
-AVFrame *frame = *rframe;
 AVFrame *out;
 int ret;
 
@@ -1585,23 +1584,10 @@ int ff_inlink_make_frame_writable(AVFilterLink *link, 
AVFrame **rframe)
 return ret;
 }
 
-switch (link->type) {
-case AVMEDIA_TYPE_VIDEO:
-av_image_copy(out->data, out->linesize, (const uint8_t **)frame->data, 
frame->linesize,
-  frame->format, frame->width, frame->height);
-break;
-case AVMEDIA_TYPE_AUDIO:
-av_samples_copy(out->extended_data, frame->extended_data,
-0, 0, frame->nb_samples,
-av_frame_get_channels(frame),
-frame->format);
-break;
-default:
-av_assert0(!"reached");
-}
-
-av_frame_free();
-*rframe = out;
+av_frame_copy(out, frame);
+av_frame_unref(frame);
+av_frame_move_ref(frame, out);
+av_frame_free();
 return 0;
 }
 
diff --git a/libavfilter/filters.h b/libavfilter/filters.h
index 2c78d60..5d32403 100644
--- a/libavfilter/filters.h
+++ b/libavfilter/filters.h
@@ -101,7 +101,7 @@ int ff_inlink_consume_samples(AVFilterLink *link, unsigned 
min, unsigned max,
  * This is similar to av_frame_make_writable() except it uses the link's
  * buffer allocation callback, and therefore allows direct rendering.
  */
-int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame **rframe);
+int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame *frame);
 
 /**
  * Test and acknowledge the change of status on the link.
-- 
2.5.0

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

Re: [FFmpeg-devel] [PATCH 2/2] avfilter: make frame writable before writing it

[wm4]

On Sat, 28 Jan 2017 22:23:54 +0700
Muhammad Faiz  wrote:

> affect filters that set partial_buf_size
> test-case
> ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp, asplit 
> [a][b];
> [a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
> [b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
> [a1][b1] vstack'
> 
> Signed-off-by: Muhammad Faiz 
> ---
>  libavfilter/avfilter.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index c8dafd2..55c653d 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1235,6 +1235,11 @@ static int take_samples(AVFilterLink *link, unsigned 
> min, unsigned max,
>  frame = ff_framequeue_peek(>fifo, 0);
>  av_samples_copy(buf->extended_data, frame->extended_data, p, 0, n,
>  link->channels, link->format);
> +
> +ret = ff_inlink_make_frame_writable(link, frame);
> +if (ret < 0)
> +return ret;
> +
>  frame->nb_samples -= n;
>  av_samples_copy(frame->extended_data, frame->extended_data, 0, n,
>  frame->nb_samples, link->channels, link->format);

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

[FFmpeg-devel] [PATCH 2/2] avfilter: make frame writable before writing it

[Muhammad Faiz]

affect filters that set partial_buf_size
test-case
ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp, asplit 
[a][b];
[a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
[b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
[a1][b1] vstack'

Signed-off-by: Muhammad Faiz 
---
 libavfilter/avfilter.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index c8dafd2..55c653d 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -1235,6 +1235,11 @@ static int take_samples(AVFilterLink *link, unsigned 
min, unsigned max,
 frame = ff_framequeue_peek(>fifo, 0);
 av_samples_copy(buf->extended_data, frame->extended_data, p, 0, n,
 link->channels, link->format);
+
+ret = ff_inlink_make_frame_writable(link, frame);
+if (ret < 0)
+return ret;
+
 frame->nb_samples -= n;
 av_samples_copy(frame->extended_data, frame->extended_data, 0, n,
 frame->nb_samples, link->channels, link->format);
-- 
2.5.0

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

Re: [FFmpeg-devel] [PATCH] lavf/tls_openssl: Support building with LibreSSL

[Marek Behun]

On Sat, 28 Jan 2017 14:07:45 +0100
wm4  wrote:

> On Sat, 28 Jan 2017 13:01:54 +
> Mark Thompson  wrote:
> 
> > On 28/01/17 11:28, wm4 wrote:  
> > > On Fri, 27 Jan 2017 19:53:50 +
> > > Mark Thompson  wrote:
> > > 
> > >> On 27/01/17 19:15, Marek Behun wrote:
> > >>> On Fri, 27 Jan 2017 18:41:09 +
> > >>> Mark Thompson  wrote:
> > >>>   
> >  On 27/01/17 17:31, Marek Behún wrote:  
> > > Use the LIBRESSL_VERSION_NUMBER macro to determine if
> > > building with LibreSSL instead of OpenSSL. This is pretty
> > > straightforward, since it is enough to add this check to
> > > existing #if macros.
> > >
> > > Signed-off-by: Marek Behun 
> > > ---
> > >  libavformat/tls_openssl.c | 12 ++--
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavformat/tls_openssl.c
> > > b/libavformat/tls_openssl.c index 3d9768a..cf1a62e 100644
> > > --- a/libavformat/tls_openssl.c
> > > +++ b/libavformat/tls_openssl.c
> > > @@ -43,7 +43,7 @@ typedef struct TLSContext {
> > >  TLSShared tls_shared;
> > >  SSL_CTX *ctx;
> > >  SSL *ssl;
> > > -#if OPENSSL_VERSION_NUMBER >= 0x101fL
> > > +#if OPENSSL_VERSION_NUMBER >= 0x101fL
> > > && !defined(LIBRESSL_VERSION_NUMBER)
> > 
> >  I don't understand what this is trying to do.
> > 
> >  Does LibreSSL support the OpenSSL 1.1.0 API:
> > 
> >  If yes, why would the additional check be needed?
> > 
> >  If no, isn't this doing nothing because the first check would
> >  be false?  
> > >>>
> > >>> LibreSSL defines OPENSSL_VERSION_NUMBER to >=0x200, thus
> > >>> OPENSSL_VERSION_NUMBER is always greater than 0x101, but
> > >>> LibreSSL does not support 1.1.0 API.  
> > >>
> > >> Er, right, so it just lies and leaves it to user programs to
> > >> sort it out.  How nice.
> > >>
> > >> Looking back, I can see this has been discussed before:
> > >> 
> > >> 
> > >>
> > >> That (beyond the disapprobation towards libressl for being
> > >> naughty) looks like people would prefer the test to be in
> > >> configure rather than copying the nontrivial #if condition
> > >> everywhere?
> > > 
> > > Maybe LibreSSL should fix this upstream.
> > > 
> > > They're doing an extreme disservice to everyone by breaking every
> > > single downstream program.
> > > 
> > > I'd even go as far as saying we shouldn't bother with LibreSSL if
> > > trying to keep compatibility is going to be a mess this huge.
> > 
> > If it becomes more inconvenient to do so, yes.   
> 
> > (At that point probably just clone tls_openssl.c to tls_libressl.c
> > and let them diverge if support is still wanted.)  

The support would be wanted, for sure. FreeBSD, OpenBSD and Gentoo want
to support LibreSSL:
- OpenBSD already removed openssl completely, since the security flaws
  of openssl are unacceptable to them.
- FreeBSD states that "Currently there are no binary distributions for
  LibreSSL-in-base but this is to change with the release of FreeBSD
  11."
- Gentoo has a USE flag for libressl and Gentoo bug #561854, which
  tracks LibreSSL incompatibilities, has majority of dependencies
  solved.

My guess is that the OpenBSD guys want the world to get rid of openssl
completely (see for example http://opensslrampage.org/), and breaking
API compatibility with openssl is their "strategy". Well, this strategy
maybe is inconveniet for others, but having seen the code of openssl, I
would rather inconveniet myself with porting to LibreSSL than use
OpenSSL.

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

Re: [FFmpeg-devel] [PATCH] lavf/tls_openssl: Support building with LibreSSL

[wm4]

On Sat, 28 Jan 2017 13:01:54 +
Mark Thompson  wrote:

> On 28/01/17 11:28, wm4 wrote:
> > On Fri, 27 Jan 2017 19:53:50 +
> > Mark Thompson  wrote:
> >   
> >> On 27/01/17 19:15, Marek Behun wrote:  
> >>> On Fri, 27 Jan 2017 18:41:09 +
> >>> Mark Thompson  wrote:
> >>> 
>  On 27/01/17 17:31, Marek Behún wrote:
> > Use the LIBRESSL_VERSION_NUMBER macro to determine if building with
> > LibreSSL instead of OpenSSL. This is pretty straightforward, since
> > it is enough to add this check to existing #if macros.
> >
> > Signed-off-by: Marek Behun 
> > ---
> >  libavformat/tls_openssl.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> > index 3d9768a..cf1a62e 100644
> > --- a/libavformat/tls_openssl.c
> > +++ b/libavformat/tls_openssl.c
> > @@ -43,7 +43,7 @@ typedef struct TLSContext {
> >  TLSShared tls_shared;
> >  SSL_CTX *ctx;
> >  SSL *ssl;
> > -#if OPENSSL_VERSION_NUMBER >= 0x101fL
> > +#if OPENSSL_VERSION_NUMBER >= 0x101fL
> > && !defined(LIBRESSL_VERSION_NUMBER)  
> 
>  I don't understand what this is trying to do.
> 
>  Does LibreSSL support the OpenSSL 1.1.0 API:
> 
>  If yes, why would the additional check be needed?
> 
>  If no, isn't this doing nothing because the first check would be
>  false?
> >>>
> >>> LibreSSL defines OPENSSL_VERSION_NUMBER to >=0x200, thus
> >>> OPENSSL_VERSION_NUMBER is always greater than 0x101, but LibreSSL
> >>> does not support 1.1.0 API.
> >>
> >> Er, right, so it just lies and leaves it to user programs to sort it out.  
> >> How nice.
> >>
> >> Looking back, I can see this has been discussed before:
> >> 
> >> 
> >>
> >> That (beyond the disapprobation towards libressl for being naughty) looks 
> >> like people would prefer the test to be in configure rather than copying 
> >> the nontrivial #if condition everywhere?  
> > 
> > Maybe LibreSSL should fix this upstream.
> > 
> > They're doing an extreme disservice to everyone by breaking every
> > single downstream program.
> > 
> > I'd even go as far as saying we shouldn't bother with LibreSSL if
> > trying to keep compatibility is going to be a mess this huge.  
> 
> If it becomes more inconvenient to do so, yes. 

> (At that point probably just clone tls_openssl.c to tls_libressl.c and
> let them diverge if support is still wanted.)

Well, I suspect LibreSSL will continue their "low quality"
compatibility with OpenSSL, so it will remain a cat and mouse game.

> On the other hand, I think the now-proposed change (configure-detected) is 
> positive even ignoring the existence of LibreSSL, since it moves a whole set 
> of repeated version-conditional #ifs into one configure variable.

Yes, until next time another part of the API breaks.

Anyway, just to make it clear: I don't mind the currently proposed
patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/tls_openssl: Support building with LibreSSL

[Mark Thompson]

On 28/01/17 11:28, wm4 wrote:
> On Fri, 27 Jan 2017 19:53:50 +
> Mark Thompson  wrote:
> 
>> On 27/01/17 19:15, Marek Behun wrote:
>>> On Fri, 27 Jan 2017 18:41:09 +
>>> Mark Thompson  wrote:
>>>   
 On 27/01/17 17:31, Marek Behún wrote:  
> Use the LIBRESSL_VERSION_NUMBER macro to determine if building with
> LibreSSL instead of OpenSSL. This is pretty straightforward, since
> it is enough to add this check to existing #if macros.
>
> Signed-off-by: Marek Behun 
> ---
>  libavformat/tls_openssl.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> index 3d9768a..cf1a62e 100644
> --- a/libavformat/tls_openssl.c
> +++ b/libavformat/tls_openssl.c
> @@ -43,7 +43,7 @@ typedef struct TLSContext {
>  TLSShared tls_shared;
>  SSL_CTX *ctx;
>  SSL *ssl;
> -#if OPENSSL_VERSION_NUMBER >= 0x101fL
> +#if OPENSSL_VERSION_NUMBER >= 0x101fL
> && !defined(LIBRESSL_VERSION_NUMBER)

 I don't understand what this is trying to do.

 Does LibreSSL support the OpenSSL 1.1.0 API:

 If yes, why would the additional check be needed?

 If no, isn't this doing nothing because the first check would be
 false?  
>>>
>>> LibreSSL defines OPENSSL_VERSION_NUMBER to >=0x200, thus
>>> OPENSSL_VERSION_NUMBER is always greater than 0x101, but LibreSSL
>>> does not support 1.1.0 API.  
>>
>> Er, right, so it just lies and leaves it to user programs to sort it out.  
>> How nice.
>>
>> Looking back, I can see this has been discussed before:
>> 
>> 
>>
>> That (beyond the disapprobation towards libressl for being naughty) looks 
>> like people would prefer the test to be in configure rather than copying the 
>> nontrivial #if condition everywhere?
> 
> Maybe LibreSSL should fix this upstream.
> 
> They're doing an extreme disservice to everyone by breaking every
> single downstream program.
> 
> I'd even go as far as saying we shouldn't bother with LibreSSL if
> trying to keep compatibility is going to be a mess this huge.

If it becomes more inconvenient to do so, yes.  (At that point probably just 
clone tls_openssl.c to tls_libressl.c and let them diverge if support is still 
wanted.)

On the other hand, I think the now-proposed change (configure-detected) is 
positive even ignoring the existence of LibreSSL, since it moves a whole set of 
repeated version-conditional #ifs into one configure variable.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: hint next marker position in ff_mjpeg_find_marker

[Matthieu Bouron]

On Sat, Jan 28, 2017 at 12:59:39PM +0100, Matthieu Bouron wrote:
> On Sat, Jan 28, 2017 at 02:25:34AM +0100, Michael Niedermayer wrote:
> > On Fri, Jan 27, 2017 at 09:47:18AM +0100, Matthieu Bouron wrote:
> > > On Fri, Jan 27, 2017 at 02:30:40AM +0100, Michael Niedermayer wrote:
> > > > On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> > > > > Speeds up next marker search when a SOS marker is found.
> > > > > 
> > > > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > > > > sample_2992x4000.jpg
> > > > > ---
> > > > >  libavcodec/mjpegdec.c | 31 +--
> > > > >  libavcodec/mjpegdec.h |  2 +-
> > > > >  libavcodec/mxpegdec.c |  5 +++--
> > > > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > > > 
> > > > this breaks decoding multiple jpeg files
> > > > for example tickets//856/nint.jpg
> > > 
> > > New patch attached fixing the issue.
> > > 
> > > [...]
> > 
> > >  mjpegdec.c |   31 +--
> > >  mjpegdec.h |2 +-
> > >  mxpegdec.c |5 +++--
> > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > > 6ba49d73c189b197bdc1a983e26ccd49713c617c  
> > > 0001-lavc-mjpegdec-hint-next-marker-position-in-ff_mjpeg_.patch
> > > From ebef63ebdc75872d52529d5b74b6d05254d4c0fd Mon Sep 17 00:00:00 2001
> > > From: Matthieu Bouron 
> > > Date: Thu, 26 Jan 2017 15:17:36 +0100
> > > Subject: [PATCH] lavc/mjpegdec: hint next marker position in
> > >  ff_mjpeg_find_marker
> > > 
> > > Speeds up next marker search when a SOS marker is found.
> > > 
> > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > > sample_2992x4000.jpg
> > 
> > the patch seems to work but
> > why is the "buf_ptr += (get_bits_count(>gb) + 7) / 8;" not
> > already skiping all that what the buf_hint does ?
> > is the difference the unescaping ?
> > maybe the buf_hint stuff can be simplified by adjusting buf_ptr instead
> > ?
> 
> I've just realized that the SOS marker handling does not consume any byte
> from the GetBitContext *only* when the frame is being discard (which
> happens in avformat_find_stream_info).
> 
> So I guess a better solution would be to consume all the bytes from the
> GetBitContext while this marker is skipped and let the code already in
> place handle that.
> However, the buf_ptr won't directly be at the next marker position since
> the sos data is unescaped. With the sample I use there is a difference of
> 13774 bytes (thus iterations in find_marker) to find the next marker. It
> reduces the function (according to perf) from 5,4% to 4.8% while the frame
> is being decoded (ffmpeg -i sample_2999x4000.jpeg -f null -) but it seems
> the performance gain is too little versus the code that the patch
> introduces and the decoder will benefits more from having aarch64 simd
> code for the various idct functions (something that i plan to do in the
> upcoming days).
> 
> I'll send a new patch soon.

Alternative patch attached.

Matthieu

[...]
>From 1bbb34b3874393507cc49a4f9685d54850ca5057 Mon Sep 17 00:00:00 2001
From: Matthieu Bouron 
Date: Sat, 28 Jan 2017 13:49:52 +0100
Subject: [PATCH] lavc/mjpegdec: consume SOS data even if the frame is
 discarded

Speeds up next marker search when a SOS marker is found but the frame is
discarded (which happens in avformat_find_stream_info).
---
 libavcodec/mjpegdec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7d17e3dfcb..07988b68b1 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2247,8 +2247,10 @@ eoi_parser:
 goto the_end;
 case SOS:
 s->cur_scan++;
-if (avctx->skip_frame == AVDISCARD_ALL)
+if (avctx->skip_frame == AVDISCARD_ALL) {
+skip_bits(>gb, get_bits_left(>gb));
 break;
+}
 
 if ((ret = ff_mjpeg_decode_sos(s, NULL, 0, NULL)) < 0 &&
 (avctx->err_recognition & AV_EF_EXPLODE))
-- 
2.11.0

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

[FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection by looking at the actual frame rather than the first detected frame.

[Marijn Meijles]

Signed-off-by: Marijn Meijles 
---
 libavformat/ac3dec.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavformat/ac3dec.c b/libavformat/ac3dec.c
index 363a32e..e85b0ac 100644
--- a/libavformat/ac3dec.c
+++ b/libavformat/ac3dec.c
@@ -49,8 +49,8 @@ static int ac3_eac3_probe(AVProbeData *p, enum AVCodecID 
expected_codec_id)
 buf2+=16;
 if (buf[0] == 0x77 && buf[1] == 0x0B) {
 for(i=0; i<8; i+=2) {
-buf3[i  ] = buf[i+1];
-buf3[i+1] = buf[i  ];
+buf3[i  ] = buf2[i+1];
+buf3[i+1] = buf2[i  ];
 }
 init_get_bits(, buf3, 54);
 }else
@@ -62,8 +62,8 @@ static int ac3_eac3_probe(AVProbeData *p, enum AVCodecID 
expected_codec_id)
 if (buf[0] == 0x77 && buf[1] == 0x0B) {
 av_assert0(phdr->frame_size <= sizeof(buf3));
 for(i=8; iframe_size; i+=2) {
-buf3[i  ] = buf[i+1];
-buf3[i+1] = buf[i  ];
+buf3[i  ] = buf2[i+1];
+buf3[i+1] = buf2[i  ];
 }
 }
 if(av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, gbc.buffer + 2, 
phdr->frame_size - 2))
-- 
2.10.1 (Apple Git-78)

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

[FFmpeg-devel] [PATCH] Fix to prevent runaway ac3 detection

[Marijn Meijles]

This is a fix for https://trac.ffmpeg.org/ticket/6106 .

The issue is that when detecting a swapped AC3 marker the data of the
frame is swapped. However, in subsequent frames the data swapped is
taken from the first frame rather than the current frame. So all frames
are considered to be correct even though they might not be. This
leads to a high probe score and eventual misidentification.

We run a service identifying millions of different wav files and 
apparently the chance of one 'correct' AC3 header in pcm data is big
enough to hit this. But the chance of subsequent frames to be 'correct'
is a lot smaller.

I tested this with AC3 and EAC3 files from your samples repo and with
some of our previously misidentified pcm wav files.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat: fix ID3v2 parser for v2.2 comment frames

[Michael Niedermayer]

On Fri, Jan 27, 2017 at 01:20:31PM -0800, Christopher Snowhill wrote:
> From: Chris Moeller 
> 
> ---
>  libavformat/id3v2.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

applied

thanks

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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

Re: [FFmpeg-devel] [PATCH] avfilter/showwavespic : case of empty audio stream, fixes ticket 6107

[Paul B Mahol]

On 1/28/17, wm4  wrote:
> On Fri, 27 Jan 2017 15:09:28 +0300
> Aleksandr Slobodenyuk  wrote:
>
>> From 8d4c7c25674f1967d9ffab69b7ef15381df42ca8 Mon Sep 17 00:00:00 2001
>> From: Aleksandr Slobodeniuk 
>> Date: Fri, 27 Jan 2017 14:57:30 +0300
>> Subject: [PATCH] avfilter/showwavespic : case of empty audio stream, fixes
>>  ticket 6107
>>
>> ---
>>  libavfilter/avf_showspectrum.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/libavfilter/avf_showspectrum.c
>> b/libavfilter/avf_showspectrum.c
>> index 8776668..b422216 100644
>> --- a/libavfilter/avf_showspectrum.c
>> +++ b/libavfilter/avf_showspectrum.c
>> @@ -1032,6 +1032,11 @@ static int
>> showspectrumpic_request_frame(AVFilterLink *outlink)
>>  int ch, spf, spb;
>>  AVFrame *fin;
>>
>> +if (samples == 0) {
>> +av_log(ctx, AV_LOG_ERROR, "Too few samples\n");
>> +return AVERROR(EINVAL);
>> +}
>> +
>>  spf = s->win_size * (samples / ((s->win_size * sz) * ceil(samples
>> / (float)(s->win_size * sz;
>>  spb = (samples / (spf * sz)) * spf;
>>
>
> Any reason why it should error, instead of returning nothing?
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

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

Re: [FFmpeg-devel] patch 2: comments style cleanup in libavcodec/cinepakenc.c

[u-9iep]

On Sat, Jan 28, 2017 at 12:31:33PM +0100, wm4 wrote:
> On Sat, 28 Jan 2017 11:50:27 +0100
> u-9...@aetey.se wrote:
> 
> > Comments style cleanup:
> >  - make all comments follow the same style (C-style)
> > 
> > No code changes, only improved consistency and clarity in the comments.
> > No changes in the comments besides whitespace and the syntactic delimiters.
> > 
> > The original file uses a mixture of C and C++ style comments, not for
> > clarity but for historical reasons (among others commenting in a hurry).
> > 
> > With the change applied the structure of the file is more consequent
> > and also makes easier a possible code reuse with different C compilers.
> > 
> > Attaching the patch.
> > 
> > Regards,
> > Rune
> 
> "//" comments are ok in the ffmpeg codebase, because it's C99.

Yes I knew this. The change is purely cosmetic and not overly important
but in my humble opinion useful.

A consequent style makes it easier to read and understand the code.

Regards,
Rune

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

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Marton Balint]

On Sat, 28 Jan 2017, Michael Niedermayer wrote:


On Sat, Jan 28, 2017 at 01:26:40AM +0100, Andreas Cadhalpun wrote:

On 27.01.2017 02:56, Marton Balint wrote:

I see 3 problems (wm4 explicitly named them, but I also had them in mind)
- Little benefit, yet
- Makes the code less clean, more cluttered
- Increases binary size

The ideas I proposed (use macros, use common / factorized checks for common
validatons and errors) might be a good compromise IMHO. Fuzzing thereforce
can be done with "debug" builds.

Anyway, I am not blocking the patch, just expressing what I would prefer in
the long run.


How about the following macro?
#define FF_RETURN_ERROR(condition, error, log_ctx, ...) {   \
if (condition) {\
if (!CONFIG_SMALL && log_ctx)   \
av_log(log_ctx, AV_LOG_ERROR, __VA_ARGS__); \
return error;   \
}   \
}

That could be used with error message:
FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS, 
AVERROR(ENOSYS),
s, "Too many channels %d > %d\n", st->codecpar->channels, 
FF_SANE_NB_CHANNELS)

Or without:
FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS, 
AVERROR(ENOSYS), NULL)


I suggest
if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
   ff_elog(s, "Too many channels %d > %d\n", st->codecpar->channels, 
FF_SANE_NB_CHANNELS);
   return AVERROR(ENOSYS);
}

or for the 2nd example:

if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
   return AVERROR(ENOSYS);


ff_elog() can then be defined to nothing based on CONFIG_SMALL

the difference between my suggestion and yours is that mine has
new-lines seperating the condition, message and return and that its
self explanatory C code.

What you do is removing newlines and standard C keywords with a custom
macro that people not working on FFmpeg on a regular basis will have
difficulty understanding

The macro is similar to writing (minus the C keywords)
if (st->codecpar->channels > FF_SANE_NB_CHANNELS) { ff_elog(
   s, "Too many channels %d > %d\n", st->codecpar->channels, 
FF_SANE_NB_CHANNELS); return AVERROR(ENOSYS); }

we dont do that becuause its just bad code
why does this suddenly become desirable when its in a macro?

Or maybe the question should be, does code become less cluttered and
cleaner when newlines and C keywords are removed ?
if not why is that done here ?
if yes why is it done just here ?



If we reduce the number of extra lines (not at any cost), I think that 
helps. There is also a solution which keeps the traditional C syntax, and 
is easy to undestand even at first glance.


if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
return ff_elog(AVERROR(ENOSYS), s, "Too many channels %d > %d\n", 
st->codecpar->channels, FF_SANE_NB_CHANNELS);

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

Re: [FFmpeg-devel] patch 2: comments style cleanup in libavcodec/cinepakenc.c

[wm4]

On Sat, 28 Jan 2017 11:50:27 +0100
u-9...@aetey.se wrote:

> Comments style cleanup:
>  - make all comments follow the same style (C-style)
> 
> No code changes, only improved consistency and clarity in the comments.
> No changes in the comments besides whitespace and the syntactic delimiters.
> 
> The original file uses a mixture of C and C++ style comments, not for
> clarity but for historical reasons (among others commenting in a hurry).
> 
> With the change applied the structure of the file is more consequent
> and also makes easier a possible code reuse with different C compilers.
> 
> Attaching the patch.
> 
> Regards,
> Rune

"//" comments are ok in the ffmpeg codebase, because it's C99.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/tls_openssl: Support building with LibreSSL

[wm4]

On Fri, 27 Jan 2017 19:53:50 +
Mark Thompson  wrote:

> On 27/01/17 19:15, Marek Behun wrote:
> > On Fri, 27 Jan 2017 18:41:09 +
> > Mark Thompson  wrote:
> >   
> >> On 27/01/17 17:31, Marek Behún wrote:  
> >>> Use the LIBRESSL_VERSION_NUMBER macro to determine if building with
> >>> LibreSSL instead of OpenSSL. This is pretty straightforward, since
> >>> it is enough to add this check to existing #if macros.
> >>>
> >>> Signed-off-by: Marek Behun 
> >>> ---
> >>>  libavformat/tls_openssl.c | 12 ++--
> >>>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> >>> index 3d9768a..cf1a62e 100644
> >>> --- a/libavformat/tls_openssl.c
> >>> +++ b/libavformat/tls_openssl.c
> >>> @@ -43,7 +43,7 @@ typedef struct TLSContext {
> >>>  TLSShared tls_shared;
> >>>  SSL_CTX *ctx;
> >>>  SSL *ssl;
> >>> -#if OPENSSL_VERSION_NUMBER >= 0x101fL
> >>> +#if OPENSSL_VERSION_NUMBER >= 0x101fL
> >>> && !defined(LIBRESSL_VERSION_NUMBER)
> >>
> >> I don't understand what this is trying to do.
> >>
> >> Does LibreSSL support the OpenSSL 1.1.0 API:
> >>
> >> If yes, why would the additional check be needed?
> >>
> >> If no, isn't this doing nothing because the first check would be
> >> false?  
> > 
> > LibreSSL defines OPENSSL_VERSION_NUMBER to >=0x200, thus
> > OPENSSL_VERSION_NUMBER is always greater than 0x101, but LibreSSL
> > does not support 1.1.0 API.  
> 
> Er, right, so it just lies and leaves it to user programs to sort it out.  
> How nice.
> 
> Looking back, I can see this has been discussed before:
> 
> 
> 
> That (beyond the disapprobation towards libressl for being naughty) looks 
> like people would prefer the test to be in configure rather than copying the 
> nontrivial #if condition everywhere?

Maybe LibreSSL should fix this upstream.

They're doing an extreme disservice to everyone by breaking every
single downstream program.

I'd even go as far as saying we shouldn't bother with LibreSSL if
trying to keep compatibility is going to be a mess this huge.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/showwavespic : case of empty audio stream, fixes ticket 6107

[wm4]

On Fri, 27 Jan 2017 15:09:28 +0300
Александр Слободенюк  wrote:

> From 8d4c7c25674f1967d9ffab69b7ef15381df42ca8 Mon Sep 17 00:00:00 2001
> From: Aleksandr Slobodeniuk 
> Date: Fri, 27 Jan 2017 14:57:30 +0300
> Subject: [PATCH] avfilter/showwavespic : case of empty audio stream, fixes
>  ticket 6107
> 
> ---
>  libavfilter/avf_showspectrum.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavfilter/avf_showspectrum.c b/libavfilter/avf_showspectrum.c
> index 8776668..b422216 100644
> --- a/libavfilter/avf_showspectrum.c
> +++ b/libavfilter/avf_showspectrum.c
> @@ -1032,6 +1032,11 @@ static int showspectrumpic_request_frame(AVFilterLink 
> *outlink)
>  int ch, spf, spb;
>  AVFrame *fin;
>  
> +if (samples == 0) {
> +av_log(ctx, AV_LOG_ERROR, "Too few samples\n");
> +return AVERROR(EINVAL);
> +}
> +
>  spf = s->win_size * (samples / ((s->win_size * sz) * ceil(samples / 
> (float)(s->win_size * sz;
>  spb = (samples / (spf * sz)) * spf;
>  

Any reason why it should error, instead of returning nothing?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: do not write to unwritable frame

[wm4]

On Sat, 28 Jan 2017 15:59:11 +0700
Muhammad Faiz  wrote:

> On 1/28/17, Nicolas George  wrote:
> > L'octidi 8 pluviôse, an CCXXV, Muhammad Faiz a écrit :  
> >> affect filters that set partial_buf_size
> >> test-case
> >> ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp, asplit
> >> [a][b];
> >> [a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
> >> [b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
> >> [a1][b1] vstack'
> >>
> >> Signed-off-by: Muhammad Faiz 
> >> ---
> >>  libavfilter/avfilter.c | 17 +
> >>  1 file changed, 17 insertions(+)  
> >
> > Maybe this can be of use:
> >
> > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/28c62df672865890cbb13e5f0e94bde29c8fbacd
> >   
> 
> Unfortunately, this modify pointer to AVFrame, i think it is incompatible
> with framequeue framework.
> 
> Note that av_frame_make_writable() takes AVFrame*,
> while ff_inlink_make_frame_writable() takes AVFrame**.

There doesn't seem to be any reason that it takes AVFrame**. AVFrame
refs can be moved, so even if a frame is newly allocated it can be put
into the AVFrame passed to the function without replacing the pointer.

Also the code for copying the data in the function could have be
reduced to 1 line by using av_frame_copy().

Maybe someone should have reviewed the commit that added this.

Anyway, maybe you can fix it and use it. That would be the ideal
outcome.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[wm4]

On Fri, 27 Jan 2017 21:48:05 -0500
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Fri, Jan 27, 2017 at 9:28 PM, Michael Niedermayer 
> wrote:
> 
> > On Sat, Jan 28, 2017 at 01:26:40AM +0100, Andreas Cadhalpun wrote:  
> > > On 27.01.2017 02:56, Marton Balint wrote:  
> > > > I see 3 problems (wm4 explicitly named them, but I also had them in  
> > mind)  
> > > > - Little benefit, yet
> > > > - Makes the code less clean, more cluttered
> > > > - Increases binary size
> > > >
> > > > The ideas I proposed (use macros, use common / factorized checks for  
> > common  
> > > > validatons and errors) might be a good compromise IMHO. Fuzzing  
> > thereforce  
> > > > can be done with "debug" builds.
> > > >
> > > > Anyway, I am not blocking the patch, just expressing what I would  
> > prefer in  
> > > > the long run.  
> > >
> > > How about the following macro?
> > > #define FF_RETURN_ERROR(condition, error, log_ctx, ...) {   \
> > > if (condition) {\
> > > if (!CONFIG_SMALL && log_ctx)   \
> > > av_log(log_ctx, AV_LOG_ERROR, __VA_ARGS__); \
> > > return error;   \
> > > }   \
> > > }
> > >
> > > That could be used with error message:
> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,  
> > AVERROR(ENOSYS),  
> > > s, "Too many channels %d > %d\n",  
> > st->codecpar->channels, FF_SANE_NB_CHANNELS)  
> > >
> > > Or without:
> > > FF_RETURN_ERROR(st->codecpar->channels > FF_SANE_NB_CHANNELS,  
> > AVERROR(ENOSYS), NULL)
> >
> > I suggest
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
> > ff_elog(s, "Too many channels %d > %d\n", st->codecpar->channels,
> > FF_SANE_NB_CHANNELS);
> > return AVERROR(ENOSYS);
> > }
> >
> > or for the 2nd example:
> >
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS)
> > return AVERROR(ENOSYS);
> >
> >
> > ff_elog() can then be defined to nothing based on CONFIG_SMALL
> >
> > the difference between my suggestion and yours is that mine has
> > new-lines seperating the condition, message and return and that its
> > self explanatory C code.
> >
> > What you do is removing newlines and standard C keywords with a custom
> > macro that people not working on FFmpeg on a regular basis will have
> > difficulty understanding
> >
> > The macro is similar to writing (minus the C keywords)
> > if (st->codecpar->channels > FF_SANE_NB_CHANNELS) { ff_elog(
> > s, "Too many channels %d > %d\n", st->codecpar->channels,
> > FF_SANE_NB_CHANNELS); return AVERROR(ENOSYS); }
> >
> > we dont do that becuause its just bad code
> > why does this suddenly become desirable when its in a macro?
> >
> > Or maybe the question should be, does code become less cluttered and
> > cleaner when newlines and C keywords are removed ?
> > if not why is that done here ?
> > if yes why is it done just here ?  
> 
> 
> I agree a macro here doesn't help. My concern wasn't with the check itself,
> I agree a file with 100 channels should error out. My concern is that these
> files will universally be the result of fuzzing, so I don't want to spam
> stderr with messages related to it, nor do I want source/binary size to
> increase because of it.
> 
> If you make ff_elog similar to assert (only if NDEBUG is not set), that may
> work for the binary size concern, but the source code size is still a
> concern. Again, not because it's bad code, but because it's needless since
> it only happens for fuzzed samples.

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

[FFmpeg-devel] patch 2: comments style cleanup in libavcodec/cinepakenc.c

[u-9iep]

Comments style cleanup:
 - make all comments follow the same style (C-style)

No code changes, only improved consistency and clarity in the comments.
No changes in the comments besides whitespace and the syntactic delimiters.

The original file uses a mixture of C and C++ style comments, not for
clarity but for historical reasons (among others commenting in a hurry).

With the change applied the structure of the file is more consequent
and also makes easier a possible code reuse with different C compilers.

Attaching the patch.

Regards,
Rune
--- libavcodec/cinepakenc.c.orig2017-01-28 10:10:47.078999401 +0100
+++ libavcodec/cinepakenc.c 2017-01-28 10:28:24.433190895 +0100
@@ -80,22 +80,22 @@
 #define STRIP_HEADER_SIZE 12
 #define CHUNK_HEADER_SIZE 4
 
-#define MB_SIZE 4   //4x4 MBs
+#define MB_SIZE 4   /* 4x4 MBs */
 #define MB_AREA (MB_SIZE*MB_SIZE)
 
-#define VECTOR_MAX 6//six or four entries per vector depending on 
format
-#define CODEBOOK_MAX 256//size of a codebook
+#define VECTOR_MAX 6/* six or four entries per vector depending on 
format */
+#define CODEBOOK_MAX 256/* size of a codebook */
 
-#define MAX_STRIPS  32  //Note: having fewer choices regarding the number 
of strips speeds up encoding (obviously)
-#define MIN_STRIPS  1   //Note: having more strips speeds up encoding the 
frame (this is less obvious)
-// MAX_STRIPS limits the maximum quality you can reach
-//when you want high quality on high resolutions,
-// MIN_STRIPS limits the minimum efficiently encodable bit rate
-//on low resolutions
-// the numbers are only used for brute force optimization for the first frame,
-// for the following frames they are adaptively readjusted
-// NOTE the decoder in ffmpeg has its own arbitrary limitation on the number
-// of strips, currently 32
+#define MAX_STRIPS  32  /* Note: having fewer choices regarding the number 
of strips speeds up encoding (obviously) */
+#define MIN_STRIPS  1   /* Note: having more strips speeds up encoding the 
frame (this is less obvious) */
+/* MAX_STRIPS limits the maximum quality you can reach */
+/*when you want high quality on high resolutions, */
+/* MIN_STRIPS limits the minimum efficiently encodable bit rate */
+/*on low resolutions */
+/* the numbers are only used for brute force optimization for the first frame, 
*/
+/* for the following frames they are adaptively readjusted */
+/* NOTE the decoder in ffmpeg has its own arbitrary limitation on the number */
+/* of strips, currently 32 */
 
 typedef enum {
 MODE_V1_ONLY = 0,
@@ -114,12 +114,12 @@
 } mb_encoding;
 
 typedef struct {
-int v1_vector;  //index into v1 codebook
-int v1_error;   //error when using V1 encoding
-int v4_vector[4];   //indices into v4 codebooks
-int v4_error;   //error when using V4 encoding
-int skip_error; //error when block is skipped (aka copied 
from last frame)
-mb_encoding best_encoding;  //last result from calculate_mode_score()
+int v1_vector;  /* index into v1 codebook */
+int v1_error;   /* error when using V1 encoding */
+int v4_vector[4];   /* indices into v4 codebooks */
+int v4_error;   /* error when using V4 encoding */
+int skip_error; /* error when block is skipped (aka copied 
from last frame) */
+mb_encoding best_encoding;  /* last result from calculate_mode_score() 
*/
 } mb_info;
 
 typedef struct {
@@ -146,15 +146,15 @@
 uint64_t lambda;
 int *codebook_input;
 int *codebook_closest;
-mb_info *mb;//MB RD state
-int min_strips;  //the current limit
-int max_strips;  //the current limit
+mb_info *mb;/* MB RD state */
+int min_strips;  /* the current limit */
+int max_strips;  /* the current limit */
 #ifdef CINEPAKENC_DEBUG
-mb_info *best_mb;   //TODO: remove. only used for 
printing stats
+mb_info *best_mb;   /* TODO: remove. only used for 
printing stats */
 int num_v1_mode, num_v4_mode, num_mc_mode;
 int num_v1_encs, num_v4_encs, num_skips;
 #endif
-// options
+/* options */
 int max_extra_cb_iterations;
 int skip_empty_cb;
 int min_min_strips;
@@ -219,10 +219,10 @@
 
 mb_count = avctx->width * avctx->height / MB_AREA;
 
-//the largest possible chunk is 0x31 with all MBs encoded in V4 mode
-//and full codebooks being replaced in INTER mode,
-// which is 34 bits per MB
-//and 2*256 extra flag bits per strip
+/* the largest possible chunk is 0x31 with all MBs encoded in V4 mode */
+/* and full codebooks being replaced in INTER mode, */
+/* which is 34 bits per MB */
+/* and 2*256 extra flag bits per strip */
 

[FFmpeg-devel] patch 1: comments cleanup in libavcodec/cinepakenc.c

[u-9iep]

Comments cleanup:
 - change the encoding of the original developer name from ISO-8859-1 to UTF-8
 - remove the stale/completed TODO list
 - fix a typo

No code changes, only improved consistency in the comments.

I kindly ask to apply this cleanup, which of course was due from the beginning.

Attaching the patch.

Regards,
Rune
--- libavcodec/cinepakenc.c.orig2016-12-05 23:07:54.0 +0100
+++ libavcodec/cinepakenc.c 2017-01-28 10:10:47.078999401 +0100
@@ -1,5 +1,5 @@
 /*
- * Cinepak encoder (c) 2011 Tomas H�rdin
+ * Cinepak encoder (c) 2011 Tomas Härdin
  * http://titan.codemill.se/~tomhar/cinepakenc.patch
  *
  * Fixes and improvements, vintage decoders compatibility
@@ -23,9 +23,6 @@
 ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 OTHER DEALINGS IN THE SOFTWARE.
 
- * TODO:
- * - optimize: color space conversion, ...
- * - implement options to set the min/max number of strips?
  * MAYBE:
  * - "optimally" split the frame into several non-regular areas
  *   using a separate codebook pair for each area and approximating
@@ -92,7 +89,7 @@
 #define MAX_STRIPS  32  //Note: having fewer choices regarding the number 
of strips speeds up encoding (obviously)
 #define MIN_STRIPS  1   //Note: having more strips speeds up encoding the 
frame (this is less obvious)
 // MAX_STRIPS limits the maximum quality you can reach
-//when you want hight quality on high resolutions,
+//when you want high quality on high resolutions,
 // MIN_STRIPS limits the minimum efficiently encodable bit rate
 //on low resolutions
 // the numbers are only used for brute force optimization for the first frame,
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: do not write to unwritable frame

[Muhammad Faiz]

On 1/28/17, Nicolas George  wrote:
> L'octidi 8 pluviôse, an CCXXV, Muhammad Faiz a écrit :
>> affect filters that set partial_buf_size
>> test-case
>> ffplay -i lavfi 'aevalsrc=sin(1000*t*t), aformat=sample_fmts=fltp, asplit
>> [a][b];
>> [a] firequalizer=fixed=on, showcqt=s=1280x360 [a1];
>> [b] firequalizer=fixed=on, showcqt=s=1280x360 [b1];
>> [a1][b1] vstack'
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  libavfilter/avfilter.c | 17 +
>>  1 file changed, 17 insertions(+)
>
> Maybe this can be of use:
>
> https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/28c62df672865890cbb13e5f0e94bde29c8fbacd

Unfortunately, this modify pointer to AVFrame, i think it is incompatible
with framequeue framework.

Note that av_frame_make_writable() takes AVFrame*,
while ff_inlink_make_frame_writable() takes AVFrame**.

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