[FFmpeg-devel] [PATCH v6] Add support for Audible AA files

[Vesselin Bontchev]

02.08.2015, 07:44, "James Almer" <jamrial at gmail.com>:
> On 30/07/15 7:46 AM, Vesselin Bontchev wrote:
>>  From 06b0c0013404a67c72ea14a3c90730c0c4bd5b9a Mon Sep 17 00:00:00 2001
>>  From: Vesselin Bontchev <vesselin.bontchev at yandex.com>
>>  Date: Sun, 19 Jul 2015 23:16:36 +0200
>>  Subject: [PATCH] Add support for Audible AA files
>>
>>  + AVIOContext *pb = s->pb;
>>  + AVStream *st = avformat_new_stream(s, NULL);
>>  + if (!st)
>>  + return AVERROR(ENOMEM);
>>  + c->tea_ctx = av_tea_alloc();
>
> You could move this below right before tea_init and replace all of the gotos below
> with simple returns.

Thanks, this really makes the code look better! :)

>>  + /* verify fixed key */
>>  + if (c->aa_fixed_key_size != 16) {
>
> This is zeroed during init but apparently never touched after that.
> You should probably check the length of the AVOption aa_fixed_key instead.

http://ffmpeg.org/doxygen/trunk/group__avoptions.html seems to say that this field is automatically updated (and in practice it does get updated according to what you pass in).

>>  +static const AVOption aa_options[] = {
>>  + { "aa_fixed_key", // extracted from libAAX_SDK.so and AAXSDKWin.dll files!
>>  + "Fixed key used for handling Audible AA files", OFFSET(aa_fixed_key),
>>  + AV_OPT_TYPE_BINARY, {.str="77214d4b196a87cd520045fd2a51d673"},
>
> This should probably be AV_OPT_TYPE_STRING.

We want to hex decode the value of "aa_fixed_key". So, AV_OPT_TYPE_BINARY seems to be appropriate.

I have incorporated rest of your feedback into a new revision of the patch, thanks!

Vesselin