| Message ID | 20170210210230.7180-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | e8d6fef3161f35878f8e0abf9d27d2c45a5d40b6 |
| Headers | show |
On 2/10/17, James Almer <jamrial@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@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > probably ok
On 2/10/2017 6:20 PM, Paul B Mahol wrote: > On 2/10/17, James Almer <jamrial@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@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@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > probably ok Pushed, thanks.
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
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@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(-)