Commit 6e99da76 for openh264
commit 6e99da76edeeaa5cedc400f04782aab397ccb02a
Author: Andrew Osmond <aosmond@mozilla.com>
Date: Sun Sep 20 04:17:11 2026 -0400
Fix regression when encoding very large frames (#4016)
WelsMoveMemoryWrapper rejects any source picture whose area exceeds
MAX_MBS_PER_FRAME. That limit bounds a coded frame, which
ParamValidationExt already enforces on the spatial layers, but this
copy runs before downsampling, so the input picture may legitimately be
larger. WelsInitScaledPic allocates pScaledInputPicture for
SUsedPicRect, so the guard rejects a copy the allocator sized to fit.
The result is that EncodeFrame returns cmUnsupportedData with
videoFrameTypeSkip for every frame, producing no bitstream, for any
caller encoding above 36864 macroblocks (~4096x2304) into a smaller
layer. Introduced by the validation guards added in:
https://github.com/cisco/openh264/pull/3961
Bound the copy by the destination's own iWidthInPixel/iHeightInPixel
instead.
This was caught when attempting to update OpenH264 for Firefox. There
is a web-platform test which encodes large frames:
https://searchfox.org/firefox-main/rev/572cbc53633d3ed8a0b1390828611752f0317938/testing/web-platform/tests/webcodecs/video-encoder-h264.https.any.js#29
diff --git a/codec/encoder/core/src/wels_preprocess.cpp b/codec/encoder/core/src/wels_preprocess.cpp
index d3376974..da7ff9e4 100644
--- a/codec/encoder/core/src/wels_preprocess.cpp
+++ b/codec/encoder/core/src/wels_preprocess.cpp
@@ -1446,14 +1446,18 @@ int32_t CWelsPreProcess::WelsMoveMemoryWrapper (SWelsSvcCodingParam* pSvcParam,
const int32_t kiDstStrideV = pDstPic->iLineSize[2];
if (pSrcY) {
- if (iSrcWidth <= 0 || iSrcHeight <= 0 || (iSrcWidth * iSrcHeight > (MAX_MBS_PER_FRAME << 8)))
+ if (iSrcWidth <= 0 || iSrcHeight <= 0)
+ return ENC_RETURN_INVALIDINPUT;
+ if (iSrcWidth > pDstPic->iWidthInPixel || iSrcHeight > pDstPic->iHeightInPixel)
return ENC_RETURN_INVALIDINPUT;
if (kiSrcTopOffsetY >= iSrcHeight || kiSrcLeftOffsetY >= iSrcWidth || iSrcWidth > kiSrcStrideY
|| (iSrcWidth >> 1) > kiSrcStrideU || (iSrcWidth >> 1) > kiSrcStrideV)
return ENC_RETURN_INVALIDINPUT;
}
if (pDstY) {
- if (kiTargetWidth <= 0 || kiTargetHeight <= 0 || (kiTargetWidth * kiTargetHeight > (MAX_MBS_PER_FRAME << 8)))
+ if (kiTargetWidth <= 0 || kiTargetHeight <= 0)
+ return ENC_RETURN_INVALIDINPUT;
+ if (kiTargetWidth > pDstPic->iWidthInPixel || kiTargetHeight > pDstPic->iHeightInPixel)
return ENC_RETURN_INVALIDINPUT;
if (kiTargetWidth > kiDstStrideY || (kiTargetWidth >> 1) > kiDstStrideU || (kiTargetWidth >> 1) > kiDstStrideV)
return ENC_RETURN_INVALIDINPUT;
diff --git a/test/api/encoder_test.cpp b/test/api/encoder_test.cpp
index e79be608..a00c3c35 100644
--- a/test/api/encoder_test.cpp
+++ b/test/api/encoder_test.cpp
@@ -635,6 +635,106 @@ TEST_F(EncoderInitTest, CustomChromaPlaneStridesInvalidSrc) {
ASSERT_EQ(cmUnsupportedData, rv);
}
+// A source picture is allowed to exceed the per-frame macroblock limit as long
+// as the spatial layer it is coded into does not: the preprocessor downsamples
+// the input before it is encoded. MAX_MBS_PER_FRAME bounds the coded frame, not
+// the picture handed to EncodeFrame, and callers rely on that to encode a large
+// capture into a smaller stream while leaving iPicWidth/iPicHeight at the
+// capture size.
+TEST_F(EncoderInitTest, SourceLargerThanMaxMbsPerFrame) {
+ // MAX_MBS_PER_FRAME (36864) macroblocks of 16x16 luma samples. Spelled out
+ // here because the limit lives in an internal header.
+ const int kMaxPixelsPerFrame = 36864 * 256;
+ const int kLayerWidth = 2048;
+ const int kLayerHeight = 1280;
+
+ SEncParamExt param;
+ encoder_->GetDefaultParams(¶m);
+
+ param.iUsageType = CAMERA_VIDEO_REAL_TIME;
+ param.iPicWidth = 4096;
+ param.iPicHeight = 2560;
+ param.fMaxFrameRate = 30.0f;
+ param.iSpatialLayerNum = 1;
+ param.iRCMode = RC_OFF_MODE;
+
+ ASSERT_GT(param.iPicWidth * param.iPicHeight, kMaxPixelsPerFrame);
+ ASSERT_LE(kLayerWidth * kLayerHeight, kMaxPixelsPerFrame);
+
+ param.sSpatialLayers[0].iVideoWidth = kLayerWidth;
+ param.sSpatialLayers[0].iVideoHeight = kLayerHeight;
+ param.sSpatialLayers[0].fFrameRate = param.fMaxFrameRate;
+ param.sSpatialLayers[0].sSliceArgument.uiSliceMode = SM_SINGLE_SLICE;
+ param.sSpatialLayers[0].iDLayerQp = 26;
+
+ int rv = encoder_->InitializeExt(¶m);
+ ASSERT_EQ(0, rv);
+
+ const int strideY = param.iPicWidth;
+ const int strideUV = param.iPicWidth >> 1;
+ std::vector<uint8_t> bufY(strideY * param.iPicHeight);
+ std::vector<uint8_t> bufU(strideUV * (param.iPicHeight >> 1));
+ std::vector<uint8_t> bufV(strideUV * (param.iPicHeight >> 1));
+ GeneratePattern(bufY.data(), strideY, bufU.data(), strideUV, bufV.data(),
+ strideUV, param.iPicWidth, param.iPicHeight);
+
+ SSourcePicture pic;
+ memset(&pic, 0, sizeof(SSourcePicture));
+ pic.iPicWidth = param.iPicWidth;
+ pic.iPicHeight = param.iPicHeight;
+ pic.iColorFormat = videoFormatI420;
+ pic.iStride[0] = strideY;
+ pic.iStride[1] = strideUV;
+ pic.iStride[2] = strideUV;
+ pic.pData[0] = bufY.data();
+ pic.pData[1] = bufU.data();
+ pic.pData[2] = bufV.data();
+
+ SFrameBSInfo info;
+ memset(&info, 0, sizeof(SFrameBSInfo));
+ rv = encoder_->EncodeFrame(&pic, &info);
+ ASSERT_EQ(0, rv);
+ ASSERT_EQ(videoFrameTypeIDR, info.eFrameType);
+
+ int len = 0;
+ for (int i = 0; i < info.iLayerNum; ++i) {
+ const SLayerBSInfo& layerInfo = info.sLayerInfo[i];
+ for (int j = 0; j < layerInfo.iNalCount; ++j) {
+ len += layerInfo.pNalLengthInByte[j];
+ }
+ }
+ ASSERT_GT(len, 0);
+
+ // The coded frame must carry the layer geometry, not the source geometry.
+ ISVCDecoder* decoder = nullptr;
+ rv = WelsCreateDecoder(&decoder);
+ ASSERT_EQ(0, rv);
+ ASSERT_TRUE(decoder != nullptr);
+
+ SDecodingParam decParam;
+ memset(&decParam, 0, sizeof(SDecodingParam));
+ decParam.uiTargetDqLayer = UCHAR_MAX;
+ decParam.eEcActiveIdc = ERROR_CON_SLICE_COPY;
+ decParam.sVideoProperty.eVideoBsType = VIDEO_BITSTREAM_DEFAULT;
+ rv = decoder->Initialize(&decParam);
+ ASSERT_EQ(0, rv);
+
+ unsigned char* pData[3] = {nullptr};
+ SBufferInfo dstBufInfo;
+ memset(&dstBufInfo, 0, sizeof(SBufferInfo));
+ rv = decoder->DecodeFrame2(info.sLayerInfo[0].pBsBuf, len, pData, &dstBufInfo);
+ ASSERT_EQ(0, rv);
+ if (dstBufInfo.iBufferStatus == 0) {
+ rv = decoder->DecodeFrame2(nullptr, 0, pData, &dstBufInfo);
+ ASSERT_EQ(0, rv);
+ }
+ ASSERT_EQ(1, dstBufInfo.iBufferStatus);
+ EXPECT_EQ(kLayerWidth, dstBufInfo.UsrData.sSystemBuffer.iWidth);
+ EXPECT_EQ(kLayerHeight, dstBufInfo.UsrData.sSystemBuffer.iHeight);
+
+ WelsDestroyDecoder(decoder);
+}
+
// SSourcePicture.bPsnrY/U/V asks for the PSNR of a single frame and the result
// is handed back in SLayerBSInfo.rPsnr. 176 is not a multiple of 32, so the
// luma plane covers both the vectorized part of WelsCalcPsnr and the columns