Opened 5 years ago

Closed 5 years ago

#8556 closed defect (invalid)

PCM 24-bit integer sample format encoding introduces more quantification error

Reported by: Prcuvu Owned by:
Priority: normal Component: avcodec
Version: unspecified Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
The 24-bit integer sample format encoding is implemented as such in libavcodec/pcm.c:

 69 #define ENCODE(type, endian, src, dst, n, shift, offset)                \
 70     samples_ ## type = (const type *) src;                              \
 71     for (; n > 0; n--) {                                                \
 72         register type v = (*samples_ ## type++ >> shift) + offset;      \
 73         bytestream_put_ ## endian(&dst, v);                             \
 74     }
...
115     case AV_CODEC_ID_PCM_S24LE:
116         ENCODE(int32_t, le24, samples, dst, n, 8, 0)
117         break;
...
609 PCM_CODEC  (PCM_S24LE,        AV_SAMPLE_FMT_S32, pcm_s24le,        "PCM signed 24-bit little-endian");

The codec uses 32-bit signed integer internally and right-shift to 24-bit on encoding without checking magnitude of the least significant 8 bits. Ideally it should do some rounding check.

In libswresample/audioconvert.c:

 80 CONV_FUNC(AV_SAMPLE_FMT_S32, int32_t, AV_SAMPLE_FMT_FLT, av_clipl_int32(llrintf(*(const float*)pi * (1U<<31))))

It is using llrintf which rounds to the nearest representable value by default.

If I try to convert a 32-bit floating point PCM wave file to 24-bit signed integer sample format, the behaviour is inconsistent with that converting to 32-bit signed integer sample format. For the 24-bit integer case, it is effectively rounding down to negative infinity.

For example, assume there is an input floating point value which is (0.75 / 223). The desired 24-bit integer value should be 1 with correct rounding.
With the current implementation, it will be converted to a 32-bit integer value of 192 first, then right-shifted by 8 bits. The final result is 0, which can be seen as being rounded down.

This issue resides in all conversions involving 24-bit integer as target format.

Possible solutions include adding a new AV_SAMPLE_FMT_S24 format in avutil or adding rounding check in ENCODE macro in libavcodec/pcm.c.

How to reproduce:

% ffmpeg -i [f32.wav] -acodec pcm_s24le [s24.wav]

Attachments (2)

f32.wav (48 bytes ) - added by Ishisashi 5 years ago.
input sample
log.txt (2.1 KB ) - added by Ishisashi 5 years ago.
console output

Download all attachments as: .zip

Change History (5)

comment:1 by Carl Eugen Hoyos, 5 years ago

Keywords: avcodec PCM sample rounding removed

This has currently no similarities with a valid ticket: Are you reporting an issue in libswresample or libavcodec? Please provide the command line you tested together with the complete, uncut console output, provide an input sample and explain what is wrong with the output file.
If you already know how to fix this issue, please send your patch to the FFmpeg development mailing list, no reason to argue here on the bug tracker.

by Ishisashi, 5 years ago

Attachment: f32.wav added

input sample

by Ishisashi, 5 years ago

Attachment: log.txt added

console output

comment:2 by Ishisashi, 5 years ago

The input value is 0x33C00000 (8.94069671630859375×10-8) and the output value is 0x000000 (0).
(8.94069671630859375×10-8×223) is 0.75, which is rounded to 0 in output.

Last edited 5 years ago by Ishisashi (previous) (diff)

comment:3 by Elon Musk, 5 years ago

Resolution: invalid
Status: newclosed

Nowhere to reproduce hearable "quantification" errors. Thus closing.

Note: See TracTickets for help on using tickets.