[FFmpeg-devel] [PATCH v2] avformat/hlsenc: improve the code readable of 1000000

[Bodecs Bela]

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 at gmail.com>:
>>
>>> On 1/23/2017 8:01 AM, Steven Liu wrote:
>>>> Reviewed-by: Bodecs Bela <bodecsb at vivanet.hu>
>>>> Signed-off-by: Steven Liu <lq at 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 at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel