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

[Thilo Borgmann]

Am 10.08.13 21:28, schrieb Michael Niedermayer:
> On Sat, Aug 10, 2013 at 09:22:47PM +0200, Thilo Borgmann wrote:
>> 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.
> 
> yes, and i guess until we have a testcase it doesnt matter much but
> naively id say if we ignore image 2..N then we also should ignore their
> exif data
> 
> OTOH if we decode each (to its own AVFrame) then each should have its
> EXIF correctly associated with it

subimage metadata silently ignored in rev 6.

-Thilo