[FFmpeg-devel] [PATCH 4/4] lavf/mov: Add support for edit list parsing.

Wed Sep 14 02:39:38 EEST 2016

Sorry for the very late reply. I was busy with other things.

On Sat, Sep 3, 2016 at 4:48 PM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Sat, Sep 03, 2016 at 12:06:39PM -0700, Sasi Inguva wrote:
> > Hi Michael,
> >
> > In fact from audacity I see that out-ingu.wav out-mp3.wav are almost
> > equivalent,
>
> They do not match. (and that is alot more vissible if you scale the
> time axis up a bit)
>
> You also dont use the existing API for handling initial padding/skip
> And you didnt reply to this concern
> its probably not that hard to fix that ...
> instead of droping just at packet granularity you would set the stuff
> for droping at sample granularity (too)
>

Yes. Looking at it more closely now, they don't matrch exactly and this is
because as you said, number of samples to drop is not exactly multiple of
number of packets. In that case we need to partially discard some samples
of a packet. This can be done by using AV_PKT_SIDE_DATA_SKIP_SAMPLES.  I
can change the code to use this packet side data instead of discard packet
flag, if it is ok.

>
> also maybe you missed my question about your oppinion on the correct
> timestamp output:
> "My first question is, entirely independant of the implementation from
>  the patches. What is the correct output ? (my primary focus is on
>  the timestamps)"
>
> Iam curious because to me the timestamps of the dropped packets look
> wrong and id like to know your oppinion about this. Is this a
> implementation issue (if so please explain) or is there some reason
> independant of the implementation  (if so please explain too)
>

The correct output according to me should be - we should show exactly the
same presentation timestmap indicated by the MOV stts, ctts atoms, for the
samples that fall inside the edits. As long as the PTS is according to what
the edit and stts,ctts atoms say. I don't really have to worry about what
the decoding timestamp for those packets should be (DTS might as well be
AV_NOPTS_VALUE for all packets) .

In the current implementation, we directly assign the timestamp in the
AVIndex to  pkt->dts .
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;h=6e80b93271a4f998af6ba1af795d7d7c5d67f5a1;hb=HEAD#l5332
. The implementation of the edit list code is such that, it needs to be
filled with "linear monotonically increasing timestamps for the
non-discarded packets " to have the samples denoted by the editlists to
have linear monotonically increasing timestamps, in essence, to avoid a big
gap between the presentation of the end of first edit list and the start of
next edit list because of the discarded packets. Hence while parsing a new
edit list we bump down the index entry timestamp to (frame_duration + last
non-discarded packet from previous edit list ) . And that's why we get
non-monotonic timestamps as a whole in the AVIndexEntry .

Let us take an example of a file with two edit lists. First edit list
ending on a B frame. This is what ffprobe -show_packet -pf compact looks
like . Where 'D' stands for discarded frame. ( I've attached a 6th patch to
ffprobe to achieve this formatting ).
./ffprobe -show_packets -print_format compact
mov-2elist-bpyramid-1elist-ends-on-bref.mov
.....
packet|codec_type=video|stream_index=0|pts=10752|pts_time=0.875000|dts=8192|dts_time=0.666667|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=1281|pos=39546|flags=__
- Pframe
packet|codec_type=video|stream_index=0|pts=9728|pts_time=0.791667|dts=8704|dts_time=0.708333|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=749|pos=40827|flags=__
- B pyramidal ref
packet|codec_type=video|stream_index=0|pts=9216|pts_time=0.750000|dts=9216|dts_time=0.750000|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=487|pos=41576|flags=__
- B
packet|codec_type=video|stream_index=0|pts=10240|pts_time=0.833333|dts=9728|dts_time=0.791667|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=486|pos=42063|flags=__
- B
packet|codec_type=video|stream_index=0|pts=12800|pts_time=1.041667|dts=10240|dts_time=0.833333|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=3455|pos=42549|flags=_D
- P
packet|codec_type=video|stream_index=0|pts=11776|pts_time=0.958333|dts=10752|dts_time=0.875000|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=848|pos=46004|flags=_D
- B pyramidal ref
packet|codec_type=video|stream_index=0|pts=11264|pts_time=0.916667|dts=11264|dts_time=0.916667|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=655|pos=46852|flags=__
- B
packet|codec_type=video|stream_index=0|pts=12288|pts_time=1.000000|dts=11776|dts_time=0.958333|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=901|pos=47507|flags=_D
- B
packet|codec_type=video|stream_index=0|pts=13312|pts_time=1.083333|dts=12288|dts_time=1.000000|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=5112|pos=48408|flags=KD
 - end 1st edit
packet|codec_type=video|stream_index=0|pts=11772|pts_time=0.958008|dts=10748|dts_time=0.874674|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=5112|pos=48408|flags=K_
- start 2nd edit
packet|codec_type=video|stream_index=0|pts=13820|pts_time=1.124674|dts=11260|dts_time=0.916341|duration=512|duration_time=0.041667|convergence_duration=N/A|convergence_duration_time=N/A|size=1179|pos=53520|flags=__
.....

The implementation basically increases dts by the amounts that stts atom
indicates, and as you can see after the end of the first edit list and
start of the next edit list, it bumps down the dts from 12288 to 10650 to
make the pts linearly increasing from the last non-discarded frame 11264 ->
11772 .

As I understand from DTS semantics in ffmpeg we have two constraints on DTS.
i) DTS should be monotonically increasing  ii) For every packet PTS >= DTS
should be true.

 Just reordering PTS to DTS doesn't work either. For example, here is the
DTS  I get from "ffprobe -show_packets " when I do "pkt->dts =
AV_NOPTS_VALUE" for all packets in mov_read_packet function
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;h=6e80b93271a4f998af6ba1af795d7d7c5d67f5a1;hb=HEAD#l5332
 . Ffmpeg should use update_dts_from_pts function to reoder pts to dts -
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/utils.c;h=76cbff4ef67937499a7c113dcaacb88389e6015e;hb=HEAD#l1005
.

The problem here is that in case of edit lists, we are trying to make the
PTS "not grow" when the packets are being discarded. If we look at the
non-discarded packets in presentation order, in the above example the PTS
is approx. linearly growing 10752, 11264, 11772, with a difference of
frame_duration=512 approx. However there are 3 discarded frames that we are
outputting in between. So, to grow DTS monotonically linearly we need to
grow DTS at a higher frame rate i.e. smaller frame duration.

If we want to make DTS  satisfy the two constraints described above , I can
recompute the DTS based on an assumed constant frame rate, and shift the
DTS so that PTS > DTS always, but this solution wont work for videos with
variable frame rates.
So in short , these are the options available I see for the DTS
i) Keep the change as it is , with non monotonic DTS
ii) Mark DTS=AV_NOPTS_VALUE for discarded frames. Maybe it is right
semantic to have DTS=N/A for some packets. I don't know.
iii) Recompute the DTS as stated above.

Attaching all the 6 patches and the test file.

> [...]
>
> > though not exactly. However if I do the same RAW->MOV->WAV
> > conversion with head, I see that the out-head.wav is shifted to the
> right .
>
> git master gets it totally wrong with mp3 in mov i agree here 100%
>
> can you fix this exactly (not just almost)  ?
>
>
Yes, I think I can fix this as described above by using SKIP_SAMPLES side
data.

> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I do not agree with what you have to say, but I'll defend to the death your
> right to say it. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avutil-frame-Add-a-flag-to-discard-frame-after-decod.patch
Type: text/x-patch
Size: 1670 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-lavf-mov-Add-support-for-edit-list-parsing.patch
Type: text/x-patch
Size: 117703 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-lavc-Add-a-flag-in-AVPacket-to-discard-packet-after-.patch
Type: text/x-patch
Size: 2921 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-avformat-avframe.h-Add-a-flag-in-AVIndexEntry-to-dis.patch
Type: text/x-patch
Size: 1818 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-ffprobe.c-Indicate-decode-but-discard-packets-when-d.patch
Type: text/x-patch
Size: 1045 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-lavf-utils-Support-av_index_search_timestamp-in-case.patch
Type: text/x-patch
Size: 1113 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mov-2elist-bpyramid-1elist-ends-on-bref.mov
Type: video/quicktime
Size: 78460 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160913/71f0ca29/attachment.mov>