fix race condition in i420 video converter leading to video artefacts (#418)

* fix race condition in i420 video converter leading to video artefacts

* avoid copy when not necessary

* store conversion result in new slice instead of copy slice
This commit is contained in:
Valentin Cocaud
2022-08-01 09:40:23 +02:00
committed by GitHub
parent 43272ea965
commit a3c15d1fb0
6 changed files with 57 additions and 32 deletions
+4 -5
View File
@@ -73,16 +73,15 @@ func ToI420(r Reader) Reader {
// Covert pixel format to I420
switch yuvImg.SubsampleRatio {
case image.YCbCrSubsampleRatio444:
i444ToI420(&yuvImg)
case image.YCbCrSubsampleRatio422:
i422ToI420(&yuvImg)
case image.YCbCrSubsampleRatio420:
case image.YCbCrSubsampleRatio444:
yuvImg = i444ToI420(yuvImg)
case image.YCbCrSubsampleRatio422:
yuvImg = i422ToI420(yuvImg)
default:
return nil, func() {}, fmt.Errorf("unsupported pixel format: %s", yuvImg.SubsampleRatio)
}
yuvImg.SubsampleRatio = image.YCbCrSubsampleRatio420
return &yuvImg, func() {}, nil
})
}
+8 -4
View File
@@ -3,6 +3,8 @@
#include "_cgo_export.h"
void i444ToI420CGO(
unsigned char *cb_dst,
unsigned char *cr_dst,
unsigned char* cb,
unsigned char* cr,
const int stride, const int h)
@@ -22,8 +24,8 @@ void i444ToI420CGO(
((uint16_t)cr[isrc0] + (uint16_t)cr[isrc1] +
(uint16_t)cr[isrc0 + 1] + (uint16_t)cr[isrc1 + 1]) /
4;
cb[idst] = cb2;
cr[idst] = cr2;
cb_dst[idst] = cb2;
cr_dst[idst] = cr2;
isrc0 += 2;
isrc1 += 2;
idst++;
@@ -34,6 +36,8 @@ void i444ToI420CGO(
}
void i422ToI420CGO(
unsigned char *cb_dst,
unsigned char *cr_dst,
unsigned char* cb,
unsigned char* cr,
const int stride, const int h)
@@ -46,8 +50,8 @@ void i422ToI420CGO(
{
const uint8_t cb2 = ((uint16_t)cb[isrc] + (uint16_t)cb[isrc + stride]) / 2;
const uint8_t cr2 = ((uint16_t)cr[isrc] + (uint16_t)cr[isrc + stride]) / 2;
cb[idst] = cb2;
cr[idst] = cr2;
cb_dst[idst] = cb2;
cr_dst[idst] = cr2;
isrc++;
idst++;
}
+17 -8
View File
@@ -1,3 +1,4 @@
//go:build cgo
// +build cgo
package video
@@ -14,27 +15,35 @@ import "C"
// All functions switched at runtime must be declared also in convert_nocgo.go.
const hasCGOConvert = true
func i444ToI420(img *image.YCbCr) {
func i444ToI420(img image.YCbCr) image.YCbCr {
h := img.Rect.Dy()
cLen := img.CStride * h / 4
cbDst, crDst := make([]uint8, cLen), make([]uint8, cLen)
C.i444ToI420CGO(
(*C.uchar)(&cbDst[0]), (*C.uchar)(&crDst[0]),
(*C.uchar)(&img.Cb[0]), (*C.uchar)(&img.Cr[0]),
C.int(img.CStride), C.int(h),
)
img.CStride = img.CStride / 2
cLen := img.CStride * (h / 2)
img.Cb = img.Cb[:cLen]
img.Cr = img.Cr[:cLen]
img.Cb = cbDst
img.Cr = crDst
img.SubsampleRatio = image.YCbCrSubsampleRatio420
return img
}
func i422ToI420(img *image.YCbCr) {
func i422ToI420(img image.YCbCr) image.YCbCr {
h := img.Rect.Dy()
cLen := img.CStride * (h / 2)
cbDst, crDst := make([]uint8, cLen), make([]uint8, cLen)
C.i422ToI420CGO(
(*C.uchar)(&cbDst[0]), (*C.uchar)(&crDst[0]),
(*C.uchar)(&img.Cb[0]), (*C.uchar)(&img.Cr[0]),
C.int(img.CStride), C.int(h),
)
cLen := img.CStride * (h / 2)
img.Cb = img.Cb[:cLen]
img.Cr = img.Cr[:cLen]
img.Cb = cbDst
img.Cr = crDst
img.SubsampleRatio = image.YCbCrSubsampleRatio420
return img
}
func rgbToYCbCrCGO(y, cb, cr *uint8, r, g, b uint8) { // For testing
+4
View File
@@ -1,9 +1,13 @@
void i444ToI420CGO(
unsigned char *cb_dst,
unsigned char *cr_dst,
unsigned char* cb,
unsigned char* cr,
const int stride, const int h);
void i422ToI420CGO(
unsigned char *cb_dst,
unsigned char *cr_dst,
unsigned char* cb,
unsigned char* cr,
const int stride, const int h);
+22 -13
View File
@@ -1,3 +1,4 @@
//go:build !cgo
// +build !cgo
package video
@@ -9,19 +10,22 @@ import (
const hasCGOConvert = false
func i444ToI420(img *image.YCbCr) {
func i444ToI420(img image.YCbCr) image.YCbCr {
h := img.Rect.Dy()
addrSrc0 := 0
addrSrc1 := img.CStride
cLen := img.CStride * (h / 2)
addrDst := 0
cbDst, crDst := make([]uint8, cLen), make([]uint8, cLen)
for i := 0; i < h/2; i++ {
for j := 0; j < img.CStride/2; j++ {
cb := uint16(img.Cb[addrSrc0]) + uint16(img.Cb[addrSrc1]) +
uint16(img.Cb[addrSrc0+1]) + uint16(img.Cb[addrSrc1+1])
cr := uint16(img.Cr[addrSrc0]) + uint16(img.Cr[addrSrc1]) +
uint16(img.Cr[addrSrc0+1]) + uint16(img.Cr[addrSrc1+1])
img.Cb[addrDst] = uint8(cb / 4)
img.Cr[addrDst] = uint8(cr / 4)
cbDst[addrDst] = uint8(cb / 4)
crDst[addrDst] = uint8(cr / 4)
addrSrc0 += 2
addrSrc1 += 2
addrDst++
@@ -30,29 +34,34 @@ func i444ToI420(img *image.YCbCr) {
addrSrc1 += img.CStride
}
img.CStride = img.CStride / 2
cLen := img.CStride * (h / 2)
img.Cb = img.Cb[:cLen]
img.Cr = img.Cr[:cLen]
img.Cb = cbDst
img.Cr = crDst
img.SubsampleRatio = image.YCbCrSubsampleRatio420
return img
}
func i422ToI420(img *image.YCbCr) {
func i422ToI420(img image.YCbCr) image.YCbCr {
h := img.Rect.Dy()
addrSrc := 0
cLen := img.CStride * (h / 2)
cbDst, crDst := make([]uint8, cLen), make([]uint8, cLen)
addrDst := 0
for i := 0; i < h/2; i++ {
for j := 0; j < img.CStride; j++ {
cb := uint16(img.Cb[addrSrc]) + uint16(img.Cb[addrSrc+img.CStride])
cr := uint16(img.Cr[addrSrc]) + uint16(img.Cr[addrSrc+img.CStride])
img.Cb[addrDst] = uint8(cb / 2)
img.Cr[addrDst] = uint8(cr / 2)
addrDst++
cbDst[addrDst] = uint8(cb / 4)
crDst[addrDst] = uint8(cr / 4)
addrSrc++
addrDst++
}
addrSrc += img.CStride
}
cLen := img.CStride * (h / 2)
img.Cb = img.Cb[:cLen]
img.Cr = img.Cr[:cLen]
img.Cb = cbDst
img.Cr = crDst
img.SubsampleRatio = image.YCbCrSubsampleRatio420
return img
}
func i444ToRGBA(dst *image.RGBA, src *image.YCbCr) {
+2 -2
View File
@@ -222,8 +222,8 @@ func BenchmarkToI420(b *testing.B) {
"RGBA": image.NewRGBA(image.Rect(0, 0, sz[0], sz[1])),
}
b.Run(name, func(b *testing.B) {
for name, img := range cases {
img := img
for _, name := range [...]string{"I444", "I422", "I420", "RGBA"} {
img := cases[name]
b.Run(name, func(b *testing.B) {
r := ToI420(ReaderFunc(func() (image.Image, func(), error) {
return img, func() {}, nil