Message ID | 20170426112759.12130-1-mfojtak@seznam.cz |
---|---|
State | New |
Headers | show |
On 4/26/2017 4:27 AM, mfojtak wrote: > --- > libavformat/dashenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > index 6232c70..fe1d6c2 100644 > --- a/libavformat/dashenc.c > +++ b/libavformat/dashenc.c > @@ -433,7 +433,7 @@ static void format_date_now(char *buf, int size) > struct tm *ptm, tmbuf; > ptm = gmtime_r(&t, &tmbuf); > if (ptm) { > - if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S", ptm)) > + if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%SZ", ptm)) > buf[0] = '\0'; > } > } > This change appears to be correct. I wasn't previously knowledgeable about the 'Z' suffix, but I looked into it and it is documented in ISO 8601 (https://en.wikipedia.org/wiki/ISO_8601 and also http://www.ecma-international.org/ecma-262/5.1/#sec-15.9.1.15 ). On a separate note, the actual format is: YYYY-MM-DDTHH:mm:ss.sssZ . The ".sss" part is missing from this implementation, which represents milliseconds. According to the specification, ".sss" may be absent, but maybe it would work with even more players if it were included. Technically, the specification states that an absent time-zone offset should be treated as 'Z', which indicates that the code was already compliant without the 'Z', even if it didn't work with most players. strftime() doesn't handle milliseconds, but perhaps it ought to populate milliseconds anyway as follows: if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S.000Z", ptm)) Aaron Levinson
Currently this muxer does not work at all. I don't know if 000Z would make it compatible with more player as I don't know any. However, adding Z makes it compatible with most popular ones like dash.js and shaka. Having this muxer working properly would bring more attention to it and maybe in the future somebody tests it with more players. But for now I suggest to roll out the change and "unblock" this muxer for some real wold use. Also, it would be great to make this muxer codec and container agnostic as it works with h264 and mp4 only. But again, nobody would bother if the muxer doesn't work at all with browsers. ---------- Původní e-mail ---------- Od: Aaron Levinson <alevinsn@aracnet.com> Komu: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Datum: 30. 4. 2017 20:47:59 Předmět: Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (dashj.js, shaka, VLC, etc.) "On 4/26/2017 4:27 AM, mfojtak wrote: > --- > libavformat/dashenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > index 6232c70..fe1d6c2 100644 > --- a/libavformat/dashenc.c > +++ b/libavformat/dashenc.c > @@ -433,7 +433,7 @@ static void format_date_now(char *buf, int size) > struct tm *ptm, tmbuf; > ptm = gmtime_r(&t, &tmbuf); > if (ptm) { > - if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S", ptm)) > + if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%SZ", ptm)) > buf[0] = '\0'; > } > } > This change appears to be correct. I wasn't previously knowledgeable about the 'Z' suffix, but I looked into it and it is documented in ISO 8601 (https://en.wikipedia.org/wiki/ISO_8601 and also http://www.ecma-international.org/ecma-262/5.1/#sec-15.9.1.15 ). On a separate note, the actual format is: YYYY-MM-DDTHH:mm:ss.sssZ . The ".sss" part is missing from this implementation, which represents milliseconds. According to the specification, ".sss" may be absent, but maybe it would work with even more players if it were included. Technically, the specification states that an absent time-zone offset should be treated as 'Z', which indicates that the code was already compliant without the 'Z', even if it didn't work with most players. strftime() doesn't handle milliseconds, but perhaps it ought to populate milliseconds anyway as follows: if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S.000Z", ptm)) Aaron Levinson "
On 5/1/2017 11:06 PM, MFojtak wrote: > Currently this muxer does not work at all. I don't know if 000Z would make > it compatible with more player as I don't know any. However, adding Z makes > it compatible with most popular ones like dash.js and shaka. Having this > muxer working properly would bring more attention to it and maybe in the > future somebody tests it with more players. But for now I suggest to roll > out the change and "unblock" this muxer for some real wold use. Also, it > would be great to make this muxer codec and container agnostic as it works > with h264 and mp4 only. But again, nobody would bother if the muxer doesn't > work at all with browsers. I think your original patch is fine, and I only offered the possibility that adding ".000Z" might be even better than just "Z". I don't have push privileges, so I can't commit your patch, but hopefully someone else will do so. Aaron Levinson
On Tue, 2 May 2017 14:17:33 -0700 Aaron Levinson <alevinsn@aracnet.com> wrote: > On 5/1/2017 11:06 PM, MFojtak wrote: > > Currently this muxer does not work at all. I don't know if 000Z would make > > it compatible with more player as I don't know any. However, adding Z makes > > it compatible with most popular ones like dash.js and shaka. Having this > > muxer working properly would bring more attention to it and maybe in the > > future somebody tests it with more players. But for now I suggest to roll > > out the change and "unblock" this muxer for some real wold use. Also, it > > would be great to make this muxer codec and container agnostic as it works > > with h264 and mp4 only. But again, nobody would bother if the muxer doesn't > > work at all with browsers. > > I think your original patch is fine, and I only offered the possibility > that adding ".000Z" might be even better than just "Z". I don't have > push privileges, so I can't commit your patch, but hopefully someone > else will do so. Before someone pushes it, please fix the commit message.
On 5/2/2017 2:29 PM, wm4 wrote: > On Tue, 2 May 2017 14:17:33 -0700 > Aaron Levinson <alevinsn@aracnet.com> wrote: > >> On 5/1/2017 11:06 PM, MFojtak wrote: >>> Currently this muxer does not work at all. I don't know if 000Z would make >>> it compatible with more player as I don't know any. However, adding Z makes >>> it compatible with most popular ones like dash.js and shaka. Having this >>> muxer working properly would bring more attention to it and maybe in the >>> future somebody tests it with more players. But for now I suggest to roll >>> out the change and "unblock" this muxer for some real wold use. Also, it >>> would be great to make this muxer codec and container agnostic as it works >>> with h264 and mp4 only. But again, nobody would bother if the muxer doesn't >>> work at all with browsers. >> >> I think your original patch is fine, and I only offered the possibility >> that adding ".000Z" might be even better than just "Z". I don't have >> push privileges, so I can't commit your patch, but hopefully someone >> else will do so. > > Before someone pushes it, please fix the commit message. MFojtak: you can make it more likely that this patch will be pushed sooner by altering the commit message (shortening it--plenty of examples on the e-mail list) and then putting any additional information later. Something like: " avformat/dashenc: Adjusts ISO date formatting for improved compatibility with most players Adjusts ISO date formatting.... " Aaron Levinson
Hello, Does anybody know what happened with this patch? ---------- Původní e-mail ---------- Od: MFojtak <MFojtak@seznam.cz> Komu: Aaron Levinson <alevinsn@aracnet.com>, FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Datum: 2. 5. 2017 8:06:29 Předmět: Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (dashj.js, shaka, VLC, etc.) "Currently this muxer does not work at all. I don't know if 000Z would make it compatible with more player as I don't know any. However, adding Z makes it compatible with most popular ones like dash.js and shaka. Having this muxer working properly would bring more attention to it and maybe in the future somebody tests it with more players. But for now I suggest to roll out the change and "unblock" this muxer for some real wold use. Also, it would be great to make this muxer codec and container agnostic as it works with h264 and mp4 only. But again, nobody would bother if the muxer doesn't work at all with browsers. ---------- Původní e-mail ---------- Od: Aaron Levinson <alevinsn@aracnet.com> Komu: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Datum: 30. 4. 2017 20:47:59 Předmět: Re: [FFmpeg-devel] [PATCH] This fixes ISO date formatissue when manifest created by this muxer is not playable in most players. This ensures compatibility with dash standard. Tested on many players (dashj.js, shaka, VLC, etc.) "On 4/26/2017 4:27 AM, mfojtak wrote: > --- > libavformat/dashenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c > index 6232c70..fe1d6c2 100644 > --- a/libavformat/dashenc.c > +++ b/libavformat/dashenc.c > @@ -433,7 +433,7 @@ static void format_date_now(char *buf, int size) > struct tm *ptm, tmbuf; > ptm = gmtime_r(&t, &tmbuf); > if (ptm) { > - if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S", ptm)) > + if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%SZ", ptm)) > buf[0] = '\0'; > } > } > This change appears to be correct. I wasn't previously knowledgeable about the 'Z' suffix, but I looked into it and it is documented in ISO 8601 (https://en.wikipedia.org/wiki/ISO_8601 and also http://www.ecma-international.org/ecma-262/5.1/#sec-15.9.1.15 ). On a separate note, the actual format is: YYYY-MM-DDTHH:mm:ss.sssZ . The ".sss" part is missing from this implementation, which represents milliseconds. According to the specification, ".sss" may be absent, but maybe it would work with even more players if it were included. Technically, the specification states that an absent time-zone offset should be treated as 'Z', which indicates that the code was already compliant without the 'Z', even if it didn't work with most players. strftime() doesn't handle milliseconds, but perhaps it ought to populate milliseconds anyway as follows: if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S.000Z", ptm)) Aaron Levinson ""
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c index 6232c70..fe1d6c2 100644 --- a/libavformat/dashenc.c +++ b/libavformat/dashenc.c @@ -433,7 +433,7 @@ static void format_date_now(char *buf, int size) struct tm *ptm, tmbuf; ptm = gmtime_r(&t, &tmbuf); if (ptm) { - if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%S", ptm)) + if (!strftime(buf, size, "%Y-%m-%dT%H:%M:%SZ", ptm)) buf[0] = '\0'; } }