Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: Modularized playlist creation to allow reuse

[Jeyapal, Karthick]

>On 11/22/17, 2:18 PM, "Hendrik Leppkes"  wrote:

>>On Wed, Nov 22, 2017 at 9:35 AM, Jeyapal, Karthick  
>>wrote:
>>>On 11/22/17, 1:47 PM, "刘歧"  wrote:
[…]
>>> move the APIs to an C source file, and use ff_ prefix, refer to: 
>>> https://ffmpeg.org/developer.html#toc-Naming-conventions
>> Thanks for the reply.
>> I am not adding it to the API or a global function.
>> Those are just ‘static’ utility functions, to allow code reuse.
>> The scope will be limited only to those files including hlsenc.h.
>> Developer guide states “For file-scope variables and functions declared as 
>> static, no prefix is required”.
>> For example, oggdec.h has two static functions without ff_ prefix.
>>
>
>Regardless, having a bunch of complex functions in a header is not the
>preferred way (because their code gets duplicated), its usually
>reserved for extremely simple functions or special cases. Instead, put
>them into a .c file and give them the ff_ prefix, like most common
>shared code we use.
Thanks for the clarification. 
I have uploaded newer patch set v2 with those functions in .c file 
>
>- Hendrik

regards,
Karthick

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: Modularized playlist creation to allow reuse

[Hendrik Leppkes]

On Wed, Nov 22, 2017 at 9:35 AM, Jeyapal, Karthick  wrote:
>>On 11/22/17, 1:47 PM, "刘歧"  wrote:
>>
>>> 在 2017年11月22日，14:53，Karthick J  写道：
>>>
>>> ---
>>> libavformat/hlsenc.c | 130 +++---
>>> libavformat/hlsenc.h | 158 
>>> +++
>>> 2 files changed, 177 insertions(+), 111 deletions(-)
>>> create mode 100644 libavformat/hlsenc.h
>>>
> […]
>> move the APIs to an C source file, and use ff_ prefix, refer to: 
>> https://ffmpeg.org/developer.html#toc-Naming-conventions
> Thanks for the reply.
> I am not adding it to the API or a global function.
> Those are just ‘static’ utility functions, to allow code reuse.
> The scope will be limited only to those files including hlsenc.h.
> Developer guide states “For file-scope variables and functions declared as 
> static, no prefix is required”.
> For example, oggdec.h has two static functions without ff_ prefix.
>

Regardless, having a bunch of complex functions in a header is not the
preferred way (because their code gets duplicated), its usually
reserved for extremely simple functions or special cases. Instead, put
them into a .c file and give them the ff_ prefix, like most common
shared code we use.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: Modularized playlist creation to allow reuse

[Jeyapal, Karthick]

>On 11/22/17, 1:47 PM, "刘歧"  wrote:
>
>> 在 2017年11月22日，14:53，Karthick J  写道：
>> 
>> ---
>> libavformat/hlsenc.c | 130 +++---
>> libavformat/hlsenc.h | 158 
>> +++
>> 2 files changed, 177 insertions(+), 111 deletions(-)
>> create mode 100644 libavformat/hlsenc.h
>> 
[…]
> move the APIs to an C source file, and use ff_ prefix, refer to: 
> https://ffmpeg.org/developer.html#toc-Naming-conventions
Thanks for the reply.
I am not adding it to the API or a global function. 
Those are just ‘static’ utility functions, to allow code reuse. 
The scope will be limited only to those files including hlsenc.h. 
Developer guide states “For file-scope variables and functions declared as 
static, no prefix is required”.
For example, oggdec.h has two static functions without ff_ prefix.

Regards,
Karthick






___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: Modularized playlist creation to allow reuse

[刘歧]

> 在 2017年11月22日，14:53，Karthick J  写道：
> 
> ---
> libavformat/hlsenc.c | 130 +++---
> libavformat/hlsenc.h | 158 +++
> 2 files changed, 177 insertions(+), 111 deletions(-)
> create mode 100644 libavformat/hlsenc.h
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 3c47ced..4e017eb 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -45,6 +45,7 @@
> 
> #include "avformat.h"
> #include "avio_internal.h"
> +#include "hlsenc.h"
> #include "internal.h"
> #include "os_support.h"
> 
> @@ -73,35 +74,11 @@ typedef struct HLSSegment {
> struct HLSSegment *next;
> } HLSSegment;
> 
> -typedef enum HLSFlags {
> -// Generate a single media file and use byte ranges in the playlist.
> -HLS_SINGLE_FILE = (1 << 0),
> -HLS_DELETE_SEGMENTS = (1 << 1),
> -HLS_ROUND_DURATIONS = (1 << 2),
> -HLS_DISCONT_START = (1 << 3),
> -HLS_OMIT_ENDLIST = (1 << 4),
> -HLS_SPLIT_BY_TIME = (1 << 5),
> -HLS_APPEND_LIST = (1 << 6),
> -HLS_PROGRAM_DATE_TIME = (1 << 7),
> -HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index in 
> segment filenames when use_localtime  e.g.: %%03d
> -HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment 
> duration (microsec) in segment filenames when use_localtime  e.g.: %%09t
> -HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size 
> (bytes) in segment filenames when use_localtime  e.g.: %%014s
> -HLS_TEMP_FILE = (1 << 11),
> -HLS_PERIODIC_REKEY = (1 << 12),
> -} HLSFlags;
> -
> typedef enum {
> SEGMENT_TYPE_MPEGTS,
> SEGMENT_TYPE_FMP4,
> } SegmentType;
> 
> -typedef enum {
> -PLAYLIST_TYPE_NONE,
> -PLAYLIST_TYPE_EVENT,
> -PLAYLIST_TYPE_VOD,
> -PLAYLIST_TYPE_NB,
> -} PlaylistType;
> -
> typedef struct VariantStream {
> unsigned number;
> int64_t sequence;
> @@ -1022,19 +999,6 @@ static void hls_free_segments(HLSSegment *p)
> }
> }
> 
> -static void write_m3u8_head_block(HLSContext *hls, AVIOContext *out, int 
> version,
> -  int target_duration, int64_t sequence)
> -{
> -avio_printf(out, "#EXTM3U\n");
> -avio_printf(out, "#EXT-X-VERSION:%d\n", version);
> -if (hls->allowcache == 0 || hls->allowcache == 1) {
> -avio_printf(out, "#EXT-X-ALLOW-CACHE:%s\n", hls->allowcache == 0 ? 
> "NO" : "YES");
> -}
> -avio_printf(out, "#EXT-X-TARGETDURATION:%d\n", target_duration);
> -avio_printf(out, "#EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
> -av_log(hls, AV_LOG_VERBOSE, "EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", 
> sequence);
> -}
> -
> static void hls_rename_temp_file(AVFormatContext *s, AVFormatContext *oc)
> {
> size_t len = strlen(oc->filename);
> @@ -1101,8 +1065,7 @@ static int create_master_playlist(AVFormatContext *s,
> goto fail;
> }
> 
> -avio_printf(master_pb, "#EXTM3U\n");
> -avio_printf(master_pb, "#EXT-X-VERSION:%d\n", hls->version);
> +hls_write_playlist_version(master_pb, hls->version);
> 
> /* For variant streams with video add #EXT-X-STREAM-INF tag with 
> attributes*/
> for (i = 0; i < hls->nb_varstreams; i++) {
> @@ -1143,18 +1106,7 @@ static int create_master_playlist(AVFormatContext *s,
> bandwidth += aud_st->codecpar->bit_rate;
> bandwidth += bandwidth / 10;
> 
> -if (!bandwidth) {
> -av_log(NULL, AV_LOG_WARNING,
> -"Bandwidth info not available, set audio and video 
> bitrates\n");
> -av_freep(_rel_name);
> -continue;
> -}
> -
> -avio_printf(master_pb, "#EXT-X-STREAM-INF:BANDWIDTH=%d", bandwidth);
> -if (vid_st && vid_st->codecpar->width > 0 && 
> vid_st->codecpar->height > 0)
> -avio_printf(master_pb, ",RESOLUTION=%dx%d", 
> vid_st->codecpar->width,
> -vid_st->codecpar->height);
> -avio_printf(master_pb, "\n%s\n\n", m3U8_rel_name);
> +hls_write_stream_info(vid_st, master_pb, bandwidth, m3U8_rel_name);
> 
> av_freep(_rel_name);
> }
> @@ -1209,12 +1161,8 @@ static int hls_window(AVFormatContext *s, int last, 
> VariantStream *vs)
> }
> 
> vs->discontinuity_set = 0;
> -write_m3u8_head_block(hls, out, hls->version, target_duration, sequence);
> -if (hls->pl_type == PLAYLIST_TYPE_EVENT) {
> -avio_printf(out, "#EXT-X-PLAYLIST-TYPE:EVENT\n");
> -} else if (hls->pl_type == PLAYLIST_TYPE_VOD) {
> -avio_printf(out, "#EXT-X-PLAYLIST-TYPE:VOD\n");
> -}
> +hls_write_playlist_header(out, hls->version, hls->allowcache,
> +  target_duration, sequence, hls->pl_type);
> 
> if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && 
> vs->discontinuity_set==0 ){
> avio_printf(out, "#EXT-X-DISCONTINUITY\n");
> @@ -1231,74 +1179,34 @@ static int hls_window(AVFormatContext *s, int last, 
> 

[FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: Modularized playlist creation to allow reuse

[Karthick J]

---
 libavformat/hlsenc.c | 130 +++---
 libavformat/hlsenc.h | 158 +++
 2 files changed, 177 insertions(+), 111 deletions(-)
 create mode 100644 libavformat/hlsenc.h

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 3c47ced..4e017eb 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -45,6 +45,7 @@
 
 #include "avformat.h"
 #include "avio_internal.h"
+#include "hlsenc.h"
 #include "internal.h"
 #include "os_support.h"
 
@@ -73,35 +74,11 @@ typedef struct HLSSegment {
 struct HLSSegment *next;
 } HLSSegment;
 
-typedef enum HLSFlags {
-// Generate a single media file and use byte ranges in the playlist.
-HLS_SINGLE_FILE = (1 << 0),
-HLS_DELETE_SEGMENTS = (1 << 1),
-HLS_ROUND_DURATIONS = (1 << 2),
-HLS_DISCONT_START = (1 << 3),
-HLS_OMIT_ENDLIST = (1 << 4),
-HLS_SPLIT_BY_TIME = (1 << 5),
-HLS_APPEND_LIST = (1 << 6),
-HLS_PROGRAM_DATE_TIME = (1 << 7),
-HLS_SECOND_LEVEL_SEGMENT_INDEX = (1 << 8), // include segment index in 
segment filenames when use_localtime  e.g.: %%03d
-HLS_SECOND_LEVEL_SEGMENT_DURATION = (1 << 9), // include segment duration 
(microsec) in segment filenames when use_localtime  e.g.: %%09t
-HLS_SECOND_LEVEL_SEGMENT_SIZE = (1 << 10), // include segment size (bytes) 
in segment filenames when use_localtime  e.g.: %%014s
-HLS_TEMP_FILE = (1 << 11),
-HLS_PERIODIC_REKEY = (1 << 12),
-} HLSFlags;
-
 typedef enum {
 SEGMENT_TYPE_MPEGTS,
 SEGMENT_TYPE_FMP4,
 } SegmentType;
 
-typedef enum {
-PLAYLIST_TYPE_NONE,
-PLAYLIST_TYPE_EVENT,
-PLAYLIST_TYPE_VOD,
-PLAYLIST_TYPE_NB,
-} PlaylistType;
-
 typedef struct VariantStream {
 unsigned number;
 int64_t sequence;
@@ -1022,19 +999,6 @@ static void hls_free_segments(HLSSegment *p)
 }
 }
 
-static void write_m3u8_head_block(HLSContext *hls, AVIOContext *out, int 
version,
-  int target_duration, int64_t sequence)
-{
-avio_printf(out, "#EXTM3U\n");
-avio_printf(out, "#EXT-X-VERSION:%d\n", version);
-if (hls->allowcache == 0 || hls->allowcache == 1) {
-avio_printf(out, "#EXT-X-ALLOW-CACHE:%s\n", hls->allowcache == 0 ? 
"NO" : "YES");
-}
-avio_printf(out, "#EXT-X-TARGETDURATION:%d\n", target_duration);
-avio_printf(out, "#EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
-av_log(hls, AV_LOG_VERBOSE, "EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
-}
-
 static void hls_rename_temp_file(AVFormatContext *s, AVFormatContext *oc)
 {
 size_t len = strlen(oc->filename);
@@ -1101,8 +1065,7 @@ static int create_master_playlist(AVFormatContext *s,
 goto fail;
 }
 
-avio_printf(master_pb, "#EXTM3U\n");
-avio_printf(master_pb, "#EXT-X-VERSION:%d\n", hls->version);
+hls_write_playlist_version(master_pb, hls->version);
 
 /* For variant streams with video add #EXT-X-STREAM-INF tag with 
attributes*/
 for (i = 0; i < hls->nb_varstreams; i++) {
@@ -1143,18 +1106,7 @@ static int create_master_playlist(AVFormatContext *s,
 bandwidth += aud_st->codecpar->bit_rate;
 bandwidth += bandwidth / 10;
 
-if (!bandwidth) {
-av_log(NULL, AV_LOG_WARNING,
-"Bandwidth info not available, set audio and video 
bitrates\n");
-av_freep(_rel_name);
-continue;
-}
-
-avio_printf(master_pb, "#EXT-X-STREAM-INF:BANDWIDTH=%d", bandwidth);
-if (vid_st && vid_st->codecpar->width > 0 && vid_st->codecpar->height 
> 0)
-avio_printf(master_pb, ",RESOLUTION=%dx%d", 
vid_st->codecpar->width,
-vid_st->codecpar->height);
-avio_printf(master_pb, "\n%s\n\n", m3U8_rel_name);
+hls_write_stream_info(vid_st, master_pb, bandwidth, m3U8_rel_name);
 
 av_freep(_rel_name);
 }
@@ -1209,12 +1161,8 @@ static int hls_window(AVFormatContext *s, int last, 
VariantStream *vs)
 }
 
 vs->discontinuity_set = 0;
-write_m3u8_head_block(hls, out, hls->version, target_duration, sequence);
-if (hls->pl_type == PLAYLIST_TYPE_EVENT) {
-avio_printf(out, "#EXT-X-PLAYLIST-TYPE:EVENT\n");
-} else if (hls->pl_type == PLAYLIST_TYPE_VOD) {
-avio_printf(out, "#EXT-X-PLAYLIST-TYPE:VOD\n");
-}
+hls_write_playlist_header(out, hls->version, hls->allowcache,
+  target_duration, sequence, hls->pl_type);
 
 if((hls->flags & HLS_DISCONT_START) && sequence==hls->start_sequence && 
vs->discontinuity_set==0 ){
 avio_printf(out, "#EXT-X-DISCONTINUITY\n");
@@ -1231,74 +1179,34 @@ static int hls_window(AVFormatContext *s, int last, 
VariantStream *vs)
 iv_string = en->iv_string;
 }
 
-if (en->discont) {
-avio_printf(out, "#EXT-X-DISCONTINUITY\n");
-}
-
 if ((hls->segment_type == SEGMENT_TYPE_FMP4) && (en == vs->segments)) {
-avio_printf(out,