[FFmpeg-devel] [PATCH] AVI metadata retrieval improvements
Michael Niedermayer
michaelni at gmx.at
Sat Apr 5 03:53:00 CEST 2014
On Wed, Mar 26, 2014 at 11:28:21AM +0100, Thilo Borgmann wrote:
> 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.
applied
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140405/7446efcf/attachment.asc>
More information about the ffmpeg-devel
mailing list