Message ID | 20170123110114.10849-1-lq@chinaffmpeg.org |
---|---|
State | Accepted |
Commit | 2f7cc21b61220d205e3c57384f354187417971fb |
Headers | show |
2017.01.23. 12:01 keltezéssel, Steven Liu írta: > Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 85d3955..0170b34 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -47,6 +47,7 @@ typedef enum { > > #define KEYSIZE 16 > #define LINE_BUFFER_SIZE 1024 > +#define HLS_MICROSECOND_UNIT 1000000 > > typedef struct HLSSegment { > char filename[1024]; > @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double > return AVERROR(ENOMEM); > } > if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename), > - filename, 't', (int64_t)round(1000000 * duration)) < 1) { > + filename, 't', (int64_t)round(duration * HLS_MICROSECOND_UNIT)) < 1) { > av_log(hls, AV_LOG_ERROR, > "Invalid second level segment filename template '%s', " > "you can try to remove second_level_segment_time flag\n", I think it is good. bb
On 1/23/2017 8:01 AM, Steven Liu wrote: > Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 85d3955..0170b34 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -47,6 +47,7 @@ typedef enum { > > #define KEYSIZE 16 > #define LINE_BUFFER_SIZE 1024 > +#define HLS_MICROSECOND_UNIT 1000000 You could instead use AV_TIME_BASE. It's more in line with the rest of the codebase. > > typedef struct HLSSegment { > char filename[1024]; > @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double > return AVERROR(ENOMEM); > } > if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename), > - filename, 't', (int64_t)round(1000000 * duration)) < 1) { > + filename, 't', (int64_t)round(duration * HLS_MICROSECOND_UNIT)) < 1) { > av_log(hls, AV_LOG_ERROR, > "Invalid second level segment filename template '%s', " > "you can try to remove second_level_segment_time flag\n", >
2017-01-23 22:14 GMT+08:00 James Almer <jamrial@gmail.com>: > On 1/23/2017 8:01 AM, Steven Liu wrote: > > Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > > --- > > libavformat/hlsenc.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > > index 85d3955..0170b34 100644 > > --- a/libavformat/hlsenc.c > > +++ b/libavformat/hlsenc.c > > @@ -47,6 +47,7 @@ typedef enum { > > > > #define KEYSIZE 16 > > #define LINE_BUFFER_SIZE 1024 > > +#define HLS_MICROSECOND_UNIT 1000000 > > You could instead use AV_TIME_BASE. It's more in line with the rest of the > codebase. > No, the 1st version, Bodecs Bela has point the 1000000 is not AV_TIME_BASE mean. > > > > > typedef struct HLSSegment { > > char filename[1024]; > > @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext > *s, HLSContext *hls, double > > return AVERROR(ENOMEM); > > } > > if (replace_int_data_in_filename(hls->avf->filename, > sizeof(hls->avf->filename), > > - filename, 't', (int64_t)round(1000000 * duration)) < > 1) { > > + filename, 't', (int64_t)round(duration * > HLS_MICROSECOND_UNIT)) < 1) { > > av_log(hls, AV_LOG_ERROR, > > "Invalid second level segment filename template > '%s', " > > "you can try to remove > second_level_segment_time flag\n", > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 1/23/2017 11:25 AM, Steven Liu wrote: > 2017-01-23 22:14 GMT+08:00 James Almer <jamrial@gmail.com>: > >> On 1/23/2017 8:01 AM, Steven Liu wrote: >>> Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> >>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> >>> --- >>> libavformat/hlsenc.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>> index 85d3955..0170b34 100644 >>> --- a/libavformat/hlsenc.c >>> +++ b/libavformat/hlsenc.c >>> @@ -47,6 +47,7 @@ typedef enum { >>> >>> #define KEYSIZE 16 >>> #define LINE_BUFFER_SIZE 1024 >>> +#define HLS_MICROSECOND_UNIT 1000000 >> >> You could instead use AV_TIME_BASE. It's more in line with the rest of the >> codebase. >> > No, the 1st version, Bodecs Bela has point the 1000000 is not AV_TIME_BASE > mean. I'm not sure i understand his reasoning, seeing that AV_TIME_BASE is defined as 1000000 in avutil.h and i don't see it changing anytime soon, but it's just a nit so not really important. This patch is good as is. > >> >>> >>> typedef struct HLSSegment { >>> char filename[1024]; >>> @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext >> *s, HLSContext *hls, double >>> return AVERROR(ENOMEM); >>> } >>> if (replace_int_data_in_filename(hls->avf->filename, >> sizeof(hls->avf->filename), >>> - filename, 't', (int64_t)round(1000000 * duration)) < >> 1) { >>> + filename, 't', (int64_t)round(duration * >> HLS_MICROSECOND_UNIT)) < 1) { >>> av_log(hls, AV_LOG_ERROR, >>> "Invalid second level segment filename template >> '%s', " >>> "you can try to remove >> second_level_segment_time flag\n", >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2017.01.23. 15:55 keltezéssel, James Almer írta: > On 1/23/2017 11:25 AM, Steven Liu wrote: >> 2017-01-23 22:14 GMT+08:00 James Almer <jamrial@gmail.com>: >> >>> On 1/23/2017 8:01 AM, Steven Liu wrote: >>>> Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> >>>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> >>>> --- >>>> libavformat/hlsenc.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>>> index 85d3955..0170b34 100644 >>>> --- a/libavformat/hlsenc.c >>>> +++ b/libavformat/hlsenc.c >>>> @@ -47,6 +47,7 @@ typedef enum { >>>> >>>> #define KEYSIZE 16 >>>> #define LINE_BUFFER_SIZE 1024 >>>> +#define HLS_MICROSECOND_UNIT 1000000 >>> You could instead use AV_TIME_BASE. It's more in line with the rest of the >>> codebase. >>> >> No, the 1st version, Bodecs Bela has point the 1000000 is not AV_TIME_BASE >> mean. > I'm not sure i understand his reasoning, seeing that AV_TIME_BASE is > defined as 1000000 in avutil.h and i don't see it changing anytime soon, > but it's just a nit so not really important. This patch is good as is. I used 1000000 because the duration is in seconds and the replacement should be in microseconds. The filename will contain the duration in microseconds. So this is a "seconds to microseconds" conversion. It would be a mistake to use AV_TIME_BASE here, because i think it is a coincindence that the 2 numbers is the same. We want ot use microseconds in filenames not AV_TIME_BASE units. Maybe a comment should be added to the code about it. The reasoning behind to put duration in microseconds into filename instead of seconds was to avoid comma in filename. > >>>> typedef struct HLSSegment { >>>> char filename[1024]; >>>> @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext >>> *s, HLSContext *hls, double >>>> return AVERROR(ENOMEM); >>>> } >>>> if (replace_int_data_in_filename(hls->avf->filename, >>> sizeof(hls->avf->filename), >>>> - filename, 't', (int64_t)round(1000000 * duration)) < >>> 1) { >>>> + filename, 't', (int64_t)round(duration * >>> HLS_MICROSECOND_UNIT)) < 1) { >>>> av_log(hls, AV_LOG_ERROR, >>>> "Invalid second level segment filename template >>> '%s', " >>>> "you can try to remove >>> second_level_segment_time flag\n", >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, 23 Jan 2017 19:01:14 +0800 Steven Liu <lq@chinaffmpeg.org> wrote: > Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 85d3955..0170b34 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -47,6 +47,7 @@ typedef enum { > > #define KEYSIZE 16 > #define LINE_BUFFER_SIZE 1024 > +#define HLS_MICROSECOND_UNIT 1000000 > > typedef struct HLSSegment { > char filename[1024]; > @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double > return AVERROR(ENOMEM); > } > if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename), > - filename, 't', (int64_t)round(1000000 * duration)) < 1) { > + filename, 't', (int64_t)round(duration * HLS_MICROSECOND_UNIT)) < 1) { > av_log(hls, AV_LOG_ERROR, > "Invalid second level segment filename template '%s', " > "you can try to remove second_level_segment_time flag\n", The commit message could be improved. It's barely English. (I'm not a native speaker either, so I have no suggestion.)
2017-01-23 23:18 GMT+08:00 wm4 <nfxjfg@googlemail.com>: > On Mon, 23 Jan 2017 19:01:14 +0800 > Steven Liu <lq@chinaffmpeg.org> wrote: > > > Reviewed-by: Bodecs Bela <bodecsb@vivanet.hu> > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > > --- > > libavformat/hlsenc.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > > index 85d3955..0170b34 100644 > > --- a/libavformat/hlsenc.c > > +++ b/libavformat/hlsenc.c > > @@ -47,6 +47,7 @@ typedef enum { > > > > #define KEYSIZE 16 > > #define LINE_BUFFER_SIZE 1024 > > +#define HLS_MICROSECOND_UNIT 1000000 > > > > typedef struct HLSSegment { > > char filename[1024]; > > @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext > *s, HLSContext *hls, double > > return AVERROR(ENOMEM); > > } > > if (replace_int_data_in_filename(hls->avf->filename, > sizeof(hls->avf->filename), > > - filename, 't', (int64_t)round(1000000 * duration)) < > 1) { > > + filename, 't', (int64_t)round(duration * > HLS_MICROSECOND_UNIT)) < 1) { > > av_log(hls, AV_LOG_ERROR, > > "Invalid second level segment filename template > '%s', " > > "you can try to remove > second_level_segment_time flag\n", > > The commit message could be improved. It's barely English. (I'm not a > native speaker either, so I have no suggestion.) > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > pushed
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 85d3955..0170b34 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -47,6 +47,7 @@ typedef enum { #define KEYSIZE 16 #define LINE_BUFFER_SIZE 1024 +#define HLS_MICROSECOND_UNIT 1000000 typedef struct HLSSegment { char filename[1024]; @@ -501,7 +502,7 @@ static int hls_append_segment(struct AVFormatContext *s, HLSContext *hls, double return AVERROR(ENOMEM); } if (replace_int_data_in_filename(hls->avf->filename, sizeof(hls->avf->filename), - filename, 't', (int64_t)round(1000000 * duration)) < 1) { + filename, 't', (int64_t)round(duration * HLS_MICROSECOND_UNIT)) < 1) { av_log(hls, AV_LOG_ERROR, "Invalid second level segment filename template '%s', " "you can try to remove second_level_segment_time flag\n",