[FFmpeg-devel] [PATCH] avformat/hlsenc: start_number new options
Bodecs Bela
bodecsb at vivanet.hu
Tue Jan 10 22:59:45 EET 2017
2017.01.10. 16:40 keltezéssel, Steven Liu írta:
> 2017-01-10 23:22 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>
>>
>> 2017.01.10. 12:10 keltezéssel, Steven Liu írta:
>>
>>> 2017-01-10 17:42 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>>>
>>>
>>>> 2017.01.10. 6:53 keltezéssel, Steven Liu írta:
>>>>
>>>> 2017-01-08 8:22 GMT+08:00 Steven Liu <lingjiujianke at gmail.com>:
>>>>>
>>>>> 2017-01-08 1:37 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>>>>>>
>>>>>> 2017.01.07. 0:32 keltezéssel, Steven Liu írta:
>>>>>>> 2017-01-07 0:47 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>>>>>>>
>>>>>>>> 2017.01.06. 17:33 keltezéssel, Steven Liu írta:
>>>>>>>>
>>>>>>>>> 2017-01-07 0:22 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>>>>>>>>>
>>>>>>>>> 2017.01.06. 16:50 keltezéssel, Steven Liu írta:
>>>>>>>>>> 2017-01-06 22:07 GMT+08:00 Bodecs Bela <bodecsb at vivanet.hu>:
>>>>>>>>>>> Dear All,
>>>>>>>>>>>
>>>>>>>>>>>> in avformat/hlsenc the start_number option starts the playlist
>>>>>>>>>>>>
>>>>>>>>>>>> sequence
>>>>>>>>>>>>> number
>>>>>>>>>>>>> (#EXT-X-MEDIA-SEQUENCE) from the specified number. Unless
>>>>>>>>>>>>> hls_flags
>>>>>>>>>>>>> single_file is set, it also specifies starting sequence numbers
>>>>>>>>>>>>> of
>>>>>>>>>>>>> segment and subtitle filenames. Sometimes it is usefull to have
>>>>>>>>>>>>> unique
>>>>>>>>>>>>> starting numbers at each run, but currently it is only
>>>>>>>>>>>>> achiveable
>>>>>>>>>>>>> by
>>>>>>>>>>>>> setting this parameter manually.
>>>>>>>>>>>>> This patch enables to set start_number parameter automatically
>>>>>>>>>>>>> for
>>>>>>>>>>>>> practically unique numbers. If start_number is set to -1, then
>>>>>>>>>>>>> the start number will be the seconds since epoch (1970-01-01
>>>>>>>>>>>>> 00:00:00).
>>>>>>>>>>>>> If set to -2, then the start number will be based on the current
>>>>>>>>>>>>> date/time value as YYYYmmddHHMMSS. e.g. 20161231235659.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> thank you,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Bela Bodecs
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> ffmpeg-devel mailing list
>>>>>>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Two question:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1. char b[21]; Why this is 21 ?
>>>>>>>>>>>>>
>>>>>>>>>>>>> you are right, 15 is enough.
>>>>>>>>>>>> 2. + {"start_number", "set first number in the sequence",
>>>>>>>>>>>>
>>>>>>>>>>> OFFSET(start_sequence),AV_OPT_TYPE_INT64, {.i64 = 0},
>>>>>>>>>>> -2,
>>>>>>>>>>>
>>>>>>>>>>>> INT64_MAX,
>>>>>>>>>>>> E},
>>>>>>>>>>>> Why is this -2 and the help message maybe need more infomation,
>>>>>>>>>>>> for
>>>>>>>>>>>> example
>>>>>>>>>>>> -2 mean -1 mean 0 mean, and default value.
>>>>>>>>>>>>
>>>>>>>>>>>> yes, I have altered now but I have written verbosly into the doc
>>>>>>>>>>>>
>>>>>>>>>>>> (muxers.texi), here:
>>>>>>>>>>>>
>>>>>>>>>>> +If set to -1, then the start number will be the seconds since
>>>>>>>>>>> epoch
>>>>>>>>>>> (1970-01-01 00:00:00).
>>>>>>>>>>> +If set to -2, then the start number will be based on the current
>>>>>>>>>>> date/time as YYYYmmddHHMMSS. e.g. 20161231235759.
>>>>>>>>>>> +Default value is 0.
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>
>>>>>>>>>>> ffmpeg-devel mailing list
>>>>>>>>>>>
>>>>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>>>>>
>>>>>>>>>>>> I have enclosed a fixed version. A have changed some code, where
>>>>>>>>>>>> greater
>>>>>>>>>>>>
>>>>>>>>>>>> than 32 bit long sequence numbers were not handled correctly.
>>>>>>>>>>>>
>>>>>>>>>>> (av_get_frame_filename2)
>>>>>>>>>>>
>>>>>>>>>>> thank you.
>>>>>>>>>>> Bela Bodecs
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> ffmpeg-devel mailing list
>>>>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> + {"start_number", "set first number in the sequence, 0 is
>>>>>>>>>>> default,
>>>>>>>>>>>
>>>>>>>>>>> -1:
>>>>>>>>>>>
>>>>>>>>>> second since epoch, -2: current datetime as YYYYMMDDhhmmss, actual
>>>>>>>>>> value
>>>>>>>>>> otherwise", OFFSET(start_sequence),AV_OPT_TYPE_INT64, {.i64 = 0},
>>>>>>>>>> -2,
>>>>>>>>>> INT64_MAX, E},
>>>>>>>>>>
>>>>>>>>>> I have check this option, i think add flag to control the
>>>>>>>>>> start_number
>>>>>>>>>> maybe better,
>>>>>>>>>> for example:
>>>>>>>>>> hls_flags
>>>>>>>>>> hls_playlist_type
>>>>>>>>>>
>>>>>>>>>> maybe add a start_number_flags is better, What about you think?
>>>>>>>>>>
>>>>>>>>>> Using hls_flags is not enough to specify different values for them.
>>>>>>>>>>
>>>>>>>>>> NO, i am not mean use hls_flags, i mean you can creat a new flags,
>>>>>>>>> start_number_flags
>>>>>>>> generic
>>>>>>>> epoch
>>>>>>>> datetime
>>>>>>>>
>>>>>>>> Ok, I see it. May I implement it?
>>>>>>>>
>>>>>>> yes, of course ;-)
>>>>>>>
>>>>>> I thought that there should be 3 options beside this start_number
>>>>>>>> option.
>>>>>>>>
>>>>>>>> hls_start_number_playlist, hls_start_number_segment and
>>>>>>>>
>>>>>>>>> hls_start_number_vtt
>>>>>>>>> Using start_number and any of the new 3 ones would be mutualy
>>>>>>>>> exlusive.
>>>>>>>>>
>>>>>>>>> This way anybody could use the old option (start_number) and it
>>>>>>>>> won't
>>>>>>>>> break the current behaviour.
>>>>>>>>> But those who want to have finer control, they may use the new
>>>>>>>>> options.
>>>>>>>>>
>>>>>>>>> of course -start_number x has the same effect as using
>>>>>>>>> -hls_start_number_playlist x -hls_start_number_segment x
>>>>>>>>> -hls_start_number_vtt x
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>>
>>>>>>> ffmpeg-devel mailing list
>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>
>>>>>>>
>>>>>>> Hi Bodecs,
>>>>> If you don't have enough time, i think i can do it together
>>>>> with
>>>>> you;)
>>>>>
>>>>> here it is.
>>>> _______________________________________________
>>>>
>>>>> 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
>>>>
>>>>
>>>> +typedef enum {
>>> + HLS_START_SEQUNCE_AS_START_NUMBER = 0,
>>> + HLS_START_SEQUNCE_AS_SECONDS_SINCE_EPOCH = -1,
>>> + HLS_START_SEQUNCE_AS_FORMATTED_DATETIME = -2, // YYYYMMDDhhmmss
>>> +} StartSequenceSourceType;
>>> is this better?
>>>
>>> + av_log(hls, AV_LOG_VERBOSE, "Found playlist sequence number
>>> was smaller than specified start sequence number: %"PRId64" < %"PRId64",
>>> omitting\n", tmp_sequence, hls->start_sequence);
>>>
>>> this line is too long ,
>>>
>>> Here is the corrected patch. I use only uint numbers in enum.
>>>
>>> + if (hls->start_sequence_source_type ==
>>> HLS_START_SEQUNCE_AS_SECONDS_SINCE_EPOCH ||
>>> hls->start_sequence_source_type
>>> == HLS_START_SEQUNCE_AS_FORMATTED_DATETIME) {
>>> + time_t t = time(NULL); // we will need it in either case
>>> + if (hls->start_sequence_source_type ==
>>> HLS_START_SEQUNCE_AS_SECONDS_SINCE_EPOCH) {
>>> + hls->start_sequence = (int64_t)t;
>>> + } else if (hls->start_sequence_source_type ==
>>> HLS_START_SEQUNCE_AS_FORMATTED_DATETIME) {
>>> + char b[15];
>>> + struct tm *p, tmbuf;
>>> + if (!(p = localtime_r(&t, &tmbuf)))
>>> + return AVERROR(ENOMEM);
>>> + if (!strftime(b, sizeof(b), "%Y%m%d%H%M%S", p))
>>> + return AVERROR(ENOMEM);
>>> + hls->start_sequence = strtoll(b, NULL, 10);
>>> + }
>>> + av_log(hls, AV_LOG_DEBUG, "start_number evaluated to
>>> %"PRId64"\n",
>>> hls->start_sequence);
>>> + }
>>>
>>>
>>> Why twice if here?
>>>
>>>
>>>
>>> patch probe ok,
>>>
>>> My English is poor, and wait English master to review your document :D
>>> _______________________________________________
>>> 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
>>
>>
> LGTM, except documentation.
should I split the patch into two halves: code and doc?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list