<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Roman,<div class=""><br class=""></div><div class="">Apologies for a long delay. I was across the country and 50% time for 2 months and took a couple weeks to catchup…</div><div class=""><br class=""></div><div class="">Alright, your updated patch is looking good. </div><div class="">I think the overall name change from “mp4_exact_start” to “mp4_seek_key_frame” sounds fine to me.</div><div class="">I’ve compiled current head-of-master with your patch and tested on MacOSX and it’s looking to work the same as the prior patch, kudos!</div><div class=""><br class=""></div><div class="">I want to add some temporary debug lines to make sure I understand (especially) the way you cleverly avoided an nginx alloc for extra entry :)</div><div class="">and to test on linux.</div><div class=""><br class=""></div><div class="">Both of those should be pretty straightforward and anticipate no issues/concerns.</div><div class=""><br class=""></div><div class="">What sounds good for the next steps from your POV?</div><div class=""><br class=""></div><div class="">If you imagine I wouldn’t be getting commit/push rights and added as a contributor, I’d love to add next to my name (thank you for adding that) somewhere something like:</div><div class="">Tracey Jaquith, Internet Archive</div><div class="">Tracey Jaquith <a href="mailto:tracey@archive.org" class="">tracey@archive.org</a></div><div class=""><br class=""></div><div class="">Since I worked on this primarily for my job purposes and I’d love the idea that both myself and the Archive are porting upstream and idea, code, etc.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Very appreciatively! </div><div class="">-Tracey</div><div class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jun 28, 2021, at 2:53 AM, Roman Arutyunyan <<a href="mailto:arut@nginx.com" class="">arut@nginx.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi Tracey,<br class=""><br class="">On Tue, Jun 15, 2021 at 03:49:48PM -0700, Tracey Jaquith wrote:<br class=""><blockquote type="cite" class=""># HG changeset patch<br class=""># User Tracey Jaquith <<a href="mailto:tracey@archive.org" class="">tracey@archive.org</a>><br class=""># Date 1623797180 0<br class=""># Tue Jun 15 22:46:20 2021 +0000<br class=""># Node ID 1879d49fe0cf739f48287b5a38a83d3a1adab939<br class=""># Parent 5f765427c17ac8cf753967387562201cf4f78dc4<br class="">Add optional "mp4_exact_start" nginx config off/on to show video between keyframes.<br class=""></blockquote><br class="">I've been thinking about a better name for this, but came up with nothing so<br class="">far. I feel like this name does not give the right clue to the user.<br class="">Moreover, when this feature is on, the start is not quite "exact", but shifted<br class="">a few milliseconds into the past.<br class=""><br class=""><blockquote type="cite" class=""><a href="http://archive.org" class="">archive.org</a> has been using mod_h264_streaming with a similar "exact start" patch from me since 2013.<br class="">We just moved to nginx mp4 module and are using this patch.<br class="">The technique is to find the video keyframe just before the desired "start" time, and send<br class="">that down the wire so video playback can start immediately.<br class="">Next calculate how many video samples are between the keyframe and desired "start" time<br class="">and update the STTS atom where those samples move the duration from (typically) 1001 to 1.<br class="">This way, initial unwanted video frames play at ~1/30,000s -- so visually the<br class="">video & audio start playing immediately.<br class=""><br class="">You can see an example before/after here (nginx binary built with mp4 module + patch):<br class=""><br class=""><a href="https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30" class="">https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30</a><br class="">https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30&exact=1<br class=""><br class="">Tested on linux and macosx.<br class=""><br class="">(this is me: https://github.com/traceypooh )<br class=""></blockquote><br class="">We have a few rules about patches and commit messages like 67-character limit<br class="">for the first line etc:<br class=""><br class=""><a href="http://nginx.org/en/docs/contributing_changes.html" class="">http://nginx.org/en/docs/contributing_changes.html</a><br class=""><br class=""><blockquote type="cite" class="">diff -r 5f765427c17a -r 1879d49fe0cf src/http/modules/ngx_http_mp4_module.c<br class="">--- a/src/http/modules/ngx_http_mp4_module.c Tue Jun 01 17:37:51 2021 +0300<br class="">+++ b/src/http/modules/ngx_http_mp4_module.c Tue Jun 15 22:46:20 2021 +0000<br class="">@@ -43,6 +43,7 @@<br class=""> typedef struct {<br class=""> size_t buffer_size;<br class=""> size_t max_buffer_size;<br class="">+ ngx_flag_t exact_start;<br class=""> } ngx_http_mp4_conf_t;<br class=""><br class=""><br class="">@@ -340,6 +341,13 @@<br class=""> offsetof(ngx_http_mp4_conf_t, max_buffer_size),<br class=""> NULL },<br class=""><br class="">+ { ngx_string("mp4_exact_start"),<br class="">+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,<br class=""></blockquote><br class="">NGX_CONF_TAKE1 -> NGX_CONF_FLAG<br class=""><br class=""><blockquote type="cite" class="">+ ngx_conf_set_flag_slot,<br class="">+ NGX_HTTP_LOC_CONF_OFFSET,<br class="">+ offsetof(ngx_http_mp4_conf_t, exact_start),<br class="">+ NULL },<br class="">+<br class=""> ngx_null_command<br class=""> };<br class=""><br class="">@@ -2156,6 +2164,83 @@<br class=""><br class=""><br class=""> static ngx_int_t<br class="">+ngx_http_mp4_exact_start_video(ngx_http_mp4_file_t *mp4, ngx_http_mp4_trak_t *trak)<br class="">+{<br class="">+ uint32_t n, speedup_samples, current_count;<br class="">+ ngx_uint_t sample_keyframe, start_sample_exact;<br class="">+ ngx_mp4_stts_entry_t *entry, *entries_array;<br class="">+ ngx_buf_t *data;<br class="">+<br class="">+ data = trak->out[NGX_HTTP_MP4_STTS_DATA].buf;<br class="">+<br class="">+ // Find the keyframe just before the desired start time - so that we can emit an mp4<br class="">+ // where the first frame is a keyframe. We'll "speed up" the first frames to 1000x<br class="">+ // normal speed (typically), so they won't be noticed. But this way, perceptively,<br class="">+ // playback of the _video_ track can start immediately<br class="">+ // (and not have to wait until the keyframe _after_ the desired starting time frame).<br class="">+ start_sample_exact = trak->start_sample;<br class="">+ for (n = 0; n < trak->sync_samples_entries; n++) {<br class="">+ // each element of array is the sample number of a keyframe<br class="">+ // sync samples starts from 1 -- so subtract 1<br class="">+ sample_keyframe = ngx_mp4_get_32value(trak->stss_data_buf.pos + (n * 4)) - 1;<br class=""></blockquote><br class="">This can be simplified by introducing entry/end variables like we usually do.<br class=""><br class="">Also, we don't access trak->stss_data_buf directly, but prefer<br class="">trak->out[NGX_HTTP_MP4_STSS_ATOM].buf.<br class=""><br class="">ngx_http_mp4_crop_stss_data() provides an example of iterating over stss atom.<br class=""><br class=""><blockquote type="cite" class="">+ if (sample_keyframe <= trak->start_sample) {<br class="">+ start_sample_exact = sample_keyframe;<br class="">+ }<br class="">+ if (sample_keyframe >= trak->start_sample) {<br class="">+ break;<br class="">+ }<br class="">+ }<br class="">+<br class="">+ if (start_sample_exact < trak->start_sample) {<br class="">+ // We're going to prepend an entry with duration=1 for the frames we want to "not see".<br class="">+ // MOST of the time (eg: constant video framerate),<br class="">+ // we're taking a single element entry array and making it two.<br class="">+ speedup_samples = trak->start_sample - start_sample_exact;<br class="">+<br class="">+ ngx_log_debug3(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">+ "exact trak start_sample move %l to %l (speed up %d samples)\n",<br class="">+ trak->start_sample, start_sample_exact, speedup_samples);<br class="">+<br class="">+ entries_array = ngx_palloc(mp4->request->pool,<br class="">+ (1 + trak->time_to_sample_entries) * sizeof(ngx_mp4_stts_entry_t));<br class="">+ if (entries_array == NULL) {<br class="">+ return NGX_ERROR;<br class="">+ }<br class="">+ entry = &(entries_array[1]);<br class="">+ ngx_memcpy(entry, (ngx_mp4_stts_entry_t *)data->pos,<br class="">+ trak->time_to_sample_entries * sizeof(ngx_mp4_stts_entry_t));<br class=""></blockquote><br class="">This reallocation can be avoided. Look at NGX_HTTP_MP4_STSC_START buffer<br class="">as an example of that. A new 1-element optional buffer NGX_HTTP_MP4_STTS_START<br class="">can be introduced right before the stts atom data.<br class=""><br class=""><blockquote type="cite" class="">+ current_count = ngx_mp4_get_32value(entry->count);<br class="">+ ngx_log_debug1(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">+ "exact split in 2 video STTS entry from count:%d", current_count);<br class="">+<br class="">+ if (current_count <= speedup_samples) {<br class="">+ return NGX_ERROR;<br class="">+ }<br class="">+<br class="">+ ngx_mp4_set_32value(entry->count, current_count - speedup_samples);<br class="">+ ngx_log_debug2(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">+ "exact split new[1]: count:%d duration:%d",<br class="">+ ngx_mp4_get_32value(entry->count),<br class="">+ ngx_mp4_get_32value(entry->duration));<br class="">+ entry--;<br class="">+ ngx_mp4_set_32value(entry->count, speedup_samples);<br class="">+ ngx_mp4_set_32value(entry->duration, 1);<br class="">+ ngx_log_debug1(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">+ "exact split new[0]: count:%d duration:1",<br class="">+ ngx_mp4_get_32value(entry->count));<br class="">+<br class="">+ data->pos = (u_char *) entry;<br class="">+ trak->time_to_sample_entries++;<br class="">+ trak->start_sample = start_sample_exact;<br class="">+ data->last = (u_char *) (entry + trak->time_to_sample_entries);<br class="">+ }<br class="">+<br class="">+ return NGX_OK;<br class="">+}<br class="">+<br class="">+<br class="">+static ngx_int_t<br class=""> ngx_http_mp4_crop_stts_data(ngx_http_mp4_file_t *mp4,<br class=""> ngx_http_mp4_trak_t *trak, ngx_uint_t start)<br class=""> {<br class="">@@ -2164,6 +2249,8 @@<br class=""> ngx_buf_t *data;<br class=""> ngx_uint_t start_sample, entries, start_sec;<br class=""> ngx_mp4_stts_entry_t *entry, *end;<br class="">+ ngx_http_mp4_conf_t *conf;<br class="">+<br class=""></blockquote><br class="">No need for a new empty line here.<br class=""><br class=""><blockquote type="cite" class=""> if (start) {<br class=""> start_sec = mp4->start;<br class="">@@ -2238,6 +2325,10 @@<br class=""> "start_sample:%ui, new count:%uD",<br class=""> trak->start_sample, count - rest);<br class=""><br class="">+ conf = ngx_http_get_module_loc_conf(mp4->request, ngx_http_mp4_module);<br class="">+ if (conf->exact_start) {<br class="">+ ngx_http_mp4_exact_start_video(mp4, trak);<br class="">+ }<br class=""> } else {<br class=""> ngx_mp4_set_32value(entry->count, rest);<br class=""> data->last = (u_char *) (entry + 1);<br class="">@@ -3590,6 +3681,7 @@<br class=""><br class=""> conf->buffer_size = NGX_CONF_UNSET_SIZE;<br class=""> conf->max_buffer_size = NGX_CONF_UNSET_SIZE;<br class="">+ conf->exact_start = NGX_CONF_UNSET;<br class=""></blockquote><br class="">This is not enough, a merge is needed too.<br class=""><br class=""><blockquote type="cite" class=""><br class=""> return conf;<br class=""> }<br class="">_______________________________________________<br class="">nginx-devel mailing list<br class="">nginx-devel@nginx.org<br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel<br class=""></blockquote><br class="">I've made a POC patch which incorporates the issues I've mentioned.<br class="">I didn't test is properly and the directive name is still not perfect.<br class=""><br class="">-- <br class="">Roman Arutyunyan<br class=""><span id="cid:CB49E243-6284-4ABE-8892-2E0CF41FC605"><mp4-exact-arut.txt></span>_______________________________________________<br class="">nginx-devel mailing list<br class="">nginx-devel@nginx.org<br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel</div></div></blockquote></div><br class=""><div class="">
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">-Tracey</div><div class="">@tracey_pooh</div><div class="">TV Architect <a href="https://archive.org/tv" class="">https://archive.org/tv</a></div></div></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br class=""></div></body></html>