Skip to content

Commit e845878

Browse files
fix: stop handing deprecated YUVJ formats to swscale
An MJPEG monitor decodes to YUVJ422P into an Image that is also YUVJ422P, so Image::Assign should take the av_image_copy fast path. Only the source format was being mapped through fix_deprecated_pix_fmt(), so the identity check compared YUV422P against YUVJ422P and never matched. Every frame fell through to swscale with a deprecated destination format; swscale rewrites that format inside the context, so sws_getCachedContext() never matched its cache either and tore down and rebuilt the context per frame, logging "deprecated pixel format used, make sure you did set range correctly" at INFO level - once per frame, each one an INSERT into the Logs table. Fix both ends of every conversion: map the deprecated formats before they reach swscale and state the ranges explicitly afterwards. zm_sws_set_input_range() becomes zm_sws_set_ranges(), which also flags a full-range destination - the mjpeg encode paths in zms and event snapshots convert into YUVJ420P and were writing limited-range luma into a full-range jpeg. Tests: new cases in tests/zm_swscale_range.cpp count libav's deprecated-format warnings via an av_log callback and assert full-range output survives a conversion into YUVJ420P. The Image::Assign case fails (1 warning per Assign) without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuPNKrTGdfFT4t6G2BJbug
1 parent a70c0a4 commit e845878

8 files changed

Lines changed: 164 additions & 33 deletions

File tree

src/zm_event.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,14 +284,19 @@ int Event::OpenJpegCodec(AVFrame *frame) {
284284
frame->width, frame->height, av_get_pix_fmt_name(static_cast<AVPixelFormat>(frame->format)),
285285
mJpegCodecContext->width, mJpegCodecContext->height, av_get_pix_fmt_name(AV_PIX_FMT_YUVJ420P)
286286
);
287+
// Hand swscale the non-deprecated formats and state the ranges explicitly.
288+
// Passing YUVJ* straight in makes it log "deprecated pixel format used,
289+
// make sure you did set range correctly" for every context it builds.
290+
const AVPixelFormat orig_in_fmt = static_cast<AVPixelFormat>(frame->format);
287291
mJpegSwsContext = sws_getContext(
288-
frame->width, frame->height, static_cast<AVPixelFormat>(frame->format),
289-
mJpegCodecContext->width, mJpegCodecContext->height, AV_PIX_FMT_YUVJ420P,
292+
frame->width, frame->height, fix_deprecated_pix_fmt(orig_in_fmt),
293+
mJpegCodecContext->width, mJpegCodecContext->height, fix_deprecated_pix_fmt(AV_PIX_FMT_YUVJ420P),
290294
SWS_BICUBIC, nullptr, nullptr, nullptr);
291295
if (!mJpegSwsContext) {
292296
Error("Failure to get swscontext");
293297
return -1;
294298
}
299+
zm_sws_set_ranges(mJpegSwsContext, orig_in_fmt, AV_PIX_FMT_YUVJ420P);
295300
}
296301
#if 1
297302
output_frame = av_frame_ptr{av_frame_alloc()}; // The assignment here will destruct any previous allocation

src/zm_ffmpeg.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -698,14 +698,18 @@ bool pix_fmt_is_jpeg_range(enum AVPixelFormat fmt) {
698698
}
699699
}
700700

701-
void zm_sws_set_input_range(struct SwsContext *ctx, enum AVPixelFormat original_src_fmt) {
702-
// swscale assumes limited (MPEG) input range by default. When the decoded
703-
// source was a full-range JPEG format (YUVJ*) that got mapped to its non-J
701+
void zm_sws_set_ranges(struct SwsContext *ctx,
702+
enum AVPixelFormat original_src_fmt,
703+
enum AVPixelFormat original_dst_fmt) {
704+
// swscale assumes limited (MPEG) range on both ends by default. When either
705+
// end was a full-range JPEG format (YUVJ*) that got mapped to its non-J
704706
// equivalent by fix_deprecated_pix_fmt(), swscale would otherwise treat the
705-
// full-range samples as limited and wash the colours out. Tell it the input
706-
// is full range so the YUV->RGB / YUV->YUV maths is correct. Pass the
707-
// ORIGINAL (pre-fix) format so we can tell whether the source was full range.
708-
if (!pix_fmt_is_jpeg_range(original_src_fmt)) return;
707+
// full-range samples as limited and wash the colours out (or crush them, on
708+
// output). Tell it which ends are full range so the maths is correct. Pass
709+
// the ORIGINAL (pre-fix) formats so we can tell what the caller really has.
710+
const int want_src_range = pix_fmt_is_jpeg_range(original_src_fmt) ? 1 : 0;
711+
const int want_dst_range = pix_fmt_is_jpeg_range(original_dst_fmt) ? 1 : 0;
712+
if (!want_src_range and !want_dst_range) return;
709713

710714
int *inv_table, *table;
711715
int srcRange, dstRange, brightness, contrast, saturation;
@@ -714,9 +718,12 @@ void zm_sws_set_input_range(struct SwsContext *ctx, enum AVPixelFormat original_
714718
if (sws_getColorspaceDetails(ctx, &inv_table, &srcRange, &table, &dstRange,
715719
&brightness, &contrast, &saturation) < 0)
716720
return;
717-
if (srcRange == 1) return; // already full range
718-
srcRange = 1;
719-
sws_setColorspaceDetails(ctx, inv_table, srcRange, table, dstRange,
721+
// Only push a full-range flag on, never off: a caller that handed us a
722+
// non-J format may legitimately have set full range itself.
723+
const int new_src_range = srcRange | want_src_range;
724+
const int new_dst_range = dstRange | want_dst_range;
725+
if (new_src_range == srcRange and new_dst_range == dstRange) return;
726+
sws_setColorspaceDetails(ctx, inv_table, new_src_range, table, new_dst_range,
720727
brightness, contrast, saturation);
721728
}
722729

src/zm_ffmpeg.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,15 @@ const std::string get_codecpar_string(const AVCodecParameters *par);
260260
int check_sample_fmt(const AVCodec *codec, enum AVSampleFormat sample_fmt);
261261
enum AVPixelFormat fix_deprecated_pix_fmt(enum AVPixelFormat );
262262
bool pix_fmt_is_jpeg_range(enum AVPixelFormat );
263-
// Correct swscale's default limited-range assumption when the original decoded
264-
// source was a full-range JPEG (YUVJ*) format. Call after (re)creating the
265-
// context, passing the ORIGINAL pre-fix_deprecated_pix_fmt source format.
266-
void zm_sws_set_input_range(struct SwsContext *ctx, enum AVPixelFormat original_src_fmt);
263+
// Correct swscale's default limited-range assumption when either end of the
264+
// conversion was a full-range JPEG (YUVJ*) format. Call after (re)creating the
265+
// context, passing the ORIGINAL pre-fix_deprecated_pix_fmt formats. Both ends
266+
// matter: the deprecated formats must be fixed up before they reach
267+
// sws_getCachedContext(), because swscale rewrites them inside the context and
268+
// the cache lookup then never matches, rebuilding the context on every frame.
269+
void zm_sws_set_ranges(struct SwsContext *ctx,
270+
enum AVPixelFormat original_src_fmt,
271+
enum AVPixelFormat original_dst_fmt);
267272

268273
// Return a plausible framerate for `stream`. Prefers r_frame_rate, falls
269274
// back to avg_frame_rate. ffmpeg falls back to 1/time_base when it can't

src/zm_image.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -399,18 +399,25 @@ bool Image::Assign(const AVFrame *frame) {
399399
// AVPixFormat() getter re-derives via (colours, subpixelorder) and would
400400
// pick the wrong swscale target if those legacy fields drift out of sync
401401
// (e.g. the GRAY8/YUV420P alias collision).
402-
const AVPixelFormat format = imagePixFormat;
402+
const AVPixelFormat orig_dst_fmt = imagePixFormat;
403403
// Map deprecated YUVJ* formats to their non-J equivalents before handing the
404-
// format to swscale. Passing YUVJ420P/YUVJ422P/etc directly makes swscale emit
405-
// "deprecated pixel format used, make sure you did set range correctly" (seen
406-
// in nph-zms). This mirrors what SWScale::Convert already does.
404+
// formats to swscale. Passing YUVJ420P/YUVJ422P/etc directly makes swscale
405+
// emit "deprecated pixel format used, make sure you did set range correctly",
406+
// and because swscale rewrites the format inside the context the cache lookup
407+
// in sws_getCachedContext() never matches again: the context is torn down and
408+
// rebuilt for every single frame. BOTH ends have to be fixed up - fixing only
409+
// the source left every MJPEG monitor rebuilding its context per frame.
407410
const AVPixelFormat orig_src_fmt = static_cast<AVPixelFormat>(frame->format);
408411
const AVPixelFormat src_fmt = fix_deprecated_pix_fmt(orig_src_fmt);
412+
const AVPixelFormat format = fix_deprecated_pix_fmt(orig_dst_fmt);
409413

410414
// If source and destination format + dimensions match, do a direct plane
411415
// copy instead of running through sws_scale. This avoids the overhead of
412416
// the swscale pipeline for identity conversions (e.g. YUVJ422P→YUVJ422P).
417+
// The ranges have to match too: YUVJ422P->YUV422P shares a plane layout but
418+
// needs the full->limited range conversion that only swscale does.
413419
if (src_fmt == format
420+
&& pix_fmt_is_jpeg_range(orig_src_fmt) == pix_fmt_is_jpeg_range(orig_dst_fmt)
414421
&& frame->width == static_cast<int>(width)
415422
&& frame->height == static_cast<int>(height)) {
416423
Debug(4, "Same format %s %dx%d, using av_image_copy",
@@ -441,7 +448,7 @@ bool Image::Assign(const AVFrame *frame) {
441448
Error("Unable to create conversion context");
442449
return false;
443450
}
444-
zm_sws_set_input_range(sws_convert_context, orig_src_fmt);
451+
zm_sws_set_ranges(sws_convert_context, orig_src_fmt, orig_dst_fmt);
445452
bool result = Assign(frame, sws_convert_context);
446453
update_function_pointers();
447454
return result;

src/zm_monitor.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3617,7 +3617,8 @@ int Monitor::Capture() {
36173617
} // end Monitor::Capture
36183618

36193619
bool Monitor::setupConvertContext(const AVFrame *input_frame, const Image *image) {
3620-
AVPixelFormat imagePixFormat = image->AVPixFormat();
3620+
AVPixelFormat origImagePixFormat = image->AVPixFormat();
3621+
AVPixelFormat imagePixFormat = fix_deprecated_pix_fmt(origImagePixFormat);
36213622
AVPixelFormat origPixFormat = (AVPixelFormat)input_frame->format;
36223623
AVPixelFormat inputPixFormat = fix_deprecated_pix_fmt(origPixFormat);
36233624

@@ -3635,9 +3636,9 @@ bool Monitor::setupConvertContext(const AVFrame *input_frame, const Image *image
36353636
image->Width(), image->Height(),
36363637
av_get_pix_fmt_name(imagePixFormat)
36373638
);
3638-
// Mark the input as full range when the source was a YUVJ* format so the
3639+
// Mark either end as full range when it was a YUVJ* format so the
36393640
// conversion maths doesn't crush full-range luma into limited range.
3640-
zm_sws_set_input_range(convert_context, origPixFormat);
3641+
zm_sws_set_ranges(convert_context, origPixFormat, origImagePixFormat);
36413642
}
36423643
return (convert_context != nullptr);
36433644
} //end bool Monitor::setupConvertContext(const AVFrame *input_frame, const Image *image)

src/zm_stream.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,18 +126,23 @@ bool StreamBase::initContexts(int in_width, int in_height, AVPixelFormat format,
126126
sws_freeContext(mJpegSwsContext);
127127
}
128128

129+
// Hand swscale the non-deprecated formats and state the ranges explicitly.
130+
// Passing YUVJ* straight in makes it log "deprecated pixel format used, make
131+
// sure you did set range correctly" for every context it builds - once per
132+
// zms request.
129133
mJpegSwsContext = sws_getContext(
130-
//monitor->Width(), monitor->Height(),
134+
//monitor->Width(), monitor->Height(),
131135
// theoretically, the stream can be any size not necessarily monitor size. I think. This is here more for format conversion than scaling.
132136
// No we are doing scaling here too. It got removed from prepareImage
133-
in_width, in_height, format,
134-
out_width, out_height, mJpegCodecContext->pix_fmt,
137+
in_width, in_height, fix_deprecated_pix_fmt(format),
138+
out_width, out_height, fix_deprecated_pix_fmt(mJpegCodecContext->pix_fmt),
135139
SWS_BICUBIC, nullptr, nullptr, nullptr);
136140

137141
if (!mJpegSwsContext) {
138142
Warning("Failed to alloc swscontext");
139143
return false;
140144
} else {
145+
zm_sws_set_ranges(mJpegSwsContext, format, mJpegCodecContext->pix_fmt);
141146
Debug(1, "Configured swsContext to %dx%d %d %s to %dx%d %d %s",
142147
in_width, in_height, format, av_get_pix_fmt_name(format),
143148
out_width, out_height, mJpegCodecContext->pix_fmt, av_get_pix_fmt_name(mJpegCodecContext->pix_fmt));

src/zm_swscale.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,20 @@ int SWScale::Convert(
5858
AVFrame *out_frame
5959
) {
6060

61-
AVPixelFormat orig_format = (AVPixelFormat)in_frame->format;
62-
AVPixelFormat format = fix_deprecated_pix_fmt(orig_format);
61+
AVPixelFormat orig_in_format = (AVPixelFormat)in_frame->format;
62+
AVPixelFormat orig_out_format = (AVPixelFormat)out_frame->format;
63+
AVPixelFormat in_format = fix_deprecated_pix_fmt(orig_in_format);
64+
AVPixelFormat out_format = fix_deprecated_pix_fmt(orig_out_format);
6365
/* Get the context */
6466
swscale_ctx = sws_getCachedContext(swscale_ctx,
65-
in_frame->width, in_frame->height, format,
66-
out_frame->width, out_frame->height, (AVPixelFormat)out_frame->format,
67+
in_frame->width, in_frame->height, in_format,
68+
out_frame->width, out_frame->height, out_format,
6769
SWS_FAST_BILINEAR, NULL, NULL, NULL);
6870
if ( swscale_ctx == NULL ) {
6971
Error("Failed getting swscale context");
7072
return -6;
7173
}
72-
zm_sws_set_input_range(swscale_ctx, orig_format);
74+
zm_sws_set_ranges(swscale_ctx, orig_in_format, orig_out_format);
7375
/* Do the conversion */
7476
if (!sws_scale(swscale_ctx,
7577
in_frame->data, in_frame->linesize, 0, in_frame->height,
@@ -117,7 +119,9 @@ int SWScale::Convert(
117119
}
118120

119121
const enum _AVPIXELFORMAT orig_in_pf = in_pf;
122+
const enum _AVPIXELFORMAT orig_out_pf = out_pf;
120123
in_pf = fix_deprecated_pix_fmt(in_pf);
124+
out_pf = fix_deprecated_pix_fmt(out_pf);
121125

122126
/* Warn if the input or output pixelformat is not supported */
123127
if (!sws_isSupportedInput(in_pf)) {
@@ -158,7 +162,7 @@ int SWScale::Convert(
158162
Error("Failed getting swscale context");
159163
return -6;
160164
}
161-
zm_sws_set_input_range(swscale_ctx, orig_in_pf);
165+
zm_sws_set_ranges(swscale_ctx, orig_in_pf, orig_out_pf);
162166

163167
/* Fill in the buffers. The alignments describe how the caller's buffers
164168
* are actually laid out — they are facts about the buffers, not tuning

tests/zm_swscale_range.cpp

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,40 @@
1818
#include "zm_catch2.h"
1919

2020
#include "zm_ffmpeg.h"
21+
#include "zm_image.h"
2122
#include "zm_swscale.h"
2223

2324
#include <cstdlib>
25+
#include <cstring>
2426
#include <vector>
2527

28+
namespace {
29+
30+
// libav emits "deprecated pixel format used, make sure you did set range
31+
// correctly" whenever a context is built with a YUVJ* format. Because swscale
32+
// rewrites that format inside the context, sws_getCachedContext() then never
33+
// matches its cache and rebuilds the context on every single frame - the
34+
// warning appearing once per frame in zmc_*.log is how that shows up. Count
35+
// the warnings to assert we never hand a deprecated format to swscale.
36+
int deprecated_format_warnings = 0;
37+
38+
void counting_log_callback(void *, int, const char *fmt, va_list) {
39+
if (fmt and strstr(fmt, "deprecated pixel format")) deprecated_format_warnings++;
40+
}
41+
42+
// Installs the counting callback for the lifetime of the test case.
43+
class LogCounter {
44+
public:
45+
LogCounter() {
46+
deprecated_format_warnings = 0;
47+
av_log_set_callback(counting_log_callback);
48+
}
49+
~LogCounter() { av_log_set_callback(av_log_default_callback); }
50+
int count() const { return deprecated_format_warnings; }
51+
};
52+
53+
} // namespace
54+
2655
TEST_CASE("pix_fmt_is_jpeg_range identifies full-range YUVJ formats", "[swscale]") {
2756
REQUIRE(pix_fmt_is_jpeg_range(AV_PIX_FMT_YUVJ420P));
2857
REQUIRE(pix_fmt_is_jpeg_range(AV_PIX_FMT_YUVJ422P));
@@ -62,3 +91,71 @@ TEST_CASE("SWScale treats YUVJ420P input as full range when converting to RGB",
6291
REQUIRE(std::abs(static_cast<int>(out[0]) - static_cast<int>(out[1])) <= 2);
6392
REQUIRE(std::abs(static_cast<int>(out[1]) - static_cast<int>(out[2])) <= 2);
6493
}
94+
95+
// The mjpeg encode paths (zms, event snapshots) convert INTO YUVJ420P. The
96+
// output end needs the same treatment as the input end: with swscale left on
97+
// its default limited output range, mid grey RGB 128 lands at Y=126 instead of
98+
// Y=128 and the encoded jpeg comes out with a compressed contrast range.
99+
TEST_CASE("SWScale treats YUVJ420P output as full range when converting from RGB", "[swscale]") {
100+
const int w = 16, h = 16;
101+
const uint8_t grey = 128;
102+
103+
std::vector<uint8_t> in(SWScale::GetBufferSize(AV_PIX_FMT_RGB24, w, h, 1), grey);
104+
std::vector<uint8_t> out(SWScale::GetBufferSize(AV_PIX_FMT_YUVJ420P, w, h, 1), 0);
105+
106+
SWScale scaler;
107+
REQUIRE(scaler.init());
108+
int r = scaler.Convert(in.data(), in.size(), out.data(), out.size(),
109+
AV_PIX_FMT_RGB24, AV_PIX_FMT_YUVJ420P, w, h, 1, 1);
110+
REQUIRE(r == 0);
111+
112+
// Full range: Y == the RGB level. Limited range would scale it to ~126.
113+
REQUIRE(out[0] >= 127);
114+
REQUIRE(out[0] <= 129);
115+
}
116+
117+
TEST_CASE("SWScale does not hand deprecated pixel formats to swscale", "[swscale]") {
118+
const int w = 16, h = 16;
119+
LogCounter counter;
120+
121+
std::vector<uint8_t> yuvj(SWScale::GetBufferSize(AV_PIX_FMT_YUVJ420P, w, h, 1), 128);
122+
std::vector<uint8_t> rgb(SWScale::GetBufferSize(AV_PIX_FMT_RGB24, w, h, 1), 0);
123+
124+
SWScale scaler;
125+
REQUIRE(scaler.init());
126+
REQUIRE(scaler.Convert(yuvj.data(), yuvj.size(), rgb.data(), rgb.size(),
127+
AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_RGB24, w, h, 1, 1) == 0);
128+
REQUIRE(scaler.Convert(rgb.data(), rgb.size(), yuvj.data(), yuvj.size(),
129+
AV_PIX_FMT_RGB24, AV_PIX_FMT_YUVJ420P, w, h, 1, 1) == 0);
130+
131+
REQUIRE(counter.count() == 0);
132+
}
133+
134+
// Regression: an MJPEG monitor decodes to YUVJ422P and its Image is YUVJ422P
135+
// too, so Image::Assign should take the av_image_copy fast path. Fixing only
136+
// the source format left the identity check comparing YUV422P against
137+
// YUVJ422P: every frame fell through to swscale, which rebuilt its context and
138+
// logged the deprecated-format warning 15+ times a second per monitor.
139+
TEST_CASE("Image::Assign of a matching YUVJ frame copies without swscale", "[swscale]") {
140+
const int w = 32, h = 16;
141+
config.font_file_location = "data/fonts/04_valid.zmfnt";
142+
143+
av_frame_ptr frame{av_frame_alloc()};
144+
REQUIRE(frame);
145+
frame->width = w;
146+
frame->height = h;
147+
frame->format = AV_PIX_FMT_YUVJ422P;
148+
REQUIRE(av_frame_get_buffer(frame.get(), 32) == 0);
149+
// Full-range white: a limited-range conversion would pull this down to 235.
150+
memset(frame->data[0], 255, frame->linesize[0] * h);
151+
memset(frame->data[1], 128, frame->linesize[1] * h);
152+
memset(frame->data[2], 128, frame->linesize[2] * h);
153+
154+
Image image(frame.get());
155+
REQUIRE(image.PixFormat() == AV_PIX_FMT_YUVJ422P);
156+
157+
LogCounter counter;
158+
REQUIRE(image.Assign(frame.get()));
159+
REQUIRE(image.Buffer()[0] == 255);
160+
REQUIRE(counter.count() == 0);
161+
}

0 commit comments

Comments
 (0)