[FFmpeg-devel] [PATCH] avformat/movenc: fix recognization of cover image streams

[Timo Teras]

On Wed, 13 Jun 2018 17:18:51 +0200
Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:

> 2018-06-13 16:57 GMT+02:00, Timo Teras <timo.teras at iki.fi>:
> > On Tue, 12 Jun 2018 23:09:47 +0200
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >  
> >> On Mon, Jun 04, 2018 at 05:36:19PM +0300, Timo Teräs wrote:  
> >> > For chapter images, the mov demux produces streams with
> >> > disposition set to attached_pic+timed_thumbnails. This patch
> >> > fixes to properly recognize streams that should be encoded as
> >> > cover image (ones with only and only attached_pic disposition
> >> > set).
> >> >
> >> > Signed-off-by: Timo Teräs <timo.teras at iki.fi>
> >> > ---  
> >> > > ffmpeg should act close to what a inteligent user expects.
> >> > > For example a simple ffmpeg -i inputfile outputfile
> >> > > should produce a outputfile that results in similar
> >> > > presentation as the input when played.
> >> > >...
> >> > > It may be good to minimize the amount of exceptions for how
> >> > > streams are handled.  
> >> >
> >> > Agree. My question was more about the details how the disposition
> >> > flags should be handled, because there seems to be no accurate
> >> > documentation. Apparently the logic is to handle based on what
> >> > demuxers produce.
> >> >
> >> > This patch addresses how the cover image is detected, and should
> >> > restore earlier behaviour on copying files where demuxer supports
> >> > more features than current muxer.
> >> >
> >> > This applies on top of the "[v2] avformat/movenc: properly handle
> >> > cover image codecs" patch. I can rebase the order if wanted, but
> >> > it'll require little work.
> >> >
> >> > Hopefully just the above mentioned patch, and this one could be
> >> > applied in the order.  
> >>
> >> will apply the 2 patches
> >>
> >> adding a fate test may make sense, the patches didnt change any so
> >> its not tested  
> >
> > Thanks! Please cherry pick to 4.0-stable too.  
> 
> Doesn't this change behaviour?

Not really. They are bug fixes for the cover image thing introduced in
4.0. They in fact restore pre-4.0 behaviour for certain unusual stream
disposition types.

Technically, the first of these patches exposes another error in the
original implementation, and this second follow up patch fixes that.

I did ask if these two need to be reordered, but since the issue
affected only specific disposition types which are actually never
handled right in current code, it does not really make a big difference.

So over all, these two patches together, do not produce behaviour
difference and only fix/improve things.

Timo