[FFmpeg-devel] [PATCH] Don't adjust start time for MP3 files; packets are not adjusted.

[Michael Niedermayer]

On Fri, May 29, 2020 at 03:36:47PM -0700, Dale Curtis wrote:
> On Thu, May 28, 2020 at 1:33 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > I dont really have an oppinion about start_time, its more the change in
> > timestamps & duratiion on the output side which is whats user vissible
> > and that looked worse for the testcase
> >
> 
> The difference is due to the decode stage applying the skip samples to pts:
> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/decode.c#L421
> 
> That makes my statement in the commit "skip_samples are applied by deleting
> data from a packet without changing the timestamp" incorrect -  sorry for
> getting that wrong. Since we use decoders other than ffmpeg, Chromium's
> implementation deletes skip samples indicated by the side data without
> changing the packet timestamps so audio and video line up properly. ffmpeg
> seems to do the same thing but in a more roundabout fashion that I don't
> fully understand.
> 
> Prior to my patch, avformat_find_stream_info() indicated a non-zero start
> time, av_read_frame() returned packets from time zero, but with skip
> samples marked as side data. Later avcodec_receive_frame() adjusts the pts,
> dts, and duration. Finally some other piece of code is adjusting the pts by
> the start time before it reaches the framecrc muxer. I haven't been able to
> figure out where that happens though.
> 
> I'm not sure what the right fix is here. As an API user, we've always
> expected start_time to refer to the pts of the first packet returned by
> av_read_frame(). Indeed avformat.h says "pts of the first frame of the
> stream in presentation order". If that's not the case, then it'd be helpful
> to have some guidance on how seeking works (e.g., even ffmpeg.c seeks to
> start_time, but that skips the preroll if it's non-zero) and what should be
> done with frames before the start time.
> 
> I'm less sure about this last point, but adjusting the pts during the
> decode stage seems incompatible with how edit lists are applied for mp4
> during the demuxing stage. E.g., IIUC, if an edit list slices out the time
> range [0,6] across two keyframe packets p0={pts=0, dur=5} and p1={pts=5,
> dur=5} the mov demuxer would discard p0 and p1 would be become {pts=7,
> dur=5, skip_samples=2}.
> 

> In any case, it seems incorrect that ffmpeg no longer has a timestamp of
> zero for the first decoded mp3 frame when skip samples are present. So at
> the very least that does seem to need a fix. Either by reverting this patch
> or another mechanism.

hmm, ill revert for 4.3, so we can look into this with more time available
and avoid creating a release that has a new but also somewhat wrong behavior

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200608/4167704c/attachment.sig>