* [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
@ 2023-11-13 15:32 Niklas Haas
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context Niklas Haas
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Niklas Haas @ 2023-11-13 15:32 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
This logic was incongruent with logic used elsewhere, where floating
point formats are explicitly exempted from range conversion. Fixes an
issue where floating point formats were not going through special
unscaled converters even when it was otherwise possible.
---
libswscale/swscale.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 46ba68fe6a..a66db22767 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -534,7 +534,8 @@ av_cold void ff_sws_init_range_convert(SwsContext *c)
{
c->lumConvertRange = NULL;
c->chrConvertRange = NULL;
- if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
+ if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
+ !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
if (c->dstBpc <= 14) {
if (c->srcRange) {
c->lumConvertRange = lumRangeFromJpeg_c;
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-13 15:32 [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Niklas Haas
@ 2023-11-13 15:32 ` Niklas Haas
2023-11-13 18:30 ` Michael Niedermayer
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 3/3] swscale/utils: don't early return in yuv alpha blendaway Niklas Haas
2023-11-14 2:27 ` [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Chen, Wenbin
2 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2023-11-13 15:32 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
Before cedf589, this function would return early return on RGB and float
formats, as well as when range was equal. While this commit
intentionally removed the early return for same-range YUV conversions,
it missed that RGB and float formats that have an unscaled converter
should always early return, no matter what the source range was set to.
Fixes: cedf589c09c567b72bf4c1a58db53d94622567e1
---
libswscale/utils.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libswscale/utils.c b/libswscale/utils.c
index ec822ff5d9..7ce86f83ea 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1733,6 +1733,9 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
av_log(c, AV_LOG_INFO,
"unscaled %s -> %s special converter is available\n",
av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
+
+ if (isAnyRGB(dstFormat) || isFloat(srcFormat) || isFloat(dstFormat))
+ return 0;
}
}
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context Niklas Haas
@ 2023-11-13 18:30 ` Michael Niedermayer
2023-11-14 13:14 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Michael Niedermayer @ 2023-11-13 18:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]
On Mon, Nov 13, 2023 at 04:32:33PM +0100, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
>
> Before cedf589, this function would return early return on RGB and float
> formats, as well as when range was equal. While this commit
> intentionally removed the early return for same-range YUV conversions,
> it missed that RGB and float formats that have an unscaled converter
> should always early return, no matter what the source range was set to.
>
> Fixes: cedf589c09c567b72bf4c1a58db53d94622567e1
> ---
> libswscale/utils.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index ec822ff5d9..7ce86f83ea 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1733,6 +1733,9 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> av_log(c, AV_LOG_INFO,
> "unscaled %s -> %s special converter is available\n",
> av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> +
> + if (isAnyRGB(dstFormat) || isFloat(srcFormat) || isFloat(dstFormat))
> + return 0;
this and the other patch result in difficult to understand code.
if i look back to 6.0 the 2 cases had unconditional "return 0"
the code before basically
if(set of conditions for "alphablend")
convert_unscaled = alphablend
return 0
if(set of conditions for "special converter")
convert_unscaled = set special converter
return 0
now they conditionally return
if(set of conditions)
convert_unscaled = alphablend
if set of more conditions
return 0
if(set of conditions)
convert_unscaled = set special converter
if set of more conditions
return 0
I think this could add some burden to whoever eventually has to
clean this up
i may be missing something but i wonder if not either
* convert_unscaled should only be set when used
OR
* if these are set "always" if not alphablend and convert_unscaled should be
two seperate fields. But i have not at all looked at what consequences that
would have so maybe that has issues
Also if some range convert should not be used/set for some cases then
the check should maybe be where the range convert is setup not far away
from it. I mean a check close to the related code is easier to understand
but i dont feel like i fully understand the issue here so maybe iam missing
the goal of this patchset somewhat
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-13 18:30 ` Michael Niedermayer
@ 2023-11-14 13:14 ` Niklas Haas
2023-11-14 22:52 ` Michael Niedermayer
0 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2023-11-14 13:14 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, 13 Nov 2023 19:30:08 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> but i dont feel like i fully understand the issue here so maybe iam missing
> the goal of this patchset somewhat
So, to summarize the main problem:
1. sws_init_single_context() previously hard-coded decisions based on
c->srcRange and c->dstRange. This is fundamentally broken, as
srcRange/dstRange can change at any time with
sws_setColorspaceDetails.
2. To fix this, this function was made to not early-return, and instead
run the rest of the init code just in case range conversion is needed
later. (With the check for whether or not the special converter can
be used being moved to the callsite instead of the setup site)
3. This caused problems for non-YUV inputs, because previously these
would always early return, but now they run the rest of the code,
which triggers at least one assertion for float32 formats.
4. To fix this, this commit restores the early-return for non-YUV,
preserving the status quo of existing behavior w.r.t not hitting the
rest of the init function.
5. Separately, this commit fixes an error in previous condition (2) at
the callsite, which relied on c->lumConvertRange being unset when no
range conversion is needed. However, that condition did not match the
condition used in the setup check before.
> * convert_unscaled should only be set when used
> OR
> * if these are set "always" if not alphablend and convert_unscaled should be
> two seperate fields. But i have not at all looked at what consequences that
> would have so maybe that has issues
convert_unscaled cannot be set only when used because we don't yet know
if it will be used or not. There is also no advantage I see to splitting
the fields, as they have basically the same logic attached to them
- being dependent only on whether or not range conversion is needed.
> Also if some range convert should not be used/set for some cases then
> the check should maybe be where the range convert is setup not far away
> from it. I mean a check close to the related code is easier to understand
>
One alternative that would make this possible would be to re-run whole
context init from sws_setColorspaceDetails, if the srcRange/dstRange
change.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-14 13:14 ` Niklas Haas
@ 2023-11-14 22:52 ` Michael Niedermayer
2023-11-22 12:45 ` Niklas Haas
0 siblings, 1 reply; 13+ messages in thread
From: Michael Niedermayer @ 2023-11-14 22:52 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 3105 bytes --]
Hi
On Tue, Nov 14, 2023 at 02:14:37PM +0100, Niklas Haas wrote:
> On Mon, 13 Nov 2023 19:30:08 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > but i dont feel like i fully understand the issue here so maybe iam missing
> > the goal of this patchset somewhat
>
> So, to summarize the main problem:
>
> 1. sws_init_single_context() previously hard-coded decisions based on
> c->srcRange and c->dstRange. This is fundamentally broken, as
> srcRange/dstRange can change at any time with
> sws_setColorspaceDetails.
>
> 2. To fix this, this function was made to not early-return, and instead
> run the rest of the init code just in case range conversion is needed
> later. (With the check for whether or not the special converter can
> be used being moved to the callsite instead of the setup site)
>
> 3. This caused problems for non-YUV inputs, because previously these
> would always early return, but now they run the rest of the code,
> which triggers at least one assertion for float32 formats.
>
> 4. To fix this, this commit restores the early-return for non-YUV,
> preserving the status quo of existing behavior w.r.t not hitting the
> rest of the init function.
>
> 5. Separately, this commit fixes an error in previous condition (2) at
> the callsite, which relied on c->lumConvertRange being unset when no
> range conversion is needed. However, that condition did not match the
> condition used in the setup check before.
>
> > * convert_unscaled should only be set when used
> > OR
> > * if these are set "always" if not alphablend and convert_unscaled should be
> > two seperate fields. But i have not at all looked at what consequences that
> > would have so maybe that has issues
>
> convert_unscaled cannot be set only when used because we don't yet know
> if it will be used or not. There is also no advantage I see to splitting
> the fields, as they have basically the same logic attached to them
> - being dependent only on whether or not range conversion is needed.
>
> > Also if some range convert should not be used/set for some cases then
> > the check should maybe be where the range convert is setup not far away
> > from it. I mean a check close to the related code is easier to understand
> >
>
> One alternative that would make this possible would be to re-run whole
> context init from sws_setColorspaceDetails, if the srcRange/dstRange
> change.
would this result in overall cleaner code or do you see some problems
with this ?
Given the messi-ness that the always setting results in i would maybe
suggest to explore this and see if this is cleaner.
Its conceptually not wrong that if parameters change that init should
be redone.
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-14 22:52 ` Michael Niedermayer
@ 2023-11-22 12:45 ` Niklas Haas
2023-11-22 13:16 ` Michael Niedermayer
0 siblings, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2023-11-22 12:45 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, 14 Nov 2023 23:52:50 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> would this result in overall cleaner code or do you see some problems
> with this ?
>
> Given the messi-ness that the always setting results in i would maybe
> suggest to explore this and see if this is cleaner.
>
> Its conceptually not wrong that if parameters change that init should
> be redone.
I gave this a try, but doing it internally is very tricky for a number
of reasons and does not present obvious advantages over requiring the
user to free+reinit if they wish to change range. So, the best long-term
solution here would be to simply remove srcRange/dstRange from the
signature of sws_setColorspaceDetails.
vf_scale is the only current user of this API inside ffmpeg itself, and
after the YUVJ removal series this call is no longer needed at all. (All
range setup happens at filter init time with filter range negotiation)
I think merging this series as-is represents the best short-term fix to
the existing fundamental issues with this design. But if you want to
rewrite all of swscale init code to allow graceful re-init on range
change, be my guest.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context
2023-11-22 12:45 ` Niklas Haas
@ 2023-11-22 13:16 ` Michael Niedermayer
0 siblings, 0 replies; 13+ messages in thread
From: Michael Niedermayer @ 2023-11-22 13:16 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2111 bytes --]
Hi
On Wed, Nov 22, 2023 at 01:45:05PM +0100, Niklas Haas wrote:
> On Tue, 14 Nov 2023 23:52:50 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > would this result in overall cleaner code or do you see some problems
> > with this ?
> >
> > Given the messi-ness that the always setting results in i would maybe
> > suggest to explore this and see if this is cleaner.
> >
> > Its conceptually not wrong that if parameters change that init should
> > be redone.
>
> I gave this a try, but doing it internally is very tricky for a number
> of reasons
ok
> and does not present obvious advantages over requiring the
> user to free+reinit if they wish to change range. So, the best long-term
> solution here would be to simply remove srcRange/dstRange from the
> signature of sws_setColorspaceDetails.
this doesnt feel right
logic should be:
1. alloc struct
2. set all details for everything
3. init
4+ use
n free
or some API with
convert between 2 frames and have a automatically cached and managed context
where all details are either in the metadata of the frames itself or given to
the function
The whole idea of adjusting some details which could affect the required
codepath without init is fishy unless everything can be adjusted that way
and its the normal way of initing things
So IMHO
first lets figure out how this should be in the long run (moving to a
clean API and clean implemenattion)
and then find out how to move towards that in small steps that achieves
teh short term goals quickly
I dont like trying to achieve the short term goal with messy code
and the long term unrelated.
Because i have to maintain this and so i will not agree to something
that moves us away from a clean long term result
That said, if you must change the API, change the API, that i do not
mind
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH 3/3] swscale/utils: don't early return in yuv alpha blendaway
2023-11-13 15:32 [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Niklas Haas
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context Niklas Haas
@ 2023-11-13 15:32 ` Niklas Haas
2023-11-14 2:27 ` [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Chen, Wenbin
2 siblings, 0 replies; 13+ messages in thread
From: Niklas Haas @ 2023-11-13 15:32 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
If changing YUV range after init results in the special converter no
longer being picked, then we need the rest of the init function to have
been hit.
Fixes: cedf589c09c567b72bf4c1a58db53d94622567e1
---
libswscale/utils.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libswscale/utils.c b/libswscale/utils.c
index 7ce86f83ea..294b0b5ace 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1721,7 +1721,9 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
av_log(c, AV_LOG_INFO,
"alpha blendaway %s -> %s special converter is available\n",
av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
- return 0;
+
+ if (isAnyRGB(dstFormat))
+ return 0;
}
/* unscaled special cases */
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
2023-11-13 15:32 [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Niklas Haas
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context Niklas Haas
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 3/3] swscale/utils: don't early return in yuv alpha blendaway Niklas Haas
@ 2023-11-14 2:27 ` Chen, Wenbin
2023-11-27 2:10 ` Chen, Wenbin
2 siblings, 1 reply; 13+ messages in thread
From: Chen, Wenbin @ 2023-11-14 2:27 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> From: Niklas Haas <git@haasn.dev>
>
> This logic was incongruent with logic used elsewhere, where floating
> point formats are explicitly exempted from range conversion. Fixes an
> issue where floating point formats were not going through special
> unscaled converters even when it was otherwise possible.
> ---
> libswscale/swscale.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 46ba68fe6a..a66db22767 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -534,7 +534,8 @@ av_cold void ff_sws_init_range_convert(SwsContext
> *c)
> {
> c->lumConvertRange = NULL;
> c->chrConvertRange = NULL;
> - if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> + if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
> + !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
> if (c->dstBpc <= 14) {
> if (c->srcRange) {
> c->lumConvertRange = lumRangeFromJpeg_c;
> --
> 2.42.0
>
This patchset works for me. Thanks for your quick fixing.
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
2023-11-14 2:27 ` [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Chen, Wenbin
@ 2023-11-27 2:10 ` Chen, Wenbin
2023-12-12 7:54 ` Chen, Wenbin
2024-01-10 13:15 ` Niklas Haas
0 siblings, 2 replies; 13+ messages in thread
From: Chen, Wenbin @ 2023-11-27 2:10 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> > From: Niklas Haas <git@haasn.dev>
> >
> > This logic was incongruent with logic used elsewhere, where floating
> > point formats are explicitly exempted from range conversion. Fixes an
> > issue where floating point formats were not going through special
> > unscaled converters even when it was otherwise possible.
> > ---
> > libswscale/swscale.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > index 46ba68fe6a..a66db22767 100644
> > --- a/libswscale/swscale.c
> > +++ b/libswscale/swscale.c
> > @@ -534,7 +534,8 @@ av_cold void ff_sws_init_range_convert(SwsContext
> > *c)
> > {
> > c->lumConvertRange = NULL;
> > c->chrConvertRange = NULL;
> > - if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> > + if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
> > + !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
> > if (c->dstBpc <= 14) {
> > if (c->srcRange) {
> > c->lumConvertRange = lumRangeFromJpeg_c;
> > --
> > 2.42.0
> >
>
> This patchset works for me. Thanks for your quick fixing.
Ping. When can this patchset be merged?
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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
2023-11-27 2:10 ` Chen, Wenbin
@ 2023-12-12 7:54 ` Chen, Wenbin
2024-01-10 13:15 ` Niklas Haas
1 sibling, 0 replies; 13+ messages in thread
From: Chen, Wenbin @ 2023-12-12 7:54 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> > > From: Niklas Haas <git@haasn.dev>
> > >
> > > This logic was incongruent with logic used elsewhere, where floating
> > > point formats are explicitly exempted from range conversion. Fixes an
> > > issue where floating point formats were not going through special
> > > unscaled converters even when it was otherwise possible.
> > > ---
> > > libswscale/swscale.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > > index 46ba68fe6a..a66db22767 100644
> > > --- a/libswscale/swscale.c
> > > +++ b/libswscale/swscale.c
> > > @@ -534,7 +534,8 @@ av_cold void
> ff_sws_init_range_convert(SwsContext
> > > *c)
> > > {
> > > c->lumConvertRange = NULL;
> > > c->chrConvertRange = NULL;
> > > - if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> > > + if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
> > > + !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
> > > if (c->dstBpc <= 14) {
> > > if (c->srcRange) {
> > > c->lumConvertRange = lumRangeFromJpeg_c;
> > > --
> > > 2.42.0
> > >
> >
> > This patchset works for me. Thanks for your quick fixing.
>
> Ping. When can this patchset be merged?
>
> Thanks
> Wenbin
Anyone who can help to merge this patchset?
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".
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
2023-11-27 2:10 ` Chen, Wenbin
2023-12-12 7:54 ` Chen, Wenbin
@ 2024-01-10 13:15 ` Niklas Haas
2024-01-11 2:59 ` Chen, Wenbin
1 sibling, 1 reply; 13+ messages in thread
From: Niklas Haas @ 2024-01-10 13:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, 27 Nov 2023 02:10:11 +0000 "Chen, Wenbin" <wenbin.chen-at-intel.com@ffmpeg.org> wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > >
> > > This logic was incongruent with logic used elsewhere, where floating
> > > point formats are explicitly exempted from range conversion. Fixes an
> > > issue where floating point formats were not going through special
> > > unscaled converters even when it was otherwise possible.
> > > ---
> > > libswscale/swscale.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > > index 46ba68fe6a..a66db22767 100644
> > > --- a/libswscale/swscale.c
> > > +++ b/libswscale/swscale.c
> > > @@ -534,7 +534,8 @@ av_cold void ff_sws_init_range_convert(SwsContext
> > > *c)
> > > {
> > > c->lumConvertRange = NULL;
> > > c->chrConvertRange = NULL;
> > > - if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> > > + if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
> > > + !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
> > > if (c->dstBpc <= 14) {
> > > if (c->srcRange) {
> > > c->lumConvertRange = lumRangeFromJpeg_c;
> > > --
> > > 2.42.0
> > >
> >
> > This patchset works for me. Thanks for your quick fixing.
>
> Ping. When can this patchset be merged?
Hi, do you still need this patch, now that YUV negotiation means
vf_scale no longer changes the filter range dynamically?
>
> 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".
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float
2024-01-10 13:15 ` Niklas Haas
@ 2024-01-11 2:59 ` Chen, Wenbin
0 siblings, 0 replies; 13+ messages in thread
From: Chen, Wenbin @ 2024-01-11 2:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On Mon, 27 Nov 2023 02:10:11 +0000 "Chen, Wenbin" <wenbin.chen-at-
> intel.com@ffmpeg.org> wrote:
> > > > From: Niklas Haas <git@haasn.dev>
> > > >
> > > > This logic was incongruent with logic used elsewhere, where floating
> > > > point formats are explicitly exempted from range conversion. Fixes an
> > > > issue where floating point formats were not going through special
> > > > unscaled converters even when it was otherwise possible.
> > > > ---
> > > > libswscale/swscale.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > > > index 46ba68fe6a..a66db22767 100644
> > > > --- a/libswscale/swscale.c
> > > > +++ b/libswscale/swscale.c
> > > > @@ -534,7 +534,8 @@ av_cold void
> ff_sws_init_range_convert(SwsContext
> > > > *c)
> > > > {
> > > > c->lumConvertRange = NULL;
> > > > c->chrConvertRange = NULL;
> > > > - if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> > > > + if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat) &&
> > > > + !isFloat(c->srcFormat) && !isFloat(c->dstFormat)) {
> > > > if (c->dstBpc <= 14) {
> > > > if (c->srcRange) {
> > > > c->lumConvertRange = lumRangeFromJpeg_c;
> > > > --
> > > > 2.42.0
> > > >
> > >
> > > This patchset works for me. Thanks for your quick fixing.
> >
> > Ping. When can this patchset be merged?
>
> Hi, do you still need this patch, now that YUV negotiation means
> vf_scale no longer changes the filter range dynamically?
The problem is not solved.
Command " ffmpeg -i input.png -vf format=grayf32,format=gray8 output.png " still reports
error: "Assertion c->srcBpc == 16 failed at libswscale/x86/swscale.c:533"
After I add --disable-sse2, the problem 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".
> > _______________________________________________
> > 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".
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-01-11 3:00 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 15:32 [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Niklas Haas
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context Niklas Haas
2023-11-13 18:30 ` Michael Niedermayer
2023-11-14 13:14 ` Niklas Haas
2023-11-14 22:52 ` Michael Niedermayer
2023-11-22 12:45 ` Niklas Haas
2023-11-22 13:16 ` Michael Niedermayer
2023-11-13 15:32 ` [FFmpeg-devel] [PATCH 3/3] swscale/utils: don't early return in yuv alpha blendaway Niklas Haas
2023-11-14 2:27 ` [FFmpeg-devel] [PATCH 1/3] swscale: don't assign range converters for float Chen, Wenbin
2023-11-27 2:10 ` Chen, Wenbin
2023-12-12 7:54 ` Chen, Wenbin
2024-01-10 13:15 ` Niklas Haas
2024-01-11 2:59 ` Chen, Wenbin
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git