Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter

2021-10-30 Thread Tomas Härdin
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

2021-10-30 Thread Paul B Mahol
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

2021-10-30 Thread James Almer
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

2021-10-30 Thread 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.

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

2021-10-30 Thread Tomas Härdin
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

2021-10-30 Thread Michael Niedermayer
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

2021-10-30 Thread Michael Niedermayer
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

2021-10-30 Thread Michael Niedermayer
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".