[FFmpeg-devel] [PATCH 2/3] avformat/microdvd: set AV_DISPOSITION_FRAME_BASED

wm4 nfxjfg at googlemail.com
Tue Feb 18 21:58:21 CET 2014


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

> On Mon, Feb 17, 2014 at 09:49:35PM +0100, wm4 wrote:
> > ---
> >  libavformat/microdvddec.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> 
> See comment in previous patch.
> 
> > diff --git a/libavformat/microdvddec.c b/libavformat/microdvddec.c
> > index 7b49e43..51f7f48 100644
> > --- a/libavformat/microdvddec.c
> > +++ b/libavformat/microdvddec.c
> > @@ -78,6 +78,7 @@ static int microdvd_read_header(AVFormatContext *s)
> >      AVStream *st = avformat_new_stream(s, NULL);
> >      int i = 0;
> >      char line[MAX_LINESIZE];
> > +    int has_real_fps = 0;
> >  
> >      if (!st)
> >          return AVERROR(ENOMEM);
> > @@ -98,8 +99,10 @@ static int microdvd_read_header(AVFormatContext *s)
> >  
> >              if ((sscanf(line, "{%d}{}%6lf",    &frame, &fps) == 2 ||
> >                   sscanf(line, "{%d}{%*d}%6lf", &frame, &fps) == 2)
> > -                && frame <= 1 && fps > 3 && fps < 100)
> > +                && frame <= 1 && fps > 3 && fps < 100) {
> >                  pts_info = av_d2q(fps, 100000);
> > +                has_real_fps = 1;
> 
> Note: btw, didn't you want to ignore the fps line from appearing in the
> subtitle events?

Actually, this happened because the BOM messed with line parsing in
mysterious ways, so the timestamps weren't read correctly. With my
other patch applied, it's parsed as subtitle event with duration 0.

I'll add another patch to skip the event.

> > +            }
> >              if (!st->codec->extradata && sscanf(line, "{DEFAULT}{}%c", &c) == 1) {
> >                  st->codec->extradata = av_strdup(line + 11);
> >                  if (!st->codec->extradata)
> > @@ -131,6 +134,8 @@ static int microdvd_read_header(AVFormatContext *s)
> >      avpriv_set_pts_info(st, 64, pts_info.den, pts_info.num);
> >      st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> >      st->codec->codec_id   = AV_CODEC_ID_MICRODVD;
> > +    if (!has_real_fps)
> > +        st->disposition |= AV_DISPOSITION_FRAME_BASED;
> 
> Whatever the solution, I think a comment would be welcome somewhere in the
> code (maybe in an additional commit?). I'm probably not very smart, but
> it's not obvious what you are trying to solve. Maybe something like:
> 
> "If no FPS are specified in the file, applications should ignore the
> stream time base and consider the PTS as frame number. Otherwise, it will
> only work if the video is at the same FPS as the default time base.  On
> the other hand, if the FPS are specified in that microdvd sub file, the
> pts should be honored using the stream time base, just like any other time
> base subtitle format."

This looks good. Can I copy and paste it? And where should I insert it?


More information about the ffmpeg-devel mailing list