[FFmpeg-devel] [PATCH 1/3] lavf: add AV_DISPOSITION_TIMED_THUMBNAILS

[Marton Balint]

On Fri, 8 Jan 2016, Rodger Combs wrote:

>
>> On Jan 8, 2016, at 18:30, Marton Balint <cus at passwd.hu> wrote:
>> On Fri, 8 Jan 2016, Rodger Combs wrote:
>> [...]

>>> + * APIC frame in ID3v2). The first (usually only) packet associated with it
>>> + * will be returned among the first few packets read from the file unless
>>> + * seeking takes place. It can also be accessed at any time in
>>> + * AVStream.attached_pic.
>> 
>> Maybe you should clarify that if the stream contains multiple packets, what will happen to AVStream.attached_pic. Will it contain all packets? Or only the first? Or some random?

> This does explain that (though it's awkwardly-worded so I can see why it'd be confusing). It contains the first packet.

Ah, ok, I see.

>>> +/**
>>> + * The stream is sparse, and contains thumbnail images, often corresponding
>>> + * to chapter markers. Only ever used with AV_DISPOSITION_ATTACHED_PIC.
>>> + */
>>> +#define AV_DISPOSITION_TIMED_THUMBNAILS  0x0800
>> 
>> I don't if it is better to use this flag along with attached pic instead of keeping it distinct from it.
>> 
>> Changing the semantics of attached pics streams (multiple packets, timed packets) seems quite a substantial change, so if it were up to me, I'd rather keep this new flag distinct.
>> 
>> Where do you benefit if you use ATTACHED_PIC for this as well?

> My goal there is to get existing consumers not to treat them as "normal" video streams, but I'd be OK with making this distinct instead.

That makes sense too. I don't know, if others have no opinion, it is fine 
by me then as it is.

Thanks,
Marton