[FFmpeg-devel] [PATCH] Doxument some of SwsContext.
Stefano Sabatini
stefano.sabatini-lala
Sun Jan 17 01:02:34 CET 2010
On date Saturday 2010-01-16 20:40:42 -0200, Ramiro Polla encoded:
[...]
> diff --git a/swscale_internal.h b/swscale_internal.h
> index 65174fd..8053bcc 100644
> --- a/swscale_internal.h
> +++ b/swscale_internal.h
> @@ -82,51 +82,81 @@ typedef struct SwsContext {
> int chrSrcH; ///< Height of source chroma planes.
> int chrDstW; ///< Width of destination chroma planes.
> int chrDstH; ///< Height of destination chroma planes.
General comment, our convention for the doxy of a field is: "upcased
if not a complete sentence or an incomplete sentence followed by a
complete sentence" so these doxies should be downcased and without the
final point, as they are not complete sentences.
> - int lumXInc, chrXInc;
> - int lumYInc, chrYInc;
> + int lumXInc; ///< Horizontal scale factor between source and destination luma/alpha planes.
> + int chrXInc; ///< Horizontal scale factor between source and destination chroma planes.
> + int lumYInc; ///< Vertical scale factor between source and destination luma/alpha planes.
> + int chrYInc; ///< Vertical scale factor between source and destination chroma planes.
While these doxies are a big step for improving general understanding
of the fields, I still cannot perfectly understand what these fields
are for.
What is a scale factor between a source and a destination plane? I
suppose here it is meant the size, that is e.g.:
dstLumPlaneSize = lumXInc * srcLumPlaneSize
also is this field only meaningful for planar and not for
paletted/packed formats?
Also I'm not happy about the "Inc" in the fields names, maybe the doxy
should make clear what that suffix means (I suppose it means "Increment").
> enum PixelFormat dstFormat; ///< Destination pixel format.
> enum PixelFormat srcFormat; ///< Source pixel format.
> - int chrSrcHSubSample, chrSrcVSubSample;
> - int chrDstHSubSample, chrDstVSubSample;
> - int vChrDrop;
> - int sliceDir;
> + int chrSrcHSubSample; ///< Binary logarithm of horizontal subsampling factor between luma/alpha and chroma planes in source image.
> + int chrSrcVSubSample; ///< Binary logarithm of vertical subsampling factor between luma/alpha and chroma planes in source image.
> + int chrDstHSubSample; ///< Binary logarithm of horizontal subsampling factor between luma/alpha and chroma planes in destination image.
> + int chrDstVSubSample; ///< Binary logarithm of vertical subsampling factor between luma/alpha and chroma planes in destination image.
> + int vChrDrop; ///< Binary logarithm of extra vertical subsampling factor in source image chroma planes specified by user.
> + int sliceDir; ///< Direction that slices are fed to the scaler (1 = top-to-bottom, -1 = bottom-to-top).
Maybe we could define some enum, e.g. enum SWSSliceDirection {
SWS_SLICE_DIR_TOP_DOWN = -1,
SWS_SLICE_DIR_NONE = 0,
SWS_SLICE_DIR_BOTTOM_UP = 1
};
I'm not sure if it is possible to have headache-lessly negative values
in an enumerated type.
> double param[2]; ///< Input parameters for scaling algorithms that need them.
>
> - uint32_t pal_yuv[256];
> - uint32_t pal_rgb[256];
> -
> - int16_t **lumPixBuf;
> - int16_t **chrPixBuf;
> - int16_t **alpPixBuf;
> - int16_t *hLumFilter;
> - int16_t *hLumFilterPos;
> - int16_t *hChrFilter;
> - int16_t *hChrFilterPos;
> - int16_t *vLumFilter;
> - int16_t *vLumFilterPos;
> - int16_t *vChrFilter;
> - int16_t *vChrFilterPos;
> -
> - uint8_t formatConvBuffer[VOF]; //FIXME dynamic allocation, but we have to change a lot of code for this to be useful
> -
> - int hLumFilterSize;
> - int hChrFilterSize;
> - int vLumFilterSize;
> - int vChrFilterSize;
> - int vLumBufSize;
> - int vChrBufSize;
> + uint32_t pal_yuv[256]; ///< YUV values corresponding to palettized data from source pixel format.
> + uint32_t pal_rgb[256]; ///< RGB32 values corresponding to palettized data from source pixel format.
> +
> + /**
> + * @name Scaled horizontal lines ring buffer.
> + * The horizontal scaler keeps just enough scaled lines in a ring buffer
> + * so they may be passed to the vertical scaler. The pointers to the
> + * allocated buffers for each line are duplicated in sequence in the ring
> + * buffer to simplify indexing and avoid wrapping around between lines
> + * inside the vertical scaler code. The wrapping is done before the
> + * vertical scaler is called.
> + */
> + //@{
> + int16_t **lumPixBuf; ///< Ring buffer for scaled horizontal luma plane lines to be fed to the vertical scaler.
> + int16_t **chrPixBuf; ///< Ring buffer for scaled horizontal chroma plane lines to be fed to the vertical scaler.
> + int16_t **alpPixBuf; ///< Ring buffer for scaled horizontal alpha plane lines to be fed to the vertical scaler.
> + int vLumBufSize; ///< Number of vertical luma/alpha lines allocated in the ring buffer.
> + int vChrBufSize; ///< Number of vertical chroma lines allocated in the ring buffer.
> + int lastInLumBuf; ///< Last scaled horizontal luma/alpha line from source in the ring buffer.
> + int lastInChrBuf; ///< Last scaled horizontal chroma line from source in the ring buffer.
Are these lines or indexes? In the latter case I suggest to use
explicitely the term "index" in the name.
> + int lumBufIndex; ///< Index in ring buffer of the last scaled horizontal luma/alpha line from source.
> + int chrBufIndex; ///< Index in ring buffer of the last scaled horizontal chroma line from source.
Maybe it's just me, but I cannot understand the difference between
these fields and the previous ones, seems to me that the
{lum,chr}BufIndex doxies are just a re-formulation of the
lastIn{Lum,Chr}Buf doxies, but with a mention to the term "index".
> + //@}
> +
> + //FIXME dynamic allocation, but we have to change a lot of code for this to be useful
> + uint8_t formatConvBuffer[VOF]; ///< Intermediate horizontal scaler buffer used for unscaled conversion of input data to YV12.
> +
> + /**
> + * @name Horizontal and vertical filters.
> + * To better understand the following fields, here is a pseudo-code of
> + * their usage in filtering a horizontal line:
> + * @code
> + * for (i = 0; i < width; i++) {
> + * dst[i] = 0;
> + * for (j = 0; j < filterSize; j++)
> + * dst[i] += src[ filterPos[i] + j ] * filter[ filterSize * i + j ];
> + * }
> + * @endcode
> + */
> + //@{
> + int16_t *hLumFilter; ///< Array of horizontal filter coefficients for luma/alpha planes.
> + int16_t *hChrFilter; ///< Array of horizontal filter coefficients for chroma planes.
> + int16_t *vLumFilter; ///< Array of vertical filter coefficients for luma/alpha planes.
> + int16_t *vChrFilter; ///< Array of vertical filter coefficients for chroma planes.
> + int16_t *hLumFilterPos; ///< Array of horizontal filter starting positions for each dst[i] for luma/alpha planes.
> + int16_t *hChrFilterPos; ///< Array of horizontal filter starting positions for each dst[i] for chroma planes.
> + int16_t *vLumFilterPos; ///< Array of vertical filter starting positions for each dst[i] for luma/alpha planes.
> + int16_t *vChrFilterPos; ///< Array of vertical filter starting positions for each dst[i] for chroma planes.
> + int hLumFilterSize; ///< Horizontal filter size for luma/alpha pixels.
> + int hChrFilterSize; ///< Horizontal filter size for chroma pixels.
> + int vLumFilterSize; ///< Vertical filter size for luma/alpha pixels.
> + int vChrFilterSize; ///< Vertical filter size for chroma pixels.
> + //@}
Fine and much needed explanation.
> int lumMmx2FilterCodeSize; ///< Runtime-generated MMX2 horizontal fast bilinear scaler code size for luma/alpha planes.
> int chrMmx2FilterCodeSize; ///< Runtime-generated MMX2 horizontal fast bilinear scaler code size for chroma planes.
> uint8_t *lumMmx2FilterCode; ///< Runtime-generated MMX2 horizontal fast bilinear scaler code for luma/alpha planes.
> uint8_t *chrMmx2FilterCode; ///< Runtime-generated MMX2 horizontal fast bilinear scaler code for chroma planes.
>
> - int canMMX2BeUsed;
> + int canMMX2BeUsed; ///< Whether the runtime-generated MMX2 horizontal fast bilinear scaler can be used.
>
> - int lastInLumBuf;
> - int lastInChrBuf;
> - int lumBufIndex;
> - int chrBufIndex;
> int dstY; ///< Last destination vertical line output from last slice.
> int flags; ///< Flags passed by the user to select scaler algorithm, optimizations, subsampling, etc...
> void * yuvTable; // pointer to the yuv->rgb table start so it can be freed()
> @@ -183,7 +213,7 @@ typedef struct SwsContext {
> DECLARE_ALIGNED(8, uint64_t, vOffset);
> int32_t lumMmxFilter[4*MAX_FILTER_SIZE];
> int32_t chrMmxFilter[4*MAX_FILTER_SIZE];
> - int dstW;
> + int dstW; ///< Width of destination luma/alpha planes.
> DECLARE_ALIGNED(8, uint64_t, esp);
> DECLARE_ALIGNED(8, uint64_t, vRounder);
> DECLARE_ALIGNED(8, uint64_t, u_temp);
> @@ -221,16 +251,26 @@ typedef struct SwsContext {
> #endif
>
> /* function pointers for swScale() */
> +
> + /**
> + * Generic vertical YV12 to NV12 scaler.
> + */
I'd like to avoid the term "YV12", as it is an MPlayer-ism, for
example there is no mention of it in libavutil/pixfmt.h.
I suggest to use instead "YUV" (I may be wrong here).
> void (*yuv2nv12X )(struct SwsContext *c,
> const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> uint8_t *dest, uint8_t *uDest,
> int dstW, int chrDstW, int dstFormat);
> + /**
> + * Vertical YV12 to YV12 converter without scaling or interpolating.
> + */
> void (*yuv2yuv1 )(struct SwsContext *c,
> const int16_t *lumSrc, const int16_t *chrSrc, const int16_t *alpSrc,
> uint8_t *dest,
> uint8_t *uDest, uint8_t *vDest, uint8_t *aDest,
> long dstW, long chrDstW);
> + /**
> + * Generic vertical YV12 to YV12 scaler.
> + */
> void (*yuv2yuvX )(struct SwsContext *c,
> const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> @@ -238,39 +278,67 @@ typedef struct SwsContext {
> uint8_t *dest,
> uint8_t *uDest, uint8_t *vDest, uint8_t *aDest,
> long dstW, long chrDstW);
> + /**
> + * Vertical YV12 to RGB converter interpolating chrominance for every second line.
> + */
Please use always the same terminology, is a "converter" a "scaler" or
are they different things?
> void (*yuv2packed1)(struct SwsContext *c,
> const uint16_t *buf0,
> const uint16_t *uvbuf0, const uint16_t *uvbuf1,
> const uint16_t *abuf0,
> uint8_t *dest,
> int dstW, int uvalpha, int dstFormat, int flags, int y);
> + /**
> + * Vertical bilinear YV12 to RGB scaler.
> + */
> void (*yuv2packed2)(struct SwsContext *c,
> const uint16_t *buf0, const uint16_t *buf1,
> const uint16_t *uvbuf0, const uint16_t *uvbuf1,
> const uint16_t *abuf0, const uint16_t *abuf1,
> uint8_t *dest,
> int dstW, int yalpha, int uvalpha, int y);
> + /**
> + * Generic vertical YV12 to RGB scaler.
> + */
> void (*yuv2packedX)(struct SwsContext *c,
> const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> const int16_t **alpSrc, uint8_t *dest,
> long dstW, long dstY);
>
> + /**
> + * Unscaled conversion of luma plane to YV12 for horizontal scaler.
> + */
> void (*lumToYV12)(uint8_t *dst, const uint8_t *src,
> - long width, uint32_t *pal); ///< Unscaled conversion of luma plane to YV12 for horizontal scaler.
> + long width, uint32_t *pal);
> + /**
> + * Unscaled conversion of alpha plane to YV12 for horizontal scaler.
> + */
> void (*alpToYV12)(uint8_t *dst, const uint8_t *src,
> - long width, uint32_t *pal); ///< Unscaled conversion of alpha plane to YV12 for horizontal scaler.
> + long width, uint32_t *pal);
> + /**
> + * Unscaled conversion of chroma planes to YV12 for horizontal scaler.
> + */
> void (*chrToYV12)(uint8_t *dstU, uint8_t *dstV,
> const uint8_t *src1, const uint8_t *src2,
> - long width, uint32_t *pal); ///< Unscaled conversion of chroma planes to YV12 for horizontal scaler.
> + long width, uint32_t *pal);
"Unscaled conversion" -> scaler or converter if they are such.
Also I'm not much happy about this YV12, also for some fields we use
"To", for other ones "To", this leads to confusion, maybe a general
review on the names should be done.
> +
> + /**
> + * Fast bilinear horizontal scaler for luma/alpha planes.
> + */
> void (*hyscale_fast)(struct SwsContext *c,
> int16_t *dst, long dstWidth,
> const uint8_t *src, int srcW, int xInc);
> + /**
> + * Fast bilinear horizontal scaler for chroma planes.
> + */
> void (*hcscale_fast)(struct SwsContext *c,
> int16_t *dst, long dstWidth,
> const uint8_t *src1, const uint8_t *src2,
> int srcW, int xInc);
>
> + /**
> + * Generic horizontal scaler.
> + */
> void (*hScale)(int16_t *dst, int dstW, const uint8_t *src, int srcW,
> int xInc, const int16_t *filter, const int16_t *filterPos,
> long filterSize);
> diff --git a/swscale_template.c b/swscale_template.c
> index 76297d6..6e3690d 100644
> --- a/swscale_template.c
> +++ b/swscale_template.c
> @@ -1019,9 +1019,6 @@ static inline void RENAME(yuv2yuv1)(SwsContext *c, const int16_t *lumSrc, const
> }
>
>
> -/**
> - * vertical scale YV12 to RGB
> - */
> static inline void RENAME(yuv2packedX)(SwsContext *c, const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> const int16_t **alpSrc, uint8_t *dest, long dstW, long dstY)
> @@ -1204,9 +1201,6 @@ static inline void RENAME(yuv2packedX)(SwsContext *c, const int16_t *lumFilter,
> alpSrc, dest, dstW, dstY);
> }
>
> -/**
> - * vertical bilinear scale YV12 to RGB
> - */
> static inline void RENAME(yuv2packed2)(SwsContext *c, const uint16_t *buf0, const uint16_t *buf1, const uint16_t *uvbuf0, const uint16_t *uvbuf1,
> const uint16_t *abuf0, const uint16_t *abuf1, uint8_t *dest, int dstW, int yalpha, int uvalpha, int y)
> {
> @@ -1353,9 +1347,6 @@ static inline void RENAME(yuv2packed2)(SwsContext *c, const uint16_t *buf0, cons
> YSCALE_YUV_2_ANYRGB_C(YSCALE_YUV_2_RGB2_C, YSCALE_YUV_2_PACKED2_C(void,0), YSCALE_YUV_2_GRAY16_2_C, YSCALE_YUV_2_MONO2_C)
> }
>
> -/**
> - * YV12 to RGB without scaling or interpolating
> - */
> static inline void RENAME(yuv2packed1)(SwsContext *c, const uint16_t *buf0, const uint16_t *uvbuf0, const uint16_t *uvbuf1,
> const uint16_t *abuf0, uint8_t *dest, int dstW, int uvalpha, enum PixelFormat dstFormat, int flags, int y)
> {
Regards and thanks for the much needed doxumentation job.
--
FFmpeg = Fantastic & Friendly Minimalistic Ponderous Extravagant Gymnast
More information about the ffmpeg-devel
mailing list