[FFmpeg-devel] GSoC Weely report (libswscale)

[Michael Niedermayer]

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
> >>
> >>
> >

>  Makefile           |    1 
>  slice.c            |  558 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  swscale.c          |   87 +++++++-
>  swscale_internal.h |   60 +++++
>  utils.c            |    1 
>  5 files changed, 699 insertions(+), 8 deletions(-)

breaks
./ffplay -f lavfi testsrc  -vf format=yuvj420p
(its all grey except last line)



> b1296cf793d33f7c3ec7899dfcf2c85c52a662d7  swscale.patch
> diff --git a/libswscale/Makefile b/libswscale/Makefile
> index a60b057..d876e75 100644
> --- a/libswscale/Makefile
> +++ b/libswscale/Makefile
> @@ -14,6 +14,7 @@ OBJS = hscale_fast_bilinear.o                           \
>         swscale_unscaled.o                               \
>         utils.o                                          \
>         yuv2rgb.o                                        \
> +       slice.o                                          \
>  
>  OBJS-$(CONFIG_SHARED)        += log2_tab.o
>  
> diff --git a/libswscale/slice.c b/libswscale/slice.c
> new file mode 100644
> index 0000000..bb99ac1
> --- /dev/null
> +++ b/libswscale/slice.c
> @@ -0,0 +1,558 @@
> +#include "swscale_internal.h"
> +
> +static void free_lines(SwsSlice *s)
> +{
> +    int i;

> +    for (i = 0; i < 4; ++i)
> +    {

its not important ATM but before this can be commited the {}
placement style should match to the rest of libswscale


[...]


> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 1945e1d..9f33af2 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -315,6 +315,8 @@ static av_always_inline void hcscale(SwsContext *c, int16_t *dst1,
>      if (DEBUG_SWSCALE_BUFFERS)                  \
>          av_log(c, AV_LOG_DEBUG, __VA_ARGS__)
>  
> +
> +
>  static int swscale(SwsContext *c, const uint8_t *src[],

stray lines added


[...]

> -            if (perform_gamma)
> -                gamma_convert((uint8_t **)src1, srcW, c->inv_gamma);
> +            //if (perform_gamma)
> +            //    gamma_convert((uint8_t **)src1, srcW, c->inv_gamma);
>  
>              hyscale(c, lumPixBuf[lumBufIndex], dstW, src1, srcW, lumXInc,
>                      hLumFilter, hLumFilterPos, hLumFilterSize,

is it intentional to disable this ?


> @@ -520,12 +575,12 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>                  src[2] + (lastInChrBuf + 1 - chrSrcSliceY) * srcStride[2],
>                  src[3] + (lastInChrBuf + 1 - chrSrcSliceY) * srcStride[3],
>              };
> +
>              chrBufIndex++;
>              av_assert0(chrBufIndex < 2 * vChrBufSize);
>              av_assert0(lastInChrBuf + 1 - chrSrcSliceY < (chrSrcSliceH));
>              av_assert0(lastInChrBuf + 1 - chrSrcSliceY >= 0);
>              // FIXME replace parameters through context struct (some at least)
> -
>              if (c->needs_hcscale)
>                  hcscale(c, chrUPixBuf[chrBufIndex], chrVPixBuf[chrBufIndex],
>                          chrDstW, src1, chrSrcW, chrXInc,

stray change

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- 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/05d9634a/attachment.sig>