Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@anishmgoyal
anishmgoyal committed Jun 10, 2024
1 parent 549f79e commit 4a9ce60
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 26 deletions.
22 changes: 16 additions & 6 deletions include/ppx/graphics_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,23 @@ class TextureOptions
grfx::Texture** ppTexture,
const TextureOptions& options);

// Splits the frames from a raw video, based on the format of the frames,
// and metadata such as height and width. This assumes raw video, with no
// metadata in the file itself, and no audio tracks (such as a camera
// feed). Returns if the operation succeeded.
// path: The path to the file containing the video.
// format: Describes the video format; this is important for determining
// the size of a frame.
// width: The width of each frame, in pixels, with no subsampling applied.
// height: The height of each frame, in pixels, with no subsampling applied.
// pFrames: A vector where resulting frames will be stored. This should not
// be null.
friend Result LoadFramesFromRawVideo(
grfx::Queue* pQueue,
const std::filesystem::path& path,
uint32_t width,
uint32_t height,
grfx::Texture** ppTexture,
const TextureOptions& options);
const std::filesystem::path& path,
grfx::Format format,
uint32_t width,
uint32_t height,
std::vector<std::vector<char>>* pFrames);
};

//! @fn CreateTextureFromBitmap
Expand Down
17 changes: 14 additions & 3 deletions include/ppx/grfx/grfx_format.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,24 @@ enum FormatPlaneChromaType
FORMAT_PLANE_CHROMA_TYPE_CHROMA,
};

// Note: this is distinct from FormatComponentBit because in the case of a
// member of an image plane, we only want to be able to specify one component
// bit.
enum FormatPlaneComponentType
{
FORMAT_PLANE_COMPONENT_TYPE_UNDEFINED,
FORMAT_PLANE_COMPONENT_TYPE_RED,
FORMAT_PLANE_COMPONENT_TYPE_GREEN,
FORMAT_PLANE_COMPONENT_TYPE_BLUE,
};

struct FormatPlaneDesc
{
struct Member
{
// Note: it's expected that only one bit would be set here. That being
// said, this is mostly to add clarity to plane component definitions.
FormatComponentBit component;
// For debugging purposes: the color component that this plane member
// describes.
FormatPlaneComponentType component;
// This defines whether this is a luma value, chroma value, or neither
// (will be set to undefined for non-YCbCr types).
FormatPlaneChromaType type;
Expand Down
56 changes: 45 additions & 11 deletions src/ppx/graphics_util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,35 @@ namespace grfx_util {

namespace {

const uint32_t kYuvWidth = 3152; // 3000;
const uint32_t kYuvHeight = 3840; // 3000;

// Start planar image helper functions

uint32_t GetPlaneHeightForCopy(
// Gets the height of a single plane, in terms of number of pixels represented.
// This doesn't directly correlate to the number of bits / bytes for the plane's
// height. The value returned can be used in a copy-image-to-buffer command.
// plane: The plane to get the height for (containing information about the
// color components represented in the plane).
// subsampling: The type of subsampling applied to chroma values for the image
// (e.g. 444, 422, 420).
// imageHeight: The height of the image, in pixels, with no subsampling applied.
uint32_t GetPlaneHeightInPixels(
const grfx::FormatPlaneDesc::Plane& plane,
grfx::FormatChromaSubsampling subsampling,
uint32_t imageHeight)
{
bool hasColSubsampling = subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420;
bool hasColSubsampling = (subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420);
bool hasChromaValue = false;
bool hasLumaValue = false;
for (auto it = plane.members.begin(); it != plane.members.end(); ++it) {
const grfx::FormatPlaneDesc::Member& member = *it;
if (member.type == grfx::FORMAT_PLANE_CHROMA_TYPE_CHROMA) {
hasChromaValue = true;
}
else {
else if (member.type == grfx::FORMAT_PLANE_CHROMA_TYPE_LUMA) {
hasLumaValue = true;
}
else {
PPX_LOG_WARN("Member " << member.component << "has unknown chroma type.");
}
}

if (hasColSubsampling && hasChromaValue) {
Expand All @@ -76,13 +84,21 @@ uint32_t GetPlaneHeightForCopy(
return imageHeight;
}

uint32_t GetPlaneWidthForCopy(
// Gets the width of a single plane, in terms of number of pixels represented.
// This doesn't directly correlate to the number of bits / bytes for the plane's
// height. The value returned can be used in a copy-image-to-buffer command.
// plane: The plane to get the width for (containing information about the
// color components represented in the plane).
// subsampling: The type of subsampling applied to chroma values for the image
// (e.g. 444, 422, 420).
// imageWidth: The width of the image, in pixels, with no subsampling applied.
uint32_t GetPlaneWidthInPixels(
const grfx::FormatPlaneDesc::Plane& plane,
grfx::FormatChromaSubsampling subsampling,
uint32_t imageWidth)
{
bool hasRowSubsampling = subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420 ||
subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_422;
bool hasRowSubsampling = (subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420) ||
(subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_422);
bool hasChromaValue = false;
for (auto it = plane.members.begin(); it != plane.members.end(); ++it) {
const grfx::FormatPlaneDesc::Member& member = *it;
Expand All @@ -102,14 +118,21 @@ uint32_t GetPlaneWidthForCopy(
return imageWidth;
}

// Gets the size of an image plane in bytes.
// plane: The plane to get information for. (Contains information about the
// color components represented by this plane, and their bit counts).
// subsampling: The type of chroma subsampling applied to this image (e.g.
// 444, 422, 420).
// width: The width of the image, in pixels, with no subsampling applied.
// height: The height of the image, in pixels, with no subsampling applied.
uint32_t GetPlaneSizeInBytes(
const grfx::FormatPlaneDesc::Plane& plane,
grfx::FormatChromaSubsampling subsampling,
uint32_t width,
uint32_t height)
{
bool hasColSubsampling = subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420;
bool hasRowSubsampling = hasColSubsampling || subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_422;
bool hasColSubsampling = (subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_420);
bool hasRowSubsampling = hasColSubsampling || (subsampling == grfx::FORMAT_CHROMA_SUBSAMPLING_422);
bool hasChromaValue = false;
bool hasLumaValue = false;
uint32_t rowBitFactor = 0;
Expand All @@ -121,6 +144,9 @@ uint32_t GetPlaneSizeInBytes(
else if (member.type == grfx::FORMAT_PLANE_CHROMA_TYPE_LUMA) {
hasLumaValue = true;
}
else {
PPX_LOG_WARN("Member " << member.component << "has unknown chroma type.");
}

// We only subsample chroma values.
if (member.type == grfx::FORMAT_PLANE_CHROMA_TYPE_CHROMA && hasRowSubsampling) {
Expand Down Expand Up @@ -152,6 +178,13 @@ uint32_t GetPlaneSizeInBytes(
return (width * rowBitFactor * height) / 8;
}

// Gets the total size of a planar image in bytes, by calculating the size of
// each plane individually.
// formatDesc: Information about the image format, such as the components
// represented, etc.
// planeDesc: Information about the components in the current image plane.
// width: The width of the image, in pixels, with no subsampling applied.
// height: The height of the image, in pixels, with no subsampling applied.
uint32_t GetPlanarImageSizeInBytes(
const grfx::FormatDesc& formatDesc,
const grfx::FormatPlaneDesc& planeDesc,
Expand Down Expand Up @@ -1697,6 +1730,7 @@ Result LoadFramesFromRawVideo(
return ppx::ERROR_FAILED;
}
pFrames->push_back(std::move(buffer));
totalRead += readSize;
}

return ppx::SUCCESS;
Expand Down
12 changes: 6 additions & 6 deletions src/ppx/grfx/grfx_format.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ namespace grfx {
#define OFFSETS_RGBA(R, G, B, A) { R, G, B, A }
// clang-format on

#define PLANE_MEMBER(Component, Type, BitSize) \
FormatPlaneDesc::Member \
{ \
FORMAT_COMPONENT_##Component, \
FORMAT_PLANE_CHROMA_TYPE_##Type, \
BitSize \
#define PLANE_MEMBER(Component, Type, BitSize) \
FormatPlaneDesc::Member \
{ \
FORMAT_PLANE_COMPONENT_TYPE_##Component, \
FORMAT_PLANE_CHROMA_TYPE_##Type, \
BitSize \
}

FormatPlaneDesc::FormatPlaneDesc(std::initializer_list<std::initializer_list<FormatPlaneDesc::Member>>&& planes)
Expand Down

0 comments on commit 4a9ce60

Please sign in to comment.