[FFmpeg-devel] [PATCH] AVI metadata retrieval improvements

Thilo Borgmann thilo.borgmann at mail.de
Tue Mar 25 10:07:24 CET 2014


Am 25.03.14 05:10, schrieb Michael Niedermayer:
> On Sun, Mar 23, 2014 at 10:09:41PM +0100, Thilo Borgmann wrote:
>> Am 21.03.14 20:16, schrieb Michael Niedermayer:
>>> On Fri, Mar 21, 2014 at 05:29:16PM +0100, Thilo Borgmann wrote:
>>>> Am 21.03.14 15:30, schrieb Michael Niedermayer:
>>>>> On Thu, Mar 20, 2014 at 11:12:27PM +0100, Thilo Borgmann wrote:
>>>>>> Am 04.03.14 17:16, schrieb gregory.wolfe at kodakalaris.com:
>>>>>>> This is the second of two changes I've made as part of our upgrade to
>>>>>>> the latest FFmpeg development branch.
>>>>>>>
>>>>>>> This patch enhances two aspects of metadata retrieval from AVI files.
>>>>>>> I've attached before/after command line output from ffmpeg for each
>>>>>>> modification.  Test video files that can be used to generate the
>>>>>>> before/after output have been uploaded to the FFmpeg FTP server.
>>>>>>>
>>>>>>>
>>>>>>> Patched file:  libavformat/avidec.c
>>>>>>> Description (from commit message):
>>>>>>>
>>>>>>> Added function avi_extract_stream_metadata().  Some cameras (e.g., Fuji)
>>>>>>> store stream metadata following the "strd" stream data tag.  Currently,
>>>>>>> avi_read_header() calls ff_get_extradata() to read and save this data in
>>>>>>> the codec's "extradata" slot.  This new function extracts metadata from
>>>>>>> "extradata" by first looking for the AVIF tag, then extracting metadata
>>>>>>> from the EXIF tags which follow it.
>>>>>>
>>>>>> I've rewritten almost everything to use existing EXIF functions.
>>>>>>
>>>>>> Patch attached, but there are 2 issues I need advise for:
>>>>>>
>>>>>> a) Current EXIF is in lavc only and relies on lavc/tiff.h and lavc/bytestream,
>>>>>> so these have to be moved (to lavu)?
>>>>>
>>>>> using just a header with macros/inline functions is fine
>>>>> using ff_* functions from other libs is not as ff_ is not exported
>>>>> using avpriv_* functions in lavf from lavc is ok but the ABI/API
>>>>> of such functions is then part of the libavcodec ABI/API, non public
>>>>> but still, so care has to be taken with future changes to these
>>>>> functions so their ABI/API isnt broken
>>>>
>>>> Ok these are the rules, but what should I do since there are ff_* functions
>>>> used? My guess was to move to libavu because metadata decoding seems to be
>>>> needed by formats and codecs, but I don't know if there is a better solution? Or
>>>> make exif tag decoding a public av_* funtion in lavc?
>>>
>>> i suggest to rename the functions to avpriv_*
>>> and make sure their ABI/API is future proof
>>
>> Updated patch(es) attached.
>>
>> -Thilo
> 
>>  exif.c     |    4 ++--
>>  exif.h     |    2 +-
>>  mjpegdec.c |    2 +-
>>  webp.c     |    2 +-
>>  4 files changed, 5 insertions(+), 5 deletions(-)
>> 5e78f749233830fb655a5fe154eef1bc9f95decc  0001-lavc-exif-Make-EXIF-IFD-decoding-part-of-private-API.patch
>> From 2b78e02ffbd1d36ea396d0c168c2bc17c2f44262 Mon Sep 17 00:00:00 2001
>> From: Thilo Borgmann <thilo.borgmann at mail.de>
>> Date: Sun, 23 Mar 2014 22:06:22 +0100
>> Subject: [PATCH 1/3] lavc/exif: Make EXIF IFD decoding part of private
>>  API/ABI.
>>
>> ---
>>  libavcodec/exif.c     | 4 ++--
>>  libavcodec/exif.h     | 2 +-
>>  libavcodec/mjpegdec.c | 2 +-
>>  libavcodec/webp.c     | 2 +-
>>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> needs a minor version bump
> 
> 
> [...]
>> @@ -822,6 +862,7 @@ static int avi_read_header(AVFormatContext *s)
>>                      size = FFMIN(size, list_end - cur_pos);
>>                  st = s->streams[stream_index];
>>  
>> +                if (st) {
> 
> why is this if() needed ?

I just moved it here from the extract_metadata function of Gregory's patch.
If "st = s->streams[stream_index];" is safe, it can be avoided.

Updated patches attached.

-Thilo
-------------- next part --------------
>From 3394bebb70f8b7abaea7aa67ef15405117cb9341 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Tue, 25 Mar 2014 10:03:30 +0100
Subject: [PATCH 1/3] lavc/exif: Make EXIF IFD decoding part of private
 API/ABI.

---
 libavcodec/exif.c     | 4 ++--
 libavcodec/exif.h     | 2 +-
 libavcodec/mjpegdec.c | 2 +-
 libavcodec/version.h  | 2 +-
 libavcodec/webp.c     | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/exif.c b/libavcodec/exif.c
index 9646426..9b3e8cb 100644
--- a/libavcodec/exif.c
+++ b/libavcodec/exif.c
@@ -80,7 +80,7 @@ static int exif_decode_tag(AVCodecContext *avctx, GetByteContext *gbytes, int le
     // store metadata or proceed with next IFD
     ret = ff_tis_ifd(id);
     if (ret) {
-        ret = ff_exif_decode_ifd(avctx, gbytes, le, depth + 1, metadata);
+        ret = avpriv_exif_decode_ifd(avctx, gbytes, le, depth + 1, metadata);
     } else {
         const char *name = exif_get_tag_name(id);
         char *use_name   = (char*) name;
@@ -107,7 +107,7 @@ static int exif_decode_tag(AVCodecContext *avctx, GetByteContext *gbytes, int le
 }
 
 
-int ff_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
+int avpriv_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
                        int depth, AVDictionary **metadata)
 {
     int i, ret;
diff --git a/libavcodec/exif.h b/libavcodec/exif.h
index 71fe829..e673dc0 100644
--- a/libavcodec/exif.h
+++ b/libavcodec/exif.h
@@ -164,7 +164,7 @@ static const struct exif_tag tag_list[] = { // JEITA CP-3451 EXIF specification:
 
 /** Recursively decodes all IFD's and
  *  adds included TAGS into the metadata dictionary. */
-int ff_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
+int avpriv_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
                        int depth, AVDictionary **metadata);
 
 #endif /* AVCODEC_EXIF_H */
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 2fd371a..4a7811a 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1630,7 +1630,7 @@ static int mjpeg_decode_app(MJpegDecodeContext *s)
 
         // read 0th IFD and store the metadata
         // (return values > 0 indicate the presence of subimage metadata)
-        ret = ff_exif_decode_ifd(s->avctx, &gbytes, le, 0, &s->exif_metadata);
+        ret = avpriv_exif_decode_ifd(s->avctx, &gbytes, le, 0, &s->exif_metadata);
         if (ret < 0) {
             av_log(s->avctx, AV_LOG_ERROR, "mjpeg: error decoding EXIF data\n");
             return ret;
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 381e471..f124c7b 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR 55
-#define LIBAVCODEC_VERSION_MINOR  52
+#define LIBAVCODEC_VERSION_MINOR  53
 #define LIBAVCODEC_VERSION_MICRO 102
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index 89c8f13..4dbdf78 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1449,7 +1449,7 @@ static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
             }
 
             bytestream2_seek(&exif_gb, ifd_offset, SEEK_SET);
-            if (ff_exif_decode_ifd(avctx, &exif_gb, le, 0, &s->exif_metadata) < 0) {
+            if (avpriv_exif_decode_ifd(avctx, &exif_gb, le, 0, &s->exif_metadata) < 0) {
                 av_log(avctx, AV_LOG_ERROR, "error decoding Exif data\n");
                 goto exif_end;
             }
-- 
1.8.3.2

-------------- next part --------------
>From 9ecc5c3e28b172e79358d1769fc71817d6ba1a32 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Tue, 25 Mar 2014 10:04:16 +0100
Subject: [PATCH 2/3] lavf/avidec: Read metadata EXIF tags from AVIF tag. Based
 on patch by Gregory Wolfe (Kodak Alaris) <gregory.wolfe at kodakalaris.com>.

---
 libavformat/avidec.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 0a837d4..cb1bc8f 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -34,6 +34,8 @@
 #include "dv.h"
 #include "internal.h"
 #include "riff.h"
+#include "libavcodec/bytestream.h"
+#include "libavcodec/exif.h"
 
 typedef struct AVIStream {
     int64_t frame_offset;   /* current frame (video) or byte (audio) counter
@@ -379,6 +381,44 @@ static void avi_read_nikon(AVFormatContext *s, uint64_t end)
     }
 }
 
+static int avi_extract_stream_metadata(AVStream *st)
+{
+    GetByteContext gb;
+    uint8_t *data = st->codec->extradata;
+    int data_size = st->codec->extradata_size;
+    int tag, offset;
+
+    if (!data || data_size < 8) {
+        return AVERROR_INVALIDDATA;
+    }
+
+    bytestream2_init(&gb, data, data_size);
+
+    tag = bytestream2_get_le32(&gb);
+
+    switch (tag) {
+    case MKTAG('A', 'V', 'I', 'F'):
+        // skip 4 byte padding
+        bytestream2_skip(&gb, 4);
+        offset = bytestream2_tell(&gb);
+        bytestream2_init(&gb, data + offset, data_size - offset);
+
+        // decode EXIF tags from IFD, AVI is always little-endian
+        return avpriv_exif_decode_ifd(st->codec, &gb, 1, 0, &st->metadata);
+        break;
+    case MKTAG('C', 'A', 'S', 'I'):
+        avpriv_request_sample(st->codec, "RIFF stream data tag type CASI (%u)", tag);
+        break;
+    case MKTAG('Z', 'o', 'r', 'a'):
+        avpriv_request_sample(st->codec, "RIFF stream data tag type Zora (%u)", tag);
+        break;
+    default:
+        break;
+    }
+
+    return 0;
+}
+
 static int calculate_bitrate(AVFormatContext *s)
 {
     AVIContext *avi = s->priv_data;
@@ -829,6 +869,11 @@ static int avi_read_header(AVFormatContext *s)
 
                 if (st->codec->extradata_size & 1) //FIXME check if the encoder really did this correctly
                     avio_r8(pb);
+
+                ret = avi_extract_stream_metadata(st);
+                if (ret < 0) {
+                    av_log(s, AV_LOG_WARNING, "could not decoding EXIF data in stream header.\n");
+                }
             }
             break;
         case MKTAG('i', 'n', 'd', 'x'):
-- 
1.8.3.2

-------------- next part --------------
>From 5c2f9136768f6ec79c7d8fdae17967bbe859425a Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Tue, 25 Mar 2014 10:04:52 +0100
Subject: [PATCH 3/3] Reindent after last commit.

---
 libavcodec/exif.c | 2 +-
 libavcodec/exif.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/exif.c b/libavcodec/exif.c
index 9b3e8cb..984adaf 100644
--- a/libavcodec/exif.c
+++ b/libavcodec/exif.c
@@ -108,7 +108,7 @@ static int exif_decode_tag(AVCodecContext *avctx, GetByteContext *gbytes, int le
 
 
 int avpriv_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
-                       int depth, AVDictionary **metadata)
+                           int depth, AVDictionary **metadata)
 {
     int i, ret;
     int entries;
diff --git a/libavcodec/exif.h b/libavcodec/exif.h
index e673dc0..2f509ba 100644
--- a/libavcodec/exif.h
+++ b/libavcodec/exif.h
@@ -165,6 +165,6 @@ static const struct exif_tag tag_list[] = { // JEITA CP-3451 EXIF specification:
 /** Recursively decodes all IFD's and
  *  adds included TAGS into the metadata dictionary. */
 int avpriv_exif_decode_ifd(AVCodecContext *avctx, GetByteContext *gbytes, int le,
-                       int depth, AVDictionary **metadata);
+                           int depth, AVDictionary **metadata);
 
 #endif /* AVCODEC_EXIF_H */
-- 
1.8.3.2



More information about the ffmpeg-devel mailing list