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

[Jan Ekström]

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.

> 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;
> +

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