[FFmpeg-devel] [PATCH] Add ATRAC3+ decoder

Michael Niedermayer michaelni at gmx.at
Mon Oct 14 22:52:25 CEST 2013


On Mon, Oct 14, 2013 at 10:28:59PM +0200, Maxim Polijakowski wrote:
> Am 14.10.2013 22:09, schrieb Michael Niedermayer:
> >On Mon, Oct 14, 2013 at 08:52:04PM +0200, Maxim Polijakowski wrote:
> >>>>+    const uint8_t *buf  = avpkt->data;
> >>>>+    int buf_size        = avpkt->size;
> >>>>+    ATRAC3PContext *ctx = avctx->priv_data;
> >>>>+    AVFrame *frame      = data;
> >>>>+    int i, ret, ch_unit_id, ch_block = 0, out_ch_index = 0, channels_to_process;
> >>>>+    float **samples_p = (float **)frame->extended_data;
> >>>>+
> >>>>+    frame->nb_samples = ATRAC3P_FRAME_SAMPLES;
> >>>>+    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) {
> >>>>+        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> >>>>+        return ret;
> >>>>+    }
> >>>>+
> >>>>+    init_get_bits(&ctx->gb, buf, buf_size * 8);
> >>>if ((ret = init_get_bits8(&ctx->gb, avpkt->data, avpkt->size)) < 0)
> >>>     return ret;
> >>Done.
> >>
> >>>[...]
> >>>
> >>>>+
> >>>>+/**
> >>>>+ *  @file
> >>>>+ *  DSP functions for ATRAC3+ decoder.
> >>>>+ */
> >>>>+
> >>>>+#include <math.h>
> >>>>+
> >>>>+#include "libavutil/float_dsp.h"
> >>>>+#include "avcodec.h"
> >>>>+#include "sinewin.h"
> >>>>+#include "fft.h"
> >>>>+#include "atrac3plus.h"
> >>>>+#include "atrac3plus_data.h"
> >>>>+
> >>>>+#define ATRAC3P_MDCT_SIZE (ATRAC3P_SUBBAND_SAMPLES * 2)
> >>>>+
> >>>>+static AVFloatDSPContext atrac3p_dsp;
> >>>This looks strange and is probably incorrect, it probably should be in
> >>>private context.
> >>>
> >>What is wrong with that?
> >Initialization uses a local per instance flag
> >(avctx->flags & CODEC_FLAG_BITEXACT)
> >thus if there are 2 instances, one with the flag set and one not set
> >a single global context could only be corrct for both if the flag
> >has no effect and would be unused
> >so something in this looks wrong, either it shouldt be global or there
> >should be no local flag passed to it.
> 
> Will the following code:
> 
> avpriv_float_dsp_init(&atrac3p_dsp, CODEC_FLAG_BITEXACT);
> 
> work correctly for me?

it should be ok in principle but see below


> 
> Unfortunately, the purpose of those avctx->flags is completely
> opaque to me...

the bitexact flag (which is the only used here) is used for regression
tests.
Its meaning is basically that functions that dont give sufficiently
similar output would not be enabled when bitexact is set.
That is if bitexact is set the output on all platforms is sufficiently
similar that all the regression tests pass within the needed
+-1 / psnr / or binary identical checks

for integer functions bitexat genarlly means truly identical output
for float its more "sufficiently similar" because float is simply not
exact between platforms

This also means that with bitexact set there may be only a subset of
optimizations enabled

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131014/c260f9dc/attachment.asc>


More information about the ffmpeg-devel mailing list