| Message ID | 20231031145450.94975-1-ffmpeg@haasn.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3,1/8] swscale: fix sws_setColorspaceDetails after sws_init_context | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Will apply soon.
> > Will apply soon. > Hi Niklas: This patchset causes a regression. The command: "ffmpeg -i input.png -vf format=grayf32,format=gray8 output.png" reports error. If I configure with "--disable-sse2", the error is unseen. Thanks Wenbin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, 13 Nov 2023 04:03:08 +0000 "Chen, Wenbin" <wenbin.chen-at-intel.com@ffmpeg.org> wrote: > > > > Will apply soon. > > > Hi Niklas: > > This patchset causes a regression. > The command: "ffmpeg -i input.png -vf format=grayf32,format=gray8 output.png" reports error. > If I configure with "--disable-sse2", the error is unseen. > > Thanks > Wenbin Thanks for pointing it out. Submitted a series to fix this case (and a related bug I found while debugging this one).
On Tue, Nov 07, 2023 at 02:08:35PM +0100, Niklas Haas wrote:
> Will apply soon.
It seems this causes an assertion failue:
libswscale/tests/swscale
grayf32be -> grayf32be
grayf32be 96x96 -> grayf32be 64x 64 flags= 1 CRC=d85005d7 SSD= 0, 958, 667, 0
grayf32be 96x96 -> grayf32be 64x 96 flags= 1 CRC=9175737f SSD= 0, 958, 667, 0
grayf32be 96x96 -> grayf32be 64x128 flags= 1 CRC=313242fb SSD= 0, 958, 667, 0
grayf32be 96x96 -> grayf32be 96x 64 flags= 1 CRC=d4c1d3d3 SSD= 0, 958, 667, 0
Assertion c->srcBpc == 16 failed at libswscale/x86/swscale.c:533
Aborted (core dumped)
[...]
diff --git a/libswscale/swscale.c b/libswscale/swscale.c index 90e5b299ab..46ba68fe6a 100644 --- a/libswscale/swscale.c +++ b/libswscale/swscale.c @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c, reset_ptr(src2, c->srcFormat); reset_ptr((void*)dst2, c->dstFormat); - if (c->convert_unscaled) { + if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) { int offset = srcSliceY_internal; int slice_h = srcSliceH; diff --git a/libswscale/utils.c b/libswscale/utils.c index e1ad685972..0a55657800 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -1715,30 +1715,26 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter, if (unscaled && !usesHFilter && !usesVFilter && c->alphablend != SWS_ALPHA_BLEND_NONE && isALPHA(srcFormat) && - (c->srcRange == c->dstRange || isAnyRGB(dstFormat)) && alphaless_fmt(srcFormat) == dstFormat ) { c->convert_unscaled = ff_sws_alphablendaway; if (flags & SWS_PRINT_INFO) av_log(c, AV_LOG_INFO, - "using alpha blendaway %s -> %s special converter\n", + "alpha blendaway %s -> %s special converter is available\n", av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat)); return 0; } /* unscaled special cases */ - if (unscaled && !usesHFilter && !usesVFilter && - (c->srcRange == c->dstRange || isAnyRGB(dstFormat) || - isFloat(srcFormat) || isFloat(dstFormat))){ + if (unscaled && !usesHFilter && !usesVFilter) { ff_get_unscaled_swscale(c); if (c->convert_unscaled) { if (flags & SWS_PRINT_INFO) av_log(c, AV_LOG_INFO, - "using unscaled %s -> %s special converter\n", + "unscaled %s -> %s special converter is available\n", av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat)); - return 0; } }
From: Niklas Haas <git@haasn.dev> More commonly, this fixes the case of sws_setColorspaceDetails after sws_getContext, since the latter implies sws_init_context. The problem here is that sws_init_context sets up the range conversion and fast path tables based on the values of srcRange/dstRange at init time. This may result in locking in a "wrong" path (either using unscaled fast path when range conversion later required, or using scaled slow path when range conversion becomes no longer required). There are two way outs: 1. Always initialize range conversion and unscaled converters, even if they will be unused, and extend the runtime check. 2. Re-do initialization if the values change after sws_setColorspaceDetails. I opted for approach 1 because it was simpler and easier to reason about. Reword the av_log message to make it clear that this special converter is not necessarily used, depending on whether or not there is range conversion or YUV matrix conversion going on. --- libswscale/swscale.c | 2 +- libswscale/utils.c | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-)