[MPlayer-dev-eng] [PATCH] add support of compression algorithm 3 in mkv demuxer

[Aurelien Jacobs]

Diego Biurrun wrote:

> On Fri, May 09, 2008 at 01:48:35PM +0300, Ivan Kalvachev wrote:
> > On Fri, May 9, 2008 at 1:22 PM, Aurelien Jacobs <aurel at gnuage.org>
> > wrote:
> > > Diego Biurrun wrote:
> > >
> > >> On Fri, May 09, 2008 at 04:19:55AM +0400, Evgeniy Stepanov wrote:
> > >> > On Friday 09 May 2008 03:12:42 Aurelien Jacobs wrote:
> > >> > > I'm sorry, but I've put on hold all my mplayer work
> > >> > > (including patches review), until root finally take a
> > >> > > decision about the "Uoti case". So please be patient.
> > >> >
> > >> > Does it mean that you are blocking all the work on demux_mkv
> > >> > and anything else you maintain? If not, I could commit the
> > >> > patch. I see nothing wrong with it.
> > >>
> > >> Careful, Aurelien is on the record for getting "angry" if you do
> > >> so much as add a few consts to "his" code.
> > >
> > > Weren't you the one who stated he wants to avoid flames ?
> > > So I will try to avoid flames, and just state it once and clearly,
> > > for those who may have a doubt:
> > >  I have nothing against one adding a few consts to "my" code, I
> > > have never complained about this and I will never complain about
> > > this.
> >
> > The commit in question is r26412. The problem with this commit is
> > not that Uoti have added few consts, but that with the very same
> > commit he indented the tables these consts belong.
> > Mixing functional with cosmetic changes.
> 
> You should read Aurelien's message more closely.

I thought my original message was pretty clear. Here is the relevant
quote:
  BTW: you broke rules 6 and 9 in your recent commit to demux_mkv
  (which I maintain). I was pretty hangry seeing this.
If he didn't broke rule 6, I obviously wouldn't have complained about
rule 9.

> > Aurelien, I have mixed feelings about this strike.

It wasn't really intended to be a strike. In fact I just feel not
interested to continue working in this situation.
And as the author of the patch already asked me privately for a
review, I though it wouldn't be fair to him to not explain why
I still didn't reviewed his patch.

> Please make full review of the patch, point what could be improved.

Well OK. Then I will at least make a quick review.

> diff --git a/libmpdemux/demux_mkv.c b/libmpdemux/demux_mkv.c
> index fad06ef..8d8adae 100644
> --- a/libmpdemux/demux_mkv.c
> +++ b/libmpdemux/demux_mkv.c
> @@ -543,6 +543,15 @@ lzo_fail:
>              }
>            *size = dstlen;
>          }
> +      else if (track->encodings[i].comp_algo == 3)
> +        {
> +          *dest = malloc (*size + track->encodings[i].comp_settings_len);
> +          memcpy(*dest, track->encodings[i].comp_settings,
> +                 track->encodings[i].comp_settings_len);

> +          memcpy(*dest + track->encodings[i].comp_settings_len, src, *size);

I think this memcpy can be avoided by initially allocating bigger src
and initially filling it with an offset of comp_settings_len.

Aurel