diff mbox

[FFmpeg-devel,2/2] avcodec/mlpenc: fix undefined shift

Message ID 20171128195544.5283-2-onemda@gmail.com
State New
Headers

Commit Message

Paul B Mahol Nov. 28, 2017, 7:55 p.m. UTC 
Decreases artifacts.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/mlpenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rostislav Pehlivanov Nov. 29, 2017, 4:05 a.m. UTC | #1 
On 28 November 2017 at 19:55, Paul B Mahol <onemda@gmail.com> wrote:

> Decreases artifacts.
>
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavcodec/mlpenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
> index c588f5b904..eceb0ddbb2 100644
> --- a/libavcodec/mlpenc.c
> +++ b/libavcodec/mlpenc.c
> @@ -466,7 +466,7 @@ static void default_decoding_params(MLPEncodeContext
> *ctx,
>   */
>  static int inline number_sbits(int number)
>  {
> -    if (number < 0)
> +    if (number <= 0)
>          number++;
>
>      return av_log2(FFABS(number)) + 1 + !!number;
> --
> 2.11.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Doesn't really seem to do anything to lossless check failures (not in 32
bit mode either where most of the errors are, which is why its disabled).
Which undefined shift does it fix?
James Almer Nov. 29, 2017, 4:07 a.m. UTC | #2 
On 11/29/2017 1:05 AM, Rostislav Pehlivanov wrote:
> On 28 November 2017 at 19:55, Paul B Mahol <onemda@gmail.com> wrote:
> 
>> Decreases artifacts.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/mlpenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
>> index c588f5b904..eceb0ddbb2 100644
>> --- a/libavcodec/mlpenc.c
>> +++ b/libavcodec/mlpenc.c
>> @@ -466,7 +466,7 @@ static void default_decoding_params(MLPEncodeContext
>> *ctx,
>>   */
>>  static int inline number_sbits(int number)
>>  {
>> -    if (number < 0)
>> +    if (number <= 0)
>>          number++;
>>
>>      return av_log2(FFABS(number)) + 1 + !!number;
>> --
>> 2.11.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> Doesn't really seem to do anything to lossless check failures (not in 32
> bit mode either where most of the errors are, which is why its disabled).

It does for the sample in ticket 6216. It removes the check failure
errors, but the end result is apparently still not bitexact.

> Which undefined shift does it fix?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Paul B Mahol Nov. 29, 2017, 9:22 a.m. UTC | #3 
On 11/29/17, Rostislav Pehlivanov <atomnuker@gmail.com> wrote:
> On 28 November 2017 at 19:55, Paul B Mahol <onemda@gmail.com> wrote:
>
>> Decreases artifacts.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/mlpenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
>> index c588f5b904..eceb0ddbb2 100644
>> --- a/libavcodec/mlpenc.c
>> +++ b/libavcodec/mlpenc.c
>> @@ -466,7 +466,7 @@ static void default_decoding_params(MLPEncodeContext
>> *ctx,
>>   */
>>  static int inline number_sbits(int number)
>>  {
>> -    if (number < 0)
>> +    if (number <= 0)
>>          number++;
>>
>>      return av_log2(FFABS(number)) + 1 + !!number;
>> --
>> 2.11.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> Doesn't really seem to do anything to lossless check failures (not in 32
> bit mode either where most of the errors are, which is why its disabled).
> Which undefined shift does it fix?

result of this function can be 1 and it goes to be shifted as -1 in later calls.
diff mbox

Patch

diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
index c588f5b904..eceb0ddbb2 100644
--- a/libavcodec/mlpenc.c
+++ b/libavcodec/mlpenc.c
@@ -466,7 +466,7 @@  static void default_decoding_params(MLPEncodeContext *ctx,
  */
 static int inline number_sbits(int number)
 {
-    if (number < 0)
+    if (number <= 0)
         number++;
 
     return av_log2(FFABS(number)) + 1 + !!number;