[FFmpeg-devel] [PATCH 1/3 v2] avformat/apetag: fix flag value to signal footer presence

[James Almer]

On 2/10/2017 6:20 PM, Paul B Mahol wrote:
> On 2/10/17, James Almer <jamrial at gmail.com> wrote:
>> According to the spec[1], a value of 0 means the footer is present and a
>> value
>> of 1 means it's absent, the exact opposite of header presence flag where 1
>> means present and 0 absent.
>> The reason for this is compatibility with APEv1 tags, where there's no
>> header,
>> footer presence was mandatory for all files, and the flags field was a
>> zeroed
>> reserved field.
>>
>> [1] http://wiki.hydrogenaud.io/index.php?title=Ape_Tags_Flags
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> v2 changes: Added missing fate test update.
>>
>> Files already created are obviously wrong, but fortunately having the flag
>> mistakenly reporting there's no footer is harmless in APEv2 tags stored at
>> the end of the file (The only kind we can read and write), since footer is
>> mandatory in those and no and such software really bothers looking at the
>> flag.
>>
>> I renamed the constant just to have it as reference, or until someone feels
>> like adding support for APEv2 tags at the beginning of a file, where footer
>> becomes optional.
>>
>>  libavformat/apetag.c | 7 +++----
>>  tests/ref/lavf/tta   | 2 +-
>>  2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/apetag.c b/libavformat/apetag.c
>> index 08e80f4aa3..a05b32d9e5 100644
>> --- a/libavformat/apetag.c
>> +++ b/libavformat/apetag.c
>> @@ -30,7 +30,7 @@
>>  #include "internal.h"
>>
>>  #define APE_TAG_FLAG_CONTAINS_HEADER  (1 << 31)
>> -#define APE_TAG_FLAG_CONTAINS_FOOTER  (1 << 30)
>> +#define APE_TAG_FLAG_LACKS_FOOTER     (1 << 30)
>>  #define APE_TAG_FLAG_IS_HEADER        (1 << 29)
>>  #define APE_TAG_FLAG_IS_BINARY        (1 << 1)
>>
>> @@ -189,8 +189,7 @@ int ff_ape_write_tag(AVFormatContext *s)
>>          goto end;
>>
>>      // flags
>> -    avio_wl32(dyn_bc, APE_TAG_FLAG_CONTAINS_HEADER |
>> APE_TAG_FLAG_CONTAINS_FOOTER |
>> -                     APE_TAG_FLAG_IS_HEADER);
>> +    avio_wl32(dyn_bc, APE_TAG_FLAG_CONTAINS_HEADER |
>> APE_TAG_FLAG_IS_HEADER);
>>      ffio_fill(dyn_bc, 0, 8);             // reserved
>>
>>      ff_standardize_creation_time(s);
>> @@ -232,7 +231,7 @@ int ff_ape_write_tag(AVFormatContext *s)
>>      avio_wl32(s->pb, count);            // tag count
>>
>>      // flags
>> -    avio_wl32(s->pb, APE_TAG_FLAG_CONTAINS_HEADER |
>> APE_TAG_FLAG_CONTAINS_FOOTER);
>> +    avio_wl32(s->pb, APE_TAG_FLAG_CONTAINS_HEADER);
>>      ffio_fill(s->pb, 0, 8);             // reserved
>>
>>  end:
>> diff --git a/tests/ref/lavf/tta b/tests/ref/lavf/tta
>> index 745e8d21bd..d86d097e3d 100644
>> --- a/tests/ref/lavf/tta
>> +++ b/tests/ref/lavf/tta
>> @@ -1,3 +1,3 @@
>> -f2721d06704ac43d89fdd25835b43598 *./tests/data/lavf/lavf.tta
>> +d86c5cccb2554143d34d1786ab460a31 *./tests/data/lavf/lavf.tta
>>  43200 ./tests/data/lavf/lavf.tta
>>  ./tests/data/lavf/lavf.tta CRC=0x3a1da17e
>> --
>> 2.11.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> probably ok

Pushed, thanks.