Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 21:17 -0400 skrev Ronald S. Bultje: > Hi Thomas, > > On Fri, Oct 29, 2021 at 9:12 AM Tomas Härdin > wrote: > > > tor 2021-10-28 klockan 21:09 +0200 skrev Paul B Mahol: > > > + const uint16_t *src = (const uint16_t *)ssrc; > > > > This is not -fstrict-aliasing safe > > > > I don't believe that is correct. It's true we're not allowed to cast > between two clashing types to access the same pointer (memory > location), > but the C standard would appear to make an exception for byte types. > > Quoted from https://stackoverflow.com/a/99010/4726410 because I'm too > lazy > to dig through the standard: > > "The types that C 2011 6.5 7 allows an lvalue to access are: > - a type compatible with the effective type of the object, > - a qualified version of a type compatible with the effective type of > the > object, > - a type that is the signed or unsigned type corresponding to the > effective > type of the object, > - a type that is the signed or unsigned type corresponding to a > qualified > version of the effective type of the object, > - an aggregate or union type that includes one of the aforementioned > types > among its members (including, recursively, a member of a subaggregate > or > contained union), or > - a character type." > > uint8_t is a variant of the character (aka byte) type, and so the > cast > would not seem to violate strict aliasing rules. Maybe we should upgrade to C11 then? This gives us access to more useful language features. Type-generic expressions look very useful /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
On Sat, Oct 30, 2021 at 12:28 AM Tomas Härdin wrote: > fre 2021-10-29 klockan 19:43 +0200 skrev Paul B Mahol: > > On Fri, Oct 29, 2021 at 7:26 PM Tomas Härdin > > wrote: > > > > > fre 2021-10-29 klockan 19:17 +0200 skrev Paul B Mahol: > > > > On Fri, Oct 29, 2021 at 6:59 PM Tomas Härdin > > > > wrote: > > > > > > > > > fre 2021-10-29 klockan 17:00 +0200 skrev Paul B Mahol: > > > > > > On Fri, Oct 29, 2021 at 4:46 PM Tomas Härdin < > > > > > > tjop...@acc.umu.se> > > > > > > wrote: > > > > > > > > > > > > > fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol: > > > > > > > > On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin < > > > > > > > > tjop...@acc.umu.se> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > +static double get_hx(const uint8_t *src, int > > > > > > > > > > linesize, > > > > > > > > > > int > > > > > > > > > > w, > > > > > > > > > > int h) > > > > > > > > > > +{ > > > > > > > > > > +int64_t sum = 0; > > > > > > > > > > + > > > > > > > > > > +for (int y = 0; y < h; y++) { > > > > > > > > > > +for (int x = 0; x < w; x++) { > > > > > > > > > > +sum += 12 * src[x] - > > > > > > > > > > +2 * (src[x-1] + src[x+1] + > > > > > > > > > > + src[x + linesize] + > > > > > > > > > > + src[x - linesize]) - > > > > > > > > > > +1 * (src[x - 1 - linesize] + > > > > > > > > > > + src[x + 1 - linesize] + > > > > > > > > > > + src[x - 1 + linesize] + > > > > > > > > > > + src[x + 1 + linesize]); > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +src += linesize; > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +return fabs(sum * 0.25); > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +static double get_hx16(const uint8_t *ssrc, int > > > > > > > > > > linesize, > > > > > > > > > > int w, > > > > > > > > > > int > > > > > > > > > > h) > > > > > > > > > > +{ > > > > > > > > > > +const uint16_t *src = (const uint16_t *)ssrc; > > > > > > > > > > > > > > > > > > This is not -fstrict-aliasing safe > > > > > > > > > > > > > > > > > > > > > > > > > How so? I get no warnings at all, and similar is used > > > > > > > > everywhere > > > > > > > > else. > > > > > > > > > > > > > > Then those places should be fixed. We have macros like > > > > > > > AV_RB16() > > > > > > > for a > > > > > > > reason. That gcc doesn't warn about this doesn't mean it > > > > > > > isn't > > > > > > > free > > > > > > > to > > > > > > > assume ssrc and src points to different non-overlapping > > > > > > > regions > > > > > > > of > > > > > > > memory. > > > > > > > > > > > > > > > > > > > > That is sub optimal and unacceptable solution. > > > > > > > > > > Did you do measurements to come to this conclusion? > > > > > > > > > > > Review remark ignored. > > > > > > > > > > Undefined behavior is *not* acceptable. If you want this file > > > > > specifically to be compiled with strict aliasing disabled then > > > > > you > > > > > must > > > > > at the very least change the build system accordingly > > > > > > > > > > > > > Cite source that can prove this. > > > > > > The gcc manpage > > > > > > > citations needed. > > > > > > > > > > > Compilers are silent about this. > > > > > > Irrelevant > > > > > > > > > citations needed. > > > > > > > > > > > There are hundredths if not thousands of such examples across > > > > codebase. > > > > > > This is not a valid excuse. We need less UB in the codebase, not > > > more. > > > UBs beget CVEs. > > > > > > > citations needed. > > Childish behavior doesn't make you right, Paul. > > Ad-hominem. > /Tomas > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/libaomdec: use intermediate arrays for plane pointers and strides
Fixes -Wstringop-overflow warnings with libaom >= 2.0.0, where the unused alpha plane was removed from aom_image. Signed-off-by: James Almer --- libavcodec/libaomdec.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c index 75ecc08970..e590c58d9d 100644 --- a/libavcodec/libaomdec.c +++ b/libavcodec/libaomdec.c @@ -224,9 +224,13 @@ static int aom_decode(AVCodecContext *avctx, void *data, int *got_frame, if ((img->fmt & AOM_IMG_FMT_HIGHBITDEPTH) && img->bit_depth == 8) image_copy_16_to_8(picture, img); -else -av_image_copy(picture->data, picture->linesize, (const uint8_t **)img->planes, - img->stride, avctx->pix_fmt, img->d_w, img->d_h); +else { +const uint8_t *planes[4] = { img->planes[0], img->planes[1], img->planes[2] }; +const int stride[4] = { img->stride[0], img->stride[1], img->stride[2] }; + +av_image_copy(picture->data, picture->linesize, planes, + stride, avctx->pix_fmt, img->d_w, img->d_h); +} *got_frame = 1; } return avpkt->size; -- 2.33.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
Hi, On Sat, Oct 30, 2021 at 4:57 AM Tomas Härdin wrote: > Maybe we should upgrade to C11 then? This gives us access to more > useful language features. Type-generic expressions look very useful > https://stackoverflow.com/a/7005988 (same thread, further down) appears to suggest the exact same literal wording exists in C99. No upgrade is necessary. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
lör 2021-10-30 klockan 10:28 -0400 skrev Ronald S. Bultje: > Hi, > > On Sat, Oct 30, 2021 at 4:57 AM Tomas Härdin > wrote: > > > Maybe we should upgrade to C11 then? This gives us access to more > > useful language features. Type-generic expressions look very useful > > > > https://stackoverflow.com/a/7005988 (same thread, further down) > appears to > suggest the exact same literal wording exists in C99. No upgrade is > necessary. Ah. Well disregard my ramblings then (: /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] avformat/aiffdec: Use av_rescale() for bitrate
Fixes: integer overflow Fixes: 40313/clusterfuzz-testcase-minimized-ffmpeg_dem_AIFF_fuzzer-4814761406103552 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavformat/aiffdec.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c index 648f231a523..7a995c00a66 100644 --- a/libavformat/aiffdec.c +++ b/libavformat/aiffdec.c @@ -185,8 +185,10 @@ static int get_aiff_header(AVFormatContext *s, int size, par->block_align = (av_get_bits_per_sample(par->codec_id) * par->channels) >> 3; if (aiff->block_duration) { -par->bit_rate = (int64_t)par->sample_rate * (par->block_align << 3) / -aiff->block_duration; +par->bit_rate = av_rescale(par->sample_rate, par->block_align * 8LL, + aiff->block_duration); +if (par->bit_rate < 0) +par->bit_rate = 0; } /* Chunk is over */ -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avformat/aiffdec: Check sample_rate
Signed-off-by: Michael Niedermayer --- libavformat/aiffdec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c index a826bc998ad..14063474260 100644 --- a/libavformat/aiffdec.c +++ b/libavformat/aiffdec.c @@ -120,6 +120,9 @@ static int get_aiff_header(AVFormatContext *s, int size, sample_rate = val << exp; else sample_rate = (val + (1ULL<<(-exp-1))) >> -exp; +if (sample_rate <= 0) +return AVERROR_INVALIDDATA; + par->sample_rate = sample_rate; if (size < 18) return AVERROR_INVALIDDATA; -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avformat/aiffdec: sanity check block_align
Signed-off-by: Michael Niedermayer --- libavformat/aiffdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c index 14063474260..648f231a523 100644 --- a/libavformat/aiffdec.c +++ b/libavformat/aiffdec.c @@ -368,7 +368,7 @@ got_sound: if (!st->codecpar->block_align && st->codecpar->codec_id == AV_CODEC_ID_QCELP) { av_log(s, AV_LOG_WARNING, "qcelp without wave chunk, assuming full rate\n"); st->codecpar->block_align = 35; -} else if (!st->codecpar->block_align) { +} else if (st->codecpar->block_align <= 0) { av_log(s, AV_LOG_ERROR, "could not find COMM tag or invalid block_align value\n"); return -1; } -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".