Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Paul B Mahol 
On 5/18/17, Muhammad Faiz  wrote:
> Should fix Ticket6349.
> Modifying data pointer may make it unaligned.
>
> Also change frame->nb_samples < max to frame->nb_samples <= max.
> This improves performance. Benchmark:
> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null
> null
> old:
>   25767 decicycles in take_samples,1023 runs,  1 skips
>   25422 decicycles in take_samples,2047 runs,  1 skips
>   25181 decicycles in take_samples,4095 runs,  1 skips
>   24904 decicycles in take_samples,8191 runs,  1 skips
>
> new:
> 550 decicycles in take_samples,1024 runs,  0 skips
> 548 decicycles in take_samples,2048 runs,  0 skips
> 545 decicycles in take_samples,4096 runs,  0 skips
> 544 decicycles in take_samples,8192 runs,  0 skips
>
> Signed-off-by: Muhammad Faiz 
> ---
>  libavfilter/avfilter.c   | 3 ++-
>  libavfilter/framequeue.c | 2 ++
>  libavfilter/framequeue.h | 5 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
>

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

Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Nicolas George 
Le primidi 1er prairial, an CCXXV, Paul B Mahol a écrit :
> LGTM

I want time to comment.

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] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Paul B Mahol 
On 5/20/17, Nicolas George  wrote:
> Le primidi 1er prairial, an CCXXV, Paul B Mahol a ecrit :
>> LGTM
>
> I want time to comment.

No, you just want time to prolong this nightmare.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Nicolas George 
Le primidi 1er prairial, an CCXXV, Paul B Mahol a écrit :
> No, you just want time to prolong this nightmare.

Ad-hominem, non-constructive, fanning the flame.

Muhammad: do not push this patch before I comment.

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] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Nicolas George 
Le decadi 30 floréal, an CCXXV, Muhammad Faiz a écrit :
> Every patch that can fix the crash of
> ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null -
> can claim that it fixes ticket 6349.
> 
> Other cases should be in separate tickets.

No, you are confusing the bug with its symptom. Chewing carefully does
not CURE a dental cavity, it only works around it. The correct cure is
to go to the dentist.

This is the same here: you are working around the bug, but not fixing
it. The correct fix is there:
https://patchwork.ffmpeg.org/patch/3694/
The project having become such an inhospitable place, I will no longer
be working on it, but please feel free to take it over, pushing it as is
or reworking it. Anyway, I will be rejecting any patch that pretends to
"fix" this ticket with changes in lavfi.

> IMHO, the solution is to document it properly to not mix
> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to
> av_buffersink_get_frame vs av_buffersink_get_samples.

As I said, I do not like the idea of limiting the API if it is not
absolutely necessary. And I think it is not. What about this:

If samples_skipped is set, then ff_inlink_consume_frame() can call
ff_inlink_consume_samples() with the exact number of samples to force it
to realign the frame at this point. (Of course, it requires disabling
the case that does the opposite.)

Would you look carefully at the code and tell me I did not miss
something that would make it harder than it looks.

If not, if you confirm it looks feasible, then I think this patch can go
in almost as is: we do not need to actually implement it before it is
required, that would be a waste of time, just add a comment near the
assert to tell it is the plan. (And of course, remove "fix" from the
commit message.)

Now, I am not entirely sure that I understand correctly how this patch
takes into account the number of samples skipped. I would like to look
at it more carefully, but as you can see, Paul is rushing me.

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] New to the list... thinking about a feature

2017-05-20 Thread Pedro Andres Aranda Gutierrez 
Thanks for the encourgament. Patch will follow shortly :-)

Best, /PA

On 19 May 2017 at 01:49, Carl Eugen Hoyos  wrote:

> 2017-05-18 12:20 GMT+02:00 Pedro Andres Aranda Gutierrez <
> paag...@gmail.com>:
>
> > I'm completely new to the list but have a couple of years experience as a
> > user. I happen to work a lot with things I record from  satellite. My
> > recorder will include a lot of empty streams in the transport stream and
> > that is annoying. I have tweaked ffmpeg here and there to show only
> > non-empty streams at the 'normal' verbosity level and include empty
> streams
> > at heightened verbosity.
> >
> > Is that of interest?
>
> Even if your change is not acceptable (we cannot know so far
> but I believe your change was requested repeatedly), this
> mailing list is the right place to archive it.
>
> Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>



-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellet zu werden
Georg Kreisler
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] ffmpeg/ffprobe: Change verbosity level for reporting of empty streams in a Transport Stream

2017-05-20 Thread Pedro Andres Aranda Gutierrez 
Rationale:

Some satellite PVRs store empty streams in recorded transport stream. The
list is of empty streams is sometimes too big to fit one screen-full and
you can't see what ffmpeg is actually doing.

Proposal:
Make the verbosity level required to show empty streams AV_LOG_VERBOSE
instead of AV_LOG_INFO

Impact:
Changes affect only libavformat/dump.c

Patch attached. The approach has been to set the loglevel in some
dump_function at call time. These functions have been renamed to
dump_function_loglevel and new inline functions to replicate the original
dump_function have been added.

KNOWN CAVEAT:
This behaviour for ffmpeg is inherited by ffprobe. Not tested on ffplay.

Best regards,
Pedro A. Aranda
-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellet zu werden
Georg Kreisler
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 8fd58a0..771fbeb 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -130,32 +130,44 @@ static void print_fps(double d, const char *postfix)
 av_log(NULL, AV_LOG_INFO, "%1.0fk %s", d / 1000, postfix);
 }
 
-static void dump_metadata(void *ctx, AVDictionary *m, const char *indent)
+/* dump metatadata information wiht a specific log level */
+
+static void dump_metadata_loglevel(void *ctx, AVDictionary *m, const char 
*indent, int loglevel)
 {
 if (m && !(av_dict_count(m) == 1 && av_dict_get(m, "language", NULL, 0))) {
 AVDictionaryEntry *tag = NULL;
 
-av_log(ctx, AV_LOG_INFO, "%sMetadata:\n", indent);
+av_log(ctx, loglevel, "%sMetadata:\n", indent);
 while ((tag = av_dict_get(m, "", tag, AV_DICT_IGNORE_SUFFIX)))
 if (strcmp("language", tag->key)) {
 const char *p = tag->value;
-av_log(ctx, AV_LOG_INFO,
+av_log(ctx, loglevel,
"%s  %-16s: ", indent, tag->key);
 while (*p) {
 char tmp[256];
 size_t len = strcspn(p, "\x8\xa\xb\xc\xd");
 av_strlcpy(tmp, p, FFMIN(sizeof(tmp), len+1));
-av_log(ctx, AV_LOG_INFO, "%s", tmp);
+av_log(ctx, loglevel, "%s", tmp);
 p += len;
-if (*p == 0xd) av_log(ctx, AV_LOG_INFO, " ");
-if (*p == 0xa) av_log(ctx, AV_LOG_INFO, "\n%s  %-16s: ", 
indent, "");
+if (*p == 0xd) av_log(ctx, loglevel, " ");
+if (*p == 0xa) av_log(ctx, loglevel, "\n%s  %-16s: ", 
indent, "");
 if (*p) p++;
 }
-av_log(ctx, AV_LOG_INFO, "\n");
+av_log(ctx, loglevel, "\n");
 }
 }
 }
 
+/*
+  This is the function that is used through the file,
+  will only call the new version with explicit loglevel
+  when dumping TS information.
+*/
+static inline void dump_metadata(void *ctx, AVDictionary *m, const char 
*indent)
+{
+   dump_metadata_loglevel(ctx, m, indent, AV_LOG_INFO);
+}
+
 /* param change side data*/
 static void dump_paramchange(void *ctx, AVPacketSideData *sd)
 {
@@ -378,78 +390,84 @@ static void dump_spherical(void *ctx, AVCodecParameters 
*par, AVPacketSideData *
 }
 }
 
-static void dump_sidedata(void *ctx, AVStream *st, const char *indent)
+static void dump_sidedata_loglevel(void *ctx, AVStream *st, const char *indent,
+  int loglevel)
 {
 int i;
 
 if (st->nb_side_data)
-av_log(ctx, AV_LOG_INFO, "%sSide data:\n", indent);
+av_log(ctx, loglevel, "%sSide data:\n", indent);
 
 for (i = 0; i < st->nb_side_data; i++) {
 AVPacketSideData sd = st->side_data[i];
-av_log(ctx, AV_LOG_INFO, "%s  ", indent);
+av_log(ctx, loglevel, "%s  ", indent);
 
 switch (sd.type) {
 case AV_PKT_DATA_PALETTE:
-av_log(ctx, AV_LOG_INFO, "palette");
+av_log(ctx, loglevel, "palette");
 break;
 case AV_PKT_DATA_NEW_EXTRADATA:
-av_log(ctx, AV_LOG_INFO, "new extradata");
+av_log(ctx, loglevel, "new extradata");
 break;
 case AV_PKT_DATA_PARAM_CHANGE:
-av_log(ctx, AV_LOG_INFO, "paramchange: ");
+av_log(ctx, loglevel, "paramchange: ");
 dump_paramchange(ctx, &sd);
 break;
 case AV_PKT_DATA_H263_MB_INFO:
-av_log(ctx, AV_LOG_INFO, "H.263 macroblock info");
+av_log(ctx, loglevel, "H.263 macroblock info");
 break;
 case AV_PKT_DATA_REPLAYGAIN:
-av_log(ctx, AV_LOG_INFO, "replaygain: ");
+av_log(ctx, loglevel, "replaygain: ");
 dump_replaygain(ctx, &sd);
 break;
 case AV_PKT_DATA_DISPLAYMATRIX:
-av_log(ctx, AV_LOG_INFO, "displaymatrix: rotation of %.2f degrees",
+av_log(ctx, loglevel, "displaymatrix: rotation of %.2f degrees",
av_dis

Re: [FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Muhammad Faiz 
On Sat, May 20, 2017 at 4:56 PM, Nicolas George  wrote:
> Le decadi 30 floréal, an CCXXV, Muhammad Faiz a écrit :
>> Every patch that can fix the crash of
>> ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null -
>> can claim that it fixes ticket 6349.
>>
>> Other cases should be in separate tickets.
>
> No, you are confusing the bug with its symptom. Chewing carefully does
> not CURE a dental cavity, it only works around it. The correct cure is
> to go to the dentist.
>
> This is the same here: you are working around the bug, but not fixing
> it. The correct fix is there:
> https://patchwork.ffmpeg.org/patch/3694/
> The project having become such an inhospitable place, I will no longer
> be working on it, but please feel free to take it over, pushing it as is
> or reworking it. Anyway, I will be rejecting any patch that pretends to
> "fix" this ticket with changes in lavfi.

OK.

>
>> IMHO, the solution is to document it properly to not mix
>> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to
>> av_buffersink_get_frame vs av_buffersink_get_samples.
>
> As I said, I do not like the idea of limiting the API if it is not
> absolutely necessary. And I think it is not. What about this:
>
> If samples_skipped is set, then ff_inlink_consume_frame() can call
> ff_inlink_consume_samples() with the exact number of samples to force it
> to realign the frame at this point. (Of course, it requires disabling
> the case that does the opposite.)

OK

>
> Would you look carefully at the code and tell me I did not miss
> something that would make it harder than it looks.
>
> If not, if you confirm it looks feasible, then I think this patch can go
> in almost as is: we do not need to actually implement it before it is
> required, that would be a waste of time, just add a comment near the
> assert to tell it is the plan. (And of course, remove "fix" from the
> commit message.)
>
> Now, I am not entirely sure that I understand correctly how this patch
> takes into account the number of samples skipped. I would like to look
> at it more carefully, but as you can see, Paul is rushing me.
>
> Regards,
>
> --
>   Nicolas George
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v2] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Muhammad Faiz 
Modifying data pointer when skipping samples may make it unaligned.
Workaround for Ticket6349.

This should fix the crash of ticket's testcase and a crash/regression
with avxsynth (reported by Michael Niedermayer).

Also change frame->nb_samples < max to frame->nb_samples <= max.
This improves performance. Benchmark:
./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null
old:
  25767 decicycles in take_samples,1023 runs,  1 skips
  25422 decicycles in take_samples,2047 runs,  1 skips
  25181 decicycles in take_samples,4095 runs,  1 skips
  24904 decicycles in take_samples,8191 runs,  1 skips

new:
550 decicycles in take_samples,1024 runs,  0 skips
548 decicycles in take_samples,2048 runs,  0 skips
545 decicycles in take_samples,4096 runs,  0 skips
544 decicycles in take_samples,8192 runs,  0 skips

Signed-off-by: Muhammad Faiz 
---
 libavfilter/avfilter.c   | 8 +++-
 libavfilter/framequeue.c | 2 ++
 libavfilter/framequeue.h | 5 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 08b86b0..e60b024 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, 
unsigned max,
called with enough samples. */
 av_assert1(samples_ready(link, link->min_samples));
 frame0 = frame = ff_framequeue_peek(&link->fifo, 0);
-if (frame->nb_samples >= min && frame->nb_samples < max) {
+if (!link->fifo.samples_skipped && frame->nb_samples >= min && 
frame->nb_samples <= max) {
 *rframe = ff_framequeue_take(&link->fifo);
 return 0;
 }
@@ -1522,6 +1522,12 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame 
**rframe)
 *rframe = NULL;
 if (!ff_inlink_check_available_frame(link))
 return 0;
+
+if (link->fifo.samples_skipped) {
+frame = ff_framequeue_peek(&link->fifo, 0);
+return ff_inlink_consume_samples(link, frame->nb_samples, 
frame->nb_samples, rframe);
+}
+
 frame = ff_framequeue_take(&link->fifo);
 consume_update(link, frame);
 *rframe = frame;
diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c
index 26bfa49..fed1118 100644
--- a/libavfilter/framequeue.c
+++ b/libavfilter/framequeue.c
@@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq)
 fq->tail &= fq->allocated - 1;
 fq->total_frames_tail++;
 fq->total_samples_tail += b->frame->nb_samples;
+fq->samples_skipped = 0;
 check_consistency(fq);
 return b->frame;
 }
@@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t 
samples, AVRational tim
 for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++)
 b->frame->data[i] = b->frame->extended_data[i];
 fq->total_samples_tail += samples;
+fq->samples_skipped = 1;
 ff_framequeue_update_peeked(fq, 0);
 }
diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h
index 5aa2c72..c49d872 100644
--- a/libavfilter/framequeue.h
+++ b/libavfilter/framequeue.h
@@ -100,6 +100,11 @@ typedef struct FFFrameQueue {
  */
 uint64_t total_samples_tail;
 
+/**
+ * Indicate that samples are skipped
+ */
+int samples_skipped;
+
 } FFFrameQueue;
 
 /**
-- 
2.9.3

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

Re: [FFmpeg-devel] [PATCH] lavf/mov: make invalid mdhd time_scale default to 1 instead of erroring out

2017-05-20 Thread Matthieu Bouron 
On Sat, May 20, 2017 at 12:28:53AM +0200, Michael Niedermayer wrote:
> On Wed, May 17, 2017 at 02:58:12PM +0200, Matthieu Bouron wrote:
> > On Wed, May 17, 2017 at 01:56:13PM +0200, Matthieu Bouron wrote:
> > > On Fri, May 12, 2017 at 11:12:12PM +0200, Michael Niedermayer wrote:
> > > > On Thu, May 11, 2017 at 04:33:50PM +0200, Matthieu Bouron wrote:
> > > > > Some samples have their metadata track time_scale incorrectly set to 0
> > > > > and the check introduced by a398f054fdb9b0f0b5a91c231fba6ce014143f71
> > > > > prevents playback of those samples. Setting the time_scale to 1 fixes
> > > > > playback.
> > > > > ---
> > > > >  libavformat/mov.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > should be ok
> > > 
> > > Do you agree if I extend the patch to apply this behaviour to the mvhd
> > > atoms (like a398f054fdb9b0f0b5a91c231fba6ce014143f71 originally did) ?
> > 
> > Here is an updated version of the patch (which extends the behaviour to
> > the mvhd atoms).
> > 
> > -- 
> > Matthieu B.
> 
> >  mov.c |8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 00585940d50d3c53c2041566c01fdab5b6003220  
> > 0001-lavf-mov-make-invalid-m-d-v-hd-time_scale-default-to.patch
> > From 31d808ac885bcf1af1f546a1c8f29b1af82b381b Mon Sep 17 00:00:00 2001
> > From: Matthieu Bouron 
> > Date: Thu, 11 May 2017 15:16:22 +0200
> > Subject: [PATCH] lavf/mov: make invalid m{d,v}hd time_scale default to 1
> >  instead of erroring out
> > 
> > Some samples have their metadata track time_scale incorrectly set to 0
> > and the check introduced by a398f054fdb9b0f0b5a91c231fba6ce014143f71
> > prevents playback of those samples. Setting the time_scale to 1 fixes
> > playback.
> > ---
> >  libavformat/mov.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> LGTM and seems not breaking anything

Applied, thanks.

[..]

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

Re: [FFmpeg-devel] [PATCH V3] lavc/vaapi_encode_h264: Enable MB rate control.

2017-05-20 Thread Mark Thompson 
On 19/05/17 14:10, Mark Thompson wrote:
> On 19/05/17 00:43, Jun Zhao wrote:
>>
>> On 2017/5/14 12:26, Jun Zhao wrote:
>>> V3: - Fix build error with old VAAPI version.
>>> V2: - Refine the name/value type to mb_rate_control/bool.
>>> - Only supported GEN9+ (SKL/APL/KBL/...)
>>> - i965 driver default use frame-level rate control algorithm (generate 
>>> the QP for each frame), 
>>>   when enable mb_rate_control, it's will enable the MB-level RC 
>>> algorithm (generate the QP for each MB). 
>>> - enables MB-level bitrate control that generally improves subjective 
>>> visual quality, 
>>>   but have negative impact on performance and objective visual quality 
>>> metric. 
>>>
>>
>> Ping ?
> 
> I fixed the default/enable/disable inconsistency in 
> .
>   Using 0/1 as default/on is probably fine, but it doesn't match the 
> statement that it's definitely disabled if the option is zero.  (I don't mind 
> which way around that gets fixed.)
> 
> I tested with this patch and it works fine on a Skylake GT3 (6260U), but it 
> killed the whole machine when enabled on a Skylake GT2 (6300).  I haven't yet 
> had chance to investigate that further (rebooting repeatedly is kindof 
> inconvenient), but I will try to do so soon.

This failure is completely consistent.  Reported here: 
.

You will understand that I am reluctant to commit an option which nukes my 
machine, so lets hold off on this patch while the Intel driver people look at 
it.

Thanks,

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

Re: [FFmpeg-devel] [PATCH v2] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Muhammad Faiz 
On Sat, May 20, 2017 at 5:44 PM, Muhammad Faiz  wrote:
> Modifying data pointer when skipping samples may make it unaligned.
> Workaround for Ticket6349.
>
> This should fix the crash of ticket's testcase and a crash/regression
> with avxsynth (reported by Michael Niedermayer).
>
> Also change frame->nb_samples < max to frame->nb_samples <= max.
> This improves performance. Benchmark:
> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null 
> null
> old:
>   25767 decicycles in take_samples,1023 runs,  1 skips
>   25422 decicycles in take_samples,2047 runs,  1 skips
>   25181 decicycles in take_samples,4095 runs,  1 skips
>   24904 decicycles in take_samples,8191 runs,  1 skips
>
> new:
> 550 decicycles in take_samples,1024 runs,  0 skips
> 548 decicycles in take_samples,2048 runs,  0 skips
> 545 decicycles in take_samples,4096 runs,  0 skips
> 544 decicycles in take_samples,8192 runs,  0 skips
>
> Signed-off-by: Muhammad Faiz 
> ---
>  libavfilter/avfilter.c   | 8 +++-
>  libavfilter/framequeue.c | 2 ++
>  libavfilter/framequeue.h | 5 +
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 08b86b0..e60b024 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned 
> min, unsigned max,
> called with enough samples. */
>  av_assert1(samples_ready(link, link->min_samples));
>  frame0 = frame = ff_framequeue_peek(&link->fifo, 0);
> -if (frame->nb_samples >= min && frame->nb_samples < max) {
> +if (!link->fifo.samples_skipped && frame->nb_samples >= min && 
> frame->nb_samples <= max) {
>  *rframe = ff_framequeue_take(&link->fifo);
>  return 0;
>  }
> @@ -1522,6 +1522,12 @@ int ff_inlink_consume_frame(AVFilterLink *link, 
> AVFrame **rframe)
>  *rframe = NULL;
>  if (!ff_inlink_check_available_frame(link))
>  return 0;
> +
> +if (link->fifo.samples_skipped) {
> +frame = ff_framequeue_peek(&link->fifo, 0);
> +return ff_inlink_consume_samples(link, frame->nb_samples, 
> frame->nb_samples, rframe);
> +}
> +
>  frame = ff_framequeue_take(&link->fifo);
>  consume_update(link, frame);
>  *rframe = frame;
> diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c
> index 26bfa49..fed1118 100644
> --- a/libavfilter/framequeue.c
> +++ b/libavfilter/framequeue.c
> @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq)
>  fq->tail &= fq->allocated - 1;
>  fq->total_frames_tail++;
>  fq->total_samples_tail += b->frame->nb_samples;
> +fq->samples_skipped = 0;
>  check_consistency(fq);
>  return b->frame;
>  }
> @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t 
> samples, AVRational tim
>  for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++)
>  b->frame->data[i] = b->frame->extended_data[i];
>  fq->total_samples_tail += samples;
> +fq->samples_skipped = 1;
>  ff_framequeue_update_peeked(fq, 0);
>  }
> diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h
> index 5aa2c72..c49d872 100644
> --- a/libavfilter/framequeue.h
> +++ b/libavfilter/framequeue.h
> @@ -100,6 +100,11 @@ typedef struct FFFrameQueue {
>   */
>  uint64_t total_samples_tail;
>
> +/**
> + * Indicate that samples are skipped
> + */
> +int samples_skipped;
> +
>  } FFFrameQueue;
>
>  /**
> --
> 2.9.3
>

I will push this soon.

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

[FFmpeg-devel] [PATCH] avcodec/takdec: Fix multiple runtime error: signed integer overflow: -512 * 4563386 cannot be represented in type 'int'

2017-05-20 Thread Michael Niedermayer 
Fixes: 1706/clusterfuzz-testcase-minimized-6112772670619648

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/takdec.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/takdec.c b/libavcodec/takdec.c
index 31d703135a..65dbc060a8 100644
--- a/libavcodec/takdec.c
+++ b/libavcodec/takdec.c
@@ -447,12 +447,12 @@ static int decode_subframe(TAKDecContext *s, int32_t 
*decoded,
 
 tfilter[0] = s->predictors[0] * 64;
 for (i = 1; i < filter_order; i++) {
-int32_t *p1 = &tfilter[0];
-int32_t *p2 = &tfilter[i - 1];
+uint32_t *p1 = &tfilter[0];
+uint32_t *p2 = &tfilter[i - 1];
 
 for (j = 0; j < (i + 1) / 2; j++) {
-x = *p1 + (s->predictors[i] * *p2 + 256 >> 9);
-*p2  += s->predictors[i] * *p1 + 256 >> 9;
+x = *p1 + ((int32_t)(s->predictors[i] * *p2 + 256) >> 9);
+*p2  += (int32_t)(s->predictors[i] * *p1 + 256) >> 9;
 *p1++ = x;
 p2--;
 }
-- 
2.13.0

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

Re: [FFmpeg-devel] [PATCH v2] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Muhammad Faiz 
On Sat, May 20, 2017 at 7:33 PM, Muhammad Faiz  wrote:
> On Sat, May 20, 2017 at 5:44 PM, Muhammad Faiz  wrote:
>> Modifying data pointer when skipping samples may make it unaligned.
>> Workaround for Ticket6349.
>>
>> This should fix the crash of ticket's testcase and a crash/regression
>> with avxsynth (reported by Michael Niedermayer).
>>
>> Also change frame->nb_samples < max to frame->nb_samples <= max.
>> This improves performance. Benchmark:
>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null 
>> null
>> old:
>>   25767 decicycles in take_samples,1023 runs,  1 skips
>>   25422 decicycles in take_samples,2047 runs,  1 skips
>>   25181 decicycles in take_samples,4095 runs,  1 skips
>>   24904 decicycles in take_samples,8191 runs,  1 skips
>>
>> new:
>> 550 decicycles in take_samples,1024 runs,  0 skips
>> 548 decicycles in take_samples,2048 runs,  0 skips
>> 545 decicycles in take_samples,4096 runs,  0 skips
>> 544 decicycles in take_samples,8192 runs,  0 skips
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  libavfilter/avfilter.c   | 8 +++-
>>  libavfilter/framequeue.c | 2 ++
>>  libavfilter/framequeue.h | 5 +
>>  3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
>> index 08b86b0..e60b024 100644
>> --- a/libavfilter/avfilter.c
>> +++ b/libavfilter/avfilter.c
>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned 
>> min, unsigned max,
>> called with enough samples. */
>>  av_assert1(samples_ready(link, link->min_samples));
>>  frame0 = frame = ff_framequeue_peek(&link->fifo, 0);
>> -if (frame->nb_samples >= min && frame->nb_samples < max) {
>> +if (!link->fifo.samples_skipped && frame->nb_samples >= min && 
>> frame->nb_samples <= max) {
>>  *rframe = ff_framequeue_take(&link->fifo);
>>  return 0;
>>  }
>> @@ -1522,6 +1522,12 @@ int ff_inlink_consume_frame(AVFilterLink *link, 
>> AVFrame **rframe)
>>  *rframe = NULL;
>>  if (!ff_inlink_check_available_frame(link))
>>  return 0;
>> +
>> +if (link->fifo.samples_skipped) {
>> +frame = ff_framequeue_peek(&link->fifo, 0);
>> +return ff_inlink_consume_samples(link, frame->nb_samples, 
>> frame->nb_samples, rframe);
>> +}
>> +
>>  frame = ff_framequeue_take(&link->fifo);
>>  consume_update(link, frame);
>>  *rframe = frame;
>> diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c
>> index 26bfa49..fed1118 100644
>> --- a/libavfilter/framequeue.c
>> +++ b/libavfilter/framequeue.c
>> @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq)
>>  fq->tail &= fq->allocated - 1;
>>  fq->total_frames_tail++;
>>  fq->total_samples_tail += b->frame->nb_samples;
>> +fq->samples_skipped = 0;
>>  check_consistency(fq);
>>  return b->frame;
>>  }
>> @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t 
>> samples, AVRational tim
>>  for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++)
>>  b->frame->data[i] = b->frame->extended_data[i];
>>  fq->total_samples_tail += samples;
>> +fq->samples_skipped = 1;
>>  ff_framequeue_update_peeked(fq, 0);
>>  }
>> diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h
>> index 5aa2c72..c49d872 100644
>> --- a/libavfilter/framequeue.h
>> +++ b/libavfilter/framequeue.h
>> @@ -100,6 +100,11 @@ typedef struct FFFrameQueue {
>>   */
>>  uint64_t total_samples_tail;
>>
>> +/**
>> + * Indicate that samples are skipped
>> + */
>> +int samples_skipped;
>> +
>>  } FFFrameQueue;
>>
>>  /**
>> --
>> 2.9.3
>>
>
> I will push this soon.
>
> Thank's.

Pushed and backported to 3.3 branch.

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

Re: [FFmpeg-devel] [PATCH 2/4] h264dec: be more explicit in handling container cropping

2017-05-20 Thread James Almer 
On 5/11/2017 9:56 AM, Michael Niedermayer wrote:
> On Wed, May 10, 2017 at 11:06:52PM -0300, James Almer wrote:
>> On 5/10/2017 9:55 PM, Michael Niedermayer wrote:
>>> On Wed, May 10, 2017 at 10:41:30AM -0300, James Almer wrote:
 On 5/9/2017 11:56 PM, Michael Niedermayer wrote:
> On Mon, May 08, 2017 at 03:46:23PM -0300, James Almer wrote:
>> From: Anton Khirnov 
>>
>> The current condition can trigger in cases where it shouldn't, with
>> unexpected results.
>> Make sure that:
>> - container cropping is really based on the original dimensions from the
>>   caller
>> - those dimenions are discarded on size change
>>
>> The code is still quite hacky and eventually should be deprecated and
>> removed, with the decision about which cropping is used delegated to the
>> caller.
>> ---
>> This merges commit 4fded0480f20f4d7ca5e776a85574de34dfead14 from libav
>>
>>  libavcodec/h264_slice.c | 20 +---
>>  libavcodec/h264dec.c|  3 +++
>>  libavcodec/h264dec.h|  5 +
>>  3 files changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>> index acf6a73f60..a7916e09ce 100644
>> --- a/libavcodec/h264_slice.c
>> +++ b/libavcodec/h264_slice.c
>> @@ -378,6 +378,8 @@ int ff_h264_update_thread_context(AVCodecContext 
>> *dst,
>>  h->avctx->coded_width   = h1->avctx->coded_width;
>>  h->avctx->width = h1->avctx->width;
>>  h->avctx->height= h1->avctx->height;
>> +h->width_from_caller= h1->width_from_caller;
>> +h->height_from_caller   = h1->height_from_caller;
>>  h->coded_picture_number = h1->coded_picture_number;
>>  h->first_field  = h1->first_field;
>>  h->picture_structure= h1->picture_structure;
>
>> @@ -874,13 +876,17 @@ static int init_dimensions(H264Context *h)
>>  av_assert0(sps->crop_top + sps->crop_bottom < (unsigned)h->height);
>>  
>>  /* handle container cropping */
>> -if (FFALIGN(h->avctx->width,  16) == FFALIGN(width,  16) &&
>> -FFALIGN(h->avctx->height, 16) == FFALIGN(height, 16) &&
>> -h->avctx->width  <= width &&
>> -h->avctx->height <= height
>> -) {
>> -width  = h->avctx->width;
>> -height = h->avctx->height;
>> +if (h->width_from_caller > 0 && h->height_from_caller > 0 &&
>> +!sps->crop_top && !sps->crop_left &&
>> +FFALIGN(h->width_from_caller,  16) == FFALIGN(width,  16) &&
>> +FFALIGN(h->height_from_caller, 16) == FFALIGN(height, 16) &&
>> +h->width_from_caller  <= width &&
>> +h->height_from_caller <= height) {
>> +width  = h->width_from_caller;
>> +height = h->height_from_caller;
>> +} else {
>> +h->width_from_caller  = 0;
>> +h->height_from_caller = 0;
>>  }
>
> With this, seeking in a file could affect if croping is used
> would something break if croping was unaffected by what was priorly
> decoded ?

 I don't know. Do you have a test case where this could break that i can
 check?
>>>
>>> no, it was just an thought that came to my mind when looking at the
>>> code. I dont remember seeing this break anything, it changed some
>>> files output though unless these were caused by another patch i had
>>> locally
>>
>> Could you try to confirm they weren't changed by this patch? Unless i'm
>> reading it wrong, this set afaik isn't supposed to change the output of
>> the decoder (at least not negatively), as reflected by fate, just move
>> the cropping logic to decode.c
> 
> I retested, it was
> [3/4] h264dec: export cropping information instead of handling it internally
> that caused the changes
> changes seen are with CVFC1_Sony_C.jsv and tickets/4274/sample.ts
> 
> 4247 needs "-threads 1  -flags2 showall -ss 7" for showin the
> difference, the sony file shows a difference with just plain default
> reencoding to avi
> 
> Our fate test doesnt change, i guess due to -flags unaligned in it
> 
> thx

CVFC1_Sony_C.jsv is fixed now that the cropping logic works correctly.
tickets/4274/sample.ts still shows the difference, but it changes
garbage output with slightly different, less ugly but still garbage output.

Old: http://0x0.st/ghF.png
New: http://0x0.st/ghC.png

Unless this can be reproduced with negative effects with a sane file and
not with a badly cut mpegts stream with missing references that requires
seeking and a some specific flags, i'm inclined to not consider it worth
bothering with.

I'll be pushing the set sometime next week if no other regressions are
found.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/takdec: Fix multiple runtime error: signed integer overflow: -512 * 4563386 cannot be represented in type 'int'

2017-05-20 Thread Marton Balint 


On Sat, 20 May 2017, Michael Niedermayer wrote:


Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg


This URL is a 404 for me, I guess the correct URL is 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg


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

Re: [FFmpeg-devel] [PATCH v2] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Nicolas George 
Le primidi 1er prairial, an CCXXV, Muhammad Faiz a écrit :
> > I will push this soon.
> Pushed and backported to 3.3 branch.

Well, I was probably ok with this patch, but we will never know, will
we?

Can you explain the concept behind posting a patch for review and then
pushing it before anybody could possibly review?

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 3/3] avcodec/mlpdec: Check quant_step_size against huff_lsbs

2017-05-20 Thread Michael Niedermayer 
This reorders the operations so as to avoid computations with the above 
arguments
before they have been initialized.
Fixes part of 1708/clusterfuzz-testcase-minimized-5035111957397504

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/mlpdec.c | 34 +-
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c
index c0a23c5f0d..11be380d27 100644
--- a/libavcodec/mlpdec.c
+++ b/libavcodec/mlpdec.c
@@ -825,8 +825,6 @@ static int read_channel_params(MLPDecodeContext *m, 
unsigned int substr,
 return AVERROR_INVALIDDATA;
 }
 
-cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
-
 return 0;
 }
 
@@ -838,7 +836,8 @@ static int read_decoding_params(MLPDecodeContext *m, 
GetBitContext *gbp,
 {
 SubStream *s = &m->substream[substr];
 unsigned int ch;
-int ret;
+int ret = 0;
+unsigned recompute_sho = 0;
 
 if (s->param_presence_flags & PARAM_PRESENCE)
 if (get_bits1(gbp))
@@ -878,19 +877,36 @@ static int read_decoding_params(MLPDecodeContext *m, 
GetBitContext *gbp,
 if (s->param_presence_flags & PARAM_QUANTSTEP)
 if (get_bits1(gbp))
 for (ch = 0; ch <= s->max_channel; ch++) {
-ChannelParams *cp = &s->channel_params[ch];
-
 s->quant_step_size[ch] = get_bits(gbp, 4);
 
-cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
+recompute_sho |= 1max_channel; ch++)
-if (get_bits1(gbp))
+if (get_bits1(gbp)) {
+recompute_sho |= 1codebook > 0 && cp->huff_lsbs < s->quant_step_size[ch]) {
+if (ret >= 0) {
+av_log(m->avctx, AV_LOG_ERROR, "quant_step_size larger 
than huff_lsbs\n");
+ret = AVERROR_INVALIDDATA;
+}
+s->quant_step_size[ch] = 0;
+}
+
+cp->sign_huff_offset = calculate_sign_huff(m, substr, ch);
+}
+}
+return ret;
 }
 
 #define MSB_MASK(bits)  (-1u << (bits))
-- 
2.13.0

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

[FFmpeg-devel] [PATCH 2/3] avcodec/mlpdec: Fix runtime error: shift exponent -5 is negative

2017-05-20 Thread Michael Niedermayer 
Fixes part of 1708/clusterfuzz-testcase-minimized-5035111957397504

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/mlpdec.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c
index eac19a0d5e..c0a23c5f0d 100644
--- a/libavcodec/mlpdec.c
+++ b/libavcodec/mlpdec.c
@@ -861,8 +861,13 @@ static int read_decoding_params(MLPDecodeContext *m, 
GetBitContext *gbp,
 
 if (s->param_presence_flags & PARAM_OUTSHIFT)
 if (get_bits1(gbp)) {
-for (ch = 0; ch <= s->max_matrix_channel; ch++)
+for (ch = 0; ch <= s->max_matrix_channel; ch++) {
 s->output_shift[ch] = get_sbits(gbp, 4);
+if (s->output_shift[ch] < 0) {
+avpriv_request_sample(m->avctx, "Negative output_shift");
+s->output_shift[ch] = 0;
+}
+}
 if (substr == m->max_decoded_substream)
 m->dsp.mlp_pack_output = 
m->dsp.mlp_select_pack_output(s->ch_assign,

s->output_shift,
-- 
2.13.0

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

[FFmpeg-devel] [PATCH 1/3] avcodec/escape124: Check depth against num_superblocks

2017-05-20 Thread Michael Niedermayer 
Fixes: runtime error: left shift of 66184 by 15 places cannot be represented in 
type 'int'
Fixes: 1707/clusterfuzz-testcase-minimized-6502767008940032

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/escape124.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/escape124.c b/libavcodec/escape124.c
index c3174ce6ef..eb051eba54 100644
--- a/libavcodec/escape124.c
+++ b/libavcodec/escape124.c
@@ -267,6 +267,11 @@ static int escape124_decode_frame(AVCodecContext *avctx,
 cb_size = s->num_superblocks << cb_depth;
 }
 }
+if (s->num_superblocks >= INT_MAX >> cb_depth) {
+av_log(avctx, AV_LOG_ERROR, "Depth or num_superblocks are too 
large\n");
+return AVERROR_INVALIDDATA;
+}
+
 av_freep(&s->codebooks[i].blocks);
 s->codebooks[i] = unpack_codebook(&gb, cb_depth, cb_size);
 if (!s->codebooks[i].blocks)
-- 
2.13.0

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

Re: [FFmpeg-devel] [PATCH] avcodec/takdec: Fix multiple runtime error: signed integer overflow: -512 * 4563386 cannot be represented in type 'int'

2017-05-20 Thread Michael Niedermayer 
On Sat, May 20, 2017 at 07:26:50PM +0200, Marton Balint wrote:
> 
> On Sat, 20 May 2017, Michael Niedermayer wrote:
> 
> >Found-by: continuous fuzzing process 
> >https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> 
> This URL is a 404 for me, I guess the correct URL is
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg

yes that was apparently changed in a143b9b39a51412d133f846688194d68fe4197ba

ill fix the link in future commits

thx

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- 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 v2] avfilter: take_samples: do not directly return frame when samples are skipped

2017-05-20 Thread Muhammad Faiz 
On Sun, May 21, 2017 at 1:12 AM, Nicolas George  wrote:
> Le primidi 1er prairial, an CCXXV, Muhammad Faiz a écrit :
>> > I will push this soon.
>> Pushed and backported to 3.3 branch.
>
> Well, I was probably ok with this patch, but we will never know, will
> we?

Thank's.

>
> Can you explain the concept behind posting a patch for review and then
> pushing it before anybody could possibly review?

I'm sorry for that. I was tired and I thought that it had already been
reviewed before.

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

[FFmpeg-devel] [PATCH] avcodec/vp9block: fix runtime error: signed integer overflow: 196675 * 20670 cannot be represented in type 'int'

2017-05-20 Thread Michael Niedermayer 
Fixes: 1710/clusterfuzz-testcase-minimized-4837032931098624

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/vp9block.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vp9block.c b/libavcodec/vp9block.c
index ae2f0e4c6f..a16ccdccdb 100644
--- a/libavcodec/vp9block.c
+++ b/libavcodec/vp9block.c
@@ -915,9 +915,9 @@ skip_eob:
 if (!--band_left)
 band_left = band_counts[++band];
 if (is_tx32x32)
-STORE_COEF(coef, rc, ((vp8_rac_get(c) ? -val : val) * qmul[!!i]) / 
2);
+STORE_COEF(coef, rc, (int)((vp8_rac_get(c) ? -val : val) * 
(unsigned)qmul[!!i]) / 2);
 else
-STORE_COEF(coef, rc, (vp8_rac_get(c) ? -val : val) * qmul[!!i]);
+STORE_COEF(coef, rc, (vp8_rac_get(c) ? -val : val) * 
(unsigned)qmul[!!i]);
 nnz = (1 + cache[nb[i][0]] + cache[nb[i][1]]) >> 1;
 tp = p[band][nnz];
 } while (++i < n_coeffs);
-- 
2.13.0

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

Re: [FFmpeg-devel] [PATCH] libavformat/hls: Observe Set-Cookie headers

2017-05-20 Thread Micah Galizia 

On 2017-05-17 05:23 AM, wm4 wrote:

On Sat, 6 May 2017 14:28:10 -0400
Micah Galizia  wrote:


On 2017-05-05 09:28 PM, wm4 wrote:

On Fri,  5 May 2017 20:55:05 -0400
Micah Galizia  wrote:
  

Signed-off-by: Micah Galizia 
---
   libavformat/hls.c | 12 ++--
   1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index bac53a4350..bda9abecfa 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -630,8 +630,16 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, 
const char *url,
   ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
   if (ret >= 0) {
   // update cookies on http response with setcookies.
-void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
-update_options(&c->cookies, "cookies", u);
+char *new_cookies = NULL;
+
+if (s->flags ^ AVFMT_FLAG_CUSTOM_IO)
+av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, 
(uint8_t**)&new_cookies);

Did you mean & instead of ^?

No, the original code was structured to set *u to null (and thus did not
copy cookies) iff AVFMT_FLAG_CUSTOM_IO was set in the flags.  So using ^
is logically equivalent -- cookies are copied only if
AVFMT_FLAG_CUSTOM_IO is not set.


Did you find out yet what difference AVFMT_FLAG_CUSTOM_IO is supposed
to make in the existing code?

Yes, I wrote back about it a day or two ago... here is the reference to
its original inclusion:
https://lists.ffmpeg.org/pipermail/ffmpeg-trac/2013-February/013020.html.
When the code was originally implemented we were using s->pb->opaque,
which in FFMPEG we could assume was a URLContext with options (one of
them possibly being "cookies").

Based on the email above, that wasn't true for other apps like mplayer,
and could cause crashes trying to treat opaque as a URLContext. So that
is the purpose of checking the AVFMT_FLAG_CUSTOM_IO flag (way back in 2013).

Now though, we don't seem to touch opaque at all. The options are stored
in AVIOContext's av_class, which does have options.  Based on this I
think both patches are safe: this version has less change, but the
original gets rid of the check against AVFMT_FLAG_CUSTOM_IO that I don't
think we need anymore.

I hope that clears things up.

Sorry, I missed that. I guess this code is an artifact of some severely
unclean hook up of the AVOPtion API to AVIOContext, that breaks with
custom I/O. I guess your patch is fine then.

I just wonder how an API user is supposed to pass along cookies then.
My code passes an AVDictionary via a "cookies" entry when opening the
HLS demuxer with custom I/O, so I was wondering whether the patch
changes behavior here.


I wouldn't have expected anyone to pass the cookies to the HLS demuxer 
directly -- there is an http protocol AVOption that should pass it along 
to the demuxer. But I guess thats the whole point of the custom IO, 
right? It'd replace the http demuxer?


Even so, I don't think this is a good reason to hold up the the patch - 
it corrects the problem for apps that use the http protocol and 
maintains the existing behavior -- cookies are not (and were not) copied 
when the custom flag is set because u was set to null. Am I wrong in 
that interpretation of the existing behavior?


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

Re: [FFmpeg-devel] [PATCH] avcodec/vp9block: fix runtime error: signed integer overflow: 196675 * 20670 cannot be represented in type 'int'

2017-05-20 Thread Ronald S. Bultje 
Hi,

On Sat, May 20, 2017 at 8:12 PM, Michael Niedermayer  wrote:

> Fixes: 1710/clusterfuzz-testcase-minimized-4837032931098624
>
> Found-by: continuous fuzzing process https://github.com/google/oss-
> fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/vp9block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/vp9block.c b/libavcodec/vp9block.c
> index ae2f0e4c6f..a16ccdccdb 100644
> --- a/libavcodec/vp9block.c
> +++ b/libavcodec/vp9block.c
> @@ -915,9 +915,9 @@ skip_eob:
>  if (!--band_left)
>  band_left = band_counts[++band];
>  if (is_tx32x32)
> -STORE_COEF(coef, rc, ((vp8_rac_get(c) ? -val : val) *
> qmul[!!i]) / 2);
> +STORE_COEF(coef, rc, (int)((vp8_rac_get(c) ? -val : val) *
> (unsigned)qmul[!!i]) / 2);
>  else
> -STORE_COEF(coef, rc, (vp8_rac_get(c) ? -val : val) *
> qmul[!!i]);
> +STORE_COEF(coef, rc, (vp8_rac_get(c) ? -val : val) *
> (unsigned)qmul[!!i]);
>  nnz = (1 + cache[nb[i][0]] + cache[nb[i][1]]) >> 1;
>  tp = p[band][nnz];
>  } while (++i < n_coeffs);
> --
> 2.13.0


Since this is the only use of qmul[], why don't you make the array unsigned
instead? That saves a cast.

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