Re: [FFmpeg-devel] [PATCH] avformat/aiffdec: avoid double and ldexp()
> -sample_rate = ldexp(val, exp - 16383 - 63); > +if (exp >= 0) > +sample_rate = val << exp; > +else > +sample_rate = (val + (1<<(-exp/2))) >> -exp; To round the value it should probably be something like: sample_rate = (val + ((uint64_t)1<<(-exp-1))) >> -exp; To avoid possible undefined behavior it would also need to check that exp is within range. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: autodetect OpenCL headers and ICD loader
> -Original Message-
> From: ffmpeg-devel-boun...@ffmpeg.org [mailto:ffmpeg-devel-
> boun...@ffmpeg.org] On Behalf Of James Almer
> Sent: Thursday, May 28, 2015 10:06 AM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH] configure: autodetect OpenCL headers
> and ICD loader
>
> On 28/05/15 12:28 AM, Gupta, Maneesh wrote:
> > Attached is a patch that enables configure to autodetect the presence of
> OpenCL headers and ICD loader. If the necessary headers are found, the
> OpenCL infrastructure compilation is enabled as well. Note that this does not
> modify the runtime behavior in any way. Execution of the OpenCL code path
> still requires the user to explicitly enable it.
> >
> > Please review the patch and share your comments on the same.
> >
> > - Maneesh
>
> I personally don't agree OpenCL should be autodetected.
>
> Currently, only the so called "system" libs are autodetected (zlib, iconv,
> pthreads, the like). Open*L, decoding/encoding libraries, and other
> miscellaneous libraries are enabled only if explicitly requested.
> If we enable one by default, then there's no argument against enabling
> every other as well ("Why this one but not that other one?"), and suddenly
> default builds become bloated and automated scripts get potentially broken
> until adapted.
>
> Lets keep default builds as lightweight as possible, and allow people to
> explicitly enable the functionality they require.
[Gupta, Maneesh] Apologize for the delayed response. However grep'ing the
string "autodetect" in the configure script throws up several "non-system" libs
including D3D11VA, DXVA2, VAAPI, VDA, VDPAU, sdl. Why are they turned on by
default? Several of these non-system libs enable use of hardware acceleration.
OpenCL is similar in nature with a key difference that it is more platform/OS
agnostic and can run on both CPU as well as GPU.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] patch ffserver
On Fri, May 22, 2015 at 10:07:17PM -0600, ill wrote: > So can the ffserver patch be committed? > > > > > ffserver.c |4 > ffserver_config.c|6 -- > ffserver_config.h|1 + > libavformat/allformats.c |1 + > libavformat/rawenc.c | 12 > 5 files changed, 22 insertions(+), 2 deletions(-) > cd08a86e251ccc0e104a7719ec1e686028d0580c 0001-ffserver-jpg-patch.patch > From 5b681fdaf7971f841b84cfe06412c67ba417fb00 Mon Sep 17 00:00:00 2001 > From: Caligula useraccount > Date: Mon, 11 May 2015 17:42:42 -0600 > Subject: [PATCH] ffserver jpg patch split, cleaned up and applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Reenable GHA phase inversion in Atrac3+
> > > > I've recently got a couple of appropriate samples from the PPSSPP > emulator > > project so I coud test this feature extensively. > > where can i find these samples ? > are they online somewhere? can you upload them ? > I've just uploaded a folder named "atrac3p GHA phase inversion" to upload.ffmpeg.org/incoming. Hopefully, it does contain everything you need. > > > Do we need to update FATE tests to include a test for this feature? > > a fate test might be interresting if its possible/reliable and > not too hard to implement > Unfortunately, I don't have no idea how to make FATE tests myself so any help would be very appreciated! Best regards Maxim ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Reenable GHA phase inversion in Atrac3+
On Mon, Jun 08, 2015 at 02:17:16AM +0200, Max Pole wrote: > Hello crews, > > I attached two simple patches reenabling a decoder feature been initially > disabled due to lack of real-world samples. > > I've recently got a couple of appropriate samples from the PPSSPP emulator > project so I coud test this feature extensively. where can i find these samples ? are they online somewhere? can you upload them ? > > The 2nd patch is a cosmetical one that renames the "phase_shift" variable > to "inverse_phase". This seems to give a better description of what's > actually going on (180° phase shift). > > Do we need to update FATE tests to include a test for this feature? a fate test might be interresting if its possible/reliable and not too hard to implement Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Reenable GHA phase inversion in Atrac3+
Hello crews,
I attached two simple patches reenabling a decoder feature been initially
disabled due to lack of real-world samples.
I've recently got a couple of appropriate samples from the PPSSPP emulator
project so I coud test this feature extensively.
The 2nd patch is a cosmetical one that renames the "phase_shift" variable
to "inverse_phase". This seems to give a better description of what's
actually going on (180° phase shift).
Do we need to update FATE tests to include a test for this feature?
Best regards
Maxim
From 9f4931faba250755c7d546596636ad7bdf150566 Mon Sep 17 00:00:00 2001
From: Max Poliakovski
Date: Sat, 6 Jun 2015 23:28:47 +0200
Subject: [PATCH 1/2] atrac3plus: add support for GHA phase inversion.
---
libavcodec/atrac3plus.c| 6 +-
libavcodec/atrac3plusdsp.c | 10 --
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/libavcodec/atrac3plus.c b/libavcodec/atrac3plus.c
index f998a7f..3d75749 100644
--- a/libavcodec/atrac3plus.c
+++ b/libavcodec/atrac3plus.c
@@ -1724,11 +1724,7 @@ static int decode_tones_info(GetBitContext *gb, Atrac3pChanUnitCtx *ctx,
if (num_channels == 2) {
get_subband_flags(gb, ctx->waves_info->tone_sharing, ctx->waves_info->num_tone_bands);
get_subband_flags(gb, ctx->waves_info->tone_master, ctx->waves_info->num_tone_bands);
-if (get_subband_flags(gb, ctx->waves_info->phase_shift,
- ctx->waves_info->num_tone_bands)) {
-avpriv_report_missing_feature(avctx, "GHA Phase shifting");
-return AVERROR_PATCHWELCOME;
-}
+get_subband_flags(gb, ctx->waves_info->phase_shift, ctx->waves_info->num_tone_bands);
}
ctx->waves_info->tones_index = 0;
diff --git a/libavcodec/atrac3plusdsp.c b/libavcodec/atrac3plusdsp.c
index 3c68f74..50ac134 100644
--- a/libavcodec/atrac3plusdsp.c
+++ b/libavcodec/atrac3plusdsp.c
@@ -116,6 +116,7 @@ av_cold void ff_atrac3p_init_wave_synth(void)
* @param[in]synth_param ptr to common synthesis parameters
* @param[in]waves_infoparameters for each sine wave
* @param[in]envelope envelope data for all waves in a group
+ * @param[in]fdsp ptr to floating-point DSP context
* @param[in]phase_shift flag indicates 180° phase shift
* @param[in]reg_offsetregion offset for trimming envelope data
* @param[out] out receives sythesized data
@@ -123,6 +124,7 @@ av_cold void ff_atrac3p_init_wave_synth(void)
static void waves_synth(Atrac3pWaveSynthParams *synth_param,
Atrac3pWavesData *waves_info,
Atrac3pWaveEnvelope *envelope,
+AVFloatDSPContext *fdsp,
int phase_shift, int reg_offset, float *out)
{
int i, wn, inc, pos;
@@ -146,6 +148,10 @@ static void waves_synth(Atrac3pWaveSynthParams *synth_param,
}
}
+/* 180° phase shift if requested */
+if (phase_shift)
+fdsp->vector_fmul_scalar(out, out, -1.0f, 128);
+
/* fade in with steep Hann window if requested */
if (envelope->has_start_point) {
pos = (envelope->start_pos << 2) - reg_offset;
@@ -216,11 +222,11 @@ void ff_atrac3p_generate_tones(Atrac3pChanUnitCtx *ch_unit, AVFloatDSPContext *f
/* synthesize waves for both overlapping regions */
if (tones_now->num_wavs && reg1_env_nonzero)
waves_synth(ch_unit->waves_info_prev, tones_now, &tones_now->curr_env,
-ch_unit->waves_info_prev->phase_shift[sb] & ch_num,
+fdsp, ch_unit->waves_info_prev->phase_shift[sb] & ch_num,
128, wavreg1);
if (tones_next->num_wavs && reg2_env_nonzero)
-waves_synth(ch_unit->waves_info, tones_next, &tones_next->curr_env,
+waves_synth(ch_unit->waves_info, tones_next, &tones_next->curr_env, fdsp,
ch_unit->waves_info->phase_shift[sb] & ch_num, 0, wavreg2);
/* Hann windowing for non-faded wave signals */
--
1.9.1
From 3e0a8c23c5752db78950b0c868fce9f955e789ae Mon Sep 17 00:00:00 2001
From: Max Poliakovski
Date: Sun, 7 Jun 2015 12:52:06 +0200
Subject: [PATCH 2/2] atrac3plus: give the phase_shift flag a better name.
---
libavcodec/atrac3plus.c| 2 +-
libavcodec/atrac3plus.h| 2 +-
libavcodec/atrac3plusdsp.c | 12 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/libavcodec/atrac3plus.c b/libavcodec/atrac3plus.c
index 3d75749..b16a139 100644
--- a/libavcodec/atrac3plus.c
+++ b/libavcodec/atrac3plus.c
@@ -1724,7 +1724,7 @@ static int decode_tones_info(GetBitContext *gb, Atrac3pChanUnitCtx *ctx,
if (num_channels == 2) {
get_subband_flags(gb, ctx->waves_info->tone_sharing, ctx->waves_info->num_tone_bands);
get_subband_flags(gb, ctx->waves_info->tone_master, ctx->waves_info->num_tone_bands);
-get_subband_flags(gb, ctx->waves_info->phase_shift, ctx->waves_info->num_tone_bands);
+get_sub
[FFmpeg-devel] [PATCH] avformat/aiffdec: avoid double and ldexp()
There is no support for non integer sample rates, using doubles/floats currently could only lead to rounding differences between platforms Signed-off-by: Michael Niedermayer --- libavformat/aiffdec.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c index ff04c2b..729e45b 100644 --- a/libavformat/aiffdec.c +++ b/libavformat/aiffdec.c @@ -99,7 +99,7 @@ static unsigned int get_aiff_header(AVFormatContext *s, int size, AIFFInputContext *aiff = s->priv_data; int exp; uint64_t val; -double sample_rate; +int sample_rate; unsigned int num_frames; if (size & 1) @@ -109,9 +109,12 @@ static unsigned int get_aiff_header(AVFormatContext *s, int size, num_frames = avio_rb32(pb); codec->bits_per_coded_sample = avio_rb16(pb); -exp = avio_rb16(pb); +exp = avio_rb16(pb) - 16383 - 63; val = avio_rb64(pb); -sample_rate = ldexp(val, exp - 16383 - 63); +if (exp >= 0) +sample_rate = val << exp; +else +sample_rate = (val + (1<<(-exp/2))) >> -exp; codec->sample_rate = sample_rate; size -= 18; -- 1.7.9.5 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi, On Sun, Jun 7, 2015 at 4:39 PM, Michael Niedermayer wrote: > On Sun, Jun 07, 2015 at 04:27:42PM -0400, Ronald S. Bultje wrote: > > Hi, > > > > On Sun, Jun 7, 2015 at 2:35 PM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > > > > > Hi Ronald, > > > > > > On 07.06.2015 19:37, Ronald S. Bultje wrote: > > > > On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun < > > > > andreas.cadhal...@googlemail.com> wrote: > > > >> On 07.06.2015 17:52, Ronald S. Bultje wrote: > > > >>> We can't simply claim that 8k is not a valid dimension, that would > > > make us > > > >>> a vp8-incompatible decoder if vp8 stream dimensions are allowed to > be > > > >>> 8k. > > > >>> This could particularly happen in images (webp). > > > >> > > > >> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, > so > > > feel > > > >> free to fix this limitation. ;) > > > > > > > > > > > > No, no, not so easy. For example, keyframes (like webp) would decode > just > > > > fine even thougb mvmin/max is broken, so we're causing a regression > here > > > of > > > > something that worked fine before. > > > > > > Hmm, so maybe the check could be moved to decode_mb_row_no_filter? > > > > > > > Is this an actual bug? What is the behaviour that you're trying to > fix? > > > Is > > > > this some overflow noticed on some generated/fuzzed bitstream with > > > > -fsanitize=integer? Or are we decoding a sample differently from > libvpx? > > > Or > > > > something else? > > > > > > The overflow of mv_max can cause it to be smaller than mv_min, which > causes > > > av_clip to abort for ASSERT_LEVEL >= 2. > > > > > > So what happens when you change mv_max/min to be integers (instead of > > int16_t) without further touching VP56mv, e.g. by making mv_max/min a > > VP8intminmaxmv (a newly created type just for this purpose, using int > > instead of int16_t)? > > would this work with > clamp_mv() ? > it limits based in mv_max/min but writes into a VP56mv, so if these > out of 16bit limits can actually be reached then the output of > the clip would not fit in the 16bit destination ... > but maybe this doesnt occur, ive not deeply investigated Neither have I, I just don't like the sledgehammer approach of disabling all vp8 decoding of >8k framesizes for something semi-obscure like this. I think your approach of clamping min/max to int16_t range sounds most appropriate. I'd also suggest to see what libvpx does (if anything) to make sure we decode affected content the same. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
On Sun, Jun 07, 2015 at 04:27:42PM -0400, Ronald S. Bultje wrote: > Hi, > > On Sun, Jun 7, 2015 at 2:35 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: > > > Hi Ronald, > > > > On 07.06.2015 19:37, Ronald S. Bultje wrote: > > > On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun < > > > andreas.cadhal...@googlemail.com> wrote: > > >> On 07.06.2015 17:52, Ronald S. Bultje wrote: > > >>> We can't simply claim that 8k is not a valid dimension, that would > > make us > > >>> a vp8-incompatible decoder if vp8 stream dimensions are allowed to be > > >>> 8k. > > >>> This could particularly happen in images (webp). > > >> > > >> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so > > feel > > >> free to fix this limitation. ;) > > > > > > > > > No, no, not so easy. For example, keyframes (like webp) would decode just > > > fine even thougb mvmin/max is broken, so we're causing a regression here > > of > > > something that worked fine before. > > > > Hmm, so maybe the check could be moved to decode_mb_row_no_filter? > > > > > Is this an actual bug? What is the behaviour that you're trying to fix? > > Is > > > this some overflow noticed on some generated/fuzzed bitstream with > > > -fsanitize=integer? Or are we decoding a sample differently from libvpx? > > Or > > > something else? > > > > The overflow of mv_max can cause it to be smaller than mv_min, which causes > > av_clip to abort for ASSERT_LEVEL >= 2. > > > So what happens when you change mv_max/min to be integers (instead of > int16_t) without further touching VP56mv, e.g. by making mv_max/min a > VP8intminmaxmv (a newly created type just for this purpose, using int > instead of int16_t)? would this work with clamp_mv() ? it limits based in mv_max/min but writes into a VP56mv, so if these out of 16bit limits can actually be reached then the output of the clip would not fit in the 16bit destination ... but maybe this doesnt occur, ive not deeply investigated [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp9: change type of tile_size from unsigned to int64_t
Hi,
On Sun, Jun 7, 2015 at 1:02 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> Otherwise the check 'tile_size < size' treats a negative size as
> unsigned, causing the check to pass. This subsequently leads to
> segmentation faults.
>
> This was originally fixed as part of Libav commit 72ca83, so the
> original author is one of the following developers:
> Anton Khirnov
> Diego Biurrun
> Luca Barbato
> Martin Storsjö
>
> Signed-off-by: Andreas Cadhalpun
> ---
>
> Does someone still remember who authored this particular change,
> so that he can get proper attribution?
>
> ---
> libavcodec/vp9.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index c4efd42..d5147e5 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -4106,7 +4106,7 @@ static int vp9_decode_frame(AVCodecContext *ctx,
> void *frame,
> tile_row, s->tiling.log2_tile_rows,
> s->sb_rows);
> if (s->pass != 2) {
> for (tile_col = 0; tile_col < s->tiling.tile_cols;
> tile_col++) {
> -unsigned tile_size;
> +int64_t tile_size;
Hm... OK.
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi, On Sun, Jun 7, 2015 at 2:35 PM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > Hi Ronald, > > On 07.06.2015 19:37, Ronald S. Bultje wrote: > > On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > >> On 07.06.2015 17:52, Ronald S. Bultje wrote: > >>> We can't simply claim that 8k is not a valid dimension, that would > make us > >>> a vp8-incompatible decoder if vp8 stream dimensions are allowed to be > >>> 8k. > >>> This could particularly happen in images (webp). > >> > >> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so > feel > >> free to fix this limitation. ;) > > > > > > No, no, not so easy. For example, keyframes (like webp) would decode just > > fine even thougb mvmin/max is broken, so we're causing a regression here > of > > something that worked fine before. > > Hmm, so maybe the check could be moved to decode_mb_row_no_filter? > > > Is this an actual bug? What is the behaviour that you're trying to fix? > Is > > this some overflow noticed on some generated/fuzzed bitstream with > > -fsanitize=integer? Or are we decoding a sample differently from libvpx? > Or > > something else? > > The overflow of mv_max can cause it to be smaller than mv_min, which causes > av_clip to abort for ASSERT_LEVEL >= 2. So what happens when you change mv_max/min to be integers (instead of int16_t) without further touching VP56mv, e.g. by making mv_max/min a VP8intminmaxmv (a newly created type just for this purpose, using int instead of int16_t)? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp9: change type of tile_size from unsigned to int64_t
On 07/06/15 19:02, Andreas Cadhalpun wrote: > Otherwise the check 'tile_size < size' treats a negative size as > unsigned, causing the check to pass. This subsequently leads to > segmentation faults. > > This was originally fixed as part of Libav commit 72ca83, so the > original author is one of the following developers: > Anton Khirnov > Diego Biurrun > Luca Barbato > Martin Storsjö > > Signed-off-by: Andreas Cadhalpun > --- > > Does someone still remember who authored this particular change, > so that he can get proper attribution? I'm not sure about the others but it is enough that you mention the people involved and use your name for the commit to me. lu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi Ronald, On 07.06.2015 19:37, Ronald S. Bultje wrote: > On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: >> On 07.06.2015 17:52, Ronald S. Bultje wrote: >>> We can't simply claim that 8k is not a valid dimension, that would make us >>> a vp8-incompatible decoder if vp8 stream dimensions are allowed to be >>> 8k. >>> This could particularly happen in images (webp). >> >> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so feel >> free to fix this limitation. ;) > > > No, no, not so easy. For example, keyframes (like webp) would decode just > fine even thougb mvmin/max is broken, so we're causing a regression here of > something that worked fine before. Hmm, so maybe the check could be moved to decode_mb_row_no_filter? > Is this an actual bug? What is the behaviour that you're trying to fix? Is > this some overflow noticed on some generated/fuzzed bitstream with > -fsanitize=integer? Or are we decoding a sample differently from libvpx? Or > something else? The overflow of mv_max can cause it to be smaller than mv_min, which causes av_clip to abort for ASSERT_LEVEL >= 2. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/6] avcodec/apng: Remove threading support
Fixed so threading isn't removed completely.
Sorry this took so long. Took too long to figure out that decode_ihdr_chunk()
was being called in each thread and thus overwriting the cur_w/h value set by
update_thread_context().
("Remove threading support" patch can be ignored now)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/6] avcodec/apng: Dispose previous frame properly
The spec specifies the dispose operation as how the current (i.e., currently
being rendered) frame should be disposed when the next frame is blended onto it
This is contrary to ffmpeg's current behaviour of interpreting the dispose
operation as how the previous (i.e., already rendered) frame should be disposed
This patch fixes ffmpeg's behaviour to match those of the spec, which involved
a rewrite of the blending function
Signed-off-by: Donny Yang
---
libavcodec/pngdec.c | 179 +++-
1 file changed, 92 insertions(+), 87 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 5fa428b..a70a7f5 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -45,8 +45,11 @@ typedef struct PNGDecContext {
int state;
int width, height;
int cur_w, cur_h;
+int last_w, last_h;
int x_offset, y_offset;
+int last_x_offset, last_y_offset;
uint8_t dispose_op, blend_op;
+uint8_t last_dispose_op;
int bit_depth;
int color_type;
int compression_type;
@@ -56,7 +59,6 @@ typedef struct PNGDecContext {
int bits_per_pixel;
int bpp;
-int frame_id;
uint8_t *image_buf;
int image_linesize;
uint32_t palette[256];
@@ -539,13 +541,18 @@ static int decode_ihdr_chunk(AVCodecContext *avctx,
PNGDecContext *s,
return AVERROR_INVALIDDATA;
}
-s->width = s->cur_w = bytestream2_get_be32(&s->gb);
-s->height = s->cur_h = bytestream2_get_be32(&s->gb);
+s->width = bytestream2_get_be32(&s->gb);
+s->height = bytestream2_get_be32(&s->gb);
if (av_image_check_size(s->width, s->height, 0, avctx)) {
s->width = s->height = 0;
av_log(avctx, AV_LOG_ERROR, "Invalid image size\n");
return AVERROR_INVALIDDATA;
}
+if (s->cur_w == 0 && s->cur_h == 0) {
+// Only set cur_w/h if update_thread_context() has not set it
+s->cur_w = s->width;
+s->cur_h = s->height;
+}
s->bit_depth= bytestream2_get_byte(&s->gb);
s->color_type = bytestream2_get_byte(&s->gb);
s->compression_type = bytestream2_get_byte(&s->gb);
@@ -809,6 +816,12 @@ static int decode_fctl_chunk(AVCodecContext *avctx,
PNGDecContext *s,
if (length != 26)
return AVERROR_INVALIDDATA;
+s->last_w = s->cur_w;
+s->last_h = s->cur_h;
+s->last_x_offset = s->x_offset;
+s->last_y_offset = s->y_offset;
+s->last_dispose_op = s->dispose_op;
+
sequence_number = bytestream2_get_be32(&s->gb);
s->cur_w= bytestream2_get_be32(&s->gb);
s->cur_h= bytestream2_get_be32(&s->gb);
@@ -829,15 +842,10 @@ static int decode_fctl_chunk(AVCodecContext *avctx,
PNGDecContext *s,
s->cur_w > s->width - s->x_offset|| s->cur_h > s->height - s->y_offset)
return AVERROR_INVALIDDATA;
-/* always (re)start with a clean frame */
-if (sequence_number == 0) {
+if (sequence_number == 0 && s->dispose_op == APNG_DISPOSE_OP_PREVIOUS) {
+// No previous frame to revert to for the first frame
+// Spec says to just treat it as a APNG_DISPOSE_OP_BACKGROUND
s->dispose_op = APNG_DISPOSE_OP_BACKGROUND;
-s->frame_id = 0;
-} else {
-s->frame_id++;
-if (s->frame_id == 1 && s->dispose_op == APNG_DISPOSE_OP_PREVIOUS)
-/* previous for the second frame is the first frame */
-s->dispose_op = APNG_DISPOSE_OP_NONE;
}
return 0;
@@ -866,15 +874,11 @@ static void handle_p_frame_png(PNGDecContext *s, AVFrame
*p)
static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
AVFrame *p)
{
-int i, j;
-uint8_t *pd = p->data[0];
-uint8_t *pd_last = s->last_picture.f->data[0];
-uint8_t *pd_last_region = s->dispose_op == APNG_DISPOSE_OP_PREVIOUS ?
-s->previous_picture.f->data[0] :
s->last_picture.f->data[0];
-int ls = FFMIN(av_image_get_linesize(p->format, s->width, 0), s->width *
s->bpp);
+size_t x, y;
+uint8_t *buffer = av_malloc(s->image_linesize * s->height);
-if (ls < 0)
-return ls;
+if (!buffer)
+return AVERROR(ENOMEM);
if (s->blend_op == APNG_BLEND_OP_OVER &&
avctx->pix_fmt != AV_PIX_FMT_RGBA) {
@@ -883,76 +887,76 @@ static int handle_p_frame_apng(AVCodecContext *avctx,
PNGDecContext *s,
return AVERROR_PATCHWELCOME;
}
+// Copy the previous frame to the buffer
ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
-if (s->dispose_op == APNG_DISPOSE_OP_PREVIOUS)
-ff_thread_await_progress(&s->previous_picture, INT_MAX, 0);
+memcpy(buffer, s->last_picture.f->data[0], s->image_linesize * s->height);
-for (j = 0; j < s->y_offset; j++) {
-memcpy(pd, pd_last, ls);
-pd += s->image_linesize;
-pd_last += s->image_linesize;
+// Do the disposal operation specified by the last frame on the fra
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi,
On Sun, Jun 7, 2015 at 12:44 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> Hi Ronald,
>
> On 07.06.2015 17:52, Ronald S. Bultje wrote:
> > On Sun, Jun 7, 2015 at 10:05 AM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >> +#define MARGIN (16 << 2)
> >> +#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
> >>
> >
> > So this is roughly 9 bits.
> >
> >
> >> +if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
> >> +s->avctx->coded_height > MAX_MB_SIZE * 16) {
> >> +av_log(s->avctx, AV_LOG_ERROR, "too large dimensions
> %dx%d\n",
> >> + s->avctx->coded_width, s->avctx->coded_height);
> >> +return AVERROR_INVALIDDATA;
> >> +}
> >
> >
> > And this makes it 13, so we have a max w/h of around ~8k.
>
> That is 8176 and thus mb_width/mb_height can maximally be 511.
>
> > That's not very
> > big. Does that conform to any code in libvpx? Is it possible mv_max.x/y
> > should be 32bit instead?
>
> The type of mv_max is VP56mv, which is used in quite many places, so
> I'm not overly confident in changing that.
> And it seems libvpx uses short [1], which should be equivalent to int16_t.
>
> > We can't simply claim that 8k is not a valid dimension, that would make
> us
> > a vp8-incompatible decoder if vp8 stream dimensions are allowed to be
> >8k.
> > This could particularly happen in images (webp).
>
> I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so feel
> free to fix this limitation. ;)
No, no, not so easy. For example, keyframes (like webp) would decode just
fine even thougb mvmin/max is broken, so we're causing a regression here of
something that worked fine before.
Is this an actual bug? What is the behaviour that you're trying to fix? Is
this some overflow noticed on some generated/fuzzed bitstream with
-fsanitize=integer? Or are we decoding a sample differently from libvpx? Or
something else?
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] vp9: change type of tile_size from unsigned to int64_t
Otherwise the check 'tile_size < size' treats a negative size as
unsigned, causing the check to pass. This subsequently leads to
segmentation faults.
This was originally fixed as part of Libav commit 72ca83, so the
original author is one of the following developers:
Anton Khirnov
Diego Biurrun
Luca Barbato
Martin Storsjö
Signed-off-by: Andreas Cadhalpun
---
Does someone still remember who authored this particular change,
so that he can get proper attribution?
---
libavcodec/vp9.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index c4efd42..d5147e5 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -4106,7 +4106,7 @@ static int vp9_decode_frame(AVCodecContext *ctx, void
*frame,
tile_row, s->tiling.log2_tile_rows, s->sb_rows);
if (s->pass != 2) {
for (tile_col = 0; tile_col < s->tiling.tile_cols; tile_col++)
{
-unsigned tile_size;
+int64_t tile_size;
if (tile_col == s->tiling.tile_cols - 1 &&
tile_row == s->tiling.tile_rows - 1) {
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp8: check for too large dimensions
On 07.06.2015 17:38, Michael Niedermayer wrote:
> On Sun, Jun 07, 2015 at 04:05:37PM +0200, Andreas Cadhalpun wrote:
>> If the dimensions are too large, s->mb_width or s->mb_height can become
>> too large, leading to an int16_t overflow of s->mv_max.{x,y}.
>>
>> Signed-off-by: Andreas Cadhalpun
>> ---
>> libavcodec/vp8.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>
>
>>
>> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
>> index dbba568..69cf138 100644
>> --- a/libavcodec/vp8.c
>> +++ b/libavcodec/vp8.c
>> @@ -145,6 +145,8 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
>> return frame;
>> }
>>
>> +#define MARGIN (16 << 2)
>> +#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
>> static av_always_inline
>> int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
>> {
>> @@ -160,6 +162,13 @@ int update_dimensions(VP8Context *s, int width, int
>> height, int is_vp7)
>> return ret;
>> }
>>
>> +if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
>> +s->avctx->coded_height > MAX_MB_SIZE * 16) {
>> +av_log(s->avctx, AV_LOG_ERROR, "too large dimensions %dx%d\n",
>> + s->avctx->coded_width, s->avctx->coded_height);
>> +return AVERROR_INVALIDDATA;
>
> iam not sure this should be AVERROR_INVALIDDATA or PATCHWELCOME
>
> but this patch or clamping the MV min/max to the 16bit range to avoid
> overflows should be ok.
AVERROR_PATCHWELCOME seems better. Updated patch attached.
Best regards,
Andreas
>From affc6e50db56dbdb5a7b45e9ddca2a4d361df3b4 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Sun, 7 Jun 2015 16:01:20 +0200
Subject: [PATCH] vp8: check for too large dimensions
If the dimensions are too large, s->mb_width or s->mb_height can become
too large, leading to an int16_t overflow of s->mv_max.{x,y}.
Signed-off-by: Andreas Cadhalpun
---
libavcodec/vp8.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index dbba568..98ec72d 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -145,6 +145,8 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
return frame;
}
+#define MARGIN (16 << 2)
+#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
static av_always_inline
int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
{
@@ -160,6 +162,14 @@ int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
return ret;
}
+if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
+s->avctx->coded_height > MAX_MB_SIZE * 16) {
+av_log(s->avctx, AV_LOG_ERROR,
+ "large dimensions %dx%d not supported\n",
+ s->avctx->coded_width, s->avctx->coded_height);
+return AVERROR_PATCHWELCOME;
+}
+
s->mb_width = (s->avctx->coded_width + 15) / 16;
s->mb_height = (s->avctx->coded_height + 15) / 16;
@@ -2177,7 +2187,6 @@ void filter_mb_simple(VP8Context *s, uint8_t *dst, VP8FilterStrength *f,
}
}
-#define MARGIN (16 << 2)
static av_always_inline
void vp78_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *curframe,
VP8Frame *prev_frame, int is_vp7)
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi Ronald,
On 07.06.2015 17:52, Ronald S. Bultje wrote:
> On Sun, Jun 7, 2015 at 10:05 AM, Andreas Cadhalpun <
> andreas.cadhal...@googlemail.com> wrote:
>
>> +#define MARGIN (16 << 2)
>> +#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
>>
>
> So this is roughly 9 bits.
>
>
>> +if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
>> +s->avctx->coded_height > MAX_MB_SIZE * 16) {
>> +av_log(s->avctx, AV_LOG_ERROR, "too large dimensions %dx%d\n",
>> + s->avctx->coded_width, s->avctx->coded_height);
>> +return AVERROR_INVALIDDATA;
>> +}
>
>
> And this makes it 13, so we have a max w/h of around ~8k.
That is 8176 and thus mb_width/mb_height can maximally be 511.
> That's not very
> big. Does that conform to any code in libvpx? Is it possible mv_max.x/y
> should be 32bit instead?
The type of mv_max is VP56mv, which is used in quite many places, so
I'm not overly confident in changing that.
And it seems libvpx uses short [1], which should be equivalent to int16_t.
> We can't simply claim that 8k is not a valid dimension, that would make us
> a vp8-incompatible decoder if vp8 stream dimensions are allowed to be >8k.
> This could particularly happen in images (webp).
I'd change the error to AVERROR_PATCHWELCOME as Michael suggested, so feel
free to fix this limitation. ;)
Best regards,
Andreas
1: https://sources.debian.net/src/libvpx/1.3.0-3/vp8/common/mv.h/?hl=16#L16
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 2.7
Helmut K. C. Tessarek evermeet.cx> writes: > To honor the beautiful mind who made game theory a > whole different area, what about: > > John F. Nash +1 Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [libav-devel] [PATCH] vp8: check for too large dimensions
Hi,
On Sun, Jun 7, 2015 at 10:05 AM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> +#define MARGIN (16 << 2)
> +#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
>
So this is roughly 9 bits.
> +if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
> +s->avctx->coded_height > MAX_MB_SIZE * 16) {
> +av_log(s->avctx, AV_LOG_ERROR, "too large dimensions %dx%d\n",
> + s->avctx->coded_width, s->avctx->coded_height);
> +return AVERROR_INVALIDDATA;
> +}
And this makes it 13, so we have a max w/h of around ~8k. That's not very
big. Does that conform to any code in libvpx? Is it possible mv_max.x/y
should be 32bit instead?
We can't simply claim that 8k is not a valid dimension, that would make us
a vp8-incompatible decoder if vp8 stream dimensions are allowed to be >8k.
This could particularly happen in images (webp).
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp8: check for too large dimensions
On Sun, Jun 07, 2015 at 04:05:37PM +0200, Andreas Cadhalpun wrote:
> If the dimensions are too large, s->mb_width or s->mb_height can become
> too large, leading to an int16_t overflow of s->mv_max.{x,y}.
>
> Signed-off-by: Andreas Cadhalpun
> ---
> libavcodec/vp8.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index dbba568..69cf138 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -145,6 +145,8 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
> return frame;
> }
>
> +#define MARGIN (16 << 2)
> +#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
> static av_always_inline
> int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
> {
> @@ -160,6 +162,13 @@ int update_dimensions(VP8Context *s, int width, int
> height, int is_vp7)
> return ret;
> }
>
> +if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
> +s->avctx->coded_height > MAX_MB_SIZE * 16) {
> +av_log(s->avctx, AV_LOG_ERROR, "too large dimensions %dx%d\n",
> + s->avctx->coded_width, s->avctx->coded_height);
> +return AVERROR_INVALIDDATA;
iam not sure this should be AVERROR_INVALIDDATA or PATCHWELCOME
but this patch or clamping the MV min/max to the 16bit range to avoid
overflows should be ok.
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] vp8: check for too large dimensions
If the dimensions are too large, s->mb_width or s->mb_height can become
too large, leading to an int16_t overflow of s->mv_max.{x,y}.
Signed-off-by: Andreas Cadhalpun
---
libavcodec/vp8.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index dbba568..69cf138 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -145,6 +145,8 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
return frame;
}
+#define MARGIN (16 << 2)
+#define MAX_MB_SIZE (((INT16_MAX - MARGIN) >> 6) + 1)
static av_always_inline
int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
{
@@ -160,6 +162,13 @@ int update_dimensions(VP8Context *s, int width, int
height, int is_vp7)
return ret;
}
+if (s->avctx->coded_width > MAX_MB_SIZE * 16 ||
+s->avctx->coded_height > MAX_MB_SIZE * 16) {
+av_log(s->avctx, AV_LOG_ERROR, "too large dimensions %dx%d\n",
+ s->avctx->coded_width, s->avctx->coded_height);
+return AVERROR_INVALIDDATA;
+}
+
s->mb_width = (s->avctx->coded_width + 15) / 16;
s->mb_height = (s->avctx->coded_height + 15) / 16;
@@ -2177,7 +2186,6 @@ void filter_mb_simple(VP8Context *s, uint8_t *dst,
VP8FilterStrength *f,
}
}
-#define MARGIN (16 << 2)
static av_always_inline
void vp78_decode_mv_mb_modes(AVCodecContext *avctx, VP8Frame *curframe,
VP8Frame *prev_frame, int is_vp7)
--
2.1.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffserver: Check allocations (likely not all)
On Sat, Jun 06, 2015 at 10:59:30PM -0300, Reynaldo H. Verdejo Pinochet wrote: > Hi > > On 06/06/2015 10:43 AM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > ffserver.c | 20 > > 1 file changed, 20 insertions(+) > > > > [..] > > Looks good. Thanks! applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
