From caee8748da5c25b928f699bfa9f1ac4a5f3ae0ce Mon Sep 17 00:00:00 2001 From: wm4 Date: Sun, 17 May 2020 14:57:13 +0200 Subject: video: clean up some imgfmt related stuff Remove the vaguely defined plane_bits and component_bits fields from struct mp_imgfmt_desc. Add weird replacements for existing uses. Remove the bytes[] field, replace uses with bpp[]. Fix some potential alignment issues in existing code. As a compromise, split mp_image_pixel_ptr() into 2 functions, because I think it's a bad idea to implicitly round, but for some callers being slightly less strict is convenient. This shouldn't really change anything. In fact, it's a 100% useless change. I'm just cleaning up what I started almost 8 years ago (see commit 00653a3eb052). With this I've decided to keep mp_imgfmt_desc, just removing the weird parts, and keeping the saner parts. --- video/image_writer.c | 14 ++++++++++--- video/img_format.c | 43 ++++++---------------------------------- video/img_format.h | 18 +++++------------ video/mp_image.c | 55 +++++++++++++++++++++++++++++++++++++--------------- video/mp_image.h | 1 + video/repack.c | 16 +++++++-------- video/sws_utils.c | 2 +- 7 files changed, 71 insertions(+), 78 deletions(-) (limited to 'video') diff --git a/video/image_writer.c b/video/image_writer.c index 8c3cf98d19..fb297f9c0f 100644 --- a/video/image_writer.c +++ b/video/image_writer.c @@ -259,9 +259,17 @@ static int get_encoder_format(struct AVCodec *codec, int srcfmt, bool highdepth) int fmt = pixfmt2imgfmt(pix_fmts[n]); if (!fmt) continue; - // Ignore formats larger than 8 bit per pixel. - if (!highdepth && IMGFMT_RGB_DEPTH(fmt) > 32) - continue; + if (!highdepth) { + // Ignore formats larger than 8 bit per pixel. (Or which are unknown.) + struct mp_regular_imgfmt rdesc; + if (!mp_get_regular_imgfmt(&rdesc, fmt)) { + int ofmt = mp_find_other_endian(fmt); + if (!mp_get_regular_imgfmt(&rdesc, ofmt)) + continue; + } + if (rdesc.component_size > 1) + continue; + } current = current ? mp_imgfmt_select_best(current, fmt, srcfmt) : fmt; } return current; diff --git a/video/img_format.c b/video/img_format.c index 90cce39126..8a2cabbf01 100644 --- a/video/img_format.c +++ b/video/img_format.c @@ -75,10 +75,7 @@ static const struct mp_imgfmt_entry mp_imgfmt_list[] = { .num_planes = 1, .align_x = 1, .align_y = 1, - .bytes = {4}, .bpp = {32}, - .plane_bits = 30, - .component_bits = 10, }, .forced_csp = MP_CSP_RGB, .ctype = MP_COMPONENT_TYPE_UINT, @@ -213,14 +210,11 @@ static struct mp_imgfmt_desc to_legacy_desc(int fmt, struct mp_regular_imgfmt re .num_planes = reg.num_planes, .chroma_xs = reg.chroma_xs, .chroma_ys = reg.chroma_ys, - .component_bits = reg.component_size * 8 - abs(reg.component_pad), }; desc.align_x = 1 << reg.chroma_xs; desc.align_y = 1 << reg.chroma_ys; - desc.plane_bits = desc.component_bits; for (int p = 0; p < reg.num_planes; p++) { - desc.bytes[p] = reg.component_size; - desc.bpp[p] = desc.bytes[p] * 8; + desc.bpp[p] = reg.component_size * 8; desc.xs[p] = p == 1 || p == 2 ? desc.chroma_xs : 0; desc.ys[p] = p == 1 || p == 2 ? desc.chroma_ys : 0; for (int c = 0; c < reg.planes[p].num_components; c++) { @@ -258,6 +252,7 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) int el_size = (pd->flags & AV_PIX_FMT_FLAG_BITSTREAM) ? 1 : 8; bool need_endian = false; // single component is spread over >1 bytes int shift = -1; // shift for all components, or -1 if not uniform + int comp_bits = 0; for (int c = 0; c < pd->nb_components; c++) { AVComponentDescriptor d = pd->comp[c]; // multiple components per plane -> Y is definitive, ignore chroma @@ -266,9 +261,9 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) planedepth[d.plane] += d.depth; need_endian |= (d.depth + d.shift) > 8; if (c == 0) - desc.component_bits = d.depth; - if (d.depth != desc.component_bits) - desc.component_bits = 0; + comp_bits = d.depth; + if (d.depth != comp_bits) + comp_bits = 0; if (c == 0) shift = d.shift; if (shift != d.shift) @@ -280,8 +275,6 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) desc.num_planes++; } - desc.plane_bits = planedepth[0]; - // Check whether any components overlap other components (per plane). // We're cheating/simplifying here: we assume that this happens if a shift // is set - which is wrong in general (could be needed for padding, instead @@ -292,7 +285,7 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) for (int c = 0; c < pd->nb_components; c++) { AVComponentDescriptor d = pd->comp[c]; component_byte_overlap |= d.shift > 0 && planedepth[d.plane] > 8 && - desc.component_bits < 8; + comp_bits < 8; } // If every component sits in its own byte, or all components are within @@ -327,8 +320,6 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) !(pd->flags & AV_PIX_FMT_FLAG_BITSTREAM)) { desc.flags |= MP_IMGFLAG_BYTE_ALIGNED; - for (int p = 0; p < desc.num_planes; p++) - desc.bytes[p] = desc.bpp[p] / 8; } if (pd->flags & AV_PIX_FMT_FLAG_PAL) @@ -360,8 +351,6 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) desc.flags |= MP_IMGFLAG_YUV_NV; } - if (desc.flags & (MP_IMGFLAG_YUV_P | MP_IMGFLAG_RGB_P | MP_IMGFLAG_YUV_NV)) - desc.component_bits += shift; } for (int p = 0; p < desc.num_planes; p++) { @@ -375,11 +364,6 @@ struct mp_imgfmt_desc mp_imgfmt_get_desc(int mpfmt) if ((desc.bpp[0] % 8) != 0) desc.align_x = 8 / desc.bpp[0]; // expect power of 2 - if (desc.flags & MP_IMGFLAG_HWACCEL) { - desc.component_bits = 0; - desc.plane_bits = 0; - } - return desc; } @@ -623,21 +607,6 @@ int mp_find_regular_imgfmt(struct mp_regular_imgfmt *src) return 0; } -// Find a format that has the given flags set with the following configuration. -int mp_imgfmt_find(int xs, int ys, int planes, int component_bits, int flags) -{ - for (int n = IMGFMT_START + 1; n < IMGFMT_END; n++) { - struct mp_imgfmt_desc desc = mp_imgfmt_get_desc(n); - if (desc.id && ((desc.flags & flags) == flags)) { - if (desc.num_planes == planes && desc.chroma_xs == xs && - desc.chroma_ys == ys && desc.plane_bits == component_bits && - (desc.flags & MP_IMGFLAG_NE)) - return desc.id; - } - } - return 0; -} - // Compare the dst image formats, and return the one which can carry more data // (e.g. higher depth, more color components, lower chroma subsampling, etc.), // with respect to what is required to keep most of the src format. diff --git a/video/img_format.h b/video/img_format.h index f3d9750585..ea46dbec70 100644 --- a/video/img_format.h +++ b/video/img_format.h @@ -72,11 +72,12 @@ struct mp_imgfmt_desc { int8_t align_x, align_y; // pixel count to get byte alignment and to get // to a pixel pos where luma & chroma aligns // always power of 2 - int8_t bytes[MP_MAX_PLANES]; // bytes per pixel (MP_IMGFLAG_BYTE_ALIGNED) - int8_t bpp[MP_MAX_PLANES]; // bits per pixel - int8_t plane_bits; // number of bits in use for plane 0 - int8_t component_bits; // number of bits per component (0 if uneven) + int8_t bpp[MP_MAX_PLANES]; // bits per pixel (may be "average"; the real + // byte value is determined by align_x*bpp/8 + // for align_x pixels) // chroma shifts per plane (provided for convenience with planar formats) + // Packed YUV always uses xs[0]=ys[0]=0, because plane 0 contains luma in + // addition to chroma, and thus is not sub-sampled (uses align_x=2 instead). int8_t xs[MP_MAX_PLANES]; int8_t ys[MP_MAX_PLANES]; }; @@ -253,13 +254,6 @@ enum mp_imgfmt { IMGFMT_END, }; -static inline bool IMGFMT_IS_RGB(int fmt) -{ - struct mp_imgfmt_desc desc = mp_imgfmt_get_desc(fmt); - return (desc.flags & MP_IMGFLAG_RGB) && desc.num_planes == 1; -} - -#define IMGFMT_RGB_DEPTH(fmt) (mp_imgfmt_get_desc(fmt).plane_bits) #define IMGFMT_IS_HWACCEL(fmt) (!!(mp_imgfmt_get_desc(fmt).flags & MP_IMGFLAG_HWACCEL)) int mp_imgfmt_from_name(bstr name); @@ -270,8 +264,6 @@ char **mp_imgfmt_name_list(void); #define vo_format_name mp_imgfmt_to_name -int mp_imgfmt_find(int xs, int ys, int planes, int component_bits, int flags); - int mp_imgfmt_select_best(int dst1, int dst2, int src); int mp_imgfmt_select_best_list(int *dst, int num_dst, int src); diff --git a/video/mp_image.c b/video/mp_image.c index cc581013da..e5c6d957f7 100644 --- a/video/mp_image.c +++ b/video/mp_image.c @@ -47,6 +47,10 @@ static int mp_image_layout(int imgfmt, int w, int h, int stride_align, int out_plane_size[MP_MAX_PLANES]) { struct mp_imgfmt_desc desc = mp_imgfmt_get_desc(imgfmt); + + w = MP_ALIGN_UP(w, desc.align_x); + h = MP_ALIGN_UP(h, desc.align_y); + struct mp_image_params params = {.imgfmt = imgfmt, .w = w, .h = h}; if (!mp_image_params_valid(¶ms) || desc.flags & MP_IMGFLAG_HWACCEL) @@ -61,9 +65,6 @@ static int mp_image_layout(int imgfmt, int w, int h, int stride_align, int alloc_h = MP_ALIGN_UP(h, 32) >> desc.ys[n]; int line_bytes = (alloc_w * desc.bpp[n] + 7) / 8; out_stride[n] = MP_ALIGN_UP(line_bytes, stride_align); - // also align to a multiple of desc.bytes[n] - while (desc.bytes[n] && out_stride[n] % desc.bytes[n]) - out_stride[n] += stride_align; out_plane_size[n] = out_stride[n] * alloc_h; } if (desc.flags & MP_IMGFLAG_PAL) @@ -214,13 +215,15 @@ int mp_chroma_div_up(int size, int shift) // Return the storage width in pixels of the given plane. int mp_image_plane_w(struct mp_image *mpi, int plane) { - return mp_chroma_div_up(mpi->w, mpi->fmt.xs[plane]); + return mp_chroma_div_up(MP_ALIGN_UP(mpi->w, mpi->fmt.align_x), + mpi->fmt.xs[plane]); } // Return the storage height in pixels of the given plane. int mp_image_plane_h(struct mp_image *mpi, int plane) { - return mp_chroma_div_up(mpi->h, mpi->fmt.ys[plane]); + return mp_chroma_div_up(MP_ALIGN_UP(mpi->h, mpi->fmt.align_y), + mpi->fmt.ys[plane]); } // Caller has to make sure this doesn't exceed the allocated plane data/strides. @@ -572,7 +575,10 @@ void mp_image_clear(struct mp_image *img, int x0, int y0, int x1, int y1) } else if (area.fmt.flags & MP_IMGFLAG_YUV_NV) { plane_clear[1] = 0x8080; } else if (area.fmt.flags & MP_IMGFLAG_YUV_P) { - uint16_t chroma_clear = (1 << area.fmt.plane_bits) / 2; + struct mp_regular_imgfmt desc = {0}; + mp_get_regular_imgfmt(&desc, img->imgfmt); + int depth = desc.component_size * 8 + MPMIN(0, desc.component_pad); + uint16_t chroma_clear = (1 << depth) / 2; if (!(area.fmt.flags & MP_IMGFLAG_NE)) chroma_clear = av_bswap16(chroma_clear); if (area.num_planes > 2) @@ -582,7 +588,7 @@ void mp_image_clear(struct mp_image *img, int x0, int y0, int x1, int y1) for (int p = 0; p < area.num_planes; p++) { int bpp = area.fmt.bpp[p]; - int bytes = (mp_image_plane_w(&area, p) * bpp + 7) / 8; + int bytes = mp_image_plane_bytes(&area, p, 0, area.w); if (bpp <= 8 || bpp > 16) { memset_pic(area.planes[p], plane_clear[p], bytes, mp_image_plane_h(&area, p), area.stride[p]); @@ -1045,22 +1051,39 @@ void memset16_pic(void *dst, int fill, int unitsPerLine, int height, int stride) } } -// Pixel at the given luma position on the given plane, possibly rounded down. +// Pixel at the given luma position on the given plane. x/y always refer to +// non-subsampled coordinates (even if plane is chroma). +// The coordinates must be aligned to mp_imgfmt_desc.align_x/y (these are byte +// and chroma boundaries). +// You cannot access e.g. individual luma pixels on the luma plane with yuv420p. void *mp_image_pixel_ptr(struct mp_image *img, int plane, int x, int y) { + assert(MP_IS_ALIGNED(x, img->fmt.align_x)); + assert(MP_IS_ALIGNED(y, img->fmt.align_y)); + return mp_image_pixel_ptr_ny(img, plane, x, y); +} + +// Like mp_image_pixel_ptr(), but do not require alignment on Y coordinates if +// the plane does not require it. Use with care. +// Useful for addressing luma rows. +void *mp_image_pixel_ptr_ny(struct mp_image *img, int plane, int x, int y) +{ + assert(MP_IS_ALIGNED(x, img->fmt.align_x)); + assert(MP_IS_ALIGNED(y, 1 << img->fmt.ys[plane])); return img->planes[plane] + img->stride[plane] * (ptrdiff_t)(y >> img->fmt.ys[plane]) + - (size_t)(x >> img->fmt.xs[plane]) * img->fmt.bpp[plane] / 8; + (x >> img->fmt.xs[plane]) * (size_t)img->fmt.bpp[plane] / 8; } -// Number of bytes for w pixels, using luma pixels, possibly rounded up. -// x0 is the start pixel; matters if the start pixel is rounded down. -// (E.g. 8 bpp, x0=7, w=7 => pixels 0..15 => 2 bytes) +// Return size of pixels [x0, x0+w-1] in bytes. The coordinates refer to non- +// subsampled pixels (basically plane 0), and the size is rounded to chroma +// and byte alignment boundaries for the entire image, even if plane!=0. +// x0!=0 is useful for rounding (e.g. 8 bpp, x0=7, w=7 => 0..15 => 2 bytes). size_t mp_image_plane_bytes(struct mp_image *img, int plane, int x0, int w) { - int bpp = img->fmt.bpp[plane]; + int x1 = MP_ALIGN_UP(x0 + w, img->fmt.align_x); + x0 = MP_ALIGN_DOWN(x0, img->fmt.align_x); + size_t bpp = img->fmt.bpp[plane]; int xs = img->fmt.xs[plane]; - size_t b_x0 = (x0 >> xs) * bpp / 8; - size_t b_x1 = (((x0 + w + (1 << xs) - 1) >> xs) * bpp + 7) / 8; - return b_x1 - b_x0; + return (x1 >> xs) * bpp / 8 - (x0 >> xs) * bpp / 8; } diff --git a/video/mp_image.h b/video/mp_image.h index f111d35129..5b7eada496 100644 --- a/video/mp_image.h +++ b/video/mp_image.h @@ -186,6 +186,7 @@ void memset_pic(void *dst, int fill, int bytesPerLine, int height, int stride); void memset16_pic(void *dst, int fill, int unitsPerLine, int height, int stride); void *mp_image_pixel_ptr(struct mp_image *img, int plane, int x, int y); +void *mp_image_pixel_ptr_ny(struct mp_image *img, int plane, int x, int y); size_t mp_image_plane_bytes(struct mp_image *img, int plane, int x0, int w); #endif /* MPLAYER_MP_IMAGE_H */ diff --git a/video/repack.c b/video/repack.c index db9bc31ac7..f5d8739fc4 100644 --- a/video/repack.c +++ b/video/repack.c @@ -132,8 +132,8 @@ static void copy_plane(struct mp_image *dst, int dst_x, int dst_y, assert(dst->fmt.bpp[p] == src->fmt.bpp[p]); for (int y = 0; y < h; y++) { - void *pd = mp_image_pixel_ptr(dst, p, dst_x, dst_y + y); - void *ps = mp_image_pixel_ptr(src, p, src_x, src_y + y); + void *pd = mp_image_pixel_ptr_ny(dst, p, dst_x, dst_y + y); + void *ps = mp_image_pixel_ptr_ny(src, p, src_x, src_y + y); memcpy(pd, ps, size); } } @@ -147,17 +147,17 @@ static void swap_endian(struct mp_image *dst, int dst_x, int dst_y, for (int p = 0; p < dst->fmt.num_planes; p++) { int xs = dst->fmt.xs[p]; - int bpp = dst->fmt.bytes[p]; + int bpp = dst->fmt.bpp[p] / 8; int words_per_pixel = bpp / endian_size; int num_words = ((w + (1 << xs) - 1) >> xs) * words_per_pixel; // Number of lines on this plane. int h = (1 << dst->fmt.chroma_ys) - (1 << dst->fmt.ys[p]) + 1; - assert(src->fmt.bytes[p] == bpp); + assert(src->fmt.bpp[p] == bpp * 8); for (int y = 0; y < h; y++) { - void *s = mp_image_pixel_ptr(src, p, src_x, src_y + y); - void *d = mp_image_pixel_ptr(dst, p, dst_x, dst_y + y); + void *s = mp_image_pixel_ptr_ny(src, p, src_x, src_y + y); + void *d = mp_image_pixel_ptr_ny(dst, p, dst_x, dst_y + y); switch (endian_size) { case 2: for (int x = 0; x < num_words; x++) @@ -851,8 +851,8 @@ static void repack_float(struct mp_repack *rp, for (int p = 0; p < b->num_planes; p++) { int h = (1 << b->fmt.chroma_ys) - (1 << b->fmt.ys[p]) + 1; for (int y = 0; y < h; y++) { - void *pa = mp_image_pixel_ptr(a, p, a_x, a_y + y); - void *pb = mp_image_pixel_ptr(b, p, b_x, b_y + y); + void *pa = mp_image_pixel_ptr_ny(a, p, a_x, a_y + y); + void *pb = mp_image_pixel_ptr_ny(b, p, b_x, b_y + y); packer(pa, pb, w >> b->fmt.xs[p], rp->f32_m[p], rp->f32_o[p], rp->f32_pmax[p]); diff --git a/video/sws_utils.c b/video/sws_utils.c index faa61670b1..800e4f34b3 100644 --- a/video/sws_utils.c +++ b/video/sws_utils.c @@ -400,7 +400,7 @@ struct mp_image *mp_img_swap_to_native(struct mp_image *img) } if (to == AV_PIX_FMT_NONE || !mp_image_make_writeable(img)) return img; - int elems = img->fmt.bytes[0] / 2 * img->w; + int elems = img->fmt.bpp[0] / 8 / 2 * img->w; for (int y = 0; y < img->h; y++) { uint16_t *p = (uint16_t *)(img->planes[0] + y * img->stride[0]); for (int i = 0; i < elems; i++) -- cgit v1.2.3