[FFmpeg-devel] GSoC Weely report (libswscale)
Michael Niedermayer
michael at niedermayer.cc
Thu Jul 16 18:28:38 CEST 2015
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 <bygrandao at gmail.com>:
>
> > 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 <michael at niedermayer.cc>:
> >
> >> 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
> >> ffmpeg-devel at ffmpeg.org
> >> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150716/04bf0f9d/attachment.sig>
More information about the ffmpeg-devel
mailing list