[FFmpeg-devel] Fwd: libavformat/segment : add option to increment timecode

[Martin Vignali]

2016-01-29 12:35 GMT+01:00 Stefano Sabatini <stefasab at gmail.com>:

> On date Friday 2016-01-29 12:18:02 +0100, Martin Vignali encoded:
> > Ping
> >
> > ---------- Forwarded message ----------
> > From: Martin Vignali <martin.vignali at gmail.com>
> > Date: 2015-10-03 15:49 GMT+02:00
> > Subject: libavformat/segment : add option to increment timecode
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> >
> >
> > Hello,
> >
> > in attach a patch, who add an option to increment timecode using segment.
> >
> > To test :
> >
> > ffmpeg -i src.mov -timecode 10:00:00:00 -vcodec copy -f segment
> > -segment_time 2 -reset_timestamps 1 -increment_tc 1 target_%03d.mov
> >
> > the second file have timecode 10:00:02:00 (after the patch)
> > instead of 10:00:00:00 (before the patch)
> >
> >
> > This patch is useful for two kind of uses :
> > - splitting a file, but keeping timecode of each part
> > - have a continuous timecode when recording a stream using segment.
> >
> >
> > Comments welcome.
> >
> > Martin
>
> > From e53698b08211a818e1fe924a532e142bbb64fc87 Mon Sep 17 00:00:00 2001
> > From: Martin Vignali <martin.vignali at gmail.com>
> > Date: Sat, 3 Oct 2015 15:39:41 +0200
> > Subject: [PATCH] add increment timecode option using segment.
> >
> > ---
> >  libavformat/segment.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
>
> Please add docs in doc/muxers.texi.
>
> > diff --git a/libavformat/segment.c b/libavformat/segment.c
> > index 36417f2..f3e1ab3 100644
> > --- a/libavformat/segment.c
> > +++ b/libavformat/segment.c
> > @@ -40,6 +40,7 @@
> >  #include "libavutil/parseutils.h"
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/time.h"
> > +#include "libavutil/timecode.h"
> >  #include "libavutil/time_internal.h"
> >  #include "libavutil/timestamp.h"
> >
> > @@ -92,6 +93,7 @@ typedef struct SegmentContext {
> >      char *time_str;        ///< segment duration specification string
> >      int64_t time;          ///< segment duration
> >      int use_strftime;      ///< flag to expand filename with strftime
>
> > +    int increment_tc;      ///< flag for increment timecode if found
>
> nit: flag to increment ...
>
> >
> >      char *times_str;       ///< segment times specification string
> >      int64_t *times;        ///< list of segment interval specification
> > @@ -221,6 +223,30 @@ static int segment_start(AVFormatContext *s, int
> write_header)
> >      SegmentContext *seg = s->priv_data;
> >      AVFormatContext *oc = seg->avf;
> >      int err = 0;
> > +    AVTimecode tc;
> > +    AVRational rate;
> > +
>
> > +    if (seg->increment_tc){
>
> style nit: if (...)_{
>
> here and below
>
> > +        AVDictionaryEntry * tcr = av_dict_get(s->metadata, "timecode",
> NULL, 0);
> > +        if (tcr){
> > +            if (s->nb_streams > 0){
>
> > +                rate = s->streams[0]->avg_frame_rate;//Get fps from
> first stream
>
> this looks a bit arbitrary, why the first stream?
>
> > +                err = av_timecode_init_from_string(&tc, rate,
> tcr->value, s);
> > +                if (err < 0)
> > +                    return err;
>
> > +                tc.start += (int)(seg->time / 1000000.) *
> av_q2d(rate);//second count * fps
>
> You're losing precision here. Consider using the rescaling utilities
> in libavutil/mathematics.h or something like (seg->time * rate.num) /
> (rate.den * 1000000).
>
> [...]
> --
> FFmpeg = Frightening & Faithful Mythic Programmable Elastic Gadget
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


Thanks you for your comments, i will make the modifications.

For the fps issue, you're right, i use the first stream because in my tests
it's always the video stream.
Is there a better way, to get fps ? without choosing one of the stream ?

Martin