On Sat, Jul 02, 2011 at 04:38:04PM +0200, Reinhard Tartler wrote:
>
> Generally, this diff was very hard to review. Perhaps some additional
> lines of context would have helped.
If you know a trick that achieves this, I'm all ears.
Thanks for your thorough review, you uncovered some issues that
I overlooked - good.
> On Sat, Jul 02, 2011 at 14:50:43 (CEST), Diego Biurrun wrote:
>
> > --- a/libavcodec/ansi.c
> > +++ b/libavcodec/ansi.c
> > @@ -153,7 +153,6 @@ static void draw_char(AVCodecContext *avctx, int c)
> >
> > /**
> > * Execute ANSI escape code
> > - * @param <0 error
> > */
> > static int execute_code(AVCodecContext * avctx, int c)
> > {
>
> Err, what did the comment want to say? Maybe this should be
> reformualated as @info or @warning.
I'll replace this by @return documentation.
> > --- a/libavcodec/vp8.c
> > +++ b/libavcodec/vp8.c
> > @@ -641,8 +641,6 @@ void decode_mb_mode(VP8Context *s, VP8Macroblock *mb,
> > int mb_x, int mb_y, uint8_
> > * @param block destination for block coefficients
> > * @param probs probabilities to use when reading trees from the bitstream
> > * @param i initial coeff index, 0 unless a separate DC block is coded
> > - * @param zero_nhood the initial prediction context for number of
> > surrounding
> > - * all-zero blocks (only left/top, so 0-2)
> > * @param qmul array holding the dc/ac dequant factor at position 0/1
> > * @return 0 if no coeffs were decoded
> > * otherwise, the index of the last coeff decoded plus one
>
> No, look at 1e7396795028af9eda3d69ada1402d6ef8996964, the doxygen
> comment should have been moved as well.
Done.
> > @@ -1037,7 +1035,6 @@ static const uint8_t subpel_idx[3][8] = {
> > * Generic MC function.
> > *
> > * @param s VP8 decoding context
> > - * @param luma 1 for luma (Y) planes, 0 for chroma (Cb/Cr) planes
> > * @param dst target buffer for block data at block position
> > * @param src reference picture buffer at origin (0, 0)
> > * @param mv motion vector (relative to block position) to get pixel
> > data from
>
> Since 64233e702a95df9167e3362e58aae4e82ce2ddf8, it is likely no longer a
> 'generic' mc function. I guess this doxy comment should be copied (and
> adapted) for vp8_mc_chroma as well.
Yes, I just need to figure out the difference between dst1 and dst2
there...
> > --- a/libavformat/url.h
> > +++ b/libavformat/url.h
> > @@ -67,8 +67,6 @@ typedef struct URLProtocol {
> > * Create a URLContext for accessing to the resource indicated by
> > * url, but do not initiate the connection yet.
> > *
> > - * @param puc pointer to the location where, in case of success, the
> > - * function puts the pointer to the created URLContext
> > * @param flags flags which control how the resource indicated by url
> > * is to be opened
> > * @return 0 in case of success, a negative value corresponding to an
>
> wrong. the parameter is named 'url' instead of 'poc', but the
> documentation itself doesn't need to be removed.
>
> > @@ -85,8 +83,6 @@ int ffurl_connect(URLContext *h);
> > * Create an URLContext for accessing to the resource indicated by
> > * url, and open it.
> > *
> > - * @param puc pointer to the location where, in case of success, the
> > - * function puts the pointer to the created URLContext
> > * @param flags flags which control how the resource indicated by url
> > * is to be opened
> > * @return 0 in case of success, a negative value corresponding to an
>
> same here.
Fixed.
Updated patchset forthcoming.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
