[FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction
Fixes an assertion error reported in #2686 that happens when
using prediction (either explicitly or implicitly by setting
the AAC main profile), since prediction code would allow
creating new zeroes or removing existing ones, without
properly checking for SF delta violations.
This patch forbids creating/removing zeroes, perhaps an
overly conservative approach, but a safe one. More permissive
and sophisticated approaches may be attempted in the future.
Attached
From dbda62b2a59054a76e317850a4a0a7037ca3cc02 Mon Sep 17 00:00:00 2001
From: Claudio Freire
Date: Tue, 29 Dec 2015 05:18:40 -0300
Subject: [PATCH] AAC encoder: fix assertion error with prediction
Fixes an assertion error reported in #2686 that happens when
using prediction (either explicitly or implicitly by setting
the AAC main profile), since prediction code would allow
creating new zeroes or removing existing ones, without
properly checking for SF delta violations.
This patch forbids creating/removing zeroes, perhaps an
overly conservative approach, but a safe one. More permissive
and sophisticated approaches may be attempted in the future.
---
libavcodec/aacenc_pred.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/libavcodec/aacenc_pred.c b/libavcodec/aacenc_pred.c
index d76a575..e77a3de 100644
--- a/libavcodec/aacenc_pred.c
+++ b/libavcodec/aacenc_pred.c
@@ -257,7 +257,9 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
for (sfb = PRED_SFB_START; sfb < pmax; sfb++) {
int cost1, cost2, cb_p;
float dist1, dist2, dist_spec_err = 0.0f;
-const int cb_n = sce->band_type[sfb];
+const int cb_n = sce->zeroes[sfb] ? 0 : sce->band_type[sfb];
+const int cb_min = sce->zeroes[sfb] ? 0 : 1;
+const int cb_max = sce->zeroes[sfb] ? 0 : RESERVED_BT;
const int start_coef = sce->ics.swb_offset[sfb];
const int num_coeffs = sce->ics.swb_offset[sfb + 1] - start_coef;
const FFPsyBand *band = &s->psy.ch[s->cur_channel].psy_bands[sfb];
@@ -279,7 +281,7 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
SENT[i] = sce->coeffs[start_coef + i] - sce->prcoeffs[start_coef + i];
abs_pow34_v(S34, SENT, num_coeffs);
if (cb_n < RESERVED_BT)
-cb_p = find_min_book(find_max_val(1, num_coeffs, S34), sce->sf_idx[sfb]);
+cb_p = av_clip(find_min_book(find_max_val(1, num_coeffs, S34), sce->sf_idx[sfb]), cb_min, cb_max);
else
cb_p = cb_n;
quantize_and_encode_band_cost(s, NULL, SENT, QERR, S34, num_coeffs,
@@ -291,7 +293,7 @@ void ff_aac_search_for_pred(AACEncContext *s, SingleChannelElement *sce)
sce->prcoeffs[start_coef + i] += QERR[i] != 0.0f ? (sce->prcoeffs[start_coef + i] - QERR[i]) : 0.0f;
abs_pow34_v(P34, &sce->prcoeffs[start_coef], num_coeffs);
if (cb_n < RESERVED_BT)
-cb_p = find_min_book(find_max_val(1, num_coeffs, P34), sce->sf_idx[sfb]);
+cb_p = av_clip(find_min_book(find_max_val(1, num_coeffs, P34), sce->sf_idx[sfb]), cb_min, cb_max);
else
cb_p = cb_n;
dist2 = quantize_and_encode_band_cost(s, NULL, &sce->prcoeffs[start_coef], NULL,
--
1.8.4.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction
Wouldn't it be simpler to just check if the maximum codebook was 0 after calculating cost1 and skipping the rest of the code in the loop? On 29 December 2015 at 08:23, Claudio Freire wrote: > Fixes an assertion error reported in #2686 that happens when > using prediction (either explicitly or implicitly by setting > the AAC main profile), since prediction code would allow > creating new zeroes or removing existing ones, without > properly checking for SF delta violations. > > This patch forbids creating/removing zeroes, perhaps an > overly conservative approach, but a safe one. More permissive > and sophisticated approaches may be attempted in the future. > > Attached > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec: Use get_ue_golomb_long() when needed
On Mon, Dec 28, 2015 at 11:04:08PM -0800, Mark Harris wrote: > get_ue_golomb() cannot decode values larger than 8190 (the maximum > value that can be golomb encoded in 25 bits) and produces the error > "Invalid UE golomb code" if a larger value is encountered. Use > get_ue_golomb_long() instead (which supports 63 bits, up to 4294967294) > when valid h264/hevc values can exceed 8190. > > This updates decoding of the following values: (maximum) > first_mb_in_slice36863* for level 5.2 > abs_diff_pic_num_minus1 131071 > difference_of_pic_nums_minus1 131071 > idr_pic_id 65535 > recovery_frame_cnt 65535 > frame_packing_arrangement_id4294967294 > frame_packing_arrangement_repetition_period 16384 > display_orientation_repetition_period16384 > > An alternative would be to modify get_ue_golomb() to handle encoded > values of up to 49 bits as was done for get_se_golomb() in a92816c. > In that case get_ue_golomb() could continue to be used for all of > these except frame_packing_arrangement_id. > --- > libavcodec/golomb.h | 2 +- > libavcodec/h264.c| 2 +- > libavcodec/h264_parser.c | 6 +++--- > libavcodec/h264_refs.c | 4 ++-- > libavcodec/h264_sei.c| 10 +- > libavcodec/h264_slice.c | 2 +- > libavcodec/hevc_sei.c| 2 +- > 7 files changed, 14 insertions(+), 14 deletions(-) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway 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/6] ffprobe: display loop points when applicable
On date Tuesday 2015-06-16 17:28:17 -0500, Rodger Combs encoded:
> ---
> ffprobe.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/ffprobe.c b/ffprobe.c
> index 3e5324e..d54eb87 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -2436,6 +2436,10 @@ static int show_format(WriterContext *w,
> AVFormatContext *fmt_ctx)
> }
> print_time("start_time", fmt_ctx->start_time, &AV_TIME_BASE_Q);
> print_time("duration",fmt_ctx->duration, &AV_TIME_BASE_Q);
> +if (fmt_ctx->loop_start != AV_NOPTS_VALUE)
> +print_time("loop_start", fmt_ctx->loop_start, &AV_TIME_BASE_Q);
> +if (fmt_ctx->loop_end != AV_NOPTS_VALUE)
> +print_time("loop_end", fmt_ctx->loop_end, &AV_TIME_BASE_Q);
Missing changes in doc/ffprobe.xsd.
[...]
--
FFmpeg = Freak & Fundamental Multipurpose Puristic Empowered Ghost
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Tue, Dec 29, 2015 at 07:38:04AM +0100, Mats Peterson wrote: > I forgot some stuff. Patch description follows: > > This patch for qtpalette.c and qtpalette.h adds 1-bit video to the > "palettized video" category, since if the video sample description > contains a palette, the two colors in the palette can be any color, not > necessarily black & white. > > Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder > blindly assumes that 1-bit video is black & white. I don't have enough > knowledge about the decoder to fix this, though. > > Below is a link to a sample 1-bit QuickTime Animation clip of a rotating > earth that uses blueish colors, and they will be correctly rendered in > QuickTime, but not in FFmpeg (which will use black & white). > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE > > Mats > > -- > Mats Peterson > http://matsp888.no-ip.org/~mats/ > qtpalette.c |8 +--- > qtpalette.h |5 + > 2 files changed, 10 insertions(+), 3 deletions(-) > 6aff88fb703e1d4e0a3412816d376ee0c02be605 > 0001-lavf-qtpalette-Treat-1-bit-video-as-palettized.patch > From 64dbe9e113d5ba3fc03951711ffb6c51b008 Mon Sep 17 00:00:00 2001 > From: Mats Peterson > Date: Tue, 29 Dec 2015 07:35:00 +0100 > Subject: [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized > > I forgot some stuff. Patch description follows: > > This patch for qtpalette.c and qtpalette.h adds 1-bit video to the > "palettized video" category, since if the video sample description > contains a palette, the two colors in the palette can be any color, not > necessarily black & white. > > Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder > blindly assumes that 1-bit video is black & white. I don't have enough > knowledge about the decoder to fix this, though. the pix_fmt would need to be changed to AV_PIX_FMT_PAL8 probably qtrle_decode_1bpp would eed to be changed so that pixels are stored one per byte instead of one per bit see qtrle_decode_2n4bpp vs. qtrle_decode_1bpp > > Below is a link to a sample 1-bit QuickTime Animation clip of a rotating > earth that uses blueish colors, and they will be correctly rendered in > QuickTime, but not in FFmpeg (which will use black & white). > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE patch applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffprobe: avoid unnecessary pow and exp2 calls
On date Wednesday 2015-12-23 09:56:17 -0800, Ganesh Ajjanagadde encoded:
> These are just for prefixes and may be hardcoded easily; see lavu/eval
> for this approach.
>
> Signed-off-by: Ganesh Ajjanagadde
> ---
> ffprobe.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/ffprobe.c b/ffprobe.c
> index 0b672ff..c352b44 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -218,8 +218,19 @@ static AVInputFormat *iformat = NULL;
>
> static struct AVHashContext *hash;
>
> -static const char *const binary_unit_prefixes [] = { "", "Ki", "Mi", "Gi",
> "Ti", "Pi" };
> -static const char *const decimal_unit_prefixes[] = { "", "K" , "M" , "G" ,
> "T" , "P" };
> +static const struct {
> +double bin_val;
> +double dec_val;
> +const char *bin_str;
> +const char *dec_str;
> +} si_prefixes[] = {
> +{ 1.0, 1.0, "", "" },
> +{ 1.024e3, 1e3, "Ki", "K" },
> +{ 1.048576e6, 1e6, "Mi", "M" },
> +{ 1.073741824e9, 1e9, "Gi", "G" },
> +{ 1.099511627776e12, 1e12, "Ti", "T" },
> +{ 1.125899906842624e15, 1e15, "Pi", "P" },
> +};
>
> static const char unit_second_str[] = "s";
> static const char unit_hertz_str[] = "Hz" ;
> @@ -273,14 +284,14 @@ static char *value_string(char *buf, int buf_size,
> struct unit_value uv)
>
> if (uv.unit == unit_byte_str && use_byte_value_binary_prefix) {
> index = (long long int) (log2(vald)) / 10;
> -index = av_clip(index, 0,
> FF_ARRAY_ELEMS(binary_unit_prefixes) - 1);
> -vald /= exp2(index * 10);
> -prefix_string = binary_unit_prefixes[index];
> +index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> +vald /= si_prefixes[index].bin_val;
> +prefix_string = si_prefixes[index].bin_str;
> } else {
> index = (long long int) (log10(vald)) / 3;
> -index = av_clip(index, 0,
> FF_ARRAY_ELEMS(decimal_unit_prefixes) - 1);
> -vald /= pow(10, index * 3);
> -prefix_string = decimal_unit_prefixes[index];
> +index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> +vald /= si_prefixes[index].dec_val;
> +prefix_string = si_prefixes[index].dec_str;
> }
> vali = vald;
> }
> --
> 2.6.4
LGTM, thanks.
--
FFmpeg = Fancy & Fascinating Meaningless Puritan Eretic Guru
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On Tue, Dec 29, 2015 at 02:12:44AM +, Rostislav Pehlivanov wrote: > Hmm, kinda odd. > I've attached a v2 of the RFC Patch which does this: > 1. Fixed some typos. > 2. Fixed some missing newlines at the end of daalatab* > 3. Moves the pix_fmt finding function inside the main decoder file. > > Try building now. make distclean ; ./configure && make -k -j12 ; make -k make: Target `all' not remade because of errors. CC libavcodec/daaladec.o In file included from libavcodec/daaladec.c:35:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daaladec.o] Error 1 CC libavcodec/daaladsp.o In file included from libavcodec/daaladsp.c:30:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daaladsp.o] Error 1 CC libavcodec/daalatab.o In file included from libavcodec/daalatab.c:29:0: libavcodec/daalatab.h:48:35: error: no previous prototype for ‘daala_find_p_format’ [-Werror=missing-prototypes] cc1: some warnings being treated as errors make: *** [libavcodec/daalatab.o] Error 1 make: Target `all' not remade because of errors. gcc --version gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin 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] lavf/qtpalette: Treat 1-bit video as palettized
On 12/29/2015 11:56 AM, Michael Niedermayer wrote: On Tue, Dec 29, 2015 at 07:38:04AM +0100, Mats Peterson wrote: I forgot some stuff. Patch description follows: This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ qtpalette.c |8 +--- qtpalette.h |5 + 2 files changed, 10 insertions(+), 3 deletions(-) 6aff88fb703e1d4e0a3412816d376ee0c02be605 0001-lavf-qtpalette-Treat-1-bit-video-as-palettized.patch From 64dbe9e113d5ba3fc03951711ffb6c51b008 Mon Sep 17 00:00:00 2001 From: Mats Peterson Date: Tue, 29 Dec 2015 07:35:00 +0100 Subject: [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized I forgot some stuff. Patch description follows: This patch for qtpalette.c and qtpalette.h adds 1-bit video to the "palettized video" category, since if the video sample description contains a palette, the two colors in the palette can be any color, not necessarily black & white. Unfortunately, I've noticed that the qtrle (QuickTime Animation) decoder blindly assumes that 1-bit video is black & white. I don't have enough knowledge about the decoder to fix this, though. the pix_fmt would need to be changed to AV_PIX_FMT_PAL8 probably qtrle_decode_1bpp would eed to be changed so that pixels are stored one per byte instead of one per bit see qtrle_decode_2n4bpp vs. qtrle_decode_1bpp Below is a link to a sample 1-bit QuickTime Animation clip of a rotating earth that uses blueish colors, and they will be correctly rendered in QuickTime, but not in FFmpeg (which will use black & white). https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE patch applied thanks [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Thanks for suggestions and applied patch. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
On Mon, Dec 28, 2015 at 09:27:41PM +, Eran Kornblau wrote:
> > > +case MKTAG('e','n','c','v'):// encrypted video
> > > +case MKTAG('e','n','c','a'):// encrypted audio
> > > +id = mov_codec_id(st, format);
> > > +st->codec->codec_id = id;
> >
> > this seems missing a check for st->codec->codec_id being "unset"
> > before setting it
> >
> Will add it
>
> >
> > > +sc->format = format;
> > > +break;
> > > +
> > > +default:
> > > +av_log(c->fc, AV_LOG_WARNING,
> > > + "ignoring 'frma' atom of '%.4s', stream format is
> > > '%.4s'\n",
> > > + (char*)&format, (char*)&sc->format);
> >
> > the way these are printed would lead to different results on big and
> > little endian
> >
> Correct, however it's already done this way in a few other places in this
> file (search for %.4s)
> Do you prefer that I will print it with %c%c%c%c as in
> ff_mov_read_stsd_entries ?
whatever way you prefer
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] RTP Hole Punching from SDP file
On Mon, Dec 21, 2015 at 08:45:34AM -0500, Joel Loeshelle wrote: > I am trying to stream from a local SDP file that contains the information > necessary for streaming from an IP camera on a separate network. The IP > camera is behind a NAT router so I first need to send dummy packets to set > up the connection. I see that libavformat/rtpdec.h contains a method > “ff_rtp_send_punch_packets” to do this, but it is only utilized when > playing from an RTSP stream and not from an SDP file. > > It seems like the best place for this is at the end of sdp_read_header in > libavformat/rtsp.c after we open up the RTP streams. However, I’m not > really familiar with this code so I wanted to get your feedback on this > before submitting a patch. I see that there is a condition when doing this > in RTSP streaming to exclude Windows Media Server streams above 1. > Does this apply for SDP as well? dunno but maybe gilles (in CC) knows [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]ffserver: Cast time_t value when using it in a format string.
Reynaldo H. Verdejo Pinochet osg.samsung.com> writes: > Looks good. Patch applied. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On 12/28/2015 11:12 PM, Rostislav Pehlivanov wrote:
> diff --git a/libavcodec/daalatab.h b/libavcodec/daalatab.h
> new file mode 100644
> index 000..6a4eb34
> --- /dev/null
> +++ b/libavcodec/daalatab.h
> @@ -0,0 +1,85 @@
> +/*
> + * Daala video decoder
> + *
> + * Copyright (C) 2015 Rostislav Pehlivanov
> + *
> + * 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
> + */
> +
> +#ifndef AVCODEC_DAALATAB_H
> +#define AVCODEC_DAALATAB_H
> +
> +#include "daala.h"
> +
> +typedef uint8_t daala_u8_doublet[2];
> +
> +typedef struct DaalaBandLayout {
> +const daala_u8_doublet *const tab;
> +const int tab_size;
> +const int *layout_offset;
> +const int layout_offset_size;
> +const int *band_offset;
> +const int band_offset_size;
> +} DaalaBandLayout;
> +
> +struct DaalaPixFmts {
> +enum AVPixelFormat fmt;
> +int planes, depth, depth_mode;
> +int dec[DAALA_MAX_PLANES][2];
> +};
> +
> +extern const struct DaalaPixFmts daala_valid_formats[];
> +extern const int daala_valid_formats_num;
> +
> +const inline struct DaalaPixFmts *daala_find_p_format(enum AVPixelFormat fmt)
This should be static inline const. Otherwise the compiler expects a prototype.
> +{
> +int i;
> +for (i = 0; i < daala_valid_formats_num ; i++)
> +if (daala_valid_formats[i].fmt == fmt)
> +return &daala_valid_formats[i];
> +return NULL;
> +}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On 29.12.2015 03:12, Rostislav Pehlivanov wrote:
> Hmm, kinda odd.
> I've attached a v2 of the RFC Patch which does this:
> 1. Fixed some typos.
> 2. Fixed some missing newlines at the end of daalatab*
> 3. Moves the pix_fmt finding function inside the main decoder file.
>
> Try building now.
First, this fails building with --enable-shared, because daala_find_p_format
is not static.
Second, this decoder crashes, hangs and triggers lots of undefined behavior. :-/
Third, the decoder seems to return only the first frame, repeatedly,
instead of the following frames.
More details are interleaved in the diff below.
> diff --git a/libavcodec/daala.h b/libavcodec/daala.h
> new file mode 100644
> index 000..535e78f
> --- /dev/null
> +++ b/libavcodec/daala.h
> @@ -0,0 +1,78 @@
[...]
> +typedef struct DaalaBitstreamHeader {
> +uint8_t key_frame;
> +uint8_t bipred;
> +uint8_t ref_num;
> +uint8_t act_mask;
> +uint8_t qm;
> +uint8_t haar;
> +uint8_t golden;
> +uint8_t pvq_qm[DAALA_MAX_PLANES][DAALA_QM_SIZE];
> +} DaalaBitstreamHeader;
> +
> +typedef struct DaalaSharedContext {
> +DaalaBitstreamHeader h;
> +} DaalaSharedContext;
What is the point of having DaalaSharedContext in addition to
DaalaBitstreamHeader?
> diff --git a/libavcodec/daala_entropy.h b/libavcodec/daala_entropy.h
> new file mode 100644
> index 000..3fdcaef
> --- /dev/null
> +++ b/libavcodec/daala_entropy.h
> @@ -0,0 +1,554 @@
[...]
> +/* Updates the generic exponential probability model */
> +static av_always_inline void daalaent_exp_model_update(DaalaCDF *c, int *ex,
> int x,
> + int xs, int id, int
> integrate)
> +{
> +int i, xenc;
> +ent_rng *cdf = &c->cdf[id*c->y];
> +if (cdf[15] + c->inc > 32767) {
> +for (i = 0; i < 16; i++)
> +cdf[i] = (cdf[i] >> 1) + i + 1;
> +}
> +xenc = FFMIN(15, xs);
> +for (i = xenc; i < 16; i++)
> +cdf[i] += c->inc;
> +x = FFMIN(x, 32767);
> +*ex += ((x << 16) - *ex) >> integrate;
This subtraction can overflow.
[...]
> +/* "+derf | It was a hack for the screen coding wavelet tools." */
A rather bad hack...
> +static inline int daalaent_decode_unary(DaalaEntropy *e)
> +{
> +int rval = 0;
> +while (!daalaent_decode_bits(e, 1))
This is an infinite loop that can hang the decoder.
> +rval++;
> +return rval;
> +}
[...]
> +/* Decodes quantized coefficients from the bitsteam */
> +static inline void daalaent_decode_laplace_vector(DaalaEntropy *e, dctcoef
> *y,
> + int n, int k, dctcoef
> *curr,
> + const dctcoef *means)
> +{
> +int i, exp_q8, mean_k_q8, mean_sum_ex_q8, sum_ex = 0, kn = k, ran_delta
> = 0;
> +if (k <= 1) {
> +daalaent_decode_laplace_delta(e, y, n, k, curr, means);
> +return;
> +}
> +if (!k) {
> +curr[DAALAENT_PVQ_COUNT] = DAALAENT_PVQ_NOVAL;
> +curr[DAALAENT_PVQ_COUNT_EX] = DAALAENT_PVQ_NOVAL;
> +curr[DAALAENT_PVQ_K] = 0;
> +curr[DAALAENT_PVQ_SUM_EX] = 0;
> +memset(y, 0, n*sizeof(dctcoef));
> +return;
> +}
> +mean_k_q8 = means[DAALAENT_PVQ_K];
> +mean_sum_ex_q8 = means[DAALAENT_PVQ_SUM_EX];
> +if (mean_k_q8 < 1 << 23)
> +exp_q8 = 256*mean_k_q8/(1 + mean_sum_ex_q8);
This multiplication can overflow.
> +else
> +exp_q8 = mean_k_q8/(1 + (mean_sum_ex_q8 >> 8));
> +for (i = 0; i < n; i++) {
> +int x, ex;
> +if (!kn)
> +break;
> +if (kn <= 1 && i != n - 1) {
> +daalaent_decode_laplace_delta(e, y + i, n - i, kn, curr, means);
> +ran_delta = 1;
> +i = n;
> +break;
> +}
> +ex = (2*exp_q8*kn + (n - i))/(2*(n - i));
> +if (ex > kn*256)
> +ex = kn*256;
> +sum_ex += (2*256*kn + (n - i))/(2*(n - i));
All these multiplications involving kn can overflow.
> +if (i != n - 1)
> +x = daalaent_decode_laplace_pvq(e, ex, kn);
> +else
> +x = kn;
> +y[i] = x*daalaent_cphase(e, x);
> +kn -= abs(x);
> +}
> +memset(&y[i], 0, (n - i)*sizeof(dctcoef)); /* Zero the rest */
> +if (!ran_delta) {
> +curr[DAALAENT_PVQ_COUNT] = DAALAENT_PVQ_NOVAL;
> +curr[DAALAENT_PVQ_COUNT_EX] = DAALAENT_PVQ_NOVAL;
> +}
> +curr[DAALAENT_PVQ_K] = k - kn;
> +curr[DAALAENT_PVQ_SUM_EX] = sum_ex;
> +}
> +
> +/* Expectation value is in Q16 */
> +static inline int daalaent_decode_generic(DaalaEntropy *e, DaalaCDF *c, int
> *ex,
> + int max, int integrate)
> +{
> +int rval, lsb = 0, log_ex = daalaent_log_ex(*ex);
> +const int shift = FFMAX(0, (log_ex - 5) >> 1);
> +const int id = FFMIN(DAALAENT_MODEL_TAB - 1, log_ex);
> +const int ms = (max + (1 << shift >> 1)) >> shift;
> +int xs =
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
On Tue, Dec 29, 2015 at 10:55 AM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> > +static av_always_inline void idct_1D_8(pixel *x, int xstride, const
> pixel y[16])
> > +{
> > +int t1h, t4h, t6h, t0 = y[0], t1 = y[1], t2 = y[2], t3 = y[3], t4 =
> y[4];
> > +int t5 = y[5], t6 = y[6], t7 = y[7];
> > +t5 -= (t3*2485 + 4096) >> 13;
> > +t3 += (t5*18205 + 16384) >> 15;
> > +t5 -= (t3*2485 + 4096) >> 13;
> > +t7 -= (t1*3227 + 16384) >> 15;
> > +t1 += (t7*6393 + 16384) >> 15;
> > +t7 -= (t1*3227 + 16384) >> 15;
> > +t1 += t3;
>
> These seven lines can overflow.
Why do you believe they can overflow? Look at range constraints for the
input values.
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On 29.12.2015 16:58, Ronald S. Bultje wrote:
> Hi,
>
> On Tue, Dec 29, 2015 at 10:55 AM, Andreas Cadhalpun <
> andreas.cadhal...@googlemail.com> wrote:
>
>>> +static av_always_inline void idct_1D_8(pixel *x, int xstride, const
>> pixel y[16])
>>> +{
>>> +int t1h, t4h, t6h, t0 = y[0], t1 = y[1], t2 = y[2], t3 = y[3], t4 =
>> y[4];
>>> +int t5 = y[5], t6 = y[6], t7 = y[7];
>>> +t5 -= (t3*2485 + 4096) >> 13;
>>> +t3 += (t5*18205 + 16384) >> 15;
>>> +t5 -= (t3*2485 + 4096) >> 13;
>>> +t7 -= (t1*3227 + 16384) >> 15;
>>> +t1 += (t7*6393 + 16384) >> 15;
>>> +t7 -= (t1*3227 + 16384) >> 15;
>>> +t1 += t3;
>>
>> These seven lines can overflow.
>
>
> Why do you believe they can overflow?
Because ubsan told me that.
> Look at range constraints for the input values.
Apparently that is not constrained enough, e.g.:
t3 = -1449866
t3*2485 = -3602917010 < INT32_MIN
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] oggparsedaala: check number of planes in pixel format map
This fixes crashes caused by out-of-bounds writes.
Signed-off-by: Andreas Cadhalpun
---
libavformat/oggparsedaala.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
index dda8d70..24567f9 100644
--- a/libavformat/oggparsedaala.c
+++ b/libavformat/oggparsedaala.c
@@ -130,6 +130,12 @@ static int daala_header(AVFormatContext *s, int idx)
hdr->fpr = bytestream2_get_byte(&gb);
hdr->format.planes = bytestream2_get_byte(&gb);
+if (hdr->format.planes > 4) {
+av_log(s, AV_LOG_ERROR,
+ "Invalid number of planes %d in daala pixel format map.\n",
+ hdr->format.planes);
+return AVERROR_INVALIDDATA;
+}
for (i = 0; i < hdr->format.planes; i++) {
hdr->format.xdec[i] = bytestream2_get_byte(&gb);
hdr->format.ydec[i] = bytestream2_get_byte(&gb);
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] AAC encoder: fix assertion error with prediction
On Tue, Dec 29, 2015 at 6:18 AM, Rostislav Pehlivanov wrote: > Wouldn't it be simpler to just check if the maximum codebook was 0 after > calculating cost1 and skipping the rest of the code in the loop? > > On 29 December 2015 at 08:23, Claudio Freire wrote: > >> Fixes an assertion error reported in #2686 that happens when >> using prediction (either explicitly or implicitly by setting >> the AAC main profile), since prediction code would allow >> creating new zeroes or removing existing ones, without >> properly checking for SF delta violations. >> >> This patch forbids creating/removing zeroes, perhaps an >> overly conservative approach, but a safe one. More permissive >> and sophisticated approaches may be attempted in the future. It is possible, and in fact does happen and it helps considerably to hide some artifacts, to use prediction in zeroed bands. It basically uses the predicted coefficient with no correction. May well be better than using a zero. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/avf_showspectrum: add window overlap support
Signed-off-by: Paul B Mahol
---
doc/filters.texi | 5
libavfilter/avf_showspectrum.c | 57 +++---
2 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index e1b3c69..adcbd58 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14669,6 +14669,11 @@ Default value is @code{hann}.
@item orientation
Set orientation of time vs frequency axis. Can be @code{vertical} or
@code{horizontal}. Default is @code{vertical}.
+
+@item overlap
+Set ratio of overlap window. Default value is @code{0}.
+When value is @code{1} overlap is set to recommended size for specific
+window function currently used.
@end table
The usage is very similar to the showwaves filter; see the examples in that
diff --git a/libavfilter/avf_showspectrum.c b/libavfilter/avf_showspectrum.c
index 1872135..8ec6cd5 100644
--- a/libavfilter/avf_showspectrum.c
+++ b/libavfilter/avf_showspectrum.c
@@ -1,6 +1,7 @@
/*
* Copyright (c) 2012-2013 Clément Bœsch
* Copyright (c) 2013 Rudolf Polzer
+ * Copyright (c) 2015 Paul B Mahol
*
* This file is part of FFmpeg.
*
@@ -28,9 +29,12 @@
#include
#include "libavcodec/avfft.h"
+#include "libavutil/audio_fifo.h"
#include "libavutil/avassert.h"
#include "libavutil/channel_layout.h"
#include "libavutil/opt.h"
+#include "audio.h"
+#include "video.h"
#include "avfilter.h"
#include "internal.h"
#include "window_func.h"
@@ -62,7 +66,10 @@ typedef struct {
int win_func;
double win_scale;
float overlap;
+int skip_samples;
float *combine_buffer; ///< color combining buffer (3 * h items)
+AVAudioFifo *fifo;
+int64_t pts;
} ShowSpectrumContext;
#define OFFSET(x) offsetof(ShowSpectrumContext, x)
@@ -107,6 +114,7 @@ static const AVOption showspectrum_options[] = {
{ "orientation", "set orientation", OFFSET(orientation), AV_OPT_TYPE_INT,
{.i64=VERTICAL}, 0, NB_ORIENTATIONS-1, FLAGS, "orientation" },
{ "vertical", NULL, 0, AV_OPT_TYPE_CONST, {.i64=VERTICAL}, 0, 0,
FLAGS, "orientation" },
{ "horizontal", NULL, 0, AV_OPT_TYPE_CONST, {.i64=HORIZONTAL}, 0, 0,
FLAGS, "orientation" },
+{ "overlap", "set window overlap", OFFSET(overlap), AV_OPT_TYPE_FLOAT,
{.dbl = 0}, 0, 1, FLAGS },
{ NULL }
};
@@ -137,6 +145,7 @@ static av_cold void uninit(AVFilterContext *ctx)
av_freep(&s->rdft_data);
av_freep(&s->window_func_lut);
av_frame_free(&s->outpicref);
+av_audio_fifo_free(s->fifo);
}
static int query_formats(AVFilterContext *ctx)
@@ -176,6 +185,7 @@ static int config_output(AVFilterLink *outlink)
AVFilterLink *inlink = ctx->inputs[0];
ShowSpectrumContext *s = ctx->priv;
int i, rdft_bits, win_size, h, w;
+float overlap;
outlink->w = s->w;
outlink->h = s->h;
@@ -230,7 +240,14 @@ static int config_output(AVFilterLink *outlink)
sizeof(*s->window_func_lut));
if (!s->window_func_lut)
return AVERROR(ENOMEM);
-ff_generate_window_func(s->window_func_lut, win_size, s->win_func,
&s->overlap);
+ff_generate_window_func(s->window_func_lut, win_size, s->win_func,
&overlap);
+if (s->overlap == 1)
+s->overlap = overlap;
+s->skip_samples = (1. - s->overlap) * win_size;
+if (s->skip_samples < 1) {
+av_log(ctx, AV_LOG_ERROR, "overlap %f too big\n", s->overlap);
+return AVERROR(EINVAL);
+}
for (s->win_scale = 0, i = 0; i < win_size; i++) {
s->win_scale += s->window_func_lut[i] * s->window_func_lut[i];
@@ -255,15 +272,12 @@ static int config_output(AVFilterLink *outlink)
(s->orientation == HORIZONTAL && s->xpos >= outlink->h))
s->xpos = 0;
-outlink->frame_rate = av_make_q(inlink->sample_rate, win_size);
+outlink->frame_rate = av_make_q(inlink->sample_rate, win_size *
(1.-s->overlap));
if (s->orientation == VERTICAL && s->sliding == FULLFRAME)
outlink->frame_rate.den *= outlink->w;
if (s->orientation == HORIZONTAL && s->sliding == FULLFRAME)
outlink->frame_rate.den *= outlink->h;
-inlink->min_samples = inlink->max_samples = inlink->partial_buf_size =
-win_size;
-
if (s->orientation == VERTICAL) {
s->combine_buffer =
av_realloc_f(s->combine_buffer, outlink->h * 3,
@@ -276,6 +290,11 @@ static int config_output(AVFilterLink *outlink)
av_log(ctx, AV_LOG_VERBOSE, "s:%dx%d RDFT window size:%d\n",
s->w, s->h, win_size);
+
+av_audio_fifo_free(s->fifo);
+s->fifo = av_audio_fifo_alloc(inlink->format, inlink->channels, win_size);
+if (!s->fifo)
+return AVERROR(ENOMEM);
return 0;
}
@@ -552,13 +571,33 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
*insamples)
AVFilterContext *ctx = inlink->dst;
ShowSpectrumContext *s = ctx->priv;
unsigned win_size = 1 << s->rdft_bits;
+AVFrame *fin =
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
On Tue, Dec 29, 2015 at 11:07 AM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> On 29.12.2015 16:58, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Tue, Dec 29, 2015 at 10:55 AM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >>> +static av_always_inline void idct_1D_8(pixel *x, int xstride, const
> >> pixel y[16])
> >>> +{
> >>> +int t1h, t4h, t6h, t0 = y[0], t1 = y[1], t2 = y[2], t3 = y[3], t4
> =
> >> y[4];
> >>> +int t5 = y[5], t6 = y[6], t7 = y[7];
> >>> +t5 -= (t3*2485 + 4096) >> 13;
> >>> +t3 += (t5*18205 + 16384) >> 15;
> >>> +t5 -= (t3*2485 + 4096) >> 13;
> >>> +t7 -= (t1*3227 + 16384) >> 15;
> >>> +t1 += (t7*6393 + 16384) >> 15;
> >>> +t7 -= (t1*3227 + 16384) >> 15;
> >>> +t1 += t3;
> >>
> >> These seven lines can overflow.
> >
> >
> > Why do you believe they can overflow?
>
> Because ubsan told me that.
>
> > Look at range constraints for the input values.
>
> Apparently that is not constrained enough, e.g.:
> t3 = -1449866
> t3*2485 = -3602917010 < INT32_MIN
So what type is a pixel? Isn't a pixel uint8_t? And coefs are typically
constrained alike. Or are we talking 12bpp content here? In that case, you
likely need 64bit integers for 15bit math precision (look at vp9 code), or
daala needs to reduce precision (as does hevc).
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/af_anequalizer: remove cabs, cexp dependencies
On Mon, Dec 28, 2015 at 10:27 PM, Matt Oliver wrote: > On 29 December 2015 at 11:51, Ganesh Ajjanagadde > wrote: > >> On Mon, Dec 28, 2015 at 8:22 AM, Paul B Mahol wrote: >> > On 12/27/15, Ganesh Ajjanagadde wrote: >> >> Replaces by real arithmetic. Tested the validity of these >> transformations >> >> separately. >> >> >> >> Signed-off-by: Ganesh Ajjanagadde >> >> --- >> >> configure| 1 - >> >> libavfilter/af_anequalizer.c | 25 + >> >> 2 files changed, 17 insertions(+), 9 deletions(-) >> >> >> > >> > If those are mathematically equivalent ok. >> >> mathematically equivalent, tested offline, numerical differences of >> the order of ~ 1e-15 or so, which is perfectly fine IMHO for double >> precision especially since it is a priori not clear which is more >> precise. Pushed, thanks. > > > Wouldn't it also be a good idea to remove the COMPLEX_FUNCS list from > configure as now neither cabs or cexp are used so configure can be > simplified by removing the checks for them. I asked Paul about this (when I removed them from some other filter). That time he said he wanted them for another filter (anequalizer) since the expressions were complicated. Judging from this commit, I think that was a fair assessment. In short, this is really a question for Paul (or others who use complex numbers): if there are plans on doing something with complex numbers that is not trivial to rewrite, it may be helpful not to remove as you suggest. If not, then it is perhaps ok. Personally, over the long term, I think there will be further instances of complex number usage. If we remove it now, I suspect it will get added back in at some point. As such, I won't write a patch for this; really upto others especially Paul. Maybe long run we can create an av_complex or something like that. A few problems are (among possibly many others): 1. Syntactic sugar for *, / , +, - will be lost, and one will need to create something like cmult, cdiv, and call at an increased verbosity cost. In C++, these basic operators can be overloaded (but then again stdc++ has a complex header); AFAIK there is no such mechanism that we can use in FFmpeg's "least common denominator" C. 2. Natural promotion rules will be lost, e.g suppose one passes a double to a complex function, say while multiplying a double complex by a double as in anequalizer. With proper complex number support, promotion rules kick in and everything works. With av_complex, this does not work, and explicit setting of imaginary part to 0 will be needed. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
On 29.12.2015 17:15, Ronald S. Bultje wrote:
> On Tue, Dec 29, 2015 at 11:07 AM, Andreas Cadhalpun <
> andreas.cadhal...@googlemail.com> wrote:
>
>> On 29.12.2015 16:58, Ronald S. Bultje wrote:
>>> On Tue, Dec 29, 2015 at 10:55 AM, Andreas Cadhalpun <
>>> andreas.cadhal...@googlemail.com> wrote:
>>>
> +static av_always_inline void idct_1D_8(pixel *x, int xstride, const
pixel y[16])
> +{
> +int t1h, t4h, t6h, t0 = y[0], t1 = y[1], t2 = y[2], t3 = y[3], t4
>> =
y[4];
> +int t5 = y[5], t6 = y[6], t7 = y[7];
> +t5 -= (t3*2485 + 4096) >> 13;
> +t3 += (t5*18205 + 16384) >> 15;
> +t5 -= (t3*2485 + 4096) >> 13;
> +t7 -= (t1*3227 + 16384) >> 15;
> +t1 += (t7*6393 + 16384) >> 15;
> +t7 -= (t1*3227 + 16384) >> 15;
> +t1 += t3;
These seven lines can overflow.
>>>
>>>
>>> Why do you believe they can overflow?
>>
>> Because ubsan told me that.
>>
>>> Look at range constraints for the input values.
>>
>> Apparently that is not constrained enough, e.g.:
>> t3 = -1449866
>> t3*2485 = -3602917010 < INT32_MIN
>
>
> So what type is a pixel?
#define pixel int32_t
> Isn't a pixel uint8_t?
No.
> And coefs are typically constrained alike. Or are we talking 12bpp content
> here?
I don't know.
> In that case, you likely need 64bit integers for 15bit math precision (look at
> vp9 code), or daala needs to reduce precision (as does hevc).
Yes, either the intermediate calculation needs to happen with 64bit integers,
or the input has to be sanitized in some way.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
On Tue, Dec 29, 2015 at 11:29 AM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> Hi,
>
> On 29.12.2015 17:15, Ronald S. Bultje wrote:
> > On Tue, Dec 29, 2015 at 11:07 AM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >> On 29.12.2015 16:58, Ronald S. Bultje wrote:
> >>> On Tue, Dec 29, 2015 at 10:55 AM, Andreas Cadhalpun <
> >>> andreas.cadhal...@googlemail.com> wrote:
> >>>
> > +static av_always_inline void idct_1D_8(pixel *x, int xstride, const
> pixel y[16])
> > +{
> > +int t1h, t4h, t6h, t0 = y[0], t1 = y[1], t2 = y[2], t3 = y[3],
> t4
> >> =
> y[4];
> > +int t5 = y[5], t6 = y[6], t7 = y[7];
> > +t5 -= (t3*2485 + 4096) >> 13;
> > +t3 += (t5*18205 + 16384) >> 15;
> > +t5 -= (t3*2485 + 4096) >> 13;
> > +t7 -= (t1*3227 + 16384) >> 15;
> > +t1 += (t7*6393 + 16384) >> 15;
> > +t7 -= (t1*3227 + 16384) >> 15;
> > +t1 += t3;
>
> These seven lines can overflow.
> >>>
> >>>
> >>> Why do you believe they can overflow?
> >>
> >> Because ubsan told me that.
> >>
> >>> Look at range constraints for the input values.
> >>
> >> Apparently that is not constrained enough, e.g.:
> >> t3 = -1449866
> >> t3*2485 = -3602917010 < INT32_MIN
> >
> >
> > So what type is a pixel?
>
> #define pixel int32_t
>
> > Isn't a pixel uint8_t?
>
> No.
>
> > And coefs are typically constrained alike. Or are we talking 12bpp
> content here?
>
> I don't know.
>
> > In that case, you likely need 64bit integers for 15bit math precision
> (look at
> > vp9 code), or daala needs to reduce precision (as does hevc).
>
> Yes, either the intermediate calculation needs to happen with 64bit
> integers,
> or the input has to be sanitized in some way.
It depends what the purpose and source was. Was this real input, or fuzzed,
or what? vp9 decoder can certainly overflow with garbage input and that is
specifically defined so in libvpx. "Only input generated from a real fdct"
is considered sane and has a defined outcome.
(Overflows in dsp code are typically not a security concern.)
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On 29.12.2015 17:32, Ronald S. Bultje wrote: > On Tue, Dec 29, 2015 at 11:29 AM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: >> On 29.12.2015 17:15, Ronald S. Bultje wrote: >>> In that case, you likely need 64bit integers for 15bit math precision >> (look at >>> vp9 code), or daala needs to reduce precision (as does hevc). >> >> Yes, either the intermediate calculation needs to happen with 64bit >> integers, >> or the input has to be sanitized in some way. > > > It depends what the purpose and source was. Was this real input, or fuzzed, > or what? These were fuzzed samples. > vp9 decoder can certainly overflow with garbage input and that is > specifically defined so in libvpx. "Only input generated from a real fdct" > is considered sane and has a defined outcome. Do you have a sample causing overflows in the vp9 decoder? > (Overflows in dsp code are typically not a security concern.) Well, the overflows in the imdct calculation of the aac_fixed decoder ultimately caused crashes. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE > > patch applied How exactly did you test this? Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavc/dsd_tablegen: speed up table generation
Tables are bit identical.
Sample benchmark (Haswell, GNU/Linux+gcc):
old:
814485 decicycles in dsd_ctables_tableinit, 512 runs, 0 skips
new:
356808 decicycles in dsd_ctable_tableinit, 512 runs, 0 skips
Binary size should essentially be identical, and is in fact identical on
the configuration I tested on.
Signed-off-by: Ganesh Ajjanagadde
---
libavcodec/dsd_tablegen.h | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/libavcodec/dsd_tablegen.h b/libavcodec/dsd_tablegen.h
index 6afb416..d4ef302 100644
--- a/libavcodec/dsd_tablegen.h
+++ b/libavcodec/dsd_tablegen.h
@@ -78,16 +78,17 @@ static float ctables[CTABLES][256];
static av_cold void dsd_ctables_tableinit(void)
{
-int t, e, m, k;
-double acc;
-for (t = 0; t < CTABLES; ++t) {
-k = FFMIN(HTAPS - t * 8, 8);
-for (e = 0; e < 256; ++e) {
-acc = 0.0;
-for (m = 0; m < k; ++m)
-acc += (((e >> (7 - m)) & 1) * 2 - 1) * htaps[t * 8 + m];
-ctables[CTABLES - 1 - t][e] = (float)acc;
+int t, e, m, sign;
+double acc[CTABLES];
+for (e = 0; e < 256; ++e) {
+memset(acc, 0, sizeof(acc));
+for (m = 0; m < 8; ++m) {
+sign = (((e >> (7 - m)) & 1) * 2 - 1);
+for (t = 0; t < CTABLES; ++t)
+acc[t] += sign * htaps[t * 8 + m];
}
+for (t = 0; t < CTABLES; ++t)
+ctables[CTABLES - 1 - t][e] = acc[t];
}
}
#endif /* CONFIG_HARDCODED_TABLES */
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] lavc/dsd_tablegen: always generate tables at runtime
Previous commit has sped up dsd_tablegen, and now table generation takes
~ 40k cycles. Thus, these tables can always be generated at runtime.
Tested with/without --enable-hardcoded-tables.
Signed-off-by: Ganesh Ajjanagadde
---
libavcodec/Makefile | 4 +---
libavcodec/dsd_tablegen.c | 38 --
libavcodec/dsd_tablegen.h | 5 -
3 files changed, 1 insertion(+), 46 deletions(-)
delete mode 100644 libavcodec/dsd_tablegen.c
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 1c7568b..0717d0a 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -975,7 +975,6 @@ HOSTPROGS = aacps_tablegen
\
cbrt_tablegen \
cbrt_fixed_tablegen \
cos_tablegen\
-dsd_tablegen\
dv_tablegen \
motionpixels_tablegen \
mpegaudio_tablegen \
@@ -1002,7 +1001,7 @@ $(SUBDIR)%_tablegen$(HOSTEXESUF): HOSTCFLAGS +=
-DCONFIG_SMALL=0
endif
GEN_HEADERS = cbrt_tables.h cbrt_fixed_tables.h aacps_tables.h
aacps_fixed_tables.h \
- dsd_tables.h dv_tables.h \
+ dv_tables.h \
sinewin_tables.h sinewin_fixed_tables.h mpegaudio_tables.h
motionpixels_tables.h \
pcm_tables.h qdm2_tables.h
GEN_HEADERS := $(addprefix $(SUBDIR), $(GEN_HEADERS))
@@ -1016,7 +1015,6 @@ $(SUBDIR)aacdec_fixed.o: $(SUBDIR)cbrt_fixed_tables.h
$(SUBDIR)aacps_float.o: $(SUBDIR)aacps_tables.h
$(SUBDIR)aacps_fixed.o: $(SUBDIR)aacps_fixed_tables.h
$(SUBDIR)aactab_fixed.o: $(SUBDIR)aac_fixed_tables.h
-$(SUBDIR)dsddec.o: $(SUBDIR)dsd_tables.h
$(SUBDIR)dvenc.o: $(SUBDIR)dv_tables.h
$(SUBDIR)sinewin.o: $(SUBDIR)sinewin_tables.h
$(SUBDIR)sinewin_fixed.o: $(SUBDIR)sinewin_fixed_tables.h
diff --git a/libavcodec/dsd_tablegen.c b/libavcodec/dsd_tablegen.c
deleted file mode 100644
index dbeb9fe..000
--- a/libavcodec/dsd_tablegen.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Generate a header file for hardcoded DSD tables
- *
- * 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
-#define CONFIG_HARDCODED_TABLES 0
-#include "dsd_tablegen.h"
-#include "tableprint.h"
-#include
-
-int main(void)
-{
-dsd_ctables_tableinit();
-
-write_fileheader();
-
-printf("static const double ctables[CTABLES][256] = {\n");
-write_float_2d_array(ctables, CTABLES, 256);
-printf("};\n");
-
-return 0;
-}
diff --git a/libavcodec/dsd_tablegen.h b/libavcodec/dsd_tablegen.h
index d4ef302..990d57a 100644
--- a/libavcodec/dsd_tablegen.h
+++ b/libavcodec/dsd_tablegen.h
@@ -29,10 +29,6 @@
#define HTAPS 48/** number of FIR constants */
#define CTABLES ((HTAPS + 7) / 8) /** number of "8 MACs" lookup tables */
-#if CONFIG_HARDCODED_TABLES
-#define dsd_ctables_tableinit()
-#include "libavcodec/dsd_tables.h"
-#else
#include "libavutil/common.h"
/*
@@ -91,6 +87,5 @@ static av_cold void dsd_ctables_tableinit(void)
ctables[CTABLES - 1 - t][e] = acc[t];
}
}
-#endif /* CONFIG_HARDCODED_TABLES */
#endif /* AVCODEC_DSD_TABLEGEN_H */
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: check number of planes in pixel format map
LGTM,
Thanks
On 29 December 2015 at 16:09, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> This fixes crashes caused by out-of-bounds writes.
>
> Signed-off-by: Andreas Cadhalpun
> ---
> libavformat/oggparsedaala.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
> index dda8d70..24567f9 100644
> --- a/libavformat/oggparsedaala.c
> +++ b/libavformat/oggparsedaala.c
> @@ -130,6 +130,12 @@ static int daala_header(AVFormatContext *s, int idx)
> hdr->fpr = bytestream2_get_byte(&gb);
>
> hdr->format.planes = bytestream2_get_byte(&gb);
> +if (hdr->format.planes > 4) {
> +av_log(s, AV_LOG_ERROR,
> + "Invalid number of planes %d in daala pixel format
> map.\n",
> + hdr->format.planes);
> +return AVERROR_INVALIDDATA;
> +}
> for (i = 0; i < hdr->format.planes; i++) {
> hdr->format.xdec[i] = bytestream2_get_byte(&gb);
> hdr->format.ydec[i] = bytestream2_get_byte(&gb);
> --
> 2.6.4
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffprobe: avoid unnecessary pow and exp2 calls
On Tue, Dec 29, 2015 at 3:04 AM, Stefano Sabatini wrote:
> On date Wednesday 2015-12-23 09:56:17 -0800, Ganesh Ajjanagadde encoded:
>> These are just for prefixes and may be hardcoded easily; see lavu/eval
>> for this approach.
>>
>> Signed-off-by: Ganesh Ajjanagadde
>> ---
>> ffprobe.c | 27 +++
>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/ffprobe.c b/ffprobe.c
>> index 0b672ff..c352b44 100644
>> --- a/ffprobe.c
>> +++ b/ffprobe.c
>> @@ -218,8 +218,19 @@ static AVInputFormat *iformat = NULL;
>>
>> static struct AVHashContext *hash;
>>
>> -static const char *const binary_unit_prefixes [] = { "", "Ki", "Mi", "Gi",
>> "Ti", "Pi" };
>> -static const char *const decimal_unit_prefixes[] = { "", "K" , "M" , "G" ,
>> "T" , "P" };
>> +static const struct {
>> +double bin_val;
>> +double dec_val;
>> +const char *bin_str;
>> +const char *dec_str;
>> +} si_prefixes[] = {
>> +{ 1.0, 1.0, "", "" },
>> +{ 1.024e3, 1e3, "Ki", "K" },
>> +{ 1.048576e6, 1e6, "Mi", "M" },
>> +{ 1.073741824e9, 1e9, "Gi", "G" },
>> +{ 1.099511627776e12, 1e12, "Ti", "T" },
>> +{ 1.125899906842624e15, 1e15, "Pi", "P" },
>> +};
>>
>> static const char unit_second_str[] = "s";
>> static const char unit_hertz_str[] = "Hz" ;
>> @@ -273,14 +284,14 @@ static char *value_string(char *buf, int buf_size,
>> struct unit_value uv)
>>
>> if (uv.unit == unit_byte_str && use_byte_value_binary_prefix) {
>> index = (long long int) (log2(vald)) / 10;
>> -index = av_clip(index, 0,
>> FF_ARRAY_ELEMS(binary_unit_prefixes) - 1);
>> -vald /= exp2(index * 10);
>> -prefix_string = binary_unit_prefixes[index];
>> +index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
>> +vald /= si_prefixes[index].bin_val;
>> +prefix_string = si_prefixes[index].bin_str;
>> } else {
>> index = (long long int) (log10(vald)) / 3;
>> -index = av_clip(index, 0,
>> FF_ARRAY_ELEMS(decimal_unit_prefixes) - 1);
>> -vald /= pow(10, index * 3);
>> -prefix_string = decimal_unit_prefixes[index];
>> +index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
>> +vald /= si_prefixes[index].dec_val;
>> +prefix_string = si_prefixes[index].dec_str;
>> }
>> vali = vald;
>> }
>> --
>> 2.6.4
>
> LGTM, thanks.
pushed, thanks
> --
> FFmpeg = Fancy & Fascinating Meaningless Puritan Eretic Guru
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Changelog: add note on dynaudnorm availability on all platforms
On Wed, Dec 23, 2015 at 3:29 PM, Hendrik Leppkes wrote: > Am 23.12.2015 19:47 schrieb "Ganesh Ajjanagadde" : >> >> On Wed, Dec 23, 2015 at 10:02 AM, Hendrik Leppkes > wrote: >> > Am 23.12.2015 18:30 schrieb "Ganesh Ajjanagadde" >: >> >> >> >> Commits 062e3e23824ba3d25594ea966d7834bcf34db49b and >> >> dd68cde28a44bbf5307d29ee6cb8ebd14985dea5 added support for erf and >> >> copysign on broken libms, and so dynaudnorm is now available on all >> >> platforms. >> >> >> >> This is user facing; hence the Changelog entry. >> >> >> >> Signed-off-by: Ganesh Ajjanagadde >> >> --- >> >> Changelog | 1 + >> >> 1 file changed, 1 insertion(+) >> >> >> >> diff --git a/Changelog b/Changelog >> >> index 2b46ddb..bb29afb 100644 >> >> --- a/Changelog >> >> +++ b/Changelog >> >> @@ -47,6 +47,7 @@ version : >> >> - DXVA2-accelerated VP9 decoding >> >> - SOFAlizer: virtual binaural acoustics filter >> >> - VAAPI VP9 hwaccel >> >> +- dynaudnorm filter availability on all platforms >> >> >> >> >> >> version 2.8: >> >> -- >> >> 2.6.4 >> >> >> > >> > dynaudnorm hasnt been in any previous release, has it? IIRC its still > very >> > new itself, so no need for a change log entry IMHO. >> >> As Paul points out, it was added in 2.8. It was for this precise >> reason that I wondered about the Changelog. So is the summary that the >> patch is ok (or neutral)? In other words, does anyone oppose it? >> > > Personally I wouldn't care that much to mention it in the change log, but > its not like I would oppose it, so if Paul as the author of the filter > feels its fine, then LGTM as well. Dropped, no real consensus on these things and since I am not much of a user of FFmpeg, I don't really know whether they obtain value from these things in the Changelog. Thanks. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Tue, Dec 29, 2015 at 04:56:26PM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE > > > > patch applied > > How exactly did you test this? it is neccessary to also fix the decoder to test, this was a fix for just the demuxer side i tested that the current decoder still works fine with the fixed demuxer [...] -- 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] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On Tue, Dec 29, 2015 at 02:12:44AM +, Rostislav Pehlivanov wrote: > Hmm, kinda odd. > I've attached a v2 of the RFC Patch which does this: > 1. Fixed some typos. > 2. Fixed some missing newlines at the end of daalatab* > 3. Moves the pix_fmt finding function inside the main decoder file. [...] > +#ifndef AVCODEC_DAALAENTROPY_H > +#define AVCODEC_DAALAENTROPY_H > + > +#include "libavutil/avassert.h" > + > +#include "daala.h" > +#include "daalatab.h" > + > +#define DAALAENT_CDF_ACCESS(n) &daalaent_cdf_tab[((n)*((n) - 1) >> 1) - 1] > + > +#define DAALAENT_WSIZE (int)sizeof(ent_win)*CHAR_BIT > +#define DAALAENT_BIT_ABUNDANCE 16384 > +#define DAALAENT_UINT_BITS 4 > +#define DAALAENT_MODEL_TAB 12 > +#define DAALAENT_SAT(a,b) (a - FFMIN(a,b)) the "a" should be protected by a () > + > +#define DAALAENT_PVQ_COUNT2 > +#define DAALAENT_PVQ_COUNT_EX 3 > +#define DAALAENT_PVQ_K0 > +#define DAALAENT_PVQ_SUM_EX 1 > +#define DAALAENT_PVQ_NOVAL(-2147483647-1) why is the 1 not already subtracted in the written value ? [...] > +int daaladsp_init(DaalaDSP *d, int bit_depth) needs a prefix or static [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavc/cook: get rid of wasteful pow in init_pow2table
The table is highly structured, so pow (or exp2 for that matter) can entirely
be avoided, yielding a ~ 40x speedup with no loss of accuracy.
sample benchmark (Haswell, GNU/Linux):
new:
4449 decicycles in init_pow2table(loop 1000), 254 runs, 2 skips
4411 decicycles in init_pow2table(loop 1000), 510 runs, 2 skips
4391 decicycles in init_pow2table(loop 1000),1022 runs, 2 skips
old:
183673 decicycles in init_pow2table(loop 1000), 256 runs, 0 skips
182142 decicycles in init_pow2table(loop 1000), 512 runs, 0 skips
182104 decicycles in init_pow2table(loop 1000),1024 runs, 0 skips
Signed-off-by: Ganesh Ajjanagadde
---
libavcodec/cook.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/libavcodec/cook.c b/libavcodec/cook.c
index d8fb736..aa434a2 100644
--- a/libavcodec/cook.c
+++ b/libavcodec/cook.c
@@ -166,10 +166,17 @@ static float rootpow2tab[127];
/* table generator */
static av_cold void init_pow2table(void)
{
+/* fast way of computing 2^i and 2^(0.5*i) for -63 <= i < 64 */
int i;
+static const float exp2_tab[2] = {1, M_SQRT2};
+float exp2_val = 1.0842021724855044e-19; /* 2^(-63) */
+float root_val = 2.3283064365386963e-10; /* 2^(-32) */
for (i = -63; i < 64; i++) {
-pow2tab[63 + i] = pow(2, i);
-rootpow2tab[63 + i] = sqrt(pow(2, i));
+if (!(i & 1))
+root_val *= 2;
+pow2tab[63 + i] = exp2_val;
+rootpow2tab[63 + i] = root_val * exp2_tab[i & 1];
+exp2_val *= 2;
}
}
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > On Tue, Dec 29, 2015 at 04:56:26PM +, Carl Eugen Hoyos wrote: > > Michael Niedermayer niedermayer.cc> writes: > > > > > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE > > > > > > patch applied > > > > How exactly did you test this? > > it is neccessary to also fix the decoder to test (I disagree.) If that were true, more time should be given for reviews imo. > this was a fix for just the demuxer side If you cannot test, how do you know it is correct? The palette is transparent, this indicates a bug somewhere afaict (not necessarily in the demuxer.) The palette is never used because gray is signaled, this indicates another difficult to understand issue, this one definitely demuxer-related. In any case, the patch is not a sufficient fix for the demuxer issue. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: check number of planes in pixel format map
On 29.12.2015 17:57, Rostislav Pehlivanov wrote: > LGTM, Pushed. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
Also use uint32_t for the shift calculation, as 1 << 31 is undefined
for int32_t.
This fixes ubsan runtime error: shift exponent is too large for
32-bit type 'int'
Signed-off-by: Andreas Cadhalpun
---
libavformat/oggparsedaala.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
index 24567f9..9f27ba6 100644
--- a/libavformat/oggparsedaala.c
+++ b/libavformat/oggparsedaala.c
@@ -123,7 +123,12 @@ static int daala_header(AVFormatContext *s, int idx)
hdr->frame_duration = bytestream2_get_ne32(&gb);
hdr->gpshift = bytestream2_get_byte(&gb);
-hdr->gpmask = (1 << hdr->gpshift) - 1;
+if (hdr->gpshift >= 32) {
+av_log(s, AV_LOG_ERROR, "Too large gpshift %d (>= 32).\n",
+ hdr->gpshift);
+return AVERROR_INVALIDDATA;
+}
+hdr->gpmask = ((uint32_t)1 << hdr->gpshift) - 1;
hdr->format.depth = 8 + 2*(bytestream2_get_byte(&gb)-1);
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi, On Tue, Dec 29, 2015 at 12:25 PM, Michael Niedermayer < mich...@niedermayer.cc> wrote: > On Tue, Dec 29, 2015 at 02:12:44AM +, Rostislav Pehlivanov wrote: > > +#define DAALAENT_PVQ_NOVAL(-2147483647-1) > > why is the 1 not already subtracted in the written value ? I think it's MIN_INT32, maybe just write that. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi, On Tue, Dec 29, 2015 at 11:49 AM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > On 29.12.2015 17:32, Ronald S. Bultje wrote: > > On Tue, Dec 29, 2015 at 11:29 AM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > >> On 29.12.2015 17:15, Ronald S. Bultje wrote: > >>> In that case, you likely need 64bit integers for 15bit math precision > >> (look at > >>> vp9 code), or daala needs to reduce precision (as does hevc). > >> > >> Yes, either the intermediate calculation needs to happen with 64bit > >> integers, > >> or the input has to be sanitized in some way. > > > > > > It depends what the purpose and source was. Was this real input, or > fuzzed, > > or what? > > These were fuzzed samples. > > > vp9 decoder can certainly overflow with garbage input and that is > > specifically defined so in libvpx. "Only input generated from a real > fdct" > > is considered sane and has a defined outcome. > > Do you have a sample causing overflows in the vp9 decoder? Nope, but I'm going to assume they're not hard to create, just use a few high same-sign quantized coefficients (e.g. SHORT_MAX) and a high quantizer (qidx=255, dq_ac=1828). Both dequant into int16_t (1828*SHORT_MAX doesn't fit in 16 bits) as well as the idct results themselves (because you're adding and subtracting from an already near-edge value which should fit in 16bits) will overflow. > (Overflows in dsp code are typically not a security concern.) > > Well, the overflows in the imdct calculation of the aac_fixed decoder > ultimately > caused crashes. I would prefer if for video codecs, unless specifically proven otherwise for that codec with specific content, we assume by default that overflows in performance-sensitive dsp code (specifically idct) are OK. ubsan may not like us on fuzzed content, but so be it. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On Tue, Dec 29, 2015 at 10:42 AM, Ronald S. Bultje wrote: > Hi, > > On Tue, Dec 29, 2015 at 12:25 PM, Michael Niedermayer < > mich...@niedermayer.cc> wrote: > >> On Tue, Dec 29, 2015 at 02:12:44AM +, Rostislav Pehlivanov wrote: >> > +#define DAALAENT_PVQ_NOVAL(-2147483647-1) >> >> why is the 1 not already subtracted in the written value ? Because there are no negative integer literals in C: http://en.cppreference.com/w/cpp/language/integer_literal, i.e a - sign applies unary minus, but 2147483648 is not representable in 32 bits, Thus, this literal will then have the type long int, or long long int depending on where it can fit. And because of the beauty of MSVC, it might not actually be able to use long long int, making the whole thing undefined behavior. Even without that, the type of this should really be int32_t or int. See /usr/include/stdint.h for this approach (to define INT32_MIN), thus Ronald's idea is better. > > > I think it's MIN_INT32, maybe just write that. You mean INT32_MIN :). > > Ronald > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/cook: get rid of wasteful pow in init_pow2table
On Tue, Dec 29, 2015 at 09:28:34AM -0800, Ganesh Ajjanagadde wrote:
> The table is highly structured, so pow (or exp2 for that matter) can entirely
> be avoided, yielding a ~ 40x speedup with no loss of accuracy.
>
> sample benchmark (Haswell, GNU/Linux):
> new:
> 4449 decicycles in init_pow2table(loop 1000), 254 runs, 2 skips
> 4411 decicycles in init_pow2table(loop 1000), 510 runs, 2 skips
> 4391 decicycles in init_pow2table(loop 1000),1022 runs, 2 skips
>
> old:
> 183673 decicycles in init_pow2table(loop 1000), 256 runs, 0 skips
> 182142 decicycles in init_pow2table(loop 1000), 512 runs, 0 skips
> 182104 decicycles in init_pow2table(loop 1000),1024 runs, 0 skips
>
> Signed-off-by: Ganesh Ajjanagadde
> ---
> libavcodec/cook.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cook.c b/libavcodec/cook.c
> index d8fb736..aa434a2 100644
> --- a/libavcodec/cook.c
> +++ b/libavcodec/cook.c
> @@ -166,10 +166,17 @@ static float rootpow2tab[127];
> /* table generator */
> static av_cold void init_pow2table(void)
> {
> +/* fast way of computing 2^i and 2^(0.5*i) for -63 <= i < 64 */
> int i;
> +static const float exp2_tab[2] = {1, M_SQRT2};
> +float exp2_val = 1.0842021724855044e-19; /* 2^(-63) */
> +float root_val = 2.3283064365386963e-10; /* 2^(-32) */
I'm pretty sure you can do
float exp2_val = pow(2, -63);
float root_val = pow(2, -32);
and compilers will inline them
[...]
--
Clément B.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] doc/filters: add showwavespic colorize example
Signed-off-by: Lou Logan
---
doc/filters.texi | 8
1 file changed, 8 insertions(+)
diff --git a/doc/filters.texi b/doc/filters.texi
index e1b3c69..45e1338 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14823,6 +14823,14 @@ in a 1024x800 picture using @command{ffmpeg}:
@example
ffmpeg -i audio.flac -lavfi showwavespic=split_channels=1:s=1024x800
waveform.png
@end example
+
+@item
+Colorize the waveform with colorchannelmixer. This example will make
+the waveform a green color approximately RGB(66,217,150). Additional
+channels will be shades of this color.
+@example
+ffmpeg -i audio.mp3 -filter_complex
"showwavespic,colorchannelmixer=rr=66/255:gg=217/255:bb=150/255" waveform.png
+@end example
@end itemize
@section split, asplit
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files
> > > > +case MKTAG('e','n','c','v'):// encrypted video
> > > > +case MKTAG('e','n','c','a'):// encrypted audio
> > > > +id = mov_codec_id(st, format);
> > > > +st->codec->codec_id = id;
> > >
> > > this seems missing a check for st->codec->codec_id being "unset"
> > > before setting it
> > >
> > Will add it
> >
While testing this change, I found that for audio, at this point codec_id is
actually already initialized -
it gets initialized when ff_mov_read_esds calls ff_mp4_read_dec_config_descr,
in this line:
st->codec->codec_id= ff_codec_get_id(ff_mp4_obj_type, object_type_id);
What happens with my current code is that this outputs a warning that frma is
ignored because codec_id
is not none. I think I will just skip the warning when st->codec->codec_id
"happens to be" the same as
the codec that was inferred by the frma atom.
This appears to work well:
case MKTAG('e','n','c','v'):// encrypted video
case MKTAG('e','n','c','a'):// encrypted audio
id = mov_codec_id(st, format);
if (st->codec->codec_id != AV_CODEC_ID_NONE &&
st->codec->codec_id != id) {
av_log(c->fc, AV_LOG_WARNING,
"ignoring 'frma' atom of '%.4s', stream has codec id %d\n",
(char*)&format, st->codec->codec_id);
break;
}
st->codec->codec_id = id;
sc->format = format;
break;
Please let me know if you think that is ok, and I will resubmit the patch with
all fixes.
> > >
> > > the way these are printed would lead to different results on big and
> > > little endian
> > >
> > Correct, however it's already done this way in a few other places in this
> > file (search for %.4s)
> > Do you prefer that I will print it with %c%c%c%c as in
> > ff_mov_read_stsd_entries ?
>
> whatever way you prefer
>
Ok, so I think I'll just leave it as is - big endian environments will get the
atom name in
reverse order in the log message (I assume 99% of the people use ffmpeg on
little endian anyway... :))
Thanks
Eran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/cook: get rid of wasteful pow in init_pow2table
On Tue, Dec 29, 2015 at 11:29 AM, Clément Bœsch wrote:
> On Tue, Dec 29, 2015 at 09:28:34AM -0800, Ganesh Ajjanagadde wrote:
>> The table is highly structured, so pow (or exp2 for that matter) can entirely
>> be avoided, yielding a ~ 40x speedup with no loss of accuracy.
>>
>> sample benchmark (Haswell, GNU/Linux):
>> new:
>> 4449 decicycles in init_pow2table(loop 1000), 254 runs, 2 skips
>> 4411 decicycles in init_pow2table(loop 1000), 510 runs, 2 skips
>> 4391 decicycles in init_pow2table(loop 1000),1022 runs, 2 skips
>>
>> old:
>> 183673 decicycles in init_pow2table(loop 1000), 256 runs, 0 skips
>> 182142 decicycles in init_pow2table(loop 1000), 512 runs, 0 skips
>> 182104 decicycles in init_pow2table(loop 1000),1024 runs, 0 skips
>>
>> Signed-off-by: Ganesh Ajjanagadde
>> ---
>> libavcodec/cook.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/cook.c b/libavcodec/cook.c
>> index d8fb736..aa434a2 100644
>> --- a/libavcodec/cook.c
>> +++ b/libavcodec/cook.c
>> @@ -166,10 +166,17 @@ static float rootpow2tab[127];
>> /* table generator */
>> static av_cold void init_pow2table(void)
>> {
>> +/* fast way of computing 2^i and 2^(0.5*i) for -63 <= i < 64 */
>> int i;
>> +static const float exp2_tab[2] = {1, M_SQRT2};
>
>> +float exp2_val = 1.0842021724855044e-19; /* 2^(-63) */
>> +float root_val = 2.3283064365386963e-10; /* 2^(-32) */
>
> I'm pretty sure you can do
> float exp2_val = pow(2, -63);
> float root_val = pow(2, -32);
> and compilers will inline them
Any decent compiler would.
Alternatively, if we had hexadecimal floating point literals (%a) on
all platforms, a C99 feature, it would look quite clean. Hexadecimal
floating point literals are also nice as they are bit-exact
representations of the underlying float, unlike decimal constants
where one needs to reason about how many digits one needs. I believe
"%.17g works for IEEE-754 doubles, note that for instance %.17lf does
not on very small inputs. Unfortunately, MSVC lacks hexadecimal
floating literals.
I really don't mind either way, and since you prefer pow(2,-63), I
have changed locally.
>
> [...]
>
> --
> Clément B.
>
> ___
> 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] lavfi/af_sofalizer: remove exp2 and replace clz by ff_clz
ff_clz is faster, and uses an intrinsic (at the moment on GCC). exp2 is
a wasteful function for a simple integer exponentiation.
Untested.
Signed-off-by: Ganesh Ajjanagadde
---
libavfilter/af_sofalizer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libavfilter/af_sofalizer.c b/libavfilter/af_sofalizer.c
index 6a24cbc..0bd1931 100644
--- a/libavfilter/af_sofalizer.c
+++ b/libavfilter/af_sofalizer.c
@@ -30,6 +30,7 @@
#include "libavcodec/avfft.h"
#include "libavutil/float_dsp.h"
+#include "libavutil/intmath.h"
#include "libavutil/opt.h"
#include "avfilter.h"
#include "internal.h"
@@ -996,8 +997,8 @@ static int config_input(AVFilterLink *inlink)
}
/* buffer length is longest IR plus max. delay -> next power of 2
(32 - count leading zeros gives required exponent) */
-s->buffer_length = exp2(32 - clz((uint32_t)n_max));
-s->n_fft = exp2(32 - clz((uint32_t)(n_max + inlink->sample_rate)));
+s->buffer_length = 1 << (32 - ff_clz(n_max));
+s->n_fft = 1 << (32 - ff_clz(n_max + inlink->sample_rate));
if (s->type == FREQUENCY_DOMAIN) {
av_fft_end(s->fft[0]);
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
oggparsetheora has the same bit of code to read the gpshift, so it would
probably be a good idea to add it to this patch as well.
On 29 December 2015 at 17:55, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> Also use uint32_t for the shift calculation, as 1 << 31 is undefined
> for int32_t.
>
> This fixes ubsan runtime error: shift exponent is too large for
> 32-bit type 'int'
>
> Signed-off-by: Andreas Cadhalpun
> ---
> libavformat/oggparsedaala.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
> index 24567f9..9f27ba6 100644
> --- a/libavformat/oggparsedaala.c
> +++ b/libavformat/oggparsedaala.c
> @@ -123,7 +123,12 @@ static int daala_header(AVFormatContext *s, int idx)
>
> hdr->frame_duration = bytestream2_get_ne32(&gb);
> hdr->gpshift = bytestream2_get_byte(&gb);
> -hdr->gpmask = (1 << hdr->gpshift) - 1;
> +if (hdr->gpshift >= 32) {
> +av_log(s, AV_LOG_ERROR, "Too large gpshift %d (>= 32).\n",
> + hdr->gpshift);
> +return AVERROR_INVALIDDATA;
> +}
> +hdr->gpmask = ((uint32_t)1 << hdr->gpshift) - 1;
>
> hdr->format.depth = 8 + 2*(bytestream2_get_byte(&gb)-1);
>
> --
> 2.6.4
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/29/2015 06:42 PM, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: On Tue, Dec 29, 2015 at 04:56:26PM +, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE patch applied How exactly did you test this? it is neccessary to also fix the decoder to test (I disagree.) If that were true, more time should be given for reviews imo. this was a fix for just the demuxer side If you cannot test, how do you know it is correct? The palette is transparent, this indicates a bug somewhere afaict (not necessarily in the demuxer.) The palette is never used because gray is signaled, this indicates another difficult to understand issue, this one definitely demuxer-related. In any case, the patch is not a sufficient fix for the demuxer issue. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel There is no "bug" or "demuxer issue", Carl. The problem lies entirely on the decoder side, and the fact that qtrle for 1-bit video uses AV_PIX_FMT_MONOWHITE rather than AV_PIX_FMT_PAL8, like Michael mentioned before. I just got it working properly in the decoder by the way. I'll be back. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
On Tue, Dec 29, 2015 at 6:55 PM, Andreas Cadhalpun
wrote:
> Also use uint32_t for the shift calculation, as 1 << 31 is undefined
> for int32_t.
>
> This fixes ubsan runtime error: shift exponent is too large for
> 32-bit type 'int'
>
> Signed-off-by: Andreas Cadhalpun
> ---
> libavformat/oggparsedaala.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
> index 24567f9..9f27ba6 100644
> --- a/libavformat/oggparsedaala.c
> +++ b/libavformat/oggparsedaala.c
> @@ -123,7 +123,12 @@ static int daala_header(AVFormatContext *s, int idx)
>
> hdr->frame_duration = bytestream2_get_ne32(&gb);
> hdr->gpshift = bytestream2_get_byte(&gb);
> -hdr->gpmask = (1 << hdr->gpshift) - 1;
> +if (hdr->gpshift >= 32) {
> +av_log(s, AV_LOG_ERROR, "Too large gpshift %d (>= 32).\n",
> + hdr->gpshift);
> +return AVERROR_INVALIDDATA;
> +}
> +hdr->gpmask = ((uint32_t)1 << hdr->gpshift) - 1;
1U << hdr->gpshift?
>
> hdr->format.depth = 8 + 2*(bytestream2_get_byte(&gb)-1);
>
> --
> 2.6.4
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
Thanks for the feedback.
>This is an infinite loop that can hang the decoder.
Will try to get the upstream to accept a fix.
>overflows in the entropy decoding system
The assertions should be able to catch them, so they should be fine.
They'll trigger if there's a desync with the bitstream. I might add more in
case they're not enough.
>What is the point of having DaalaSharedContext in addition to
DaalaBitstreamHeader?
I was looking at VP9 when starting to write this and it had this structure.
I'll remove the shared context one.
>Third, the decoder seems to return only the first frame, repeatedly,
>instead of the following frames.
You did read that it can only decode I-frames (keyframes). So you need to
encode your files with -k 1 to make all frames keyframes.
>First, this fails building with --enable-shared, because
daala_find_p_format
>is not static.
Fixed yesterday, but sent the same patch twice by mistake. It's been fixed,
but I'd like to post a v3 of the patch to address some of the issues found.
Thanks,
Rostislav
On 29 December 2015 at 15:55, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:
> On 29.12.2015 03:12, Rostislav Pehlivanov wrote:
> > Hmm, kinda odd.
> > I've attached a v2 of the RFC Patch which does this:
> > 1. Fixed some typos.
> > 2. Fixed some missing newlines at the end of daalatab*
> > 3. Moves the pix_fmt finding function inside the main decoder file.
> >
> > Try building now.
>
> First, this fails building with --enable-shared, because
> daala_find_p_format
> is not static.
>
> Second, this decoder crashes, hangs and triggers lots of undefined
> behavior. :-/
>
> Third, the decoder seems to return only the first frame, repeatedly,
> instead of the following frames.
>
> More details are interleaved in the diff below.
>
> > diff --git a/libavcodec/daala.h b/libavcodec/daala.h
> > new file mode 100644
> > index 000..535e78f
> > --- /dev/null
> > +++ b/libavcodec/daala.h
> > @@ -0,0 +1,78 @@
> [...]
> > +typedef struct DaalaBitstreamHeader {
> > +uint8_t key_frame;
> > +uint8_t bipred;
> > +uint8_t ref_num;
> > +uint8_t act_mask;
> > +uint8_t qm;
> > +uint8_t haar;
> > +uint8_t golden;
> > +uint8_t pvq_qm[DAALA_MAX_PLANES][DAALA_QM_SIZE];
> > +} DaalaBitstreamHeader;
> > +
> > +typedef struct DaalaSharedContext {
> > +DaalaBitstreamHeader h;
> > +} DaalaSharedContext;
>
> What is the point of having DaalaSharedContext in addition to
> DaalaBitstreamHeader?
>
> > diff --git a/libavcodec/daala_entropy.h b/libavcodec/daala_entropy.h
> > new file mode 100644
> > index 000..3fdcaef
> > --- /dev/null
> > +++ b/libavcodec/daala_entropy.h
> > @@ -0,0 +1,554 @@
> [...]
> > +/* Updates the generic exponential probability model */
> > +static av_always_inline void daalaent_exp_model_update(DaalaCDF *c, int
> *ex, int x,
> > + int xs, int id,
> int integrate)
> > +{
> > +int i, xenc;
> > +ent_rng *cdf = &c->cdf[id*c->y];
> > +if (cdf[15] + c->inc > 32767) {
> > +for (i = 0; i < 16; i++)
> > +cdf[i] = (cdf[i] >> 1) + i + 1;
> > +}
> > +xenc = FFMIN(15, xs);
> > +for (i = xenc; i < 16; i++)
> > +cdf[i] += c->inc;
> > +x = FFMIN(x, 32767);
> > +*ex += ((x << 16) - *ex) >> integrate;
>
> This subtraction can overflow.
>
> [...]
> > +/* "+derf | It was a hack for the screen coding wavelet tools." */
>
> A rather bad hack...
>
> > +static inline int daalaent_decode_unary(DaalaEntropy *e)
> > +{
> > +int rval = 0;
> > +while (!daalaent_decode_bits(e, 1))
>
> This is an infinite loop that can hang the decoder.
>
> > +rval++;
> > +return rval;
> > +}
> [...]
> > +/* Decodes quantized coefficients from the bitsteam */
> > +static inline void daalaent_decode_laplace_vector(DaalaEntropy *e,
> dctcoef *y,
> > + int n, int k, dctcoef
> *curr,
> > + const dctcoef *means)
> > +{
> > +int i, exp_q8, mean_k_q8, mean_sum_ex_q8, sum_ex = 0, kn = k,
> ran_delta = 0;
> > +if (k <= 1) {
> > +daalaent_decode_laplace_delta(e, y, n, k, curr, means);
> > +return;
> > +}
> > +if (!k) {
> > +curr[DAALAENT_PVQ_COUNT] = DAALAENT_PVQ_NOVAL;
> > +curr[DAALAENT_PVQ_COUNT_EX] = DAALAENT_PVQ_NOVAL;
> > +curr[DAALAENT_PVQ_K] = 0;
> > +curr[DAALAENT_PVQ_SUM_EX] = 0;
> > +memset(y, 0, n*sizeof(dctcoef));
> > +return;
> > +}
> > +mean_k_q8 = means[DAALAENT_PVQ_K];
> > +mean_sum_ex_q8 = means[DAALAENT_PVQ_SUM_EX];
> > +if (mean_k_q8 < 1 << 23)
> > +exp_q8 = 256*mean_k_q8/(1 + mean_sum_ex_q8);
>
> This multiplication can overflow.
>
> > +else
> > +exp_q8 = mean_k_q8/(1 + (mean_sum_ex_q8 >> 8));
> > +for (i = 0; i < n; i++) {
> > +int x, ex;
> > +if (!kn)
> > +break;
> > +if (kn <= 1 && i !=
[FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
The following patch fixes the lack of palettized display of 1-bit video
in the qtrle decoder. It is related to my previous patch of
lavf/qtpalette, which added 1-bit video to the "palettized video"
category. As far as I can see, everything works fine, but comments are
of course welcome.
Below are links to sample files, which should now be displayed properly
with bluish colors, but which were previously displayed in black &
white.
Matroska:
https://drive.google.com/open?id=0B3_pEBoLs0faNjI0cHBMWDhYY2c
QuickTime (mov):
https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE
Mats
--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From f625f83504258c475c49aae00de3bad108a48791 Mon Sep 17 00:00:00 2001
From: Mats Peterson
Date: Tue, 29 Dec 2015 22:50:56 +0100
Subject: [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
The following patch fixes the lack of palettized display of 1-bit video
in the qtrle decoder. It is related to my previous patch of
lavf/qtpalette, which added 1-bit video to the "palettized video"
category. As far as I can see, everything works fine, but comments are
of course welcome.
Below are links to sample files, which should now be displayed properly
with bluish colors, but which were previously displayed in black &
white.
Matroska:
https://drive.google.com/open?id=0B3_pEBoLs0faNjI0cHBMWDhYY2c
QuickTime (mov):
https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE
Mats
---
libavcodec/qtrle.c | 45 +
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c
index 1fcf5b3..3f482f4 100644
--- a/libavcodec/qtrle.c
+++ b/libavcodec/qtrle.c
@@ -83,9 +83,9 @@ static void qtrle_decode_1bpp(QtrleContext *s, int row_ptr, int lines_to_change)
if(skip & 0x80) {
lines_to_change--;
row_ptr += row_inc;
-pixel_ptr = row_ptr + 2 * (skip & 0x7f);
+pixel_ptr = row_ptr + 2 * 8 * (skip & 0x7f);
} else
-pixel_ptr += 2 * skip;
+pixel_ptr += 2 * 8 * skip;
CHECK_PIXEL_PTR(0); /* make sure pixel_ptr is positive */
if(rle_code == -1)
@@ -99,19 +99,42 @@ static void qtrle_decode_1bpp(QtrleContext *s, int row_ptr, int lines_to_change)
pi0 = bytestream2_get_byte(&s->g);
pi1 = bytestream2_get_byte(&s->g);
-CHECK_PIXEL_PTR(rle_code * 2);
+CHECK_PIXEL_PTR(rle_code * 2 * 8);
while (rle_code--) {
-rgb[pixel_ptr++] = pi0;
-rgb[pixel_ptr++] = pi1;
+rgb[pixel_ptr++] = (pi0 >> 7) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 6) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 5) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 4) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 3) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 2) & 0x01;
+rgb[pixel_ptr++] = (pi0 >> 1) & 0x01;
+rgb[pixel_ptr++] = pi0 & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 7) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 6) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 5) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 4) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 3) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 2) & 0x01;
+rgb[pixel_ptr++] = (pi1 >> 1) & 0x01;
+rgb[pixel_ptr++] = pi1 & 0x01;
}
} else {
/* copy the same pixel directly to output 2 times */
rle_code *= 2;
-CHECK_PIXEL_PTR(rle_code);
+CHECK_PIXEL_PTR(rle_code * 8);
-bytestream2_get_buffer(&s->g, &rgb[pixel_ptr], rle_code);
-pixel_ptr += rle_code;
+while (rle_code--) {
+int x = bytestream2_get_byte(&s->g);
+rgb[pixel_ptr++] = (x >> 7) & 0x01;
+rgb[pixel_ptr++] = (x >> 6) & 0x01;
+rgb[pixel_ptr++] = (x >> 5) & 0x01;
+rgb[pixel_ptr++] = (x >> 4) & 0x01;
+rgb[pixel_ptr++] = (x >> 3) & 0x01;
+rgb[pixel_ptr++] = (x >> 2) & 0x01;
+rgb[pixel_ptr++] = (x >> 1) & 0x01;
+rgb[pixel_ptr++] = x & 0x01;
+}
}
}
}
@@ -364,13 +387,10 @@ static av_cold int qtrle_decode_init(AVCodecContext *avctx)
s->avctx = avctx;
switch (avctx->bits_per_coded_sample) {
case 1:
-case 33:
-avctx->pix_fmt = AV_PIX_FMT_MONOWHITE;
-break;
-
case 2:
case 4:
case 8:
+case 33:
case 34:
case 36:
case 40:
@@ -446,6 +466,7 @@ static int qtrle_decode_frame(AVCodecContext *avctx,
case 1:
case 33:
qtrle_decode_1bpp(s, row_ptr, height);
+has_palette = 1;
break;
case 2:
--
1.7.10.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpe
[FFmpeg-devel] [PATCH] configure: Add missing extralib for gcrypt
Signed-off-by: Ricardo Constantino
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index e3b7904..6d41343 100755
--- a/configure
+++ b/configure
@@ -5396,7 +5396,7 @@ enabled avisynth && { { check_lib2 "windows.h"
LoadLibrary; } ||
enabled chromaprint && require chromaprint chromaprint.h
chromaprint_get_version -lchromaprint
enabled decklink && { check_header DeckLinkAPI.h || die "ERROR:
DeckLinkAPI.h header not found"; }
enabled frei0r&& { check_header frei0r.h || die "ERROR: frei0r.h
header not found"; }
-enabled gcrypt&& require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
+enabled gcrypt&& require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
-lgpg-error
enabled gmp && require2 gmp gmp.h mpz_export -lgmp
enabled gnutls&& require_pkg_config gnutls gnutls/gnutls.h
gnutls_global_init
enabled ladspa&& { check_header ladspa.h || die "ERROR: ladspa.h
header not found"; }
--
2.6.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter
Signed-off-by: Paul B Mahol
---
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/avf_ahistogram.c | 235 +++
3 files changed, 237 insertions(+)
create mode 100644 libavfilter/avf_ahistogram.c
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index e334016..931ced6 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -280,6 +280,7 @@ OBJS-$(CONFIG_NULLSINK_FILTER) +=
vsink_nullsink.o
# multimedia filters
OBJS-$(CONFIG_ADRAWGRAPH_FILTER) += f_drawgraph.o
+OBJS-$(CONFIG_AHISTOGRAM_FILTER) += avf_ahistogram.o
OBJS-$(CONFIG_APHASEMETER_FILTER)+= avf_aphasemeter.o
OBJS-$(CONFIG_AVECTORSCOPE_FILTER) += avf_avectorscope.o
OBJS-$(CONFIG_CONCAT_FILTER) += avf_concat.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index a039a39..0f96c3e 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -300,6 +300,7 @@ void avfilter_register_all(void)
/* multimedia filters */
REGISTER_FILTER(ADRAWGRAPH, adrawgraph, avf);
+REGISTER_FILTER(AHISTOGRAM, ahistogram, avf);
REGISTER_FILTER(APHASEMETER,aphasemeter,avf);
REGISTER_FILTER(AVECTORSCOPE, avectorscope, avf);
REGISTER_FILTER(CONCAT, concat, avf);
diff --git a/libavfilter/avf_ahistogram.c b/libavfilter/avf_ahistogram.c
new file mode 100644
index 000..46862c7
--- /dev/null
+++ b/libavfilter/avf_ahistogram.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright (c) 2015 Paul B Mahol
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/avassert.h"
+#include "libavutil/opt.h"
+#include "libavutil/parseutils.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "audio.h"
+#include "video.h"
+#include "internal.h"
+
+enum DisplayScale { LINEAR, SQRT, CBRT, LOG, NB_SCALES };
+enum AmplitudeScale { ALINEAR, ALOG, NB_ASCALES };
+
+typedef struct AudioHistogramContext {
+const AVClass *class;
+AVFrame *out;
+int w, h;
+AVRational frame_rate;
+uint64_t *histogram;
+int ascale;
+int scale;
+} AudioHistogramContext;
+
+#define OFFSET(x) offsetof(AudioHistogramContext, x)
+#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
+
+static const AVOption ahistogram_options[] = {
+{ "rate", "set video rate", OFFSET(frame_rate), AV_OPT_TYPE_VIDEO_RATE,
{.str="25"}, 0, 0, FLAGS },
+{ "r","set video rate", OFFSET(frame_rate), AV_OPT_TYPE_VIDEO_RATE,
{.str="25"}, 0, 0, FLAGS },
+{ "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 },
+{ "scale", "set display scale", OFFSET(scale), AV_OPT_TYPE_INT,
{.i64=LINEAR}, LINEAR, NB_SCALES-1, FLAGS, "scale" },
+{ "log", "logarithmic", 0, AV_OPT_TYPE_CONST, {.i64=LOG},0, 0,
FLAGS, "scale" },
+{ "sqrt", "square root", 0, AV_OPT_TYPE_CONST, {.i64=SQRT}, 0, 0,
FLAGS, "scale" },
+{ "cbrt", "cubic root", 0, AV_OPT_TYPE_CONST, {.i64=CBRT}, 0, 0,
FLAGS, "scale" },
+{ "lin", "linear", 0, AV_OPT_TYPE_CONST, {.i64=LINEAR}, 0, 0,
FLAGS, "scale" },
+{ "ascale", "set amplitude scale", OFFSET(ascale), AV_OPT_TYPE_INT,
{.i64=LINEAR}, LINEAR, NB_ASCALES-1, FLAGS, "ascale" },
+{ "log", "logarithmic", 0, AV_OPT_TYPE_CONST, {.i64=ALOG},0, 0,
FLAGS, "ascale" },
+{ "lin", "linear", 0, AV_OPT_TYPE_CONST, {.i64=ALINEAR}, 0, 0,
FLAGS, "ascale" },
+{ NULL }
+};
+
+AVFILTER_DEFINE_CLASS(ahistogram);
+
+static int query_formats(AVFilterContext *ctx)
+{
+AVFilterFormats *formats = NULL;
+AVFilterChannelLayouts *layouts = NULL;
+AVFilterLink *inlink = ctx->inputs[0];
+AVFilterLink *outlink = ctx->outputs[0];
+static const enum AVSampleFormat sample_fmts[] = { AV_SAMPLE_FMT_FLT,
AV_SAMPLE_FMT_NONE };
+static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_RGBA,
AV_PIX_FMT_NONE };
+int ret = AVERROR(EINVAL);
+
+formats = ff_make_format_list(sample_fmts);
+if ((ret = ff_formats_ref (formats, &inlink->out_formats))
< 0 ||
+(
Re: [FFmpeg-devel] [PATCH] configure: Add missing extralib for gcrypt
Le nonidi 9 nivôse, an CCXXIV, Ricardo Constantino a écrit :
> Signed-off-by: Ricardo Constantino
> ---
> configure | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index e3b7904..6d41343 100755
> --- a/configure
> +++ b/configure
> @@ -5396,7 +5396,7 @@ enabled avisynth && { { check_lib2 "windows.h"
> LoadLibrary; } ||
> enabled chromaprint && require chromaprint chromaprint.h
> chromaprint_get_version -lchromaprint
> enabled decklink && { check_header DeckLinkAPI.h || die "ERROR:
> DeckLinkAPI.h header not found"; }
> enabled frei0r&& { check_header frei0r.h || die "ERROR: frei0r.h
> header not found"; }
> -enabled gcrypt&& require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
> +enabled gcrypt&& require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
> -lgpg-error
> enabled gmp && require2 gmp gmp.h mpz_export -lgmp
> enabled gnutls&& require_pkg_config gnutls gnutls/gnutls.h
> gnutls_global_init
> enabled ladspa&& { check_header ladspa.h || die "ERROR: ladspa.h
> header not found"; }
This should be using libgcrypt-config, as per documentation:
https://gnupg.org/documentation/manuals/gcrypt/Building-sources.html
Otherwise, we have to duplicate the effort each time a dependency changes.
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] rtmpdh: Initialize gcrypt before using it
Trying to use gcrypt-enabled FFmpeg with rtmpe returns a few errors: Fatal: failed to create the RNG lock: Invalid argument FATAL: failed to acquire the FSM lock in libgrypt: Invalid argument This is probably not how or where this should be done, but it does fix the problem for me. You can test with: ffmpeg -i "rtmpe:// viacommtvstrmfs.fplive.net/viacommtvstrm/gsp.comedystor/com/dailyshow/TDS/season_20/episode_117/ds_20_117_act2_5a0dcad450_384x216_200_b30.mp4 " 0001-rtmpdh-Initialize-gcrypt-before-using-it.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: Add missing extralib for gcrypt
On 29 December 2015 at 22:17, Nicolas George wrote: > > This should be using libgcrypt-config, as per documentation: > > https://gnupg.org/documentation/manuals/gcrypt/Building-sources.html > > Otherwise, we have to duplicate the effort each time a dependency changes. > > Regards, > > -- > Nicolas George > I see only SDL is using something like that. Since libgcrypt doesn't come with a pkg-config file, something like SDL's check would be needed? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
Mats Peterson ffmpeg.org> writes: > Below are links to sample files, which should now be > displayed properly with bluish colors Sorry, I am not a native speaker: Could you explain what "should" means in above sentence? Please trim your quotes, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/29/2015 11:37 PM, Carl Eugen Hoyos wrote: Mats Peterson ffmpeg.org> writes: Below are links to sample files, which should now be displayed properly with bluish colors Sorry, I am not a native speaker: Could you explain what "should" means in above sentence? Please trim your quotes, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Neither am I, Carl, so we're in the same boat. Anyway, the video sample description in that rotating earth file contains a 1-bit palette consisting of two bluish colors. In QuickTime in Windows or Mac they will render correctly, but without this patch, they (and all other 1-bit files with a palette) will be displayed in black & white. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/29/2015 11:41 PM, Mats Peterson wrote: On 12/29/2015 11:37 PM, Carl Eugen Hoyos wrote: Mats Peterson ffmpeg.org> writes: Below are links to sample files, which should now be displayed properly with bluish colors Sorry, I am not a native speaker: Could you explain what "should" means in above sentence? Please trim your quotes, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Neither am I, Carl, so we're in the same boat. Anyway, the video sample description in that rotating earth file contains a 1-bit palette consisting of two bluish colors. In QuickTime in Windows or Mac they will render correctly, but without this patch, they (and all other 1-bit files with a palette) will be displayed in black & white. Mats Just because the video is 1-bit, it doesn't mean it has to be black & white. Thw two colors can be anything. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] configure: Use libgcrypt-config if available
Signed-off-by: Ricardo Constantino
---
configure | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index c986aba..d639b8d 100755
--- a/configure
+++ b/configure
@@ -5396,7 +5396,6 @@ enabled avisynth && { { check_lib2 "windows.h"
LoadLibrary; } ||
enabled chromaprint && require chromaprint chromaprint.h
chromaprint_get_version -lchromaprint
enabled decklink && { check_header DeckLinkAPI.h || die "ERROR:
DeckLinkAPI.h header not found"; }
enabled frei0r&& { check_header frei0r.h || die "ERROR: frei0r.h
header not found"; }
-enabled gcrypt&& require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
enabled gmp && require2 gmp gmp.h mpz_export -lgmp
enabled gnutls&& require_pkg_config gnutls gnutls/gnutls.h
gnutls_global_init
enabled ladspa&& { check_header ladspa.h || die "ERROR: ladspa.h
header not found"; }
@@ -,6 +5554,17 @@ if enabled libdc1394; then
enable libdc1394_1; } ||
die "ERROR: No version of libdc1394 found "
fi
+
+if enabled gcrypt; then
+GCRYPT_CONFIG="${cross_prefix}libgcrypt-config"
+if "${GCRYPT_CONFIG}" --version > /dev/null 2>&1; then
+gcrypt_libs=$("${GCRYPT_CONFIG}" --libs)
+require2 gcrypt gcrypt.h gcry_mpi_new $gcrypt_libs
+else
+require2 gcrypt gcrypt.h gcry_mpi_new -lgcrypt
+fi
+fi
+
if ! disabled sdl; then
SDL_CONFIG="${cross_prefix}sdl-config"
if check_pkg_config sdl SDL_events.h SDL_PollEvent; then
--
2.6.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
Mats Peterson ffmpeg.org> writes: > On 12/29/2015 11:37 PM, Carl Eugen Hoyos wrote: > > Mats Peterson ffmpeg.org> writes: > > > >> Below are links to sample files, which should now be > >> displayed properly with bluish colors > > > > Sorry, I am not a native speaker: > > Could you explain what "should" means in > > above sentence? Please try to answer this question, do not repeat things that all developers already understood when you told us two weeks ago. > > Please trim your quotes, Carl Eugen And please respect this! Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/29/2015 11:48 PM, Carl Eugen Hoyos wrote: Could you explain what "should" means in above sentence? Please try to answer this question, do not repeat things that all developers already understood when you told us two weeks ago. This is a whole different issue from the one I was ranting about before. I already told you what I meant regarding the "should". Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
Mats Peterson ffmpeg.org> writes: > Below are links to sample files, which should now be > displayed properly with bluish colors Just to make sure: Please do not apply this patch until above is clarified. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/29/2015 11:54 PM, Carl Eugen Hoyos wrote: Mats Peterson ffmpeg.org> writes: Below are links to sample files, which should now be displayed properly with bluish colors Just to make sure: Please do not apply this patch until above is clarified. Carl Eugen I'm not the one to apply it. That's Michael's job. If you don't understand what I mean, I can't help you. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] BDA (DTV) Capture / tuning -- work-in-progress
> OK I had this great idea to capture the incoming MPEG2 transport
> stream from the digital TV capture device. Kind of like "raw" stream,
> so FFmpeg could have access to, for instance, all the audio stream.
>
> It appears that the type coming out of the tuner is:
> MEDIATYPE_Stream
> with subtype
> KSDATAFORMAT_SUBTYPE_BDA_MPEG2_TRANSPORT
> which is apparently treated the same as MEDIASUBTYPE_MPEG2_TRANSPORT
> by the built in directshow mpeg demuxer [1] so I assume is some kind
> of typical MPEG2 TS stream.
>
> I had hoped that if I set dshow's AVStream's codec_id to
> codec->codec_id = AV_CODEC_ID_NONE;
> codec->codec_type = AVMEDIA_TYPE_DATA;
Oops that was meant to be
codec->codec_id = AV_CODEC_ID_MPEG2TS;
codec->codec_type = AVMEDIA_TYPE_DATA;
> That it would somehow recognize that I was sending it an MPEG stream
> and insert an appropriate demuxer for me.
>
> However, when I run it, it fails like this:
>
> Input #0, dshow, from 'video=Hauppauge WinTV 885 BDA Tuner/Demod':
> Duration: N/A, bitrate: N/A
> Codec 0x2 is not in the full list.
> Stream #0:0, 0, 1/2700: Data: unknown_codec, 0/1
> Successfully opened the file.
> Output #0, mp4, to 'yo.mp4':
> Output file #0 does not contain any stream
>
> so it's definitely not, for instance, doing a probe or analyze on the
> MPEG stream
>
> Is this possible or any hints/tips/tricks I could possibly use?
To answer my own question, I turned the dshow capture device into both
an AVInputFormat *and* a URLProtocol that "wraps" the AVInputFormat.
Apparently ffmpeg can receive "raw" bytes only from URLProtocol (?)
This way worked great both "segmented" MPEG2VIDEO streams as well as
"raw" MPEG TS streams.
Still a bit unexpected to be necessary, but in case its interesting to
followers, here was the wrapper code (will hopefully be committed to
git trunk soon'ish once I clean it up).
Thanks all.
-roger-
static int dshow_url_open(URLContext *h, const char *filename, int flags)
{
struct dshow_ctx *ctx = h->priv_data;
if (!(ctx->protocol_av_format_context = avformat_alloc_context()))
return AVERROR(ENOMEM);
ctx->protocol_av_format_context->flags = h->flags;
av_strstart(filename, "dshowbda:", &filename); // remove prefix "dshowbda:"
if (filename)
av_strlcpy(ctx->protocol_av_format_context->filename, filename,
1024); // 1024 max bytes
ctx->protocol_av_format_context->iformat = &ff_dshow_demuxer;
ctx->protocol_latest_packet = av_packet_alloc();
ctx->protocol_latest_packet->pos = 0; // default is -1
if (!ctx->protocol_latest_packet)
return AVERROR(ENOMEM);
ctx->protocol_av_format_context->priv_data = ctx; // a bit
circular, but needed to pass through the settings
return dshow_read_header(ctx->protocol_av_format_context);
}
static int dshow_url_read(URLContext *h, uint8_t *buf, int max_size)
{
struct dshow_ctx *ctx = h->priv_data;
int packet_size_or_fail;
int bytes_to_copy;
int bytes_left = ctx->protocol_latest_packet->size -
ctx->protocol_latest_packet->pos;
if (bytes_left == 0) {
av_packet_unref(ctx->protocol_latest_packet);
packet_size_or_fail =
dshow_read_packet(ctx->protocol_av_format_context,
ctx->protocol_latest_packet);
if (packet_size_or_fail < 0)
return packet_size_or_fail;
av_assert0(ctx->protocol_latest_packet->stream_index == 0); //
this should be a stream based, so only one stream, so always index 0
av_assert0(packet_size_or_fail ==
ctx->protocol_latest_packet->size); // should match...
ctx->protocol_latest_packet->pos = 0; // default is -1
bytes_left = ctx->protocol_latest_packet->size -
ctx->protocol_latest_packet->pos;
av_log(h, AV_LOG_VERBOSE, "dshow_url_read read packet of size
%d\n", ctx->protocol_latest_packet->size);
}
bytes_to_copy = FFMIN(bytes_left, max_size);
if (bytes_to_copy != bytes_left)
av_log(h, AV_LOG_DEBUG, "passing partial dshow packet %d >
%d\n", bytes_left, max_size);
memcpy(buf,
&ctx->protocol_latest_packet->data[ctx->protocol_latest_packet->pos],
bytes_to_copy);
ctx->protocol_latest_packet->pos += bytes_to_copy;
av_log(h, AV_LOG_VERBOSE, "dshow_url_read returning %d\n", bytes_to_copy);
return bytes_to_copy;;
}
static int dshow_url_close(URLContext *h)
{
struct dshow_ctx *ctx = h->priv_data;
int ret = dshow_read_close(ctx->protocol_av_format_context);
ctx->protocol_av_format_context->priv_data = NULL; // just in case
it would be freed below
avformat_free_context(ctx->protocol_av_format_context);
av_packet_free(&ctx->protocol_latest_packet); // free wrapper,
also does an unref
return ret;
}
Thanks!
-roger-
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] BDA (DTV) Capture / tuning -- work-in-progress
On Wed, Dec 30, 2015 at 12:23 AM, Roger Pack wrote: >> OK I had this great idea to capture the incoming MPEG2 transport >> stream from the digital TV capture device. Kind of like "raw" stream, >> so FFmpeg could have access to, for instance, all the audio stream. >> >> It appears that the type coming out of the tuner is: >> MEDIATYPE_Stream >> with subtype >> KSDATAFORMAT_SUBTYPE_BDA_MPEG2_TRANSPORT >> which is apparently treated the same as MEDIASUBTYPE_MPEG2_TRANSPORT >> by the built in directshow mpeg demuxer [1] so I assume is some kind >> of typical MPEG2 TS stream. >> >> I had hoped that if I set dshow's AVStream's codec_id to >> codec->codec_id = AV_CODEC_ID_NONE; >> codec->codec_type = AVMEDIA_TYPE_DATA; > > Oops that was meant to be > > codec->codec_id = AV_CODEC_ID_MPEG2TS; > codec->codec_type = AVMEDIA_TYPE_DATA; > >> That it would somehow recognize that I was sending it an MPEG stream >> and insert an appropriate demuxer for me. >> >> However, when I run it, it fails like this: >> >> Input #0, dshow, from 'video=Hauppauge WinTV 885 BDA Tuner/Demod': >> Duration: N/A, bitrate: N/A >> Codec 0x2 is not in the full list. >> Stream #0:0, 0, 1/2700: Data: unknown_codec, 0/1 >> Successfully opened the file. >> Output #0, mp4, to 'yo.mp4': >> Output file #0 does not contain any stream >> >> so it's definitely not, for instance, doing a probe or analyze on the >> MPEG stream >> >> Is this possible or any hints/tips/tricks I could possibly use? > > To answer my own question, I turned the dshow capture device into both > an AVInputFormat *and* a URLProtocol that "wraps" the AVInputFormat. > Apparently ffmpeg can receive "raw" bytes only from URLProtocol (?) > This way worked great both "segmented" MPEG2VIDEO streams as well as > "raw" MPEG TS streams. > You should probably just make it use the mpegts demuxer directly, say like the rtsp demuxer. We have an API for that in the form of avpriv_mpegts_parse_packet Making an input device a protocol seems rather ugly, the user needs to know beforehand how to call it that way. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] BDA (DTV) Capture / tuning -- work-in-progress
On Wed, Dec 30, 2015 at 12:33 AM, Hendrik Leppkes wrote: > On Wed, Dec 30, 2015 at 12:23 AM, Roger Pack wrote: >>> OK I had this great idea to capture the incoming MPEG2 transport >>> stream from the digital TV capture device. Kind of like "raw" stream, >>> so FFmpeg could have access to, for instance, all the audio stream. >>> >>> It appears that the type coming out of the tuner is: >>> MEDIATYPE_Stream >>> with subtype >>> KSDATAFORMAT_SUBTYPE_BDA_MPEG2_TRANSPORT >>> which is apparently treated the same as MEDIASUBTYPE_MPEG2_TRANSPORT >>> by the built in directshow mpeg demuxer [1] so I assume is some kind >>> of typical MPEG2 TS stream. >>> >>> I had hoped that if I set dshow's AVStream's codec_id to >>> codec->codec_id = AV_CODEC_ID_NONE; >>> codec->codec_type = AVMEDIA_TYPE_DATA; >> >> Oops that was meant to be >> >> codec->codec_id = AV_CODEC_ID_MPEG2TS; >> codec->codec_type = AVMEDIA_TYPE_DATA; >> >>> That it would somehow recognize that I was sending it an MPEG stream >>> and insert an appropriate demuxer for me. >>> >>> However, when I run it, it fails like this: >>> >>> Input #0, dshow, from 'video=Hauppauge WinTV 885 BDA Tuner/Demod': >>> Duration: N/A, bitrate: N/A >>> Codec 0x2 is not in the full list. >>> Stream #0:0, 0, 1/2700: Data: unknown_codec, 0/1 >>> Successfully opened the file. >>> Output #0, mp4, to 'yo.mp4': >>> Output file #0 does not contain any stream >>> >>> so it's definitely not, for instance, doing a probe or analyze on the >>> MPEG stream >>> >>> Is this possible or any hints/tips/tricks I could possibly use? >> >> To answer my own question, I turned the dshow capture device into both >> an AVInputFormat *and* a URLProtocol that "wraps" the AVInputFormat. >> Apparently ffmpeg can receive "raw" bytes only from URLProtocol (?) >> This way worked great both "segmented" MPEG2VIDEO streams as well as >> "raw" MPEG TS streams. >> > > You should probably just make it use the mpegts demuxer directly, say > like the rtsp demuxer. > We have an API for that in the form of avpriv_mpegts_parse_packet > > Making an input device a protocol seems rather ugly, the user needs to > know beforehand how to call it that way. > Or as an alternative, why not just add the mpegts demuxer to the DShow graph as well and directly get video/audio streams. For Live TV functionality, the MS demuxer seems to be working quite well. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Tue, Dec 29, 2015 at 05:42:26PM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > On Tue, Dec 29, 2015 at 04:56:26PM +, Carl Eugen Hoyos wrote: > > > Michael Niedermayer niedermayer.cc> writes: > > > > > > > > https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE > > > > > > > > patch applied > > > > > > How exactly did you test this? > > > > it is neccessary to also fix the decoder to test > > (I disagree.) > If that were true, more time should be given for > reviews imo. yes > > > this was a fix for just the demuxer side > > If you cannot test, how do you know it is correct? it works with the now available patch fixing the decoder and the Earth\ Spin\ 1-bit\ qtrle.mov file I dont know if bugs remain, if there are any bugs/issues left, they should be fixed of course ... [...] -- 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] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi,
On 29.12.2015 19:46, Ronald S. Bultje wrote:
> On Tue, Dec 29, 2015 at 11:49 AM, Andreas Cadhalpun <
> andreas.cadhal...@googlemail.com> wrote:
>> Do you have a sample causing overflows in the vp9 decoder?
>
>
> Nope, but I'm going to assume they're not hard to create, just use a few
> high same-sign quantized coefficients (e.g. SHORT_MAX) and a high quantizer
> (qidx=255, dq_ac=1828). Both dequant into int16_t (1828*SHORT_MAX doesn't
> fit in 16 bits) as well as the idct results themselves (because you're
> adding and subtracting from an already near-edge value which should fit in
> 16bits) will overflow.
I guess hard depends on who you ask. I've fuzzed the vp9 decoder and
haven't yet come across a sample that triggers a signed integer overflow there.
In the case of this daala decoder, 3/4 of the fuzzed samples do...
>> (Overflows in dsp code are typically not a security concern.)
>>
>> Well, the overflows in the imdct calculation of the aac_fixed decoder
>> ultimately
>> caused crashes.
>
>
> I would prefer if for video codecs, unless specifically proven otherwise
> for that codec with specific content, we assume by default that overflows
> in performance-sensitive dsp code (specifically idct) are OK. ubsan may not
> like us on fuzzed content, but so be it.
I prefer that input for such performance-sensitive dsp code gets sanitized,
so that overflows are (nearly) impossible.
The daala decoder uses fixed point approximations for multiplication
with floats like:
/* 13573/32768 ~= Tan[pi/8] ~= 0.414213562373095 */ \
t0 += (t1*13573 + 16384) >> 15; \
For these multiplications with up to 2^15 to fit into int32_t the input must
be in the +/- 2^16 range, so it should be clipped to that range, e.g. with:
--- a/libavcodec/daaladec.c
+++ b/libavcodec/daaladec.c
@@ -393,6 +393,19 @@ static inline void decode_block_pvq(DaalaContext *s, int
x, int y, int p,
daala_coding_to_raster(&d[boffset], aw, pred, n);
+for (int i = 0; i < 1 << (bsize + 2); i += 1) {
+for (int j = 0; j < 1 << (bsize + 2); j += 1) {
+int idx = boffset + i * aw + j;
+if (s->dcoef[p][idx] > UINT16_MAX) {
+av_log(NULL, AV_LOG_WARNING, "dcoef %d too large\n",
s->dcoef[p][idx]);
+s->dcoef[p][idx] = UINT16_MAX;
+} else if (s->dcoef[p][idx] < -UINT16_MAX) {
+av_log(NULL, AV_LOG_WARNING, "dcoef %d too small\n",
s->dcoef[p][idx]);
+s->dcoef[p][idx] = -UINT16_MAX;
+}
+}
+}
+
/* IDCT */
if (s->dsp.idct[bsize])
s->dsp.idct[bsize]((uint8_t *)(s->ccoef[p] + boffset), aw,
This prevents most of the overflows in the idct and prints warnings instead of
silent failure. If these happen with valid input, the idct has to be changed,
and otherwise the clipping should be harmless.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
On 29.12.2015 22:27, Rostislav Pehlivanov wrote:
> oggparsetheora has the same bit of code to read the gpshift, so it would
> probably be a good idea to add it to this patch as well.
No, oggparsetheora only reads 5 bits for gpshift.
The only thing from this patch that also applies there is the (theoretical)
issue of 1<<31 not being defined for int32_t.
On 29.12.2015 22:32, Hendrik Leppkes wrote:
> 1U << hdr->gpshift?
Sure. Updated patch attached.
Best regards,
Andreas
>From 4380123388f38eb9bbd11db34b0ac82a9ec18d5a Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun
Date: Tue, 29 Dec 2015 18:32:01 +0100
Subject: [PATCH] oggparsedaala: reject too large gpshift
Also use a unsigned constant for the shift calculation, as 1 << 31 is
undefined for int32_t. This is also fixed oggparsetheora.
This fixes ubsan runtime error: shift exponent is too large for
32-bit type 'int'
Signed-off-by: Andreas Cadhalpun
---
libavformat/oggparsedaala.c | 7 ++-
libavformat/oggparsetheora.c | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/libavformat/oggparsedaala.c b/libavformat/oggparsedaala.c
index 24567f9..3651ca1 100644
--- a/libavformat/oggparsedaala.c
+++ b/libavformat/oggparsedaala.c
@@ -123,7 +123,12 @@ static int daala_header(AVFormatContext *s, int idx)
hdr->frame_duration = bytestream2_get_ne32(&gb);
hdr->gpshift = bytestream2_get_byte(&gb);
-hdr->gpmask = (1 << hdr->gpshift) - 1;
+if (hdr->gpshift >= 32) {
+av_log(s, AV_LOG_ERROR, "Too large gpshift %d (>= 32).\n",
+ hdr->gpshift);
+return AVERROR_INVALIDDATA;
+}
+hdr->gpmask = (1U << hdr->gpshift) - 1;
hdr->format.depth = 8 + 2*(bytestream2_get_byte(&gb)-1);
diff --git a/libavformat/oggparsetheora.c b/libavformat/oggparsetheora.c
index 6e6a362..5f057c3 100644
--- a/libavformat/oggparsetheora.c
+++ b/libavformat/oggparsetheora.c
@@ -108,7 +108,7 @@ static int theora_header(AVFormatContext *s, int idx)
skip_bits(&gb, 2);
thp->gpshift = get_bits(&gb, 5);
-thp->gpmask = (1 << thp->gpshift) - 1;
+thp->gpmask = (1U << thp->gpshift) - 1;
st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
st->codec->codec_id = AV_CODEC_ID_THEORA;
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > > If you cannot test, how do you know it is correct? > > it works with the now available patch fixing the decoder and the > Earth\ Spin\ 1-bit\ qtrle.mov > file Sorry, it seems I am very slow today: What does "it works" mean here? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
On Tue, Dec 29, 2015 at 4:00 PM, Andreas Cadhalpun wrote: > On 29.12.2015 22:27, Rostislav Pehlivanov wrote: >> oggparsetheora has the same bit of code to read the gpshift, so it would >> probably be a good idea to add it to this patch as well. > > No, oggparsetheora only reads 5 bits for gpshift. > The only thing from this patch that also applies there is the (theoretical) > issue of 1<<31 not being defined for int32_t. Can you clarify precisely what you mean by this? I am pretty sure ubsan and others do fail for 1<<31, and I know that it is undefined behavior. Are you saying that it is impossible to trigger a 1<<31, or only that it is highly improbable? [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Wed, Dec 30, 2015 at 12:00:20AM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > > If you cannot test, how do you know it is correct? > > > > it works with the now available patch fixing the decoder and the > > Earth\ Spin\ 1-bit\ qtrle.mov > > file > > Sorry, it seems I am very slow today: > What does "it works" mean here? its blue not black and white [] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On 29.12.2015 22:34, Rostislav Pehlivanov wrote: >> This is an infinite loop that can hang the decoder. > Will try to get the upstream to accept a fix. Thanks. >> overflows in the entropy decoding system > The assertions should be able to catch them, so they should be fine. Are you talking about av_assert0 here? None of them was triggered, which is good, as triggering an assert is usually much worse than an overflow. > They'll trigger if there's a desync with the bitstream. I might add more in > case they're not enough. I don't think that more asserts will fix the crash caused by one of these overflows. That has to be fixed differently. >> What is the point of having DaalaSharedContext in addition to > DaalaBitstreamHeader? > I was looking at VP9 when starting to write this and it had this structure. > I'll remove the shared context one. I see. >> Third, the decoder seems to return only the first frame, repeatedly, >> instead of the following frames. > You did read that it can only decode I-frames (keyframes). So you need to > encode your files with -k 1 to make all frames keyframes. I must have missed that, but I hope this decoder will support other frame types in the future. ;) >> First, this fails building with --enable-shared, because > daala_find_p_format >> is not static. > Fixed yesterday, but sent the same patch twice by mistake. It's been fixed, > but I'd like to post a v3 of the patch to address some of the issues found. OK. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavc/dsd_tablegen: speed up table generation
On Tue, Dec 29, 2015 at 08:56:48AM -0800, Ganesh Ajjanagadde wrote: > Tables are bit identical. > Sample benchmark (Haswell, GNU/Linux+gcc): > old: > 814485 decicycles in dsd_ctables_tableinit, 512 runs, 0 skips > > new: > 356808 decicycles in dsd_ctable_tableinit, 512 runs, 0 skips > > Binary size should essentially be identical, and is in fact identical on > the configuration I tested on. > > Signed-off-by: Ganesh Ajjanagadde > --- > libavcodec/dsd_tablegen.h | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) probably ok [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > > On Wed, Dec 30, 2015 at 12:00:20AM +, Carl Eugen Hoyos wrote: > > Michael Niedermayer niedermayer.cc> writes: > > > > > > If you cannot test, how do you know it is correct? > > > > > > it works with the now available patch fixing the decoder and the > > > Earth\ Spin\ 1-bit\ qtrle.mov > > > file > > > > Sorry, it seems I am very slow today: > > What does "it works" mean here? > > its blue not black and white I tested the following: $ git checkout b0e23f2a37b45e8fa7e9ae91301d0ef572e80181 $ wget http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151229/c12907af/attachment.bin $ md5sum attachment.bin 1656289c65333dab470046c3dc96 attachment.bin $ patch -p1http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
On 30.12.2015 01:04, Ganesh Ajjanagadde wrote: > On Tue, Dec 29, 2015 at 4:00 PM, Andreas Cadhalpun > wrote: >> On 29.12.2015 22:27, Rostislav Pehlivanov wrote: >>> oggparsetheora has the same bit of code to read the gpshift, so it would >>> probably be a good idea to add it to this patch as well. >> >> No, oggparsetheora only reads 5 bits for gpshift. >> The only thing from this patch that also applies there is the (theoretical) >> issue of 1<<31 not being defined for int32_t. > > Can you clarify precisely what you mean by this? I am pretty sure > ubsan and others do fail for 1<<31, and I know that it is undefined > behavior. Are you saying that it is impossible to trigger a 1<<31, or > only that it is highly improbable? Yes, ubsan fails for 1<<31 and it is theoretically undefined behavior, but it works in practice in contrast to e.g. 1<<40, which just doesn't fit. That's what I meant. The 1<<31 case can be triggered in both. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] oggparsedaala: reject too large gpshift
On Tue, Dec 29, 2015 at 4:14 PM, Andreas Cadhalpun wrote: > On 30.12.2015 01:04, Ganesh Ajjanagadde wrote: >> On Tue, Dec 29, 2015 at 4:00 PM, Andreas Cadhalpun >> wrote: >>> On 29.12.2015 22:27, Rostislav Pehlivanov wrote: oggparsetheora has the same bit of code to read the gpshift, so it would probably be a good idea to add it to this patch as well. >>> >>> No, oggparsetheora only reads 5 bits for gpshift. >>> The only thing from this patch that also applies there is the (theoretical) >>> issue of 1<<31 not being defined for int32_t. >> >> Can you clarify precisely what you mean by this? I am pretty sure >> ubsan and others do fail for 1<<31, and I know that it is undefined >> behavior. Are you saying that it is impossible to trigger a 1<<31, or >> only that it is highly improbable? > > Yes, ubsan fails for 1<<31 and it is theoretically undefined behavior, but > it works in practice in contrast to e.g. 1<<40, which just doesn't fit. > That's what I meant. The 1<<31 case can be triggered in both. Ok. I still deem 1<<31 as worthy of fixing, as anyway it is easy to do. > > Best regards, > Andreas > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Wed, Dec 30, 2015 at 12:12:31AM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > > > On Wed, Dec 30, 2015 at 12:00:20AM +, Carl Eugen Hoyos wrote: > > > Michael Niedermayer niedermayer.cc> writes: > > > > > > > > If you cannot test, how do you know it is correct? > > > > > > > > it works with the now available patch fixing the decoder and the > > > > Earth\ Spin\ 1-bit\ qtrle.mov > > > > file > > > > > > Sorry, it seems I am very slow today: > > > What does "it works" mean here? > > > > its blue not black and white > > I tested the following: > $ git checkout b0e23f2a37b45e8fa7e9ae91301d0ef572e80181 > $ wget > http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151229/c12907af/attachment.bin > $ md5sum attachment.bin > 1656289c65333dab470046c3dc96 attachment.bin > $ patch -p1 $ ./configure && make ffmpeg > $ ./ffmpeg -i Earth\ Spin\ 1-bit\ qtrle.mov out.avi > > The output looks black & white here with MPlayer > and vlc, what do I miss? then theres a bug somewhere i tested with ffplay, and there it works [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavf/concatdec: do not access packet if av_read_frame returned error
Signed-off-by: Marton Balint
---
libavformat/concatdec.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index d21805f..d226e15 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -555,9 +555,7 @@ static int concat_read_packet(AVFormatContext *avf,
AVPacket *pkt)
while (1) {
ret = av_read_frame(cat->avf, pkt);
-if (ret == AVERROR_EOF || packet_after_outpoint(cat, pkt)) {
-if (ret == 0)
-av_packet_unref(pkt);
+if (ret == AVERROR_EOF) {
if ((ret = open_next_file(avf)) < 0)
return ret;
continue;
@@ -568,6 +566,12 @@ static int concat_read_packet(AVFormatContext *avf,
AVPacket *pkt)
av_packet_unref(pkt);
return ret;
}
+if (packet_after_outpoint(cat, pkt)) {
+av_packet_unref(pkt);
+if ((ret = open_next_file(avf)) < 0)
+return ret;
+continue;
+}
cs = &cat->cur_file->streams[pkt->stream_index];
if (cs->out_stream_index < 0) {
av_packet_unref(pkt);
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] lavf/concatdec: add outpoint_interleave_delay option
Wait at most outpoint_interleave_delay at outpoint before considering it an
end of file condition.
Signed-off-by: Marton Balint
---
doc/demuxers.texi | 20
libavformat/concatdec.c | 13 ++---
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index fb1e4fb..b178195 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -137,8 +137,7 @@ may overlap between two concatenated files.
@item @code{outpoint @var{timestamp}}
Out point of the file. When the demuxer reaches the specified decoding
-timestamp in any of the streams, it handles it as an end of file condition and
-skips the current and all the remaining packets from all streams.
+timestamp it drops the current and all the remaining packets in the stream.
Out point is exclusive, which means that the demuxer will not output packets
with a decoding timestamp greater or equal to Out point.
@@ -148,12 +147,25 @@ are tightly interleaved. For non-intra frame codecs you
will usually get
additional packets with presentation timestamp after Out point therefore the
decoded content will most likely contain frames after Out point too. If your
streams are not tightly interleaved you may not get all the packets from all
-streams before Out point and you may only will be able to decode the earliest
-stream until Out point.
+streams before Out point, see @code{outpoint_interleave_delay}.
The duration of the files (if not specified by the @code{duration}
directive) will be reduced based on their specified Out point.
+@item @code{outpoint_interleave_delay @var{dur}}
+With this directive you can specify how long the demuxer should wait before it
+assumes that packets with timestamps before outpoint have appeared form all
+streams, so there is no need to read the remaining packets from the input.
+
+If an Out point is specified and a packet timestamp is greater or equal to
+@code{outpoint} plus the specified @code{outpoint_interleave_delay} in any of
+the streams, the demuxer handles it as an end of file condition and skips the
+current and all the remaining packets from all streams.
+
+The default value is 0. If your streams are not tightly interleaved, you should
+increase this, otherwise you will only be able to decode the earliest stream
+until Out point.
+
@item @code{file_packet_metadata @var{key=value}}
Metadata of the packets of the file. The specified metadata will be set for
each file packet. You can specify this directive multiple times to add multiple
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index d226e15..afa822d 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -48,6 +48,7 @@ typedef struct {
int64_t inpoint;
int64_t outpoint;
AVDictionary *metadata;
+int64_t outpoint_interleave_delay;
int nb_streams;
} ConcatFile;
@@ -378,7 +379,7 @@ static int concat_read_header(AVFormatContext *avf)
}
if ((ret = add_file(avf, filename, &file, &nb_files_alloc)) < 0)
goto fail;
-} else if (!strcmp(keyword, "duration") || !strcmp(keyword, "inpoint")
|| !strcmp(keyword, "outpoint")) {
+} else if (!strcmp(keyword, "duration") || !strcmp(keyword, "inpoint")
|| !strcmp(keyword, "outpoint") || !strcmp(keyword,
"outpoint_interleave_delay")) {
char *dur_str = get_keyword(&cursor);
int64_t dur;
if (!file) {
@@ -397,6 +398,8 @@ static int concat_read_header(AVFormatContext *avf)
file->inpoint = dur;
else if (!strcmp(keyword, "outpoint"))
file->outpoint = dur;
+else if (!strcmp(keyword, "outpoint_interleave_delay"))
+file->outpoint_interleave_delay = dur;
} else if (!strcmp(keyword, "file_packet_metadata")) {
char *metadata;
if (!file) {
@@ -567,9 +570,13 @@ static int concat_read_packet(AVFormatContext *avf,
AVPacket *pkt)
return ret;
}
if (packet_after_outpoint(cat, pkt)) {
+int after_max_delay = av_compare_ts(pkt->dts,
cat->avf->streams[pkt->stream_index]->time_base,
+cat->cur_file->outpoint +
cat->cur_file->outpoint_interleave_delay, AV_TIME_BASE_Q) >= 0;
av_packet_unref(pkt);
-if ((ret = open_next_file(avf)) < 0)
-return ret;
+if (after_max_delay) {
+if ((ret = open_next_file(avf)) < 0)
+return ret;
+}
continue;
}
cs = &cat->cur_file->streams[pkt->stream_index];
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > i tested with ffplay, and there it works I tested the following: $ git checkout b0e23f2a37b45e8fa7e9ae91301d0ef572e80181 $ wget http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151229/c12907af/attachment.bin $ md5sum attachment.bin 1656289c65333dab470046c3dc96 attachment.bin $ patch -p1http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
Hi, On Tue, Dec 29, 2015 at 6:54 PM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > Hi, > > On 29.12.2015 19:46, Ronald S. Bultje wrote: > > On Tue, Dec 29, 2015 at 11:49 AM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > >> Do you have a sample causing overflows in the vp9 decoder? > > > > > > Nope, but I'm going to assume they're not hard to create, just use a few > > high same-sign quantized coefficients (e.g. SHORT_MAX) and a high > quantizer > > (qidx=255, dq_ac=1828). Both dequant into int16_t (1828*SHORT_MAX doesn't > > fit in 16 bits) as well as the idct results themselves (because you're > > adding and subtracting from an already near-edge value which should fit > in > > 16bits) will overflow. > > I guess hard depends on who you ask. I've fuzzed the vp9 decoder and > haven't yet come across a sample that triggers a signed integer overflow > there. > In the case of this daala decoder, 3/4 of the fuzzed samples do... > > >> (Overflows in dsp code are typically not a security concern.) > >> > >> Well, the overflows in the imdct calculation of the aac_fixed decoder > >> ultimately > >> caused crashes. > > > > > > I would prefer if for video codecs, unless specifically proven otherwise > > for that codec with specific content, we assume by default that overflows > > in performance-sensitive dsp code (specifically idct) are OK. ubsan may > not > > like us on fuzzed content, but so be it. > > I prefer that input for such performance-sensitive dsp code gets sanitized, > so that overflows are (nearly) impossible. I don't think it makes sense to make the decoder 5% slower just so that ubsan is happy. We don't do that for other video decoders either. And yes, the coefficient decoding loop is typically a major hotspot in video codecs. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On Tue, Dec 29, 2015 at 5:09 PM, Ronald S. Bultje wrote: > Hi, > > On Tue, Dec 29, 2015 at 6:54 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: > >> Hi, >> >> On 29.12.2015 19:46, Ronald S. Bultje wrote: >> > On Tue, Dec 29, 2015 at 11:49 AM, Andreas Cadhalpun < >> > andreas.cadhal...@googlemail.com> wrote: >> >> Do you have a sample causing overflows in the vp9 decoder? >> > >> > >> > Nope, but I'm going to assume they're not hard to create, just use a few >> > high same-sign quantized coefficients (e.g. SHORT_MAX) and a high >> quantizer >> > (qidx=255, dq_ac=1828). Both dequant into int16_t (1828*SHORT_MAX doesn't >> > fit in 16 bits) as well as the idct results themselves (because you're >> > adding and subtracting from an already near-edge value which should fit >> in >> > 16bits) will overflow. >> >> I guess hard depends on who you ask. I've fuzzed the vp9 decoder and >> haven't yet come across a sample that triggers a signed integer overflow >> there. >> In the case of this daala decoder, 3/4 of the fuzzed samples do... >> >> >> (Overflows in dsp code are typically not a security concern.) >> >> >> >> Well, the overflows in the imdct calculation of the aac_fixed decoder >> >> ultimately >> >> caused crashes. >> > >> > >> > I would prefer if for video codecs, unless specifically proven otherwise >> > for that codec with specific content, we assume by default that overflows >> > in performance-sensitive dsp code (specifically idct) are OK. ubsan may >> not >> > like us on fuzzed content, but so be it. >> >> I prefer that input for such performance-sensitive dsp code gets sanitized, >> so that overflows are (nearly) impossible. > > > I don't think it makes sense to make the decoder 5% slower just so that > ubsan is happy. We don't do that for other video decoders either. And yes, > the coefficient decoding loop is typically a major hotspot in video codecs. Basically, I think we need to strike a very fine balance here, and I offer my own thoughts on this below: 1. On the one hand, it is difficult to reason how undefined behavior in a tight loop can impact other places in the code. It may or may not be safe in a practical context. If things can be changed to be safe at negligible performance impact, it should be done as a defensive programming measure IMHO. Theoretically, undefined means that anything can happen, but practically the range of possibilities is far more limited (no one runs ubsan in production). We already do not take into account all theoretical aspects: for example, nothing dictates the choice of the IEEE-754 floating point format, but some code does assume it in FFmpeg. As such, we are anyway operating in a "practical" context, and theoretical purity must be balanced with practical impact, something I forget myself a lot. 2. If things can't be transformed to be safe without leading to significant loss of performance, such things must be clearly documented as comments, fully justified, and very carefully scrutinized IMHO. 3. Note that integer overflow is a hard problem, with endless debate all round: https://lwn.net/Articles/547649/, https://news.ycombinator.com/item?id=8765714, https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md - even Rust essentially cops out by default on release builds. The only thing is that they took more responsibility regarding the semantics than the C standard - although C leaves it undefined, in practice Rust and C will generate the same code here AFAIK (note: I have not studied Rust, just casually read about it). The real problem comes in what clients do with the outputs of such overflow: in Rust for instance array bounds derived from such arithmetic will be checked, in C it will lead to big trouble. Then again, the funny thing is: just having "safety" is not enough, the burden is anyway on the programmer to handle such "weird" cases via some form of checking code, regardless of the language, or careful thought to ensure that such things don't happen (e.g the naive (min+max)/2 vs min + (max-min)/2 for binary search). No language can do the "right thing" automatically: the "right thing" varies depending on the situation. See the next point. 4. Even with integer overflow checks, note that the method of handling can vary significantly, as can be seen in FFmpeg. It is not easy to audit every line, and create brittle checks that make hard to understand code even harder to understand. Wish there were alternatives, but I see none: some things are inherently not simple. > > Ronald > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On Wed, Dec 30, 2015 at 12:26:30AM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > i tested with ffplay, and there it works > > I tested the following: > $ git checkout b0e23f2a37b45e8fa7e9ae91301d0ef572e80181 > $ wget > http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151229/c12907af/attachment.bin > $ md5sum attachment.bin > 1656289c65333dab470046c3dc96 attachment.bin > $ patch -p1 $ ./configure && make ffplay > $ ffplay Earth\ Spin\ 1-bit\ qtrle.mov > > Looks black and white here, what do I miss? i dont know, i tried the same commands and its blue unless you mixed ffplay and ./ffplay or the .mov differs: md5sum Earth\ Spin\ 1-bit\ qtrle.mov 2a73988ea94a1201a9cf477d71d3ff29 Earth Spin 1-bit qtrle.mov [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB DNS cache poisoning attacks, popular search engine, Google internet authority dont be evil, please signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC 2/3] daala_parser: add a Daala parser
On 12/28/2015 1:58 PM, Rostislav Pehlivanov wrote: > Not much to say about this other than "it works". > If this parser is needed to get the muxing code from patch 3/3 working then it may be a good idea to commit the relevant functions from the decoder with this first, assuming what it needs from daala_entropy.h doesn't represent half the decoder in question and is properly reviewed, since i predict there will be several rounds of reviews for the decoder and having the muxer committed in the meantime would make things easier. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] configure: make gcc 2 detection more robust
Solves an issue that will get triggered when gcc 20 rolls in.
Found-by: Hendrik Leppkes
Signed-off-by: Ganesh Ajjanagadde
---
configure | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 4bf72a6..76dcf38 100755
--- a/configure
+++ b/configure
@@ -3700,7 +3700,8 @@ probe_cc(){
gcc_ext_ver=$(expr "$gcc_version" : ".*$gcc_pkg_ver $gcc_basever
\\(.*\\)")
_ident=$(cleanws "gcc $gcc_basever $gcc_pkg_ver $gcc_ext_ver")
case $gcc_basever in
-2*) ;;
+2) ;;
+2.*) ;;
*) _depflags='-MMD -MF $(@:.o=.d) -MT $@' ;;
esac
if [ "$first" = true ]; then
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
I agree with BBB (Ronald), this project claims to have the fastest decoders available and we shouldn't sacrifice performance. Since basically the only input of the decoder goes through the entropy decoding system occasionally checking for overflows in a few performance sensitive parts there is worth it, as it has been pointed out. Also I'd like to remind people that this is an RFC and WIP. Overflow checking and fuzzing aren't high priority yet. I wanted to know if I was doing something horribly wrong, typos, naive code, asserts/crashes on normal files, glitches, etc. On 30 December 2015 at 02:12, Ganesh Ajjanagadde wrote: > On Tue, Dec 29, 2015 at 5:09 PM, Ronald S. Bultje > wrote: > > Hi, > > > > On Tue, Dec 29, 2015 at 6:54 PM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > > > >> Hi, > >> > >> On 29.12.2015 19:46, Ronald S. Bultje wrote: > >> > On Tue, Dec 29, 2015 at 11:49 AM, Andreas Cadhalpun < > >> > andreas.cadhal...@googlemail.com> wrote: > >> >> Do you have a sample causing overflows in the vp9 decoder? > >> > > >> > > >> > Nope, but I'm going to assume they're not hard to create, just use a > few > >> > high same-sign quantized coefficients (e.g. SHORT_MAX) and a high > >> quantizer > >> > (qidx=255, dq_ac=1828). Both dequant into int16_t (1828*SHORT_MAX > doesn't > >> > fit in 16 bits) as well as the idct results themselves (because you're > >> > adding and subtracting from an already near-edge value which should > fit > >> in > >> > 16bits) will overflow. > >> > >> I guess hard depends on who you ask. I've fuzzed the vp9 decoder and > >> haven't yet come across a sample that triggers a signed integer overflow > >> there. > >> In the case of this daala decoder, 3/4 of the fuzzed samples do... > >> > >> >> (Overflows in dsp code are typically not a security concern.) > >> >> > >> >> Well, the overflows in the imdct calculation of the aac_fixed decoder > >> >> ultimately > >> >> caused crashes. > >> > > >> > > >> > I would prefer if for video codecs, unless specifically proven > otherwise > >> > for that codec with specific content, we assume by default that > overflows > >> > in performance-sensitive dsp code (specifically idct) are OK. ubsan > may > >> not > >> > like us on fuzzed content, but so be it. > >> > >> I prefer that input for such performance-sensitive dsp code gets > sanitized, > >> so that overflows are (nearly) impossible. > > > > > > I don't think it makes sense to make the decoder 5% slower just so that > > ubsan is happy. We don't do that for other video decoders either. And > yes, > > the coefficient decoding loop is typically a major hotspot in video > codecs. > > Basically, I think we need to strike a very fine balance here, and I > offer my own thoughts on this below: > 1. On the one hand, it is difficult to reason how undefined behavior > in a tight loop can impact other places in the code. It may or may not > be safe in a practical context. If things can be changed to be safe at > negligible performance impact, it should be done as a defensive > programming measure IMHO. Theoretically, undefined means that anything > can happen, but practically the range of possibilities is far more > limited (no one runs ubsan in production). We already do not take into > account all theoretical aspects: for example, nothing dictates the > choice of the IEEE-754 floating point format, but some code does > assume it in FFmpeg. As such, we are anyway operating in a "practical" > context, and theoretical purity must be balanced with practical > impact, something I forget myself a lot. > > 2. If things can't be transformed to be safe without leading to > significant loss of performance, such things must be clearly > documented as comments, fully justified, and very carefully > scrutinized IMHO. > > 3. Note that integer overflow is a hard problem, with endless debate > all round: https://lwn.net/Articles/547649/, > https://news.ycombinator.com/item?id=8765714, > https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md > - even Rust essentially cops out by default on release builds. The > only thing is that they took more responsibility regarding the > semantics than the C standard - although C leaves it undefined, in > practice Rust and C will generate the same code here AFAIK (note: I > have not studied Rust, just casually read about it). The real problem > comes in what clients do with the outputs of such overflow: in Rust > for instance array bounds derived from such arithmetic will be > checked, in C it will lead to big trouble. Then again, the funny thing > is: just having "safety" is not enough, the burden is anyway on the > programmer to handle such "weird" cases via some form of checking > code, regardless of the language, or careful thought to ensure that > such things don't happen (e.g the naive (min+max)/2 vs min + > (max-min)/2 for binary search). No language can do the "right thing" > automatically: the "right thing" varies depending on
Re: [FFmpeg-devel] [RFC v2 3/3] daaladec: Implement a native Daala decoder
On Tue, Dec 29, 2015 at 7:13 PM, Rostislav Pehlivanov wrote: > I agree with BBB (Ronald), this project claims to have the fastest decoders > available and we shouldn't sacrifice performance. Since basically the only > input of the decoder goes through the entropy decoding system occasionally > checking for overflows in a few performance sensitive parts there is worth > it, as it has been pointed out. > Also I'd like to remind people that this is an RFC and WIP. Overflow > checking and fuzzing aren't high priority yet. I wanted to know if I was > doing something horribly wrong, typos, naive code, asserts/crashes on > normal files, glitches, etc. Definitely. I was just making some general remarks on the never ending security/performance debate; they were not meant for your patch per se. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
Michael Niedermayer niedermayer.cc> writes: > md5sum Earth\ Spin\ 1-bit\ qtrle.mov > 2a73988ea94a1201a9cf477d71d3ff29 Earth Spin 1-bit qtrle.mov Mats has indeed decided to offer different files with the same name for download. I suggest we test the original unchanged file. (Although it's of course hard to say which one it actually is.) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] ffmpeg: replace log2 by faster variant
The log is anyway rounded to an integer, so one may use an frexp
based approach. Note that this may be made frexpf; if arguments are less than
2^24 there is no loss. Kept as double precision for simplicity; 2^32 is
exactly representable as a double.
Signed-off-by: Ganesh Ajjanagadde
---
ffmpeg.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/ffmpeg.c b/ffmpeg.c
index 6d01987..ee72f91 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1486,6 +1486,17 @@ static void print_final_stats(int64_t total_size)
}
}
+static inline int log2i(double d)
+{
+int exp;
+double mant;
+
+mant = frexp(d, &exp);
+if (mant >= M_SQRT1_2)
+return exp;
+return exp-1;
+}
+
static void print_report(int is_last_report, int64_t timer_start, int64_t
cur_time)
{
char buf[1024];
@@ -1559,7 +1570,7 @@ static void print_report(int is_last_report, int64_t
timer_start, int64_t cur_ti
if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
qp_histogram[qp]++;
for (j = 0; j < 32; j++)
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
"%X", (int)lrintf(log2(qp_histogram[j] + 1)));
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
"%X", log2i(qp_histogram[j] + 1));
}
if ((enc->flags & AV_CODEC_FLAG_PSNR) && (ost->pict_type !=
AV_PICTURE_TYPE_NONE || is_last_report)) {
--
2.6.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 12:43 AM, Michael Niedermayer wrote: it works with the now available patch fixing the decoder and the Earth\ Spin\ 1-bit\ qtrle.mov file I dont know if bugs remain, if there are any bugs/issues left, they should be fixed of course ... I don't know what he's doing on his side. Both you and I can confirm that it's working. Whether it's good or bad to "promote" 1-bit to pal8 is another story. But it works. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 01:26 AM, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: i tested with ffplay, and there it works I tested the following: $ git checkout b0e23f2a37b45e8fa7e9ae91301d0ef572e80181 $ wget http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151229/c12907af/attachment.bin $ md5sum attachment.bin 1656289c65333dab470046c3dc96 attachment.bin $ patch -p1http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Carl, perhaps you missed to compile MPlayer using the very latest FFmpeg? Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 05:19 AM, Mats Peterson wrote: Carl, perhaps you missed to compile MPlayer using the very latest FFmpeg? Mats Forget that suggestion, I thought the patch was applied already. In any case, I have patched qtrle.c in both FFmpeg and MPlayer here, and the rotating earth movie is blue in both cases. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 04:38 AM, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: md5sum Earth\ Spin\ 1-bit\ qtrle.mov 2a73988ea94a1201a9cf477d71d3ff29 Earth Spin 1-bit qtrle.mov Mats has indeed decided to offer different files with the same name for download. I suggest we test the original unchanged file. (Although it's of course hard to say which one it actually is.) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel It's as easy as I have run the .mov through mkvmerge to create a Matroska file for reference. Any problems with that? ;) Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/af_anequalizer: remove cabs, cexp dependencies
On 30 December 2015 at 03:15, Ganesh Ajjanagadde wrote: > On Mon, Dec 28, 2015 at 10:27 PM, Matt Oliver > wrote: > > On 29 December 2015 at 11:51, Ganesh Ajjanagadde > > > wrote: > > > >> On Mon, Dec 28, 2015 at 8:22 AM, Paul B Mahol wrote: > >> > On 12/27/15, Ganesh Ajjanagadde wrote: > >> >> Replaces by real arithmetic. Tested the validity of these > >> transformations > >> >> separately. > >> >> > >> >> Signed-off-by: Ganesh Ajjanagadde > >> >> --- > >> >> configure| 1 - > >> >> libavfilter/af_anequalizer.c | 25 + > >> >> 2 files changed, 17 insertions(+), 9 deletions(-) > >> >> > >> > > >> > If those are mathematically equivalent ok. > >> > >> mathematically equivalent, tested offline, numerical differences of > >> the order of ~ 1e-15 or so, which is perfectly fine IMHO for double > >> precision especially since it is a priori not clear which is more > >> precise. Pushed, thanks. > > > > > > Wouldn't it also be a good idea to remove the COMPLEX_FUNCS list from > > configure as now neither cabs or cexp are used so configure can be > > simplified by removing the checks for them. > > I asked Paul about this (when I removed them from some other filter). > That time he said he wanted them for another filter (anequalizer) > since the expressions were complicated. Judging from this commit, I > think that was a fair assessment. > > In short, this is really a question for Paul (or others who use > complex numbers): if there are plans on doing something with complex > numbers that is not trivial to rewrite, it may be helpful not to > remove as you suggest. If not, then it is perhaps ok. > > Personally, over the long term, I think there will be further > instances of complex number usage. If we remove it now, I suspect it > will get added back in at some point. As such, I won't write a patch > for this; really upto others especially Paul. > > Maybe long run we can create an av_complex or something like that. A > few problems are (among possibly many others): > 1. Syntactic sugar for *, / , +, - will be lost, and one will need to > create something like cmult, cdiv, and call at an increased verbosity > cost. In C++, these basic operators can be overloaded (but then again > stdc++ has a complex header); AFAIK there is no such mechanism that we > can use in FFmpeg's "least common denominator" C. > 2. Natural promotion rules will be lost, e.g suppose one passes a > double to a complex function, say while multiplying a double complex > by a double as in anequalizer. With proper complex number support, > promotion rules kick in and everything works. With av_complex, this > does not work, and explicit setting of imaginary part to 0 will be > needed. Fair enough, I was just checking. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 05:25 AM, Mats Peterson wrote: On 12/30/2015 04:38 AM, Carl Eugen Hoyos wrote: Michael Niedermayer niedermayer.cc> writes: md5sum Earth\ Spin\ 1-bit\ qtrle.mov 2a73988ea94a1201a9cf477d71d3ff29 Earth Spin 1-bit qtrle.mov Mats has indeed decided to offer different files with the same name for download. I suggest we test the original unchanged file. (Although it's of course hard to say which one it actually is.) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel It's as easy as I have run the .mov through mkvmerge to create a Matroska file for reference. Any problems with that? ;) Mats The original .mov file has the grayscale bit set in the bit depth field of the video sample description, but since that seemed highly illogical to me, I cleared it for the purposes of this demonstration. For some reason, QuickTime in Win or Mac seems to ignore the grayscale bit, since it will display the file in blue colors regardless. Perhaps we should do the same? This is not documented behaviour, though. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavf/qtpalette: Treat 1-bit video as palettized
On 12/30/2015 05:47 AM, Mats Peterson wrote: The original .mov file has the grayscale bit set in the bit depth field of the video sample description, but since that seemed highly illogical to me, I cleared it for the purposes of this demonstration. For some reason, QuickTime in Win or Mac seems to ignore the grayscale bit, since it will display the file in blue colors regardless. Perhaps we should do the same? This is not documented behaviour, though. Mats The QuickTime File Format Specification says this: "Values of 34, 36, and 40 indicate 2-, 4-, and 8-bit grayscale, respectively, for grayscale images." They don't mention anything about value 33 (0x21), which is 1-bit video (0x01) with the grayscale bit (0x20) set. Perhaps we should just ignore the grayscale bit when processing 1-bit video then? Suggestions welcome. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/qtpalette: Ignore greyscale bit in 1-bit video
The QuickTime File Format Specification states the following:
"Depth: A 16-bit integer that indicates the pixel depth of the
compressed image. Values of 1, 2, 4, 8 ,16, 24, and 32 indicate the
depth of color images. The value 32 should be used only if the image
contains an alpha channel. Values of 34, 36, and 40 indicate 2-, 4-, and
8-bit grayscale, respectively, for grayscale images."
There is no mention of value 33, i.e. 1-bit video (0x01) with the
greyscale bit (0x20) set. I therefore suggest that we ignore the
greyscale bit when processing 1-bit video. Another reason to do this is
that the sample file below will be displayed properly with blue colors
in QuickTime in Windows or Mac *in spite of* the greyscale bit being
set.
Sample file:
https://drive.google.com/open?id=0B3_pEBoLs0faTThSek1EeXQ0ZHM
Mats
--
Mats Peterson
http://matsp888.no-ip.org/~mats/
>From 60798a04ef1204dadf5395cbf7205ea3957fafbb Mon Sep 17 00:00:00 2001
From: Mats Peterson
Date: Wed, 30 Dec 2015 07:47:54 +0100
Subject: [PATCH] lavf/qtpalette: Ignore greyscale bit in 1-bit video
The QuickTime File Format Specification states the following:
"Depth: A 16-bit integer that indicates the pixel depth of the
compressed image. Values of 1, 2, 4, 8 ,16, 24, and 32 indicate the
depth of color images. The value 32 should be used only if the image
contains an alpha channel. Values of 34, 36, and 40 indicate 2-, 4-, and
8-bit grayscale, respectively, for grayscale images."
There is no mention of value 33, i.e. 1-bit video (0x01) with the
greyscale bit (0x20) set. I therefore suggest that we ignore the
greyscale bit when processing 1-bit video. Another reason to do this is
that the sample file below will be displayed properly with blue colors
in QuickTime in Windows or Mac *in spite of* the greyscale bit being
set.
Sample file:
https://drive.google.com/open?id=0B3_pEBoLs0faTThSek1EeXQ0ZHM
Mats
---
libavformat/qtpalette.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c
index 6544a55..0555605 100644
--- a/libavformat/qtpalette.c
+++ b/libavformat/qtpalette.c
@@ -51,7 +51,7 @@ int ff_get_qtpalette(int codec_id, AVIOContext *pb, uint32_t *palette)
int color_count, color_start, color_end;
uint32_t a, r, g, b;
-if (greyscale) {
+if (greyscale && bit_depth > 1) {
int color_index, color_dec;
/* compute the greyscale palette */
color_count = 1 << bit_depth;
--
1.7.10.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/29/2015 11:08 PM, Mats Peterson wrote: The following patch fixes the lack of palettized display of 1-bit video in the qtrle decoder. It is related to my previous patch of lavf/qtpalette, which added 1-bit video to the "palettized video" category. As far as I can see, everything works fine, but comments are of course welcome. Below are links to sample files, which should now be displayed properly with bluish colors, but which were previously displayed in black & white. Matroska: https://drive.google.com/open?id=0B3_pEBoLs0faNjI0cHBMWDhYY2c QuickTime (mov): https://drive.google.com/open?id=0B3_pEBoLs0faUlItWm9KaGJSTEE Mats ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel When comparing the output in QuickTime and FFmpeg, I noticed that the background is blue in FFmpeg, while it is white (or transparent, if you will) in QuickTime. Hold your horses on this patch. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/qtrle: Use AV_PIX_FMT_PAL8 for 1-bit video
On 12/30/2015 08:17 AM, Mats Peterson wrote: When comparing the output in QuickTime and FFmpeg, I noticed that the background is blue in FFmpeg, while it is white (or transparent, if you will) in QuickTime. Hold your horses on this patch. Mats I noticed that the "alpha" component (there is NOTHING mentioned about this component in the description of the color table in the QuickTime File Format Specification, for the record) is set to "1" in the second color in the color table. Just wanted to mention that. Mats -- Mats Peterson http://matsp888.no-ip.org/~mats/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
