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

Michael Niedermayer michaelni at gmx.at
Tue Sep 10 01:28:34 CEST 2013


On Mon, Sep 09, 2013 at 10:22:58AM -0700, Vignesh Venkatasubramanian wrote:
> On Thu, Sep 5, 2013 at 4:09 PM, Vignesh Venkatasubramanian
> <vigneshv at google.com> wrote:
> > On Thu, Sep 5, 2013 at 12:33 PM, Nicolas George
> > <nicolas.george at normalesup.org> wrote:
> >> L'octidi 18 fructidor, an CCXXI, Michael Niedermayer a écrit :
> >>> i confirm this fixes the issues
> >>>
> >>> anyone else has any comments about the patchset ?
> >>> nicolas ? you worked with (lib)opus IIRC
> >>
> >> I worked on the decoder, not the encoder. Assuming I understand correctly
> >> what ff_af_queue does, the libopusenc part looks fine (except style:
> >> "uint8_t* side_data" -> "uint8_t *side_data" and "side_data == NULL" ->
> >> "!side_data").
> >>
> >> I can not comment usefully on the Matroska part, though. It looks fine at
> >> first glance, but I would not spot a subtle mistake.
> >>
> >> The "(side_data_size && additional_id == 1)" and "(side_data_size &&
> >> additional_id == 1) || discard_padding" tests look a bit fragile, and may
> >> become a maintenance burden. Maybe checking block_group to see if it was
> >> opened
> >
> > Not sure what you mean by this. But we are opening block_group based on the
> > condition which you want to eliminate.
> >
> >> or setting a flag at the same time would be cleaner.
> >
> > Setting a flag will mean a lot more refactoring. BlockGroup can arise only in 2
> > cases now which are covered by the checks currently. If we need to use flags,
> > then we might need more than one flag (one each for different types of case
> > that could trigger opening a BlockGroup) to make the refactor worth it. If you
> > think it will be cleaner that way, please let me know. I can make the change.
> >
> 
> Michael/Nicolas, any thoughts on this?

i thought too it can be made cleaner but my try didnt end up looking
any simpler/cleaner so ill apply that patch as is.

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

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130910/fc6591b8/attachment.asc>


More information about the ffmpeg-devel mailing list