[FFmpeg-devel] GSoC Weely report (libswscale)
Pedro Arthur
bygrandao at gmail.com
Fri Jul 17 14:31:13 CEST 2015
The gamma code is disabled until I warp it into a SwsFilterDescriptor.
2015-07-16 13:28 GMT-03:00 Michael Niedermayer <michael at niedermayer.cc>:
> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
More information about the ffmpeg-devel
mailing list