[FFmpeg-devel] [PATCH] AVI metadata retrieval improvements
Thilo Borgmann
thilo.borgmann at mail.de
Wed Mar 26 11:28:21 CET 2014
Am 25.03.14 23:56, schrieb Michael Niedermayer:
> On Tue, Mar 25, 2014 at 10:07:24AM +0100, Thilo Borgmann wrote:
>> 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.
>
> [...]
>> 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
>
> micro should be reset to 100 if minor is bumped
Done and attached.
Thanks,
Thilo
-------------- next part --------------
>From 03790973ee8ee7e1d71fd83dfb55a9b3509834be Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Wed, 26 Mar 2014 11:25:00 +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 | 4 ++--
libavcodec/webp.c | 2 +-
5 files changed, 7 insertions(+), 7 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..528c30b 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,8 +29,8 @@
#include "libavutil/version.h"
#define LIBAVCODEC_VERSION_MAJOR 55
-#define LIBAVCODEC_VERSION_MINOR 52
-#define LIBAVCODEC_VERSION_MICRO 102
+#define LIBAVCODEC_VERSION_MINOR 53
+#define LIBAVCODEC_VERSION_MICRO 100
#define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
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 dc9569fa0971b285de78950acea8b26562c4c62e Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Wed, 26 Mar 2014 11:25:40 +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 2afaf637ed795498a1f0f2fe6cdc23e18442c1fb Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Wed, 26 Mar 2014 11:26:08 +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