Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
On Thu, 20 Mar 2014, Ben Avison wrote: Verified with profiling that this doesn't have a measurable effect upon overall performance. --- libavcodec/mlpdec.c | 40 +++- libavcodec/mlpdsp.c | 38 ++ libavcodec/mlpdsp.h | 22 ++ 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c index b9d1704..49353d9 100644 --- a/libavcodec/mlpdec.c +++ b/libavcodec/mlpdec.c @@ -360,6 +360,10 @@ static int read_major_sync(MLPDecodeContext *m, GetBitContext *gb) m->avctx->sample_fmt = AV_SAMPLE_FMT_S32; else m->avctx->sample_fmt = AV_SAMPLE_FMT_S16; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(m->substream[m->max_decoded_substream].ch_assign, + m->substream[m->max_decoded_substream].output_shift, + m->substream[m->max_decoded_substream].max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); I'm not sure if this is the right way to do this, or if the mlp_packet_output function pointer should be in MLPDecodeContext instead? Practically it probably doesn't matter though. // Martin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
On Thu, 20 Mar 2014 11:38:28 -, Diego Biurrun wrote: On Wed, Mar 19, 2014 at 07:43:49PM -, Ben Avison wrote: >>--- a/libavcodec/mlpdsp.c >>+++ b/libavcodec/mlpdsp.c >>@@ -89,10 +89,46 @@ void ff_mlp_rematrix_channel(int32_t *samples, >>+int32_t *data_32 = (int32_t *)data; >>+int16_t *data_16 = (int16_t *)data; >pointless void* casts Fair enough, those were cut-and-pastes from their original location. Which location? I'll change them .. Those (and the lines a bit further down the same function where you objected to the formatting) were moved from rematrix_channels() in mlpdec.c. My patch series already deletes the offenders from their original position. :) Ben ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
Verified with profiling that this doesn't have a measurable effect upon overall performance. --- libavcodec/mlpdec.c | 40 +++- libavcodec/mlpdsp.c | 38 ++ libavcodec/mlpdsp.h | 22 ++ 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c index b9d1704..49353d9 100644 --- a/libavcodec/mlpdec.c +++ b/libavcodec/mlpdec.c @@ -360,6 +360,10 @@ static int read_major_sync(MLPDecodeContext *m, GetBitContext *gb) m->avctx->sample_fmt = AV_SAMPLE_FMT_S32; else m->avctx->sample_fmt = AV_SAMPLE_FMT_S16; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(m->substream[m->max_decoded_substream].ch_assign, + m->substream[m->max_decoded_substream].output_shift, + m->substream[m->max_decoded_substream].max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); m->params_valid = 1; for (substr = 0; substr < MAX_SUBSTREAMS; substr++) @@ -588,6 +592,10 @@ FF_ENABLE_DEPRECATION_WARNINGS if (substr == m->max_decoded_substream) { m->avctx->channels = s->max_matrix_channel + 1; m->avctx->channel_layout = s->ch_layout; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->ch_assign, + s->output_shift, + s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); } return 0; @@ -818,9 +826,15 @@ static int read_decoding_params(MLPDecodeContext *m, GetBitContext *gbp, return ret; if (s->param_presence_flags & PARAM_OUTSHIFT) -if (get_bits1(gbp)) +if (get_bits1(gbp)) { for (ch = 0; ch <= s->max_matrix_channel; ch++) s->output_shift[ch] = get_sbits(gbp, 4); +if (substr == m->max_decoded_substream) +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->ch_assign, + s->output_shift, + s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); +} if (s->param_presence_flags & PARAM_QUANTSTEP) if (get_bits1(gbp)) @@ -1019,9 +1033,6 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, { AVCodecContext *avctx = m->avctx; SubStream *s = &m->substream[substr]; -unsigned int i, out_ch = 0; -int32_t *data_32; -int16_t *data_16; int ret; int is32 = (m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); @@ -1041,19 +1052,14 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n"); return ret; } -data_32 = (int32_t *)frame->data[0]; -data_16 = (int16_t *)frame->data[0]; - -for (i = 0; i < s->blockpos; i++) { -for (out_ch = 0; out_ch <= s->max_matrix_channel; out_ch++) { -int mat_ch = s->ch_assign[out_ch]; -int32_t sample = m->sample_buffer[i][mat_ch] - << s->output_shift[mat_ch]; -s->lossless_check_data ^= (sample & 0xff) << mat_ch; -if (is32) *data_32++ = sample << 8; -else *data_16++ = sample >> 8; -} -} +s->lossless_check_data = m->dsp.mlp_pack_output(s->lossless_check_data, +s->blockpos, +m->sample_buffer, +frame->data[0], +s->ch_assign, +s->output_shift, +s->max_matrix_channel, +is32); /* Update matrix encoding side data */ if ((ret = ff_side_data_update_matrix_encoding(frame, s->matrix_encoding)) < 0) diff --git a/libavcodec/mlpdsp.c b/libavcodec/mlpdsp.c index dfa13af..aded554 100644 --- a/libavcodec/mlpdsp.c +++ b/libavcodec/mlpdsp.c @@ -89,10 +89,48 @@ void ff_mlp_rematrix_channel(int32_t *samples, } } +static int32_t (*mlp_select_pack_output(uint8_t *ch_assign, +int8_t *output_shift, +uint8_t max_matrix_channel, +int is32))(int32_t, uint16_t, int32_t (*)[], void *, uint8_t*, int8_t *, uint8_t, int) +{ +return ff_mlp_p
Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
On Wed, Mar 19, 2014 at 07:43:49PM -, Ben Avison wrote: > >>--- a/libavcodec/mlpdsp.c > >>+++ b/libavcodec/mlpdsp.c > >>@@ -89,10 +89,46 @@ void ff_mlp_rematrix_channel(int32_t *samples, > >>+int32_t *data_32 = (int32_t *)data; > >>+int16_t *data_16 = (int16_t *)data; > >pointless void* casts > > Fair enough, those were cut-and-pastes from their original location. Which location? I'll change them .. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
Verified with profiling that this doesn't have a measurable effect upon overall performance. --- libavcodec/mlpdec.c | 40 +++- libavcodec/mlpdsp.c | 38 ++ libavcodec/mlpdsp.h | 22 ++ 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c index b9d1704..49353d9 100644 --- a/libavcodec/mlpdec.c +++ b/libavcodec/mlpdec.c @@ -360,6 +360,10 @@ static int read_major_sync(MLPDecodeContext *m, GetBitContext *gb) m->avctx->sample_fmt = AV_SAMPLE_FMT_S32; else m->avctx->sample_fmt = AV_SAMPLE_FMT_S16; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(m->substream[m->max_decoded_substream].ch_assign, + m->substream[m->max_decoded_substream].output_shift, + m->substream[m->max_decoded_substream].max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); m->params_valid = 1; for (substr = 0; substr < MAX_SUBSTREAMS; substr++) @@ -588,6 +592,10 @@ FF_ENABLE_DEPRECATION_WARNINGS if (substr == m->max_decoded_substream) { m->avctx->channels = s->max_matrix_channel + 1; m->avctx->channel_layout = s->ch_layout; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->ch_assign, + s->output_shift, + s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); } return 0; @@ -818,9 +826,15 @@ static int read_decoding_params(MLPDecodeContext *m, GetBitContext *gbp, return ret; if (s->param_presence_flags & PARAM_OUTSHIFT) -if (get_bits1(gbp)) +if (get_bits1(gbp)) { for (ch = 0; ch <= s->max_matrix_channel; ch++) s->output_shift[ch] = get_sbits(gbp, 4); +if (substr == m->max_decoded_substream) +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->ch_assign, + s->output_shift, + s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); +} if (s->param_presence_flags & PARAM_QUANTSTEP) if (get_bits1(gbp)) @@ -1019,9 +1033,6 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, { AVCodecContext *avctx = m->avctx; SubStream *s = &m->substream[substr]; -unsigned int i, out_ch = 0; -int32_t *data_32; -int16_t *data_16; int ret; int is32 = (m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); @@ -1041,19 +1052,14 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n"); return ret; } -data_32 = (int32_t *)frame->data[0]; -data_16 = (int16_t *)frame->data[0]; - -for (i = 0; i < s->blockpos; i++) { -for (out_ch = 0; out_ch <= s->max_matrix_channel; out_ch++) { -int mat_ch = s->ch_assign[out_ch]; -int32_t sample = m->sample_buffer[i][mat_ch] - << s->output_shift[mat_ch]; -s->lossless_check_data ^= (sample & 0xff) << mat_ch; -if (is32) *data_32++ = sample << 8; -else *data_16++ = sample >> 8; -} -} +s->lossless_check_data = m->dsp.mlp_pack_output(s->lossless_check_data, +s->blockpos, +m->sample_buffer, +frame->data[0], +s->ch_assign, +s->output_shift, +s->max_matrix_channel, +is32); /* Update matrix encoding side data */ if ((ret = ff_side_data_update_matrix_encoding(frame, s->matrix_encoding)) < 0) diff --git a/libavcodec/mlpdsp.c b/libavcodec/mlpdsp.c index dfa13af..aded554 100644 --- a/libavcodec/mlpdsp.c +++ b/libavcodec/mlpdsp.c @@ -89,10 +89,48 @@ void ff_mlp_rematrix_channel(int32_t *samples, } } +static int32_t (*mlp_select_pack_output(uint8_t *ch_assign, +int8_t *output_shift, +uint8_t max_matrix_channel, +int is32))(int32_t, uint16_t, int32_t (*)[], void *, uint8_t*, int8_t *, uint8_t, int) +{ +return ff_mlp_p
Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
--- a/libavcodec/mlpdsp.c +++ b/libavcodec/mlpdsp.c @@ -89,10 +89,46 @@ void ff_mlp_rematrix_channel(int32_t *samples, + +int32_t ff_mlp_pack_output(int32_t lossless_check_data, This function is not used outside of the file, so it can be made static and the ff_ prefix can be removed. It's used from several places in arm/mlpdsp_init_arm.c. +int32_t *data_32 = (int32_t *)data; +int16_t *data_16 = (int16_t *)data; pointless void* casts Fair enough, those were cut-and-pastes from their original location. +lossless_check_data ^= (sample & 0xff) << mat_ch; +if (is32) *data_32++ = sample << 8; +else *data_16++ = sample >> 8; Please break the lines. OK, can do. Ben ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
On 19/03/14 2:24 PM, Ben Avison wrote: > diff --git a/libavcodec/mlpdsp.h b/libavcodec/mlpdsp.h > index bd864d9..7b7640e 100644 > --- a/libavcodec/mlpdsp.h > +++ b/libavcodec/mlpdsp.h > @@ -23,6 +23,7 @@ > #define AVCODEC_MLPDSP_H > > #include > +#include "mlp.h" > > void ff_mlp_rematrix_channel(int32_t *samples, > const int32_t *coeffs, > @@ -36,6 +37,15 @@ void ff_mlp_rematrix_channel(int32_t *samples, > int access_unit_size_pow2, > int32_t mask); > > +int32_t ff_mlp_pack_output(int32_t lossless_check_data, > + int32_t (*sample_buffer)[MAX_CHANNELS], > + void *data, > + uint16_t blockpos, > + uint8_t max_matrix_channel, > + int is32, > + uint8_t *ch_assign, > + int8_t *output_shift); > + > typedef struct MLPDSPContext { > void (*mlp_filter_channel)(int32_t *state, const int32_t *coeff, > int firorder, int iirorder, > @@ -52,6 +62,18 @@ typedef struct MLPDSPContext { > int matrix_noise_shift, > int access_unit_size_pow2, > int32_t mask); > +int32_t (*(*mlp_select_pack_output)(uint8_t max_matrix_channel, > +int is32, > +uint8_t *ch_assign, > +int8_t *output_shift))(int32_t, > int32_t (*)[], void *, uint16_t, uint8_t, int, uint8_t*, int8_t *); > +int32_t (*mlp_pack_output)(int32_t lossless_check_data, > + int32_t (*sample_buffer)[MAX_CHANNELS], > + void *data, > + uint16_t blockpos, > + uint8_t max_matrix_channel, > + int is32, > + uint8_t *ch_assign, > + int8_t *output_shift); > } MLPDSPContext; > > void ff_mlpdsp_init(MLPDSPContext *c); > Please put pointers first if possible, like you did for mlp_rematrix_channel. Something like +int32_t (*mlp_pack_output)(int32_t (*sample_buffer)[MAX_CHANNELS], + void *data, + uint8_t *ch_assign, + int8_t *output_shift, + int32_t lossless_check_data, + uint16_t blockpos, + uint8_t max_matrix_channel, + int is32); ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
On Wed, Mar 19, 2014 at 05:24:26PM +, Ben Avison wrote: > --- a/libavcodec/mlpdsp.c > +++ b/libavcodec/mlpdsp.c > @@ -89,10 +89,46 @@ void ff_mlp_rematrix_channel(int32_t *samples, > + > +int32_t ff_mlp_pack_output(int32_t lossless_check_data, This function is not used outside of the file, so it can be made static and the ff_ prefix can be removed. > + int32_t (*sample_buffer)[MAX_CHANNELS], > + void *data, > + uint16_t blockpos, > + uint8_t max_matrix_channel, > + int is32, > + uint8_t *ch_assign, > + int8_t *output_shift) > +{ > +unsigned int i, out_ch = 0; > +int32_t *data_32 = (int32_t *)data; > +int16_t *data_16 = (int16_t *)data; pointless void* casts > +lossless_check_data ^= (sample & 0xff) << mat_ch; > +if (is32) *data_32++ = sample << 8; > +else *data_16++ = sample >> 8; Please break the lines. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.
Verified with profiling that this doesn't have a measurable effect upon overall performance. --- libavcodec/mlpdec.c | 40 +++- libavcodec/mlpdsp.c | 36 libavcodec/mlpdsp.h | 22 ++ 3 files changed, 81 insertions(+), 17 deletions(-) diff --git a/libavcodec/mlpdec.c b/libavcodec/mlpdec.c index b9d1704..9dde60e 100644 --- a/libavcodec/mlpdec.c +++ b/libavcodec/mlpdec.c @@ -360,6 +360,10 @@ static int read_major_sync(MLPDecodeContext *m, GetBitContext *gb) m->avctx->sample_fmt = AV_SAMPLE_FMT_S32; else m->avctx->sample_fmt = AV_SAMPLE_FMT_S16; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(m->substream[m->max_decoded_substream].max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32, + m->substream[m->max_decoded_substream].ch_assign, + m->substream[m->max_decoded_substream].output_shift); m->params_valid = 1; for (substr = 0; substr < MAX_SUBSTREAMS; substr++) @@ -588,6 +592,10 @@ FF_ENABLE_DEPRECATION_WARNINGS if (substr == m->max_decoded_substream) { m->avctx->channels = s->max_matrix_channel + 1; m->avctx->channel_layout = s->ch_layout; +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32, + s->ch_assign, + s->output_shift); } return 0; @@ -818,9 +826,15 @@ static int read_decoding_params(MLPDecodeContext *m, GetBitContext *gbp, return ret; if (s->param_presence_flags & PARAM_OUTSHIFT) -if (get_bits1(gbp)) +if (get_bits1(gbp)) { for (ch = 0; ch <= s->max_matrix_channel; ch++) s->output_shift[ch] = get_sbits(gbp, 4); +if (substr == m->max_decoded_substream) +m->dsp.mlp_pack_output = m->dsp.mlp_select_pack_output(s->max_matrix_channel, + m->avctx->sample_fmt == AV_SAMPLE_FMT_S32, + s->ch_assign, + s->output_shift); +} if (s->param_presence_flags & PARAM_QUANTSTEP) if (get_bits1(gbp)) @@ -1019,9 +1033,6 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, { AVCodecContext *avctx = m->avctx; SubStream *s = &m->substream[substr]; -unsigned int i, out_ch = 0; -int32_t *data_32; -int16_t *data_16; int ret; int is32 = (m->avctx->sample_fmt == AV_SAMPLE_FMT_S32); @@ -1041,19 +1052,14 @@ static int output_data(MLPDecodeContext *m, unsigned int substr, av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n"); return ret; } -data_32 = (int32_t *)frame->data[0]; -data_16 = (int16_t *)frame->data[0]; - -for (i = 0; i < s->blockpos; i++) { -for (out_ch = 0; out_ch <= s->max_matrix_channel; out_ch++) { -int mat_ch = s->ch_assign[out_ch]; -int32_t sample = m->sample_buffer[i][mat_ch] - << s->output_shift[mat_ch]; -s->lossless_check_data ^= (sample & 0xff) << mat_ch; -if (is32) *data_32++ = sample << 8; -else *data_16++ = sample >> 8; -} -} +s->lossless_check_data = m->dsp.mlp_pack_output(s->lossless_check_data, +m->sample_buffer, +frame->data[0], +s->blockpos, +s->max_matrix_channel, +is32, +s->ch_assign, +s->output_shift); /* Update matrix encoding side data */ if ((ret = ff_side_data_update_matrix_encoding(frame, s->matrix_encoding)) < 0) diff --git a/libavcodec/mlpdsp.c b/libavcodec/mlpdsp.c index dfa13af..b3b5ffd 100644 --- a/libavcodec/mlpdsp.c +++ b/libavcodec/mlpdsp.c @@ -89,10 +89,46 @@ void ff_mlp_rematrix_channel(int32_t *samples, } } +static int32_t (*mlp_select_pack_output(uint8_t max_matrix_channel, +int is32, +uint8_t *ch_assign, +int8_t *output_shift))(int32_t, int32_t (*)[], void *, uint16_t, uint8_t, int, uint8_t*, int8_t *) +{ +return ff_mlp_pa