diff mbox

[FFmpeg-devel] fate: add id3v2 tests

Message ID 20180123175438.998-1-rshaffer@tunein.com
State New
Headers show

Commit Message

rshaffer@tunein.com Jan. 23, 2018, 5:54 p.m. UTC
From: Richard Shaffer <rshaffer@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

Comments

wm4 Jan. 23, 2018, 10:32 p.m. UTC | #1
On Tue, 23 Jan 2018 09:54:38 -0800
rshaffer@tunein.com wrote:

> From: Richard Shaffer <rshaffer@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.
rshaffer@tunein.com Jan. 24, 2018, 1:03 a.m. UTC | #2
On Tue, Jan 23, 2018 at 2:32 PM, wm4 <nfxjfg@googlemail.com> wrote:

>
> 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.
>

It's definitely not my desire to create a maintenance burden for anyone. I
used the ffmpeg utility initially to test my changes, and I'm sure that
shell scripting would have been easier than writing a C program.

There were pretty good templates for other test programs, but I didn't come
across a collection of shell scripts or Makefile targets that seemed to
illustrate a clear pattern. I'll look a little harder. If there is a
pointer to an example test, though, that would probably be useful.

Thanks,

-Richard

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
wm4 Jan. 24, 2018, 1:46 a.m. UTC | #3
On Tue, 23 Jan 2018 17:03:26 -0800
Richard Shaffer <rshaffer@tunein.com> wrote:

> On Tue, Jan 23, 2018 at 2:32 PM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> >
> > 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.
> >  
> 
> It's definitely not my desire to create a maintenance burden for anyone. I
> used the ffmpeg utility initially to test my changes, and I'm sure that
> shell scripting would have been easier than writing a C program.
> 
> There were pretty good templates for other test programs, but I didn't come
> across a collection of shell scripts or Makefile targets that seemed to
> illustrate a clear pattern. I'll look a little harder. If there is a
> pointer to an example test, though, that would probably be useful.

Most FATE tests use some sort of ffmpeg CLI invocation via shell, but
it's pretty cryptic. Not sure what's a good example.
James Almer Jan. 24, 2018, 2:15 a.m. UTC | #4
On 1/23/2018 10:46 PM, wm4 wrote:
> On Tue, 23 Jan 2018 17:03:26 -0800
> Richard Shaffer <rshaffer@tunein.com> wrote:
> 
>> On Tue, Jan 23, 2018 at 2:32 PM, wm4 <nfxjfg@googlemail.com> wrote:
>>
>>>
>>> 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.
>>>  
>>
>> It's definitely not my desire to create a maintenance burden for anyone. I
>> used the ffmpeg utility initially to test my changes, and I'm sure that
>> shell scripting would have been easier than writing a C program.
>>
>> There were pretty good templates for other test programs, but I didn't come
>> across a collection of shell scripts or Makefile targets that seemed to
>> illustrate a clear pattern. I'll look a little harder. If there is a
>> pointer to an example test, though, that would probably be useful.
> 
> Most FATE tests use some sort of ffmpeg CLI invocation via shell, but
> it's pretty cryptic. Not sure what's a good example.

ffprobe -show_format is probably best for an id3v2 tag. Add a new
probeformat() function to fate-run.sh like

probeformat(){
    run ffprobe${PROGSUF} -show_format -v 0 "$@"
}

Then call it with the sample in question a argument. See how exif.mak
uses probeframes().
rshaffer@tunein.com Jan. 24, 2018, 5:28 p.m. UTC | #5
On Tue, Jan 23, 2018 at 6:15 PM, James Almer <jamrial@gmail.com> wrote:
>
>
> ffprobe -show_format is probably best for an id3v2 tag. Add a new
> probeformat() function to fate-run.sh like
>
> probeformat(){
>     run ffprobe${PROGSUF} -show_format -v 0 "$@"
> }
>
> Then call it with the sample in question a argument. See how exif.mak
> uses probeframes().
>

Thanks, James. That's helpful. I'll take a look today.

-Richard

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

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()
+{
+    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