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" <ceffm...@gmail.com> wrote:
>2017-11-23 4:37 GMT+01:00 <vdi...@akamai.com>:
>> From: Vishwanath Dixit <vdi...@akamai.com>
>>
>> Signed-off-by: Karthick J <kjeya...@akamai.com>
>> ---
>> 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
