* [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
@ 2023-10-25 11:12 Paul B Mahol
2023-10-25 18:39 ` Tomas Härdin
0 siblings, 1 reply; 8+ messages in thread
From: Paul B Mahol @ 2023-10-25 11:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 14 bytes --]
Set attached.
[-- Attachment #2: 0003-avcodec-mlpdec-support-for-truehd-with-channels-not-.patch --]
[-- Type: text/x-patch, Size: 1225 bytes --]
From 0e211362a11f58b8cac49b3740c1949b0468541c Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Wed, 25 Oct 2023 12:43:13 +0200
Subject: [PATCH 3/4] avcodec/mlpdec: support for truehd with channels not
representable with 5bit field in second stream
Fixes decoding for 4.0/4.1 layouts.
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/mlpdec.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c
index 11b5d2fe98..f1524b95a6 100644
--- a/libavcodec/mlpdec.c
+++ b/libavcodec/mlpdec.c
@@ -452,7 +452,10 @@ static int read_major_sync(MLPDecodeContext *m, GetBitContext *gb)
else
m->substream[2].mask = mh.channel_layout_thd_stream1;
if (m->avctx->ch_layout.nb_channels > 2)
- m->substream[mh.num_substreams > 1].mask = mh.channel_layout_thd_stream1;
+ if (mh.num_substreams > 2)
+ m->substream[1].mask = mh.channel_layout_thd_stream1;
+ else
+ m->substream[mh.num_substreams > 1].mask = mh.channel_layout_thd_stream2;
}
m->needs_reordering = mh.channel_arrangement >= 18 && mh.channel_arrangement <= 20;
--
2.42.0
[-- Attachment #3: 0001-avcodec-mlpenc-replace-naive-rematrix-with-brute-for.patch --]
[-- Type: text/x-patch, Size: 8655 bytes --]
From f5257ca9ed821e9fb3dd9edc3487da4d06ba47a3 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Wed, 25 Oct 2023 09:58:24 +0200
Subject: [PATCH 1/4] avcodec/mlpenc: replace naive rematrix with brute-force
search
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/mlpenc.c | 183 +++++++++++++++++++++++++++++++-------------
1 file changed, 129 insertions(+), 54 deletions(-)
diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
index 6b801605db..27ef5f2c82 100644
--- a/libavcodec/mlpenc.c
+++ b/libavcodec/mlpenc.c
@@ -136,7 +136,8 @@ typedef struct MLPEncodeContext {
int min_restart_interval; ///< Min interval of access units in between two major frames.
int cur_restart_interval;
int lpc_coeff_precision;
- int rematrix_precision;
+ int rematrix_search_step;
+ int rematrix_search_limit;
int lpc_type;
int lpc_passes;
int prediction_order;
@@ -1399,79 +1400,150 @@ static void determine_filters(MLPEncodeContext *ctx, MLPSubstream *s)
set_filter(ctx, s, ch, 0);
}
+static int invert2x2(const int32_t *s, int32_t *d)
+{
+ int64_t det;
+
+ d[0] = +s[3];
+ d[1] = -s[1];
+ d[2] = -s[2];
+ d[3] = +s[0];
+
+ det = (int64_t)s[0] * d[0] + (int64_t)s[1] * d[2];
+ if (det == 0LL)
+ return -1;
+
+ d[0] = (d[0] * (1LL << 28)) / det;
+ d[1] = (d[1] * (1LL << 28)) / det;
+ d[2] = (d[2] * (1LL << 28)) / det;
+ d[3] = (d[3] * (1LL << 28)) / det;
+
+ return 0;
+}
+
static int estimate_coeff(MLPEncodeContext *ctx, MLPSubstream *s,
- MatrixParams *mp,
- int ch0, int ch1)
+ MatrixParams *mp, int ch0, int ch1)
{
- int32_t maxl = INT32_MIN, maxr = INT32_MIN, minl = INT32_MAX, minr = INT32_MAX;
- int64_t summ = 0, sums = 0, suml = 0, sumr = 0, enl = 0, enr = 0;
- const int shift = 14 - ctx->rematrix_precision;
- int32_t cf0, cf1, e[4], d[4], ml, mr;
- int i, count = 0;
+ const int search_limit = 1 << ctx->rematrix_search_limit;
+ const int search_step = 1 << ctx->rematrix_search_step;
+ int32_t best[4], d[4], e[4], count = 0, chan = -1;
+ uint64_t best_sum = UINT64_MAX;
+ int32_t v[2], inc;
- for (int j = 0; j <= ctx->cur_restart_interval; j++) {
- DecodingParams *dp = &s->b[j].decoding_params;
- const int32_t *ch[2];
+ v[0] = 0;
+ v[1] = 0;
- ch[0] = dp->sample_buffer[ch0];
- ch[1] = dp->sample_buffer[ch1];
+ inc = search_step;
- for (int i = 0; i < dp->blocksize; i++) {
- int32_t lm = ch[0][i], rm = ch[1][i];
+ while (1) {
+ for (int c = 0; c < 2; c++) {
+ uint64_t sum = 0;
+
+ if (c) {
+ e[0] = 1 << 14;
+ e[1] = 0 << 14;
+ e[2] = v[1];
+ e[3] = v[0];
+ } else {
+ e[0] = v[0];
+ e[1] = v[1];
+ e[2] = 0 << 14;
+ e[3] = 1 << 14;
+ }
- enl += FFABS(lm);
- enr += FFABS(rm);
+ if (invert2x2(e, d)) {
+ sum = UINT64_MAX;
+ goto next;
+ }
- summ += FFABS(lm + rm);
- sums += FFABS(lm - rm);
+ for (int i = 0; i < 4; i++) {
+ if (d[i] != av_clip_intp2(d[i], 15)) {
+ sum = UINT64_MAX;
+ goto next;
+ }
+ }
- suml += lm;
- sumr += rm;
+ for (int j = 0; j <= ctx->cur_restart_interval; j++) {
+ DecodingParams *dp = &s->b[j].decoding_params;
+ const int32_t *ch[2];
- maxl = FFMAX(maxl, lm);
- maxr = FFMAX(maxr, rm);
+ ch[0] = dp->sample_buffer[ch0];
+ ch[1] = dp->sample_buffer[ch1];
- minl = FFMIN(minl, lm);
- minr = FFMIN(minr, rm);
- }
- }
+ for (int i = 0; i < dp->blocksize; i++) {
+ const int64_t lm = ch[0][i], rm = ch[1][i];
+ int64_t lt, rt, v = 0;
- summ -= FFABS(suml + sumr);
- sums -= FFABS(suml - sumr);
+ lt = ((lm * e[0]) >> 14) + ((rm * e[1]) >> 14);
+ rt = ((lm * e[2]) >> 14) + ((rm * e[3]) >> 14);
- ml = maxl - minl;
- mr = maxr - minr;
+ if (FFABS(lt) > (1LL << 23) ||
+ FFABS(rt) > (1LL << 23)) {
+ sum = UINT64_MAX;
+ goto next;
+ }
- if (!summ && !sums)
- return 0;
+ if (c)
+ v += FFABS(rt);
+ else
+ v += FFABS(lt);
+ sum += v;
+ if (sum > best_sum)
+ goto next;
+
+ if ((((lt * d[0]) >> 14) + ((rt * d[1]) >> 14)) != lm) {
+ sum = UINT64_MAX;
+ goto next;
+ }
- if (!ml || !mr)
- return 0;
+ if ((((lt * d[2]) >> 14) + ((rt * d[3]) >> 14)) != rm) {
+ sum = UINT64_MAX;
+ goto next;
+ }
+ }
+ }
- if ((FFABS(ml) + FFABS(mr)) >= (1 << 24))
- return 0;
+next:
+ if (sum < best_sum) {
+ chan = c;
+ best_sum = sum;
+ memcpy(best, e, sizeof(e));
+ }
+ }
- cf0 = (FFMIN(FFABS(mr), FFABS(ml)) * (1LL << 14)) / FFMAX(FFABS(ml), FFABS(mr));
- cf0 = (cf0 >> shift) << shift;
- cf1 = -cf0;
+ v[1] += inc;
+
+ if (v[1] < -search_limit) {
+ if (v[0] > search_limit) {
+ v[0] = -search_step;
+ } else if (v[0] >= 0) {
+ v[0] += search_step;
+ } else if (v[0] >= -search_limit) {
+ v[0] -= search_step;
+ } else {
+ break;
+ }
- if (sums > summ)
- FFSWAP(int32_t, cf0, cf1);
+ inc = search_step;
+ } else if (v[1] > search_limit) {
+ v[1] = 0;
+ inc = -search_step;
+ }
- count = 1;
- i = enl < enr;
- mp->outch[0] = ch0 + i;
+ if (best_sum == 0ULL)
+ break;
+ }
- d[!i] = cf0;
- d[ i] = 1 << 14;
- e[!i] = cf1;
- e[ i] = 1 << 14;
+ if (chan < 0)
+ return 0;
- mp->coeff[0][ch0] = av_clip_intp2(d[0], 15);
- mp->coeff[0][ch1] = av_clip_intp2(d[1], 15);
+ mp->outch[0] = chan;
+ memcpy(e, best, sizeof(e));
+ invert2x2(e, d);
+ count = 1;
- mp->forco[0][ch0] = av_clip_intp2(e[0], 15);
- mp->forco[0][ch1] = av_clip_intp2(e[1], 15);
+ mp->coeff[0][ch0] = d[chan * 2 + 0]; mp->coeff[0][ch1] = d[chan * 2 + 1];
+ mp->forco[0][ch0] = e[chan * 2 + 0]; mp->forco[0][ch1] = e[chan * 2 + 1];
return count;
}
@@ -2060,11 +2132,13 @@ static void set_major_params(MLPEncodeContext *ctx, MLPSubstream *s)
for (int index = 0; index < s->b[ctx->restart_intervals-1].seq_size; index++) {
memcpy(&s->b[index].major_decoding_params,
&s->b[index].decoding_params, sizeof(DecodingParams));
+
for (int ch = 0; ch <= rh->max_matrix_channel; ch++) {
int8_t shift = s->b[index].decoding_params.output_shift[ch];
max_shift = FFMAX(max_shift, shift);
}
+
for (int ch = rh->min_channel; ch <= rh->max_channel; ch++) {
uint8_t huff_lsbs = s->b[index].channel_params[ch].huff_lsbs;
@@ -2277,7 +2351,8 @@ static const AVOption mlp_options[] = {
{ "prediction_order", "Search method for selecting prediction order", OFFSET(prediction_order), AV_OPT_TYPE_INT, {.i64 = ORDER_METHOD_EST }, ORDER_METHOD_EST, ORDER_METHOD_SEARCH, FLAGS, "predm" },
{ "estimation", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = ORDER_METHOD_EST }, 0, 0, FLAGS, "predm" },
{ "search", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = ORDER_METHOD_SEARCH }, 0, 0, FLAGS, "predm" },
-{ "rematrix_precision", "Rematrix coefficient precision", OFFSET(rematrix_precision), AV_OPT_TYPE_INT, {.i64 = 1 }, 0, 14, FLAGS },
+{ "rematrix_limit", "Rematrix search limit precision", OFFSET(rematrix_search_limit), AV_OPT_TYPE_INT, {.i64 = 16 }, 14, 20, FLAGS },
+{ "rematrix_step", "Rematrix search step precision", OFFSET(rematrix_search_step), AV_OPT_TYPE_INT, {.i64 = 10 }, 1, 14, FLAGS },
{ NULL },
};
--
2.42.0
[-- Attachment #4: 0004-avcodec-mlpenc-add-support-for-4.0-4.1-ch-layout.patch --]
[-- Type: text/x-patch, Size: 2794 bytes --]
From 85b0d51d2eb974dc099176e5a8c9fe54022227d2 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Wed, 25 Oct 2023 12:46:24 +0200
Subject: [PATCH 4/4] avcodec/mlpenc: add support for 4.0/4.1 ch layout
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/mlpenc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
index 3a7893b3f0..308dece5a0 100644
--- a/libavcodec/mlpenc.c
+++ b/libavcodec/mlpenc.c
@@ -179,7 +179,7 @@ typedef struct MLPEncodeContext {
uint8_t noise_type;
uint8_t channel_arrangement; ///< channel arrangement for MLP streams
- uint8_t channel_arrangement8; ///< 8 channel arrangement for THD streams
+ uint16_t channel_arrangement8; ///< 8 channel arrangement for THD streams
uint8_t multichannel_type6ch; ///< channel modifier for TrueHD stream 0
uint8_t multichannel_type8ch; ///< channel modifier for TrueHD stream 0
@@ -601,6 +601,8 @@ static av_cold int mlp_encode_init(AVCodecContext *avctx)
case AV_CH_LAYOUT_2POINT1:
case AV_CH_LAYOUT_SURROUND:
case AV_CH_LAYOUT_3POINT1:
+ case AV_CH_LAYOUT_4POINT0:
+ case AV_CH_LAYOUT_4POINT1:
case AV_CH_LAYOUT_5POINT0:
case AV_CH_LAYOUT_5POINT1:
ctx->ch2_presentation_mod= 0;
@@ -2400,13 +2402,15 @@ const FFCodec ff_truehd_encoder = {
.p.priv_class = &mlp_class,
.p.sample_fmts = (const enum AVSampleFormat[]) {AV_SAMPLE_FMT_S16P, AV_SAMPLE_FMT_S32P, AV_SAMPLE_FMT_NONE},
.p.supported_samplerates = (const int[]) {44100, 48000, 88200, 96000, 176400, 192000, 0},
- CODEC_OLD_CHANNEL_LAYOUTS(AV_CH_LAYOUT_MONO, AV_CH_LAYOUT_STEREO, AV_CH_LAYOUT_2POINT1, AV_CH_LAYOUT_SURROUND, AV_CH_LAYOUT_3POINT1, AV_CH_LAYOUT_5POINT0, AV_CH_LAYOUT_5POINT1)
+ CODEC_OLD_CHANNEL_LAYOUTS(AV_CH_LAYOUT_MONO, AV_CH_LAYOUT_STEREO, AV_CH_LAYOUT_2POINT1, AV_CH_LAYOUT_SURROUND, AV_CH_LAYOUT_3POINT1, AV_CH_LAYOUT_4POINT0, AV_CH_LAYOUT_4POINT1, AV_CH_LAYOUT_5POINT0, AV_CH_LAYOUT_5POINT1)
.p.ch_layouts = (const AVChannelLayout[]) {
AV_CHANNEL_LAYOUT_MONO,
AV_CHANNEL_LAYOUT_STEREO,
AV_CHANNEL_LAYOUT_2POINT1,
AV_CHANNEL_LAYOUT_SURROUND,
AV_CHANNEL_LAYOUT_3POINT1,
+ AV_CHANNEL_LAYOUT_4POINT0,
+ AV_CHANNEL_LAYOUT_4POINT1,
AV_CHANNEL_LAYOUT_5POINT0,
AV_CHANNEL_LAYOUT_5POINT1,
{ 0 }
--
2.42.0
[-- Attachment #5: 0002-avcodec-mlpenc-add-3.1-ch-layout-support-for-truehd.patch --]
[-- Type: text/x-patch, Size: 2023 bytes --]
From 310979c0394ab8572b34754ae1436537512c5afd Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Wed, 25 Oct 2023 11:05:35 +0200
Subject: [PATCH 2/4] avcodec/mlpenc: add 3.1 ch layout support for truehd
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/mlpenc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/mlpenc.c b/libavcodec/mlpenc.c
index 27ef5f2c82..3a7893b3f0 100644
--- a/libavcodec/mlpenc.c
+++ b/libavcodec/mlpenc.c
@@ -600,6 +600,7 @@ static av_cold int mlp_encode_init(AVCodecContext *avctx)
break;
case AV_CH_LAYOUT_2POINT1:
case AV_CH_LAYOUT_SURROUND:
+ case AV_CH_LAYOUT_3POINT1:
case AV_CH_LAYOUT_5POINT0:
case AV_CH_LAYOUT_5POINT1:
ctx->ch2_presentation_mod= 0;
@@ -2399,12 +2400,13 @@ const FFCodec ff_truehd_encoder = {
.p.priv_class = &mlp_class,
.p.sample_fmts = (const enum AVSampleFormat[]) {AV_SAMPLE_FMT_S16P, AV_SAMPLE_FMT_S32P, AV_SAMPLE_FMT_NONE},
.p.supported_samplerates = (const int[]) {44100, 48000, 88200, 96000, 176400, 192000, 0},
- CODEC_OLD_CHANNEL_LAYOUTS(AV_CH_LAYOUT_MONO, AV_CH_LAYOUT_STEREO, AV_CH_LAYOUT_2POINT1, AV_CH_LAYOUT_SURROUND, AV_CH_LAYOUT_5POINT0, AV_CH_LAYOUT_5POINT1)
+ CODEC_OLD_CHANNEL_LAYOUTS(AV_CH_LAYOUT_MONO, AV_CH_LAYOUT_STEREO, AV_CH_LAYOUT_2POINT1, AV_CH_LAYOUT_SURROUND, AV_CH_LAYOUT_3POINT1, AV_CH_LAYOUT_5POINT0, AV_CH_LAYOUT_5POINT1)
.p.ch_layouts = (const AVChannelLayout[]) {
AV_CHANNEL_LAYOUT_MONO,
AV_CHANNEL_LAYOUT_STEREO,
AV_CHANNEL_LAYOUT_2POINT1,
AV_CHANNEL_LAYOUT_SURROUND,
+ AV_CHANNEL_LAYOUT_3POINT1,
AV_CHANNEL_LAYOUT_5POINT0,
AV_CHANNEL_LAYOUT_5POINT1,
{ 0 }
--
2.42.0
[-- Attachment #6: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 11:12 [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements Paul B Mahol
@ 2023-10-25 18:39 ` Tomas Härdin
2023-10-25 18:58 ` Paul B Mahol
2023-10-25 19:00 ` Paul B Mahol
0 siblings, 2 replies; 8+ messages in thread
From: Tomas Härdin @ 2023-10-25 18:39 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> if (c) {
> e[0] = 1 << 14;
> e[1] = 0 << 14;
> e[2] = v[1];
> e[3] = v[0];
> } else {
> e[0] = v[0];
> e[1] = v[1];
> e[2] = 0 << 14;
> e[3] = 1 << 14;
> }
>
> if (invert2x2(e, d)) {
> sum = UINT64_MAX;
> goto next;
> }
>
You can make use of the properties of e to simplify calculating the
inverse. The determinant is always v[0]<<14, so you can just do if
(!v[0]) continue; and skip the determinant check altogether.
> if (d[i] != av_clip_intp2(d[i], 15)) {
d[i] < INT16_MIN || d[i] > INT16_MAX is more clear and probably faster
> + lt = ((lm * e[0]) >> 14) + ((rm * e[1]) >> 14);
> + rt = ((lm * e[2]) >> 14) + ((rm * e[3]) >> 14);
Result is implementation-defined. Use division by (1<<14). Also add
then divide. The intermediate result is 49 bits so fits easily in 64
bits.
You could also simplify this calculation by again making use of the
properties of e.
> if (c)
> v += FFABS(rt);
> else
> v += FFABS(lt);
> sum += v;
> if (sum > best_sum)
> goto next;
Seems like this reduces to solving a linear program.
> if ((((lt * d[0]) >> 14) + ((rt * d[1]) >> 14))
> != lm) {
> sum = UINT64_MAX;
> goto next;
> }
>
> if ((((lt * d[2]) >> 14) + ((rt * d[3]) >> 14))
> != rm) {
> sum = UINT64_MAX;
> goto next;
> }
Looks like a massive hack. I'd prefer to formally verify that the
arithmetic works out. Also again you can make use of the properties of
e, or inv(e) as it were.
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 18:39 ` Tomas Härdin
@ 2023-10-25 18:58 ` Paul B Mahol
2023-10-25 19:00 ` Paul B Mahol
1 sibling, 0 replies; 8+ messages in thread
From: Paul B Mahol @ 2023-10-25 18:58 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se> wrote:
>
> > if (c) {
> > e[0] = 1 << 14;
> > e[1] = 0 << 14;
> > e[2] = v[1];
> > e[3] = v[0];
> > } else {
> > e[0] = v[0];
> > e[1] = v[1];
> > e[2] = 0 << 14;
> > e[3] = 1 << 14;
> > }
> >
> > if (invert2x2(e, d)) {
> > sum = UINT64_MAX;
> > goto next;
> > }
> >
>
> You can make use of the properties of e to simplify calculating the
> inverse. The determinant is always v[0]<<14, so you can just do if
> (!v[0]) continue; and skip the determinant check altogether.
>
> > if (d[i] != av_clip_intp2(d[i], 15)) {
>
> d[i] < INT16_MIN || d[i] > INT16_MAX is more clear and probably faster
>
> > + lt = ((lm * e[0]) >> 14) + ((rm * e[1]) >> 14);
> > + rt = ((lm * e[2]) >> 14) + ((rm * e[3]) >> 14);
>
> Result is implementation-defined. Use division by (1<<14). Also add
> then divide. The intermediate result is 49 bits so fits easily in 64
> bits.
>
Division by (1<<14) will give incorrect results. been there done that,
you can check all your "reviews" validity by testing patches and that
results is bitexact, otherwise I'm just wasting time here.
Additions are done before not later, again check your comments validity
before commenting more. Thanks.
> You could also simplify this calculation by again making use of the
> properties of e.
>
> > if (c)
> > v += FFABS(rt);
> > else
> > v += FFABS(lt);
> > sum += v;
> > if (sum > best_sum)
> > goto next;
>
> Seems like this reduces to solving a linear program.
>
> > if ((((lt * d[0]) >> 14) + ((rt * d[1]) >> 14))
> > != lm) {
> > sum = UINT64_MAX;
> > goto next;
> > }
> >
> > if ((((lt * d[2]) >> 14) + ((rt * d[3]) >> 14))
> > != rm) {
> > sum = UINT64_MAX;
> > goto next;
> > }
>
> Looks like a massive hack. I'd prefer to formally verify that the
> arithmetic works out. Also again you can make use of the properties of
> e, or inv(e) as it were.
>
Arithmetic may not always work out.
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 18:39 ` Tomas Härdin
2023-10-25 18:58 ` Paul B Mahol
@ 2023-10-25 19:00 ` Paul B Mahol
2023-10-25 19:03 ` Tomas Härdin
1 sibling, 1 reply; 8+ messages in thread
From: Paul B Mahol @ 2023-10-25 19:00 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se> wrote:
>
> > if (c) {
> > e[0] = 1 << 14;
> > e[1] = 0 << 14;
> > e[2] = v[1];
> > e[3] = v[0];
> > } else {
> > e[0] = v[0];
> > e[1] = v[1];
> > e[2] = 0 << 14;
> > e[3] = 1 << 14;
> > }
> >
> > if (invert2x2(e, d)) {
> > sum = UINT64_MAX;
> > goto next;
> > }
> >
>
> You can make use of the properties of e to simplify calculating the
> inverse. The determinant is always v[0]<<14, so you can just do if
> (!v[0]) continue; and skip the determinant check altogether.
>
Even for real 2x2 matrix case? (Once one of rows is not 1, 0) ?
May added such cases later.
>
> > if (d[i] != av_clip_intp2(d[i], 15)) {
>
> d[i] < INT16_MIN || d[i] > INT16_MAX is more clear and probably faster
>
> > + lt = ((lm * e[0]) >> 14) + ((rm * e[1]) >> 14);
> > + rt = ((lm * e[2]) >> 14) + ((rm * e[3]) >> 14);
>
> Result is implementation-defined. Use division by (1<<14). Also add
> then divide. The intermediate result is 49 bits so fits easily in 64
> bits.
>
> You could also simplify this calculation by again making use of the
> properties of e.
>
> > if (c)
> > v += FFABS(rt);
> > else
> > v += FFABS(lt);
> > sum += v;
> > if (sum > best_sum)
> > goto next;
>
> Seems like this reduces to solving a linear program.
>
> > if ((((lt * d[0]) >> 14) + ((rt * d[1]) >> 14))
> > != lm) {
> > sum = UINT64_MAX;
> > goto next;
> > }
> >
> > if ((((lt * d[2]) >> 14) + ((rt * d[3]) >> 14))
> > != rm) {
> > sum = UINT64_MAX;
> > goto next;
> > }
>
> Looks like a massive hack. I'd prefer to formally verify that the
> arithmetic works out. Also again you can make use of the properties of
> e, or inv(e) as it were.
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 19:00 ` Paul B Mahol
@ 2023-10-25 19:03 ` Tomas Härdin
2023-10-25 19:59 ` Paul B Mahol
0 siblings, 1 reply; 8+ messages in thread
From: Tomas Härdin @ 2023-10-25 19:03 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Wed, 2023-10-25 at 21:00 +0200, Paul B Mahol wrote:
> On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se> wrote:
>
> >
> > > if (c) {
> > > e[0] = 1 << 14;
> > > e[1] = 0 << 14;
> > > e[2] = v[1];
> > > e[3] = v[0];
> > > } else {
> > > e[0] = v[0];
> > > e[1] = v[1];
> > > e[2] = 0 << 14;
> > > e[3] = 1 << 14;
> > > }
> > >
> > > if (invert2x2(e, d)) {
> > > sum = UINT64_MAX;
> > > goto next;
> > > }
> > >
> >
> > You can make use of the properties of e to simplify calculating the
> > inverse. The determinant is always v[0]<<14, so you can just do if
> > (!v[0]) continue; and skip the determinant check altogether.
> >
>
> Even for real 2x2 matrix case? (Once one of rows is not 1, 0) ?
> May added such cases later.
You can just work the math out on paper. Inverse of
1 0
v[1] v[0]
is
1 0
-v[1]/v[0] 1/v[0]
not accounting for shifts.
Also RE: my other comments, you are right. I didn't take into account
that MLP is lossless and that there may be off-by-one errors.
And as I said on IRC you can formulate this as a least squares problem,
then solve it using a linear system solve. This patch seems finds a
solution that minimizes L1 rather than L2 though. Not sure what the
implications of that are compressionwise. What happens if you replace
FFABS() with a square for scoring?
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 19:03 ` Tomas Härdin
@ 2023-10-25 19:59 ` Paul B Mahol
2023-10-30 13:14 ` Tomas Härdin
0 siblings, 1 reply; 8+ messages in thread
From: Paul B Mahol @ 2023-10-25 19:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Wed, Oct 25, 2023 at 9:03 PM Tomas Härdin <git@haerdin.se> wrote:
> On Wed, 2023-10-25 at 21:00 +0200, Paul B Mahol wrote:
> > On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se> wrote:
> >
> > >
> > > > if (c) {
> > > > e[0] = 1 << 14;
> > > > e[1] = 0 << 14;
> > > > e[2] = v[1];
> > > > e[3] = v[0];
> > > > } else {
> > > > e[0] = v[0];
> > > > e[1] = v[1];
> > > > e[2] = 0 << 14;
> > > > e[3] = 1 << 14;
> > > > }
> > > >
> > > > if (invert2x2(e, d)) {
> > > > sum = UINT64_MAX;
> > > > goto next;
> > > > }
> > > >
> > >
> > > You can make use of the properties of e to simplify calculating the
> > > inverse. The determinant is always v[0]<<14, so you can just do if
> > > (!v[0]) continue; and skip the determinant check altogether.
> > >
> >
> > Even for real 2x2 matrix case? (Once one of rows is not 1, 0) ?
> > May added such cases later.
>
> You can just work the math out on paper. Inverse of
>
> 1 0
> v[1] v[0]
>
> is
>
> 1 0
> -v[1]/v[0] 1/v[0]
>
> not accounting for shifts.
>
But I want to add real 2x2 matrix with no 0 cell, with:
a, b
c, d
later. (even though gains are small, as encoded files use it rarely)
>
> Also RE: my other comments, you are right. I didn't take into account
> that MLP is lossless and that there may be off-by-one errors.
>
> And as I said on IRC you can formulate this as a least squares problem,
> then solve it using a linear system solve. This patch seems finds a
> solution that minimizes L1 rather than L2 though. Not sure what the
> implications of that are compressionwise. What happens if you replace
> FFABS() with a square for scoring?
>
It reduces size usually by less then 0.002 %
Linear system solver gives vectors to create equations for both channels at
same time?
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-25 19:59 ` Paul B Mahol
@ 2023-10-30 13:14 ` Tomas Härdin
2023-10-30 13:30 ` Paul B Mahol
0 siblings, 1 reply; 8+ messages in thread
From: Tomas Härdin @ 2023-10-30 13:14 UTC (permalink / raw)
To: FFmpeg development discussions and patches
ons 2023-10-25 klockan 21:59 +0200 skrev Paul B Mahol:
> On Wed, Oct 25, 2023 at 9:03 PM Tomas Härdin <git@haerdin.se> wrote:
>
> > On Wed, 2023-10-25 at 21:00 +0200, Paul B Mahol wrote:
> > > On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se>
> > > wrote:
> > >
> > > >
> > > > > if (c) {
> > > > > e[0] = 1 << 14;
> > > > > e[1] = 0 << 14;
> > > > > e[2] = v[1];
> > > > > e[3] = v[0];
> > > > > } else {
> > > > > e[0] = v[0];
> > > > > e[1] = v[1];
> > > > > e[2] = 0 << 14;
> > > > > e[3] = 1 << 14;
> > > > > }
> > > > >
> > > > > if (invert2x2(e, d)) {
> > > > > sum = UINT64_MAX;
> > > > > goto next;
> > > > > }
> > > > >
> > > >
> > > > You can make use of the properties of e to simplify calculating
> > > > the
> > > > inverse. The determinant is always v[0]<<14, so you can just do
> > > > if
> > > > (!v[0]) continue; and skip the determinant check altogether.
> > > >
> > >
> > > Even for real 2x2 matrix case? (Once one of rows is not 1, 0) ?
> > > May added such cases later.
> >
> > You can just work the math out on paper. Inverse of
> >
> > 1 0
> > v[1] v[0]
> >
> > is
> >
> > 1 0
> > -v[1]/v[0] 1/v[0]
> >
> > not accounting for shifts.
> >
>
> But I want to add real 2x2 matrix with no 0 cell, with:
>
> a, b
> c, d
>
> later. (even though gains are small, as encoded files use it rarely)
If this is possible within MLP then yes, do that. It is not clear from
what you've told me so far and from my brief reading of the code how
capable the format is.
> > Also RE: my other comments, you are right. I didn't take into
> > account
> > that MLP is lossless and that there may be off-by-one errors.
> >
> > And as I said on IRC you can formulate this as a least squares
> > problem,
> > then solve it using a linear system solve. This patch seems finds a
> > solution that minimizes L1 rather than L2 though. Not sure what the
> > implications of that are compressionwise. What happens if you
> > replace
> > FFABS() with a square for scoring?
> >
>
> It reduces size usually by less then 0.002 %
>
> Linear system solver gives vectors to create equations for both
> channels at
> same time?
L2 minimization allows using ordinary least squarse. As I said on IRC,
the rub lies in formulating the problem properly. Minimizing L1 is much
harder, since it involves solving a linear program. Of course for
practical purposes we don't need an exact solution.
Looking a bit more at the code, what is important is the decoding
coefficients, the d matrix. The encoder is free to choose d and the
encoded residuals so long as it decodes correctly. The decoder is
specified on d, not e.
Currently only one matrix is used (count=1 in estimate_coeff). With two
matrices something akin to a lifting scheme can be performed. This
means almost any 2x2 transform should be possible to perform (modulo
bitexactness concerns).
What I mean by lifting scheme here is that any 2x2 matrix A can be
decomposed into the product of two or more matrices on the form that e
has. I think.
We could potentially do something like alternating transforms on this
form:
l += k1*r;
r += k2*l;
l += k3*r;
r += k4*l;
This can always be inverted provided the intermediate results don't go
out of range, or in the event that they do go out of range, the decoder
is sufficiently well specified so that encoder and decoder don't go out
of sync. Compare how YCoCg-R is specified and fits in 3*8 bits. In fact
the WP article on YCoCg perhaps gets the point across better:
https://en.wikipedia.org/wiki/YCoCg
it in turn links this stackoverflow post which makes the same point:
https://stackoverflow.com/questions/10566668/lossless-rgb-to-ycbcr-transformation/12146329#12146329
I believe any transformed found by PCA can be converted into an
equivalent lifting scheme, and it will always be lossless provided
modulo is specified correctly in the codec. I have no idea if it is.
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements
2023-10-30 13:14 ` Tomas Härdin
@ 2023-10-30 13:30 ` Paul B Mahol
0 siblings, 0 replies; 8+ messages in thread
From: Paul B Mahol @ 2023-10-30 13:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, Oct 30, 2023 at 2:15 PM Tomas Härdin <git@haerdin.se> wrote:
> ons 2023-10-25 klockan 21:59 +0200 skrev Paul B Mahol:
> > On Wed, Oct 25, 2023 at 9:03 PM Tomas Härdin <git@haerdin.se> wrote:
> >
> > > On Wed, 2023-10-25 at 21:00 +0200, Paul B Mahol wrote:
> > > > On Wed, Oct 25, 2023 at 8:39 PM Tomas Härdin <git@haerdin.se>
> > > > wrote:
> > > >
> > > > >
> > > > > > if (c) {
> > > > > > e[0] = 1 << 14;
> > > > > > e[1] = 0 << 14;
> > > > > > e[2] = v[1];
> > > > > > e[3] = v[0];
> > > > > > } else {
> > > > > > e[0] = v[0];
> > > > > > e[1] = v[1];
> > > > > > e[2] = 0 << 14;
> > > > > > e[3] = 1 << 14;
> > > > > > }
> > > > > >
> > > > > > if (invert2x2(e, d)) {
> > > > > > sum = UINT64_MAX;
> > > > > > goto next;
> > > > > > }
> > > > > >
> > > > >
> > > > > You can make use of the properties of e to simplify calculating
> > > > > the
> > > > > inverse. The determinant is always v[0]<<14, so you can just do
> > > > > if
> > > > > (!v[0]) continue; and skip the determinant check altogether.
> > > > >
> > > >
> > > > Even for real 2x2 matrix case? (Once one of rows is not 1, 0) ?
> > > > May added such cases later.
> > >
> > > You can just work the math out on paper. Inverse of
> > >
> > > 1 0
> > > v[1] v[0]
> > >
> > > is
> > >
> > > 1 0
> > > -v[1]/v[0] 1/v[0]
> > >
> > > not accounting for shifts.
> > >
> >
> > But I want to add real 2x2 matrix with no 0 cell, with:
> >
> > a, b
> > c, d
> >
> > later. (even though gains are small, as encoded files use it rarely)
>
> If this is possible within MLP then yes, do that. It is not clear from
> what you've told me so far and from my brief reading of the code how
> capable the format is.
>
> > > Also RE: my other comments, you are right. I didn't take into
> > > account
> > > that MLP is lossless and that there may be off-by-one errors.
> > >
> > > And as I said on IRC you can formulate this as a least squares
> > > problem,
> > > then solve it using a linear system solve. This patch seems finds a
> > > solution that minimizes L1 rather than L2 though. Not sure what the
> > > implications of that are compressionwise. What happens if you
> > > replace
> > > FFABS() with a square for scoring?
> > >
> >
> > It reduces size usually by less then 0.002 %
> >
> > Linear system solver gives vectors to create equations for both
> > channels at
> > same time?
>
> L2 minimization allows using ordinary least squarse. As I said on IRC,
> the rub lies in formulating the problem properly. Minimizing L1 is much
> harder, since it involves solving a linear program. Of course for
> practical purposes we don't need an exact solution.
>
> Looking a bit more at the code, what is important is the decoding
> coefficients, the d matrix. The encoder is free to choose d and the
> encoded residuals so long as it decodes correctly. The decoder is
> specified on d, not e.
>
> Currently only one matrix is used (count=1 in estimate_coeff). With two
> matrices something akin to a lifting scheme can be performed. This
> means almost any 2x2 transform should be possible to perform (modulo
> bitexactness concerns).
>
> What I mean by lifting scheme here is that any 2x2 matrix A can be
> decomposed into the product of two or more matrices on the form that e
> has. I think.
>
> We could potentially do something like alternating transforms on this
> form:
>
> l += k1*r;
> r += k2*l;
> l += k3*r;
> r += k4*l;
>
> This can always be inverted provided the intermediate results don't go
> out of range, or in the event that they do go out of range, the decoder
> is sufficiently well specified so that encoder and decoder don't go out
> of sync. Compare how YCoCg-R is specified and fits in 3*8 bits. In fact
> the WP article on YCoCg perhaps gets the point across better:
> https://en.wikipedia.org/wiki/YCoCg
> it in turn links this stackoverflow post which makes the same point:
>
> https://stackoverflow.com/questions/10566668/lossless-rgb-to-ycbcr-transformation/12146329#12146329
>
> I believe any transformed found by PCA can be converted into an
> equivalent lifting scheme, and it will always be lossless provided
> modulo is specified correctly in the codec. I have no idea if it is.
>
L = k1 * l + k2 * r
R = L * k3 + r * k4
This is affine transform for 2x2 matrix case, and here typical PCA or
lifting fails.
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-30 13:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-25 11:12 [FFmpeg-devel] [PATCH] avcodec/mlp*: improvements Paul B Mahol
2023-10-25 18:39 ` Tomas Härdin
2023-10-25 18:58 ` Paul B Mahol
2023-10-25 19:00 ` Paul B Mahol
2023-10-25 19:03 ` Tomas Härdin
2023-10-25 19:59 ` Paul B Mahol
2023-10-30 13:14 ` Tomas Härdin
2023-10-30 13:30 ` Paul B Mahol
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git