<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="">Hello Roman,<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jun 9, 2021, at 5:10 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="">Hello Tracey.<br class=""><br class="">Thanks for your patch, it looks very interesting.<br class=""></div></div></blockquote><div><br class=""></div><div>Yay! :) </div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">On Thu, Jun 03, 2021 at 07:54:49PM +0000, 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 1622678642 0<br class="">#      Thu Jun 03 00:04:02 2021 +0000<br class=""># Node ID 5da9c62fa61016600f2c59982ae184e2811be427<br class=""># Parent  5f765427c17ac8cf753967387562201cf4f78dc4<br class="">Add optional "&exact=1" CGI arg to show video between keyframes.<br class=""></blockquote><br class="">I think this can be a directive (like mp4_buffer_size/mp4_max_buffer_size)<br class="">rather than an argument.  Is it possible than we need both exact and non-exact<br class="">in the same location?  It feels like we don't need to give clients control over<br class="">this.  The exact mode introduces some overhead and should only be used when<br class="">needed.<br class=""></div></div></blockquote><div><br class=""></div><div>That’s a fine idea.  </div><div>So far, we’ve been using “exact clipping” at <a href="http://archive.org" class="">archive.org</a> both ways — but there’s some interest from our TV team to just always do the “exact” mode, anyway.</div><div>So that could work for us.</div><div>That something I should look into for a revised / 2nd patch, sounds like?</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""><a href="http://archive.org" class="">archive.org</a> has been using mod_h264_streaming with an "&exact=1" 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 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="">I realize two var declarations should style-wise get moved "up" in the lower hooked in method.<br class="">However, to minimize the changeset/patch, I figured we should at least start here and see what you folks think.<br class=""></blockquote><br class="">Syle-wise there are a few minor issues.  We'll figure these out later.<br class=""></div></div></blockquote><div><br class=""></div><div>Perfect thanks.  I figured _at least_ those var decls but am sure there are probably some more since I’m stepping into your house :)</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Below are my comments on ways to simplify the code.<br class=""><br class=""><blockquote type="cite" class="">(this is me: <a href="https://github.com/traceypooh" class="">https://github.com/traceypooh</a> )<br class=""><br class="">diff -r 5f765427c17a -r 5da9c62fa610 src/http/modules/ngx_http_mp4_module.c<br class="">--- a/src/http/modules/ngx_http_mp4_module.c<span class="Apple-tab-span" style="white-space:pre">      </span>Tue Jun 01 17:37:51 2021 +0300<br class="">+++ b/src/http/modules/ngx_http_mp4_module.c<span class="Apple-tab-span" style="white-space:pre">     </span>Thu Jun 03 00:04:02 2021 +0000<br class="">@@ -2045,6 +2045,109 @@<br class="">     u_char    duration[4];<br class=""> } ngx_mp4_stts_entry_t;<br class=""><br class="">+typedef struct {<br class="">+    uint32_t              speedup_samples;<br class="">+    ngx_uint_t            speedup_seconds;<br class="">+} ngx_mp4_exact_t;<br class="">+<br class="">+static void<br class="">+exact_video_adjustment(ngx_http_mp4_file_t *mp4, ngx_http_mp4_trak_t *trak, ngx_mp4_exact_t *exact)<br class="">+{<br class="">+    // will parse STTS -- time-to-sample atom<br class="">+    ngx_str_t             value;<br class="">+    ngx_buf_t            *stts_data;<br class="">+    ngx_buf_t            *atom;<br class="">+    ngx_mp4_stts_entry_t *stts_entry, *stts_end;<br class="">+    uint32_t              count, duration, j, n, sample_keyframe, sample_num;<br class="">+    uint64_t              sample_time, seconds, start_seconds_closest_keyframe;<br class="">+    uint8_t               is_keyframe;<br class="">+<br class="">+    exact->speedup_samples = 0;<br class="">+    exact->speedup_seconds = 0;<br class="">+<br class="">+    // if '&exact=1' CGI arg isn't present, do nothing<br class="">+    if (!(ngx_http_arg(mp4->request, (u_char *) "exact", 5, &value) == NGX_OK)) {<br class="">+        return;<br class="">+    }<br class="">+<br class="">+    if (!trak->sync_samples_entries) {<br class="">+        // Highly unlikely video STSS got parsed and _every_ sample is a keyframe.<br class="">+        // However, if the case, we don't need to adjust the video at all.<br class="">+        return;<br class="">+    }<br class="">+<br class="">+    // check HDLR atom to see if this trak is video or audio<br class="">+    atom = trak->out[NGX_HTTP_MP4_HDLR_ATOM].buf;<br class="">+    // 'vide' or 'soun'<br class="">+    if (!(atom->pos[16] == 'v' &&<br class="">+          atom->pos[17] == 'i' &&<br class="">+          atom->pos[18] == 'd' &&<br class="">+          atom->pos[19] == 'e')) {<br class="">+        return; // do nothing if not video<br class="">+    }<br class=""></blockquote><br class="">Do we really need to check this?  If a track has an stss atom, this seems<br class="">enough to do the work.<br class=""></div></div></blockquote><div><br class=""></div><div>With the .mp4 that we make at <a href="http://archive.org" class="">archive.org</a>, I am seeing STSS consistently for the audio track.</div><div>That’s pointing out the start times of the audio samples, so that sort of makes sense to me.</div><div>So I found we needed a way to do nothing for any audio tracks.</div><div><br class=""></div><div>I guess… we _could_ maybe run the same logic on any audio track(s) since, presumably,</div><div>each audio sample is a “keyframe” (and so theoretically we should do nothing below)?</div><div>If you’d prefer something like that, I’d need to do a little digging/testing.</div><div><br class=""></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+    stts_data = trak->out[NGX_HTTP_MP4_STTS_DATA].buf;<br class="">+    stts_entry = (ngx_mp4_stts_entry_t *) stts_data->pos;<br class="">+    stts_end   = (ngx_mp4_stts_entry_t *) stts_data->last;<br class="">+<br class="">+    sample_num = 0; // they start at one <shrug><br class="">+    sample_time = 0;<br class="">+    start_seconds_closest_keyframe = 0;<br class="">+    while (stts_entry < stts_end) {<br class="">+        // STTS === time-to-sample atom<br class="">+        //    each entry is 4B and [sample count][sample duration]  (since durations can vary)<br class="">+        count = ngx_mp4_get_32value(stts_entry->count);<br class="">+        duration = ngx_mp4_get_32value(stts_entry->duration);<br class="">+<br class="">+        for (j = 0; j < count; j++) {<br class="">+            sample_num++;<br class="">+<br class="">+            // search STSS sync sample entries to see if this sample is a keyframe<br class="">+            is_keyframe = (trak->sync_samples_entries ? 0 : 1);<br class=""></blockquote><br class="">Considering the above, trak->sync_samples_entries is always non-zero here.<br class=""></div></div></blockquote><div><br class=""></div>Oh, excellent eyes, thanks!</div><div>I will update that to </div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 14px; line-height: 16px; white-space: pre;" class=""><span style="color: rgb(9, 89, 132);" class="">is_keyframe</span> = <span style="color: rgb(73, 104, 57);" class="">0</span>;</div><div><br class=""></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class=""><blockquote type="cite" class="">+            for (n = 0; n < trak->sync_samples_entries; n++) {<br class="">+                // each one of this these are a video sample number keyframe<br class="">+                sample_keyframe = ngx_mp4_get_32value(trak->stss_data_buf.pos + (n * 4));<br class="">+                if (sample_keyframe == sample_num) {<br class="">+                    is_keyframe = 1;<br class="">+                    break;<br class="">+                }<br class="">+                if (sample_keyframe > sample_num) {<br class="">+                    break;<br class="">+                }<br class="">+            }<br class="">+<br class="">+            seconds = sample_time * 1000 / trak->timescale;<br class="">+            sample_time += duration;<br class="">+<br class="">+            if (seconds > mp4->start) {<br class="">+                goto found;<br class="">+            }<br class="">+<br class="">+            if (is_keyframe) {<br class="">+                start_seconds_closest_keyframe = seconds;<br class="">+                exact->speedup_samples = 0;<br class="">+            } else {<br class="">+                exact->speedup_samples++;<br class="">+            }<br class="">+        }<br class="">+<br class="">+        stts_entry++;<br class="">+    }<br class="">+<br class="">+    found:<br class="">+<br class="">+    ngx_log_debug2(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">+                   "exact_video_adjustment() new keyframe start: %d, speedup first %d samples",<br class="">+                   start_seconds_closest_keyframe,<br class="">+                   exact->speedup_samples);<br class="">+<br class="">+    // NOTE: begin 1 start position before keyframe to ensure first video frame emitted is always<br class="">+    // a keyframe<br class="">+    exact->speedup_seconds = mp4->start - start_seconds_closest_keyframe<br class="">+                             - (start_seconds_closest_keyframe ? 1 : 0);<br class="">+}<br class="">+<br class=""><br class=""> static ngx_int_t<br class=""> ngx_http_mp4_read_stts_atom(ngx_http_mp4_file_t *mp4, uint64_t atom_data_size)<br class="">@@ -2164,10 +2267,14 @@<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_mp4_exact_t        exact;<br class=""><br class="">     if (start) {<br class="">         start_sec = mp4->start;<br class=""><br class="">+        exact_video_adjustment(mp4, trak, &exact);<br class="">+        start_sec -= exact.speedup_seconds;<br class=""></blockquote><br class="">Here you find the closest key frame from the past and adjust start_sec to<br class="">match it.  But what if we don't do this here, but continue with the original<br class="">start_sec.  When we find the right (probably non-key) frame, we'll have its<br class="">number in start_sample.  All we'll need to do at that point is to search for<br class="">the closest key frame to that sample number - just a simple loop.  When we<br class="">find the number of samples into the past until the most recent key frame,<br class="">we'll just add one entry to the stts array with (this number, 1) and then<br class="">decrease start_sample accordingly.  Sounds simpler.<br class=""></div></div></blockquote><div><br class=""></div><div>Ah, ooh, that’s very interesting!  I like the sound of it.</div><div>So we’d use</div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 14px; line-height: 16px; white-space: pre;" class=""><span style="color: rgb(9, 89, 132);" class="">trak</span>-><span style="color: rgb(9, 89, 132);" class="">start_sample</span></div></div><div>Sounds like, and go… either forwards or backwards in a simple loop over the </div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 14px; line-height: 16px; white-space: pre;" class=""><span style="color: rgb(9, 89, 132);" class="">trak</span>-><span style="color: rgb(9, 89, 132);" class="">sync_samples_entries</span></div></div><div>until we find the the keyframe entry number that is just less than or equal to the </div><div><div><div style="color: rgb(54, 54, 54); background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 14px; line-height: 16px; white-space: pre;" class=""><span style="color: rgb(9, 89, 132);" class="">trak</span>-><span style="color: rgb(9, 89, 132);" class="">start_sample</span></div><div class="">Does that sound right?</div></div></div></div><div><br class=""></div><div>And I think we could be adding up to 300 STTS entries compared to the non-exact method</div><div>(depending on video, but assuming up to 10s between keyframes and 30fps).</div><div><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+<br class="">         ngx_log_debug1(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,<br class="">                        "mp4 stts crop start_time:%ui", start_sec);<br class=""><br class="">@@ -2230,6 +2337,42 @@<br class=""><br class="">     if (start) {<br class="">         ngx_mp4_set_32value(entry->count, count - rest);<br class="">+<br class="">+        if (exact.speedup_samples) {<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, we're taking a single element entry array and making it two.<br class="">+            uint32_t current_count = ngx_mp4_get_32value(entry->count);<br class="">+            ngx_mp4_stts_entry_t* entries_array = ngx_palloc(mp4->request->pool,<br class="">+                (1 + entries) * sizeof(ngx_mp4_stts_entry_t));<br class="">+            if (entries_array == NULL) {<br class="">+                return NGX_ERROR;<br class="">+            }<br class=""></blockquote><br class="">I believe there are situations when there's an entry from the past in the old<br class="">array, that can be reused.<br class=""></div></div></blockquote><div><br class=""></div><div>So this is indeed a _very_ interesting point!</div><div>Most of the time, certainly for our created .mp4, we’re talking about constant frame rates for the video.</div><div>(The audio is indeed often a huge list of entries w/ minor variance in their durations).</div><div>So for the video, I’m mostly only ever seeing a single entry list with duration “1001”</div><div>(For the 29.97 fps typical TV we see in US where it’s 30000 / 1001 => 29.97).</div><div><br class=""></div><div>I did start trying to make the logical list split anywhere it might be found for 2+ entries,</div><div>but it _really_ was making my head hurt since there were so many computations to do</div><div>while trying to figure out where all the frames were, and where we might be splitting.</div><div><br class=""></div><div>In the end, I found I have almost entirely single-entry arrays, splitting into two.</div><div>(Though the current patch doesn’t care what the number of array elements is — it just</div><div> prepends a new entry (of duration 1 instead of typical 1001) via a logical `realloc()`)</div><div><br class=""></div><div>If you’d prefer something for 2+ entries case, that tries to see if we’re dropping enough entries from </div><div>the front — and to just reuse one (and not realloc — though we’d <still> need a realloc case for</div><div>corner cases anyway, _i think_) then perhaps I could use some help or thoughts on that since</div><div>it was getting super complicated when I was trying to diagram it out and was fretting and</div><div>nearly paralyzed with how to proceed cleanly and safely :-p </div><div><br class=""></div><div>Thanks so much for the detailed feedback and time for your thoughts!</div><div>Super appreciated and I’m very upbeat about this all.</div><div><br class=""></div><div>-Tracey</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">+            ngx_copy(&(entries_array[1]), entry, entries * sizeof(ngx_mp4_stts_entry_t));<br class="">+<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 <= exact.speedup_samples)<br class="">+                return NGX_ERROR;<br class="">+<br class="">+            entry = &(entries_array[1]);<br class="">+            ngx_mp4_set_32value(entry->count, current_count - exact.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, exact.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->last = (u_char *) (entry + 2);<br class="">+<br class="">+            entries++;<br class="">+        }<br class="">+<br class="">         data->pos = (u_char *) entry;<br class="">         trak->time_to_sample_entries = entries;<br class="">         trak->start_sample = start_sample;<br class="">_______________________________________________<br class="">nginx-devel mailing list<br class=""><a href="mailto:nginx-devel@nginx.org" class="">nginx-devel@nginx.org</a><br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel<br class=""></blockquote><br class="">-- <br class="">Roman Arutyunyan<br class="">_______________________________________________<br class="">nginx-devel mailing list<br class=""><a href="mailto:nginx-devel@nginx.org" class="">nginx-devel@nginx.org</a><br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel<br class=""></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=""></body></html>