| Message ID | 20190709201901.26693-1-me@jailuthra.in |
|---|---|
| State | Superseded |
| Headers | show |
Jul 9, 2019, 9:18 PM by me@jailuthra.in: > we need two bits instead of one bit to represent -1 in bitstream > > Signed-off-by: Jai Luthra <me@jailuthra.in> > --- > libavcodec/mlpenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c > index deb171645c..f4948451f1 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 < -1) > number++; > This is different from the first patch's version. Sure its correct now?
On Wed, Jul 10, 2019 at 12:14:56AM +0200, Lynne wrote: >Jul 9, 2019, 9:18 PM by me@jailuthra.in: > >> we need two bits instead of one bit to represent -1 in bitstream >> >> Signed-off-by: Jai Luthra <me@jailuthra.in> >> --- >> libavcodec/mlpenc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c >> index deb171645c..f4948451f1 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 < -1) >> number++; >> > >This is different from the first patch's version. Sure its correct now? Yep. Previous patch produced valid bitstream too, but this provides better compression [1] by representing numbers of the form -2^x with one less bit for x >= 1. This makes more sense, as we can represent -2 in two-bit twos-complement notation as `10` so output should be 2 bits (instead of 3 by previous patch). (similarly for -4, -8, -16, ...) The lossless errors were being caused when a block of samples were all either -1 or 0. This function implied all samples could be represented as single bit each, but down the pipeline after huff vlc calculations, the encoder pushed <nothing> on the bitstream for all samples, which was always interpreted as 0 by the decoder and never -1. NB: One can argue -1 and 0 in fact can be represented in a single bit as two's complement, 0 being `0` and -1 being `1`. But imho, single bit two's complement is a weird boundary case, and not considering it solves the issue here. If someone has a better idea pls suggest. [1]: tested using both patches on https://samples.ffmpeg.org/flac/When%20I%20Grow%20Up.flac. Previous patch compressed it to 28788216 byte MLP stream, this one compresses it to 28787834 byte MLP stream. Both streams are valid and decode to lossless bit-exact output. thx -- jlut
diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c index deb171645c..f4948451f1 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 < -1) number++; return av_log2(FFABS(number)) + 1 + !!number;
we need two bits instead of one bit to represent -1 in bitstream Signed-off-by: Jai Luthra <me@jailuthra.in> --- libavcodec/mlpenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)