Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 12:30:32PM -0500, Ronald S. Bultje wrote: > Hi, > > On Thu, Jan 14, 2016 at 12:06 PM, Michael Niedermayer < > mich...@niedermayer.cc> wrote: > > > On Thu, Jan 14, 2016 at 11:42:47AM -0500, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer < > > > mich...@niedermayer.cc> wrote: > > > > > > > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > > > > Hi, > > > > > > > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer < > > michae...@gmx.at> > > > > > wrote: > > > > > > > > > > > From: Michael Niedermayer > > > > > > > > > > > > This makes SWS more robust > > > > > > Fixes: > > > > > > > > > > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > > > > Fixes: out of array read > > > > > > > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > > > > Signed-off-by: Michael Niedermayer > > > > > > --- > > > > > > libswscale/swscale_internal.h |2 +- > > > > > > libswscale/yuv2rgb.c | 88 > > > > > > - > > > > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > > > > > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB > > in > > > > > swscale, probably through automated testing of ffmpeg.exe -i > > file.264 -f > > > > > null -. Can you elaborate on what happens exactly? I guess what I'm > > > > trying > > > > > to get at is, what's the input format (I'm going to assume it's > > yuv420), > > > > > what's the output (maybe rgb24?), why is it converting like that (is > > the > > > > > 264 changing pixfmt from frame to frame?), are the coefficients > > defaults > > > > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what > > _are_ > > > > > the coefficients, what coefficients can swscale use in these > > functions, > > > > and > > > > > what are the theoretical bounds of the index in each array based on > > these > > > > > coefficients? > > > > > > > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 > > in > > > > > each of these tables? How do we verify that it is correct? I remember > > > > Jason > > > > > tried to speed up av_clip_uint8() at some point by making it a > > table, and > > > > > we had to revert the use of that in many places (e.g. idcts) because > > we > > > > > cannot give a theoretical limit on input values, i.e. the table would > > > > have > > > > > to be infinitely long. I'm trying to figure out if that's the case > > here > > > > > also. > > > > > > > > input was IIRC 10bit YUV (probably with out of range values) > > > > > > > > > Ah! This gets very interesting. OK, so then it all makes a lot of sense. > > So > > > ... Is this valid input? I mean, the h264 decoder clips the output in > > each > > > stage (prediction, idct, loopfilter) to fit within the range, no? Or is > > > this something like PCM or whatever where we don't clip? Should we? > > > > > > I'm not saying we shouldn't allow clipping of input values in swscale > > also, > > > but perhaps we could have a flag that says that the input is already > > > clipped (similar to swresample). > > > > possibly, but that might be hard to get right > > one path for too big values is uinitialized memory, our error > > concealment code doesnt support some corner cases so it doesnt always > > clean all unset set stuff up, memory is also IIRC not cleared after > > alloc > > > > my reasoning for fixing it in swscale was that users will expect > > sws not to crash from out of range input samples > > > Uninitialized data in output frames seems like a pretty big deal to me > regardless of bugs in swscale, no? iam not too happy about it either [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
Hi, On Thu, Jan 14, 2016 at 12:06 PM, Michael Niedermayer < mich...@niedermayer.cc> wrote: > On Thu, Jan 14, 2016 at 11:42:47AM -0500, Ronald S. Bultje wrote: > > Hi, > > > > On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer < > > mich...@niedermayer.cc> wrote: > > > > > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > > > Hi, > > > > > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer < > michae...@gmx.at> > > > > wrote: > > > > > > > > > From: Michael Niedermayer > > > > > > > > > > This makes SWS more robust > > > > > Fixes: > > > > > > > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > > > Fixes: out of array read > > > > > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > > > Signed-off-by: Michael Niedermayer > > > > > --- > > > > > libswscale/swscale_internal.h |2 +- > > > > > libswscale/yuv2rgb.c | 88 > > > > > - > > > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB > in > > > > swscale, probably through automated testing of ffmpeg.exe -i > file.264 -f > > > > null -. Can you elaborate on what happens exactly? I guess what I'm > > > trying > > > > to get at is, what's the input format (I'm going to assume it's > yuv420), > > > > what's the output (maybe rgb24?), why is it converting like that (is > the > > > > 264 changing pixfmt from frame to frame?), are the coefficients > defaults > > > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what > _are_ > > > > the coefficients, what coefficients can swscale use in these > functions, > > > and > > > > what are the theoretical bounds of the index in each array based on > these > > > > coefficients? > > > > > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 > in > > > > each of these tables? How do we verify that it is correct? I remember > > > Jason > > > > tried to speed up av_clip_uint8() at some point by making it a > table, and > > > > we had to revert the use of that in many places (e.g. idcts) because > we > > > > cannot give a theoretical limit on input values, i.e. the table would > > > have > > > > to be infinitely long. I'm trying to figure out if that's the case > here > > > > also. > > > > > > input was IIRC 10bit YUV (probably with out of range values) > > > > > > Ah! This gets very interesting. OK, so then it all makes a lot of sense. > So > > ... Is this valid input? I mean, the h264 decoder clips the output in > each > > stage (prediction, idct, loopfilter) to fit within the range, no? Or is > > this something like PCM or whatever where we don't clip? Should we? > > > > I'm not saying we shouldn't allow clipping of input values in swscale > also, > > but perhaps we could have a flag that says that the input is already > > clipped (similar to swresample). > > possibly, but that might be hard to get right > one path for too big values is uinitialized memory, our error > concealment code doesnt support some corner cases so it doesnt always > clean all unset set stuff up, memory is also IIRC not cleared after > alloc > > my reasoning for fixing it in swscale was that users will expect > sws not to crash from out of range input samples Uninitialized data in output frames seems like a pretty big deal to me regardless of bugs in swscale, no? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 11:42:47AM -0500, Ronald S. Bultje wrote: > Hi, > > On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer < > mich...@niedermayer.cc> wrote: > > > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer > > > wrote: > > > > > > > From: Michael Niedermayer > > > > > > > > This makes SWS more robust > > > > Fixes: > > > > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > > Fixes: out of array read > > > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > libswscale/swscale_internal.h |2 +- > > > > libswscale/yuv2rgb.c | 88 > > > > - > > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB in > > > swscale, probably through automated testing of ffmpeg.exe -i file.264 -f > > > null -. Can you elaborate on what happens exactly? I guess what I'm > > trying > > > to get at is, what's the input format (I'm going to assume it's yuv420), > > > what's the output (maybe rgb24?), why is it converting like that (is the > > > 264 changing pixfmt from frame to frame?), are the coefficients defaults > > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what _are_ > > > the coefficients, what coefficients can swscale use in these functions, > > and > > > what are the theoretical bounds of the index in each array based on these > > > coefficients? > > > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 in > > > each of these tables? How do we verify that it is correct? I remember > > Jason > > > tried to speed up av_clip_uint8() at some point by making it a table, and > > > we had to revert the use of that in many places (e.g. idcts) because we > > > cannot give a theoretical limit on input values, i.e. the table would > > have > > > to be infinitely long. I'm trying to figure out if that's the case here > > > also. > > > > input was IIRC 10bit YUV (probably with out of range values) > > > Ah! This gets very interesting. OK, so then it all makes a lot of sense. So > ... Is this valid input? I mean, the h264 decoder clips the output in each > stage (prediction, idct, loopfilter) to fit within the range, no? Or is > this something like PCM or whatever where we don't clip? Should we? > > I'm not saying we shouldn't allow clipping of input values in swscale also, > but perhaps we could have a flag that says that the input is already > clipped (similar to swresample). possibly, but that might be hard to get right one path for too big values is uinitialized memory, our error concealment code doesnt support some corner cases so it doesnt always clean all unset set stuff up, memory is also IIRC not cleared after alloc my reasoning for fixing it in swscale was that users will expect sws not to crash from out of range input samples [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
Hi, On Thu, Jan 14, 2016 at 11:34 AM, Michael Niedermayer < mich...@niedermayer.cc> wrote: > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > Hi, > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer > > wrote: > > > > > From: Michael Niedermayer > > > > > > This makes SWS more robust > > > Fixes: > > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > Fixes: out of array read > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > Signed-off-by: Michael Niedermayer > > > --- > > > libswscale/swscale_internal.h |2 +- > > > libswscale/yuv2rgb.c | 88 > > > - > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB in > > swscale, probably through automated testing of ffmpeg.exe -i file.264 -f > > null -. Can you elaborate on what happens exactly? I guess what I'm > trying > > to get at is, what's the input format (I'm going to assume it's yuv420), > > what's the output (maybe rgb24?), why is it converting like that (is the > > 264 changing pixfmt from frame to frame?), are the coefficients defaults > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what _are_ > > the coefficients, what coefficients can swscale use in these functions, > and > > what are the theoretical bounds of the index in each array based on these > > coefficients? > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 in > > each of these tables? How do we verify that it is correct? I remember > Jason > > tried to speed up av_clip_uint8() at some point by making it a table, and > > we had to revert the use of that in many places (e.g. idcts) because we > > cannot give a theoretical limit on input values, i.e. the table would > have > > to be infinitely long. I'm trying to figure out if that's the case here > > also. > > input was IIRC 10bit YUV (probably with out of range values) Ah! This gets very interesting. OK, so then it all makes a lot of sense. So ... Is this valid input? I mean, the h264 decoder clips the output in each stage (prediction, idct, loopfilter) to fit within the range, no? Or is this something like PCM or whatever where we don't clip? Should we? I'm not saying we shouldn't allow clipping of input values in swscale also, but perhaps we could have a flag that says that the input is already clipped (similar to swresample). Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 05:34:25PM +0100, Michael Niedermayer wrote: > On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > > Hi, > > > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer > > wrote: > > > > > From: Michael Niedermayer > > > > > > This makes SWS more robust > > > Fixes: > > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > > Fixes: out of array read > > > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > > Signed-off-by: Michael Niedermayer > > > --- > > > libswscale/swscale_internal.h |2 +- > > > libswscale/yuv2rgb.c | 88 > > > - > > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB in > > swscale, probably through automated testing of ffmpeg.exe -i file.264 -f > > null -. Can you elaborate on what happens exactly? I guess what I'm trying > > to get at is, what's the input format (I'm going to assume it's yuv420), > > what's the output (maybe rgb24?), why is it converting like that (is the > > 264 changing pixfmt from frame to frame?), are the coefficients defaults > > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what _are_ > > the coefficients, what coefficients can swscale use in these functions, and > > what are the theoretical bounds of the index in each array based on these > > coefficients? > > > > In other words, why do we need a headroom of 256/512/384/326/896/838 in > > each of these tables? How do we verify that it is correct? I remember Jason > > tried to speed up av_clip_uint8() at some point by making it a table, and > > we had to revert the use of that in many places (e.g. idcts) because we > > cannot give a theoretical limit on input values, i.e. the table would have > > to be infinitely long. I'm trying to figure out if that's the case here > > also. > > input was IIRC 10bit YUV (probably with out of range values) > > the yuv tables likely are still not large enough, but a theoretical > limit exists due to intermediate being 16bit -> hscale -> 32bit >>19 > i tried av_clip_uint8 but it was alot slower speedloss for clip was 1% overall that is time ffmpeg ... (thats with cpuflags 0 as the code isnt used otherwise) the clip check was with if((y1|y2|u|v) & CONST) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 10:09:13AM -0500, Ronald S. Bultje wrote: > Hi, > > On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer > wrote: > > > From: Michael Niedermayer > > > > This makes SWS more robust > > Fixes: > > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > > Fixes: out of array read > > > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > > Signed-off-by: Michael Niedermayer > > --- > > libswscale/swscale_internal.h |2 +- > > libswscale/yuv2rgb.c | 88 > > - > > 2 files changed, 45 insertions(+), 45 deletions(-) > > > So ... I'm kind of confused. You have a 264 file that triggers a OOB in > swscale, probably through automated testing of ffmpeg.exe -i file.264 -f > null -. Can you elaborate on what happens exactly? I guess what I'm trying > to get at is, what's the input format (I'm going to assume it's yuv420), > what's the output (maybe rgb24?), why is it converting like that (is the > 264 changing pixfmt from frame to frame?), are the coefficients defaults > for a particular colorspace/range (e.g. bt601/mpeg) or custom, what _are_ > the coefficients, what coefficients can swscale use in these functions, and > what are the theoretical bounds of the index in each array based on these > coefficients? > > In other words, why do we need a headroom of 256/512/384/326/896/838 in > each of these tables? How do we verify that it is correct? I remember Jason > tried to speed up av_clip_uint8() at some point by making it a table, and > we had to revert the use of that in many places (e.g. idcts) because we > cannot give a theoretical limit on input values, i.e. the table would have > to be infinitely long. I'm trying to figure out if that's the case here > also. input was IIRC 10bit YUV (probably with out of range values) the yuv tables likely are still not large enough, but a theoretical limit exists due to intermediate being 16bit -> hscale -> 32bit >>19 i tried av_clip_uint8 but it was alot slower [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
Hi, On Wed, Jan 13, 2016 at 9:50 PM, Michael Niedermayer wrote: > From: Michael Niedermayer > > This makes SWS more robust > Fixes: > 07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264 > Fixes: out of array read > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > Signed-off-by: Michael Niedermayer > --- > libswscale/swscale_internal.h |2 +- > libswscale/yuv2rgb.c | 88 > - > 2 files changed, 45 insertions(+), 45 deletions(-) So ... I'm kind of confused. You have a 264 file that triggers a OOB in swscale, probably through automated testing of ffmpeg.exe -i file.264 -f null -. Can you elaborate on what happens exactly? I guess what I'm trying to get at is, what's the input format (I'm going to assume it's yuv420), what's the output (maybe rgb24?), why is it converting like that (is the 264 changing pixfmt from frame to frame?), are the coefficients defaults for a particular colorspace/range (e.g. bt601/mpeg) or custom, what _are_ the coefficients, what coefficients can swscale use in these functions, and what are the theoretical bounds of the index in each array based on these coefficients? In other words, why do we need a headroom of 256/512/384/326/896/838 in each of these tables? How do we verify that it is correct? I remember Jason tried to speed up av_clip_uint8() at some point by making it a table, and we had to revert the use of that in many places (e.g. idcts) because we cannot give a theoretical limit on input values, i.e. the table would have to be infinitely long. I'm trying to figure out if that's the case here also. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 10:09:02AM +, Paul B Mahol wrote: > On 1/14/16, Christophe Gisquet wrote: > > Hi, > > > > 2016-01-14 9:41 GMT+01:00 Michael Niedermayer : > >> there are 2 seperate tables, the headroom in them also differs > >> curently, also the exact relation between input values and headroom > >> needed in the 2nd table depends on the yuv-rgb coefficients > >> > >> i can replace the 2nd tables numbers by a define if people want ? > > > > The required changes are probably non-trivial, but seeing how much > > code that change actually impacted, I would feel very uncomfortable > > next time someone else has to change this. > > > > I'm not sure about the road to follow. Surely there are people with > > strong opinions on swscale that could provide some on this topic. > > Leave it to the maintainer hands. factored the define out and applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On 1/14/16, Christophe Gisquet wrote: > Hi, > > 2016-01-14 9:41 GMT+01:00 Michael Niedermayer : >> there are 2 seperate tables, the headroom in them also differs >> curently, also the exact relation between input values and headroom >> needed in the 2nd table depends on the yuv-rgb coefficients >> >> i can replace the 2nd tables numbers by a define if people want ? > > The required changes are probably non-trivial, but seeing how much > code that change actually impacted, I would feel very uncomfortable > next time someone else has to change this. > > I'm not sure about the road to follow. Surely there are people with > strong opinions on swscale that could provide some on this topic. Leave it to the maintainer hands. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
Hi, 2016-01-14 9:41 GMT+01:00 Michael Niedermayer : > there are 2 seperate tables, the headroom in them also differs > curently, also the exact relation between input values and headroom > needed in the 2nd table depends on the yuv-rgb coefficients > > i can replace the 2nd tables numbers by a define if people want ? The required changes are probably non-trivial, but seeing how much code that change actually impacted, I would feel very uncomfortable next time someone else has to change this. I'm not sure about the road to follow. Surely there are people with strong opinions on swscale that could provide some on this topic. Best regards, Christophe ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
On Thu, Jan 14, 2016 at 08:58:20AM +0100, Christophe Gisquet wrote: > > -#define YUVRGB_TABLE_HEADROOM 256 > > +#define YUVRGB_TABLE_HEADROOM 512 > [...] > > -const int yoffs = fullRange ? 384 : 326; > > +const int yoffs = fullRange ? 896 : 838; > > I think it's time to use that macro everywhere it is actually used without > showing up. there are 2 seperate tables, the headroom in them also differs curently, also the exact relation between input values and headroom needed in the 2nd table depends on the yuv-rgb coefficients i can replace the 2nd tables numbers by a define if people want ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
> -#define YUVRGB_TABLE_HEADROOM 256 > +#define YUVRGB_TABLE_HEADROOM 512 [...] > -const int yoffs = fullRange ? 384 : 326; > +const int yoffs = fullRange ? 896 : 838; I think it's time to use that macro everywhere it is actually used without showing up. Best regards, Christophe ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] swscale/yuv2rgb: Increase YUV2RGB table headroom
From: Michael Niedermayer
This makes SWS more robust
Fixes:
07650a772d98aa63b0fed6370dc89037/asan_heap-oob_27ddeaf_2657_2c81ff264dee5d9712cb3251fb9c3bbb.264
Fixes: out of array read
Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
Signed-off-by: Michael Niedermayer
---
libswscale/swscale_internal.h |2 +-
libswscale/yuv2rgb.c | 88 -
2 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index a53fdc4..305db4a 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -39,7 +39,7 @@
#define STR(s) AV_TOSTRING(s) // AV_STRINGIFY is too long
-#define YUVRGB_TABLE_HEADROOM 256
+#define YUVRGB_TABLE_HEADROOM 512
#define MAX_FILTER_SIZE SWS_MAX_FILTER_SIZE
diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c
index 1d682ba..723bec2 100644
--- a/libswscale/yuv2rgb.c
+++ b/libswscale/yuv2rgb.c
@@ -776,7 +776,7 @@ av_cold int ff_yuv2rgb_c_init_tables(SwsContext *c, const
int inv_table[4],
uint16_t *y_table16;
uint32_t *y_table32;
int i, base, rbase, gbase, bbase, av_uninit(abase), needAlpha;
-const int yoffs = fullRange ? 384 : 326;
+const int yoffs = fullRange ? 896 : 838;
int64_t crv = inv_table[0];
int64_t cbu = inv_table[1];
@@ -833,10 +833,10 @@ av_cold int ff_yuv2rgb_c_init_tables(SwsContext *c, const
int inv_table[4],
return AVERROR(ENOMEM);
switch (bpp) {
case 1:
-ALLOC_YUV_TABLE(1024);
+ALLOC_YUV_TABLE(2048);
y_table = c->yuvTable;
-yb = -(384 << 16) - oy;
-for (i = 0; i < 1024 - 110; i++) {
+yb = -(384 << 16) - 512*cy - oy;
+for (i = 0; i < 2048 - 110; i++) {
y_table[i + 110] = av_clip_uint8((yb + 0x8000) >> 16) >> 7;
yb += cy;
}
@@ -848,60 +848,60 @@ av_cold int ff_yuv2rgb_c_init_tables(SwsContext *c, const
int inv_table[4],
rbase = isRgb ? 3 : 0;
gbase = 1;
bbase = isRgb ? 0 : 3;
-ALLOC_YUV_TABLE(1024 * 3);
+ALLOC_YUV_TABLE(2048 * 3);
y_table = c->yuvTable;
-yb = -(384 << 16) - oy;
-for (i = 0; i < 1024 - 110; i++) {
+yb = -(384 << 16) - 512*cy - oy;
+for (i = 0; i < 2048 - 110; i++) {
int yval= av_clip_uint8((yb + 0x8000) >> 16);
y_table[i + 110]= (yval >> 7)<< rbase;
-y_table[i + 37 + 1024] = ((yval + 43) / 85) << gbase;
-y_table[i + 110 + 2048] = (yval >> 7)<< bbase;
+y_table[i + 37 + 2048] = ((yval + 43) / 85) << gbase;
+y_table[i + 110 + 4096] = (yval >> 7)<< bbase;
yb += cy;
}
fill_table(c->table_rV, 1, crv, y_table + yoffs);
-fill_table(c->table_gU, 1, cgu, y_table + yoffs + 1024);
-fill_table(c->table_bU, 1, cbu, y_table + yoffs + 2048);
+fill_table(c->table_gU, 1, cgu, y_table + yoffs + 2048);
+fill_table(c->table_bU, 1, cbu, y_table + yoffs + 4096);
fill_gv_table(c->table_gV, 1, cgv);
break;
case 8:
rbase = isRgb ? 5 : 0;
gbase = isRgb ? 2 : 3;
bbase = isRgb ? 0 : 6;
-ALLOC_YUV_TABLE(1024 * 3);
+ALLOC_YUV_TABLE(2048 * 3);
y_table = c->yuvTable;
-yb = -(384 << 16) - oy;
-for (i = 0; i < 1024 - 38; i++) {
+yb = -(384 << 16) - 512*cy - oy;
+for (i = 0; i < 2048 - 38; i++) {
int yval = av_clip_uint8((yb + 0x8000) >> 16);
y_table[i + 16]= ((yval + 18) / 36) << rbase;
-y_table[i + 16 + 1024] = ((yval + 18) / 36) << gbase;
-y_table[i + 37 + 2048] = ((yval + 43) / 85) << bbase;
+y_table[i + 16 + 2048] = ((yval + 18) / 36) << gbase;
+y_table[i + 37 + 4096] = ((yval + 43) / 85) << bbase;
yb += cy;
}
fill_table(c->table_rV, 1, crv, y_table + yoffs);
-fill_table(c->table_gU, 1, cgu, y_table + yoffs + 1024);
-fill_table(c->table_bU, 1, cbu, y_table + yoffs + 2048);
+fill_table(c->table_gU, 1, cgu, y_table + yoffs + 2048);
+fill_table(c->table_bU, 1, cbu, y_table + yoffs + 4096);
fill_gv_table(c->table_gV, 1, cgv);
break;
case 12:
rbase = isRgb ? 8 : 0;
gbase = 4;
bbase = isRgb ? 0 : 8;
-ALLOC_YUV_TABLE(1024 * 3 * 2);
+ALLOC_YUV_TABLE(2048 * 3 * 2);
y_table16 = c->yuvTable;
-yb = -(384 << 16) - oy;
-for (i = 0; i < 1024; i++) {
+yb = -(384 << 16) - 512*cy - oy;
+for (i = 0; i < 2048; i++) {
uint8_t yval= av_clip_uint8((yb + 0x8000) >> 16);
y_table16[i]= (yval >> 4) << rbase;
-y_table16[i + 1024] = (yval >> 4) << gbas
