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

Ramiro Polla ramiro.polla
Wed Aug 13 21:43:56 CEST 2008


On Wed, Aug 13, 2008 at 11:11 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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

Applied. And changed comments afterwards.




More information about the ffmpeg-devel mailing list