On Wed, Jul 15, 2015 at 08:06:49PM -0300, Pedro Arthur wrote: > Hi, > > I removed the asserts referencing unused variables and it passed all tests. > The following patch contains the changes and some improvements to the code > as a single diff. > > 2015-07-12 10:04 GMT-03:00 Pedro Arthur <[email protected]>: > > > I'll check it, I think most of these asserts are from some unused > > variables which were replaced by the SwsSlice struct. > > > > 2015-07-11 23:51 GMT-03:00 Michael Niedermayer <[email protected]>: > > > >> On Sat, Jul 11, 2015 at 04:57:22PM -0300, Pedro Arthur wrote: > >> > Here is the full patch rebased including all previous changes. > >> > >> if you build ffmpeg with --assert-level=2 > >> > >> fails fate > >> for example in fate-vsynth1-cinepak > >> > >> its probably best to always build with --assert-level=2 > >> so you wont miss such failures > >> > >> --- ./tests/ref/vsynth/vsynth1-cinepak 2015-07-11 12:25:01.268257903 > >> +0200 > >> +++ tests/data/fate/vsynth1-cinepak 2015-07-12 04:11:06.041453781 > >> +0200 > >> @@ -1,4 +0,0 @@ > >> -546c7c1069f9e418aa787f469b693b94 *tests/data/fate/vsynth1-cinepak.mov > >> -99465 tests/data/fate/vsynth1-cinepak.mov > >> -bee091c200262be3427a233a2812388c > >> *tests/data/fate/vsynth1-cinepak.out.rawvideo > >> -stddev: 8.46 PSNR: 29.58 MAXDIFF: 105 bytes: 7603200/ 456192 > >> > >> Assertion lumSrcPtr + vLumFilterSize - 1 < (const int16_t **)lumPixBuf + > >> vLumBufSize * 2 failed at libswscale/swscale.c:694 > >> > >> there are also many other fate tests which fail with --assert-level=2 > >> > >> also it might make more sense to stash all these commits together > >> for submission, > >> if you like you could keep them seperate in your branch (could make > >> sense for debug/bisect) > >> > >> but the commits showing the step wise development are rather hard to > >> review and would be impossible to push to master git > >> commits for master git should not introduce known issues that are > >> fixed in later commits and should be split in selfcontained changesets > >> > >> a single stashed together patch should be fine and easy to generate > >> > >> thanks > >> > >> [...] > >> > >> -- > >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > >> > >> Avoid a single point of failure, be that a person or equipment. > >> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> [email protected] > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> > >
[...]
> +static int alloc_slice(SwsSlice *s, enum AVPixelFormat fmt, int lumLines,
> int chrLines, int h_sub_sample, int v_sub_sample, int ring)
> +{
> + int i;
> + int err = 0;
> +
> + int size[4] = { lumLines,
> + chrLines,
> + chrLines,
> + lumLines };
> +
> + //s->width;
> + s->h_chr_sub_sample = h_sub_sample;
> + s->v_chr_sub_sample = v_sub_sample;
> + s->fmt = fmt;
> + s->is_ring = ring;
> + s->should_free_lines = 0;
> +
> + for (i = 0; i < 4; ++i)
> + {
> + int j;
> + int n = size[i] * ( ring == 0 ? 1 : 2);
> + s->plane[i].line = av_malloc_array(sizeof(uint8_t*), n);
> + if (!s->plane[i].line)
> + {
> + err = AVERROR(ENOMEM);
> + break;
> + }
> + for (int j = 0; j < n; ++j)
theres already a int j above
[...]
> +static int lum_h_scale(SwsContext *c, SwsFilterDescriptor *desc, int sliceY,
> int sliceH)
> +{
> + ScaleInstance *instance = desc->instance;
> + int srcW = desc->src->width;
> + int dstW = desc->dst->width;
> + int xInc = instance->xInc;
> +
> + int i;
> + for (i = 0; i < sliceH; ++i)
> + {
> + uint8_t ** src = desc->src->plane[0].line;
> + uint8_t ** dst = desc->dst->plane[0].line;
> + int src_pos = sliceY+i - desc->src->plane[0].sliceY;
> + int dst_pos = sliceY+i - desc->dst->plane[0].sliceY;
> +
> +
> + if (!c->hyscale_fast) {
> + c->hyScale(c, (int16_t*)dst[dst_pos], dstW, (const uint8_t
> *)src[src_pos], instance->filter,
> + instance->filter_pos, instance->filter_size);
> + } else { // fast bilinear upscale / crap downscale
> + c->hyscale_fast(c, (int16_t*)dst[dst_pos], dstW, src[src_pos],
> srcW, xInc);
> + }
> +
> + if (c->lumConvertRange)
> + c->lumConvertRange((int16_t*)dst[dst_pos], dstW);
> +
> + desc->dst->plane[0].sliceH += 1;
> +
> + if (desc->alpha)
> + {
> + src = desc->src->plane[3].line;
> + dst = desc->dst->plane[3].line;
> +
> + src_pos = sliceY+i - desc->src->plane[3].sliceY;
> + dst_pos = sliceY+i - desc->dst->plane[3].sliceY;
> +
> + desc->dst->plane[3].sliceH += 1;
> +
> + if (!c->hyscale_fast) {
> + c->hyScale(c, (int16_t*)dst[dst_pos], dstW, (const uint8_t
> *)src[src_pos], instance->filter,
> + instance->filter_pos, instance->filter_size);
> + } else { // fast bilinear upscale / crap downscale
> + c->hyscale_fast(c, (int16_t*)dst[dst_pos], dstW,
> src[src_pos], srcW, xInc);
> + }
slightly simpler would be
if (c->hyscale_fast) {
c->hyscale_fast(...
}else {
...
> + }
> + }
> +
> + return 1;
> +}
> +
> +static int lum_convert(SwsContext *c, SwsFilterDescriptor *desc, int sliceY,
> int sliceH)
> +{
> + int srcW = desc->src->width;
> + ConvertInstance * instance = desc->instance;
> + uint32_t * pal = instance->pal;
> +
> + desc->dst->plane[0].sliceY = sliceY;
> + desc->dst->plane[0].sliceH = sliceH;
> + desc->dst->plane[3].sliceY = sliceY;
> + desc->dst->plane[3].sliceH = sliceH;
> +
> + int i;
mixed declaration and code
[...]
> +int ff_init_filters(SwsContext * c)
> +{
> + int i;
> + int index;
> + int num_ydesc;
> + int num_cdesc;
> + int need_lum_conv = c->lumToYV12 || c->readLumPlanar || c->alpToYV12 ||
> c->readAlpPlanar;
> + int need_chr_conv = c->chrToYV12 || c->readChrPlanar;
> + int srcIdx, dstIdx;
> + int dst_stride = FFALIGN(c->dstW * sizeof(int16_t) + 66, 16);
> +
> + uint32_t * pal = usePal(c->srcFormat) ? c->pal_yuv :
> (uint32_t*)c->input_rgb2yuv_table;
> +
> + if (c->dstBpc == 16)
> + dst_stride <<= 1;
> +
> + num_ydesc = need_lum_conv ? 2 : 1;
> + num_cdesc = need_chr_conv ? 2 : 1;
> +
> + c->numSlice = FFMAX(num_ydesc, num_cdesc) + 1;
> + c->numDesc = num_ydesc + num_cdesc;
> + c->descIndex[0] = num_ydesc;
> + c->descIndex[1] = num_ydesc + num_cdesc;
> +
> +
> +
> + c->desc = av_malloc_array(sizeof(SwsFilterDescriptor), c->numDesc);
> + c->slice = av_malloc_array(sizeof(SwsSlice), c->numSlice);
missing malloc failure checks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
