Hi Ramiro On Mon, Dec 02, 2024 at 02:45:34PM +0100, Ramiro Polla wrote: > Hi Michael, > > On Mon, Dec 2, 2024 at 1:33 AM Michael Niedermayer > <mich...@niedermayer.cc> wrote: > > On Sat, Sep 28, 2024 at 02:01:33AM +0200, Michael Niedermayer wrote: > > > On Wed, Sep 25, 2024 at 03:16:30PM +0200, Ramiro Polla wrote: > > > > On Tue, Sep 24, 2024 at 3:35 PM Michael Niedermayer > > > > <mich...@niedermayer.cc> wrote: > > > > > On Mon, Sep 23, 2024 at 12:42:22AM +0200, Ramiro Polla wrote: > > > > > > Hi, > > > > > > > > > > > > On Mon, Sep 23, 2024 at 12:04 AM Michael Niedermayer > > > > > > <mich...@niedermayer.cc> wrote: > > > > > > > > > > > > > > Fixes: out of array read > > > > > > > Fixes: > > > > > > > 71726/clusterfuzz-testcase-ffmpeg_SWS_fuzzer-5876893532880896 > > > > > > > > > > > > > > Found-by: continuous fuzzing process > > > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > > > > > --- > > > > > > > libswscale/swscale_unscaled.c | 2 ++ > > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > diff --git a/libswscale/swscale_unscaled.c > > > > > > > b/libswscale/swscale_unscaled.c > > > > > > > index dc1d5f35932..d403c953cc7 100644 > > > > > > > --- a/libswscale/swscale_unscaled.c > > > > > > > +++ b/libswscale/swscale_unscaled.c > > > > > > > @@ -230,6 +230,8 @@ static void nv24_to_yuv420p_chroma(uint8_t > > > > > > > *dst1, int dstStride1, > > > > > > > const uint8_t *src2 = src + srcStride; > > > > > > > // average 4 pixels into 1 (interleaved U and V) > > > > > > > for (int y = 0; y < h; y += 2) { > > > > > > > + if (y + 1 == h) > > > > > > > + src2 = src1; > > > > > > > for (int x = 0; x < w; x++) { > > > > > > > dst1[x] = (src1[4 * x + 0] + src1[4 * x + 2] + > > > > > > > src2[4 * x + 0] + src2[4 * x + 2]) >> 2; > > > > > > > > > > > > I would prefer to keep nv24_to_yuv420p_chroma() expecting height to > > > > > > be > > > > > > a multiple of 2. We could add && !(c->srcH & 1) before selecting > > > > > > nv24ToYuv420Wrapper. > > > > > > > > > > what advantage does this have ? > > > > > > > > This would also have the benefit of keeping the function clearer, and > > > > we wouldn't have to add special cases that might or might not be ok. > > > > > > > It would also make it easier to write simd code, since we wouldn't > > > > have to worry about these special cases. > > > > > > The SIMD code could support a subset of the c code > > > > > > > > > > > > > > It is also easy to forget these corner cases, which leads to some > > > > converters behaving differently in corner cases. For example, the > > > > current yuyv422 to yuv420p scaler just doesn't output the last line if > > > > it has an odd height. What is the correct fix? Should we convert the > > > > last line differently like in this case? Should we duplicate the > > > > second-to-last line? Should we just add a couple lines of code to > > > > tweak the converter? Should we instead call this function with an even > > > > height, and call another function for the last line? > > > > > > > > > > > IMO a converter to yuv420p with odd width or height shouldn't be an > > > > unscaled converter, since we're effectively scaling the chroma planes > > > > (and not just by a factor of two). In this case, the regular scaler > > > > would be used. > > > > > > Iam not sure i understand correctly, but > > > assuming a 99 lines 4:4:4 scaled to 99 lines 4:2:0 > > > the chroma here is not a simple "99 to 50 lines rescaling" > > > the chroma samples must stay co-located in relation to the luma samples > > > that can only happen with s 2:1 not a 99:50 rescaling > > > but maybe i misunderstood what you meant > > > > The fuzzer found another sample for this issue: > > 377735917/clusterfuzz-testcase-minimized-ffmpeg_SWS_fuzzer-6686071112400896 > > > > I still think my patch is fine, if someone else wants to disable the code > > from being used for odd sizes thats not breaking the code, it just would > > then not be used in some cases it could be used for. > > > > If you prefer to only disable this for odd cases, please disable it. > > I just want the bug fixed and not left open > > I planned on going over the unscaled subsampled conversion functions > and making sure they all work with odd widths and heights in a > consistent way, but I haven't gotten around to it, and I don't think > I'll have time in the near future. > > So I guess for now this patch is ok.
will apply thanks! [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus
signature.asc
Description: PGP signature
_______________________________________________ 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".
