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

Thilo Borgmann thilo.borgmann at mail.de
Thu Apr 3 13:49:24 CEST 2014


Am 26.03.14 11:28, schrieb Thilo Borgmann:
> 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.

Ping.

-Thilo



More information about the ffmpeg-devel mailing list