[MPlayer-dev-eng] [PATCH] mencoder: correctly consider encoder delays when examining muxer time

[Corey Hickey]

On 2010-10-16 18:37, Reimar Döffinger wrote:
> On Sat, Oct 16, 2010 at 03:54:56PM -0700, Corey Hickey wrote:
>> @@ -1243,9 +1247,15 @@
>>      float a_pts=0;
>>      float v_pts=0;
>>      int skip_flag=0; // 1=skip  -1=duplicate
>> +    double a_muxer_time;
>> +    double v_muxer_time;
>>  
>> +    if (mux_a)
>> +        a_muxer_time = adjusted_muxer_time(mux_a);
>> +    v_muxer_time = adjusted_muxer_time(mux_v);
> 
> You could make the function return NOPTS_VALUE for NULL argument,
> might make this a bit simpler.

Ok.

> And since you seem to understand it, please document what each of those
> time values actually are.

Done, in a comment to adjusted_muxer_time().

>> @@ -1359,7 +1369,7 @@
>>  				mux_a->buffer_len += len;
>>  			}
>>  	    }
>> -	    if (mux_v->timer == 0) mux_a->h.dwInitialFrames++;
>> +	    if (v_muxer_time == 0) mux_a->h.dwInitialFrames++;
> 
> Sure this is really using the right timer?
> I'd naively expect that v_muxer_time possibly is never 0.

mux_v->timer (and, hence, v_muxer_time) stores the time of the video
stream, so it is 0 before muxing any video frames; there can be one or
more audio frames first.

After some research, I think this line of code should be moved to the
muxer itself, or simply removed. MPlayer ignores dwInitialFrames when
reading files; ffmpeg ignores it and always sets it to 0. I also found
this in VirtualDub's release notes:

http://www.virtualdub.org/blog/pivot/entry.php?id=317
-----------------------------------------------------------------------
The first important bug fix is that the dwInitialFrames field is now
unilaterally cleared when writing AVI files. In some cases, VirtualDub
was passing this value through from source files, which caused
compatibility problems as some players didn't like non-zero values. Like
many header fields, this one is also underdocumented in Microsoft docs,
is misinterpreted by various players, and isn't supposed to have any
effect on audio sync or sample timing. Some users were encountering
issues with this value being left non-zero, though, so the program now
always zeroes it.
-----------------------------------------------------------------------

I could remove the line in a separate patch; or I could refrain from
changing something which isn't known to be causing problems. Either way,
I'm neutral.

Thanks,
Corey
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: encoder_delay3.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101017/2fea91d5/attachment.asc>