Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
On 11/24/17, 4:41 PM, "Carl Eugen Hoyos" wrote:
>2017-11-23 4:37 GMT+01:00 :
>> From: Vishwanath Dixit
>> +// either provide codec string for both active streams or for none
>> +if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
>> +acodec[0] = vcodec[0] = '\0';
>> +av_log(NULL, AV_LOG_INFO, "Codec string not available for audio or
>> video stream\n");
>
>What happened to this check?
It has been handled indirectly in the new function.
In the new function will hit the else condition and “goto fail”
+} else {
+goto fail;
+}
regards,
Karthick
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
2017-11-23 4:37 GMT+01:00 :
> From: Vishwanath Dixit
> +// either provide codec string for both active streams or for none
> +if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
> +acodec[0] = vcodec[0] = '\0';
> +av_log(NULL, AV_LOG_INFO, "Codec string not available for audio or
> video stream\n");
What happened to this check?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
Since Vishwanath is on leave today, I have made the changes required and have
sent patchset v2.
On 11/23/17, 4:11 PM, "Carl Eugen Hoyos" wrote:
>2017-11-23 4:37 GMT+01:00 :
>> From: Vishwanath Dixit
>>
>> Signed-off-by: Karthick J
>> ---
>> libavformat/hlsenc.c | 67
>> +++-
>> 1 file changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index 9fed6a3..32246c4 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -1065,6 +1065,52 @@ static int get_relative_url(const char *master_url,
>> const char *media_url,
>> return 0;
>> }
>>
>> +static void get_codec_str(AVStream *vid_st, AVStream *aud_st, char *vcodec,
>> + char *acodec, int vcodec_len, int acodec_len) {
>
>> +if (vcodec_len > 0)
>> +vcodec[0] = '\0';
>> +else
>> +return;
>> +
>> +if (acodec_len > 0)
>> +acodec[0] = '\0';
>> +else
>> +return;
>
>What are these supposed to do?
>Actually: Please add a line below to avoid these.
Made it based on av_malloc inside the function and caller has to free it.
Now the code has become much simpler
>
>> +
>> +if (!vid_st && !aud_st) {
>> +av_log(NULL, AV_LOG_WARNING, "Atleast one stream shoud be valid\n");
>> +return;
>> +}
>
>This looks like the wrong place for such a check.
I have removed the warning, as it is a wrong place.
Retaining the sanity check tough.
>
>> +
>> +if (vid_st && vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
>> +vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
>> +vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
>> +snprintf(vcodec, vcodec_len, "avc1.%02x00%02x",
>> +vid_st->codecpar->profile, vid_st->codecpar->level);
>> +}
>
>The rfc does not specify a string for unknown profile?
Couldn’t find any explicit mention for unknown profile.
https://tools.ietf.org/html/rfc6381
>
>> +
>> +if (aud_st) {
>> +if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
>> +snprintf(acodec, acodec_len, "mp4a.40.33");
>> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
>> +snprintf(acodec, acodec_len, "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 */
>
>Yes.
>Is this the only special case already known?
No. Also we are not setting constrained_set flags of H264 properly. Have added
a TODO there also.
>
>> +snprintf(acodec, acodec_len, "mp4a.40.2");
>> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
>> +snprintf(acodec, acodec_len, "mp4a.A5");
>> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
>> +snprintf(acodec, acodec_len, "mp4a.A6");
>> +}
>> +}
>> +
>> +// either provide codec string for both active streams or for none
>> +if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
>> +acodec[0] = vcodec[0] = '\0';
>> +av_log(NULL, AV_LOG_INFO, "Codec string not available for audio or
>> video stream\n");
>> +}
>
>This needs a context and maybe a higher verbosity.
>
>> +}
>> +
>> static int create_master_playlist(AVFormatContext *s,
>>VariantStream * const input_vs)
>> {
>> @@ -1075,7 +1121,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, vcodec[32], acodec[32];
>
>I suspect you should initialize the first byte here to
>avoid the check on top.
>
>>
>> input_vs->m3u8_created = 1;
>> if (!hls->master_m3u8_created) {
>> @@ -1203,6 +1249,25 @@ static int create_master_playlist(AVFormatContext *s,
>> avio_printf(master_pb, ",RESOLUTION=%dx%d",
>> vid_st->codecpar->width,
>> vid_st->codecpar->height);
>>
>> +get_codec_str(vid_st, aud_st, vcodec, acodec, sizeof(vcodec),
>> + sizeof(acodec));
>
>Imo, use defines instead of sizeof() here.
>
>> +if (strlen(vcodec) || strlen(acodec))
>
>Even if not speed-critical code, checking the first byte
>may be simpler.
>
>Using "{" here simplifies the code below.
>
regards,
Karthick
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
2017-11-23 4:37 GMT+01:00 :
> From: Vishwanath Dixit
>
> Signed-off-by: Karthick J
> ---
> libavformat/hlsenc.c | 67
> +++-
> 1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 9fed6a3..32246c4 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1065,6 +1065,52 @@ static int get_relative_url(const char *master_url,
> const char *media_url,
> return 0;
> }
>
> +static void get_codec_str(AVStream *vid_st, AVStream *aud_st, char *vcodec,
> + char *acodec, int vcodec_len, int acodec_len) {
> +if (vcodec_len > 0)
> +vcodec[0] = '\0';
> +else
> +return;
> +
> +if (acodec_len > 0)
> +acodec[0] = '\0';
> +else
> +return;
What are these supposed to do?
Actually: Please add a line below to avoid these.
> +
> +if (!vid_st && !aud_st) {
> +av_log(NULL, AV_LOG_WARNING, "Atleast one stream shoud be valid\n");
> +return;
> +}
This looks like the wrong place for such a check.
> +
> +if (vid_st && vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
> +vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
> +vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
> +snprintf(vcodec, vcodec_len, "avc1.%02x00%02x",
> +vid_st->codecpar->profile, vid_st->codecpar->level);
> +}
The rfc does not specify a string for unknown profile?
> +
> +if (aud_st) {
> +if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
> +snprintf(acodec, acodec_len, "mp4a.40.33");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
> +snprintf(acodec, acodec_len, "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 */
Yes.
Is this the only special case already known?
> +snprintf(acodec, acodec_len, "mp4a.40.2");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
> +snprintf(acodec, acodec_len, "mp4a.A5");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
> +snprintf(acodec, acodec_len, "mp4a.A6");
> +}
> +}
> +
> +// either provide codec string for both active streams or for none
> +if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
> +acodec[0] = vcodec[0] = '\0';
> +av_log(NULL, AV_LOG_INFO, "Codec string not available for audio or
> video stream\n");
> +}
This needs a context and maybe a higher verbosity.
> +}
> +
> static int create_master_playlist(AVFormatContext *s,
>VariantStream * const input_vs)
> {
> @@ -1075,7 +1121,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, vcodec[32], acodec[32];
I suspect you should initialize the first byte here to
avoid the check on top.
>
> input_vs->m3u8_created = 1;
> if (!hls->master_m3u8_created) {
> @@ -1203,6 +1249,25 @@ static int create_master_playlist(AVFormatContext *s,
> avio_printf(master_pb, ",RESOLUTION=%dx%d",
> vid_st->codecpar->width,
> vid_st->codecpar->height);
>
> +get_codec_str(vid_st, aud_st, vcodec, acodec, sizeof(vcodec),
> + sizeof(acodec));
Imo, use defines instead of sizeof() here.
> +if (strlen(vcodec) || strlen(acodec))
Even if not speed-critical code, checking the first byte
may be simpler.
Using "{" here simplifies the code below.
> +avio_printf(master_pb, ",CODECS=\"");
> +
> +if (strlen(vcodec)) {
> +avio_printf(master_pb, "%s", vcodec);
> +
> +if (strlen(acodec))
> +avio_printf(master_pb, ",");
> +}
> +
> +if (strlen(acodec))
> +avio_printf(master_pb, "%s", acodec);
> +
> +if (strlen(vcodec) || strlen(acodec))
> +avio_printf(master_pb, "\"");
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
>From: Aman Gupta >On Wed, Nov 22, 2017 at 7:38 PM wrote: >>From: Vishwanath Dixit >> >>Signed-off-by: Karthick J > >LGTM. > >Have you looked at adding HEVC support? Thanks for the reply. From our side, there are no immediate plans to add HEVC support. But after this patch, it should be very simple to add the same. Regards, Karthick ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
On Wed, Nov 22, 2017 at 7:38 PM wrote:
> From: Vishwanath Dixit
>
> Signed-off-by: Karthick J
> ---
> libavformat/hlsenc.c | 67
> +++-
> 1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 9fed6a3..32246c4 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1065,6 +1065,52 @@ static int get_relative_url(const char *master_url,
> const char *media_url,
> return 0;
> }
>
> +static void get_codec_str(AVStream *vid_st, AVStream *aud_st, char
> *vcodec,
> + char *acodec, int vcodec_len, int acodec_len) {
> +if (vcodec_len > 0)
> +vcodec[0] = '\0';
> +else
> +return;
> +
> +if (acodec_len > 0)
> +acodec[0] = '\0';
> +else
> +return;
> +
> +if (!vid_st && !aud_st) {
> +av_log(NULL, AV_LOG_WARNING, "Atleast one stream shoud be
> valid\n");
> +return;
> +}
> +
> +if (vid_st && vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
> +vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
> +vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
> +snprintf(vcodec, vcodec_len, "avc1.%02x00%02x",
> +vid_st->codecpar->profile, vid_st->codecpar->level);
> +}
> +
> +if (aud_st) {
> +if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
> +snprintf(acodec, acodec_len, "mp4a.40.33");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
> +snprintf(acodec, acodec_len, "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(acodec, acodec_len, "mp4a.40.2");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
> +snprintf(acodec, acodec_len, "mp4a.A5");
> +} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
> +snprintf(acodec, acodec_len, "mp4a.A6");
> +}
> +}
> +
> +// either provide codec string for both active streams or for none
> +if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
> +acodec[0] = vcodec[0] = '\0';
> +av_log(NULL, AV_LOG_INFO, "Codec string not available for audio
> or video stream\n");
> +}
> +}
> +
> static int create_master_playlist(AVFormatContext *s,
>VariantStream * const input_vs)
> {
> @@ -1075,7 +1121,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, vcodec[32], acodec[32];
>
> input_vs->m3u8_created = 1;
> if (!hls->master_m3u8_created) {
> @@ -1203,6 +1249,25 @@ static int create_master_playlist(AVFormatContext
> *s,
> avio_printf(master_pb, ",RESOLUTION=%dx%d",
> vid_st->codecpar->width,
> vid_st->codecpar->height);
>
> +get_codec_str(vid_st, aud_st, vcodec, acodec, sizeof(vcodec),
> + sizeof(acodec));
> +
> +if (strlen(vcodec) || strlen(acodec))
> +avio_printf(master_pb, ",CODECS=\"");
> +
> +if (strlen(vcodec)) {
> +avio_printf(master_pb, "%s", vcodec);
> +
> +if (strlen(acodec))
> +avio_printf(master_pb, ",");
> +}
> +
> +if (strlen(acodec))
> +avio_printf(master_pb, "%s", acodec);
> +
> +if (strlen(vcodec) || strlen(acodec))
> +avio_printf(master_pb, "\"");
> +
> if (vs->agroup && aud_st)
> avio_printf(master_pb, ",AUDIO=\"group_%s\"", vs->agroup);
LGTM.
Have you looked at adding HEVC support?
>
> --
> 1.9.1
>
> ___
> 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] [PATCH 3/3] avformat/hlsenc:addition of CODECS attribute in the master playlist
From: Vishwanath Dixit
Signed-off-by: Karthick J
---
libavformat/hlsenc.c | 67 +++-
1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 9fed6a3..32246c4 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1065,6 +1065,52 @@ static int get_relative_url(const char *master_url,
const char *media_url,
return 0;
}
+static void get_codec_str(AVStream *vid_st, AVStream *aud_st, char *vcodec,
+ char *acodec, int vcodec_len, int acodec_len) {
+if (vcodec_len > 0)
+vcodec[0] = '\0';
+else
+return;
+
+if (acodec_len > 0)
+acodec[0] = '\0';
+else
+return;
+
+if (!vid_st && !aud_st) {
+av_log(NULL, AV_LOG_WARNING, "Atleast one stream shoud be valid\n");
+return;
+}
+
+if (vid_st && vid_st->codecpar->profile != FF_PROFILE_UNKNOWN &&
+vid_st->codecpar->level != FF_LEVEL_UNKNOWN &&
+vid_st->codecpar->codec_id == AV_CODEC_ID_H264) {
+snprintf(vcodec, vcodec_len, "avc1.%02x00%02x",
+vid_st->codecpar->profile, vid_st->codecpar->level);
+}
+
+if (aud_st) {
+if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP2) {
+snprintf(acodec, acodec_len, "mp4a.40.33");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_MP3) {
+snprintf(acodec, acodec_len, "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(acodec, acodec_len, "mp4a.40.2");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_AC3) {
+snprintf(acodec, acodec_len, "mp4a.A5");
+} else if (aud_st->codecpar->codec_id == AV_CODEC_ID_EAC3) {
+snprintf(acodec, acodec_len, "mp4a.A6");
+}
+}
+
+// either provide codec string for both active streams or for none
+if (vid_st && aud_st && (!strlen(vcodec) || !strlen(acodec))) {
+acodec[0] = vcodec[0] = '\0';
+av_log(NULL, AV_LOG_INFO, "Codec string not available for audio or
video stream\n");
+}
+}
+
static int create_master_playlist(AVFormatContext *s,
VariantStream * const input_vs)
{
@@ -1075,7 +1121,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, vcodec[32], acodec[32];
input_vs->m3u8_created = 1;
if (!hls->master_m3u8_created) {
@@ -1203,6 +1249,25 @@ static int create_master_playlist(AVFormatContext *s,
avio_printf(master_pb, ",RESOLUTION=%dx%d",
vid_st->codecpar->width,
vid_st->codecpar->height);
+get_codec_str(vid_st, aud_st, vcodec, acodec, sizeof(vcodec),
+ sizeof(acodec));
+
+if (strlen(vcodec) || strlen(acodec))
+avio_printf(master_pb, ",CODECS=\"");
+
+if (strlen(vcodec)) {
+avio_printf(master_pb, "%s", vcodec);
+
+if (strlen(acodec))
+avio_printf(master_pb, ",");
+}
+
+if (strlen(acodec))
+avio_printf(master_pb, "%s", acodec);
+
+if (strlen(vcodec) || strlen(acodec))
+avio_printf(master_pb, "\"");
+
if (vs->agroup && aud_st)
avio_printf(master_pb, ",AUDIO=\"group_%s\"", vs->agroup);
--
1.9.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
