Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [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

* [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 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 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 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

* 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