On Sun, Oct 30, 2011 at 08:36:07PM +0100, Kostya Shishkov wrote:
> On Sun, Oct 30, 2011 at 08:01:52PM +0100, Diego Biurrun wrote:
> > On Sun, Oct 30, 2011 at 07:05:22PM +0100, Kostya Shishkov wrote:
> > > Here's a new hit from Elvis Presley - You're The Devil in Disguise.
> > > Nothing to do with Apple this time though.
> >
> > Nah, Elvis is French nowadays and the author of this code is
> > German or Russian depending on how you want to see it..
>
> Whatever, it's traditional ;)
>
> > > /**
> > > * @file
> > > * This is a decoder for Intel Indeo Video v3.
> > > * It's based on vector quantization, run-length coding and motion
> > > compensation.
> >
> > nit: it is
>
> I find this contraction perfectly valid though.
The idea is to avoid them in slightly more formal written English,
whatever..
> > > if (luma_width < 16 || luma_width > 640 ||
> > > luma_height < 16 || luma_height > 480 ||
> > > luma_width & 3 || luma_height & 3) {
> > > av_log(avctx, AV_LOG_ERROR, "Invalid picture dimensions: %d x
> > > %d!\n",
> > > luma_width, luma_height);
> > > return -1;
> >
> > AVERROR_INVALIDDATE I think.
>
> changed to AVERROR_INVALIDTIME where appropriate
lol :)
> > > ctx->frame_num = frame_num;
> > > ctx->frame_flags = bytestream_get_le16(&buf_ptr);
> > > ctx->data_size = (bytestream_get_le32(&buf_ptr) + 7) >> 3;
> >
> > pointless () - matter of taste
>
> gcc is Lispy, IIRC it prints a warning when those parentheses are not present
gcc does not let additions leave the house without parents it seems...
> /**
> * @file
> * This is a decoder for Intel Indeo Video v3.
> * It is based on vector quantization, run-length coding and motion
> compensation.
> * Known container formats: .avi and .mov
> * Known FOURCCs: 'IV31', 'IV32'
> * For documentation see:
> * @see http://wiki.multimedia.cx/index.php?title=Indeo_3
> */
This is not exactly what I had in mind. Please try running doxygen and
look at the output. The "For documentation see:" does not end up where
I think you expect it. Just drop it or place after the @see.
> /**
> * Allocate frame buffers and initialize plane descriptors.
> */
> static av_cold int allocate_frame_buffers(Indeo3DecodeContext *ctx,
> AVCodecContext *avctx)
>
> /**
> * Fill n lines with 64bit pixel value pix
> */
> static inline void fill_64(uint8_t *dst, const uint64_t pix, int32_t n,
> int32_t row_offset)
>
> /**
> * Parse plane binary tree recursively.
> */
> static int parse_bintree(Indeo3DecodeContext *ctx, AVCodecContext *avctx,
> Plane *plane, int code, Cell *ref_cell,
> const int depth, const int strip_width)
If you Add Doxygen comments for the functions without documenting the
parameters documentation is incomplete. Plus we will get a new warning
for each missing parameter.
> /**
> * Replicate each even pixel as follows:
> * ABCDEFGH -> AACCEEGG
> */
> static inline uint64_t replicate64(uint64_t a) {
> #if HAVE_BIGENDIAN
> a &= 0xFF00FF00FF00FF00;
> a |= a >> 8;
> #else
> a &= 0x00FF00FF00FF00FF;
> a |= a << 8;
> #endif
> return a;
> }
>
> static inline uint32_t replicate32(uint32_t a) {
> #if HAVE_BIGENDIAN
> a &= 0xFF00FF00;
> a |= a >> 8;
> #else
> a &= 0x00FF00FF;
> a |= a << 8;
> #endif
> return a;
> }
same here
Also, if one is doxygenized, so should the other.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
