[PATCH] Add optional "mp4_exact_start" nginx config off/on to show video between keyframes

Tracey Jaquith tracey at archive.org
Wed Sep 8 00:31:01 UTC 2021


Hi Roman,

Apologies for a long delay.  I was across the country and 50% time for 2 months and took a couple weeks to catchup…

Alright, your updated patch is looking good.  
I think the overall name change from “mp4_exact_start” to “mp4_seek_key_frame” sounds fine to me.
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!

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 :)
and to  test on linux.

Both of those should be pretty straightforward and anticipate no issues/concerns.

What sounds good for the next steps from your POV?

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:
Tracey Jaquith, Internet Archive
Tracey Jaquith tracey at archive.org <mailto:tracey at archive.org>

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.


Very appreciatively! 
-Tracey


> On Jun 28, 2021, at 2:53 AM, Roman Arutyunyan <arut at nginx.com> wrote:
> 
> Hi Tracey,
> 
> On Tue, Jun 15, 2021 at 03:49:48PM -0700, Tracey Jaquith wrote:
>> # HG changeset patch
>> # User Tracey Jaquith <tracey at archive.org>
>> # Date 1623797180 0
>> #      Tue Jun 15 22:46:20 2021 +0000
>> # Node ID 1879d49fe0cf739f48287b5a38a83d3a1adab939
>> # Parent  5f765427c17ac8cf753967387562201cf4f78dc4
>> Add optional "mp4_exact_start" nginx config off/on to show video between keyframes.
> 
> I've been thinking about a better name for this, but came up with nothing so
> far.  I feel like this name does not give the right clue to the user.
> Moreover, when this feature is on, the start is not quite "exact", but shifted
> a few milliseconds into the past.
> 
>> archive.org has been using mod_h264_streaming with a similar "exact start" patch from me since 2013.
>> We just moved to nginx mp4 module and are using this patch.
>> The technique is to find the video keyframe just before the desired "start" time, and send
>> that down the wire so video playback can start immediately.
>> Next calculate how many video samples are between the keyframe and desired "start" time
>> and update the STTS atom where those samples move the duration from (typically) 1001 to 1.
>> This way, initial unwanted video frames play at ~1/30,000s -- so visually the
>> video & audio start playing immediately.
>> 
>> You can see an example before/after here (nginx binary built with mp4 module + patch):
>> 
>> https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30
>> https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30&exact=1
>> 
>> Tested on linux and macosx.
>> 
>> (this is me: https://github.com/traceypooh )
> 
> We have a few rules about patches and commit messages like 67-character limit
> for the first line etc:
> 
> http://nginx.org/en/docs/contributing_changes.html
> 
>> diff -r 5f765427c17a -r 1879d49fe0cf src/http/modules/ngx_http_mp4_module.c
>> --- a/src/http/modules/ngx_http_mp4_module.c    Tue Jun 01 17:37:51 2021 +0300
>> +++ b/src/http/modules/ngx_http_mp4_module.c    Tue Jun 15 22:46:20 2021 +0000
>> @@ -43,6 +43,7 @@
>> typedef struct {
>>     size_t                buffer_size;
>>     size_t                max_buffer_size;
>> +    ngx_flag_t            exact_start;
>> } ngx_http_mp4_conf_t;
>> 
>> 
>> @@ -340,6 +341,13 @@
>>       offsetof(ngx_http_mp4_conf_t, max_buffer_size),
>>       NULL },
>> 
>> +    { ngx_string("mp4_exact_start"),
>> +      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
> 
> NGX_CONF_TAKE1 -> NGX_CONF_FLAG
> 
>> +      ngx_conf_set_flag_slot,
>> +      NGX_HTTP_LOC_CONF_OFFSET,
>> +      offsetof(ngx_http_mp4_conf_t, exact_start),
>> +      NULL },
>> +
>>       ngx_null_command
>> };
>> 
>> @@ -2156,6 +2164,83 @@
>> 
>> 
>> static ngx_int_t
>> +ngx_http_mp4_exact_start_video(ngx_http_mp4_file_t *mp4, ngx_http_mp4_trak_t *trak)
>> +{
>> +    uint32_t               n, speedup_samples, current_count;
>> +    ngx_uint_t             sample_keyframe, start_sample_exact;
>> +    ngx_mp4_stts_entry_t  *entry, *entries_array;
>> +    ngx_buf_t             *data;
>> +
>> +    data = trak->out[NGX_HTTP_MP4_STTS_DATA].buf;
>> +
>> +    // Find the keyframe just before the desired start time - so that we can emit an mp4
>> +    // where the first frame is a keyframe.  We'll "speed up" the first frames to 1000x
>> +    // normal speed (typically), so they won't be noticed.  But this way, perceptively,
>> +    // playback of the _video_ track can start immediately
>> +    // (and not have to wait until the keyframe _after_ the desired starting time frame).
>> +    start_sample_exact = trak->start_sample;
>> +    for (n = 0; n < trak->sync_samples_entries; n++) {
>> +        // each element of array is the sample number of a keyframe
>> +        // sync samples starts from 1 -- so subtract 1
>> +        sample_keyframe = ngx_mp4_get_32value(trak->stss_data_buf.pos + (n * 4)) - 1;
> 
> This can be simplified by introducing entry/end variables like we usually do.
> 
> Also, we don't access trak->stss_data_buf directly, but prefer
> trak->out[NGX_HTTP_MP4_STSS_ATOM].buf.
> 
> ngx_http_mp4_crop_stss_data() provides an example of iterating over stss atom.
> 
>> +        if (sample_keyframe <= trak->start_sample) {
>> +            start_sample_exact = sample_keyframe;
>> +        }
>> +        if (sample_keyframe >= trak->start_sample) {
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (start_sample_exact < trak->start_sample) {
>> +        // We're going to prepend an entry with duration=1 for the frames we want to "not see".
>> +        // MOST of the time (eg: constant video framerate),
>> +        // we're taking a single element entry array and making it two.
>> +        speedup_samples = trak->start_sample - start_sample_exact;
>> +
>> +        ngx_log_debug3(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,
>> +                       "exact trak start_sample move %l to %l (speed up %d samples)\n",
>> +                       trak->start_sample, start_sample_exact, speedup_samples);
>> +
>> +        entries_array = ngx_palloc(mp4->request->pool,
>> +            (1 + trak->time_to_sample_entries) * sizeof(ngx_mp4_stts_entry_t));
>> +        if (entries_array == NULL) {
>> +            return NGX_ERROR;
>> +        }
>> +        entry = &(entries_array[1]);
>> +        ngx_memcpy(entry, (ngx_mp4_stts_entry_t *)data->pos,
>> +                   trak->time_to_sample_entries * sizeof(ngx_mp4_stts_entry_t));
> 
> This reallocation can be avoided.  Look at NGX_HTTP_MP4_STSC_START buffer
> as an example of that.  A new 1-element optional buffer NGX_HTTP_MP4_STTS_START
> can be introduced right before the stts atom data.
> 
>> +        current_count = ngx_mp4_get_32value(entry->count);
>> +        ngx_log_debug1(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,
>> +                       "exact split in 2 video STTS entry from count:%d", current_count);
>> +
>> +        if (current_count <= speedup_samples) {
>> +            return NGX_ERROR;
>> +        }
>> +
>> +        ngx_mp4_set_32value(entry->count, current_count - speedup_samples);
>> +        ngx_log_debug2(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,
>> +                       "exact split new[1]: count:%d duration:%d",
>> +                       ngx_mp4_get_32value(entry->count),
>> +                       ngx_mp4_get_32value(entry->duration));
>> +        entry--;
>> +        ngx_mp4_set_32value(entry->count, speedup_samples);
>> +        ngx_mp4_set_32value(entry->duration, 1);
>> +        ngx_log_debug1(NGX_LOG_DEBUG_HTTP, mp4->file.log, 0,
>> +                       "exact split new[0]: count:%d duration:1",
>> +                       ngx_mp4_get_32value(entry->count));
>> +
>> +        data->pos = (u_char *) entry;
>> +        trak->time_to_sample_entries++;
>> +        trak->start_sample = start_sample_exact;
>> +        data->last = (u_char *) (entry + trak->time_to_sample_entries);
>> +    }
>> +
>> +    return NGX_OK;
>> +}
>> +
>> +
>> +static ngx_int_t
>> ngx_http_mp4_crop_stts_data(ngx_http_mp4_file_t *mp4,
>>     ngx_http_mp4_trak_t *trak, ngx_uint_t start)
>> {
>> @@ -2164,6 +2249,8 @@
>>     ngx_buf_t             *data;
>>     ngx_uint_t             start_sample, entries, start_sec;
>>     ngx_mp4_stts_entry_t  *entry, *end;
>> +    ngx_http_mp4_conf_t   *conf;
>> +
> 
> No need for a new empty line here.
> 
>>     if (start) {
>>         start_sec = mp4->start;
>> @@ -2238,6 +2325,10 @@
>>                        "start_sample:%ui, new count:%uD",
>>                        trak->start_sample, count - rest);
>> 
>> +        conf = ngx_http_get_module_loc_conf(mp4->request, ngx_http_mp4_module);
>> +        if (conf->exact_start) {
>> +            ngx_http_mp4_exact_start_video(mp4, trak);
>> +        }
>>     } else {
>>         ngx_mp4_set_32value(entry->count, rest);
>>         data->last = (u_char *) (entry + 1);
>> @@ -3590,6 +3681,7 @@
>> 
>>     conf->buffer_size = NGX_CONF_UNSET_SIZE;
>>     conf->max_buffer_size = NGX_CONF_UNSET_SIZE;
>> +    conf->exact_start = NGX_CONF_UNSET;
> 
> This is not enough, a merge is needed too.
> 
>> 
>>     return conf;
>> }
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
> 
> I've made a POC patch which incorporates the issues I've mentioned.
> I didn't test is properly and the directive name is still not perfect.
> 
> -- 
> Roman Arutyunyan
> <mp4-exact-arut.txt>_______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

-Tracey
@tracey_pooh
TV Architect  https://archive.org/tv <https://archive.org/tv>





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20210907/35c1a3b6/attachment-0001.htm>


More information about the nginx-devel mailing list