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. 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. _______________________________________________ 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".
