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

[Clément Bœsch]

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?

> +            }
>              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."

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140218/7a6d5386/attachment.asc>