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

[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 ?


[...]
-- 
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/20140325/a77b7ea2/attachment.asc>