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

Sasi Inguva isasi at google.com
Sun Sep 18 01:14:24 EEST 2016


On Thu, Sep 15, 2016 at 7:46 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Thu, Sep 15, 2016 at 01:08:40PM -0700, Sasi Inguva wrote:
> > On Wed, Sep 14, 2016 at 1:57 PM, Michael Niedermayer
> <michael at niedermayer.cc
> > > wrote:
> >
> > > On Wed, Sep 14, 2016 at 12:20:52AM -0700, Sasi Inguva wrote:
> > > > On Tue, Sep 13, 2016 at 4:39 PM, Sasi Inguva <isasi at google.com>
> wrote:
> > > >
> > > > > 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.
> > >
> > > ok
> > >
> > > I have put my foot in my mouth here. There is no way I can add
> > AV_PKT_SIDE_DATA_SKIP_SAMPLES to each packet, unless I add a new field
> > "skip_samples" in AVIndexEntry and populate it in mov_fix_index function.
> > Adding another entry to AVIndexEntry seems contentious because we want to
> > keep the AVIndexEntry as compact as possible, I presume. So what I wil do
> > for now is just set st->skip_samples field in mov_fix_index function for
> > the first audio edit list. So at least single edit list audio cases ,will
> > be correct. In multiple edit list case, the the first edit will be
> > correctly output but the second audio edit might have some samples extra.
> > Also note that, there is no mechanism for doing "discard_padding" too, in
> > MOV demuxer. If we have skip_samples and discard_padding fields in
> > AVIndexEntry, then we can implement both of them correctly.
> >
> > With the attached patch the test that you have showed, comparing
> > out-ingu.wav and out-mp3.wav - are  exactly equivalent.
>
> yes, the patch also improves some audio sync cases that where bad
> but it breaks audio sync with faac
>
> ./ffmpeg-ref -f u8 -ar 16000 -ac 1 -i sync_audio.raw -acodec libfaac
> test-ref.mov
> ./ffmpeg_g -f u8 -ar 16000 -ac 1 -i sync_audio.raw -acodec libfaac test.mov
> ./ffmpeg-ref -i test-ref.mov test-ref.mov.wav
> ./ffmpeg_g -i test.mov test.mov.wav
>
> Yes. This fails because we inject skip_samples specifically only for
libfaac inside aac decoder http://git.videolan.or
g/?p=ffmpeg.git;a=blob;f=libavcodec/aacdec_template.c;h=883e
d527f7a756c7d66e44e14b9d9a8f4de28e30;hb=HEAD#l2287 . And the demuxer is
skipping the first packet ( 1024 samples ) by using the discard flag.  so
we are skipping 2048 samples or 2 packets in total.

This is a general problem of both demuxer and decoder trying to inject the
same skip_samples data. OPUS decoder also injects skip_samples by itself
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavc
odec/libopusdec.c;h=acc62f14d86fc9d369dc81c9a266fc67aa0d73c4;hb=HEAD#l113 (
though i guess MOV doesn't support OPUS ). For MOV at least the spec says -
use container to specify skip_samples https://developer
.apple.com/library/content/documentation/QuickTime/QTFF/
QTFFAppenG/QTFFAppenG.html . To fix this, what I've done here is to ignore
the skip_samples set by decoder when we use DISCARD flag and reset it to
what was set by the demuxer.


>
> [...]
> > @@ -2756,6 +2757,343 @@ static int mov_read_sbgp(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
> >      return pb->eof_reached ? AVERROR_EOF : 0;
> >  }
> >
> > +/**
> > + * Get ith edit list entry (media time, duration).
> > + */
> > +static int get_edit_list_entry(const MOVStreamContext *msc,
> > +                               unsigned int edit_list_index,
> > +                               int64_t *edit_list_media_time,
> > +                               int64_t *edit_list_duration,
> > +                               int64_t global_timescale)
> > +{
> > +    if (edit_list_index == msc->elst_count) {
> > +        return 0;
> > +    }
> > +    *edit_list_media_time = msc->elst_data[edit_list_index].time;
> > +    *edit_list_duration = msc->elst_data[edit_list_index].duration;
> > +    /* duration is in global timescale units;convert to msc timescale */
> > +    *edit_list_duration = av_rescale(*edit_list_duration,
> msc->time_scale,
> > +                                     global_timescale);
>
> global_timescale can be 0 here leading to division by 0
>

Added an assert for global_timescale > 0

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker.
> User
> questions about the command line tools should be sent to the ffmpeg-user
> ML.
> And questions about how to use libav* should be sent to the libav-user ML.
>
> _______________________________________________
> 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: 0004-lavf-mov-Add-support-for-edit-list-parsing.patch
Type: text/x-patch
Size: 120344 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160917/445d75c5/attachment.bin>


More information about the ffmpeg-devel mailing list