[FFmpeg-devel] [PATCH] Split common code for MLP to mlp.[ch].
Wed Aug 13 16:11:13 CEST 2008
On Wed, Aug 13, 2008 at 10:13:04AM -0300, Ramiro Polla wrote:
> 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;
> > 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?
but following looks better:
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
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel