diff --git a/android/app/src/main/cpp/openstream_srt.cpp b/android/app/src/main/cpp/openstream_srt.cpp index cdfa6085..1ff101a5 100644 --- a/android/app/src/main/cpp/openstream_srt.cpp +++ b/android/app/src/main/cpp/openstream_srt.cpp @@ -113,14 +113,13 @@ bool hasStartCode(const std::vector &data, size_t offset) { data[offset + 2] == 0x01; } -std::vector normalizeAnnexB(const uint8_t *bytes, size_t size) { - std::vector input(bytes, bytes + size); +std::vector normalizeAnnexB(std::vector input) { if (input.empty() || hasStartCode(input, 0)) { return input; } - std::vector output; - output.reserve(input.size() + 16); + // Validate the complete access unit before changing any length prefixes. + // Invalid input must retain its original bytes for the existing fallback. size_t offset = 0; while (offset + 4 <= input.size()) { const uint32_t nalSize = (static_cast(input[offset]) << 24u) | @@ -128,16 +127,29 @@ std::vector normalizeAnnexB(const uint8_t *bytes, size_t size) { (static_cast(input[offset + 2]) << 8u) | static_cast(input[offset + 3]); offset += 4; - if (nalSize == 0 || offset + nalSize > input.size()) { + if (nalSize == 0 || nalSize > input.size() - offset) { return input; } - output.insert(output.end(), {0x00, 0x00, 0x00, 0x01}); - output.insert(output.end(), input.begin() + static_cast(offset), - input.begin() + static_cast(offset + nalSize)); offset += nalSize; } + if (offset != input.size()) { + return input; + } - return offset == input.size() ? output : input; + // Four-byte NAL lengths and Annex B start codes occupy the same space. + offset = 0; + while (offset < input.size()) { + const uint32_t nalSize = (static_cast(input[offset]) << 24u) | + (static_cast(input[offset + 1]) << 16u) | + (static_cast(input[offset + 2]) << 8u) | + static_cast(input[offset + 3]); + input[offset] = 0; + input[offset + 1] = 0; + input[offset + 2] = 0; + input[offset + 3] = 1; + offset += 4 + nalSize; + } + return input; } int aacSampleRateIndex(int sampleRate) { @@ -1133,7 +1145,7 @@ Java_dev_openstream_app_stream_SrtNativeBridge_nativeSendVideo( return JNI_FALSE; } - std::vector annexB = normalizeAnnexB(bytes.data(), bytes.size()); + std::vector annexB = normalizeAnnexB(std::move(bytes)); if ((flags & kMediaCodecBufferFlagCodecConfig) != 0) { g_state.codecConfig = std::move(annexB); logInfo("Stored codec config for keyframe pre-roll"); @@ -1142,7 +1154,9 @@ Java_dev_openstream_app_stream_SrtNativeBridge_nativeSendVideo( const bool keyFrame = (flags & kMediaCodecBufferFlagKeyFrame) != 0; if (keyFrame && !g_state.codecConfig.empty()) { - std::vector withConfig = g_state.codecConfig; + std::vector withConfig; + withConfig.reserve(g_state.codecConfig.size() + annexB.size()); + withConfig.insert(withConfig.end(), g_state.codecConfig.begin(), g_state.codecConfig.end()); withConfig.insert(withConfig.end(), annexB.begin(), annexB.end()); annexB = std::move(withConfig); } diff --git a/obs-plugin/src/openstream-source.cpp b/obs-plugin/src/openstream-source.cpp index 9e6cea49..2b00a2e9 100644 --- a/obs-plugin/src/openstream-source.cpp +++ b/obs-plugin/src/openstream-source.cpp @@ -1549,7 +1549,9 @@ bool output_decoded_frame(OpenStreamSource *ctx, return true; } - SwsContext *current_sws = sws_ctx->get(); + // sws_getCachedContext frees an incompatible old context, including on failure. + // Transfer ownership before calling it so the smart pointer cannot free it again. + SwsContext *current_sws = sws_ctx->release(); SwsContext *scaled = sws_getCachedContext( current_sws, width, @@ -1562,13 +1564,11 @@ bool output_decoded_frame(OpenStreamSource *ctx, nullptr, nullptr, nullptr); + sws_ctx->reset(scaled); if (!scaled) { blog(LOG_WARNING, "[OpenStream] Could not create BGRA converter"); return false; } - if (scaled != current_sws) { - sws_ctx->reset(scaled); - } const int linesize = width * 4; const size_t buffer_size = static_cast(linesize) * height; diff --git a/obs-plugin/tests/test_source_settings.cpp b/obs-plugin/tests/test_source_settings.cpp index 57d65664..84315403 100644 --- a/obs-plugin/tests/test_source_settings.cpp +++ b/obs-plugin/tests/test_source_settings.cpp @@ -19,6 +19,14 @@ using openstream_test::require; void openstream_register_dock() {} void openstream_unregister_dock() {} +// Capture the OBS output boundary without starting its graphics runtime. +static uint32_t captured_video_width = 0; +void obs_source_output_video(obs_source_t *, const struct obs_source_frame *frame) { + require(frame != nullptr && frame->format == VIDEO_FORMAT_BGRA, + "converter did not deliver a BGRA frame to OBS"); + captured_video_width = frame->width; +} + struct ReleaseProbe { int listener = -1; int port = 0; @@ -134,7 +142,36 @@ void test_worker_releases_reservation_on_exit() { "worker exit kept a stale active reservation after release"); } +void test_converter_survives_format_changes() { + OpenStreamSource context; + CodecContextPtr decoder(avcodec_alloc_context3(nullptr)); + SwsContextPtr converter(nullptr); + std::vector pixels; + for (const auto &[width, format] : std::vector>{ + {64, AV_PIX_FMT_RGB24}, {64, AV_PIX_FMT_RGB24}, + {128, AV_PIX_FMT_BGR24}, {32, AV_PIX_FMT_RGB24}, {64, AV_PIX_FMT_BGR24}}) { + FramePtr frame(av_frame_alloc()); + frame->width = width; + frame->height = 32; + frame->format = format; + require(av_frame_get_buffer(frame.get(), 32) == 0, "could not allocate test frame"); + for (int row = 0; row < frame->height; ++row) { + std::memset(frame->data[0] + row * frame->linesize[0], 0, width * 3); + } + require(output_decoded_frame(&context, decoder.get(), frame.get(), 0, + &converter, &pixels), + "frame conversion failed after a size change"); + require(captured_video_width == static_cast(width), + "OBS received a frame with stale dimensions"); + require(pixels.size() == static_cast(width * 32 * 4), + "converted frame retained the old dimensions"); + require(pixels[0] == 0 && pixels[1] == 0 && pixels[2] == 0 && pixels[3] == 255, + "converted black frame has incorrect BGRA pixels"); + } +} + int main() { + test_converter_survives_format_changes(); test_worker_releases_reservation_on_exit(); require(std::string(openstream_source_info.id) == "shin_phone_source", diff --git a/tests/test_native_video_normalization.py b/tests/test_native_video_normalization.py new file mode 100644 index 00000000..bfc379c4 --- /dev/null +++ b/tests/test_native_video_normalization.py @@ -0,0 +1,86 @@ +"""Run the production C++ normalizer and muxer without an Android runtime.""" +import shutil +import subprocess + +import pytest + +from _helpers import read_text + + +def test_native_video_normalization_preserves_payload_and_storage(tmp_path): + compiler = shutil.which("g++") or shutil.which("clang++") + if compiler is None: + pytest.skip("native video test requires a C++ compiler") + source = read_text("android/app/src/main/cpp/openstream_srt.cpp") + media = source[source.index("enum class VideoCodec") : source.index("struct SrtUrl")] + harness = r""" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +constexpr int kAudioSampleRate = 48000; +constexpr int kAudioChannelCount = 1; +""" + media + r""" +void check(std::vector input, const std::vector& expected) { + const auto* storage = input.data(); + auto result = normalizeAnnexB(std::move(input)); + assert(result == expected); + assert(result.empty() || result.data() == storage); + MpegTsMuxer actual(VideoCodec::Avc), reference(VideoCodec::Avc); + for (int frame = 0; frame < 35; ++frame) { + assert(actual.muxAccessUnit(result, frame * 33333, frame == 0) == + reference.muxAccessUnit(expected, frame * 33333, frame == 0)); + } +} +int main() { + check({}, {}); + check({0,0,1,0x65,9}, {0,0,1,0x65,9}); + check({0,0,0,1,0x65,9}, {0,0,0,1,0x65,9}); + check({0,0,0,2,0x65,9,0,0,0,2,0x41,8}, + {0,0,0,1,0x65,9,0,0,0,1,0x41,8}); + // A malformed second NAL must not leave the first NAL partly converted. + for (auto invalid : std::vector>{ + {0,0}, {0,0,0,0}, {0xff,0xff,0xff,0xff,9}, + {0,0,0,2,0x65,9,0,0,0,4,8}, {0,0,0,2,0x65,9,0}}) { + check(invalid, invalid); + } + // Preserve the existing start-code precedence for ambiguous length prefixes. + std::vector ambiguous(260, 0x65); + ambiguous[0] = 0; ambiguous[1] = 0; ambiguous[2] = 1; ambiguous[3] = 0; + check(ambiguous, ambiguous); + std::mt19937 random(42); + for (int sample = 0; sample < 200; ++sample) { + std::vector input, expected; + for (unsigned nal = 0, count = 1 + random() % 20; nal < count; ++nal) { + const uint32_t size = 2 + random() % 250; + input.insert(input.end(), {0, 0, static_cast(size >> 8), + static_cast(size)}); + expected.insert(expected.end(), {0, 0, 0, 1}); + for (uint32_t byte = 0; byte < size; ++byte) { + const uint8_t value = static_cast(random()); + input.push_back(value); + expected.push_back(value); + } + } + check(input, expected); + input.push_back(0); // Incomplete final length: preserve the entire access unit. + check(input, input); + } + std::vector large(1024 * 1024 + 4, 0x65); + large[0] = 0; large[1] = 0x10; large[2] = 0; large[3] = 0; + auto expected = large; + expected[1] = 0; expected[3] = 1; + check(std::move(large), expected); +} +""" + cpp = tmp_path / "video_test.cpp" + executable = tmp_path / "video_test" + cpp.write_text(harness) + subprocess.run([compiler, "-std=c++20", "-O2", str(cpp), "-o", str(executable)], check=True) + subprocess.run([str(executable)], check=True, timeout=20)