| Message ID | ca518189-b2c0-9dd3-6305-c87354ed86cd@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2b634e7c23f9e51a5e457985cd116cba5962f552 |
| Headers | show |
On 6/24/19 4:23 PM, Alfred E. Heggestad wrote: > 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@gmail.com> 于2019年6月17日周一 下午4:02写道： >>>>>>>>> >>>>>>>>> From 923da82598bddd1ed05750427dbc71e607d296a2 Mon Sep 17 00:00:00 2001 >>>>>>>>> From: "Alfred E. Heggestad" <alfred.heggestad@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@ffmpeg.org >>>>>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>>>>>> >>>>>>>>> To unsubscribe, visit link above, or email >>>>>>>>> ffmpeg-devel-request@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@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. Thanks. I have applied it. Regards, Karthick > > > > /alfred > >
On 25/06/2019 11:43, Jeyapal, Karthick wrote: > > On 6/24/19 4:23 PM, Alfred E. Heggestad wrote: <snip> >> >> Hi Karthick, >> >> >> here is the rebased patch as an attachment. >> >> >> please review and apply if it looks okay. > > Thanks. I have applied it. > Thanks for applying it :) /alfred
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); }