[FFmpeg-devel] [PATCH] dash: change default MP4 extension to .m4s
Alfred E. Heggestad
alfred.heggestad at gmail.com
Mon Jun 24 13:53:41 EEST 2019
On 24/06/2019 11:24, Jeyapal, Karthick wrote:
>
> On 6/20/19 3:00 PM, Alfred E. Heggestad wrote:
>>
>>
>> On 20/06/2019 05:19, Jeyapal, Karthick wrote:
>>>
>>> On 6/19/19 3:08 PM, Alfred E. Heggestad wrote:
>>>> On 19/06/2019 07:21, Jeyapal, Karthick wrote:
>>>>>
>>>>> On 6/18/19 1:48 PM, Alfred E. Heggestad wrote:
>>>>>> On 18/06/2019 04:02, Steven Liu wrote:
>>>>>>> Alfred E. Heggestad <alfred.heggestad at gmail.com> 于2019年6月17日周一 下午4:02写道：
>>>>>>>>
>>>>>>>> From 923da82598bddd1ed05750427dbc71e607d296a2 Mon Sep 17 00:00:00 2001
>>>>>>>> From: "Alfred E. Heggestad" <alfred.heggestad at gmail.com>
>>>>>>>> Date: Mon, 17 Jun 2019 09:59:04 +0200
>>>>>>>> Subject: [PATCH] dash: change default MP4 extension to .m4s
>>>>>>>>
>>>>>>>> this was changed in commit 281a21ed50849e3c8c0d03005230e9fd07c24370
>>>>>>>> ---
>>>>>>>> libavformat/dashenc.c | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>>>>>>> index 3fd7e78166..a51a1da0ca 100644
>>>>>>>> --- a/libavformat/dashenc.c
>>>>>>>> +++ b/libavformat/dashenc.c
>>>>>>>> @@ -166,7 +166,7 @@ static struct format_string {
>>>>>>>> const char *str;
>>>>>>>> } formats[] = {
>>>>>>>> { SEGMENT_TYPE_AUTO, "auto" },
>>>>>>>> - { SEGMENT_TYPE_MP4, "mp4" },
>>>>>>>> + { SEGMENT_TYPE_MP4, "m4s" },
>>>>>>>> { SEGMENT_TYPE_WEBM, "webm" },
>>>>>>>> { 0, NULL }
>>>>>>>> };
>>>>>>>> --
>>>>>>>> 2.20.1 (Apple Git-117)
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> ffmpeg-devel mailing list
>>>>>>>> ffmpeg-devel at ffmpeg.org
>>>>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>>>
>>>>>>>> To unsubscribe, visit link above, or email
>>>>>>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> LGTM
>>>>>>>
>>>>>>
>>>>>> the background for this is the extension for DASH media files
>>>>>> used to be *.m4s and it is now *.mp4
>>>>>>
>>>>>>
>>>>>> the patch is a suggestion and should be checked by the DASH experts
>>>>>>
>>>>>> what is correct according to the standard ?
>>>>>>
>>>>>> the media-file is not really an .mp4 file, it cannot be
>>>>>> played with e.g. ffplay:
>>>>>>
>>>>>> $ ffplay chunk-stream1-00001.m4s
>>>>> Thanks for submitting the patch. I agree that m4s should be extension for media segments.
>>>>> mp4 should be used only for complete files.
>>>>> With respect to the patch, dashenc generates either multiple segments or a single file(with byte range as segments) based on "single_file" option.
>>>>> The default of mp4 is correct when "single_file" is enabled. But it is wrong when "single_file" is disabled. The proposed patch just reverses this situation.
>>>>> I would suggest the patch should handle both cases correctly.
>>>>
>>>> Hi,
>>>>
>>>> many thanks for your review comments.
>>>>
>>>> I have updated the patch based on your comments, please see below.
>>>>
>>>>
>>>> this code works in my application (both single and multi files)
>>>> but the code should be reviewed by someone who has better
>>>> knowledge with the code.
>>> Thanks for sending a revised patch promptly.
>>> I think your patch below might adversely affect the following code
>>> avio_printf(out, "\t\t\t<Representation id=\"%d\" mimeType=\"video/%s\" codecs=\"%s\"%s width=\"%d\" height=\"%d\"",
>>> i, os->format_name, os->codec_str, bandwidth_str, s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
>>>
>>> mimetype will become "video/m4s" for if the file extension is m4s. But I am not sure if it is correct or if such a mimetype exists.
>>> I guess mimetype should remain as "video/mp4" even if the file extension is m4s.
>>> Please let me know your views on this.
>>>
>>
>> I tested with ffmpeg 4.1.3 and the mimetype is video/mp4
>>
>>
>> <Representation id="0" mimeType="video/mp4" codecs="avc1.4d401f" bandwidth="1011342" width="1280" height="720">
>>
>>
>>
>> I made a new patch which is a bit more readable. Please see here:
>>
>>
>>
>> From c90254066e08a8dc46f275fbc2a1d65f26608bd4 Mon Sep 17 00:00:00 2001
>> From: "Alfred E. Heggestad" <alfred.heggestad at gmail.com>
>> Date: Thu, 20 Jun 2019 11:27:53 +0200
>> Subject: [PATCH] dash: split extension for MP4 into .mp4 or .m4s
>>
>> ---
>> libavformat/dashenc.c | 23 ++++++++++++++++++++---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>> index 3fd7e78166..b25afb40aa 100644
>> --- a/libavformat/dashenc.c
>> +++ b/libavformat/dashenc.c
>> @@ -89,6 +89,7 @@ typedef struct OutputStream {
>> int bit_rate;
>> SegmentType segment_type; /* segment type selected for this particular stream */
>> const char *format_name;
>> + const char *extension_name;
>> const char *single_file_name; /* file names selected for this particular stream */
>> const char *init_seg_name;
>> const char *media_seg_name;
>> @@ -217,6 +218,16 @@ static const char *get_format_str(SegmentType segment_type) {
>> return NULL;
>> }
>>
>> +static const char *get_extension_str(SegmentType type, int single_file)
>> +{
>> + switch (type) {
>> +
>> + case SEGMENT_TYPE_MP4: return single_file ? "mp4" : "m4s";
>> + case SEGMENT_TYPE_WEBM: return "webm";
>> + default: return NULL;
>> + }
>> +}
>> +
>> static int handle_io_open_error(AVFormatContext *s, int err, char *url) {
>> DASHContext *c = s->priv_data;
>> char errbuf[AV_ERROR_MAX_STRING_SIZE];
>> @@ -254,6 +265,12 @@ static int init_segment_types(AVFormatContext *s)
>> av_log(s, AV_LOG_ERROR, "Could not select DASH segment type for stream %d\n", i);
>> return AVERROR_MUXER_NOT_FOUND;
>> }
>> + os->extension_name = get_extension_str(segment_type, c->single_file);
>> + if (!os->extension_name) {
>> + av_log(s, AV_LOG_ERROR, "Could not get extension type for stream %d\n", i);
>> + return AVERROR_MUXER_NOT_FOUND;
>> + }
>> +
>> has_mp4_streams |= segment_type == SEGMENT_TYPE_MP4;
>> }
>>
>> @@ -1179,17 +1196,17 @@ static int dash_init(AVFormatContext *s)
>> return AVERROR(ENOMEM);
>>
>> if (c->init_seg_name) {
>> - os->init_seg_name = av_strireplace(c->init_seg_name, "$ext$", os->format_name);
>> + os->init_seg_name = av_strireplace(c->init_seg_name, "$ext$", os->extension_name);
>> if (!os->init_seg_name)
>> return AVERROR(ENOMEM);
>> }
>> if (c->media_seg_name) {
>> - os->media_seg_name = av_strireplace(c->media_seg_name, "$ext$", os->format_name);
>> + os->media_seg_name = av_strireplace(c->media_seg_name, "$ext$", os->extension_name);
>> if (!os->media_seg_name)
>> return AVERROR(ENOMEM);
>> }
>> if (c->single_file_name) {
>> - os->single_file_name = av_strireplace(c->single_file_name, "$ext$", os->format_name);
>> + os->single_file_name = av_strireplace(c->single_file_name, "$ext$", os->extension_name);
>> if (!os->single_file_name)
>> return AVERROR(ENOMEM);
>> }
> Thanks. This new patch looks fine. I wanted to apply it. But I am getting the following error.
>
> Applying: dash: split extension for MP4 into .mp4 or .m4s
> error: patch failed: libavformat/dashenc.c:89
> error: libavformat/dashenc.c: patch does not apply
> Patch failed at 0001 dash: split extension for MP4 into .mp4 or .m4s
>
> Could you please rebase the patch to the latest master and resend it patch as an attachment?
> Also, in this new patch please mention 'dashenc' instead 'dash' in the commit message/subject.
>
Hi Karthick,
here is the rebased patch as an attachment.
please review and apply if it looks okay.
/alfred
-------------- next part --------------
From a4a6bdaed333bc66760e8e359f0807b041ac7977 Mon Sep 17 00:00:00 2001
From: "Alfred E. Heggestad" <alfred.heggestad at gmail.com>
Date: Mon, 24 Jun 2019 12:50:34 +0200
Subject: [PATCH] dashenc: split extension for MP4 into .mp4 or .m4s
---
libavformat/dashenc.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 3fd7e78166..b25afb40aa 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -89,6 +89,7 @@ typedef struct OutputStream {
int bit_rate;
SegmentType segment_type; /* segment type selected for this particular stream */
const char *format_name;
+ const char *extension_name;
const char *single_file_name; /* file names selected for this particular stream */
const char *init_seg_name;
const char *media_seg_name;
@@ -217,6 +218,16 @@ static const char *get_format_str(SegmentType segment_type) {
return NULL;
}
+static const char *get_extension_str(SegmentType type, int single_file)
+{
+ switch (type) {
+
+ case SEGMENT_TYPE_MP4: return single_file ? "mp4" : "m4s";
+ case SEGMENT_TYPE_WEBM: return "webm";
+ default: return NULL;
+ }
+}
+
static int handle_io_open_error(AVFormatContext *s, int err, char *url) {
DASHContext *c = s->priv_data;
char errbuf[AV_ERROR_MAX_STRING_SIZE];
@@ -254,6 +265,12 @@ static int init_segment_types(AVFormatContext *s)
av_log(s, AV_LOG_ERROR, "Could not select DASH segment type for stream %d\n", i);
return AVERROR_MUXER_NOT_FOUND;
}
+ os->extension_name = get_extension_str(segment_type, c->single_file);
+ if (!os->extension_name) {
+ av_log(s, AV_LOG_ERROR, "Could not get extension type for stream %d\n", i);
+ return AVERROR_MUXER_NOT_FOUND;
+ }
+
has_mp4_streams |= segment_type == SEGMENT_TYPE_MP4;
}
@@ -1179,17 +1196,17 @@ static int dash_init(AVFormatContext *s)
return AVERROR(ENOMEM);
if (c->init_seg_name) {
- os->init_seg_name = av_strireplace(c->init_seg_name, "$ext$", os->format_name);
+ os->init_seg_name = av_strireplace(c->init_seg_name, "$ext$", os->extension_name);
if (!os->init_seg_name)
return AVERROR(ENOMEM);
}
if (c->media_seg_name) {
- os->media_seg_name = av_strireplace(c->media_seg_name, "$ext$", os->format_name);
+ os->media_seg_name = av_strireplace(c->media_seg_name, "$ext$", os->extension_name);
if (!os->media_seg_name)
return AVERROR(ENOMEM);
}
if (c->single_file_name) {
- os->single_file_name = av_strireplace(c->single_file_name, "$ext$", os->format_name);
+ os->single_file_name = av_strireplace(c->single_file_name, "$ext$", os->extension_name);
if (!os->single_file_name)
return AVERROR(ENOMEM);
}
--
2.20.1 (Apple Git-117)
More information about the ffmpeg-devel
mailing list