Opened 22 months ago

Last modified 22 months ago

#10136 new defect

Can't convert color range with sws_scale

Reported by: diogo.r Owned by:
Priority: normal Component: swscale
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
I'm trying to convert a frame with sws_scale while selecting the color range of the input and output, but for some input formats it doesn't work, it ignores the color ranges I set. I looked into it and found weird this check in 'libswscale/utils.c:1046':

    //The srcBpc check is possibly wrong but we seem to lack a definitive reference to test this
    //and what we have in ticket 2939 looks better with this check
    if (need_reinit && (c->srcBpc == 8 || !isYUV(c->srcFormat)))
        ff_sws_init_range_convert(c);

I looked at ticket 2939, but didn't find reasoning about why the (c->srcBpc == 8 || !isYUV(c->srcFormat) check is necessary.

How to reproduce:


extern "C"
{
	#include <libswscale/swscale.h>
}
#include "assert.h"

int main() {
	AVPixelFormat in_pix_fmt = AVPixelFormat::AV_PIX_FMT_YUV420P10LE;
	AVPixelFormat out_pix_fmt = AVPixelFormat::AV_PIX_FMT_YUV420P;
	int width = 32;
	int height = 32;

	SwsContext *sws_ctx = (struct SwsContext*)sws_getContext(
		width,
		height,
		in_pix_fmt,
		width,
		height,
		out_pix_fmt,
		SWS_BILINEAR,
		NULL,
		NULL,
		NULL
	);
	
	int *table = NULL;
	int *inv_table = NULL;
	int srcRange = -1;
	int dstRange = -1;
	int brightness = -1;
	int contrast = -1;
	int saturation = -1;

	int res = sws_getColorspaceDetails(
		sws_ctx,
		&inv_table,
		&srcRange,
		&table,
		&dstRange,
		&brightness,
		&contrast,
		&saturation
	);
	assert(res == 0);

	printf(
		R"(
	srcRange 	= %d
	dstRange 	= %d
	brightness 	= %d
	contrast 	= %d
	saturation 	= %d
	table		= [%d, %d, %d, %d]
	inv_table	= [%d, %d, %d, %d]
)",
		srcRange,
		dstRange,
		brightness,
		contrast,
		saturation,
		table[0],
		table[1],
		table[2],
		table[3],
		inv_table[0],
		inv_table[1],
		inv_table[2],
		inv_table[3]
	);

	srcRange = 1;
	
	res = sws_setColorspaceDetails(
		sws_ctx,
		inv_table,
		srcRange,
		table,
		dstRange,
		brightness,
		contrast,
		saturation
	);

	// Do the conversion
	// sws_scale(...)

	assert(res == 0);
	return 0;
}

Change History (5)

comment:1 by Balling, 22 months ago

That was added in b53bdae11f1eceea1a2e25a98aee81e1d1954e14

It says it is needed for rgb to full range YCbCr convertion.

I was testing #3785.

Without 4959a4fcf76e7c595dbb23c4e3bf59abf2e60ea4 it will not even work at all.

Last edited 22 months ago by Balling (previous) (diff)

comment:2 by diogo.r, 22 months ago

According with this comment (http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=e645a1ddb90a863e129108aad9aa7e2d417f3615) the problem is that when setting color range options after initializing sws_scale, some fast paths might already be chosen and we can't change it anymore.

I'm wondering if it works when I set everything at the beginning, like so:

	SwsContext *sws_ctx = (struct SwsContext*)sws_alloc_context();
	av_opt_set_int(sws_ctx, "srcw", width, 0);
	av_opt_set_int(sws_ctx, "srch", height, 0);
	av_opt_set_int(sws_ctx, "src_format", in_pix_fmt, 0);
	av_opt_set_int(sws_ctx, "dstw", width, 0);
	av_opt_set_int(sws_ctx, "dsth", height, 0);
	av_opt_set_int(sws_ctx, "dst_format", out_pix_fmt, 0);
	av_opt_set_int(sws_ctx, "sws_flags", SWS_BILINEAR, 0);
	av_opt_set_int(sws_ctx, "param0", NULL, 0);
	av_opt_set_int(sws_ctx, "param1", NULL, 0);
	av_opt_set_int(sws_ctx, "src_range", 1, 0);
	// av_opt_set_int(sws_ctx, "dst_range", 1, 0);
	int res = sws_init_context(sws_ctx, NULL, NULL);

It seems to work, is that the proper way to do it? If that's the case, it might be helpful to return an error for my first attempt.

comment:3 by Balling, 22 months ago

#9576 it 100% a valid issue. I verified it. Why there are people confused about it (#9898)?

comment:4 by diogo.r, 22 months ago

I read through those issues, but I still don't understand how what you said @Balling relates to this issue, could you please clarify? Also keep in mind I'm not an expert in ffmpeg nor colorspace stuff.

comment:5 by Balling, 22 months ago

The problem with returning an error is to point out where in the code that error should be.

Note: See TracTickets for help on using tickets.