Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.

2014-03-25 Thread Martin Storsjö 

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.

2014-03-20 Thread Diego Biurrun 
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.

2014-03-20 Thread Ben Avison 
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_pack_output;
+}
+
+int32_t ff_mlp_pack_output(int32_t lossless_check_data,
+

Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.

2014-03-20 Thread Ben Avison 

On Thu, 20 Mar 2014 11:38:28 -, Diego Biurrun di...@biurrun.de 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.

2014-03-19 Thread Ben Avison 
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_pack_output;
+}
+
+int32_t ff_mlp_pack_output(int32_t lossless_check_data,
+

Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.

2014-03-19 Thread Diego Biurrun 
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

Re: [libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.

2014-03-19 Thread James Almer 
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 stdint.h
 +#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.

2014-03-19 Thread Ben Avison 

--- 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

[libav-devel] [PATCH 5/6] truehd: break out part of output_data into platform-specific callback.

2014-03-19 Thread Ben Avison 
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_pack_output;
+}
+
+int32_t ff_mlp_pack_output(int32_t lossless_check_data,
+