Skip to content

Commit d7d5c5a

Browse files
committed
Fix correctness, safety, and performance issues in video frame handling
Addresses three critical issues identified in code review: 1. Performance: Extract duplicate stride calculation logic into a shared calculateAlignedStride helper function to eliminate code duplication between NewFramebuffer and resizeMat 2. Memory Safety: Add buffer bounds validation in avcodec_decoder_convert_frame to prevent potential buffer overruns when stride exceeds allocated space 3. Const Correctness: Remove const qualifier from avcodec_decoder_decode as it modifies decoder state during multi-frame extraction
1 parent 2524f7d commit d7d5c5a

File tree

3 files changed

+25
-16
lines changed

3 files changed

+25
-16
lines changed

avcodec.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,14 @@ static int avcodec_decoder_convert_frame(const avcodec_decoder d, opencv_mat mat
496496
// This ensures consistency with OpenCV operations and encoding
497497
int stepSize = cvMat->step;
498498

499+
// Validate that the stride and height are within the allocated buffer bounds
500+
size_t required_size = stepSize * cvMat->rows;
501+
size_t available_size = (cvMat->datalimit && cvMat->data) ?
502+
(cvMat->datalimit - cvMat->data) : 0;
503+
if (available_size > 0 && required_size > available_size) {
504+
return -1;
505+
}
506+
499507
// Create SwsContext for converting the frame format and scaling
500508
struct SwsContext* sws =
501509
sws_getContext(frame->width,
@@ -582,7 +590,7 @@ static int avcodec_decoder_decode_packet(const avcodec_decoder d, opencv_mat mat
582590
return res;
583591
}
584592

585-
bool avcodec_decoder_decode(const avcodec_decoder d, opencv_mat mat)
593+
bool avcodec_decoder_decode(avcodec_decoder d, opencv_mat mat)
586594
{
587595
if (!d || !d->container || !d->codec || !mat) {
588596
return false;

avcodec.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ int avcodec_decoder_get_width(const avcodec_decoder d);
1717
int avcodec_decoder_get_height(const avcodec_decoder d);
1818
int avcodec_decoder_get_orientation(const avcodec_decoder d);
1919
float avcodec_decoder_get_duration(const avcodec_decoder d);
20-
bool avcodec_decoder_decode(const avcodec_decoder d, opencv_mat mat);
20+
bool avcodec_decoder_decode(avcodec_decoder d, opencv_mat mat);
2121
bool avcodec_decoder_is_streamable(const opencv_mat buf);
2222
bool avcodec_decoder_has_subtitles(const avcodec_decoder d);
2323
const char* avcodec_decoder_get_description(const avcodec_decoder d);

opencv.go

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,19 @@ func (h *ImageHeader) ContentLength() int {
197197
return h.contentLength
198198
}
199199

200+
// calculateAlignedStride computes a 32-byte aligned stride for the given width and channel count.
201+
// This alignment is crucial for SIMD performance.
202+
// For example, with 4 channels (BGRA): 8 pixels * 4 bytes = 32 bytes alignment.
203+
func calculateAlignedStride(width, channels int) int {
204+
stride := width * channels
205+
alignmentPixels := 32 / channels // 8 for 4-channel, 16 for 2-channel, etc.
206+
if alignmentPixels > 0 && width%alignmentPixels != 0 {
207+
alignedWidth := width + alignmentPixels - (width % alignmentPixels)
208+
stride = alignedWidth * channels
209+
}
210+
return stride
211+
}
212+
200213
// NewFramebuffer creates a backing store for a pixel frame buffer with the specified dimensions.
201214
// The buffer is allocated with 32-byte aligned strides for optimal SIMD performance.
202215
//
@@ -209,13 +222,7 @@ func (h *ImageHeader) ContentLength() int {
209222
//
210223
// The resizeMat method will use the appropriate stride based on the actual pixel type.
211224
func NewFramebuffer(width, height int) *Framebuffer {
212-
// Calculate aligned stride (32-byte aligned for SIMD performance)
213-
// 8 pixels * 4 bytes = 32 bytes
214-
stride := width * 4
215-
if width%8 != 0 {
216-
alignedWidth := width + 8 - (width % 8)
217-
stride = alignedWidth * 4
218-
}
225+
stride := calculateAlignedStride(width, 4) // Allocate for 4-channel (BGRA)
219226
return &Framebuffer{
220227
buf: make([]byte, stride*height),
221228
mat: nil,
@@ -268,13 +275,7 @@ func (f *Framebuffer) resizeMat(width, height int, pixelType PixelType) error {
268275
}
269276

270277
// Calculate aligned stride (32-byte aligned for SIMD performance)
271-
// For BGRA (4 channels): 8 pixels * 4 bytes = 32 bytes
272-
stride := width * pixelType.Channels()
273-
alignmentPixels := 32 / pixelType.Channels() // 8 for 4-channel, 16 for 2-channel, etc.
274-
if alignmentPixels > 0 && width%alignmentPixels != 0 {
275-
alignedWidth := width + alignmentPixels - (width % alignmentPixels)
276-
stride = alignedWidth * pixelType.Channels()
277-
}
278+
stride := calculateAlignedStride(width, pixelType.Channels())
278279

279280
newMat := C.opencv_mat_create_from_data_with_stride(C.int(width), C.int(height), C.int(pixelType), unsafe.Pointer(&f.buf[0]), C.size_t(len(f.buf)), C.size_t(stride))
280281
if newMat == nil {

0 commit comments

Comments
 (0)