Re: [FFmpeg-devel] [PATCH] area changed:in cfhd height initialization was buggy for chroma plane
2018-03-16 11:27 GMT+01:00, Gagandeep Singh : > From: Gagandeep Singh > diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c > index fd834b..a064cd1599 100644 > --- a/libavcodec/cfhd.c > +++ b/libavcodec/cfhd.c > @@ -195,14 +195,14 @@ static int alloc_buffers(AVCodecContext *avctx) > int w8, h8, w4, h4, w2, h2; > int width = i ? avctx->width >> chroma_x_shift : avctx->width; > int height = i ? avctx->height >> chroma_y_shift : avctx->height; > -ptrdiff_t stride = FFALIGN(width / 8, 8) * 8; > -height = FFALIGN(height / 8, 2) * 8; > +ptrdiff_t stride = FFALIGN(width / 8, 8) * 8; > +if (chroma_y_shift) height = FFALIGN(height / 8, 2) * 8; > s->plane[i].width = width; > s->plane[i].height = height; > s->plane[i].stride = stride; > > w8 = FFALIGN(s->plane[i].width / 8, 8); > -h8 = FFALIGN(s->plane[i].height / 8, 2); > +h8 = height / 8; In addition to what was already discussed, this patch "breaks" fate (it is of course our mistake to use a broken test but you still have to fix the issue now). Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] area changed:in cfhd height initialization was buggy for chroma plane
2018-03-17 7:05 GMT+01:00, Gagandeep Singh : > Thanks, sure will take care next time :) Next task for you is to find out what "top-posting" means and avoid it here;-) Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] area changed:in cfhd height initialization was buggy for chroma plane
Thanks, sure will take care next time :) On Fri, 16 Mar 2018, 20:26 Carl Eugen Hoyos, wrote: > 2018-03-16 11:27 GMT+01:00, Gagandeep Singh : > > From: Gagandeep Singh > > Thank you for the important patch! > > The first line of the commit message should not start with "area changed" > but something similar to "lavc/cfhd: " which means you can also remove > "cfhd" from the rest of the first line. > > > description:when the chroma_y_shift was not present, the FFALIGN used to > > round the height was unnecessary for 0 chroma shift in y direction. > > The word description is unnecessary. > Please mention ticket #6675, without it is impossible to ever do a > regression test. > > > --- > > libavcodec/cfhd.c | 6 +++--- > > > libavcodec/tests/codec_desc | Bin 0 -> 189776 bytes > > You should not add the file codec_desc to your commit... > > Thank you, 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
Re: [FFmpeg-devel] [PATCH] area changed:in cfhd height initialization was buggy for chroma plane
2018-03-16 11:27 GMT+01:00, Gagandeep Singh : > From: Gagandeep Singh Thank you for the important patch! The first line of the commit message should not start with "area changed" but something similar to "lavc/cfhd: " which means you can also remove "cfhd" from the rest of the first line. > description:when the chroma_y_shift was not present, the FFALIGN used to > round the height was unnecessary for 0 chroma shift in y direction. The word description is unnecessary. Please mention ticket #6675, without it is impossible to ever do a regression test. > --- > libavcodec/cfhd.c | 6 +++--- > libavcodec/tests/codec_desc | Bin 0 -> 189776 bytes You should not add the file codec_desc to your commit... Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
