[FFmpeg-devel] [libav-devel] [PATCH] Add ATRAC3+ decoder
Vitor Sessak
vitor1001 at gmail.com
Fri Oct 11 21:18:44 CEST 2013
On Thu, Oct 10, 2013 at 9:14 PM, Maxim Polijakowski <max_pole at gmx.de> wrote:
> Hi crews,
>
> the attached patch adds an open-source decoder for Sony's ATRAC3+ format.
> There is a partial description of its internals located here:
> http://wiki.multimedia.cx/**index.php?title=ATRAC3plus<http://wiki.multimedia.cx/index.php?title=ATRAC3plus>
>
>
Nice! I have a few comments:
+av_cold void ff_atrac3p_init_vlcs(ATRAC3PVlcTabs *vlc_tabs)
+{
+ int i;
+
+ ff_init_vlc_sparse(&vlc_tabs->wl_vlc_tabs[0], 2, 3,
+ ff_atrac3p_wl_huff_bits1, 1, 1,
+ ff_atrac3p_wl_huff_code1, 1, 1,
+ ff_atrac3p_wl_huff_xlat1, 1, 1, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->wl_vlc_tabs[1], 3, 5,
+ ff_atrac3p_wl_huff_bits2, 1, 1,
+ ff_atrac3p_wl_huff_code2, 1, 1,
+ ff_atrac3p_wl_huff_xlat2, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->wl_vlc_tabs[2], 5, 8,
+ ff_atrac3p_wl_huff_bits3, 1, 1,
+ ff_atrac3p_wl_huff_code3, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->wl_vlc_tabs[3], 5, 8,
+ ff_atrac3p_wl_huff_bits4, 1, 1,
+ ff_atrac3p_wl_huff_code4, 1, 1, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->sf_vlc_tabs[0], 9, 64,
+ ff_atrac3p_sf_huff_bits1, 1, 1,
+ ff_atrac3p_sf_huff_code1, 2, 2,
+ ff_atrac3p_sf_huff_xlat1, 1, 1, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->sf_vlc_tabs[1], 9, 64,
+ ff_atrac3p_sf_huff_bits1, 1, 1,
+ ff_atrac3p_sf_huff_code1, 2, 2,
+ ff_atrac3p_sf_huff_xlat2, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->sf_vlc_tabs[2], 9, 64,
+ ff_atrac3p_sf_huff_bits2, 1, 1,
+ ff_atrac3p_sf_huff_code2, 2, 2, 0);
+
+ init_vlc(&vlc_tabs->sf_vlc_tabs[3], 9, 64,
+ ff_atrac3p_sf_huff_bits3, 1, 1,
+ ff_atrac3p_sf_huff_code3, 2, 2, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->sf_vlc_tabs[4], 6, 16,
+ ff_atrac3p_sf_huff_bits4, 1, 1,
+ ff_atrac3p_sf_huff_code4, 1, 1,
+ ff_atrac3p_sf_huff_xlat4, 1, 1, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->sf_vlc_tabs[5], 6, 16,
+ ff_atrac3p_sf_huff_bits4, 1, 1,
+ ff_atrac3p_sf_huff_code4, 1, 1,
+ ff_atrac3p_sf_huff_xlat5, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->sf_vlc_tabs[6], 7, 16,
+ ff_atrac3p_sf_huff_bits5, 1, 1,
+ ff_atrac3p_sf_huff_code5, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->sf_vlc_tabs[7], 7, 16,
+ ff_atrac3p_sf_huff_bits6, 1, 1,
+ ff_atrac3p_sf_huff_code6, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->ct_vlc_tabs[0], 3, 4,
+ ff_atrac3p_ct_huff_bits1, 1, 1,
+ ff_atrac3p_ct_huff_code1, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->ct_vlc_tabs[1], 4, 8,
+ ff_atrac3p_ct_huff_bits2, 1, 1,
+ ff_atrac3p_ct_huff_code2, 1, 1, 0);
+
+ ff_init_vlc_sparse(&vlc_tabs->ct_vlc_tabs[2], 4, 8,
+ ff_atrac3p_ct_huff_bits2, 1, 1,
+ ff_atrac3p_ct_huff_code2, 1, 1,
+ ff_atrac3p_ct_huff_xlat1, 1, 1, 0);
+
+ init_vlc(&vlc_tabs->ct_vlc_tabs[3], 4, 8,
+ ff_atrac3p_ct_huff_bits3, 1, 1,
+ ff_atrac3p_ct_huff_code3, 1, 1, 0);
You could define:
const uint8_t *ff_atrac3p_sf_huff_bits[] = {atrac3p_sf_huff_bits1,
atrac3p_sf_huff_bits2, atrac3p_sf_huff_bits1, ...};
And use one (or three) loops to init it (also valid for the ones
initialized with build_canonical_huff).
+static void subtract_sf_weights(Atrac3pChanUnitCtx *ctx,
+ Atrac3pChanParams *chan, int wtab_idx)
+{
+ int i;
+ const int8_t *weights_tab;
+
+ weights_tab = &ff_atrac3p_sf_weights[wtab_idx - 1][0];
+
+ for (i = 0; i < ctx->used_quant_units; i++)
+ chan->qu_sf_idx[i] -= weights_tab[i];
+}
Please move this function right after add_wordlen_weights(). Even if I
understand the reason for the code duplication, I think grouping them
together improves readability.
There is a pretty large number of decode_* functions with four different
coding modes, some more or less similar. In the cases where applicable, It
would be nice to have some comments like:
/**
* This function almost identical to decode_something_else(), but VLC delta
coding use
* different coefficients.
*/
+/**
+ * ATRAC3+ uses two different MDCT windows:
+ * - The first one is just the plain sine window of size 256.
+ * - The 2nd one is the plain sine window of size 128
+ * wrapped into zero (at the start) and one (at the end) regions.
+ * Both regions are 32 samples long. */
+static float mdct_wind_steep[128]; ///< second MDCT window
+
+av_cold void ff_atrac3p_init_imdct(AVCodecContext *avctx, FFTContext
*mdct_ctx)
+{
+ int i;
+
+ avpriv_float_dsp_init(&atrac3p_dsp, avctx->flags &
CODEC_FLAG_BITEXACT);
+
+ ff_init_ff_sine_windows(7);
+ ff_init_ff_sine_windows(6);
+
+ /* Copy the 2nd sine window and place it between one/zero regions. */
+ memcpy(&mdct_wind_steep[32], ff_sine_64, sizeof(ff_sine_64));
+
+ for (i = 0; i < 32; i++) {
+ mdct_wind_steep[i] = 0.0f;
+ mdct_wind_steep[127 - i] = 1.0f;
+ }
I think you could use ff_sine_64 directly if you modify ff_atrac3p_imdct()
doing something like
atrac3p_dsp.vector_fmul(pOut+64, pOut+64, ff_sine_64, 64);
for the steep window case.
+
+ /* generate amplitude mantissas table */
+ for (i = 0; i < 16; i++)
+ amp_mant_tab[i] = (1.0f / 15.13f) * (i + 1);
+}
+
I think this can be done efficiently without a table.
+ /* Hann windowing for non-faded wave signals */
+ if (tones_now->num_wavs && tones_next->num_wavs &&
+ reg1_env_nonzero && reg2_env_nonzero) {
+ for (i = 0; i < 128; i++) {
+ wavreg1[i] *= hann_window[128 - i];
+ wavreg2[i] *= hann_window[i];
+ }
+ } else {
+ if (tones_now->num_wavs && !tones_now->curr_env.has_stop_point)
+ for (i = 0; i < 128; i++)
+ wavreg1[i] *= hann_window[128 - i];
+
+ if (tones_next->num_wavs && !tones_next->curr_env.has_start_point)
+ for (i = 0; i < 128; i++)
+ wavreg2[i] *= hann_window[i];
+ }
+
+ /* Overlap and add to residual */
+ for (i = 0; i < 128; i++)
+ out[i] += wavreg1[i] + wavreg2[i];
+}
I think we have DSP functions for multiplying and adding floats.
+void ff_atrac3p_ipqf(FFTContext *dct_ctx, Atrac3pIPQFChannelCtx *hist,
+ float *in, float *out)
+{
+ int i, s, sb, t, pos_now, pos_next;
+ DECLARE_ALIGNED(32, float, idct_in)[ATRAC3P_SUBBANDS];
+ DECLARE_ALIGNED(32, float, idct_out)[ATRAC3P_SUBBANDS];
+
+ memset(out, 0, ATRAC3P_FRAME_SAMPLES * sizeof(*out));
+
+ for (s = 0; s < ATRAC3P_SUBBAND_SAMPLES; s++) {
+ /* pick up one sample from each subband */
+ for (sb = 0; sb < ATRAC3P_SUBBANDS; sb++)
+ idct_in[sb] = in[sb * ATRAC3P_SUBBAND_SAMPLES + s];
+
+ /* Calculate the sine and cosine part of the PQF using IDCT-IV */
+ dct_ctx->imdct_half(dct_ctx, idct_out, idct_in);
IDCT-IV or IMDCT?
Finally, I'll try to find time to look at the rest of the patch better.
-Vitor
More information about the ffmpeg-devel
mailing list