diff mbox

[FFmpeg-devel] fix typo in dashenc

Message ID b26adcb2-7b52-f226-c7f1-93eef44b4ad1@gmail.com
State New
Headers show

Commit Message

Alfred E. Heggestad June 4, 2019, 10:58 a.m. UTC 
Hi Karthick,

thanks for your feedback. here is an updated patch
as you suggested.






 From 7879e39c25ede24693f166eb6a63094757d9fb1e Mon Sep 17 00:00:00 2001
From: "Alfred E. Heggestad" <alfred.heggestad@gmail.com>
Date: Tue, 4 Jun 2019 09:39:59 +0200
Subject: [PATCH] dashenc: fix typo in seg_duration description

doc: fix typo (seconds -> microseconds)
---
  doc/muxers.texi       | 2 +-
  libavformat/dashenc.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

+    { "seg_duration", "segment duration (in microseconds, fractional 
value can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
5000000 }, 0, INT_MAX, E },
      { "remove_at_exit", "remove all segments when finished", 
OFFSET(remove_at_exit), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E },
      { "use_template", "Use SegmentTemplate instead of SegmentList", 
OFFSET(use_template), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
      { "use_timeline", "Use SegmentTimeline in SegmentTemplate", 
OFFSET(use_timeline), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },

Comments

Gyan Doshi June 4, 2019, 12:07 p.m. UTC | #1 
On 04-06-2019 04:28 PM, Alfred E. Heggestad wrote:
> Hi Karthick,
>
> thanks for your feedback. here is an updated patch
> as you suggested.
>
>
>
>
>
>
> From 7879e39c25ede24693f166eb6a63094757d9fb1e Mon Sep 17 00:00:00 2001
> From: "Alfred E. Heggestad" <alfred.heggestad@gmail.com>
> Date: Tue, 4 Jun 2019 09:39:59 +0200
> Subject: [PATCH] dashenc: fix typo in seg_duration description
>
> doc: fix typo (seconds -> microseconds)
> ---
>  doc/muxers.texi       | 2 +-
>  libavformat/dashenc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index c73719c421..be02912b29 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -232,7 +232,7 @@ ffmpeg -re -i <input> -map 0 -map 0 -c:a 
> libfdk_aac -c:v libx264
>  @item -min_seg_duration @var{microseconds}
>  This is a deprecated option to set the segment length in 
> microseconds, use @var{seg_duration} instead.
>  @item -seg_duration @var{duration}
> -Set the segment length in seconds (fractional value can be set). The 
> value is
> +Set the segment length in microseconds (fractional value can be set). 
> The value is
>  treated as average segment duration when @var{use_template} is 
> enabled and
>  @var{use_timeline} is disabled and as minimum segment duration for 
> all the other
>  use cases.
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 94b198ceb8..a70e9d176c 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -1863,7 +1863,7 @@ static const AVOption options[] = {
>  #if FF_API_DASH_MIN_SEG_DURATION
>      { "min_seg_duration", "minimum segment duration (in microseconds) 
> (will be deprecated)", OFFSET(min_seg_duration), AV_OPT_TYPE_INT, { 
> .i64 = 5000000 }, 0, INT_MAX, E },
>  #endif
> -    { "seg_duration", "segment duration (in seconds, fractional value 
> can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
> 5000000 }, 0, INT_MAX, E },
> +    { "seg_duration", "segment duration (in microseconds, fractional 
> value can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 
> = 5000000 }, 0, INT_MAX, E },
>      { "remove_at_exit", "remove all segments when finished", 
> OFFSET(remove_at_exit), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E },
>      { "use_template", "Use SegmentTemplate instead of SegmentList", 
> OFFSET(use_template), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
>      { "use_timeline", "Use SegmentTimeline in SegmentTemplate", 
> OFFSET(use_timeline), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },

This doesn't look right.

I just tested ffmpeg cli with -seg_duration 4 and -seg_duration 4000000. 
The latter failed with "Value 4000000.000000 for parameter 
'seg_duration' out of range [0 - 2147.48]" and the former produced 
segments of duration 4 seconds each.

The field type is AV_OPT_TYPE_DURATION, and the avutil parser returns a 
microsecond value to the caller, but it expects the user string to be 
either HH:MM:SS or MM:SS or S+. Millisecond and microsecond values can 
be fed, with a suffix of ms or us, respectively.

Gyan
Alfred E. Heggestad June 4, 2019, 12:59 p.m. UTC | #2 
On 04/06/2019 14:07, Gyan wrote:
> 
> 
> On 04-06-2019 04:28 PM, Alfred E. Heggestad wrote:
>> Hi Karthick,
>>
>> thanks for your feedback. here is an updated patch
>> as you suggested.
>>
>>
>>
>>
>>
>>
>> From 7879e39c25ede24693f166eb6a63094757d9fb1e Mon Sep 17 00:00:00 2001
>> From: "Alfred E. Heggestad" <alfred.heggestad@gmail.com>
>> Date: Tue, 4 Jun 2019 09:39:59 +0200
>> Subject: [PATCH] dashenc: fix typo in seg_duration description
>>
>> doc: fix typo (seconds -> microseconds)
>> ---
>>  doc/muxers.texi       | 2 +-
>>  libavformat/dashenc.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index c73719c421..be02912b29 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -232,7 +232,7 @@ ffmpeg -re -i <input> -map 0 -map 0 -c:a 
>> libfdk_aac -c:v libx264
>>  @item -min_seg_duration @var{microseconds}
>>  This is a deprecated option to set the segment length in 
>> microseconds, use @var{seg_duration} instead.
>>  @item -seg_duration @var{duration}
>> -Set the segment length in seconds (fractional value can be set). The 
>> value is
>> +Set the segment length in microseconds (fractional value can be set). 
>> The value is
>>  treated as average segment duration when @var{use_template} is 
>> enabled and
>>  @var{use_timeline} is disabled and as minimum segment duration for 
>> all the other
>>  use cases.
>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>> index 94b198ceb8..a70e9d176c 100644
>> --- a/libavformat/dashenc.c
>> +++ b/libavformat/dashenc.c
>> @@ -1863,7 +1863,7 @@ static const AVOption options[] = {
>>  #if FF_API_DASH_MIN_SEG_DURATION
>>      { "min_seg_duration", "minimum segment duration (in microseconds) 
>> (will be deprecated)", OFFSET(min_seg_duration), AV_OPT_TYPE_INT, { 
>> .i64 = 5000000 }, 0, INT_MAX, E },
>>  #endif
>> -    { "seg_duration", "segment duration (in seconds, fractional value 
>> can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
>> 5000000 }, 0, INT_MAX, E },
>> +    { "seg_duration", "segment duration (in microseconds, fractional 
>> value can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 
>> = 5000000 }, 0, INT_MAX, E },
>>      { "remove_at_exit", "remove all segments when finished", 
>> OFFSET(remove_at_exit), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E },
>>      { "use_template", "Use SegmentTemplate instead of SegmentList", 
>> OFFSET(use_template), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
>>      { "use_timeline", "Use SegmentTimeline in SegmentTemplate", 
>> OFFSET(use_timeline), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
> 
> This doesn't look right.
> 
> I just tested ffmpeg cli with -seg_duration 4 and -seg_duration 4000000. 
> The latter failed with "Value 4000000.000000 for parameter 
> 'seg_duration' out of range [0 - 2147.48]" and the former produced 
> segments of duration 4 seconds each.
> 
> The field type is AV_OPT_TYPE_DURATION, and the avutil parser returns a 
> microsecond value to the caller, but it expects the user string to be 
> either HH:MM:SS or MM:SS or S+. Millisecond and microsecond values can 
> be fed, with a suffix of ms or us, respectively.
> 


this code is working for me:

	ret |= av_opt_set_int(obj, "seg_duration",  10000000, 0);



if you look in the declarations of options in dashenc.c:

#if FF_API_DASH_MIN_SEG_DURATION
     { "min_seg_duration", "minimum segment duration (in microseconds) 
(will be deprecated)", OFFSET(min_seg_duration), AV_OPT_TYPE_INT, { .i64 
= 5000000 }, 0, INT_MAX, E },
#endif
     { "seg_duration", "segment duration (in seconds, fractional value 
can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
5000000 }, 0, INT_MAX, E },


see the default value:

     .i64 = 5000000




/alfred
Gyan Doshi June 4, 2019, 1:09 p.m. UTC | #3 
On 04-06-2019 06:29 PM, Alfred E. Heggestad wrote:
> On 04/06/2019 14:07, Gyan wrote:
>>
>>
>> On 04-06-2019 04:28 PM, Alfred E. Heggestad wrote:
>>> Hi Karthick,
>>>
>>> thanks for your feedback. here is an updated patch
>>> as you suggested.
>>>
>>>
>>>
>>>
>>>
>>>
>>> From 7879e39c25ede24693f166eb6a63094757d9fb1e Mon Sep 17 00:00:00 2001
>>> From: "Alfred E. Heggestad" <alfred.heggestad@gmail.com>
>>> Date: Tue, 4 Jun 2019 09:39:59 +0200
>>> Subject: [PATCH] dashenc: fix typo in seg_duration description
>>>
>>> doc: fix typo (seconds -> microseconds)
>>> ---
>>>  doc/muxers.texi       | 2 +-
>>>  libavformat/dashenc.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>>> index c73719c421..be02912b29 100644
>>> --- a/doc/muxers.texi
>>> +++ b/doc/muxers.texi
>>> @@ -232,7 +232,7 @@ ffmpeg -re -i <input> -map 0 -map 0 -c:a 
>>> libfdk_aac -c:v libx264
>>>  @item -min_seg_duration @var{microseconds}
>>>  This is a deprecated option to set the segment length in 
>>> microseconds, use @var{seg_duration} instead.
>>>  @item -seg_duration @var{duration}
>>> -Set the segment length in seconds (fractional value can be set). 
>>> The value is
>>> +Set the segment length in microseconds (fractional value can be 
>>> set). The value is
>>>  treated as average segment duration when @var{use_template} is 
>>> enabled and
>>>  @var{use_timeline} is disabled and as minimum segment duration for 
>>> all the other
>>>  use cases.
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index 94b198ceb8..a70e9d176c 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -1863,7 +1863,7 @@ static const AVOption options[] = {
>>>  #if FF_API_DASH_MIN_SEG_DURATION
>>>      { "min_seg_duration", "minimum segment duration (in 
>>> microseconds) (will be deprecated)", OFFSET(min_seg_duration), 
>>> AV_OPT_TYPE_INT, { .i64 = 5000000 }, 0, INT_MAX, E },
>>>  #endif
>>> -    { "seg_duration", "segment duration (in seconds, fractional 
>>> value can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { 
>>> .i64 = 5000000 }, 0, INT_MAX, E },
>>> +    { "seg_duration", "segment duration (in microseconds, 
>>> fractional value can be set)", OFFSET(seg_duration), 
>>> AV_OPT_TYPE_DURATION, { .i64 = 5000000 }, 0, INT_MAX, E },
>>>      { "remove_at_exit", "remove all segments when finished", 
>>> OFFSET(remove_at_exit), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, E },
>>>      { "use_template", "Use SegmentTemplate instead of SegmentList", 
>>> OFFSET(use_template), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
>>>      { "use_timeline", "Use SegmentTimeline in SegmentTemplate", 
>>> OFFSET(use_timeline), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, E },
>>
>> This doesn't look right.
>>
>> I just tested ffmpeg cli with -seg_duration 4 and -seg_duration 
>> 4000000. The latter failed with "Value 4000000.000000 for parameter 
>> 'seg_duration' out of range [0 - 2147.48]" and the former produced 
>> segments of duration 4 seconds each.
>>
>> The field type is AV_OPT_TYPE_DURATION, and the avutil parser returns 
>> a microsecond value to the caller, but it expects the user string to 
>> be either HH:MM:SS or MM:SS or S+. Millisecond and microsecond values 
>> can be fed, with a suffix of ms or us, respectively.
>>
>
>
> this code is working for me:
>
>     ret |= av_opt_set_int(obj, "seg_duration",  10000000, 0);
>
>
>
> if you look in the declarations of options in dashenc.c:
>
> #if FF_API_DASH_MIN_SEG_DURATION
>     { "min_seg_duration", "minimum segment duration (in microseconds) 
> (will be deprecated)", OFFSET(min_seg_duration), AV_OPT_TYPE_INT, { 
> .i64 = 5000000 }, 0, INT_MAX, E },
> #endif
>     { "seg_duration", "segment duration (in seconds, fractional value 
> can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
> 5000000 }, 0, INT_MAX, E },
>
>
> see the default value:
>
>     .i64 = 5000000

You should be using av_opt_set(), which will parse the value as per its 
type, which is AV_OPT_TYPE_DURATION and not AV_OPT_TYPE_INT. There is no 
specific setter for DURATION type.

Gyan
diff mbox

Patch

diff --git a/doc/muxers.texi b/doc/muxers.texi
index c73719c421..be02912b29 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -232,7 +232,7 @@  ffmpeg -re -i <input> -map 0 -map 0 -c:a libfdk_aac 
-c:v libx264
  @item -min_seg_duration @var{microseconds}
  This is a deprecated option to set the segment length in microseconds, 
use @var{seg_duration} instead.
  @item -seg_duration @var{duration}
-Set the segment length in seconds (fractional value can be set). The 
value is
+Set the segment length in microseconds (fractional value can be set). 
The value is
  treated as average segment duration when @var{use_template} is enabled and
  @var{use_timeline} is disabled and as minimum segment duration for all 
the other
  use cases.
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 94b198ceb8..a70e9d176c 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1863,7 +1863,7 @@  static const AVOption options[] = {
  #if FF_API_DASH_MIN_SEG_DURATION
      { "min_seg_duration", "minimum segment duration (in microseconds) 
(will be deprecated)", OFFSET(min_seg_duration), AV_OPT_TYPE_INT, { .i64 
= 5000000 }, 0, INT_MAX, E },
  #endif
-    { "seg_duration", "segment duration (in seconds, fractional value 
can be set)", OFFSET(seg_duration), AV_OPT_TYPE_DURATION, { .i64 = 
5000000 }, 0, INT_MAX, E },