[FFmpeg-devel] [PATCH 1/3] avformat: add AV_DISPOSITION_FRAME_BASED flag

[wm4]

On Tue, 18 Feb 2014 21:41:12 +0100
Clément Bœsch <u at pkh.me> wrote:

> On Mon, Feb 17, 2014 at 09:49:34PM +0100, wm4 wrote:
> > This is needed to distinguish subtitles with real timestamps, and
> > subtitles which use frame numbers. Formats like MicroDVD can have both,
> > depending on the file header.
> > ---
> >  doc/APIchanges         | 3 +++
> >  libavformat/avformat.h | 7 +++++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 047b9bd..32532ca 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil:     2012-10-22
> >  
> >  API changes, most recent first:
> >  
> > +2014-02-xx - xxxxxxx - lavf 55.33.xxx - avformat.h
> > +  Add AV_DISPOSITION_FRAME_BASED flag to indicate subtitle timestamp type.
> > +
> >  2014-02-xx - xxxxxxx - lavd 55.10.100 - avdevice.h
> >    Add avdevice_list_devices() and avdevice_free_list_devices()
> >  
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index c990ad6..9491c82 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -725,6 +725,13 @@ typedef struct AVIndexEntry {
> >  #define AV_DISPOSITION_METADATA     0x40000
> >  
> >  /**
> > + * This is a frame-based subtitle format. It does not contain any real
> > + * timestamps, just frame numbers. Returned AVPackets will have the pts field
> > + * set to the frame number, and the stream time_base is a made-up fallback.
> > + */
> > +#define AV_DISPOSITION_FRAME_BASED  0x80000
> > +
> > +/**
> >   * Options for behavior on timestamp wrap detection.
> >   */
> >  #define AV_PTS_WRAP_IGNORE      0   ///< ignore the wrap
> 
> OK so you prefer introducing a new symbol instead of adding a private
> option the microdvd demuxer? (to read the FPS and check if it was set or
> not).

Sure, that could be done too. If we don't find another subtitle format
that does this, maybe it would be better. But if we, I think I'd prefer
something that can be documented in a central place, rather than being
format specific or using a way too "dynamic" API.

(There's generally this weird trend to export obscure or format
specific data as AVOptions, AVDictionary metadata, side-data...)

But I'm fine with either. I can change it to an AVOption. What should I
name it and what values should it take?

> I'm a bit uncomfortable adding a generic flag for just a single format;
> can you find another format in the same situation? (AQTitle?)

I didn't pay AQTitle any attention, because it's an unused (?) format,
and  But I agree it should use whatever new API we introduce, and I can
send a patch for that too. Apparently, AQTitle is always frame-based.

It seems according to mplayer code, MicroDVD and AQTitle are the only
frame-based formats.

> If you can't, I'd really like to have the private option way...
> 
> (If you insist on that solution, can you update the stack of
> PRINT_DISPOSITION() in ffprobe.c?)