[Ffmpeg-cvslog] r6161 - in trunk: libavcodec/dv.c libavcodec/dvdata.h libavformat/avformat.h libavformat/dv.c tests/ffmpeg.regression.ref tests/libav.regression.ref tests/rotozoom.regression.ref

[Michael Niedermayer]

Hi

On Mon, Sep 04, 2006 at 05:33:12AM +0200, romansh wrote:
> Author: romansh
> Date: Mon Sep  4 05:33:11 2006
> New Revision: 6161
> 
> Modified:
>    trunk/libavcodec/dv.c
>    trunk/libavcodec/dvdata.h
>    trunk/libavformat/avformat.h
>    trunk/libavformat/dv.c
>    trunk/tests/ffmpeg.regression.ref
>    trunk/tests/libav.regression.ref
>    trunk/tests/rotozoom.regression.ref
> 
> Log:
> 
>   * Restructuring the division of labor between DV codec and DV format
>     [ Based on a patch by Brian Brice (bbrice at newtek dot com) ]

[...]
> +enum dv_pack_type {
> +     dv_header525     = 0x3f, /* see dv_write_pack for important details on */
> +     dv_header625     = 0xbf, /* these two packs */

not doxygen compatible comment


[...]
> Modified: trunk/libavformat/avformat.h
> ==============================================================================
> --- trunk/libavformat/avformat.h	(original)
> +++ trunk/libavformat/avformat.h	Mon Sep  4 05:33:11 2006
> @@ -434,6 +434,16 @@
>  void fifo_write(FifoBuffer *f, const uint8_t *buf, int size, uint8_t **wptr_ptr);
>  int put_fifo(ByteIOContext *pb, FifoBuffer *f, int buf_size, uint8_t **rptr_ptr);
>  void fifo_realloc(FifoBuffer *f, unsigned int size);
> +static inline uint8_t fifo_peek(FifoBuffer *f, int offs)
> +{
> +    return f->buffer[(f->rptr - f->buffer + offs) % (f->end - f->buffer)];
> +}
> +static inline void fifo_drain(FifoBuffer *f, int size)
> +{
> +    f->rptr += size;
> +    if (f->rptr >= f->end)
> +        f->rptr = f->buffer + (f->rptr - f->end);
> +}

avformat.h is a public header, changes to it should be disscussed on
ffmpeg-dev (even more so as you are not the maintainer of it)
such static inline functions are also not a good idea in a public header
because they make the user app unnecesarily depend on the implementation
and thus lead to binary compatibility issues, furthermore these functions
are used just by the dv code so they really dont belong to avformat.h
and does the usage of these functions even merit that they are static inline?

please reverse this change to avformat.h
additionally, expect to loose your svn write account if you add such stuff
to non dv specific code the next time without discussion on ffmpeg-dev.
you have done this several times by now ...


[...]
> Modified: trunk/tests/ffmpeg.regression.ref
> ==============================================================================
> --- trunk/tests/ffmpeg.regression.ref	(original)
> +++ trunk/tests/ffmpeg.regression.ref	Mon Sep  4 05:33:11 2006
> @@ -141,11 +141,11 @@
>  3534748 ./data/a-snow53.avi
>  799d3db687f6cdd7a837ec156efc171f *./data/out.yuv
>  stddev:  0.00 PSNR:99.99 bytes:7602176
> -85529d9838c5a8aef041445fa8ef3c85 *./data/a-dv.dv
> +2fcbcdc63816e1321bf4b6b5380338d2 *./data/a-dv.dv
>  7200000 ./data/a-dv.dv
>  c2082cd8adf417c4ebc32654e446cba1 *./data/out.yuv
>  stddev:  8.86 PSNR:29.17 bytes:7602176
> -ff5eb084624f9a2846840a215b95a112 *./data/a-dv.dv
> +4f71942eb699bf3b12508a9e777a319f *./data/a-dv.dv
>  14400000 ./data/a-dv.dv
>  fc866b8879a34f5b440647e5135e4bfb *./data/out.yuv
>  stddev:  8.45 PSNR:29.58 bytes:7602176
> 
> Modified: trunk/tests/libav.regression.ref
> ==============================================================================
> --- trunk/tests/libav.regression.ref	(original)
> +++ trunk/tests/libav.regression.ref	Mon Sep  4 05:33:11 2006
> @@ -28,7 +28,7 @@
>  8bf16d40a2ec19fa36b124a928e47e23 *./data/b-libav.nut
>   332358 ./data/b-libav.nut
>  ./data/b-libav.nut CRC=0x1fdd352c
> -2cc647904d3cca01703f36056b9d3f42 *./data/b-libav.dv
> +26c41db318d9aacfd6b9e734c0ea4d94 *./data/b-libav.dv
>  3600000 ./data/b-libav.dv
>  ./data/b-libav.dv CRC=0xa6b8b635
>  304ee17506e4526ba04285f6cd14630f *./data/b-libav.gxf
> 
> Modified: trunk/tests/rotozoom.regression.ref
> ==============================================================================
> --- trunk/tests/rotozoom.regression.ref	(original)
> +++ trunk/tests/rotozoom.regression.ref	Mon Sep  4 05:33:11 2006
> @@ -141,11 +141,11 @@
>  2726606 ./data/a-snow53.avi
>  dde5895817ad9d219f79a52d0bdfb001 *./data/out.yuv
>  stddev:  0.00 PSNR:99.99 bytes:7602176
> -6568326ee439099e8a0e1680336ed351 *./data/a-dv.dv
> +af9f474238c9c68cb32e389659ee25ab *./data/a-dv.dv
>  7200000 ./data/a-dv.dv
>  bb69dda7a84a5b166434e28e1243d3d1 *./data/out.yuv
>  stddev:  2.99 PSNR:38.59 bytes:7602176
> -177cbbbe50cbb67ae3bd6e7f10ff968b *./data/a-dv.dv
> +91dc1c1dc4a8cca72f27d85db6d7636f *./data/a-dv.dv
>  14400000 ./data/a-dv.dv
>  74b01209bb5e096d570dd4df112bb82d *./data/out.yuv
>  stddev:  2.98 PSNR:38.61 bytes:7602176

could you explain why the regression checksums change, your commit message
makes the change sound like a pure restructuring which wouldnt change the
output?
also update the commit message with the awnser

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is