[FFmpeg-devel] [Read EXIF metadata 3/3] Read EXIF metadata in JPEG input.

[Thilo Borgmann]

Am 10.08.13 21:09, schrieb Michael Niedermayer:
> On Sat, Aug 10, 2013 at 01:25:23PM +0200, Thilo Borgmann wrote:
>>
>>>> @@ -215,6 +218,8 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>>>>      int len, nb_components, i, width, height, pix_fmt_id;
>>>>      int h_count[MAX_COMPONENTS];
>>>>      int v_count[MAX_COMPONENTS];
>>>> +    AVDictionary **metadata = avpriv_frame_get_metadatap(s->picture_ptr);
>>>> +    AVDictionary *tmp;
>>>>
>>>>      s->cur_scan = 0;
>>>>      s->upscale_h = s->upscale_v = 0;
>>>> @@ -459,6 +464,10 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>>>>              s->avctx->pix_fmt = AV_PIX_FMT_GRAY16;
>>>>      }
>>>>
>>>> +    // keep metadata alive by hiding AVDictioniary behind *tmp
>>>> +    tmp       = *metadata;
>>>> +    *metadata = NULL;
>>>> +
>>>>      av_frame_unref(s->picture_ptr);
>>>>      if (ff_get_buffer(s->avctx, s->picture_ptr, AV_GET_BUFFER_FLAG_REF) < 0)
>>>>          return -1;
>>>> @@ -466,6 +475,9 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>>>>      s->picture_ptr->key_frame = 1;
>>>>      s->got_picture            = 1;
>>>>
>>>> +    // restore metadata in current buffer
>>>> +    *avpriv_frame_get_metadatap(s->picture_ptr) = tmp;
>>>> +
>>>>      for (i = 0; i < 3; i++)
>>>>          s->linesize[i] = s->picture_ptr->linesize[i] << s->interlaced;
>>>>
>>>
>>> this looks quite odd, metadata should only be stored in the correct
>>> frame, not the old frame
>>
>> Yes a hack. However, my alternative solution does somehow loose
>> the metadata after _decode_frame() is finished and I don't know why.......
>> attached as follow-up patch 4/3 of rev 5 (0004-Remove-hack).
>> So I need another idea or a hint how to fix that alternative.
>>
>>
>>>> +        // read all IFDs and store the metadata
>>>> +        ifds_read = 0;
>>>> +        while ((ifd_offset = ff_exif_decode_ifd(s->avctx, &gbytes, le,
>> avpriv_frame_get_metadatap(s->picture_ptr))) > 0) {
>>>> +            if (++ifds_read > FF_ARRAY_ELEMS(ifd_tags)) {
>>>> +                av_log(s->avctx, AV_LOG_INFO, "mjpeg: found more IFD's in
>> EXIF data than expected. Skipping.\n");
>>>> +                break;
>>>> +            }
>>>> +            bytestream2_seek(&gbytes, ifd_offset, SEEK_SET);
>>>> +        }
>>>
>>> I would assume that offsets always move forward but that theres no
>>> real limit om the number of tags
>>
>> TIFF 6 spec says:
>> "This file offset may point anywhere in the file, even after the image data."
>> So it might also point backwards again. Also, according to EXIF specs, there are
>> exactly these three IFD's defined.
> 
> Theres also possibly an additional problem
> if i understand the tiff spec  correctly
> each IFD represents a seperate picture and each could have
> exif, gps ... data. Throwing all in the same metadata is maybe not
> the best thing to do. Especially if we decode just one picture

Yes this case is untested. Each (sub-)image would have to have its own APP1 tag,
including their corresponding IFDs.

So we could either add up all metadata (like now),
or just use the first APP1's metadata,
or add up all metadata and do not duplicate and/or overwrite existing tags
(avdict can avoid duplicates?)
or extend the whole API to support an array of metadata dictionaries - but do we
support decoding subimages (thumbnails) anyway?

I don't know what the best would be. Maybe we could also add a prefix to the
tags to tell to which image (1..n) they belong...

-Thilo