media: Expose HEVC Main10 encode on macOS [chromium/src : main]
Sida Zhu (Gerrit)
Sida Zhu voted and added 5 comments![]( "Open in Gerrit")
Votes added by Sida Zhu
| Commit-Queue | +1 |
5 comments
const validateDecodedPixels =Sida ZhuCan we just make the validate pixels an arg to the test? Or always setup main10 in such a way that i can be validated?
Done
if (codec && codec.startsWith('hvc1.2.') &&Sida ZhuI don't love this, it'd be better to make it a param to the test.
Done
result_frame->set_hdr_metadata(txt_frame->hdr_metadata());Sida ZhuI don't think this will ever have hdr metadata, but seems harmless.
Acknowledged
std::vector<uint8_t> ExtractHvcC(base::span<const uint8_t> mp4) {Sida ZhuShould be a parser for this in media/formats?
Done
frame->acquire_sync_token(), gfx::ColorSpace::CreateREC709(),Sida ZhuDo not hard-code to REC709 here.
Done.
Related details
- Dale Curtis
- Qiu, Jianlin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 1 comment![]( "Open in Gerrit")
gfx::ColorSpace GetReadbackYuvColorSpace(Does all this actually work? I'm not positive that the readback path supports all these color conversion. I'd only bet on 601, 709, and 2020.
Related details
- Qiu, Jianlin
- Sida Zhu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sida Zhu (Gerrit)
Sida Zhu added 1 comment![]( "Open in Gerrit")
gfx::ColorSpace GetReadbackYuvColorSpace(Does all this actually work? I'm not positive that the readback path supports all these color conversion. I'd only bet on 601, 709, and 2020.
The lower copy path goes through `ColorSpace::ToSkYUVColorSpace()` before
`skia::BlitRGBAToYUVA()`, and that does have mappings for `FCC` and `SMPTE240M` in
addition to `601/709/2020`. I feels like we should honer the original colorspace if possible.
Related details
- Dale Curtis
- Qiu, Jianlin
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
gfx::ColorSpace GetReadbackYuvColorSpace(Sida ZhuDoes all this actually work? I'm not positive that the readback path supports all these color conversion. I'd only bet on 601, 709, and 2020.
The lower copy path goes through `ColorSpace::ToSkYUVColorSpace()` before
`skia::BlitRGBAToYUVA()`, and that does have mappings for `FCC` and `SMPTE240M` in
addition to `601/709/2020`. I feels like we should honer the original colorspace if possible.
- Qiu, Jianlin
- Sida Zhu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qiu, Jianlin (Gerrit)
Qiu, Jianlin voted and added 1 comment![]( "Open in Gerrit")
Votes added by Qiu, Jianlin
| Code-Review | +1 |
1 comment
if (source_matrix != gfx::ColorSpace::MatrixID::RGB &&
source_matrix != gfx::ColorSpace::MatrixID::GBR) {
return source_color_space.GetWithMatrixAndRange(
source_matrix, gfx::ColorSpace::RangeID::LIMITED);
}
// RGB/GBR are identity matrices. Select a YCbCr matrix from the primaries
// before RGB-to-YUV readback.
return source_color_space.GetWithMatrixAndRange(
GetYuvMatrixForPrimaries(source_color_space.GetPrimaryID()),
gfx::ColorSpace::RangeID::LIMITED);
}Previously we hard-code to REC709 so it was fine to ask for limited. Is it intentional that for source p010 at full-range, you also do this for reading back?
Related details
- Sida Zhu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sida Zhu (Gerrit)
Sida Zhu added 1 comment![]( "Open in Gerrit")
if (source_matrix != gfx::ColorSpace::MatrixID::RGB &&
source_matrix != gfx::ColorSpace::MatrixID::GBR) {
return source_color_space.GetWithMatrixAndRange(
source_matrix, gfx::ColorSpace::RangeID::LIMITED);
}
// RGB/GBR are identity matrices. Select a YCbCr matrix from the primaries
// before RGB-to-YUV readback.
return source_color_space.GetWithMatrixAndRange(
GetYuvMatrixForPrimaries(source_color_space.GetPrimaryID()),
gfx::ColorSpace::RangeID::LIMITED);
}Previously we hard-code to REC709 so it was fine to ask for limited. Is it intentional that for source p010 at full-range, you also do this for reading back?
Thanks, I traced the complete allocation and conversion path.
LIMITED is not an inherent requirement of NV12 or of
`CopyRGBATextureToVideoFrame()` in general. It is intentional for the current
macOS destination backing.
The readback pool currently always requests an NV12 mappable SharedImage. On
macOS it uses `SCANOUT_VEA_CPU_READ` together with the VideoToolbox usage, so
SharedImageFactory selects an IOSurface backing. The kNV12 SharedImage format
is currently mapped to
`kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange` (`420v`).
The IOSurface is allocated before the conversion. CopySharedImage() then
obtains writable SkSurfaces for the Y and UV planes of that IOSurface, and
`skia::BlitRGBAToYUVA()` writes the converted samples directly into those
planes. Therefore the matrix/range passed to the blit must agree with the
range represented by the destination backing. Writing full-range samples while
the IOSurface remains `420v` would make the sample values and their downstream
interpretation inconsistent.
This is different on Windows, where the same readback pool normally creates a
compound SharedImage consisting of a renderable GPU backing and a mappable
shared-memory backing. `BlitRGBAToYUVA()` writes the GPU backing first, and the
result is then copied into the shared-memory GMB. Android does not use this
accelerated GMB readback path and instead reads the RGBA texture into a CPU
RGBA frame before conversion in `VideoEncodeAcceleratorAdapter`.
A full-range P010 input does not enter this RGBA readback path. It is handled
directly by the VEA/VideoToolbox path, which selects the P010 CVPixelBuffer
format and compression-session range from the frame ColorSpace.
For Main10 encoding from an RGBA texture, the current path is:
RGBA texture -> limited-range NV12 readback
-> NV12-to-P010 in VideoEncodeAcceleratorAdapter
-> VideoToolbox
This produces valid P010 input for VideoToolbox, but it is not a true 10-bit
readback because the pixels have already been quantized to 8-bit NV12. Direct
RGBA-to-P010 readback should be implemented in a follow-up CL.
That follow-up is more involved than changing the pool format. P010 uses
R16/RG16 planes, and the current macOS Ganesh IOSurface representation cannot
expose those planes as writable SkSurfaces. It will require a supported
Graphite/Dawn multiplanar rendering path or a dedicated platform conversion
shader, together with validation of P010's 10-bit MSB packing.
Related details
- Qiu, Jianlin
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qiu, Jianlin (Gerrit)
Qiu, Jianlin added 1 comment![]( "Open in Gerrit")
if (source_matrix != gfx::ColorSpace::MatrixID::RGB &&
source_matrix != gfx::ColorSpace::MatrixID::GBR) {
return source_color_space.GetWithMatrixAndRange(
source_matrix, gfx::ColorSpace::RangeID::LIMITED);
}
// RGB/GBR are identity matrices. Select a YCbCr matrix from the primaries
// before RGB-to-YUV readback.
return source_color_space.GetWithMatrixAndRange(
GetYuvMatrixForPrimaries(source_color_space.GetPrimaryID()),
gfx::ColorSpace::RangeID::LIMITED);
}- Sida Zhu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |