2016-02-08 20:04 GMT+01:00 Stefano Sabatini <stefa...@gmail.com>:
> On date Friday 2016-02-05 11:41:06 +0100, Martin Vignali encoded:
> > 2016-01-30 10:44 GMT+01:00 Stefano Sabatini <stefa...@gmail.com>:
> >
> > > On date Friday 2016-01-29 17:38:13 +0100, Martin Vignali encoded:
> > > > 2016-01-29 12:35 GMT+01:00 Stefano Sabatini <stefa...@gmail.com>:
> > > [...]
> > > > > + 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).
> > > [...]
> > > > 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 ?
> > >
> > > My guess is that timecode is always related to a video stream, right?
> > >
> > > In this case you can loop through the streams and select the first
> > > video stream (check also the select_reference_stream() function in
> > > segment.c).
> > >
> > >
> > I follow your proposal, and search the first video stream, to take fps.
> >
> > In attach the new patch (and a documentation).
> >
> > Best regards
> >
> > Martin
> >
> >
> >
> > ping
>
> > From c266385a63b5f768e484e0aaccfa27de1f870663 Mon Sep 17 00:00:00 2001
> > From: Martin Vignali <martin.vign...@gmail.com>
> > Date: Sat, 30 Jan 2016 20:52:54 +0100
> > Subject: [PATCH 1/2] add increment timecode option using segment v2
>
> Nit: lavf/segment: add increment timecode option
>
> You could describe a use case in the log (optional).
>
> >
> > ---
> > libavformat/segment.c | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/libavformat/segment.c b/libavformat/segment.c
> > index 9da5168..933abd1 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"
> >
> > @@ -94,6 +95,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 to increment timecode if found
> >
> > char *times_str; ///< segment times specification string
> > int64_t *times; ///< list of segment interval specification
> > @@ -223,6 +225,32 @@ 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;
> > + int i;
> > +
> > + if (seg->increment_tc) {
> > + AVDictionaryEntry * tcr = av_dict_get(s->metadata, "timecode",
> NULL, 0);
>
> nit: *tcr
>
> > + if (tcr) {
> > + /* search the first video stream */
> > + for (i = 0; i < s->nb_streams; i++) {
> > + if (s->streams[i]->codec->codec_type ==
> AVMEDIA_TYPE_VIDEO) {
> > + rate = s->streams[i]->avg_frame_rate;/* Get fps
> from the video stream */
> > + err = av_timecode_init_from_string(&tc, rate,
> tcr->value, s);
> > + if (err < 0)
> > + return err;
>
> > + tc.start += (seg->time * rate.num) / (rate.den
> > * 1000000);/* increment timecode */
>
> This can overflow (yes I know you literally used my own
> suggestion). You can use av_mul_q() to avoid that, or cast to floating
> point numbers before the multiplication.
>
> > + char buf[AV_TIMECODE_STR_SIZE];
> > + av_dict_set(&s->metadata, "timecode",
> > + av_timecode_make_string(&tc, buf, 0),
> 0);/* Set the new tc. */
> > + break;
> > + }
> > + }
>
> Please drop the comments in this section, since the code is
> self-evident.
>
> > + }
> > + else{
> > + av_log(s, AV_LOG_ERROR, "can't increment timecode. No
> timecode found");
>
>
> Style nit:
>
> } else {
>
> Also this should be an AV_LOG_WARNING since it's not an hard failure
> (that is, the process will continue to run).
>
> About the message, it could be more descriptive, something like:
> "Could not increment timecode, no timecode metadata found"
>
> > + }
> > + }
> >
> > if (write_header) {
> > avformat_free_context(oc);
> > @@ -946,6 +974,7 @@ static const AVOption options[] = {
> > { "segment_start_number", "set the sequence number of the first
> segment", OFFSET(segment_idx), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, E },
> > { "segment_wrap_number", "set the number of wrap before the first
> segment", OFFSET(segment_idx_wrap_nb), AV_OPT_TYPE_INT, {.i64 = 0}, 0,
> INT_MAX, E },
> > { "strftime", "set filename expansion with strftime at
> segment creation", OFFSET(use_strftime), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
> 1, E },
> > + { "increment_tc", "increment timecode between each
> segment", OFFSET(increment_tc), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 1, E },
> > { "break_non_keyframes", "allow breaking segments on
> non-keyframes", OFFSET(break_non_keyframes), AV_OPT_TYPE_BOOL, {.i64 = 0},
> 0, 1, E },
> >
> > { "individual_header_trailer", "write header/trailer to each
> segment", OFFSET(individual_header_trailer), AV_OPT_TYPE_BOOL, {.i64 = 1},
> 0, 1, E },
> > --
> > 1.9.3 (Apple Git-50)
>
> > From 28844b00c5d80e236abfb719a348fc4be6a202ea Mon Sep 17 00:00:00 2001
> > From: Martin Vignali <martin.vign...@gmail.com>
> > Date: Sat, 30 Jan 2016 20:54:37 +0100
> > Subject: [PATCH 2/2] doc for increment_tc in segment
> >
> > ---
> > doc/muxers.texi | 7 +++++++
> > 1 file changed, 7 insertions(+)
>
> Please merge these two patches (use git rebase -i and squeeze the
> document patch on top of the first one).
>
> >
> > diff --git a/doc/muxers.texi b/doc/muxers.texi
> > index 4ba8883..0a43dfc 100644
> > --- a/doc/muxers.texi
> > +++ b/doc/muxers.texi
> > @@ -997,6 +997,13 @@ implementation for HLS segmentation.
> > The segment muxer supports the following options:
> >
> > @table @option
> > +
> > +@item increment_tc @var{1|0}
>
> > +Use the @code{increment_tc} function to increment timecode
> > +between each segment.
>
> If set to 1, increment timecode between each segment.
>
> > If this is selected, the input need to have
> > +a timecode in the first video stream. Default value is
> > +@code{0}.
> > +
>
> LGTM otherwise.
> --
> FFmpeg = Fierce & Fundamentalist Moronic Prodigious Enhanced Game
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Hello,
Thanks for your comments,
I think i made all the modifications in the patch in attach.
Best regards
Martin
From 8395ed7c66327f754e656622bacb5c2dd91d00ca Mon Sep 17 00:00:00 2001
From: Martin Vignali <martin.vign...@gmail.com>
Date: Sat, 13 Feb 2016 21:46:38 +0100
Subject: [PATCH] lavf/segment: add increment timecode option
for example you can split a file, keeping a continuous timecode between
each segment :
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
---
doc/muxers.texi | 7 +++++++
libavformat/segment.c | 28 ++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 2e6bb4c..21f3525 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1017,6 +1017,13 @@ implementation for HLS segmentation.
The segment muxer supports the following options:
@table @option
+
+@item increment_tc @var{1|0}
+if set to @code{1}, increment timecode between each segment
+If this is selected, the input need to have
+a timecode in the first video stream. Default value is
+@code{0}.
+
@item reference_stream @var{specifier}
Set the reference stream, as specified by the string @var{specifier}.
If @var{specifier} is set to @code{auto}, the reference is chosen
diff --git a/libavformat/segment.c b/libavformat/segment.c
index dd3b092..fc997cc 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -41,6 +41,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"
@@ -95,6 +96,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 to increment timecode if found
char *times_str; ///< segment times specification string
int64_t *times; ///< list of segment interval specification
@@ -227,6 +229,31 @@ 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;
+ int i;
+
+ if (seg->increment_tc) {
+ AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
+ if (tcr) {
+ /* search the first video stream */
+ for (i = 0; i < s->nb_streams; i++) {
+ if (s->streams[i]->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
+ rate = s->streams[i]->avg_frame_rate;/* Get fps from the video stream */
+ err = av_timecode_init_from_string(&tc, rate, tcr->value, s);
+ if (err < 0)
+ return err;
+ tc.start += (int) av_q2d(av_mul_q(av_make_q(seg->time, 1000000 ), rate));/* increment timecode */
+ char buf[AV_TIMECODE_STR_SIZE];
+ av_dict_set(&s->metadata, "timecode",
+ av_timecode_make_string(&tc, buf, 0), 0);
+ break;
+ }
+ }
+ } else {
+ av_log(s, AV_LOG_WARNING, "Could not increment timecode, no timecode metadata found");
+ }
+ }
if (write_header) {
avformat_free_context(oc);
@@ -948,6 +975,7 @@ static const AVOption options[] = {
{ "segment_start_number", "set the sequence number of the first segment", OFFSET(segment_idx), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, E },
{ "segment_wrap_number", "set the number of wrap before the first segment", OFFSET(segment_idx_wrap_nb), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, E },
{ "strftime", "set filename expansion with strftime at segment creation", OFFSET(use_strftime), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, E },
+ { "increment_tc", "increment timecode between each segment", OFFSET(increment_tc), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 1, E },
{ "break_non_keyframes", "allow breaking segments on non-keyframes", OFFSET(break_non_keyframes), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, E },
{ "individual_header_trailer", "write header/trailer to each segment", OFFSET(individual_header_trailer), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, E },
--
1.9.3 (Apple Git-50)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
