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

[Thilo Borgmann]

Am 10.08.13 21:20, schrieb 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.

My fault: one APP1 tag, several IFDs for several subimages.

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