[PATCH] avcodec/texturedspenc: Fix indexing in color distribution determination
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(-) 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]; } } -- 2.11.0
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. -- Vittorio
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 } [...] -- 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
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 -- Vittorio
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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Never trust a computer, one day, it may think you are the virus. -- Compn
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
participants (4)
-
James Almer -
Marton Balint -
Michael Niedermayer -
Vittorio Giovara