[FFmpeg-devel] [PATCH] avformat/mov: fix regression in processing .aax files

[Vesselin Bontchev]

01.08.2015, 03:43, "Michael Niedermayer" <michael at niedermayer.cc>:
> On Fri, Jul 31, 2015 at 01:36:58PM +0300, Vesselin Bontchev wrote:
>>   mov.c | 8 ++++----
>>
>>  From 418d573659dd011de503e45622307974cf437df5 Mon Sep 17 00:00:00 2001
>>  From: Vesselin Bontchev <vesselin.bontchev at yandex.com>
>>
>>       /* verify activation data */
>>       if (!activation_bytes || c->activation_bytes_size != 4) {
>
> not part of this patch but
> both check for c->activation_bytes_size != 4
> is this intended? isn't the 2nd unreachable that way?

Good catch, this is not intended at all!

>>  - av_log(c->fc, AV_LOG_FATAL, "[aax] activation_bytes option is missing!\n");
>>  - ret = AVERROR(EINVAL);
>>  + av_log(c->fc, AV_LOG_WARNING, "[aax] activation_bytes option is missing!\n");
>>  + ret = 0; /* allow ffprobe to continue working on .aax files */
>>           goto fail;
>>       }
>
>>       if (c->activation_bytes_size != 4) {
>>  - av_log(c->fc, AV_LOG_FATAL, "[aax] activation_bytes value needs to be 4 bytes!\n");
>>  - ret = AVERROR(EINVAL);
>>  + av_log(c->fc, AV_LOG_WARNING, "[aax] activation_bytes value needs to be 4 bytes!\n");
>>  + ret = 0; /* allow ffprobe to continue working on .aax files */
>>           goto fail;
>>       }
>
> are both needed for ffprobe ?
> wouldn't ffprobe simply have it not set and take the first if() always

You are absolutely right. I am attaching a new fixed revision of the patch.

Vesselin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avformat-mov-fix-regression-in-processing-.aax-files.patch
Type: text/x-diff
Size: 1295 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150802/0b27da57/attachment.patch>