| Message ID | 20211022214538.13139-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | c7699f95bb00cd7835aca4452d83320e42550a1d |
| Headers | show |
| Series | [FFmpeg-devel,1/2] swscale/utils: Set all threads to the same colorspace even on failure | expand |
On 10/22/2021 6:45 PM, Michael Niedermayer wrote: > Fixes: ./ffplay dav.y4m -vf "scale=hd1080:threads=4" > Found-by: Paul > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libswscale/utils.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libswscale/utils.c b/libswscale/utils.c > index 367b0ea5015..3752c3ec38c 100644 > --- a/libswscale/utils.c > +++ b/libswscale/utils.c > @@ -874,15 +874,16 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4], > int need_reinit = 0; > > if (c->nb_slice_ctx) { > + int parent_ret = 0; > for (int i = 0; i < c->nb_slice_ctx; i++) { > int ret = sws_setColorspaceDetails(c->slice_ctx[i], inv_table, > srcRange, table, dstRange, > brightness, contrast, saturation); > if (ret < 0) > - return ret; > + parent_ret = ret; > } > > - return 0; > + return parent_ret; > } > > handle_formats(c); If you make this function not propagate the error value returned by av_image_alloc() so it will effectively return either 0 or -1 as stated by the doxy, you can change this to just do ret |= sws_setColorspaceDetails(...) instead of adding a new variable.
On Fri, Oct 22, 2021 at 10:53:07PM -0300, James Almer wrote: > On 10/22/2021 6:45 PM, Michael Niedermayer wrote: > > Fixes: ./ffplay dav.y4m -vf "scale=hd1080:threads=4" > > Found-by: Paul > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libswscale/utils.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/libswscale/utils.c b/libswscale/utils.c > > index 367b0ea5015..3752c3ec38c 100644 > > --- a/libswscale/utils.c > > +++ b/libswscale/utils.c > > @@ -874,15 +874,16 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4], > > int need_reinit = 0; > > if (c->nb_slice_ctx) { > > + int parent_ret = 0; > > for (int i = 0; i < c->nb_slice_ctx; i++) { > > int ret = sws_setColorspaceDetails(c->slice_ctx[i], inv_table, > > srcRange, table, dstRange, > > brightness, contrast, saturation); > > if (ret < 0) > > - return ret; > > + parent_ret = ret; > > } > > - return 0; > > + return parent_ret; > > } > > handle_formats(c); > > If you make this function not propagate the error value returned by > av_image_alloc() so it will effectively return either 0 or -1 as stated by > the doxy, you can change this to just do ret |= > sws_setColorspaceDetails(...) instead of adding a new variable. I think we should propagate the return of sws_setColorspaceDetails to itself ill post a patch improving the doxy thx [...]
On Fri, Oct 22, 2021 at 11:45:37PM +0200, Michael Niedermayer wrote: > Fixes: ./ffplay dav.y4m -vf "scale=hd1080:threads=4" > Found-by: Paul > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libswscale/utils.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) will apply [...]
diff --git a/libswscale/utils.c b/libswscale/utils.c index 367b0ea5015..3752c3ec38c 100644 --- a/libswscale/utils.c +++ b/libswscale/utils.c @@ -874,15 +874,16 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4], int need_reinit = 0; if (c->nb_slice_ctx) { + int parent_ret = 0; for (int i = 0; i < c->nb_slice_ctx; i++) { int ret = sws_setColorspaceDetails(c->slice_ctx[i], inv_table, srcRange, table, dstRange, brightness, contrast, saturation); if (ret < 0) - return ret; + parent_ret = ret; } - return 0; + return parent_ret; } handle_formats(c);
Fixes: ./ffplay dav.y4m -vf "scale=hd1080:threads=4" Found-by: Paul Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libswscale/utils.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)