Re: [FFmpeg-devel] [PATCH] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
On Thu, Dec 15, 2016 at 12:31 AM, Matthew Wolenetz wrote: > Some toolchains failed to link a dynamic library containing wavdec.c, > but with either CONFIG_SPDIF_DEMUXER or CONFIG_W64_DEMUXER disabled. > This change adds #if's to explicitly exclude code rather than depend on > toolchain code elision of same condition using "if". > Reference https://crbug.com/591845. Adapted from 2 Chromium ffmpeg > patches for code style: > This seems like a rather odd issue to be toolchain specific. Can you please provide a configure command line that would result in a broken build? - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/6] genh: prevent overflow during block alignment calculation
On 12/15/16, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/genh.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/genh.c b/libavformat/genh.c > index b683e02..5684352 100644 > --- a/libavformat/genh.c > +++ b/libavformat/genh.c > @@ -74,6 +74,11 @@ static int genh_read_header(AVFormatContext *s) > case 0: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_PSX;break; > case 1: > case 11: st->codecpar->bits_per_coded_sample = 4; > + if (st->codecpar->channels > INT_MAX / 36) { > +av_log(s, AV_LOG_ERROR, "Overflow during block alignment > calculation 36 * %d\n", > + st->codecpar->channels); > +return AVERROR_INVALIDDATA; > + } > st->codecpar->block_align = 36 * st->codecpar->channels; > st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_WAV;break; > case 2: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_DTK;break; > -- > 2.10.2 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > AFAIK codec supports only 1 or 2 channels and nothing more, but patch if fine anyway. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/6] ircamdec: prevent overflow during block alignment, calculation
On 12/15/16, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/ircamdec.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/ircamdec.c b/libavformat/ircamdec.c > index 59f3a49..3efc2b4 100644 > --- a/libavformat/ircamdec.c > +++ b/libavformat/ircamdec.c > @@ -96,6 +96,11 @@ static int ircam_read_header(AVFormatContext *s) > } > > st->codecpar->bits_per_coded_sample = > av_get_bits_per_sample(st->codecpar->codec_id); > +if (st->codecpar->channels && st->codecpar->bits_per_coded_sample > > INT_MAX / st->codecpar->channels) { > +av_log(s, AV_LOG_ERROR, "Overflow during block alignment > calculation %d * %d\n", > + st->codecpar->bits_per_coded_sample, > st->codecpar->channels); > +return AVERROR_INVALIDDATA; > +} > st->codecpar->block_align = st->codecpar->bits_per_coded_sample * > st->codecpar->channels / 8; > avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); > avio_skip(s->pb, 1008); > -- > 2.10.2 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ok ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
2016-12-15 0:31 GMT+01:00 Matthew Wolenetz : > Some toolchains failed to link a dynamic library containing wavdec.c, > but with either CONFIG_SPDIF_DEMUXER or CONFIG_W64_DEMUXER disabled. DCE is needed for FFmpeg compilation, this should get fixed by Steve Lhomme's configure patch. I find it hard to understand why wavdec.c would be the only file showing this issue... Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/6] nistspheredec: prevent overflow during block alignment, calculation
On 12/15/16, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/nistspheredec.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c > index 782d1df..9472e47 100644 > --- a/libavformat/nistspheredec.c > +++ b/libavformat/nistspheredec.c > @@ -80,6 +80,11 @@ static int nist_read_header(AVFormatContext *s) > > avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); > > +if (st->codecpar->channels && > st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) { > +av_log(s, AV_LOG_ERROR, "Overflow during block alignment > calculation %d * %d\n", > + st->codecpar->bits_per_coded_sample, > st->codecpar->channels); > +return AVERROR_INVALIDDATA; > +} > st->codecpar->block_align = st->codecpar->bits_per_coded_sample > * st->codecpar->channels / 8; > > if (avio_tell(s->pb) > header_size) > -- > 2.10.2 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ok ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/af_atempo: fix assertion failure on empty input
On Wed, 14 Dec 2016, Pavel Koshevoy wrote: On Wed, Dec 14, 2016 at 7:27 PM, Marton Balint wrote: Signed-off-by: Marton Balint --- libavfilter/af_atempo.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) I'd like to understand these changes a little better ... how can I reproduce the problem this is trying to fix? Basically it happens on empty input: ffmpeg -f s16be -i /dev/null -af atempo output.wav Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/udp: Enable FIFO when using windows sockets.
Le duodi 22 frimaire, an CCXXV, Matt Oliver a écrit : > --- > libavformat/udp.c | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/libavformat/udp.c b/libavformat/udp.c > index f8c861d..3cafb32 100644 > --- a/libavformat/udp.c > +++ b/libavformat/udp.c > @@ -64,6 +64,14 @@ > #define HAVE_PTHREAD_CANCEL 0 > #endif > > +#if HAVE_THREADS && HAVE_WINSOCK2_H > +/* Winsock2 recv function can be unblocked by shutting down the socket */ > +#define pthread_setcancelstate(x, y) > +#define pthread_cancel > +#undef HAVE_PTHREAD_CANCEL > +#define HAVE_PTHREAD_CANCEL 1 > +#endif > + > #if HAVE_PTHREAD_CANCEL > #include "libavutil/thread.h" > #endif > @@ -526,6 +534,8 @@ static void *circular_buffer_task_rx( void *_URLContext) > goto end; > } > continue; > +} else if (len == 0) { > +goto end; Unfortunately, UDP packets of size 0 exist and are returned to the application. If len == 0 is the only criterion to detect a read interrupted by shutdown(), it will not be usable for UDP. You can still combine with your original idea of an atomic flag, though. > } > AV_WL32(s->tmp, len); > > @@ -1144,8 +1154,13 @@ static int udp_close(URLContext *h) > if (s->thread_started) { > int ret; > // Cancel only read, as write has been signaled as success to the > user > -if (h->flags & AVIO_FLAG_READ) > +if (h->flags & AVIO_FLAG_READ) { > +# if HAVE_THREADS && HAVE_WINSOCK2_H > +shutdown(s->udp_fd, SD_BOTH); > +# else > pthread_cancel(s->circular_buffer_thread); > +# endif > +} > ret = pthread_join(s->circular_buffer_thread, NULL); > if (ret != 0) > av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret)); 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] lavf/vsrc_testsrc: fix SMPTE segfault with small output size
The memset (line 1336) in draw_bar is passed a negative size if the output width is less than 36 pixels Signed-off-by: Josh de Kock --- libavfilter/vsrc_testsrc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavfilter/vsrc_testsrc.c b/libavfilter/vsrc_testsrc.c index 08f6e07..9f2e91f 100644 --- a/libavfilter/vsrc_testsrc.c +++ b/libavfilter/vsrc_testsrc.c @@ -1413,6 +1413,11 @@ static av_cold int smptebars_init(AVFilterContext *ctx) { TestSourceContext *test = ctx->priv; +if (test->w < 36 || test->h < 1) { +av_log(ctx, AV_LOG_FATAL, "Size should be 36x1 or larger (%dx%d).\n", test->w, test->h); +return AVERROR(EINVAL); +} + test->fill_picture_fn = smptebars_fill_picture; test->draw_once = 1; return init(ctx); -- 2.10.1 (Apple Git-78) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
Hi, On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > On 14.12.2016 02:46, Ronald S. Bultje wrote: > > Not wanting to discourage you, but I wonder if there's really a point to > > this...? > > These patches are prerequisites for enforcing the validity of codec > parameters [1]. > > > I don't see how the user experience changes. > > Users won't see ffmpeg claiming nonsense bit rates like -1184293205235990 > kb/s > anymore. I don't think you understand my question. - does this belong in 4xm.c? (Or in generic code? Or in the app?) - should it return an error? (Or just clip parameters? Or ignore the invalid value?) > This isn't specifically intended at this patch, but rather at the sort of > > rabbit hole this change might lead to, > > I have a pretty good map of this rabbit hole (i.e. lots of samples > triggering > UBSan errors) and one day I might try to dig it up, but for now I'm > limiting > myself to the codec parameters. I'm not saying mplayer was great, but one of the principles I believe we always copied from them was to try to play files to the best of our abilities and not error out at the first convenience. This isn't just a theoretical thing, a lot of people credited mplayer with playing utterly broken AVI files that probably even ffmpeg rejects. What ffmpeg added on top of that is to make a project maintainable by not being an utter crapshoot. This isn't 4xm.c-specific, this is a general philosophical question: - should we error out? - should this be in generic code if we're likely to repeat such checks all over the place? > which would cause the code to be uber-full of such checks, none of which > > really have any significance. But maybe others disagree... > > Not relying on undefined behavior is a significant improvement. And doing > these checks consequently where the values are set makes it possible > for other code to rely on their validity without further checks. Unless "UB" leads to actual bad behaviour, I personally don't necessarily care. I'm very scared that when you go beyond codec parameters, and you want to check for overflows all over the place, we'll never see the end of it... I'd appreciate if others could chime in here, I don't want to carry this argument all by myself if nobody cares. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 6/6] pvfdec: prevent overflow during block alignment, calculation
Hi, On Wed, Dec 14, 2016 at 8:19 PM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/pvfdec.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/pvfdec.c b/libavformat/pvfdec.c > index b9f6d4f..5eecc22 100644 > --- a/libavformat/pvfdec.c > +++ b/libavformat/pvfdec.c > @@ -56,6 +56,11 @@ static int pvf_read_header(AVFormatContext *s) > st->codecpar->sample_rate = sample_rate; > st->codecpar->codec_id= ff_get_pcm_codec_id(bps, 0, 1, 0x); > st->codecpar->bits_per_coded_sample = bps; > +if (bps > INT_MAX / st->codecpar->channels) { > +av_log(s, AV_LOG_ERROR, "Overflow during block alignment > calculation %d * %d\n", > + bps, st->codecpar->channels); > +return AVERROR_INVALIDDATA; > +} And this is what I meant. Please stop. No. No. No. No. No. Not in codec code. Add these checks in generic code if you care about the outcome, but please don't make each codec a crapshoot like this. Please. From a maintenance point of view, that's a much better approach. Please stop for a second and think about my point of view here. I beg you. Please. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
On Thu, Dec 15, 2016 at 08:02:52AM -0500, Ronald S. Bultje wrote: > Hi, > > On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: > > > On 14.12.2016 02:46, Ronald S. Bultje wrote: > > > Not wanting to discourage you, but I wonder if there's really a point to > > > this...? > > > > These patches are prerequisites for enforcing the validity of codec > > parameters [1]. > > > > > I don't see how the user experience changes. > > > > Users won't see ffmpeg claiming nonsense bit rates like -1184293205235990 > > kb/s > > anymore. > > > I don't think you understand my question. > > - does this belong in 4xm.c? (Or in generic code? Or in the app?) > - should it return an error? (Or just clip parameters? Or ignore the > invalid value?) > > > This isn't specifically intended at this patch, but rather at the sort of > > > rabbit hole this change might lead to, > > > > I have a pretty good map of this rabbit hole (i.e. lots of samples > > triggering > > UBSan errors) and one day I might try to dig it up, but for now I'm > > limiting > > myself to the codec parameters. > > > I'm not saying mplayer was great, but one of the principles I believe we > always copied from them was to try to play files to the best of our > abilities and not error out at the first convenience. This isn't just a > theoretical thing, a lot of people credited mplayer with playing utterly > broken AVI files that probably even ffmpeg rejects. What ffmpeg added on > top of that is to make a project maintainable by not being an utter > crapshoot. > > This isn't 4xm.c-specific, this is a general philosophical question: > - should we error out? > - should this be in generic code if we're likely to repeat such checks all > over the place? > > > which would cause the code to be uber-full of such checks, none of which > > > really have any significance. But maybe others disagree... > > > > Not relying on undefined behavior is a significant improvement. And doing > > these checks consequently where the values are set makes it possible > > for other code to rely on their validity without further checks. > > > Unless "UB" leads to actual bad behaviour, I personally don't necessarily > care. I'm very scared that when you go beyond codec parameters, and you > want to check for overflows all over the place, we'll never see the end of > it... > > I'd appreciate if others could chime in here, I don't want to carry this > argument all by myself if nobody cares. as you are asking for others oppinion valid C code must not trigger undefined behavior undefined behavior means anything can happen and this is not guranteed to be limited to the value a operation produces. if you write a*b the compiler can assume that a and b are small enough for this not to overflow, it can use this to simplify checks before and after the operation. A silly example would be a*b/b != a (with signed a and b) which any good compiler should replace by 0 We have seen bugs from undefined behavior like strict aliassing violations were the compiler assumed that things wouldnt be accessed as they had a different type ... I think we should fix all instances of UB. If we find cases that we cannot fix without slowdown or without making the code hard to maintain by the people who activly work on it then iam not against making exceptions. But in my oppinion in general UB should be fixed [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/vsrc_testsrc: fix SMPTE segfault with small output size
On 12/15/16, Josh de Kock wrote: > The memset (line 1336) in draw_bar is passed a negative size if the output > width is less than 36 pixels > > Signed-off-by: Josh de Kock > --- > libavfilter/vsrc_testsrc.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavfilter/vsrc_testsrc.c b/libavfilter/vsrc_testsrc.c > index 08f6e07..9f2e91f 100644 > --- a/libavfilter/vsrc_testsrc.c > +++ b/libavfilter/vsrc_testsrc.c > @@ -1413,6 +1413,11 @@ static av_cold int smptebars_init(AVFilterContext > *ctx) > { > TestSourceContext *test = ctx->priv; > > +if (test->w < 36 || test->h < 1) { > +av_log(ctx, AV_LOG_FATAL, "Size should be 36x1 or larger > (%dx%d).\n", test->w, test->h); > +return AVERROR(EINVAL); > +} > + > test->fill_picture_fn = smptebars_fill_picture; > test->draw_once = 1; > return init(ctx); > -- > 2.10.1 (Apple Git-78) > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Fixed differently. Thanks for reporting bug. smptehdbars filter uses same function so it had similar bug. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024
On Wed, Dec 14, 2016 at 10:05:50PM -0700, pkoshe...@gmail.com wrote: > From: Pavel Koshevoy > > --- > libavcodec/utils.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 44ecc09..2ad96e4 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -2788,8 +2788,6 @@ static int do_decode(AVCodecContext *avctx, AVPacket > *pkt) > if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { > ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); > -if (ret >= 0) > -ret = pkt->size; > } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { > ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); This breaks fate --- ./tests/ref/fate/rscc-32bit 2016-12-15 04:39:33.924670759 +0100 +++ tests/data/fate/rscc-32bit 2016-12-15 15:59:04.507407670 +0100 @@ -3,11 +3,3 @@ #codec_id 0: rawvideo #dimensions 0: 320x240 #sar 0: 0/1 -0, 0, 0,1, 307200, 0xe36c2c38 -0, 1, 1,1, 307200, 0xa2e3476b -0, 2, 2,1, 307200, 0x65167c95 -0, 3, 3,1, 307200, 0x6acd02ac -0, 4, 4,1, 307200, 0x37935e0b -0, 5, 5,1, 307200, 0x8e6918bc -0, 6, 6,1, 307200, 0x0a246578 -0, 7, 7,1, 307200, 0x9c1a2f4c Test rscc-32bit failed. Look at tests/data/fate/rscc-32bit.err for details. make: *** [fate-rscc-32bit] Error 137 make: *** Waiting for unfinished jobs --- ./tests/ref/fate/rscc-16bit 2016-12-15 04:39:33.924670759 +0100 +++ tests/data/fate/rscc-16bit 2016-12-15 15:59:04.491407670 +0100 @@ -3,18 +3,3 @@ #codec_id 0: rawvideo #dimensions 0: 320x240 #sar 0: 0/1 -0, 0, 0,1, 153600, 0x33759daf -0, 1, 1,1, 153600, 0x43e0c910 -0, 2, 2,1, 153600, 0x6c75a8f0 -0, 3, 3,1, 153600, 0xaace1255 -0, 4, 4,1, 153600, 0x42d3f439 -0, 5, 5,1, 153600, 0xb225b396 -0, 6, 6,1, 153600, 0xa615221c -0, 7, 7,1, 153600, 0x5401f5a9 -0, 8, 8,1, 153600, 0xd274cef3 -0, 9, 9,1, 153600, 0x7935f992 -0, 10, 10,1, 153600, 0x7669e7c6 -0, 11, 11,1, 153600, 0x527bd6a1 -0, 12, 12,1, 153600, 0x0f33b2ed -0, 13, 13,1, 153600, 0xe62ddb32 -0, 14, 14,1, 153600, 0x81541aa2 Test rscc-16bit failed. Look at tests/data/fate/rscc-16bit.err for details. make: *** [fate-rscc-16bit] Error 137 --- ./tests/ref/fate/iscc 2016-12-15 04:39:33.920670759 +0100 +++ tests/data/fate/iscc2016-12-15 15:59:04.479407669 +0100 @@ -3,8 +3,3 @@ #codec_id 0: rawvideo #dimensions 0: 1760x968 #sar 0: 0/1 -0, 0, 0,1, 6814720, 0x1365f8ef -0, 1, 1,1, 6814720, 0x90838983 -0, 2, 2,1, 6814720, 0xf0cc3131 -0, 3, 3,1, 6814720, 0xc07e404d -0, 4, 4,1, 6814720, 0x945962dd Test iscc failed. Look at tests/data/fate/iscc.err for details. make: *** [fate-iscc] Error 137 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate/psd : add test for psd decoder
2016-11-25 0:17 GMT+01:00 Michael Niedermayer : > On Thu, Nov 24, 2016 at 09:43:38PM +0100, Martin Vignali wrote: > > Hello, > > > > In attach patch to add fate test for the psd decoder > > > > Sample can be found here : > > https://we.tl/KvRaABCsdY > > uploaded > > [...] > > > Ping for the patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024
On 12/15/16, Michael Niedermayer wrote: > On Wed, Dec 14, 2016 at 10:05:50PM -0700, pkoshe...@gmail.com wrote: >> From: Pavel Koshevoy >> >> --- >> libavcodec/utils.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >> index 44ecc09..2ad96e4 100644 >> --- a/libavcodec/utils.c >> +++ b/libavcodec/utils.c >> @@ -2788,8 +2788,6 @@ static int do_decode(AVCodecContext *avctx, AVPacket >> *pkt) >> if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { >> ret = avcodec_decode_video2(avctx, >> avctx->internal->buffer_frame, >> &got_frame, pkt); >> -if (ret >= 0) >> -ret = pkt->size; >> } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { >> ret = avcodec_decode_audio4(avctx, >> avctx->internal->buffer_frame, >> &got_frame, pkt); > > This breaks fate > > --- ./tests/ref/fate/rscc-32bit 2016-12-15 04:39:33.924670759 +0100 > +++ tests/data/fate/rscc-32bit 2016-12-15 15:59:04.507407670 +0100 > @@ -3,11 +3,3 @@ > #codec_id 0: rawvideo > #dimensions 0: 320x240 > #sar 0: 0/1 > -0, 0, 0,1, 307200, 0xe36c2c38 > -0, 1, 1,1, 307200, 0xa2e3476b > -0, 2, 2,1, 307200, 0x65167c95 > -0, 3, 3,1, 307200, 0x6acd02ac > -0, 4, 4,1, 307200, 0x37935e0b > -0, 5, 5,1, 307200, 0x8e6918bc > -0, 6, 6,1, 307200, 0x0a246578 > -0, 7, 7,1, 307200, 0x9c1a2f4c > Test rscc-32bit failed. Look at tests/data/fate/rscc-32bit.err for details. > make: *** [fate-rscc-32bit] Error 137 > make: *** Waiting for unfinished jobs > --- ./tests/ref/fate/rscc-16bit 2016-12-15 04:39:33.924670759 +0100 > +++ tests/data/fate/rscc-16bit 2016-12-15 15:59:04.491407670 +0100 > @@ -3,18 +3,3 @@ > #codec_id 0: rawvideo > #dimensions 0: 320x240 > #sar 0: 0/1 > -0, 0, 0,1, 153600, 0x33759daf > -0, 1, 1,1, 153600, 0x43e0c910 > -0, 2, 2,1, 153600, 0x6c75a8f0 > -0, 3, 3,1, 153600, 0xaace1255 > -0, 4, 4,1, 153600, 0x42d3f439 > -0, 5, 5,1, 153600, 0xb225b396 > -0, 6, 6,1, 153600, 0xa615221c > -0, 7, 7,1, 153600, 0x5401f5a9 > -0, 8, 8,1, 153600, 0xd274cef3 > -0, 9, 9,1, 153600, 0x7935f992 > -0, 10, 10,1, 153600, 0x7669e7c6 > -0, 11, 11,1, 153600, 0x527bd6a1 > -0, 12, 12,1, 153600, 0x0f33b2ed > -0, 13, 13,1, 153600, 0xe62ddb32 > -0, 14, 14,1, 153600, 0x81541aa2 > Test rscc-16bit failed. Look at tests/data/fate/rscc-16bit.err for details. > make: *** [fate-rscc-16bit] Error 137 > --- ./tests/ref/fate/iscc 2016-12-15 04:39:33.920670759 +0100 > +++ tests/data/fate/iscc2016-12-15 15:59:04.479407669 +0100 > @@ -3,8 +3,3 @@ > #codec_id 0: rawvideo > #dimensions 0: 1760x968 > #sar 0: 0/1 > -0, 0, 0,1, 6814720, 0x1365f8ef > -0, 1, 1,1, 6814720, 0x90838983 > -0, 2, 2,1, 6814720, 0xf0cc3131 > -0, 3, 3,1, 6814720, 0xc07e404d > -0, 4, 4,1, 6814720, 0x945962dd > Test iscc failed. Look at tests/data/fate/iscc.err for details. > make: *** [fate-iscc] Error 137 > > [...] Those are bugs in decoders. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/6] 4xm: prevent overflow during block alignment calculation
On Thu, Dec 15, 2016 at 02:18:17AM +0100, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/4xm.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/4xm.c b/libavformat/4xm.c > index 2758b69..308d889 100644 > --- a/libavformat/4xm.c > +++ b/libavformat/4xm.c > @@ -187,6 +187,11 @@ static int parse_strk(AVFormatContext *s, > st->codecpar->bit_rate = (int64_t)st->codecpar->channels * >st->codecpar->sample_rate * > > st->codecpar->bits_per_coded_sample; > +if (st->codecpar->channels && st->codecpar->bits_per_coded_sample > > INT_MAX / st->codecpar->channels) { > +av_log(s, AV_LOG_ERROR, "Overflow during block alignment calculation > %d * %d\n", > + st->codecpar->channels, st->codecpar->bits_per_coded_sample); > +return AVERROR_INVALIDDATA; > +} > st->codecpar->block_align = st->codecpar->channels * > > st->codecpar->bits_per_coded_sample; should be ok, alternatively the parameters could be limited tighter, they would need to be unrealistically large for an overflow thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024
On Thu, Dec 15, 2016 at 4:07 PM, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: >> On Wed, Dec 14, 2016 at 10:05:50PM -0700, pkoshe...@gmail.com wrote: >>> From: Pavel Koshevoy >>> >>> --- >>> libavcodec/utils.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >>> index 44ecc09..2ad96e4 100644 >>> --- a/libavcodec/utils.c >>> +++ b/libavcodec/utils.c >>> @@ -2788,8 +2788,6 @@ static int do_decode(AVCodecContext *avctx, AVPacket >>> *pkt) >>> if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { >>> ret = avcodec_decode_video2(avctx, >>> avctx->internal->buffer_frame, >>> &got_frame, pkt); >>> -if (ret >= 0) >>> -ret = pkt->size; >>> } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { >>> ret = avcodec_decode_audio4(avctx, >>> avctx->internal->buffer_frame, >>> &got_frame, pkt); >> >> This breaks fate >> >> --- ./tests/ref/fate/rscc-32bit 2016-12-15 04:39:33.924670759 +0100 >> +++ tests/data/fate/rscc-32bit 2016-12-15 15:59:04.507407670 +0100 >> @@ -3,11 +3,3 @@ >> #codec_id 0: rawvideo >> #dimensions 0: 320x240 >> #sar 0: 0/1 >> -0, 0, 0,1, 307200, 0xe36c2c38 >> -0, 1, 1,1, 307200, 0xa2e3476b >> -0, 2, 2,1, 307200, 0x65167c95 >> -0, 3, 3,1, 307200, 0x6acd02ac >> -0, 4, 4,1, 307200, 0x37935e0b >> -0, 5, 5,1, 307200, 0x8e6918bc >> -0, 6, 6,1, 307200, 0x0a246578 >> -0, 7, 7,1, 307200, 0x9c1a2f4c >> Test rscc-32bit failed. Look at tests/data/fate/rscc-32bit.err for details. >> make: *** [fate-rscc-32bit] Error 137 >> make: *** Waiting for unfinished jobs >> --- ./tests/ref/fate/rscc-16bit 2016-12-15 04:39:33.924670759 +0100 >> +++ tests/data/fate/rscc-16bit 2016-12-15 15:59:04.491407670 +0100 >> @@ -3,18 +3,3 @@ >> #codec_id 0: rawvideo >> #dimensions 0: 320x240 >> #sar 0: 0/1 >> -0, 0, 0,1, 153600, 0x33759daf >> -0, 1, 1,1, 153600, 0x43e0c910 >> -0, 2, 2,1, 153600, 0x6c75a8f0 >> -0, 3, 3,1, 153600, 0xaace1255 >> -0, 4, 4,1, 153600, 0x42d3f439 >> -0, 5, 5,1, 153600, 0xb225b396 >> -0, 6, 6,1, 153600, 0xa615221c >> -0, 7, 7,1, 153600, 0x5401f5a9 >> -0, 8, 8,1, 153600, 0xd274cef3 >> -0, 9, 9,1, 153600, 0x7935f992 >> -0, 10, 10,1, 153600, 0x7669e7c6 >> -0, 11, 11,1, 153600, 0x527bd6a1 >> -0, 12, 12,1, 153600, 0x0f33b2ed >> -0, 13, 13,1, 153600, 0xe62ddb32 >> -0, 14, 14,1, 153600, 0x81541aa2 >> Test rscc-16bit failed. Look at tests/data/fate/rscc-16bit.err for details. >> make: *** [fate-rscc-16bit] Error 137 >> --- ./tests/ref/fate/iscc 2016-12-15 04:39:33.920670759 +0100 >> +++ tests/data/fate/iscc2016-12-15 15:59:04.479407669 +0100 >> @@ -3,8 +3,3 @@ >> #codec_id 0: rawvideo >> #dimensions 0: 1760x968 >> #sar 0: 0/1 >> -0, 0, 0,1, 6814720, 0x1365f8ef >> -0, 1, 1,1, 6814720, 0x90838983 >> -0, 2, 2,1, 6814720, 0xf0cc3131 >> -0, 3, 3,1, 6814720, 0xc07e404d >> -0, 4, 4,1, 6814720, 0x945962dd >> Test iscc failed. Look at tests/data/fate/iscc.err for details. >> make: *** [fate-iscc] Error 137 >> >> [...] > > Those are bugs in decoders. Technically yes, however it has always been assumed that video decoders consumer entire packets - with the exception of truncated mode. Personally I think we should be deprecating truncated mode, the new API cleanly specifies always consuming full packets anyway. In the meantime if this should be fixed, it should check for the flag being set and then change behavior, because only truncate-aware video decoders in truncate mode should really ever consume less then the full packet. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavc: Fix ticket 6024 (v2)
From: Pavel Koshevoy --- libavcodec/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 44ecc09..be50459 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -2788,7 +2788,7 @@ static int do_decode(AVCodecContext *avctx, AVPacket *pkt) if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, &got_frame, pkt); -if (ret >= 0) +if (ret >= 0 && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) ret = pkt->size; } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, -- 2.6.6 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024
On Wed, 14 Dec 2016 22:05:50 -0700 pkoshe...@gmail.com wrote: > From: Pavel Koshevoy > > --- > libavcodec/utils.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 44ecc09..2ad96e4 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -2788,8 +2788,6 @@ static int do_decode(AVCodecContext *avctx, AVPacket > *pkt) > if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { > ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); > -if (ret >= 0) > -ret = pkt->size; > } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { > ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); This is not correct. Also your commit message/mail subject line contains absolutely no information (unless you go out of your way to look at the issue, work through all the verbose crap, and draw your own conclusion for what this patch _might_ have been needed). Write an informative commit message why this patch is needed and why this is the right fix for whatever it fixes. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024 (v2)
On Thu, 15 Dec 2016 09:49:36 -0700 pkoshe...@gmail.com wrote: > From: Pavel Koshevoy > > --- > libavcodec/utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 44ecc09..be50459 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -2788,7 +2788,7 @@ static int do_decode(AVCodecContext *avctx, AVPacket > *pkt) > if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { > ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); > -if (ret >= 0) > +if (ret >= 0 && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) > ret = pkt->size; > } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { > ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, Still lacks commit message. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavc: Fix ticket 6024, truncated mode decoding
From: Pavel Koshevoy The assumption that avcodec_send_packet makes regarding decoders consuming the entire packet is not true if the codec supports truncated decoding mode and the truncated flag is turned on. Steps to reproduce: ./ffmpeg_g -flags truncated \ -i "http://samples.ffmpeg.org/MPEG2/test-ebu-422.4.pakets.ts"; \ -c:v ffv1 -c:a copy -y /tmp/truncated.nut --- libavcodec/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 44ecc09..be50459 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -2788,7 +2788,7 @@ static int do_decode(AVCodecContext *avctx, AVPacket *pkt) if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, &got_frame, pkt); -if (ret >= 0) +if (ret >= 0 && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) ret = pkt->size; } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, -- 2.6.6 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc: Fix ticket 6024, truncated mode decoding
passes fate, but I don't do that often so someone might want to double-check Pavel. On Thu, Dec 15, 2016 at 10:19 AM, wrote: > From: Pavel Koshevoy > > The assumption that avcodec_send_packet makes regarding decoders > consuming the entire packet is not true if the codec supports > truncated decoding mode and the truncated flag is turned on. > > Steps to reproduce: > ./ffmpeg_g -flags truncated \ > -i "http://samples.ffmpeg.org/MPEG2/test-ebu-422.4.pakets.ts"; \ > -c:v ffv1 -c:a copy -y /tmp/truncated.nut > --- > libavcodec/utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 44ecc09..be50459 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -2788,7 +2788,7 @@ static int do_decode(AVCodecContext *avctx, AVPacket > *pkt) > if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) { > ret = avcodec_decode_video2(avctx, avctx->internal->buffer_frame, > &got_frame, pkt); > -if (ret >= 0) > +if (ret >= 0 && !(avctx->flags & AV_CODEC_FLAG_TRUNCATED)) > ret = pkt->size; > } else if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) { > ret = avcodec_decode_audio4(avctx, avctx->internal->buffer_frame, > -- > 2.6.6 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/libopusdec.c Fix ff_vorbis_channel_layouts OOB
Ah, you're right. My fix was based on a slightly earlier version that didn't yet have your fix in it. Disregard my patch. Matt On Wed, Dec 14, 2016 at 5:43 PM, Andreas Cadhalpun < andreas.cadhal...@googlemail.com> wrote: > On 15.12.2016 00:39, Matthew Wolenetz wrote: > > From 141e56ccf7fc56646424484d357b6c74a486d2e2 Mon Sep 17 00:00:00 2001 > > From: Matt Wolenetz > > Date: Mon, 21 Nov 2016 17:30:50 -0800 > > Subject: [PATCH] lavc/libopusdec.c Fix ff_vorbis_channel_layouts OOB > > > > Similar to existing lavc/vorbisdec.c code which first checks that > > avc->channels is valid for accessing ff_vorbis_channel_layouts, this > > change adds protection to libopusdec.c to prevent accessing that > > array with a negative index. Reference https://crbug.com/666794. > > --- > > libavcodec/libopusdec.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c > > index acc62f1..c2c7adc 100644 > > --- a/libavcodec/libopusdec.c > > +++ b/libavcodec/libopusdec.c > > @@ -50,6 +50,10 @@ static av_cold int libopus_decode_init(AVCodecContext > *avc) > > avc->sample_rate= 48000; > > avc->sample_fmt = avc->request_sample_fmt == AV_SAMPLE_FMT_FLT ? > >AV_SAMPLE_FMT_FLT : AV_SAMPLE_FMT_S16; > > +if (avc->channels <= 0) { > > +av_log(avc, AV_LOG_ERROR, "Invalid number of channels\n"); > > +return AVERROR(EINVAL); > > +} > > avc->channel_layout = avc->channels > 8 ? 0 : > >ff_vorbis_channel_layouts[avc->channels - 1]; > > > > What version of ffmpeg is this based on? > > I'm pretty sure I fixed this issue with commit > 8c8f543b81aa2b50bb6a6cfd370a0061281492a3. > > 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] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
> > This seems like a rather odd issue to be toolchain specific. Can you > please provide a configure command line that would result in a broken > build? I believe the link-time failures occurred with configs like these: IIRC, on Mac, Chromium build hit this with: - https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/mac/x64/config.h?rcl=0&l=4 On Windows, one or both of the following hit this: - https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/ia32/config.h?rcl=0&l=4 - https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/x64/config.h?rcl=0&l=4 DCE is needed for FFmpeg compilation, this should get fixed by Steve > Lhomme's > configure patch. > I find it hard to understand why wavdec.c would be the only file > showing this issue... Notably, windows still hit this (VS2015 Update 3 19.00.24213.1) *even with -O2.* On Thu, Dec 15, 2016 at 1:06 AM, Carl Eugen Hoyos wrote: > 2016-12-15 0:31 GMT+01:00 Matthew Wolenetz : > > Some toolchains failed to link a dynamic library containing wavdec.c, > > but with either CONFIG_SPDIF_DEMUXER or CONFIG_W64_DEMUXER disabled. > > DCE is needed for FFmpeg compilation, this should get fixed by Steve > Lhomme's > configure patch. > > I find it hard to understand why wavdec.c would be the only file > showing this issue... > > Carl Eugen > ___ > 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 5/7] avcodec/h263dec: Return the correct error code in explode mode
Signed-off-by: Michael Niedermayer --- libavcodec/h263dec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c index d0da1d31c1..75fc688e78 100644 --- a/libavcodec/h263dec.c +++ b/libavcodec/h263dec.c @@ -713,7 +713,7 @@ frame_end: } if (slice_ret < 0 && (avctx->err_recognition & AV_EF_EXPLODE)) -return ret; +return slice_ret; else return get_consumed_bytes(s, buf_size); } -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/7] avcodec/rscc: return the packet size instead of 0
Most decoders return the amount of data used. This is more consistent Signed-off-by: Michael Niedermayer --- libavcodec/rscc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c index d01f05c022..f13706cd75 100644 --- a/libavcodec/rscc.c +++ b/libavcodec/rscc.c @@ -324,6 +324,7 @@ static int rscc_decode_frame(AVCodecContext *avctx, void *data, } *got_frame = 1; +ret = avpkt->size; end: av_free(inflated_tiles); return ret; -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/dump: be more verbose when printing spherical metadata information
Signed-off-by: James Almer --- This prints spherical: equirectangular, yaw=0.00, pitch=0.00, roll=0.00 Instead of spherical: equirectangular (0.00/0.00/0.00) libavformat/dump.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libavformat/dump.c b/libavformat/dump.c index d9aa3af..73914a5 100644 --- a/libavformat/dump.c +++ b/libavformat/dump.c @@ -354,9 +354,9 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd) } if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR) -av_log(ctx, AV_LOG_INFO, "equirectangular "); +av_log(ctx, AV_LOG_INFO, "equirectangular, "); else if (spherical->projection == AV_SPHERICAL_CUBEMAP) -av_log(ctx, AV_LOG_INFO, "cubemap "); +av_log(ctx, AV_LOG_INFO, "cubemap, "); else { av_log(ctx, AV_LOG_WARNING, "unknown"); return; @@ -365,7 +365,7 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd) yaw = ((double)spherical->yaw) / (1 << 16); pitch = ((double)spherical->pitch) / (1 << 16); roll = ((double)spherical->roll) / (1 << 16); -av_log(ctx, AV_LOG_INFO, "(%f/%f/%f) ", yaw, pitch, roll); +av_log(ctx, AV_LOG_INFO, "yaw=%f, pitch=%f, roll=%f ", yaw, pitch, roll); } static void dump_sidedata(void *ctx, AVStream *st, const char *indent) -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/7] avcodec/screenpresso: return the packet size instead of 0
Most decoders return the amount of data used. This is more consistent Signed-off-by: Michael Niedermayer --- libavcodec/screenpresso.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/screenpresso.c b/libavcodec/screenpresso.c index 2fadca46b7..fb8bfd4701 100644 --- a/libavcodec/screenpresso.c +++ b/libavcodec/screenpresso.c @@ -179,7 +179,7 @@ static int screenpresso_decode_frame(AVCodecContext *avctx, void *data, } *got_frame = 1; -return 0; +return avpkt->size; } AVCodec ff_screenpresso_decoder = { -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 6/7] avcodec/rawdec: Return the packet size on decoder not potentially internal buffer size
Signed-off-by: Michael Niedermayer --- libavcodec/rawdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c index e53eb2eacc..f281c98720 100644 --- a/libavcodec/rawdec.c +++ b/libavcodec/rawdec.c @@ -479,7 +479,7 @@ static int raw_decode(AVCodecContext *avctx, void *data, int *got_frame, } *got_frame = 1; -return buf_size; +return avpkt->size; } static av_cold int raw_close_decoder(AVCodecContext *avctx) -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/7] avcodec/smvjpegdec: return the packet size instead of 0
Most decoders return the amount of data used. This is more consistent Signed-off-by: Michael Niedermayer --- libavcodec/smvjpegdec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavcodec/smvjpegdec.c b/libavcodec/smvjpegdec.c index e319e5781b..018e135b30 100644 --- a/libavcodec/smvjpegdec.c +++ b/libavcodec/smvjpegdec.c @@ -200,9 +200,11 @@ static int smvjpeg_decode_frame(AVCodecContext *avctx, void *data, int *data_siz s->picture[1]->linesize[i] = mjpeg_data->linesize[i]; ret = av_frame_ref(data, s->picture[1]); +if (ret < 0) +return ret; } -return ret; +return avpkt->size; } static const AVClass smvjpegdec_class = { -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 7/7] avcodec/vp3: Do not return random positive values but the buf size
Signed-off-by: Michael Niedermayer --- libavcodec/vp3.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c index fa749be0b7..86e5852e32 100644 --- a/libavcodec/vp3.c +++ b/libavcodec/vp3.c @@ -2022,8 +2022,9 @@ static int vp3_decode_frame(AVCodecContext *avctx, ret = vp3_decode_init(avctx); if (ret < 0) { vp3_decode_end(avctx); +return ret; } -return ret; +return buf_size; } else if (type == 2) { vp3_decode_end(avctx); ret = theora_decode_tables(avctx, &gb); @@ -2031,8 +2032,9 @@ static int vp3_decode_frame(AVCodecContext *avctx, ret = vp3_decode_init(avctx); if (ret < 0) { vp3_decode_end(avctx); +return ret; } -return ret; +return buf_size; } av_log(avctx, AV_LOG_ERROR, -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/7] avcodec/tdsc: return the packet size instead of 0
Most decoders return the amount of data used. This is more consistent Signed-off-by: Michael Niedermayer --- libavcodec/tdsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/tdsc.c b/libavcodec/tdsc.c index d1265a006d..8460568d7d 100644 --- a/libavcodec/tdsc.c +++ b/libavcodec/tdsc.c @@ -609,7 +609,7 @@ static int tdsc_decode_frame(AVCodecContext *avctx, void *data, } *got_frame = 1; -return 0; +return avpkt->size; } AVCodec ff_tdsc_decoder = { -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/7] avcodec/smvjpegdec: return the packet size instead of 0
On 12/15/16, Michael Niedermayer wrote: > Most decoders return the amount of data used. > This is more consistent > > Signed-off-by: Michael Niedermayer > --- > libavcodec/smvjpegdec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/smvjpegdec.c b/libavcodec/smvjpegdec.c > index e319e5781b..018e135b30 100644 > --- a/libavcodec/smvjpegdec.c > +++ b/libavcodec/smvjpegdec.c > @@ -200,9 +200,11 @@ static int smvjpeg_decode_frame(AVCodecContext *avctx, > void *data, int *data_siz > s->picture[1]->linesize[i] = mjpeg_data->linesize[i]; > > ret = av_frame_ref(data, s->picture[1]); > +if (ret < 0) > +return ret; > } > > -return ret; > +return avpkt->size; > } > > static const AVClass smvjpegdec_class = { > -- > 2.11.0 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/7] avcodec/h263dec: Return the correct error code in explode mode
On 12/15/16, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer > --- > libavcodec/h263dec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c > index d0da1d31c1..75fc688e78 100644 > --- a/libavcodec/h263dec.c > +++ b/libavcodec/h263dec.c > @@ -713,7 +713,7 @@ frame_end: > } > > if (slice_ret < 0 && (avctx->err_recognition & AV_EF_EXPLODE)) > -return ret; > +return slice_ret; > else > return get_consumed_bytes(s, buf_size); > } > -- > 2.11.0 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/7] avcodec/rscc: return the packet size instead of 0
On 12/15/16, Michael Niedermayer wrote: > Most decoders return the amount of data used. > This is more consistent > > Signed-off-by: Michael Niedermayer > --- > libavcodec/rscc.c | 1 + > 1 file changed, 1 insertion(+) > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/screenpresso: return the packet size instead of 0
On 12/15/16, Michael Niedermayer wrote: > Most decoders return the amount of data used. > This is more consistent > > Signed-off-by: Michael Niedermayer > --- > libavcodec/screenpresso.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/tdsc: return the packet size instead of 0
On 12/15/16, Michael Niedermayer wrote: > Most decoders return the amount of data used. > This is more consistent > > Signed-off-by: Michael Niedermayer > --- > libavcodec/tdsc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > lgtm ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
Hi, On Thu, Dec 15, 2016 at 9:28 AM, Michael Niedermayer wrote: > On Thu, Dec 15, 2016 at 08:02:52AM -0500, Ronald S. Bultje wrote: > > Hi, > > > > On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < > > andreas.cadhal...@googlemail.com> wrote: > > > > > On 14.12.2016 02:46, Ronald S. Bultje wrote: > > > > Not wanting to discourage you, but I wonder if there's really a > point to > > > > this...? > > > > > > These patches are prerequisites for enforcing the validity of codec > > > parameters [1]. > > > > > > > I don't see how the user experience changes. > > > > > > Users won't see ffmpeg claiming nonsense bit rates like > -1184293205235990 > > > kb/s > > > anymore. > > > > > > I don't think you understand my question. > > > > - does this belong in 4xm.c? (Or in generic code? Or in the app?) > > - should it return an error? (Or just clip parameters? Or ignore the > > invalid value?) > > > > > This isn't specifically intended at this patch, but rather at the sort > of > > > > rabbit hole this change might lead to, > > > > > > I have a pretty good map of this rabbit hole (i.e. lots of samples > > > triggering > > > UBSan errors) and one day I might try to dig it up, but for now I'm > > > limiting > > > myself to the codec parameters. > > > > > > I'm not saying mplayer was great, but one of the principles I believe we > > always copied from them was to try to play files to the best of our > > abilities and not error out at the first convenience. This isn't just a > > theoretical thing, a lot of people credited mplayer with playing utterly > > broken AVI files that probably even ffmpeg rejects. What ffmpeg added on > > top of that is to make a project maintainable by not being an utter > > crapshoot. > > > > This isn't 4xm.c-specific, this is a general philosophical question: > > - should we error out? > > - should this be in generic code if we're likely to repeat such checks > all > > over the place? > > > > > which would cause the code to be uber-full of such checks, none of > which > > > > really have any significance. But maybe others disagree... > > > > > > Not relying on undefined behavior is a significant improvement. And > doing > > > these checks consequently where the values are set makes it possible > > > for other code to rely on their validity without further checks. > > > > > > Unless "UB" leads to actual bad behaviour, I personally don't necessarily > > care. I'm very scared that when you go beyond codec parameters, and you > > want to check for overflows all over the place, we'll never see the end > of > > it... > > > > > I'd appreciate if others could chime in here, I don't want to carry this > > argument all by myself if nobody cares. > > as you are asking for others oppinion > valid C code must not trigger undefined behavior So, I asked on IRC, here's 3 suggestions from Roger Combs: - in case we want to be pedantic (I personally don't care, but I understand other people are), it might make sense to just make these members (channels, block_align, bitrate) unsigned. That removes the UB of the overflow, and it fixes the negative number reporting in client apps for bitrate, but you can still have positive crazy numbers and it doesn't return an error. - if for whatever reason some things cannot be done in generic code or by changing the type (this should really cover most cases), and we want specific overflow checks, then maybe we want to have some generic helper macros that make them one-liners in decoders. This would return an error along with fixing the UB. - have overflow-safe multiplication functions instead of checking each argument before doing a multiply, like __builtin_mul_overflow, and then return INT64_MAX if it would overflow inside that function. This fixes display of numbers in client applications and the UB, but without returning an error. What I want most - and this comment applies to all patches of this sort - is to have something that we can all agree is OK for pretty much any decoder out there without significant overhead in code (source - not binary) size. This way, you have a template to work from and fix specific instances without us having to argue over every single time you do a next run with ubsan. I personally like suggestion (1), unsigned is a pretty good type for things that shouldn't have negative values. WDYT? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/7] avcodec/smvjpegdec: return the packet size instead of 0
On Thu, Dec 15, 2016 at 09:35:25PM +0100, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: > > Most decoders return the amount of data used. > > This is more consistent > > > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/smvjpegdec.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/smvjpegdec.c b/libavcodec/smvjpegdec.c > > index e319e5781b..018e135b30 100644 > > --- a/libavcodec/smvjpegdec.c > > +++ b/libavcodec/smvjpegdec.c > > @@ -200,9 +200,11 @@ static int smvjpeg_decode_frame(AVCodecContext *avctx, > > void *data, int *data_siz > > s->picture[1]->linesize[i] = mjpeg_data->linesize[i]; > > > > ret = av_frame_ref(data, s->picture[1]); > > +if (ret < 0) > > +return ret; > > } > > > > -return ret; > > +return avpkt->size; > > } > > > > static const AVClass smvjpegdec_class = { > > -- > > 2.11.0 > > > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > lgtm applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB You can kill me, but you cannot change the truth. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/tdsc: return the packet size instead of 0
On Thu, Dec 15, 2016 at 09:37:35PM +0100, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: > > Most decoders return the amount of data used. > > This is more consistent > > > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/tdsc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > lgtm applied thx [...] -- 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 1/7] avcodec/rscc: return the packet size instead of 0
On Thu, Dec 15, 2016 at 09:36:35PM +0100, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: > > Most decoders return the amount of data used. > > This is more consistent > > > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/rscc.c | 1 + > > 1 file changed, 1 insertion(+) > > > > lgtm applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/screenpresso: return the packet size instead of 0
On Thu, Dec 15, 2016 at 09:36:57PM +0100, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: > > Most decoders return the amount of data used. > > This is more consistent > > > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/screenpresso.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > lgtm applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato 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/7] avcodec/h263dec: Return the correct error code in explode mode
On Thu, Dec 15, 2016 at 09:35:54PM +0100, Paul B Mahol wrote: > On 12/15/16, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/h263dec.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c > > index d0da1d31c1..75fc688e78 100644 > > --- a/libavcodec/h263dec.c > > +++ b/libavcodec/h263dec.c > > @@ -713,7 +713,7 @@ frame_end: > > } > > > > if (slice_ret < 0 && (avctx->err_recognition & AV_EF_EXPLODE)) > > -return ret; > > +return slice_ret; > > else > > return get_consumed_bytes(s, buf_size); > > } > > -- > > 2.11.0 > > > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > lgtm applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship: All citizens are under surveillance, all their steps and actions recorded, for the politicians to enforce control. Democracy: All politicians are under surveillance, all their steps and actions recorded, for the citizens to enforce control. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avfilter/vf_ssim: add 9 and 10 bit depth support
Hi, patch attached From ebf2f89fd191ed661c503726106e46b7c81d6cba Mon Sep 17 00:00:00 2001 From: Paul B Mahol Date: Thu, 15 Dec 2016 22:33:42 +0100 Subject: [PATCH] avfilter/vf_ssim: add 9 and 10 bit depth suppport Signed-off-by: Paul B Mahol --- libavfilter/vf_ssim.c | 130 + libavfilter/x86/vf_ssim_init.c | 2 +- 2 files changed, 120 insertions(+), 12 deletions(-) diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c index dd8f264..83e2904 100644 --- a/libavfilter/vf_ssim.c +++ b/libavfilter/vf_ssim.c @@ -51,6 +51,7 @@ typedef struct SSIMContext { FILE *stats_file; char *stats_file_str; int nb_components; +int max; uint64_t nb_frames; double ssim[4], ssim_total; char comps[4]; @@ -60,6 +61,11 @@ typedef struct SSIMContext { int planeheight[4]; int *temp; int is_rgb; +float (*ssim_plane)(SSIMDSPContext *dsp, +uint8_t *main, int main_stride, +uint8_t *ref, int ref_stride, +int width, int height, void *temp, +int max); SSIMDSPContext dsp; } SSIMContext; @@ -87,9 +93,45 @@ static void set_meta(AVDictionary **metadata, const char *key, char comp, float } } -static void ssim_4x4xn(const uint8_t *main, ptrdiff_t main_stride, - const uint8_t *ref, ptrdiff_t ref_stride, - int (*sums)[4], int width) +static void ssim_4x4xn_16bit(const uint8_t *main8, ptrdiff_t main_stride, + const uint8_t *ref8, ptrdiff_t ref_stride, + int64_t (*sums)[4], int width) +{ +const uint16_t *main = (const uint16_t *)main8; +const uint16_t *ref = (const uint16_t *)ref8; +int x, y, z; + +main_stride >>= 1; +ref_stride >>= 1; + +for (z = 0; z < width; z++) { +uint64_t s1 = 0, s2 = 0, ss = 0, s12 = 0; + +for (y = 0; y < 4; y++) { +for (x = 0; x < 4; x++) { +int a = main[x + y * main_stride]; +int b = ref[x + y * ref_stride]; + +s1 += a; +s2 += b; +ss += a*a; +ss += b*b; +s12 += a*b; +} +} + +sums[z][0] = s1; +sums[z][1] = s2; +sums[z][2] = ss; +sums[z][3] = s12; +main += 4; +ref += 4; +} +} + +static void ssim_4x4xn_8bit(const uint8_t *main, ptrdiff_t main_stride, +const uint8_t *ref, ptrdiff_t ref_stride, +int (*sums)[4], int width) { int x, y, z; @@ -118,6 +160,22 @@ static void ssim_4x4xn(const uint8_t *main, ptrdiff_t main_stride, } } +static float ssim_end1x(int64_t s1, int64_t s2, int64_t ss, int64_t s12, int max) +{ +int64_t ssim_c1 = (int64_t)(.01*.01*max*max*64 + .5); +int64_t ssim_c2 = (int64_t)(.03*.03*max*max*64*63 + .5); + +int64_t fs1 = s1; +int64_t fs2 = s2; +int64_t fss = ss; +int64_t fs12 = s12; +int64_t vars = fss * 64 - fs1 * fs1 - fs2 * fs2; +int64_t covar = fs12 * 64 - fs1 * fs2; + +return (float)(2 * fs1 * fs2 + ssim_c1) * (float)(2 * covar + ssim_c2) + / ((float)(fs1 * fs1 + fs2 * fs2 + ssim_c1) * (float)(vars + ssim_c2)); +} + static float ssim_end1(int s1, int s2, int ss, int s12) { static const int ssim_c1 = (int)(.01*.01*255*255*64 + .5); @@ -134,7 +192,21 @@ static float ssim_end1(int s1, int s2, int ss, int s12) / ((float)(fs1 * fs1 + fs2 * fs2 + ssim_c1) * (float)(vars + ssim_c2)); } -static float ssim_endn(const int (*sum0)[4], const int (*sum1)[4], int width) +static float ssim_endn_16bit(const int64_t (*sum0)[4], const int64_t (*sum1)[4], int width, int max) +{ +float ssim = 0.0; +int i; + +for (i = 0; i < width; i++) +ssim += ssim_end1x(sum0[i][0] + sum0[i + 1][0] + sum1[i][0] + sum1[i + 1][0], + sum0[i][1] + sum0[i + 1][1] + sum1[i][1] + sum1[i + 1][1], + sum0[i][2] + sum0[i + 1][2] + sum1[i][2] + sum1[i + 1][2], + sum0[i][3] + sum0[i + 1][3] + sum1[i][3] + sum1[i + 1][3], + max); +return ssim; +} + +static float ssim_endn_8bit(const int (*sum0)[4], const int (*sum1)[4], int width) { float ssim = 0.0; int i; @@ -147,10 +219,41 @@ static float ssim_endn(const int (*sum0)[4], const int (*sum1)[4], int width) return ssim; } +static float ssim_plane_16bit(SSIMDSPContext *dsp, + uint8_t *main, int main_stride, + uint8_t *ref, int ref_stride, + int width, int height, void *temp, + int max) +{ +int z = 0, y; +float ssim = 0.0; +int64_t (*sum0)[4] = temp; +int64_t (*sum1)[4] = sum0 + (width >> 2) + 3; + +width >>= 2; +height >>= 2; +main_
Re: [FFmpeg-devel] [PATCH 2/2] avformat/hls: Add subtitle support
Hi, I tested this code by converting HLS streams into mp4 files and it seemed to work fine. However I recently compiled mpv with these changes and when I play back the Apple example stream given in ticket #2833, it seems to skip a lot of subtitles. It works fine for the real world streams I'm using it for but it's probably worth holding off with this change until that's worked out. Also I tried to make minimal changes to the existing code path because there don't seem to be any tests for HLS making it hard to know when something is broken which I guess is why there is the duplication. However I think you are right and it should be refactored, I had a further look at how it could be done and will continue working on it. If you have any advice for testing the current HLS functionality so I can be confident that there are no regressions, I would appreciate that. Thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
2016-12-15 20:43 GMT+01:00 Matthew Wolenetz : >> >> This seems like a rather odd issue to be toolchain specific. >> Can you please provide a configure command line that >> would result in a broken build? > > I believe the link-time failures occurred with configs like these: > > IIRC, on Mac, Chromium build hit this with: > >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/mac/x64/config.h?rcl=0&l=4 > > On Windows, one or both of the following hit this: > >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/ia32/config.h?rcl=0&l=4 >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/x64/config.h?rcl=0&l=4 These do not look like configure lines (only configure is supported)... > > DCE is needed for FFmpeg compilation, this should get fixed >> by Steve Lhomme's configure patch. >> I find it hard to understand why wavdec.c would be the only file >> showing this issue... > > Notably, windows still hit this (VS2015 Update 3 19.00.24213.1) > *even with -O2.* I believe this indicates a compiler issue that you should report upstream and for which we may not accept a workaround. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
On Thu, Dec 15, 2016 at 8:43 PM, Matthew Wolenetz wrote: >> >> This seems like a rather odd issue to be toolchain specific. Can you >> please provide a configure command line that would result in a broken >> build? > > > I believe the link-time failures occurred with configs like these: > > IIRC, on Mac, Chromium build hit this with: > >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/mac/x64/config.h?rcl=0&l=4 > > On Windows, one or both of the following hit this: > >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/ia32/config.h?rcl=0&l=4 >- > > https://cs.chromium.org/chromium/src/third_party/ffmpeg/chromium/config/Chromium/win/x64/config.h?rcl=0&l=4 > > DCE is needed for FFmpeg compilation, this should get fixed by Steve >> Lhomme's >> configure patch. >> I find it hard to understand why wavdec.c would be the only file >> showing this issue... > > > Notably, windows still hit this (VS2015 Update 3 19.00.24213.1) *even with > -O2.* > > You are using a custom build system, there is no way to verify any issues for us unless you can provide a configure commandline that fails on our own build-system. The patches do look like they are trying to avoid the use of DCE - and as Carl already pointed out, quite a lot of our code relies on DCE being performed by the compiler/linker, so fixing it in one or two isolated spots wouldn't really fix anything but your own customized specific build, and make our code-base less consistent within itself. DCE generally works fine with Visual Studio, and I do not know of any configure setup that makes it fail - of course with the exception of using --disable-optimizations, as that turns everything of for MSVC right now, including DCE, which makes the build impossible. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
On Thu, Dec 15, 2016 at 03:57:57PM -0500, Ronald S. Bultje wrote: > Hi, > > On Thu, Dec 15, 2016 at 9:28 AM, Michael Niedermayer > wrote: > > > On Thu, Dec 15, 2016 at 08:02:52AM -0500, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < > > > andreas.cadhal...@googlemail.com> wrote: > > > > > > > On 14.12.2016 02:46, Ronald S. Bultje wrote: > > > > > Not wanting to discourage you, but I wonder if there's really a > > point to > > > > > this...? > > > > > > > > These patches are prerequisites for enforcing the validity of codec > > > > parameters [1]. > > > > > > > > > I don't see how the user experience changes. > > > > > > > > Users won't see ffmpeg claiming nonsense bit rates like > > -1184293205235990 > > > > kb/s > > > > anymore. > > > > > > > > > I don't think you understand my question. > > > > > > - does this belong in 4xm.c? (Or in generic code? Or in the app?) > > > - should it return an error? (Or just clip parameters? Or ignore the > > > invalid value?) > > > > > > > This isn't specifically intended at this patch, but rather at the sort > > of > > > > > rabbit hole this change might lead to, > > > > > > > > I have a pretty good map of this rabbit hole (i.e. lots of samples > > > > triggering > > > > UBSan errors) and one day I might try to dig it up, but for now I'm > > > > limiting > > > > myself to the codec parameters. > > > > > > > > > I'm not saying mplayer was great, but one of the principles I believe we > > > always copied from them was to try to play files to the best of our > > > abilities and not error out at the first convenience. This isn't just a > > > theoretical thing, a lot of people credited mplayer with playing utterly > > > broken AVI files that probably even ffmpeg rejects. What ffmpeg added on > > > top of that is to make a project maintainable by not being an utter > > > crapshoot. > > > > > > This isn't 4xm.c-specific, this is a general philosophical question: > > > - should we error out? > > > - should this be in generic code if we're likely to repeat such checks > > all > > > over the place? > > > > > > > which would cause the code to be uber-full of such checks, none of > > which > > > > > really have any significance. But maybe others disagree... > > > > > > > > Not relying on undefined behavior is a significant improvement. And > > doing > > > > these checks consequently where the values are set makes it possible > > > > for other code to rely on their validity without further checks. > > > > > > > > > Unless "UB" leads to actual bad behaviour, I personally don't necessarily > > > care. I'm very scared that when you go beyond codec parameters, and you > > > want to check for overflows all over the place, we'll never see the end > > of > > > it... > > > > > > > > I'd appreciate if others could chime in here, I don't want to carry this > > > argument all by myself if nobody cares. > > > > as you are asking for others oppinion > > valid C code must not trigger undefined behavior > > > So, I asked on IRC, here's 3 suggestions from Roger Combs: > > - in case we want to be pedantic (I personally don't care, but I understand > other people are), it might make sense to just make these members > (channels, block_align, bitrate) unsigned. That removes the UB of the > overflow, and it fixes the negative number reporting in client apps for > bitrate, but you can still have positive crazy numbers and it doesn't > return an error. > - if for whatever reason some things cannot be done in generic code or by > changing the type (this should really cover most cases), and we want > specific overflow checks, then maybe we want to have some generic helper > macros that make them one-liners in decoders. This would return an error > along with fixing the UB. > - have overflow-safe multiplication functions instead of checking each > argument before doing a multiply, like __builtin_mul_overflow, and then > return INT64_MAX if it would overflow inside that function. This fixes > display of numbers in client applications and the UB, but without returning > an error. > > What I want most - and this comment applies to all patches of this sort - > is to have something that we can all agree is OK for pretty much any > decoder out there without significant overhead in code (source - not > binary) size. This way, you have a template to work from and fix specific > instances without us having to argue over every single time you do a next > run with ubsan. I personally like suggestion (1), unsigned is a pretty good > type for things that shouldn't have negative values. WDYT? unsigned is not unproblematic. If you use unsigned values, all computations touched are unsigned ( or a larger data type). If unsigned they cannot be negative. This requires great care buffer end, index and start computations can become invalid very easily by changing the signedness of a type involved. also unsigned makes detecting overflows impossible with existing tools i
Re: [FFmpeg-devel] [PATCH] avfilter/vf_ssim: add 9 and 10 bit depth support
On Thu, Dec 15, 2016 at 10:39:37PM +0100, Paul B Mahol wrote: > Hi, > > patch attached > vf_ssim.c | 130 > - > x86/vf_ssim_init.c |2 > 2 files changed, 120 insertions(+), 12 deletions(-) > 7b60c3cadb5319fb93f3875942b50ab9f9b89e1e > 0001-avfilter-vf_ssim-add-9-and-10-bit-depth-suppport.patch > From ebf2f89fd191ed661c503726106e46b7c81d6cba Mon Sep 17 00:00:00 2001 > From: Paul B Mahol > Date: Thu, 15 Dec 2016 22:33:42 +0100 > Subject: [PATCH] avfilter/vf_ssim: add 9 and 10 bit depth suppport > > Signed-off-by: Paul B Mahol > --- > libavfilter/vf_ssim.c | 130 > + > libavfilter/x86/vf_ssim_init.c | 2 +- > 2 files changed, 120 insertions(+), 12 deletions(-) this produces some build warnings CC libavfilter/vf_ssim.o libavfilter/vf_ssim.c: In function ‘ssim_4x4xn_16bit’: libavfilter/vf_ssim.c:100:21: warning: ‘main’ is usually a function [-Wmain] libavfilter/vf_ssim.c: In function ‘ssim_plane_16bit’: libavfilter/vf_ssim.c:246:9: warning: passing argument 1 of ‘ssim_endn_16bit’ from incompatible pointer type [enabled by default] libavfilter/vf_ssim.c:195:14: note: expected ‘const int64_t (*)[4]’ but argument is of type ‘int64_t (*)[4]’ libavfilter/vf_ssim.c:246:9: warning: passing argument 2 of ‘ssim_endn_16bit’ from incompatible pointer type [enabled by default] libavfilter/vf_ssim.c:195:14: note: expected ‘const int64_t (*)[4]’ but argument is of type ‘int64_t (*)[4]’ CC libavfilter/x86/vf_ssim_init.o libavfilter/x86/vf_ssim_init.c: In function ‘ff_ssim_init_x86’: libavfilter/x86/vf_ssim_init.c:40:28: warning: assignment from incompatible pointer type [enabled by default] [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: fix linking with MSVC when using --disable-optimizations
2016-12-14 16:47 GMT+01:00 Steve Lhomme : > From: Steve Lhomme > > Without any optimization flags, MSVC does no dead code elimination (DCE) at > all, even for the most trivial cases. DCE is a prerequisite for building > ffmpeg > correctly, otherwise there are undefined references to functions for other > architectures and disabled components. > > -Os -Og is the minimal optimization flags for MSVC that does include DCE. The patch looks good to me if -Og alone doesn't help, thank you for the improved commit message. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] configure: fix linking with MSVC when using --disable-optimizations
On 16 December 2016 at 11:20, Carl Eugen Hoyos wrote: > 2016-12-14 16:47 GMT+01:00 Steve Lhomme : > > From: Steve Lhomme > > > > Without any optimization flags, MSVC does no dead code elimination (DCE) > at > > all, even for the most trivial cases. DCE is a prerequisite for building > ffmpeg > > correctly, otherwise there are undefined references to functions for > other > > architectures and disabled components. > > > > -Os -Og is the minimal optimization flags for MSVC that does include DCE. > > The patch looks good to me if -Og alone doesn't help, > thank you for the improved commit message. Im not sure this is a good idea. It doesnt seem right to me that if the user selects to disable optimizations that configure still performs some anyway. As far as I see it if the user selects to have no optimizations then there should be no optimizations. This seems like a slightly incorrect hack to get around ffmpeg using DCE as opposed to tackling the broader issue of DCE breaking builds, and not just with msvc and not just with no optimizations enabled. As a slightly more appropriate (although still hacky fix) you can instead add /FORCE:UNRESOLVED to the linker command which will still result in a large number off errors being generated however they will be ignored and an output binary will still be generated. As the errors all relate to functions that never get called the output binary is still perfectly functional and i think importantly doesnt contain optimizations just as the user requested. Also this fixes debug builds aswell. I still think the best solution would be to apply a broader ranging fix to the use of DCE but until a consensus is made on that then the above may be of some help. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/wavdec: Eliminate goto for clang -O0 DCE
Clang is not able to eliminate the reference to ff_spdif_probe() when there is a goto target in the same block and optimization is disabled. This fixes the following build failure on OS X: ./configure --disable-everything --disable-doc \ --enable-decoder=pcm_s16le --enable-demuxer=wav \ --enable-protocol=file --disable-optimizations --cc=clang make ... Undefined symbols for architecture x86_64: "_ff_spdif_probe", referenced from: _set_spdif in libavformat.a(wavdec.o) ld: symbol(s) not found for architecture x86_64 --- libavformat/wavdec.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index 7176cd6f2d..ae42a6167f 100644 --- a/libavformat/wavdec.c +++ b/libavformat/wavdec.c @@ -64,34 +64,30 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav) { if (CONFIG_SPDIF_DEMUXER && s->streams[0]->codecpar->codec_tag == 1) { enum AVCodecID codec; -uint8_t *buf = NULL; int len = 1<<16; int ret = ffio_ensure_seekback(s->pb, len); -int64_t pos = avio_tell(s->pb); -if (ret < 0) -goto end; - -buf = av_malloc(len); -if (!buf) { -ret = AVERROR(ENOMEM); -goto end; +if (ret >= 0) { +uint8_t *buf = av_malloc(len); +if (!buf) { +ret = AVERROR(ENOMEM); +} else { +int64_t pos = avio_tell(s->pb); +len = ret = avio_read(s->pb, buf, len); +if (len >= 0) { +ret = ff_spdif_probe(buf, len, &codec); +if (ret > AVPROBE_SCORE_EXTENSION) { +s->streams[0]->codecpar->codec_id = codec; +wav->spdif = 1; +} +} +avio_seek(s->pb, pos, SEEK_SET); +av_free(buf); +} } -len = ret = avio_read(s->pb, buf, len); -if (ret < 0) -goto end; - -ret = ff_spdif_probe(buf, len, &codec); -if (ret > AVPROBE_SCORE_EXTENSION) { -s->streams[0]->codecpar->codec_id = codec; -wav->spdif = 1; -} -end: -avio_seek(s->pb, pos, SEEK_SET); if (ret < 0) av_log(s, AV_LOG_WARNING, "Cannot check for SPDIF\n"); -av_free(buf); } } -- 2.11.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/wavdec.c: Fix unresolved symbols on Mac and VS2015 Update 3
On 16 December 2016 at 09:54, Hendrik Leppkes wrote: > On Thu, Dec 15, 2016 at 8:43 PM, Matthew Wolenetz > wrote: > >> > >> This seems like a rather odd issue to be toolchain specific. Can you > >> please provide a configure command line that would result in a broken > >> build? > > > > > > I believe the link-time failures occurred with configs like these: > > > > IIRC, on Mac, Chromium build hit this with: > > > >- > >https://cs.chromium.org/chromium/src/third_party/ > ffmpeg/chromium/config/Chromium/mac/x64/config.h?rcl=0&l=4 > > > > On Windows, one or both of the following hit this: > > > >- > >https://cs.chromium.org/chromium/src/third_party/ > ffmpeg/chromium/config/Chromium/win/ia32/config.h?rcl=0&l=4 > >- > >https://cs.chromium.org/chromium/src/third_party/ > ffmpeg/chromium/config/Chromium/win/x64/config.h?rcl=0&l=4 > > > > DCE is needed for FFmpeg compilation, this should get fixed by Steve > >> Lhomme's > >> configure patch. > >> I find it hard to understand why wavdec.c would be the only file > >> showing this issue... > > > > > > Notably, windows still hit this (VS2015 Update 3 19.00.24213.1) *even > with > > -O2.* > > > > > > You are using a custom build system, there is no way to verify any > issues for us unless you can provide a configure commandline that > fails on our own build-system. > > The patches do look like they are trying to avoid the use of DCE - and > as Carl already pointed out, quite a lot of our code relies on DCE > being performed by the compiler/linker, so fixing it in one or two > isolated spots wouldn't really fix anything but your own customized > specific build, and make our code-base less consistent within itself. > DCE generally works fine with Visual Studio, and I do not know of any > configure setup that makes it fail - of course with the exception of > using --disable-optimizations, as that turns everything of for MSVC > right now, including DCE, which makes the build impossible. Debug builds and lto still have issues with DCE with msvc and icl. Also in an original email that mentioned a similar problem https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-April/193454.html people also mentioned issues with clang and gold compilers due to DCE. As per the linked email thread though I dont think adding patches to fix 1 or 2 cases of DCE issues should be excepted, instead we need to work out a global solution to the issues with DCE. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/dump: be more verbose when printing spherical metadata information
On Thu, Dec 15, 2016 at 05:20:33PM -0300, James Almer wrote: > Signed-off-by: James Almer > --- > This prints > > spherical: equirectangular, yaw=0.00, pitch=0.00, roll=0.00 > > Instead of > > spherical: equirectangular (0.00/0.00/0.00) > > libavformat/dump.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) should be ok i think thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate/psd : add test for psd decoder
On Thu, Nov 24, 2016 at 09:43:38PM +0100, Martin Vignali wrote: > Hello, > > In attach patch to add fate test for the psd decoder > > Sample can be found here : > https://we.tl/KvRaABCsdY > > test uncompress file in all currently support colorspace > test rle decompression, and odd and even dimensions. > > Theses samples need to be put inside ./fate-suite/psd (the folder doesn't > exist) > > fate test can be run with > make fate-psd SAMPLES=fate-suite/ > > Martin > fate/image.mak| 25 + > ref/fate/psd-gray16 |6 ++ > ref/fate/psd-gray8|6 ++ > ref/fate/psd-lena-127x127-rgb24 |6 ++ > ref/fate/psd-lena-rgb-rle-127x127-16b |6 ++ > ref/fate/psd-lena-rgb-rle-127x127-8b |6 ++ > ref/fate/psd-lena-rgba-rle-128x128-8b |6 ++ > ref/fate/psd-rgb24|6 ++ > ref/fate/psd-rgb48|6 ++ > ref/fate/psd-rgba |6 ++ > ref/fate/psd-rgba64 |6 ++ > ref/fate/psd-ya16 |6 ++ > ref/fate/psd-ya8 |6 ++ > 13 files changed, 97 insertions(+) > 568d1c982f7a2a0e8a36463e214b1125d2c3beac > 0003-fate-psd-add-tests-for-uncompress-and-rle-samples.patch > From 8e1e7679a89e2fc0e3155cfce98911f6012fdb85 Mon Sep 17 00:00:00 2001 > From: Martin Vignali > Date: Thu, 24 Nov 2016 21:30:51 +0100 > Subject: [PATCH 3/3] fate/psd : add tests for uncompress and rle samples applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: Clarify doxy for av_image_check_size2()
On Thu, Dec 15, 2016 at 02:22:07AM +0100, Andreas Cadhalpun wrote: > On 11.12.2016 22:51, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > libavutil/imgutils.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h > > index 19f34deced..40aee8b98e 100644 > > --- a/libavutil/imgutils.h > > +++ b/libavutil/imgutils.h > > @@ -193,7 +193,8 @@ int av_image_check_size(unsigned int w, unsigned int h, > > int log_offset, void *lo > > > > /** > > * Check if the given dimension of an image is valid, meaning that all > > - * bytes of the image can be addressed with a signed int. > > + * bytes of a plane of an image with the specified pix_fmt can be addressed > > + * with a signed int. > > * > > * @param w the width of the picture > > * @param h the height of the picture > > > > LGTM. applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB You can kill me, but you cannot change the truth. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/udp: Enable FIFO when using windows sockets.
On 15 December 2016 at 21:42, Nicolas George wrote: > Le duodi 22 frimaire, an CCXXV, Matt Oliver a écrit : > > --- > > libavformat/udp.c | 17 - > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/udp.c b/libavformat/udp.c > > index f8c861d..3cafb32 100644 > > --- a/libavformat/udp.c > > +++ b/libavformat/udp.c > > @@ -64,6 +64,14 @@ > > #define HAVE_PTHREAD_CANCEL 0 > > #endif > > > > +#if HAVE_THREADS && HAVE_WINSOCK2_H > > +/* Winsock2 recv function can be unblocked by shutting down the socket > */ > > +#define pthread_setcancelstate(x, y) > > +#define pthread_cancel > > +#undef HAVE_PTHREAD_CANCEL > > +#define HAVE_PTHREAD_CANCEL 1 > > +#endif > > + > > #if HAVE_PTHREAD_CANCEL > > #include "libavutil/thread.h" > > #endif > > @@ -526,6 +534,8 @@ static void *circular_buffer_task_rx( void > *_URLContext) > > goto end; > > } > > continue; > > > +} else if (len == 0) { > > +goto end; > > Unfortunately, UDP packets of size 0 exist and are returned to the > application. If len == 0 is the only criterion to detect a read > interrupted by shutdown(), it will not be usable for UDP. > > You can still combine with your original idea of an atomic flag, though. On further reading it turns out the zero length check is not required (as its not needed for connectionless sockets such as udp). So it turns out it works without the addition of that check as instead it just returns an error of WSAESHUTDOW. So ive updated the patch locally to remove then len==0 check. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/1] Fixing 3GPP Timed Text (TTXT / tx3g / mov_text) encoding for UTF-8 (ticket 6021)
According to the format specification (3GPP TS 26.245, section 5.2) "storage lengths are specified as byte-counts, wheras highlighting is specified using character offsets." This patch replaces byte counting with character counting for highlighting. See the following page for a link to the specification: https://gpac.wp.mines-telecom.fr/mp4box/ttxt-format-documentation/ --- libavcodec/movtextenc.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index 20e01e2..3ae015a 100644 --- a/libavcodec/movtextenc.c +++ b/libavcodec/movtextenc.c @@ -70,6 +70,7 @@ typedef struct { uint8_t style_fontsize; uint32_t style_color; uint16_t text_pos; +uint16_t text_pos_chars; } MovTextContext; typedef struct { @@ -216,10 +217,10 @@ static void mov_text_style_cb(void *priv, const char style, int close) } s->style_attributes_temp->style_flag = 0; -s->style_attributes_temp->style_start = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_start = AV_RB16(&s->text_pos_chars); } else { if (s->style_attributes_temp->style_flag) { //break the style record here and start a new one -s->style_attributes_temp->style_end = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_end = AV_RB16(&s->text_pos_chars); av_dynarray_add(&s->style_attributes, &s->count, s->style_attributes_temp); s->style_attributes_temp = av_malloc(sizeof(*s->style_attributes_temp)); if (!s->style_attributes_temp) { @@ -230,10 +231,10 @@ static void mov_text_style_cb(void *priv, const char style, int close) } s->style_attributes_temp->style_flag = s->style_attributes[s->count - 1]->style_flag; -s->style_attributes_temp->style_start = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_start = AV_RB16(&s->text_pos_chars); } else { s->style_attributes_temp->style_flag = 0; -s->style_attributes_temp->style_start = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_start = AV_RB16(&s->text_pos_chars); } } switch (style){ @@ -248,7 +249,7 @@ static void mov_text_style_cb(void *priv, const char style, int close) break; } } else { -s->style_attributes_temp->style_end = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_end = AV_RB16(&s->text_pos_chars); av_dynarray_add(&s->style_attributes, &s->count, s->style_attributes_temp); s->style_attributes_temp = av_malloc(sizeof(*s->style_attributes_temp)); @@ -273,7 +274,7 @@ static void mov_text_style_cb(void *priv, const char style, int close) break; } if (s->style_attributes_temp->style_flag) { //start of new style record -s->style_attributes_temp->style_start = AV_RB16(&s->text_pos); +s->style_attributes_temp->style_start = AV_RB16(&s->text_pos_chars); } } s->box_flags |= STYL_BOX; @@ -284,11 +285,11 @@ static void mov_text_color_cb(void *priv, unsigned int color, unsigned int color MovTextContext *s = priv; if (color_id == 2) {//secondary color changes if (s->box_flags & HLIT_BOX) { //close tag -s->hlit.end = AV_RB16(&s->text_pos); +s->hlit.end = AV_RB16(&s->text_pos_chars); } else { s->box_flags |= HCLR_BOX; s->box_flags |= HLIT_BOX; -s->hlit.start = AV_RB16(&s->text_pos); +s->hlit.start = AV_RB16(&s->text_pos_chars); s->hclr.color = color | (0xFF << 24); //set alpha value to FF } } @@ -302,7 +303,10 @@ static void mov_text_text_cb(void *priv, const char *text, int len) { MovTextContext *s = priv; av_bprint_append_data(&s->buffer, text, len); -s->text_pos += len; +s->text_pos += len; // length of text in bytes +for (int i = 0; i < len; i++) // length of text in UTF-8 characters +if ((text[i] & 0xC0) != 0x80) +s->text_pos_chars++; } static void mov_text_new_line_cb(void *priv, int forced) @@ -310,6 +314,7 @@ static void mov_text_new_line_cb(void *priv, int forced) MovTextContext *s = priv; av_bprint_append_data(&s->buffer, "\n", 1); s->text_pos += 1; +s->text_pos_chars += 1; } static const ASSCodesCallbacks mov_text_callbacks = { @@ -328,6 +333,7 @@ static int mov_text_encode_frame(AVCodecContext *avctx, unsigned char *buf, size_t j; s->text_pos = 0; +s->text_pos_chars = 0; s->count = 0; s->box_flags = 0; s->style_entries = 0; -- 1.9.5 (Apple Git-50.3) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/l
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
On 15.12.2016 14:02, Ronald S. Bultje wrote: > On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: >> On 14.12.2016 02:46, Ronald S. Bultje wrote: >>> Not wanting to discourage you, but I wonder if there's really a point to >>> this...? >> >> These patches are prerequisites for enforcing the validity of codec >> parameters [1]. >> >>> I don't see how the user experience changes. >> >> Users won't see ffmpeg claiming nonsense bit rates like -1184293205235990 >> kb/s >> anymore. > > > I don't think you understand my question. Maybe you just didn't understand my answer. > - does this belong in 4xm.c? (Or in generic code? Or in the app?) I think it belongs both in 4xm.c and generic code, as I have explained in detail [1] in the discussion back then. > - should it return an error? (Or just clip parameters? Or ignore the > invalid value?) This was also already discussed [2]. On 15.12.2016 21:57, Ronald S. Bultje wrote: > So, I asked on IRC, here's 3 suggestions from Roger Combs: > > - in case we want to be pedantic (I personally don't care, but I understand > other people are), it might make sense to just make these members > (channels, block_align, bitrate) unsigned. That removes the UB of the > overflow, and it fixes the negative number reporting in client apps for > bitrate, but you can still have positive crazy numbers and it doesn't > return an error. These are public fields, so changing them is not easy and more importantly changing them from signed to unsigned is a very bad idea, as it will most probably cause all kinds of subtle bugs for API users, e.g. when comparing, subtracting, etc. and not expecting unsigned behavior. > - if for whatever reason some things cannot be done in generic code or by > changing the type (this should really cover most cases), and we want > specific overflow checks, then maybe we want to have some generic helper > macros that make them one-liners in decoders. This would return an error > along with fixing the UB. I don't think the number of overflow checks added justifies the additional complexity of factoring things out. These checks are also subtly different, so it's not easy to write a generic helper for that. However, I plan to do this for the actually common cases when validating codec parameters, like checking that a parameter is not negative. > - have overflow-safe multiplication functions instead of checking each > argument before doing a multiply, like __builtin_mul_overflow, and then > return INT64_MAX if it would overflow inside that function. This fixes > display of numbers in client applications and the UB, but without returning > an error. I think that would be over-engineered. > What I want most - and this comment applies to all patches of this sort - > is to have something that we can all agree is OK for pretty much any > decoder out there without significant overhead in code (source - not > binary) size. This way, you have a template to work from and fix specific > instances without us having to argue over every single time you do a next > run with ubsan. UBSan is not only about overflows, undefined shifts are also quite common. But I haven't really looked into these cases yet, so I don't know what kind of template, if any, would make sense for them. Best regards, Andreas 1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-October/201742.html 2: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/203257.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr
On 15.12.2016 03:25, James Almer wrote: > On 12/14/2016 10:39 PM, Andreas Cadhalpun wrote: >> On 15.12.2016 00:34, Matthew Wolenetz wrote: >>> >>> From fd878457cd55690d4a27d74411b68a30c9fb2313 Mon Sep 17 00:00:00 2001 >>> From: Matt Wolenetz >>> Date: Fri, 2 Dec 2016 18:10:39 -0800 >>> Subject: [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr >>> >>> Core of patch is from p...@paulmehta.com >>> Reference https://crbug.com/643950 >>> --- >>> libavformat/mov.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 2a69890..7254505 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -739,6 +739,8 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext >>> *pb, MOVAtom atom) >>> >>> title_size = atom.size - 24; >>> if (title_size > 0) { >>> +if (title_size >= UINT_MAX) >> >> I think this should use SIZE_MAX. > > title_size is int64_t and SIZE_MAX is UINT64_MAX on x86_64. Yes, but the argument of av_malloc is size_t. >> >>> +return AVERROR_INVALIDDATA; >>> title_str = av_malloc(title_size + 1); /* Add null terminator */ So this should cast the argument to size_t to fix the issue on x86_64: title_str = av_malloc((size_t)title_size + 1); /* Add null terminator */ Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] swscale: also save ebx register when using PIE
Otherwise the build fails when configuring with --toolchain=hardened --disable-pic on i386 using gcc 4.8: error: PIC register clobbered by '%ebx' in 'asm' Signed-off-by: Andreas Cadhalpun --- libswscale/x86/hscale_fast_bilinear_simd.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libswscale/x86/hscale_fast_bilinear_simd.c b/libswscale/x86/hscale_fast_bilinear_simd.c index 2cba5f0..3f0f5f5 100644 --- a/libswscale/x86/hscale_fast_bilinear_simd.c +++ b/libswscale/x86/hscale_fast_bilinear_simd.c @@ -199,7 +199,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst, #if ARCH_X86_64 uint64_t retsave; #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) uint64_t ebxsave; #endif #endif @@ -209,7 +209,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst, "mov -8(%%rsp), %%"FF_REG_a"\n\t" "mov%%"FF_REG_a", %5 \n\t" // retsave #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) "mov%%"FF_REG_b", %5 \n\t" // ebxsave #endif #endif @@ -255,7 +255,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst, "mov %5, %%"FF_REG_a" \n\t" "mov%%"FF_REG_a", -8(%%rsp)\n\t" #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) "mov %5, %%"FF_REG_b" \n\t" #endif #endif @@ -264,12 +264,12 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst, #if ARCH_X86_64 ,"m"(retsave) #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) ,"m" (ebxsave) #endif #endif : "%"FF_REG_a, "%"FF_REG_c, "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_D -#if ARCH_X86_64 || !defined(PIC) +#if ARCH_X86_64 || !(defined(PIC) || defined(__PIE__)) ,"%"FF_REG_b #endif ); @@ -289,7 +289,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2, #if ARCH_X86_64 DECLARE_ALIGNED(8, uint64_t, retsave); #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) DECLARE_ALIGNED(8, uint64_t, ebxsave); #endif #endif @@ -298,7 +298,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2, "mov -8(%%rsp), %%"FF_REG_a"\n\t" "mov %%"FF_REG_a", %7 \n\t" // retsave #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) "mov %%"FF_REG_b", %7 \n\t" // ebxsave #endif #endif @@ -332,7 +332,7 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2, "mov%7, %%"FF_REG_a" \n\t" "mov %%"FF_REG_a", -8(%%rsp)\n\t" #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) "mov %7, %%"FF_REG_b"\n\t" #endif #endif @@ -341,12 +341,12 @@ void ff_hcscale_fast_mmxext(SwsContext *c, int16_t *dst1, int16_t *dst2, #if ARCH_X86_64 ,"m"(retsave) #else -#if defined(PIC) +#if defined(PIC) || defined(__PIE__) ,"m" (ebxsave) #endif #endif : "%"FF_REG_a, "%"FF_REG_c, "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_D -#if ARCH_X86_64 || !defined(PIC) +#if ARCH_X86_64 || !(defined(PIC) || defined(__PIE__)) ,"%"FF_REG_b #endif ); -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
> On Dec 15, 2016, at 19:21, Andreas Cadhalpun > wrote: > > On 15.12.2016 14:02, Ronald S. Bultje wrote: >> On Wed, Dec 14, 2016 at 7:11 PM, Andreas Cadhalpun < >> andreas.cadhal...@googlemail.com> wrote: >>> On 14.12.2016 02:46, Ronald S. Bultje wrote: Not wanting to discourage you, but I wonder if there's really a point to this...? >>> >>> These patches are prerequisites for enforcing the validity of codec >>> parameters [1]. >>> I don't see how the user experience changes. >>> >>> Users won't see ffmpeg claiming nonsense bit rates like -1184293205235990 >>> kb/s >>> anymore. >> >> >> I don't think you understand my question. > > Maybe you just didn't understand my answer. > >> - does this belong in 4xm.c? (Or in generic code? Or in the app?) > > I think it belongs both in 4xm.c and generic code, as I have explained > in detail [1] in the discussion back then. > >> - should it return an error? (Or just clip parameters? Or ignore the >> invalid value?) > > This was also already discussed [2]. > > On 15.12.2016 21:57, Ronald S. Bultje wrote: >> So, I asked on IRC, here's 3 suggestions from Roger Combs: >> >> - in case we want to be pedantic (I personally don't care, but I understand >> other people are), it might make sense to just make these members >> (channels, block_align, bitrate) unsigned. That removes the UB of the >> overflow, and it fixes the negative number reporting in client apps for >> bitrate, but you can still have positive crazy numbers and it doesn't >> return an error. > > These are public fields, so changing them is not easy and more importantly > changing them from signed to unsigned is a very bad idea, as it will most > probably cause all kinds of subtle bugs for API users, e.g. when comparing, > subtracting, etc. and not expecting unsigned behavior. > Fair enough. >> - if for whatever reason some things cannot be done in generic code or by >> changing the type (this should really cover most cases), and we want >> specific overflow checks, then maybe we want to have some generic helper >> macros that make them one-liners in decoders. This would return an error >> along with fixing the UB. > > I don't think the number of overflow checks added justifies the additional > complexity of factoring things out. These checks are also subtly different, > so it's not easy to write a generic helper for that. > However, I plan to do this for the actually common cases when validating > codec parameters, like checking that a parameter is not negative. > My proposal was for something like: #define BAIL_ON_OVERFLOW(x) if (x) {av_log(avctx, AV_LOG_ERROR, "Overflow check failed: " #x); return AVERROR_INVALIDDATA;} Which basically reduces the code overhead down to a simple one-liner. It's hard to get detailed error prints out of this, but if we're saying these cases are so unlikely (fuzzer-only?) that we're comfortable outright failing on them, the level of precision in the message probably doesn't matter much? >> - have overflow-safe multiplication functions instead of checking each >> argument before doing a multiply, like __builtin_mul_overflow, and then >> return INT64_MAX if it would overflow inside that function. This fixes >> display of numbers in client applications and the UB, but without returning >> an error. > > I think that would be over-engineered. > >> What I want most - and this comment applies to all patches of this sort - >> is to have something that we can all agree is OK for pretty much any >> decoder out there without significant overhead in code (source - not >> binary) size. This way, you have a template to work from and fix specific >> instances without us having to argue over every single time you do a next >> run with ubsan. > > UBSan is not only about overflows, undefined shifts are also quite common. > But I haven't really looked into these cases yet, so I don't know what kind > of template, if any, would make sense for them. > > Best regards, > Andreas > > > 1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-October/201742.html > 2: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/203257.html > ___ > 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] lavc/vaapi_encode_h264: disable B frame in baseline profile
From a4b410e02ac4864c7d82b15474a65ed42a84da6a Mon Sep 17 00:00:00 2001 From: Jun Zhao Date: Fri, 16 Dec 2016 09:49:57 +0800 Subject: [PATCH] lavc/vaapi_encode_h264: disable B frame in baseline profile. disable B frames when usd baseline/constrined baseline profile, it's base on H.264 spec Annex A.2.1. Signed-off-by: Jun Zhao Signed-off-by: Yi A Wang --- libavcodec/vaapi_encode_h264.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index 69cc483..075f800 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -1190,9 +1190,19 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext *avctx) switch (avctx->profile) { case FF_PROFILE_H264_CONSTRAINED_BASELINE: ctx->va_profile = VAProfileH264ConstrainedBaseline; +if (avctx->max_b_frames != 0) { +avctx->max_b_frames = 0; +av_log(avctx, AV_LOG_WARNING, "H.264 constrained baseline " + "profile don't support encode B frame.\n"); +} break; case FF_PROFILE_H264_BASELINE: ctx->va_profile = VAProfileH264Baseline; +if (avctx->max_b_frames != 0) { +avctx->max_b_frames = 0; +av_log(avctx, AV_LOG_WARNING, "H.264 baseline " + "profile don't support encode B frame.\n"); +} break; case FF_PROFILE_H264_MAIN: ctx->va_profile = VAProfileH264Main; -- 2.9.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] 4xm: prevent overflow during bit rate calculation
On 16.12.2016 02:29, Rodger Combs wrote: >> On Dec 15, 2016, at 19:21, Andreas Cadhalpun >> wrote: >> On 15.12.2016 14:02, Ronald S. Bultje wrote: >>> - if for whatever reason some things cannot be done in generic code or by >>> changing the type (this should really cover most cases), and we want >>> specific overflow checks, then maybe we want to have some generic helper >>> macros that make them one-liners in decoders. This would return an error >>> along with fixing the UB. >> >> I don't think the number of overflow checks added justifies the additional >> complexity of factoring things out. These checks are also subtly different, >> so it's not easy to write a generic helper for that. >> However, I plan to do this for the actually common cases when validating >> codec parameters, like checking that a parameter is not negative. >> > > My proposal was for something like: > #define BAIL_ON_OVERFLOW(x) if (x) {av_log(avctx, AV_LOG_ERROR, "Overflow > check failed: " #x); return AVERROR_INVALIDDATA;} > Which basically reduces the code overhead down to a simple one-liner. Yeah, that's similar to how I plan to handle the more common cases. > It's hard to get detailed error prints out of this, but if we're saying these > cases are so unlikely (fuzzer-only?) that we're comfortable outright failing > on them, the level of precision in the message probably doesn't matter much? Agreed, so I've updated the patch series using this approach. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/7] avutil: add FF_BAIL_ON_OVERFLOW
Suggested-by: Rodger Combs Signed-off-by: Andreas Cadhalpun --- libavutil/common.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavutil/common.h b/libavutil/common.h index 8142b31..00b7504 100644 --- a/libavutil/common.h +++ b/libavutil/common.h @@ -99,6 +99,8 @@ #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) +#define FF_BAIL_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR, "Overflow check failed: " #x"\n"); return AVERROR_INVALIDDATA;} + /* misc math functions */ #ifdef HAVE_AV_CONFIG_H -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/7] 4xm: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/4xm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/4xm.c b/libavformat/4xm.c index 2758b69..9332f78 100644 --- a/libavformat/4xm.c +++ b/libavformat/4xm.c @@ -187,6 +187,7 @@ static int parse_strk(AVFormatContext *s, st->codecpar->bit_rate = (int64_t)st->codecpar->channels * st->codecpar->sample_rate * st->codecpar->bits_per_coded_sample; +FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels && st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) st->codecpar->block_align = st->codecpar->channels * st->codecpar->bits_per_coded_sample; -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/7] electronicarts: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/electronicarts.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c index 30eb723..9088fe1 100644 --- a/libavformat/electronicarts.c +++ b/libavformat/electronicarts.c @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s) st->codecpar->codec_tag = 0; /* no tag */ st->codecpar->channels = ea->num_channels; st->codecpar->sample_rate = ea->sample_rate; +FF_BAIL_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2) st->codecpar->bits_per_coded_sample = ea->bytes * 8; st->codecpar->bit_rate = (int64_t)st->codecpar->channels * st->codecpar->sample_rate * -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/7] genh: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/genh.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/genh.c b/libavformat/genh.c index b683e02..e23c1b2 100644 --- a/libavformat/genh.c +++ b/libavformat/genh.c @@ -74,6 +74,7 @@ static int genh_read_header(AVFormatContext *s) case 0: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_PSX;break; case 1: case 11: st->codecpar->bits_per_coded_sample = 4; + FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels > INT_MAX / 36) st->codecpar->block_align = 36 * st->codecpar->channels; st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_WAV;break; case 2: st->codecpar->codec_id = AV_CODEC_ID_ADPCM_DTK;break; -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 5/7] ircamdec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/ircamdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/ircamdec.c b/libavformat/ircamdec.c index 59f3a49..3f92b6c 100644 --- a/libavformat/ircamdec.c +++ b/libavformat/ircamdec.c @@ -96,6 +96,7 @@ static int ircam_read_header(AVFormatContext *s) } st->codecpar->bits_per_coded_sample = av_get_bits_per_sample(st->codecpar->codec_id); +FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels && st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) st->codecpar->block_align = st->codecpar->bits_per_coded_sample * st->codecpar->channels / 8; avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); avio_skip(s->pb, 1008); -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/7] 4xm: prevent overflow during block alignment calculation
> On Dec 15, 2016, at 20:32, Andreas Cadhalpun > wrote: > > Signed-off-by: Andreas Cadhalpun > --- > libavformat/4xm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/4xm.c b/libavformat/4xm.c > index 2758b69..9332f78 100644 > --- a/libavformat/4xm.c > +++ b/libavformat/4xm.c > @@ -187,6 +187,7 @@ static int parse_strk(AVFormatContext *s, > st->codecpar->bit_rate = (int64_t)st->codecpar->channels * > st->codecpar->sample_rate * > st->codecpar->bits_per_coded_sample; > +FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels && > st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) Shouldn't this go before the actual (potentially-overflowing) calculation is done? > st->codecpar->block_align = st->codecpar->channels * > st->codecpar->bits_per_coded_sample; > > -- > 2.10.2 > > ___ > 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 7/7] pvfdec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/pvfdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/pvfdec.c b/libavformat/pvfdec.c index b9f6d4f..868ef0b 100644 --- a/libavformat/pvfdec.c +++ b/libavformat/pvfdec.c @@ -56,6 +56,7 @@ static int pvf_read_header(AVFormatContext *s) st->codecpar->sample_rate = sample_rate; st->codecpar->codec_id= ff_get_pcm_codec_id(bps, 0, 1, 0x); st->codecpar->bits_per_coded_sample = bps; +FF_BAIL_ON_OVERFLOW(s, bps > INT_MAX / st->codecpar->channels) st->codecpar->block_align = bps * st->codecpar->channels / 8; avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 6/7] nistspheredec: prevent overflow during block alignment calculation
Signed-off-by: Andreas Cadhalpun --- libavformat/nistspheredec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c index 782d1df..b5ebcb8 100644 --- a/libavformat/nistspheredec.c +++ b/libavformat/nistspheredec.c @@ -80,6 +80,7 @@ static int nist_read_header(AVFormatContext *s) avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); +FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels && st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) st->codecpar->block_align = st->codecpar->bits_per_coded_sample * st->codecpar->channels / 8; if (avio_tell(s->pb) > header_size) -- 2.10.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/7] 4xm: prevent overflow during block alignment calculation
On 16.12.2016 03:33, Rodger Combs wrote: > >> On Dec 15, 2016, at 20:32, Andreas Cadhalpun >> wrote: >> >> Signed-off-by: Andreas Cadhalpun >> --- >> libavformat/4xm.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/libavformat/4xm.c b/libavformat/4xm.c >> index 2758b69..9332f78 100644 >> --- a/libavformat/4xm.c >> +++ b/libavformat/4xm.c >> @@ -187,6 +187,7 @@ static int parse_strk(AVFormatContext *s, >> st->codecpar->bit_rate = (int64_t)st->codecpar->channels * >> st->codecpar->sample_rate * >> >> st->codecpar->bits_per_coded_sample; >> +FF_BAIL_ON_OVERFLOW(s, st->codecpar->channels && >> st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels) > > Shouldn't this go before the actual (potentially-overflowing) calculation is > done? It is. >> st->codecpar->block_align = st->codecpar->channels * >> >> st->codecpar->bits_per_coded_sample; This here is the problem. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Removing DCE
Recently we have again received several patches that are trying to add workarounds for ffmpegs use of DCE. This is not the first time this has happened and wont be the last until a decision is made about the use of DCE. So I think its time that we made a official decision on the use of DCE. This is of course something that should be properly agreed upon by developers going forward as different people have different opinions on the matter so to help assist this I will summaries all of the previously made arguments from both sides of the discussion. For DCE: 1) Ends up with a horrible mess of ifdefs. 2) Disabled parts of code will not be checked for syntax. Against DCE: 3) DCE is not actually specified in the C specification. So compilers are actually being 100% compliant by not supporting it and should not be expected to change just for ffmpegs use case. 4) Breaks debug and lto builds on msvc. 5) Breaks debug and lto builds on icl. 6) Issues with lto with Clang and Gold. 7) Other unspecified issues with debug builds Rebuttals against above arguments: 8) There are already 3961 #ifs(not including header guards) in the code so there is already a lot of code that doesn't use DCE. (In response to #1 for DCE). 9) Avoiding #ifdefs is a personal opinion as to whether they are ugly or not (some prefer ifdefs as IDEs will correctly highlight disabled sections). Someones personal preference for what looks better should not be justification to break supported configurations/compilers. (In response to #1 for DCE). 10) There is --enable-random which is supposed to be used to find configurations that don't compile. (in response to #2 for DCE). 11) Just because something compiles does not mean that it actually works, relying on just syntax checking is dangerous as it gives false security as the code is not actually being tested. (in response to #2 for DCE) 12) There are numerous FATE tests that should find all the configuration errors. (in response to #2 for DCE) 12) MSVC is broken and we shouldn't support it so Microsoft are forced to fix it (in response to #4 against DCE) - This point is countermanded by #3 against DCE and reported issues with other compilers as well. Most of the above points were taken from peoples posts in the following mailing list thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-April/193437.html https://ffmpeg.org/pipermail/ffmpeg-devel/2016-May/194115.html Its my personal opinion that DCE should be removed from the code but this is something I am aware will require a developer consensus and perhaps even a vote. Stating something is broken is one thing so I will also put forward a solution in that if it is agreed upon to remove DCE usage then I will spend the time and effort to go through the existing code base and replace DCE with appropriate #ifs. Matt ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale: also save ebx register when using PIE
On Fri, Dec 16, 2016 at 02:36:53AM +0100, Andreas Cadhalpun wrote: > Otherwise the build fails when configuring with --toolchain=hardened > --disable-pic on i386 using gcc 4.8: > error: PIC register clobbered by '%ebx' in 'asm' > > Signed-off-by: Andreas Cadhalpun > --- > libswscale/x86/hscale_fast_bilinear_simd.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/libswscale/x86/hscale_fast_bilinear_simd.c > b/libswscale/x86/hscale_fast_bilinear_simd.c > index 2cba5f0..3f0f5f5 100644 > --- a/libswscale/x86/hscale_fast_bilinear_simd.c > +++ b/libswscale/x86/hscale_fast_bilinear_simd.c > @@ -199,7 +199,7 @@ void ff_hyscale_fast_mmxext(SwsContext *c, int16_t *dst, > #if ARCH_X86_64 > uint64_t retsave; > #else > -#if defined(PIC) > +#if defined(PIC) || defined(__PIE__) please correct me if iam wrong our configure adds -DPIC to define PIC when its enabled, it does not add that in this case but gcc is still generating PIC code that doesnt seem good [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c
Signed-off-by: Thomas Turner --- libavutil/Makefile | 1 + libavutil/tests/audio_fifo.c | 210 +++ tests/fate/libavutil.mak | 4 + tests/ref/fate/audio_fifo| 228 +++ 4 files changed, 443 insertions(+) create mode 100644 libavutil/tests/audio_fifo.c create mode 100644 tests/ref/fate/audio_fifo diff --git a/libavutil/Makefile b/libavutil/Makefile index 9841645..2dd91b8 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -182,6 +182,7 @@ SKIPHEADERS-$(CONFIG_OPENCL) += opencl.h TESTPROGS = adler32 \ aes \ atomic \ +audio_fifo \ avstring\ base64 \ blowfish\ diff --git a/libavutil/tests/audio_fifo.c b/libavutil/tests/audio_fifo.c new file mode 100644 index 000..e3a3484 --- /dev/null +++ b/libavutil/tests/audio_fifo.c @@ -0,0 +1,210 @@ +/* + * 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 +#include +//#include "libavutil/audio_fifo.c" +#include "../audio_fifo.c" +#include "libavutil/audio_fifo.h" +#include "libavutil/error.h" + +#define MAX_CHANNELS32 + + +typedef struct TestStruct { +const char *message; +const enum AVSampleFormat format; +const int nb_ch; +void *data_planes[MAX_CHANNELS]; +int nb_samples_pch; +} TestStruct; + +static uint8_t data_U8 [] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 }; +static int16_t data_S16[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 }; +static float data_FLT[] = {0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0}; + + +static const TestStruct test_struct[] = { +{.format = AV_SAMPLE_FMT_U8 , .nb_ch = 1, .data_planes = {data_U8 , }, .nb_samples_pch = 12}, +{.format = AV_SAMPLE_FMT_U8P , .nb_ch = 2, .data_planes = {data_U8 , data_U8 +6, }, .nb_samples_pch = 6 }, +{.format = AV_SAMPLE_FMT_S16 , .nb_ch = 1, .data_planes = {data_S16, }, .nb_samples_pch = 12}, +{.format = AV_SAMPLE_FMT_S16P , .nb_ch = 2, .data_planes = {data_S16, data_S16+6, }, .nb_samples_pch = 6 }, +{.format = AV_SAMPLE_FMT_FLT , .nb_ch = 1, .data_planes = {data_FLT, }, .nb_samples_pch = 12}, +{.format = AV_SAMPLE_FMT_FLTP , .nb_ch = 2, .data_planes = {data_FLT, data_FLT+6, }, .nb_samples_pch = 6 } +}; + +static int is_little_endian(void) +{ +const unsigned int i = 1; +return *(uint8_t*)&i; +} + +static void* allocate_memory(size_t size) +{ +void *ptr = malloc(size); +if (ptr == NULL){ +fprintf(stderr, "failed to allocate memory!\n"); +exit(1); +} +return ptr; +} + +static void print_audio_bytes(const TestStruct *test_sample, void **data_planes, int nb_samples) +{ +int p, b, f; +int e= is_little_endian(); +int byte_offset = av_get_bytes_per_sample(test_sample->format); +int buffers = av_sample_fmt_is_planar(test_sample->format) + ? test_sample->nb_ch : 1; +int line_size= (buffers > 1) ? nb_samples * byte_offset + : nb_samples * byte_offset * test_sample->nb_ch; +for (p = 0; p < buffers; ++p){ +for(b = 0; b < line_size; b+=byte_offset){ +for (f = 0; f < byte_offset; f++){ +int order = e ? (byte_offset - f - 1) : f; +printf("%02x", *((uint8_t*)data_planes[p] + b + order)); +} +putchar(' '); +} +putchar('\n'); +} +} + +static int read_samples_from_audio_fifo(AVAudioFifo* afifo, void ***output, int nb_samples) +{ +int i, planes; +int samples= FFMIN(nb_samples, afifo->nb_samples); +int tot_elements = !(planes = av_sample_fmt_is_planar(afifo->sample_fmt)) + ? sample
Re: [FFmpeg-devel] [PATCH 1/7] avutil: add FF_BAIL_ON_OVERFLOW
On 12/16/16, Andreas Cadhalpun wrote: > Suggested-by: Rodger Combs > Signed-off-by: Andreas Cadhalpun > --- > libavutil/common.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 8142b31..00b7504 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -99,6 +99,8 @@ > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > +#define FF_BAIL_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR, > "Overflow check failed: " #x"\n"); return AVERROR_INVALIDDATA;} Where is the overflow check calculation? What about functions that need clean up with goto before return? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/7] electronicarts: prevent overflow during block alignment calculation
looks good. On Fri, Dec 16, 2016 at 03:33:26AM +0100, Andreas Cadhalpun wrote: > Signed-off-by: Andreas Cadhalpun > --- > libavformat/electronicarts.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c > index 30eb723..9088fe1 100644 > --- a/libavformat/electronicarts.c > +++ b/libavformat/electronicarts.c > @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s) > st->codecpar->codec_tag = 0; /* no tag */ > st->codecpar->channels = ea->num_channels; > st->codecpar->sample_rate = ea->sample_rate; > +FF_BAIL_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2) > st->codecpar->bits_per_coded_sample = ea->bytes * 8; > st->codecpar->bit_rate = > (int64_t)st->codecpar->channels * >st->codecpar->sample_rate * > -- > 2.10.2 > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Peter (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B) signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel