[FFmpeg-devel] [PATCH 1/2] avformat/dashenc: Added option for Segment file format

[Jeyapal, Karthick]

On 5/12/18 8:47 PM, Jan Ekström wrote:
> On Fri, May 4, 2018 at 9:32 AM, Karthick J <kjeyapal at akamai.com> wrote:
>> From: Karthick Jeyapal <kjeyapal at akamai.com>
>>
>> Right now segment file format is chosen to be either mp4 or webm based on the codec format.
>> This patch makes that choice configurable by the user, instead of being decided by the muxer.
>> ---
>>  doc/muxers.texi       |  8 ++++++++
>>  libavformat/dashenc.c | 48 ++++++++++++++++++++++--------------------------
>>  2 files changed, 30 insertions(+), 26 deletions(-)
>>
>
> Hi,
>
> Sorry for getting to this so late, been busy on various things (as
> usual). Thanks for prodding me.
Thanks for your reply.
>
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index 6f03bba..2429f8e 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -282,6 +282,14 @@ corrects that index value.
>>  Typically this logic is needed in live streaming use cases. The network bandwidth
>>  fluctuations are common during long run streaming. Each fluctuation can cause
>>  the segment indexes fall behind the expected real time position.
>> +
>> + at item dash_segment_type @var{dash_segment_type}
>> +Possible values:
>> + at item mp4
>> +If this flag is set, the dash segment files will be in in ISOBMFF format. This is the default format.
>> +
>> + at item webm
>> +If this flag is set, the dash segment files will be in in WebM format.
>>  @end table
>>
>>  @anchor{framecrc}
>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>> index 1dd6333..412f074 100644
>> --- a/libavformat/dashenc.c
>> +++ b/libavformat/dashenc.c
>> @@ -48,6 +48,11 @@
>>  #include "vpcc.h"
>>  #include "dash.h"
>>
>> +typedef enum {
>> +    SEGMENT_TYPE_MP4,
>> +    SEGMENT_TYPE_WEBM,
>> +} SegmentType;
>> +
>
I agree with all your comments below. 
Please find the newer patch in http://ffmpeg.org/pipermail/ffmpeg-devel/2018-May/229998.html which tries to address most of them.
> Ah yes, an enum :) I really like the checks being equality/inequality
> now. I've seen things like SEGMENT_TYPE_NB used for the stopper so
> that in the AVOption you can then set the maximum to *_NB - 1 instead
> of then having to change it if it ever gets anything added to it.
>
> Maybe consider making something like the `codecs[]` array for formats
> and make the thing in DASHContext as a char pointer, so that you can
> just point the string pointer to its value in init() instead of doing
> a run-time strncpy.
>
> This does remove the "dynamicness" of  the per-stream selection, which
> possibly should be mentioned. But at least personally I think this is
> what people actually wanted with WebM vs ISOBMFF DASH selection ;) ,
> as in not having surprises between streams.
>
> Otherwise this patch generally looks alright, leaving just the segment
> file name part not automatical just yet :) (I feel like we need to
> have separate options for the general template and the extension).
>
> Best regards,
> Jan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel