avformat/asf: Factor common code out

Both functions to read attached pictures coincide since
e83f27a21a6d2f602b55e541ef66e365400e9827 (save for some log messages
in case av_dict_set failed).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>

libavformat/asf.c

@@ -19,6 +19,7 @@
  */
 
 #include "asf.h"
+#include "id3v2.h"
 
 const ff_asf_guid ff_asf_header = {
     0x30, 0x26, 0xB2, 0x75, 0x8E, 0x66, 0xCF, 0x11, 0xA6, 0xD9, 0x00, 0xAA, 0x00, 0x62, 0xCE, 0x6C
@@ -170,3 +171,119 @@ const AVMetadataConv ff_asf_metadata_conv[] = {
 //  { "Year"               , "date"        }, TODO: conversion year<->date
     { 0 }
 };
+
+/* MSDN claims that this should be "compatible with the ID3 frame, APIC",
+ * but in reality this is only loosely similar */
+static int asf_read_picture(AVFormatContext *s, int len)
+{
+    AVPacket pkt          = { 0 };
+    const CodecMime *mime = ff_id3v2_mime_tags;
+    enum  AVCodecID id    = AV_CODEC_ID_NONE;
+    char mimetype[64];
+    uint8_t  *desc = NULL;
+    AVStream   *st = NULL;
+    int ret, type, picsize, desc_len;
+
+    /* type + picsize + mime + desc */
+    if (len < 1 + 4 + 2 + 2) {
+        av_log(s, AV_LOG_ERROR, "Invalid attached picture size: %d.\n", len);
+        return AVERROR_INVALIDDATA;
+    }
+
+    /* picture type */
+    type = avio_r8(s->pb);
+    len--;
+    if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types) || type < 0) {
+        av_log(s, AV_LOG_WARNING, "Unknown attached picture type: %d.\n", type);
+        type = 0;
+    }
+
+    /* picture data size */
+    picsize = avio_rl32(s->pb);
+    len    -= 4;
+
+    /* picture MIME type */
+    len -= avio_get_str16le(s->pb, len, mimetype, sizeof(mimetype));
+    while (mime->id != AV_CODEC_ID_NONE) {
+        if (!strncmp(mime->str, mimetype, sizeof(mimetype))) {
+            id = mime->id;
+            break;
+        }
+        mime++;
+    }
+    if (id == AV_CODEC_ID_NONE) {
+        av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
+               mimetype);
+        return 0;
+    }
+
+    if (picsize >= len) {
+        av_log(s, AV_LOG_ERROR, "Invalid attached picture data size: %d >= %d.\n",
+               picsize, len);
+        return AVERROR_INVALIDDATA;
+    }
+
+    /* picture description */
+    desc_len = (len - picsize) * 2 + 1;
+    desc     = av_malloc(desc_len);
+    if (!desc)
+        return AVERROR(ENOMEM);
+    len -= avio_get_str16le(s->pb, len - picsize, desc, desc_len);
+
+    ret = av_get_packet(s->pb, &pkt, picsize);
+    if (ret < 0)
+        goto fail;
+
+    st  = avformat_new_stream(s, NULL);
+    if (!st) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    st->disposition              |= AV_DISPOSITION_ATTACHED_PIC;
+    st->codecpar->codec_type      = AVMEDIA_TYPE_VIDEO;
+    st->codecpar->codec_id        = id;
+    st->attached_pic              = pkt;
+    st->attached_pic.stream_index = st->index;
+    st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
+
+    if (*desc) {
+        if (av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL) < 0)
+            av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
+    } else
+        av_freep(&desc);
+
+    if (av_dict_set(&st->metadata, "comment", ff_id3v2_picture_types[type], 0) < 0)
+        av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
+
+    return 0;
+
+fail:
+    av_freep(&desc);
+    av_packet_unref(&pkt);
+    return ret;
+}
+
+static int get_id3_tag(AVFormatContext *s, int len)
+{
+    ID3v2ExtraMeta *id3v2_extra_meta = NULL;
+
+    ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, len);
+    if (id3v2_extra_meta) {
+        ff_id3v2_parse_apic(s, id3v2_extra_meta);
+        ff_id3v2_parse_chapters(s, id3v2_extra_meta);
+        ff_id3v2_free_extra_meta(&id3v2_extra_meta);
+    }
+    return 0;
+}
+
+int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
+                             int val_len)
+{
+    if (!strcmp(name, "WM/Picture")) // handle cover art
+        return asf_read_picture(s, val_len);
+    else if (!strcmp(name, "ID3")) // handle ID3 tag
+        return get_id3_tag(s, val_len);
+
+    return 1;
+}

libavformat/asf.h

@@ -104,6 +104,16 @@ extern const ff_asf_guid ff_asf_mutex_language;
 
 extern const AVMetadataConv ff_asf_metadata_conv[];
 
+/**
+ * Handles both attached pictures as well as id3 tags.
+ *
+ * @return Returns < 0 on error, 1 if the type of the byte array
+ *         is unsupported by this function and 0 otherwise.
+ */
+int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
+                             int val_len);
+
+
 #define ASF_PACKET_FLAG_ERROR_CORRECTION_PRESENT 0x80 //1000 0000
 
 

libavformat/asfdec_f.c

@@ -33,7 +33,6 @@
 #include "avformat.h"
 #include "avio_internal.h"
 #include "avlanguage.h"
-#include "id3v2.h"
 #include "internal.h"
 #include "riff.h"
 #include "asf.h"
@@ -219,107 +218,6 @@ static int get_value(AVIOContext *pb, int type, int type2_size)
     }
 }
 
-/* MSDN claims that this should be "compatible with the ID3 frame, APIC",
- * but in reality this is only loosely similar */
-static int asf_read_picture(AVFormatContext *s, int len)
-{
-    AVPacket pkt          = { 0 };
-    const CodecMime *mime = ff_id3v2_mime_tags;
-    enum  AVCodecID id    = AV_CODEC_ID_NONE;
-    char mimetype[64];
-    uint8_t  *desc = NULL;
-    AVStream   *st = NULL;
-    int ret, type, picsize, desc_len;
-
-    /* type + picsize + mime + desc */
-    if (len < 1 + 4 + 2 + 2) {
-        av_log(s, AV_LOG_ERROR, "Invalid attached picture size: %d.\n", len);
-        return AVERROR_INVALIDDATA;
-    }
-
-    /* picture type */
-    type = avio_r8(s->pb);
-    len--;
-    if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types) || type < 0) {
-        av_log(s, AV_LOG_WARNING, "Unknown attached picture type: %d.\n", type);
-        type = 0;
-    }
-
-    /* picture data size */
-    picsize = avio_rl32(s->pb);
-    len    -= 4;
-
-    /* picture MIME type */
-    len -= avio_get_str16le(s->pb, len, mimetype, sizeof(mimetype));
-    while (mime->id != AV_CODEC_ID_NONE) {
-        if (!strncmp(mime->str, mimetype, sizeof(mimetype))) {
-            id = mime->id;
-            break;
-        }
-        mime++;
-    }
-    if (id == AV_CODEC_ID_NONE) {
-        av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
-               mimetype);
-        return 0;
-    }
-
-    if (picsize >= len) {
-        av_log(s, AV_LOG_ERROR, "Invalid attached picture data size: %d >= %d.\n",
-               picsize, len);
-        return AVERROR_INVALIDDATA;
-    }
-
-    /* picture description */
-    desc_len = (len - picsize) * 2 + 1;
-    desc     = av_malloc(desc_len);
-    if (!desc)
-        return AVERROR(ENOMEM);
-    len -= avio_get_str16le(s->pb, len - picsize, desc, desc_len);
-
-    ret = av_get_packet(s->pb, &pkt, picsize);
-    if (ret < 0)
-        goto fail;
-
-    st  = avformat_new_stream(s, NULL);
-    if (!st) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-    st->disposition              |= AV_DISPOSITION_ATTACHED_PIC;
-    st->codecpar->codec_type      = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id        = id;
-    st->attached_pic              = pkt;
-    st->attached_pic.stream_index = st->index;
-    st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
-
-    if (*desc)
-        av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL);
-    else
-        av_freep(&desc);
-
-    av_dict_set(&st->metadata, "comment", ff_id3v2_picture_types[type], 0);
-
-    return 0;
-
-fail:
-    av_freep(&desc);
-    av_packet_unref(&pkt);
-    return ret;
-}
-
-static void get_id3_tag(AVFormatContext *s, int len)
-{
-    ID3v2ExtraMeta *id3v2_extra_meta = NULL;
-
-    ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, len);
-    if (id3v2_extra_meta) {
-        ff_id3v2_parse_apic(s, id3v2_extra_meta);
-        ff_id3v2_parse_chapters(s, id3v2_extra_meta);
-    }
-    ff_id3v2_free_extra_meta(&id3v2_extra_meta);
-}
-
 static void get_tag(AVFormatContext *s, const char *key, int type, int len, int type2_size)
 {
     ASFContext *asf = s->priv_data;
@@ -345,13 +243,8 @@ static void get_tag(AVFormatContext *s, const char *key, int type, int len, int
         value[len]=0;
         break;
     case ASF_BYTE_ARRAY:
-        if (!strcmp(key, "WM/Picture")) { // handle cover art
+        if (ff_asf_handle_byte_array(s, key, len) > 0)
-            asf_read_picture(s, len);
-        } else if (!strcmp(key, "ID3")) { // handle ID3 tag
-            get_id3_tag(s, len);
-        } else {
             av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n", key);
-        }
         goto finish;
     case ASF_BOOL:
     case ASF_DWORD:

libavformat/asfdec_o.c

@@ -20,19 +20,15 @@
  */
 
 #include "libavutil/attributes.h"
-#include "libavutil/avstring.h"
-#include "libavutil/bswap.h"
 #include "libavutil/common.h"
 #include "libavutil/dict.h"
 #include "libavutil/internal.h"
 #include "libavutil/mathematics.h"
-#include "libavutil/opt.h"
 #include "libavutil/time_internal.h"
 
 #include "avformat.h"
 #include "avio_internal.h"
 #include "avlanguage.h"
-#include "id3v2.h"
 #include "internal.h"
 #include "riff.h"
 #include "asf.h"
@@ -356,110 +352,6 @@ static int asf_set_metadata(AVFormatContext *s, const uint8_t *name,
     return 0;
 }
 
-/* MSDN claims that this should be "compatible with the ID3 frame, APIC",
- * but in reality this is only loosely similar */
-static int asf_read_picture(AVFormatContext *s, int len)
-{
-    AVPacket pkt          = { 0 };
-    const CodecMime *mime = ff_id3v2_mime_tags;
-    enum  AVCodecID id    = AV_CODEC_ID_NONE;
-    char mimetype[64];
-    uint8_t  *desc = NULL;
-    AVStream   *st = NULL;
-    int ret, type, picsize, desc_len;
-
-    /* type + picsize + mime + desc */
-    if (len < 1 + 4 + 2 + 2) {
-        av_log(s, AV_LOG_ERROR, "Invalid attached picture size: %d.\n", len);
-        return AVERROR_INVALIDDATA;
-    }
-
-    /* picture type */
-    type = avio_r8(s->pb);
-    len--;
-    if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types) || type < 0) {
-        av_log(s, AV_LOG_WARNING, "Unknown attached picture type: %d.\n", type);
-        type = 0;
-    }
-
-    /* picture data size */
-    picsize = avio_rl32(s->pb);
-    len    -= 4;
-
-    /* picture MIME type */
-    len -= avio_get_str16le(s->pb, len, mimetype, sizeof(mimetype));
-    while (mime->id != AV_CODEC_ID_NONE) {
-        if (!strncmp(mime->str, mimetype, sizeof(mimetype))) {
-            id = mime->id;
-            break;
-        }
-        mime++;
-    }
-    if (id == AV_CODEC_ID_NONE) {
-        av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
-               mimetype);
-        return 0;
-    }
-
-    if (picsize >= len) {
-        av_log(s, AV_LOG_ERROR, "Invalid attached picture data size: %d >= %d.\n",
-               picsize, len);
-        return AVERROR_INVALIDDATA;
-    }
-
-    /* picture description */
-    desc_len = (len - picsize) * 2 + 1;
-    desc     = av_malloc(desc_len);
-    if (!desc)
-        return AVERROR(ENOMEM);
-    len -= avio_get_str16le(s->pb, len - picsize, desc, desc_len);
-
-    ret = av_get_packet(s->pb, &pkt, picsize);
-    if (ret < 0)
-        goto fail;
-
-    st  = avformat_new_stream(s, NULL);
-    if (!st) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-
-    st->disposition              |= AV_DISPOSITION_ATTACHED_PIC;
-    st->codecpar->codec_type      = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id        = id;
-    st->attached_pic              = pkt;
-    st->attached_pic.stream_index = st->index;
-    st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
-
-    if (*desc) {
-        if (av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL) < 0)
-            av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
-    } else
-        av_freep(&desc);
-
-    if (av_dict_set(&st->metadata, "comment", ff_id3v2_picture_types[type], 0) < 0)
-        av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
-
-    return 0;
-
-fail:
-    av_freep(&desc);
-    av_packet_unref(&pkt);
-    return ret;
-}
-
-static void get_id3_tag(AVFormatContext *s, int len)
-{
-    ID3v2ExtraMeta *id3v2_extra_meta = NULL;
-
-    ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, len);
-    if (id3v2_extra_meta) {
-        ff_id3v2_parse_apic(s, id3v2_extra_meta);
-        ff_id3v2_parse_chapters(s, id3v2_extra_meta);
-    }
-    ff_id3v2_free_extra_meta(&id3v2_extra_meta);
-}
-
 static int process_metadata(AVFormatContext *s, const uint8_t *name, uint16_t name_len,
                             uint16_t val_len, uint16_t type, AVDictionary **met)
 {
@@ -472,11 +364,7 @@ static int process_metadata(AVFormatContext *s, const uint8_t *name, uint16_t na
             asf_read_value(s, name, val_len, type, met);
             break;
         case ASF_BYTE_ARRAY:
-            if (!strcmp(name, "WM/Picture")) // handle cover art
+            if (ff_asf_handle_byte_array(s, name, val_len) > 0)
-                asf_read_picture(s, val_len);
-            else if (!strcmp(name, "ID3")) // handle ID3 tag
-                get_id3_tag(s, val_len);
-            else
                 asf_read_value(s, name, val_len, type, met);
             break;
         case ASF_GUID: