[FFmpeg-devel] [PATCH] Bink file demuxer and audio decoder

[Daniel Verkamp]

On Wed, Jan 14, 2009 at 5:03 AM, Diego Biurrun <diego at biurrun.de> wrote:
>> - ATI/* has various chunks of files; appending the
>> --append.this.to.complete.bik to the end of the un-suffixed file
>> produces the full video, which works;
>
> IOW, these should be merged back together, right?

Yes, i.e. 'cat ATI-9700-Animusic-Movie-v1.0.bik
ATI-9700-Animusic-Movie-v1.0.bik--append.this.to.complete.bik >
complete.bik'  - this should result in a file with md5sum
f36b8b7fb3ee45f33e9a0a158845b085.

>> the .x file is just another truncated part of the video
>
> Truncated part of which video exactly?

The same as the other files in the dir; if the complete movie is
concatenated as above, this file (and the other partial files) can
just be removed.  I assume they are just a result of uploads that
failed part of the way through...

>
>> --- /dev/null
>> +++ b/libavcodec/dct.c
>> @@ -0,0 +1,39 @@
>> +#include "dsputil.h"
>> +#include "fft4g.h"
>
> license header

>> --- /dev/null
>> +++ b/libavcodec/rdft.c
>> @@ -0,0 +1,39 @@
>> +#include "dsputil.h"
>> +#include "fft4g.h"
>
> license header

These files are not intended to be committed as-is, since they are
just wrappers around the fft4g functions, which are incompatibly
licensed; I can add the license header, but really these files should
be replaced outright before any commit.

>> +    s->nbits = nbits;
>> +    s->inverse = inverse;
>> +    s->w = av_malloc(sizeof(double) * n * 5 / 4);
>> +    s->data_dbl = av_malloc(sizeof(double) * n);
>
> This could be aligned.

>> +    s->nbits = nbits;
>> +    s->inverse = inverse;
>> +    s->w = av_malloc(sizeof(double) * n / 2);
>> +    s->data_dbl = av_malloc(sizeof(double) * n);
>
> alignment

Similarly to above, these files are just temporary hacks until someone
writes a real implementation based on the internal FFT.

>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -310,6 +310,7 @@ enum CodecID {
>>      CODEC_ID_MP1,
>> +    CODEC_ID_BINKAUDIO,
>
> Why not just BINK instead of BINKAUDIO?

As Kostya noted, eventually BINKVIDEO will (hopefully) exist as well.

>> --- /dev/null
>> +++ b/libavcodec/binkaudio.c
>> @@ -0,0 +1,285 @@
>> +// from wmadata.h
>
> Ahem :)

Indeed, I noticed the duplication, but I don't know how to fix it -
including wmadata.h directly causes a bunch of duplicated definitions
at link time (perhaps some of the tables need to be made static?).  I
will look into this before the next iteration of the patch.

>> +        sample_rate *= avctx->channels;
>> +        s->frame_len *= avctx->channels;
>
> alignment
>
>> +    s->overlap_len = s->frame_len / 16;
>> +    sample_rate_half = (sample_rate + 1) / 2;
>> +    s->root = 2.0 / sqrt(s->frame_len);
>
> ditto
>
>> +static int binkaudio_decode_frame(AVCodecContext *avctx, void *outdata, int *outdata_size, const uint8_t *buf, int buf_size)
>
> Maybe you could shorten such long lines..

Will fix the above for next iteration...

>> --- /dev/null
>> +++ b/libavformat/bink.c
>> @@ -0,0 +1,254 @@
>> +/*
>> + * Bink file Demuxer
>
> demuxer
>
> Documentation and changelog updates are missing from your patches.

Sorry, didn't notice these needed to be updated; will something like
the attached work?

Thanks,
-- Daniel Verkamp
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Bink-demuxer-and-audio-decoder-docs-and-changelog.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090114/bc2379df/attachment.txt>