[FFmpeg-devel] [PATCH] opus/matroska: Adding support for End Trimming in demuxer/decoder

[Vignesh Venkatasubramanian]

On Tue, Sep 3, 2013 at 2:03 PM, Vignesh Venkatasubramanian
<vigneshv at google.com> wrote:
> On Tue, Sep 3, 2013 at 12:12 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> On Tue, Sep 03, 2013 at 10:46:34AM -0700, Vignesh Venkatasubramanian wrote:
>>> On Mon, Sep 2, 2013 at 4:30 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>> > On Fri, Aug 30, 2013 at 11:25:12AM -0700, Vignesh Venkatasubramanian wrote:
>>> >> Implementing support for end trimming Opus in Matroska by making
>>> >> use of the DiscardPadding value from the container and discarding
>>> >> the samples accordingly. With this patch and its companion muxer
>>> >> patch, we can ensure number of samples in == number of samples out
>>> >> for Opus in Matroska.
>>> >>
>>> >> Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
>>> >> ---
>>> >>  libavcodec/utils.c        | 21 +++++++++++++++++++++
>>> >>  libavformat/matroskadec.c | 30 +++++++++++++++++++++++++-----
>>> >>  2 files changed, 46 insertions(+), 5 deletions(-)
>>> >
>>> > this (with the other patched) causes fate to fail
>>>
>>> when you say other patched, do you mean the muxer and encoder patches? or the
>>> memset patch? because i applied all the 4 patches and fate passes for me.
>>>
>>> just to be clear, by 4 patches i mean the patches with following titles:
>>> * avpacket: Fixing side data copy when src == dst
>>> * opus/matroska: Adding support for DiscardPadding in muxer
>>> * lavcodec: Adding support for End Trimming in Opus encoder
>>> * opus/matroska: Adding support for End Trimming in demuxer/decoder
>>>
>>> could you please check once again? thanks.
>>
>> the following 2 patches are sufficient to make it break fate:
>>
>> btw, it would be easier if you had a git repo with the patches so
>> i could test exactly the code you test
>> the avpacket patch also has been applied already
>>
>> commit d2ccbc879b69be7ead2061863e0ae84d9da07b32
>> Author: Vignesh Venkatasubramanian <vigneshv at google.com>
>> Date:   Fri Aug 30 11:26:37 2013 -0700
>>
>>     opus/matroska: Adding support for DiscardPadding in muxer
>>
>>     Support for end trimming Opus in Matroska is implemented by using
>>     the DiscardPadding container element in the Block data. The last
>>     chunk is stored as a Block instead of SimpleBlock and the
>>     trimming information is stored and used to discard samples that
>>     were padded by the Opus codec. This patch adds support for muxing
>>     DiscardPadding element into the container with appropriate value.
>>     Matroska spec for the DiscardPadding element can be found here:
>>     http://matroska.org/technical/specs/index.html#DiscardPadding
>>
>>     Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
>>     Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>>
>> commit 75c84127c36972b1d0d65188ff90e07a51a8a11d
>> Author: Vignesh Venkatasubramanian <vigneshv at google.com>
>> Date:   Fri Aug 30 11:25:12 2013 -0700
>>
>>     opus/matroska: Adding support for End Trimming in demuxer/decoder
>>
>>     Implementing support for end trimming Opus in Matroska by making
>>     use of the DiscardPadding value from the container and discarding
>>     the samples accordingly. With this patch and its companion muxer
>>     patch, we can ensure number of samples in == number of samples out
>>     for Opus in Matroska.
>>
>>     Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
>>     Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>>
>>
>
> i am able to reproduce the fate failure now. the fate passes if i do configure
> in a non trivial way (including a bunch of libraries, etc). but it
> fails if i merely
> run configure without any option. i will look into it.
>

I sent a patch just now which fixes the fate failure on this (and related)
patches. Can you please check now ? thanks!

FYI the new patch i am talking about is titled:
lavf/utils: Fixing skip samples to adhere to side data format


>>
>>
>>>
>>> > --- ./tests/ref/fate/exif-image-embedded    2013-09-02 12:10:30.045471774 +0200
>>> > +++ tests/data/fate/exif-image-embedded 2013-09-03 01:29:07.774481269 +0200
>>> > @@ -34,7 +34,7 @@
>>> >  pkt_pos=16292
>>> >  pkt_size=417
>>> >  sample_fmt=s16p
>>> > -nb_samples=47
>>> > +nb_samples=-11006
>>> >  channels=2
>>> >  channel_layout=stereo
>>> >  [/FRAME]
>>> >
>>> > [...]
>>> > --
>>> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>> >
>>> > The real ebay dictionary, page 2
>>> > "100% positive feedback" - "All either got their money back or didnt complain"
>>> > "Best seller ever, very honest" - "Seller refunded buyer after failed scam"
>>>
>>>
>>>
>>> --
>>> Vignesh Venkat | Software Engineer | vigneshv at google.com | 650-861-1330
>>>
>>
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Republics decline into democracies and democracies degenerate into
>> despotisms. -- Aristotle