| Message ID | 20170101232833.24472-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On 1/1/2017 8:28 PM, Michael Niedermayer wrote: > Fixes CID1396405 > > Untested except fate which does not test this, please someone test this The encoder seems to need libsnappy, so no FATE test can be added. CCing Vittorio since he wrote this. He may be able to test and/or review. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/texturedspenc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/texturedspenc.c b/libavcodec/texturedspenc.c > index 8b2863033b..5171c69d82 100644 > --- a/libavcodec/texturedspenc.c > +++ b/libavcodec/texturedspenc.c > @@ -255,11 +255,11 @@ static void optimize_colors(const uint8_t *block, ptrdiff_t stride, > > muv = minv = maxv = bp[0]; > for (y = 0; y < 4; y++) { > - for (x = 4; x < 4; x += 4) { > + for (x = 0; x < 4; x++) { > muv += bp[x * 4 + y * stride]; > - if (bp[x] < minv) > + if (bp[x * 4 + y * stride] < minv) > minv = bp[x * 4 + y * stride]; > - else if (bp[x] > maxv) > + else if (bp[x * 4 + y * stride] > maxv) > maxv = bp[x * 4 + y * stride]; > } > } >
On Mon, Jan 2, 2017 at 2:00 AM, James Almer <jamrial@gmail.com> wrote: > On 1/1/2017 8:28 PM, Michael Niedermayer wrote: >> Fixes CID1396405 What is the CID about? >> Untested except fate which does not test this, please someone test this > > The encoder seems to need libsnappy, so no FATE test can be added. Only the HAP encoder needs libsnappy, this module has no external dependencies. A possible way to test this is to encode a HAP video using -format option and hap_alpha or hap_q modes and then decode it back with the internal decoder. > CCing Vittorio since he wrote this. He may be able to test and/or > review. Thanks for the CC.
On Tue, Jan 03, 2017 at 02:15:20PM +0100, Vittorio Giovara wrote: > On Mon, Jan 2, 2017 at 2:00 AM, James Almer <jamrial@gmail.com> wrote: > > On 1/1/2017 8:28 PM, Michael Niedermayer wrote: > >> Fixes CID1396405 > > What is the CID about? sorry for not quoting this prviously, it isnt very interresting. my patch is based on just this and guesswork, which is why it should be tested. 257 for (y = 0; y < 4; y++) { assignment: Assigning: x = 4. const: At condition x < 4, the value of x must be equal to 4. dead_error_condition: The condition x < 4 cannot be true. 258 for (x = 4; x < 4; x += 4) { CID 1396405 (#1 of 1): Logically dead code (DEADCODE)dead_error_begin: Execution cannot reach this statement: muv += bp[x * 4 + y * stride];. 259 muv += bp[x * 4 + y * stride]; 260 if (bp[x] < minv) 261 minv = bp[x * 4 + y * stride]; 262 else if (bp[x] > maxv) 263 maxv = bp[x * 4 + y * stride]; 264 } 265 } [...]
On Wed, Jan 4, 2017 at 1:06 AM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Jan 03, 2017 at 02:15:20PM +0100, Vittorio Giovara wrote: >> On Mon, Jan 2, 2017 at 2:00 AM, James Almer <jamrial@gmail.com> wrote: >> > On 1/1/2017 8:28 PM, Michael Niedermayer wrote: >> >> Fixes CID1396405 >> >> What is the CID about? > > sorry for not quoting this prviously, it isnt very interresting. > my patch is based on just this and guesswork, which is why it should > be tested. > > 257 for (y = 0; y < 4; y++) { > assignment: Assigning: x = 4. > const: At condition x < 4, the value of x must be equal to 4. > dead_error_condition: The condition x < 4 cannot be true. > 258 for (x = 4; x < 4; x += 4) { > CID 1396405 (#1 of 1): Logically dead code (DEADCODE)dead_error_begin: Execution cannot reach this statement: muv += bp[x * 4 + y * stride];. Ah, yes that seems wrong, probably a leftover of the code conversion (from https://github.com/nothings/stb/blob/master/stb_dxt.h#L293-L298) I believe it should be sufficient to do `for (x = 0; x < 4; x++)` but I won't be able to test that for a while I'm afraid
On Wed, Jan 04, 2017 at 11:52:50PM +0100, Vittorio Giovara wrote: > On Wed, Jan 4, 2017 at 1:06 AM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Tue, Jan 03, 2017 at 02:15:20PM +0100, Vittorio Giovara wrote: > >> On Mon, Jan 2, 2017 at 2:00 AM, James Almer <jamrial@gmail.com> wrote: > >> > On 1/1/2017 8:28 PM, Michael Niedermayer wrote: > >> >> Fixes CID1396405 > >> > >> What is the CID about? > > > > sorry for not quoting this prviously, it isnt very interresting. > > my patch is based on just this and guesswork, which is why it should > > be tested. > > > > 257 for (y = 0; y < 4; y++) { > > assignment: Assigning: x = 4. > > const: At condition x < 4, the value of x must be equal to 4. > > dead_error_condition: The condition x < 4 cannot be true. > > 258 for (x = 4; x < 4; x += 4) { > > CID 1396405 (#1 of 1): Logically dead code (DEADCODE)dead_error_begin: Execution cannot reach this statement: muv += bp[x * 4 + y * stride];. > > Ah, yes that seems wrong, probably a leftover of the code conversion > (from https://github.com/nothings/stb/blob/master/stb_dxt.h#L293-L298) > I believe it should be sufficient to do `for (x = 0; x < 4; x++)` but > I won't be able to test that for a while I'm afraid This issue seems stil open, i just stumbled accross the CID again when looking at coverity anyone has means to test this ? thx [...]
On Tue, 24 May 2022, Michael Niedermayer wrote: > On Wed, Jan 04, 2017 at 11:52:50PM +0100, Vittorio Giovara wrote: >> On Wed, Jan 4, 2017 at 1:06 AM, Michael Niedermayer >> <michael@niedermayer.cc> wrote: >>> On Tue, Jan 03, 2017 at 02:15:20PM +0100, Vittorio Giovara wrote: >>>> On Mon, Jan 2, 2017 at 2:00 AM, James Almer <jamrial@gmail.com> wrote: >>>>> On 1/1/2017 8:28 PM, Michael Niedermayer wrote: >>>>>> Fixes CID1396405 >>>> >>>> What is the CID about? >>> >>> sorry for not quoting this prviously, it isnt very interresting. >>> my patch is based on just this and guesswork, which is why it should >>> be tested. >>> >>> 257 for (y = 0; y < 4; y++) { >>> assignment: Assigning: x = 4. >>> const: At condition x < 4, the value of x must be equal to 4. >>> dead_error_condition: The condition x < 4 cannot be true. >>> 258 for (x = 4; x < 4; x += 4) { >>> CID 1396405 (#1 of 1): Logically dead code (DEADCODE)dead_error_begin: Execution cannot reach this statement: muv += bp[x * 4 + y * stride];. >> >> Ah, yes that seems wrong, probably a leftover of the code conversion >> (from https://github.com/nothings/stb/blob/master/stb_dxt.h#L293-L298) >> I believe it should be sufficient to do `for (x = 0; x < 4; x++)` but >> I won't be able to test that for a while I'm afraid > > This issue seems stil open, i just stumbled accross the CID again > when looking at coverity > anyone has means to test this ? Actually fate now covers this in vbn-dxt5 and vbn-dxt1 tests. Average MSE/PSNR is slightly improved with the patch, so I have applied it. Regards, Marton
diff --git a/libavcodec/texturedspenc.c b/libavcodec/texturedspenc.c index 8b2863033b..5171c69d82 100644 --- a/libavcodec/texturedspenc.c +++ b/libavcodec/texturedspenc.c @@ -255,11 +255,11 @@ static void optimize_colors(const uint8_t *block, ptrdiff_t stride, muv = minv = maxv = bp[0]; for (y = 0; y < 4; y++) { - for (x = 4; x < 4; x += 4) { + for (x = 0; x < 4; x++) { muv += bp[x * 4 + y * stride]; - if (bp[x] < minv) + if (bp[x * 4 + y * stride] < minv) minv = bp[x * 4 + y * stride]; - else if (bp[x] > maxv) + else if (bp[x * 4 + y * stride] > maxv) maxv = bp[x * 4 + y * stride]; } }
Fixes CID1396405 Untested except fate which does not test this, please someone test this Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/texturedspenc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)