[FFmpeg-devel] [PATCH] AVI metadata retrieval improvements
Thilo Borgmann
thilo.borgmann at mail.de
Sun Mar 23 22:09:41 CET 2014
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
-------------- next part --------------
>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(-)
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/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 d446774bff35dba75dc7c7d509e64618c2d4e278 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Sun, 23 Mar 2014 22:07:23 +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 | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 0a837d4..7d6b66c 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;
@@ -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) {
if (size<(1<<30)) {
if (ff_get_extradata(st->codec, pb, size) < 0)
return AVERROR(ENOMEM);
@@ -829,6 +870,12 @@ 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 a8e291586c685357dee5d8694703398fa4ddd94b Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Sun, 23 Mar 2014 22:08:32 +0100
Subject: [PATCH 3/3] Reindent after last commit.
---
libavcodec/exif.c | 2 +-
libavcodec/exif.h | 2 +-
libavformat/avidec.c | 20 ++++++++++----------
3 files changed, 12 insertions(+), 12 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 */
diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 7d6b66c..54ce291 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -863,18 +863,18 @@ static int avi_read_header(AVFormatContext *s)
st = s->streams[stream_index];
if (st) {
- if (size<(1<<30)) {
- if (ff_get_extradata(st->codec, pb, size) < 0)
- return AVERROR(ENOMEM);
- }
+ if (size<(1<<30)) {
+ if (ff_get_extradata(st->codec, pb, size) < 0)
+ return AVERROR(ENOMEM);
+ }
- if (st->codec->extradata_size & 1) //FIXME check if the encoder really did this correctly
- avio_r8(pb);
+ 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");
- }
+ 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;
--
1.8.3.2
More information about the ffmpeg-devel
mailing list