On Tue, 23 Jan 2018 09:54:38 -0800 rshaf...@tunein.com wrote: > From: Richard Shaffer <rshaf...@tunein.com> > > Adds basic unit tests for parsing and writing ID3v2 tags. > --- > This requires the patch to "add option to parse/store ID3 PRIV tags in > metadata." > > libavformat/Makefile | 3 +- > libavformat/tests/.gitignore | 1 + > libavformat/tests/id3v2.c | 229 > +++++++++++++++++++++++++++++++++++++++++++ > tests/fate/libavformat.mak | 4 + > tests/ref/fate/id3v2 | 4 + > 5 files changed, 240 insertions(+), 1 deletion(-) > create mode 100644 libavformat/tests/id3v2.c > create mode 100644 tests/ref/fate/id3v2 > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index de0de921c2..753edd9cd5 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -609,7 +609,8 @@ SLIBOBJS-$(HAVE_GNU_WINDRES) += avformatres.o > SKIPHEADERS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpdh.h > SKIPHEADERS-$(CONFIG_NETWORK) += network.h rtsp.h > > -TESTPROGS = seek \ > +TESTPROGS = id3v2 \ > + seek \ > url \ > # async \ > > diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore > index 7ceb7a356b..5a06704afd 100644 > --- a/libavformat/tests/.gitignore > +++ b/libavformat/tests/.gitignore > @@ -1,4 +1,5 @@ > /fifo_muxer > +/id3v2 > /movenc > /noproxy > /rtmpdh > diff --git a/libavformat/tests/id3v2.c b/libavformat/tests/id3v2.c > new file mode 100644 > index 0000000000..9efd0f9011 > --- /dev/null > +++ b/libavformat/tests/id3v2.c > @@ -0,0 +1,229 @@ > +/* > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +#include <stdio.h> > +#include <string.h> > + > +#include "libavformat/avio.h" > +#include "libavformat/avio_internal.h" > +#include "libavformat/id3v2.h" > +#include "libavutil/dict.h" > + > +static int tag_equal(AVDictionary *meta, const char * tag, const char * > value) > +{ > + int failures = 0; > + AVDictionaryEntry *entry = av_dict_get(meta, tag, NULL, 0); > + if (!entry) { > + failures++; > + fprintf(stderr, "expected metadata tag '%s' not found\n", tag); > + } > + if (strcmp(entry->value, value)) { > + failures++; > + fprintf(stderr, "metadata tag '%s' has value '%s'; expected '%s'\n", > + tag, entry->value, value); > + } > + return failures; > +} > + > +static int test_id3v2_read()
In C, functions with no arguments need to use (void). Just using () has the legacy meaning of "accepts any arguments", which should never be used. > +{ > + uint8_t buf[] = { > + 'I', 'D', '3', /*ver*/ 4, 0, /*flags*/ 0, /*size*/ 0, 0, 0, 53, > + > + 'T', 'I', 'T', '2', /*size*/ 0, 0, 0, 12, /*flags*/ 0, 0, /*utf8*/ 3, > + 'T', 'e', 's', 't', ' ', 'T', 'i', 't', 'l', 'e', 0, > + > + 'P', 'R', 'I', 'V', /* size */ 0, 0, 0, 21, /*flags*/ 0, 0, > + 't', 'e', 's', 't', 'o', 'w', 'n', 'e', 'r', 0, > + 't', 'e', 's', 't', 'd', 'a', 't', 'a', /*some extra data*/ 0, 1, 2, > + }; > + int failures = 0; > + AVIOContext io; > + AVDictionary *meta = NULL; > + ID3v2ExtraMeta *extra_meta = NULL; > + > + ffio_init_context(&io, buf, sizeof(buf), 0, NULL, NULL, NULL, NULL); > + > + ff_id3v2_read_dict(&io, &meta, ID3v2_DEFAULT_MAGIC, &extra_meta); > + ff_id3v2_parse_priv_dict(&meta, &extra_meta); > + > + failures += tag_equal(meta, "title", "Test Title"); > + failures += tag_equal(meta, ID3v2_PRIV_METADATA_PREFIX "testowner", > + "testdata\\x00\\x01\\x02"); > + > + av_dict_free(&meta); > + ff_id3v2_free_extra_meta(&extra_meta); > + return failures; > +} > + > +static int check_header(const uint8_t *buf, size_t buflen, const char * > magic, int version, int size) > +{ > + int failures = 0; > + int i, s; > + if (buflen < 10) { > + fprintf(stderr, "id3v2 wrote short header\n"); > + return 1; > + } > + if (memcmp(buf, magic, 3)) { > + failures++; > + fprintf(stderr, "id3v2 wrote magic '%.3s'; expected '%s'\n", magic, > buf); > + } > + if (buf[3] != version || buf[4] != 0) { > + failures++; > + fprintf(stderr, "id3v2 wrote version %d.%d; expected 4.0\n", buf[3], > buf[4]); > + } > + if (buf[5]) { > + failures++; > + fprintf(stderr, "id3v2 wrote flags %#x; expected 0x0\n", buf[5]); > + } > + for (i = 6, s = 0; i < 10; i++) { > + if (buf[i] & 0x80) { > + failures++; > + fprintf(stderr, "id3v2 wrote invalid synchsafe integer in header > size\n"); > + } > + s |= (buf[i] & 0x7f) << ((9 - i) * 8); > + } > + if (s != size) { > + failures++; > + fprintf(stderr, "id3v2 wrote tag size %d; expected %d\n", s, size); > + } > + return failures; > +} > + > +static int check_frame(const char *id, const uint8_t *buf, int buflen, const > uint8_t *expected, int expectedlen) > +{ > + int failures = 0, s, i; > + if (buflen < 10) { > + fprintf(stderr, "id3v2 wrote short frame header\n"); > + return 1; > + } > + if (strncmp(id, buf, 4)) { > + failures++; > + fprintf(stderr, "id3v2 wrote frame id %.4s; expected %s\n", buf, id); > + } > + for (i = 4, s = 0; i < 8; i++) { > + if (buf[i] & 0x80) { > + failures++; > + fprintf(stderr, "id3v2 wrote invalid synchsafe integer in header > size\n"); > + } > + s |= (buf[i] & 0x7f) << ((7 - i) * 8); > + } > + if (buf[8] || buf[9]) { > + failures++; > + fprintf(stderr, "id3v2 wrote frame flags %#x; expected 0x0\n", > (buf[8] << 8) | buf[9]); > + } > + if (s != expectedlen) { > + failures++; > + fprintf(stderr, "id3v2 wrote frame size %d; expected %d\n", s, > expectedlen); > + } > + if (buflen - 10 < expectedlen) { > + failures++; > + fprintf(stderr, "id3v2 wrote short frame\n"); > + } else if (memcmp(&buf[10], expected, expectedlen)) { > + failures++; > + fprintf(stderr, "id3v2 wrote unexpected frame data\n"); > + } > + return failures; > +} > + > +static int test_id3v2_write() > +{ > + int failures = 0, ret, len; > + uint8_t *buf; > + const uint8_t tit2_data[] = { > + 3, 'T', 'e', 's', 't', ' ', 'T', 'i', 't', 'l', 'e', 0}; > + const uint8_t priv_data[] = { > + 't', 'e', 's', 't', 'o', 'w', 'n', 'e', 'r', 0, > + 't', 'e', 's', 't', 'd', 'a', 't', 'a', 0, 1, 2 > + }; > + AVFormatContext *s = avformat_alloc_context(); > + > + avio_open_dyn_buf(&s->pb); > + av_dict_set(&s->metadata, "title", "Test Title", 0); > + av_dict_set(&s->metadata, ID3v2_PRIV_METADATA_PREFIX "testowner", > + "testdata\\x00\\x01\\x02", 0); > + > + if ((ret = ff_id3v2_write_simple(s, 4, ID3v2_DEFAULT_MAGIC))) { > + failures++; > + fprintf(stderr, "writing id3v2 tag return unexpected error %d\n", > ret); > + } > + > + len = avio_close_dyn_buf(s->pb, &buf); > + avformat_free_context(s); > + > + if (len < 63) { > + failures++; > + fprintf(stderr, "id3v2 wrote %d bytes; expected 63\n", ret); > + } > + > + failures += check_header(buf, len, ID3v2_DEFAULT_MAGIC, 4, len - 10); > + failures += check_frame("TIT2", &buf[10], len - 10, tit2_data, > sizeof(tit2_data)); > + failures += check_frame("PRIV", &buf[32], len - 32, priv_data, > sizeof(priv_data)); > + > + return failures; > +} > + > +static int test_id3v2_write_bad_priv() > +{ > + const char * values[] = { > + "test\\xXXdata", "testdata\\xXX", "testdata\\xX", "testdata\\x", > + "testdata\\x1", NULL > + }; > + int i, failures = 0, ret; > + AVFormatContext *s = avformat_alloc_context(); > + > + for (i = 0; values[i]; i++) { > + avio_open_dyn_buf(&s->pb); > + av_dict_set(&s->metadata, ID3v2_PRIV_METADATA_PREFIX "testowner", > values[i], 0); > + ret = ff_id3v2_write_simple(s, 4, ID3v2_DEFAULT_MAGIC); > + if (ret != AVERROR(EINVAL)) { > + failures++; > + fprintf(stderr, "writing id3v2 data returned %d; expected > AVERROR(EINVAL)\n", ret); > + } > + ffio_free_dyn_buf(&s->pb); > + av_dict_free(&s->metadata); > + } > + > + avformat_free_context(s); > + > + return failures; > +} > + > +int main(int argc, char **argv) > +{ > + int failures = 0, i; > + > + #define ID3v2_TEST(x) {x, #x} > + > + struct { int(*test)(void); const char * name; } tests[] = { > + ID3v2_TEST(test_id3v2_read), > + ID3v2_TEST(test_id3v2_write), > + ID3v2_TEST(test_id3v2_write_bad_priv), > + {NULL, NULL}, > + }; > + > + for (i = 0; tests[i].test; i++) { > + int f = tests[i].test(); > + printf("%s %s\n", tests[i].name, f ? "failed" : "passed"); > + failures += f; > + } > + > + printf("Ran %d tests; %d failed\n", i, failures); > + > + return failures; > +} > \ No newline at end of file > diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak > index cf1ba189dd..de59ab5ee5 100644 > --- a/tests/fate/libavformat.mak > +++ b/tests/fate/libavformat.mak > @@ -2,6 +2,10 @@ > #fate-async: libavformat/tests/async$(EXESUF) > #fate-async: CMD = run libavformat/tests/async > > +FATE_LIBAVFORMAT-yes += fate-id3v2 > +fate-id3v2: libavformat/tests/id3v2$(EXESUF) > +fate-id3v2: CMD = run libavformat/tests/id3v2 > + > FATE_LIBAVFORMAT-$(CONFIG_NETWORK) += fate-noproxy > fate-noproxy: libavformat/tests/noproxy$(EXESUF) > fate-noproxy: CMD = run libavformat/tests/noproxy > diff --git a/tests/ref/fate/id3v2 b/tests/ref/fate/id3v2 > new file mode 100644 > index 0000000000..594e09cfbd > --- /dev/null > +++ b/tests/ref/fate/id3v2 > @@ -0,0 +1,4 @@ > +test_id3v2_read passed > +test_id3v2_write passed > +test_id3v2_write_bad_priv passed > +Ran 3 tests; 0 failed I think this test program is pretty nice, though usually we try to get by with running the "ffmpeg" utility to test the libs. Having a relatively big program to test some obscure functionality might have a maintenance burden. I'm not sure how others feel about it; if nobody has a strong opinion on this I'll probably apply this in a week or so. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel