[FFmpeg-devel] [PATCH] [RFC] Second try at pixdesc.h:write_line()

[Stefano Sabatini]

On date Sunday 2009-04-19 17:44:02 +0200, Michael Niedermayer encoded:
> On Sun, Apr 19, 2009 at 05:20:57PM +0200, Stefano Sabatini wrote:
[...]
> > +/**
> > + * Writes the values from \p src to the pixel format components \p c
> > + * of an image line.
> 
> \p ...

Fixed.

> > + *
> > + * @param data the array containing the pointers to the planes of the image
> > + * @param linesizes the array containing the linesizes of the image
> > + * @param desc the pixel format descriptor for the image
> > + * @param x the horizontal coordinate of the first pixel to write
> > + * @param y the vertical coordinate of the first pixel to write
> > + * @param w the width of the line to write, that is the number of
> > + * values to write to the image line
> > + */
> > +static inline void write_line(const uint16_t *src, uint8_t *data[4], const int linesize[4],
> > +                              const AVPixFmtDescriptor *desc, int x, int y, int c, int w)
> 
> you are missing a param in the doxy

OK, though is almost redundant.
 
> > +{
> > +    AVComponentDescriptor comp = desc->comp[c];
> > +    int plane = comp.plane;
> > +    int depth = comp.depth_minus1+1;
> > +    int step  = comp.step_minus1+1;
> > +    int flags = desc->flags;
> > +    uint32_t mask  = (1<<depth)-1;
> > +
> > +    if (flags & PIX_FMT_BITSTREAM) {
> 
> > +        int skip = x*step + comp.offset_plus1-1;
> > +        uint8_t *p = data[plane] + y*linesize[plane] + (skip>>3);
> > +        int shift = 8 - depth - (skip&7);
> > +
> > +        while (w--) {
> > +            *p |= *src++ << shift;
> > +            shift -= step;
> > +            p -= shift>>3;
> > +            shift &= 7;
> > +        }
> 
> isnt this alot nicer than the redesigned bitstream writer? 

Well but I was looking for a more general solution, for which this one
couldn't work.

BTW, should be change accordingly read_line()?
 
> > +    } else {
> > +        int shift = comp.shift;
> > +        uint8_t *p = data[plane]+ y*linesize[plane] + x*step + comp.offset_plus1-1;
> > +
> > +        while (w--) {
> > +            if (flags & PIX_FMT_BE) {
> > +                uint16_t val = (AV_RB16(p) & ~(mask<<shift)) | (*src++<<shift);
> > +                AV_WB16(p, val);
> > +            } else {
> > +                uint16_t val = (AV_RL16(p) & ~(mask<<shift)) | (*src++<<shift);
> 
> mask is useless, your initial buffer is supposed to be zeroed

Fixed.
  
[...]
 
> this is just for testing?
> either way, there is no chance i would approve this for svn.
> for fliping of common formats its too slow and for uncommon ones
> its far too ugly, you can fliped the "unpacked" line

I see.
 
Regards.
-- 
FFmpeg = Fundamental & Forgiving Martial Pitiful Extended Generator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pixdesc-write-line.patch
Type: text/x-diff
Size: 2169 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090419/515ed0fc/attachment.patch>