[FFmpeg-devel] [PATCH] Split common code for MLP to mlp.[ch].

[Michael Niedermayer]

On Wed, Aug 13, 2008 at 10:13:04AM -0300, Ramiro Polla wrote:
> Hello,
> 
> I replied to this e-mail with two sets of big patches and it was too
> big for the ML. I'm sending them again separately.
> 
> On Wed, Aug 13, 2008 at 9:38 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > SSOn Wed, Aug 13, 2008 at 12:38:07AM -0300, Ramiro Polla wrote:
> >> Hello,
> >>
> >> Attached patches remove common code from the MLP encoder in the SoC
> >> repo and the decoder (and the parser) and puts it in common files.
> >>
> >> To commit, the split to mlp.[ch] would be made with svn cp from
> >> mlpdec.c. I attached them as the resulting file to easy reviewing (or
> >> is it better to have the diffs to mlpdec.c?).
> >
> > you could just attach both they are both usefull for reviewing
> 
> I'm attaching them as diffs again. One for each file, but to be
> committed all at once. What changed is that I removed the duplicate
> comment for both checksum functions in mlp.h. Also I copied with the
> */ in the same line, since that's how it is in mlpdec.
> 
> I'll post a followup to this reply with the second set of patches.
> 
> > as is my awnser to the patch is
> > its ok under the condition that it just moves stuff around (hard to
> > tell as the new files are not attached as diff)
> > and that the issue pointed out below is fixed
> >
> > [...]
> >
> >> } ChannelParams;
> >>
> >> /** Tables defining the Huffman codes.
> >>  *  There are three entropy coding methods used in MLP (four if you count
> >>  *  "none" as a method). These use the same sequences for codes starting with
> >>  *  00 or 01, but have different codes starting with 1. */
> >>
> >> extern const uint8_t ff_mlp_huffman_tables[3][18][2];
> >
> > the */ should be on a line of its own so adding more text does not
> > need to change the previous last line
> 
> Do you prefer if I change all comments to end with */ in the next
> line? I'll do that as a separate commit then.

yes and commit no need for a patch!


> 
> > also there should be more \n above than below, as is the comment is exactly
> > between 2 things and that looks a little odd. moving the */ wil likely be
> > enough though when theres no empty line after it..
> 
> Maybe it's just because I'm tired, but I didn't quite understand this
> part. More \n above than below what?

func()

/**
 * jhfdhf
 */

func2()

looks odd
but following looks better:

func()

/**
 * jhfdhf
 */
func2()

or

func()


/**
 * jhfdhf
 */

func2()
-------------

and patch ok

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080813/0a6455e6/attachment.pgp>