On 5/12/18 8:47 PM, Jan Ekström wrote:
> On Fri, May 4, 2018 at 9:32 AM, Karthick J <kjeya...@akamai.com> wrote:
>> From: Karthick Jeyapal <kjeya...@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.
>> +
>> +@item dash_segment_type @var{dash_segment_type}
>> +Possible values:
>> +@item mp4
>> +If this flag is set, the dash segment files will be in in ISOBMFF format.
>> This is the default format.
>> +
>> +@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@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
