Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Dixit, Vishwanath]

>On 12/15/17, 8:19 PM, "Steven Liu"  wrote:
>
>2017-12-15 16:21 GMT+08:00 Jeyapal, Karthick :
>>
>>
>>>On 12/15/17, 4:45 AM, "Liu Steven"  wrote:
>>>
>>>
 在 2017年12月15日，上午6:28，Liu Steven  写道：

>
> 在 2017年12月15日，上午12:29，Jeyapal, Karthick  写道：
>
>> On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
>>
>>
>>> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
>>> […]
>>> +libavformat/reverse.c
>>
>> this need double check for a better way
> The better way of doing this is to share ff_reverse function.
 No I don’t think that is a better way here.
> But such a patch submitted recently was not pushed due to several 
> objections. 
> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
> Hence, we are left with only this option. Moreover like same approach was 
> used for avcodec and avdevice(to add reverse.c), as well. So, extending 
> the same approach for avformat shouldn’t deviate from ffmpeg’s principles.
 I think there have better way, Please don’t define the ffmpeg’s principles 
 to every place. you are duplicate the code from libavcodec//reverse.c to 
 libavformat, IMHO, that is not a good option.
>>>I need some time to think about that.
>> Oh sure. Thanks.
>> In that case, could you please merge PATCH v6 1/3 alone, since that is a 
>> relatively independent feature.
>
>
>patch -p1 < 
>~/Downloads/FFmpeg-devel-v6-1-3-avformat-hlsenc-addition-of-EXT-X-MEDIA-tag-and-AUDIO-attribute.patch
>cd xxx/
>rm -rf *
>../configure --disable-network --disable-everything --enable-muxer=hls
>make
>
>
>CC libavformat/hlsenc.o
>src/libavformat/hlsenc.c:1142:21: error: use of undeclared identifier
>'master_pb'
>avio_printf(master_pb, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"",
>^
>src/libavformat/hlsenc.c:1144:21: error: use of undeclared identifier
>'master_pb'
>avio_printf(master_pb, ",NAME=\"audio_0\",DEFAULT=YES,URI=\"%s\"\n",
>^
>2 errors generated.
>make: *** [libavformat/hlsenc.o] Error 1
>make: *** Waiting for unfinished jobs
The patch needed rebasing since an hlsenc patch ([PATCH 3/3] avformat/hlsenc: 
Extend persistent http connections to playlists) was pushed on 15-dec. I have 
rebased the patch set and submitted with version v7. Could you please merge 
PATCH v7 1/3 (https://patchwork.ffmpeg.org/patch/6857/)

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

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Steven Liu]

2017-12-15 16:21 GMT+08:00 Jeyapal, Karthick :
>
>
>>On 12/15/17, 4:45 AM, "Liu Steven"  wrote:
>>
>>
>>> 在 2017年12月15日，上午6:28，Liu Steven  写道：
>>>

 在 2017年12月15日，上午12:29，Jeyapal, Karthick  写道：

> On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
>
>
>> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
>> […]
>> +libavformat/reverse.c
>
> this need double check for a better way
 The better way of doing this is to share ff_reverse function.
>>> No I don’t think that is a better way here.
 But such a patch submitted recently was not pushed due to several 
 objections. 
 http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
 Hence, we are left with only this option. Moreover like same approach was 
 used for avcodec and avdevice(to add reverse.c), as well. So, extending 
 the same approach for avformat shouldn’t deviate from ffmpeg’s principles.
>>> I think there have better way, Please don’t define the ffmpeg’s principles 
>>> to every place. you are duplicate the code from libavcodec//reverse.c to 
>>> libavformat, IMHO, that is not a good option.
>>I need some time to think about that.
> Oh sure. Thanks.
> In that case, could you please merge PATCH v6 1/3 alone, since that is a 
> relatively independent feature.


patch -p1 < 
~/Downloads/FFmpeg-devel-v6-1-3-avformat-hlsenc-addition-of-EXT-X-MEDIA-tag-and-AUDIO-attribute.patch
cd xxx/
rm -rf *
../configure --disable-network --disable-everything --enable-muxer=hls
make


CC libavformat/hlsenc.o
src/libavformat/hlsenc.c:1142:21: error: use of undeclared identifier
'master_pb'
avio_printf(master_pb, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"",
^
src/libavformat/hlsenc.c:1144:21: error: use of undeclared identifier
'master_pb'
avio_printf(master_pb, ",NAME=\"audio_0\",DEFAULT=YES,URI=\"%s\"\n",
^
2 errors generated.
make: *** [libavformat/hlsenc.o] Error 1
make: *** Waiting for unfinished jobs
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Jeyapal, Karthick]

>On 12/15/17, 4:45 AM, "Liu Steven"  wrote:
>
>
>> 在 2017年12月15日，上午6:28，Liu Steven  写道：
>> 
>>> 
>>> 在 2017年12月15日，上午12:29，Jeyapal, Karthick  写道：
>>> 
 On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
 
 
> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
> […] 
> +libavformat/reverse.c
 
 this need double check for a better way
>>> The better way of doing this is to share ff_reverse function.
>> No I don’t think that is a better way here.
>>> But such a patch submitted recently was not pushed due to several 
>>> objections. 
>>> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
>>> Hence, we are left with only this option. Moreover like same approach was 
>>> used for avcodec and avdevice(to add reverse.c), as well. So, extending the 
>>> same approach for avformat shouldn’t deviate from ffmpeg’s principles.
>> I think there have better way, Please don’t define the ffmpeg’s principles 
>> to every place. you are duplicate the code from libavcodec//reverse.c to 
>> libavformat, IMHO, that is not a good option.
>I need some time to think about that.
Oh sure. Thanks. 
In that case, could you please merge PATCH v6 1/3 alone, since that is a 
relatively independent feature.
>
>Thanks
>
>Steven
>> 
>> Thanks
>> 
>> Steven
>>> 
>>> Regards,
>>> Karthick
>>> 
>>> […]
>>> Thanks
>>> 
>>> 
>>> Steven





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

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Liu Steven]

> 在 2017年12月15日，上午6:28，Liu Steven  写道：
> 
>> 
>> 在 2017年12月15日，上午12:29，Jeyapal, Karthick  写道：
>> 
>>> On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
>>> 
>>> 
 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
 […] 
 +libavformat/reverse.c
>>> 
>>> this need double check for a better way
>> The better way of doing this is to share ff_reverse function.
> No I don’t think that is a better way here.
>> But such a patch submitted recently was not pushed due to several 
>> objections. 
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
>> Hence, we are left with only this option. Moreover like same approach was 
>> used for avcodec and avdevice(to add reverse.c), as well. So, extending the 
>> same approach for avformat shouldn’t deviate from ffmpeg’s principles.
> I think there have better way, Please don’t define the ffmpeg’s principles to 
> every place. you are duplicate the code from libavcodec//reverse.c to 
> libavformat, IMHO, that is not a good option.
I need some time to think about that.

Thanks

Steven
> 
> Thanks
> 
> Steven
>> 
>> Regards,
>> Karthick
>>> 
>>> […]
>>> Thanks
>>> 
>>> 
>>> Steven
>> 
>> 
>> 
>> ___
>> 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



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

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Liu Steven]

> 在 2017年12月15日，上午12:29，Jeyapal, Karthick  写道：
> 
>> On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
>> 
>> 
>>> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
>>> […] 
>>> +libavformat/reverse.c
>> 
>> this need double check for a better way
> The better way of doing this is to share ff_reverse function.
No I don’t think that is a better way here.
> But such a patch submitted recently was not pushed due to several objections. 
> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
> Hence, we are left with only this option. Moreover like same approach was 
> used for avcodec and avdevice(to add reverse.c), as well. So, extending the 
> same approach for avformat shouldn’t deviate from ffmpeg’s principles.
I think there have better way, Please don’t define the ffmpeg’s principles to 
every place. you are duplicate the code from libavcodec//reverse.c to 
libavformat, IMHO, that is not a good option.

Thanks

Steven
> 
> Regards,
> Karthick
>> 
>> […]
>> Thanks
>> 
>> 
>> Steven
> 
> 
> 
> ___
> 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

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Jeyapal, Karthick]

>On 12/14/17, 8:24 PM, "Steven Liu"  wrote:
>
>
>> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
>> […] 
>> +libavformat/reverse.c
>
>this need double check for a better way
The better way of doing this is to share ff_reverse function.
But such a patch submitted recently was not pushed due to several objections. 
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221472.html
Hence, we are left with only this option. Moreover like same approach was used 
for avcodec and avdevice(to add reverse.c), as well. So, extending the same 
approach for avformat shouldn’t deviate from ffmpeg’s principles.

Regards,
Karthick
>
>[…]
>Thanks
>
>
>Steven



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

Re: [FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[Steven Liu]

> 在 2017年12月14日，下午6:55，vdi...@akamai.com 写道：
> 
> From: Vishwanath Dixit 
> 
> ---
> libavformat/Makefile  |  2 +-
> libavformat/dashenc.c |  2 +-
> libavformat/hlsenc.c  | 65 +--
> libavformat/hlsplaylist.c |  5 +++-
> libavformat/hlsplaylist.h |  3 ++-
> libavformat/reverse.c |  1 +
> tests/ref/fate/source |  1 +
> 7 files changed, 73 insertions(+), 6 deletions(-)
> create mode 100644 libavformat/reverse.c
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 734b703..b7e042d 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -61,7 +61,7 @@ OBJS-$(CONFIG_RTPDEC)+= rdt.o   
> \
> rtpdec_vp9.o\
> rtpdec_xiph.o
> OBJS-$(CONFIG_RTPENC_CHAIN)  += rtpenc_chain.o rtp.o
> -OBJS-$(CONFIG_SHARED)+= log2_tab.o golomb_tab.o
> +OBJS-$(CONFIG_SHARED)+= log2_tab.o golomb_tab.o reverse.o
> OBJS-$(CONFIG_SRTP)  += srtp.o
> 
> # muxers/demuxers
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index f363418..016ada3 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -760,7 +760,7 @@ static int write_manifest(AVFormatContext *s, int final)
> AVStream *st = s->streams[i];
> get_hls_playlist_name(playlist_file, sizeof(playlist_file), NULL, 
> i);
> ff_hls_write_stream_info(st, out, st->codecpar->bit_rate,
> -playlist_file, NULL);
> +playlist_file, NULL, NULL);
> }
> avio_close(out);
> if (use_rename)
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 273dd8a..ed64847 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -39,6 +39,7 @@
> #include "libavutil/avstring.h"
> #include "libavutil/intreadwrite.h"
> #include "libavutil/random_seed.h"
> +#include "libavutil/reverse.h"
> #include "libavutil/opt.h"
> #include "libavutil/log.h"
> #include "libavutil/time_internal.h"
> @@ -1078,6 +1079,63 @@ static int get_relative_url(const char *master_url, 
> const char *media_url,
> return 0;
> }
> 
> +static char *get_codec_str(AVStream *vid_st, AVStream *aud_st) {
> +size_t codec_str_size = 64;
> +char *codec_str = av_malloc(codec_str_size);
> +int video_str_len = 0;
> +
> +if (!codec_str)
> +return NULL;
> +
> +if (!vid_st && !aud_st) {
> +goto fail;
> +}
> +
> +if (vid_st) {
> +if (vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
> +vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
> +vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
> +snprintf(codec_str, codec_str_size, "avc1.%02x%02x%02x",
> + vid_st->codecpar->profile & 0xFF,
> + ff_reverse[(vid_st->codecpar->profile >> 8) & 0xFF],
> + vid_st->codecpar->level);
> +} else {
> +goto fail;
> +}
> +video_str_len = strlen(codec_str);
> +}
> +
> +if (aud_st) {
> +char *audio_str = codec_str;
> +if (video_str_len) {
> +codec_str[video_str_len] = ',';
> +video_str_len += 1;
> +audio_str += video_str_len;
> +codec_str_size -= video_str_len;
> +}
> +if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
> +snprintf(audio_str, codec_str_size, "mp4a.40.33");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
> +snprintf(audio_str, codec_str_size, "mp4a.40.34");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AAC) {
> +/* TODO : For HE-AAC, HE-AACv2, the last digit needs to be set 
> to 5 and 29 respectively */
> +snprintf(audio_str, codec_str_size, "mp4a.40.2");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
> +snprintf(audio_str, codec_str_size, "mp4a.A5");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
> +snprintf(audio_str, codec_str_size, "mp4a.A6");
> +} else {
> +goto fail;
> +}
> +}
> +
> +return codec_str;
> +
> +fail:
> +av_free(codec_str);
> +return NULL;
> +}
> +
> static int create_master_playlist(AVFormatContext *s,
>   VariantStream * const input_vs)
> {
> @@ -1088,7 +1146,7 @@ static int create_master_playlist(AVFormatContext *s,
> AVDictionary *options = NULL;
> unsigned int i, j;
> int m3u8_name_size, ret, bandwidth;
> -char *m3u8_rel_name;
> +char *m3u8_rel_name, *codec_str;
> 
> input_vs->m3u8_created = 1;
> if (!hls->master_m3u8_created) {
> @@ -1202,9 +1260,12 @@ static int create_master_playlist(AVFormatContext *s,
> bandwidth += 

[FFmpeg-devel] [PATCH v6 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist

[vdixit]

From: Vishwanath Dixit 

---
 libavformat/Makefile  |  2 +-
 libavformat/dashenc.c |  2 +-
 libavformat/hlsenc.c  | 65 +--
 libavformat/hlsplaylist.c |  5 +++-
 libavformat/hlsplaylist.h |  3 ++-
 libavformat/reverse.c |  1 +
 tests/ref/fate/source |  1 +
 7 files changed, 73 insertions(+), 6 deletions(-)
 create mode 100644 libavformat/reverse.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 734b703..b7e042d 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -61,7 +61,7 @@ OBJS-$(CONFIG_RTPDEC)+= rdt.o 
  \
 rtpdec_vp9.o\
 rtpdec_xiph.o
 OBJS-$(CONFIG_RTPENC_CHAIN)  += rtpenc_chain.o rtp.o
-OBJS-$(CONFIG_SHARED)+= log2_tab.o golomb_tab.o
+OBJS-$(CONFIG_SHARED)+= log2_tab.o golomb_tab.o reverse.o
 OBJS-$(CONFIG_SRTP)  += srtp.o
 
 # muxers/demuxers
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index f363418..016ada3 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -760,7 +760,7 @@ static int write_manifest(AVFormatContext *s, int final)
 AVStream *st = s->streams[i];
 get_hls_playlist_name(playlist_file, sizeof(playlist_file), NULL, 
i);
 ff_hls_write_stream_info(st, out, st->codecpar->bit_rate,
-playlist_file, NULL);
+playlist_file, NULL, NULL);
 }
 avio_close(out);
 if (use_rename)
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 273dd8a..ed64847 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -39,6 +39,7 @@
 #include "libavutil/avstring.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/random_seed.h"
+#include "libavutil/reverse.h"
 #include "libavutil/opt.h"
 #include "libavutil/log.h"
 #include "libavutil/time_internal.h"
@@ -1078,6 +1079,63 @@ static int get_relative_url(const char *master_url, 
const char *media_url,
 return 0;
 }
 
+static char *get_codec_str(AVStream *vid_st, AVStream *aud_st) {
+size_t codec_str_size = 64;
+char *codec_str = av_malloc(codec_str_size);
+int video_str_len = 0;
+
+if (!codec_str)
+return NULL;
+
+if (!vid_st && !aud_st) {
+goto fail;
+}
+
+if (vid_st) {
+if (vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
+vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
+vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
+snprintf(codec_str, codec_str_size, "avc1.%02x%02x%02x",
+ vid_st->codecpar->profile & 0xFF,
+ ff_reverse[(vid_st->codecpar->profile >> 8) & 0xFF],
+ vid_st->codecpar->level);
+} else {
+goto fail;
+}
+video_str_len = strlen(codec_str);
+}
+
+if (aud_st) {
+char *audio_str = codec_str;
+if (video_str_len) {
+codec_str[video_str_len] = ',';
+video_str_len += 1;
+audio_str += video_str_len;
+codec_str_size -= video_str_len;
+}
+if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
+snprintf(audio_str, codec_str_size, "mp4a.40.33");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
+snprintf(audio_str, codec_str_size, "mp4a.40.34");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AAC) {
+/* TODO : For HE-AAC, HE-AACv2, the last digit needs to be set to 
5 and 29 respectively */
+snprintf(audio_str, codec_str_size, "mp4a.40.2");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
+snprintf(audio_str, codec_str_size, "mp4a.A5");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
+snprintf(audio_str, codec_str_size, "mp4a.A6");
+} else {
+goto fail;
+}
+}
+
+return codec_str;
+
+fail:
+av_free(codec_str);
+return NULL;
+}
+
 static int create_master_playlist(AVFormatContext *s,
   VariantStream * const input_vs)
 {
@@ -1088,7 +1146,7 @@ static int create_master_playlist(AVFormatContext *s,
 AVDictionary *options = NULL;
 unsigned int i, j;
 int m3u8_name_size, ret, bandwidth;
-char *m3u8_rel_name;
+char *m3u8_rel_name, *codec_str;
 
 input_vs->m3u8_created = 1;
 if (!hls->master_m3u8_created) {
@@ -1202,9 +1260,12 @@ static int create_master_playlist(AVFormatContext *s,
 bandwidth += aud_st->codecpar->bit_rate;
 bandwidth += bandwidth / 10;
 
+codec_str = get_codec_str(vid_st, aud_st);
+
 ff_hls_write_stream_info(vid_st, master_pb, bandwidth, m3u8_rel_name,
-aud_st ? vs->agroup : NULL);
+codec_str,