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

[Michael Niedermayer]

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


[...]
> @@ -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

[...]
-- 
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160916/aea3e13b/attachment.sig>