[FFmpeg-devel] [PATCH] [RFC] Second try at pixdesc.h:write_line()
Michael Niedermayer
michaelni
Sun Apr 19 17:44:02 CEST 2009
On Sun, Apr 19, 2009 at 05:20:57PM +0200, Stefano Sabatini wrote:
> On date Sunday 2009-04-19 15:56:10 +0200, Michael Niedermayer encoded:
> > On Sun, Apr 19, 2009 at 12:35:44PM +0200, Stefano Sabatini wrote:
> > > Mmh what about this?
> > >
> > > while (w--) {
> > > *p |= *src++ << (8 - depth + shift);
> > > shift -= step;
> > > p -= shift >> 3;
> > > shift &= 7;
> > > }
> > >
> > > I'm not still sure it is what you meant. Also AFAIU this can only
> > > work for a pixel depth of 1, while pixel/source can contain pixels
> > > with a depth up to 16 bits (uint16_t).
> > >
> > > Note that I cannot shift for a negative value (result is undefined).
> >
> > i meant what i wrote
> > *p |= *src++ << shift;
> >
> > and
> > shift &= 7; done priorly in all cases makes sure this is not negative
> > so i am no sure what you meant
> > did i miss something ?
> >
> > and yes it of course wont work with >8bit per sample or where a sample
> > crosses a byte boundary but i dont think there are any pixel formats that
> > would use the bitstream case and require these that we want to support.
> > If you know of a specific example pixel format that would fail and that
> > actually exists iam interrested ...
>
> OK, thanks, it was exactly this which was puzzling me, maybe we should
> add a notice to the docs saying something like:
>
> "read_line() will not work with bitstream formats with a depth per
> sample greater than 8, or where a sample crosses a byte boundary."
>
> I'm attaching the updated hflip filter patch, which I'm using to test
> it (togheter with the pixdesc definition already posted).
>
> Regards.
> --
> FFmpeg = Foolish Fostering Multimedia Portable Ecumenical Game
> Index: libavfilter-soc/ffmpeg/libavcodec/pixdesc.h
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavcodec/pixdesc.h 2009-04-19 13:17:27.000000000 +0200
> +++ libavfilter-soc/ffmpeg/libavcodec/pixdesc.h 2009-04-19 16:51:31.000000000 +0200
> @@ -137,3 +137,53 @@
> }
> }
> }
> +
> +/**
> + * Writes the values from \p src to the pixel format components \p c
> + * of an image line.
\p ...
> + *
> + * @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
> +{
> + 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?
> + } 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
> + AV_WL16(p, val);
> + }
> + p+= step;
> + }
> + }
> +}
> Index: libavfilter-soc/ffmpeg/libavfilter/vf_hflip.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/libavfilter/vf_hflip.c 2009-04-19 17:04:08.000000000 +0200
> +++ libavfilter-soc/ffmpeg/libavfilter/vf_hflip.c 2009-04-19 17:13:36.000000000 +0200
[...]
> +static inline void write_line_flipped(const uint16_t *src, uint8_t *data[4], const int linesize[4],
> + const AVPixFmtDescriptor *desc, int x, int y, int c, int w)
> +{
> + 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;
> +
> + src += w;
> +
> + 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;
> + }
> + } 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);
> + AV_WL16(p, val);
> + }
> + p+= step;
> + }
> + }
> +}
> +
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090419/d7e90d74/attachment.pgp>
More information about the ffmpeg-devel
mailing list