[FFmpeg-devel] [Read EXIF metadata 1/3] Refactor TIFF tag related functions to share the code.

[Thilo Borgmann]

Am 05.08.13 13:39, schrieb Michael Niedermayer:
> On Fri, Aug 02, 2013 at 10:15:05PM +0200, Thilo Borgmann wrote:
>> Updated Patch according to latest comments.
>>
>> -Thilo
> 
> [...]
>> @@ -702,10 +546,7 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
>>      uint32_t *pal;
>>      double *dp;
>>  
>> -    tag   = tget_short(&s->gb, s->le);
>> -    type  = tget_short(&s->gb, s->le);
>> -    count = tget_long(&s->gb, s->le);
>> -    off   = tget_long(&s->gb, s->le);
>> +    ff_tread_tag_data(&s->gb, s->le, &tag, &type, &count, &off);
>>      start = bytestream2_tell(&s->gb);
>>  
>>      if (type == 0 || type >= FF_ARRAY_ELEMS(type_sizes)) {
>> @@ -714,30 +555,20 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
>>          return 0;
>>      }
>>  
>> +    ff_tseek_tag_value(&s->gb, type, count, off);
>>      if (count == 1) {
>>          switch (type) {
>>          case TIFF_BYTE:
>>          case TIFF_SHORT:
>> -            bytestream2_seek(&s->gb, -4, SEEK_CUR);
>> -            value = tget(&s->gb, type, s->le);
>> +            value = ff_tget(&s->gb, type, s->le);
>>              break;
>>          case TIFF_LONG:
>>              value = off;
>>              break;
>> -        case TIFF_STRING:
>> -            if (count <= 4) {
>> -                bytestream2_seek(&s->gb, -4, SEEK_CUR);
>> -                break;
>> -            }
>>          default:
>>              value = UINT_MAX;
>>              bytestream2_seek(&s->gb, off, SEEK_SET);
>>          }
>> -    } else {
>> -        if (count <= 4 && type_sizes[type] * count <= 4)
>> -            bytestream2_seek(&s->gb, -4, SEEK_CUR);
>> -        else
>> -            bytestream2_seek(&s->gb, off, SEEK_SET);
>>      }
>>  
>>      switch (tag) {
> 
> this should be factored differently.
> what the change does is having a function read some values and
> then having a 2nd function (ff_tseek_tag_value) check if it was
> wrong, seek back and then have code outside the functions read the
> correct values.
> I think there should be no seek back needed and all the reading and
> skiping/seeking should be in the same function if possible

send rev 2.

For tiff.c there still is a seek back - changing this would require extensive
testing not to introduce bugs in the huge switch(tag) {...}. (What I cannot do
because I lack a lot of examples covering all these tags)

-Thilo