[PATCH] Add optional "mp4_exact_start" nginx config off/on to show video between keyframes
Tracey Jaquith
tracey at archive.org
Mon Oct 4 22:41:47 UTC 2021
Hi Roman,
OK, thanks!
I’ve tested this on macosx & linux, so far with: chrome, safari, Firefox and iOS.
However, I’m seeing Firefox is having alternate behavior where it plays video from the prior keyframe,
without audio, until it hits the desired start time in at least one video, though it’s not consistently doing this.
I suspect it’s the edit list — a nice solve for this.
I’ve had minor issues with edit lists in the past, for what that’s worth.
I like the new name change, congrats.
Very clear and easy to understand exactly what this new config flag will do.
And deep apologies…
> Another problem is track delay
I *should have* mentioned when I initially wrote in, that I was aware of the a/v sync slight slip
— and that in practice and running for over 3 months now, it hasn’t seemed to be any kind of issue.
Assuming:
* the average (US TV) video might be 29.97 fps
* and thus timescale / duration of 30000 / 1001
* and that a typical max distance between keyframe GOPs w/ ffmpeg encoders and similar is 300 frames or about 10s
Then:
* with a max of 10s between keyframes
* and 300 frames max would get “sped up” from 1001 => 1
Then we’re looking at a maximum additional video frames duration of 1/100th of a second.
(300 * 1001 / 30000) == 10.01
(300 * 1 / 30000) == 0.01
So the most the A/V sync could “drift” from those early added frames is 1/100th of a second,
where average might be 2-3x smaller than that.
In practice, it didn’t seem noticeable —
but I am quite impressed by your desire to minimize/eliminate that.
(In practice, from the broadcasters at least in the US, 1/100th of a second A/V slip is not uncommon).
If we really wanted to avoid that minor A/V “slip”,
another alternative could be to adjust the STTS of all the video frames going out the door,
or the audio frames
(and any other related supporting atom minor changes, if any other were needed).
> On Sep 30, 2021, at 6:48 AM, Roman Arutyunyan <arut at nginx.com> wrote:
>
> Hi Tracey,
>
> On Mon, Sep 20, 2021 at 12:39:15PM -0700, Tracey Jaquith wrote:
>> Hi Roman,
>>
>> I had an idea for considering for the feature name / config flag / function name, in case of interest.
>>
>> What about “startfast”? (or even “faststart”, I suppose)
>>
>> That parallels nicely with the `qt-faststart` utility and the `ffmpeg -movflags faststart`
>> where the moov atom is moved to the front of the mp4.
>>
>> Since the `mp4` module is already rewriting a smaller moov atom for the desired clip,
>> *and* the mp4 module will move the moov atom to the front
>> (in case the source mp4 file has moov atom at the back),
>> It seems like “startfast” might convey the moov atom approach *and* the concept
>> that we’re going to send early visually undesired frames out at ~30,000 fps :)
>>
>> For your consideration, thanks,
>> -Tracey
>
> Thanks for your suggestion. Currently we're considering the name
> "mp4_start_key_frame" which has the word "start" in it, which is the
> argument that enables the feature.
>
> But there's something more important I want to talk about. While doing
> internal review of the patch, we were conserned with some potential
> problems the patch could introduce. Specifically, when the video track has
> B-frames, PTS - DTS delay is stored in the "ctts" atom, which was not changed
> by the patch. This means that some frames from the hidden part of the video
> could show up in the visible part of it. I believe this could be handled
I haven’t done too much with testing videos w/ B-frames,
but the prior nginx mp4 module I was using and patching was doing CTTS changes.
I never had to patch that part of the code — but I could look at what they were up to
and/or point out their code in case that’s of interest?
-Tracey
> properly, but the solution would be much more sophisticated than just
> zeroing out the initial part of ctts. Another problem is track delay, which
> was obvious from the start. The hidden part of the video still takes
> some time to play, which ruins synchronization between tracks. This may or
> may not be noticable in particular cases, but anyway the problem is still there.
>
> I've reimplemented the feature by using mp4 edit lists. In a nutshell, all
> frames up to the latest key frame are included in the video. Then, the
> initial part of the video is hidden from presentation by cutting it with an
> edit list. Looks like this solution does not have the problems I mentioned
> above.
>
> Can you try the new patch in your environment? We would really appreciate
> your feedback.
>
>>> On Jun 28, 2021, at 2:53 AM, Roman Arutyunyan <arut at nginx.com <mailto: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 <mailto:tracey at archive.org> <mailto:tracey at archive.org <mailto: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 <http://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>
>>>> https://pi.archive.org/0/items/CSPAN_20160425_022500_2011_White_House_Correspondents_Dinner.mp4?start=12&end=30&exact=1 <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 <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 <http://nginx.org/en/docs/contributing_changes.html> <http://nginx.org/en/docs/contributing_changes.html <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 <mailto:nginx-devel at nginx.org> <mailto:nginx-devel at nginx.org <mailto:nginx-devel at nginx.org>>
>>>> http://mailman.nginx.org/mailman/listinfo/nginx-devel <http://mailman.nginx.org/mailman/listinfo/nginx-devel> <http://mailman.nginx.org/mailman/listinfo/nginx-devel <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 <mailto:nginx-devel at nginx.org> <mailto:nginx-devel at nginx.org <mailto:nginx-devel at nginx.org>>
>>> http://mailman.nginx.org/mailman/listinfo/nginx-devel <http://mailman.nginx.org/mailman/listinfo/nginx-devel> <http://mailman.nginx.org/mailman/listinfo/nginx-devel <http://mailman.nginx.org/mailman/listinfo/nginx-devel>>
>> -Tracey
>> @tracey_pooh
>> TV Architect https://archive.org/tv <https://archive.org/tv> <https://archive.org/tv <https://archive.org/tv>>
>>
>>
>>
>>
>>
>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org <mailto:nginx-devel at nginx.org>
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel <http://mailman.nginx.org/mailman/listinfo/nginx-devel>
>
>
> --
> Roman Arutyunyan
> <mp4-start-key-frame.txt>_______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org <mailto:nginx-devel at nginx.org>
> http://mailman.nginx.org/mailman/listinfo/nginx-devel <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/20211004/438d749b/attachment-0001.htm>
More information about the nginx-devel
mailing list